2:22 a.m.
Royce Lv has uploaded a new change for review.
Change subject: fix circularr reference of storage pool
......................................................................
fix circularr reference of storage pool
BZ#806774:storage domain uuid can't be reused after storage pool destroyed
Change-Id: Ie2993b0ad8d85898ca86008f7056973ff493e8de
---
M vdsm/storage/hsm.py
M vdsm/storage/sp.py
M vdsm/storage/task.py
3 files changed, 12 insertions(+), 3 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/42/3242/1
--
To view, visit http://gerrit.ovirt.org/3242
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie2993b0ad8d85898ca86008f7056973ff493e8de
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
5:33 a.m.
New subject: Change in vdsm[master]: fix circular reference of storage pool
Royce Lv has posted comments on this change.
Change subject: fix circular reference of storage pool
......................................................................
Patch Set 3: Verified
--
To view, visit http://gerrit.ovirt.org/3242
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2993b0ad8d85898ca86008f7056973ff493e8de
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
5:24 a.m.
New subject: Change in vdsm[master]: fix circular reference of storage pool
Ayal Baron has posted comments on this change.
Change subject: fix circular reference of storage pool
......................................................................
Patch Set 3:
And I assume you also checked that it solved the circular dependency problem in which case
you can mark the patch as verified.
--
To view, visit http://gerrit.ovirt.org/3242
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2993b0ad8d85898ca86008f7056973ff493e8de
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
5:22 a.m.
New subject: Change in vdsm[master]: fix circular reference of storage pool
Royce Lv has posted comments on this change.
Change subject: fix circular reference of storage pool
......................................................................
Patch Set 3:
Ayal,thanks for reviewing.Tested upgradeStoragePool,see callback called,it works~
--
To view, visit http://gerrit.ovirt.org/3242
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2993b0ad8d85898ca86008f7056973ff493e8de
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
2:46 a.m.
New subject: Change in vdsm[master]: fix circular reference of storage pool
Ayal Baron has posted comments on this change.
Change subject: fix circular reference of storage pool
......................................................................
Patch Set 3: Looks good to me, but someone else must approve
Did you test that domain upgrade actually works after this change?
--
To view, visit http://gerrit.ovirt.org/3242
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2993b0ad8d85898ca86008f7056973ff493e8de
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
5:04 p.m.
New subject: Change in vdsm[master]: fix circular reference of storage pool
Ayal Baron has posted comments on this change.
Change subject: fix circular reference of storage pool
......................................................................
Patch Set 2: I would prefer that you didn't submit this
Question is can't we just pass a weakref to self._upgradeCallback to avoid creation of
the circular reference to begin with.
e.g.
self.domainMonitor.onDomainConnectivityStateChange.register(weakref.ref(self._upgradeCallback))
(but need to see how to handle unregister in this case).
On the face of it, it looks much simpler this way...
--
To view, visit http://gerrit.ovirt.org/3242
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2993b0ad8d85898ca86008f7056973ff493e8de
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
9:53 a.m.
New subject: Change in vdsm[master]: fix circular reference of storage pool
Royce Lv has posted comments on this change.
Change subject: fix circular reference of storage pool
......................................................................
Patch Set 2:
Hi Saggi,
I'm sorry I can't understand what do you mean by "could't it be solved
by weakref?".
1.If you mean this is not a problem because we have storage domain weakref cache.I
think it's not true,you can check bug 806774 and reproduce it by checking storage pool
refcount after storagepool create.circular reference means storage pool's partial
function refers to itself.the sdc won't help this problem.
2.If you mean using weakref in the partial function to solve this problem, I guess this
may look wired write in that way.and we need to do a lot of extra changes,I think it maybe
the clearest way to fix this.
please check sp.py(__init__) and bug 806774 for details of this patch.Thanks for your
review.Would you please explain us more detailly?
--
To view, visit http://gerrit.ovirt.org/3242
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2993b0ad8d85898ca86008f7056973ff493e8de
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
9:09 a.m.
New subject: Change in vdsm[master]: fix circular reference of storage pool
Saggi Mizrahi has posted comments on this change.
Change subject: fix circular reference of storage pool
......................................................................
Patch Set 2: I would prefer that you didn't submit this
Couldn't this be solved with weakrefs?
--
To view, visit http://gerrit.ovirt.org/3242
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2993b0ad8d85898ca86008f7056973ff493e8de
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
11:39 a.m.
Adam Litke has posted comments on this change.
Change subject: fix circularr reference of storage pool
......................................................................
Patch Set 1: I would prefer that you didn't submit this
(2 inline comments)
Thanks for the patch! Please add some additional comments and improve the commit message
as I suggest in the embedded comments.
....................................................
Commit Message
Line 11: Change-Id: Ie2993b0ad8d85898ca86008f7056973ff493e8de
In this commit message, please explain why your change fixes the problem. It should be
clear to the patch reviewer what you are changing. To me, it is not obvious why changing
the pool upgrade callback would fix the problem.
....................................................
File vdsm/storage/task.py
Line 321: del self.cmd
Please add a comment to explain why an explicit del is required.
--
To view, visit http://gerrit.ovirt.org/3242
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2993b0ad8d85898ca86008f7056973ff493e8de
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
4:03 p.m.
New subject: Change in vdsm[master]: fix circular reference of storage pool
Dan Kenigsberg has submitted this change and it was merged.
Change subject: fix circular reference of storage pool
......................................................................
fix circular reference of storage pool
master storage domain can not be freed because storage pool refers to it.
storage pool can't be freed because upgradeCallback is a partial function
which refers to itself, making this a circular reference,
using proxy of storage pool instead of real ref in upgradeCallback
makes them freeable.
Change-Id: Ie2993b0ad8d85898ca86008f7056973ff493e8de
Signed-off-by: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
---
M vdsm/storage/sp.py
1 file changed, 2 insertions(+), 1 deletion(-)
Approvals:
Ayal Baron: Looks good to me, but someone else must approve
Royce Lv: Verified
Saggi Mizrahi: Looks good to me, but someone else must approve
Dan Kenigsberg: Looks good to me, approved
--
To view, visit http://gerrit.ovirt.org/3242
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie2993b0ad8d85898ca86008f7056973ff493e8de
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
4:03 p.m.
New subject: Change in vdsm[master]: fix circular reference of storage pool
Dan Kenigsberg has posted comments on this change.
Change subject: fix circular reference of storage pool
......................................................................
Patch Set 3: Looks good to me, approved
--
To view, visit http://gerrit.ovirt.org/3242
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2993b0ad8d85898ca86008f7056973ff493e8de
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
4:02 p.m.
New subject: Change in vdsm[master]: fix circular reference of storage pool
Saggi Mizrahi has posted comments on this change.
Change subject: fix circular reference of storage pool
......................................................................
Patch Set 3: Looks good to me, but someone else must approve
--
To view, visit http://gerrit.ovirt.org/3242
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2993b0ad8d85898ca86008f7056973ff493e8de
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
4421
days inactive
4435
days old
vdsm-patches@lists.fedorahosted.org
11 comments
5 participants
participants (5)
-
abaron＠redhat.com -
agl＠us.ibm.com -
Dan Kenigsberg -
lvroyce＠linux.vnet.ibm.com -
smizrahi＠redhat.com