Yaniv Bronhaim has uploaded a new change for review.
Change subject: prepare volume instead of prepare blockSD ......................................................................
prepare volume instead of prepare blockSD
Bug-Id: https://bugzilla.redhat.com/show_bug.cgi?id=851837
change the call of activateVolume to llprepare volume, and teardown instead of deactiveVolume, which sperates lv syscall operation to only verifies that the lv is in used as part of the volume. If the sd is in use by the checked volume, we want to keep ref counter to sign this volume, and when we teardown it, we verify that there are no references to the same sd before deactivate it.
Change-Id: I25b9c260a7de5f883420a5322f90195b3379e80e Signed-off-by: Yaniv Bronhaim ybronhei@redhat.com --- M vdsm/storage/image.py 1 file changed, 2 insertions(+), 6 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/50/8050/1
diff --git a/vdsm/storage/image.py b/vdsm/storage/image.py index d004046..97112dc 100644 --- a/vdsm/storage/image.py +++ b/vdsm/storage/image.py @@ -383,8 +383,7 @@ chain.insert(0, tmplVolume)
# Activating the volumes - sdCache.produce(sdUUID).activateVolumes( - volUUIDs=[vol.volUUID for vol in chain]) + sdCache.produce(sdUUID).llPrepare()
return chain
@@ -392,11 +391,8 @@ chain = self.getChain(sdUUID, imgUUID, volUUID)
# Deactivating the volumes - sdCache.produce(sdUUID).deactivateVolumes( - volUUIDs=[vol.volUUID for vol in chain]) - # Do not deactivate the template yet (might be in use by an other vm) - # TODO: reference counting to deactivate when unused + sdCache.produce(sdUUID).teardown(sdUUID, volUUIDs=[vol.volUUID for vol in chain])
def __templateRelink(self, destDom, imgUUID, volUUID): """
-- To view, visit http://gerrit.ovirt.org/8050 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange Gerrit-Change-Id: I25b9c260a7de5f883420a5322f90195b3379e80e Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim ybronhei@redhat.com
Saggi Mizrahi has posted comments on this change.
Change subject: prepare volume instead of prepare blockSD ......................................................................
Patch Set 2: I would prefer that you didn't submit this
Please use prepare and not llprepare
-- To view, visit http://gerrit.ovirt.org/8050 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I25b9c260a7de5f883420a5322f90195b3379e80e Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com
Eduardo has posted comments on this change.
Change subject: prepare volume instead of prepare blockSD ......................................................................
Patch Set 3: I would prefer that you didn't submit this
(4 inline comments)
.................................................... Commit Message Line 7: prepare volume instead of prepare blockSD Line 8: Line 9: Bug-Id: https://bugzilla.redhat.com/show_bug.cgi?id=851837 Line 10: Line 11: change the call of activateVolume to llprepare volume, and outdated reference to llprepare. Line 12: teardown instead of deactiveVolume, which sperates lv Line 13: syscall operation to only verifies that the lv is in used as Line 14: part of the volume. If the sd is in use by the checked volume, Line 15: we want to keep ref counter to sign this volume, and when we
Line 14: part of the volume. If the sd is in use by the checked volume, Line 15: we want to keep ref counter to sign this volume, and when we Line 16: teardown it, we verify that there are no references to the same Line 17: sd before deactivate it. Line 18: Please clarify the idea. Line 19: Change-Id: I25b9c260a7de5f883420a5322f90195b3379e80e
.................................................... File vdsm/storage/image.py Line 382: if tmplVolume: Line 383: chain.insert(0, tmplVolume) Line 384: Line 385: # Activating the volumes Line 386: sdCache.produce(sdUUID).prepare() SD.prepare() function does not exist! Line 387: Line 388: return chain Line 389: Line 390: def teardown(self, sdUUID, imgUUID, volUUID=None):
Line 391: chain = self.getChain(sdUUID, imgUUID, volUUID) Line 392: Line 393: # Deactivating the volumes Line 394: # Do not deactivate the template yet (might be in use by an other vm) Line 395: sdCache.produce(sdUUID).teardown(sdUUID, volUUIDs=[vol.volUUID for vol in chain]) SD.teardown() function does not exist! Line 396: Line 397: def __templateRelink(self, destDom, imgUUID, volUUID): Line 398: """ Line 399: Relink all hardlinks of the template 'volUUID' in all VMs based on it.
-- To view, visit http://gerrit.ovirt.org/8050 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I25b9c260a7de5f883420a5322f90195b3379e80e Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com
Saggi Mizrahi has posted comments on this change.
Change subject: prepare volume instead of prepare blockSD ......................................................................
Patch Set 6: Looks good to me, but someone else must approve
-- To view, visit http://gerrit.ovirt.org/8050 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I25b9c260a7de5f883420a5322f90195b3379e80e Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com
Ayal Baron has posted comments on this change.
Change subject: prepare volume instead of prepare blockSD ......................................................................
Patch Set 6: I would prefer that you didn't submit this
(2 inline comments)
.................................................... File vdsm/storage/image.py Line 378: Line 379: # Adding the image template to the chain Line 380: tmplVolume = chain[0].getParentVolume() Line 381: Line 382: tmplVolume.prepare() if no template this will be None.prepare() I'm not sure how this worked for you exactly Line 383: Line 384: if tmplVolume: Line 385: chain.insert(0, tmplVolume) Line 386:
Line 389: def teardown(self, sdUUID, imgUUID, volUUID=None): Line 390: chain = self.getChain(sdUUID, imgUUID, volUUID) Line 391: Line 392: tmplVolume = chain[0].getParentVolume() Line 393: tmplVolume.teardown(sdUUID, tmplVolume.volUUID) again, need to check there is a template Line 394: Line 395: def __templateRelink(self, destDom, imgUUID, volUUID): Line 396: """ Line 397: Relink all hardlinks of the template 'volUUID' in all VMs based on it.
-- To view, visit http://gerrit.ovirt.org/8050 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I25b9c260a7de5f883420a5322f90195b3379e80e Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsland@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com
Saggi Mizrahi has posted comments on this change.
Change subject: Prepare each vm volumes instead of prepare blockSD ......................................................................
Patch Set 7: I would prefer that you didn't submit this
(7 inline comments)
.................................................... File vdsm/storage/image.py Line 348: if tmplVolume: Line 349: chain.insert(0, tmplVolume) Line 350: Line 351: # perform prepare on top volume (this will recusivly prepare all chain) Line 352: if chain[-1]: I think you meant, if len(chain) >= 1: Line 353: chain[-1].prepare() Line 354: self.log.debug("MyPrepare: producing prepare on top chain") Line 355: else: Line 356: self.log.debug("MyPrepare: no chain exist - weird")
Line 350: Line 351: # perform prepare on top volume (this will recusivly prepare all chain) Line 352: if chain[-1]: Line 353: chain[-1].prepare() Line 354: self.log.debug("MyPrepare: producing prepare on top chain") What is MyPrepare? Line 355: else: Line 356: self.log.debug("MyPrepare: no chain exist - weird") Line 357: Line 358: sdCache.produce(sdUUID).activateVolumes(
Line 352: if chain[-1]: Line 353: chain[-1].prepare() Line 354: self.log.debug("MyPrepare: producing prepare on top chain") Line 355: else: Line 356: self.log.debug("MyPrepare: no chain exist - weird") Warning Line 357: Line 358: sdCache.produce(sdUUID).activateVolumes( Line 359: volUUIDs=[vol.volUUID for vol in chain]) Line 360: return chain
Line 361: Line 362: def teardown(self, sdUUID, imgUUID, volUUID=None): Line 363: chain = self.getChain(sdUUID, imgUUID, volUUID) Line 364: Line 365: if chain[-1]: Again Line 366: chain[-1].teardown(sdUUID, chain[-1].volUUID) Line 367: self.log.debug("MyTearDown: doing that") Line 368: else: Line 369: self.log.debug("MyTearDown: something wrong..")
Line 363: chain = self.getChain(sdUUID, imgUUID, volUUID) Line 364: Line 365: if chain[-1]: Line 366: chain[-1].teardown(sdUUID, chain[-1].volUUID) Line 367: self.log.debug("MyTearDown: doing that") Is this for testing? Line 368: else: Line 369: self.log.debug("MyTearDown: something wrong..") Line 370: Line 371: # Deactivating the volumes
Line 365: if chain[-1]: Line 366: chain[-1].teardown(sdUUID, chain[-1].volUUID) Line 367: self.log.debug("MyTearDown: doing that") Line 368: else: Line 369: self.log.debug("MyTearDown: something wrong..") warning Line 370: Line 371: # Deactivating the volumes Line 372: sdCache.produce(sdUUID).deactivateVolumes( Line 373: volUUIDs=[vol.volUUID for vol in chain])
.................................................... File vdsm/storage/resourceFactories.py Line 70: lvvg = resourceName.split('/') Line 71: return VolumeWatcher(lvvg[0], lvvg[1]) Line 72: except: Line 73: log.error("error creating resource name: %s", resourceName) Line 74: unrelated Line 75: class LvmActivation(object): Line 76: """ Line 77: Represents activation state of the LV. Line 78: When the resource is created (i.e. the LV is being activated)
-- To view, visit http://gerrit.ovirt.org/8050 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I25b9c260a7de5f883420a5322f90195b3379e80e Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsland@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com
Douglas Schilling Landgraf has posted comments on this change.
Change subject: Prepare each vm volumes instead of prepare blockSD ......................................................................
Patch Set 7: I would prefer that you didn't submit this
agreed with Saggi's comments.
-- To view, visit http://gerrit.ovirt.org/8050 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I25b9c260a7de5f883420a5322f90195b3379e80e Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsland@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com
Yaniv Bronhaim has abandoned this change.
Change subject: Prepare each vm volumes instead of prepare blockSD ......................................................................
Patch Set 7: Abandoned
-- To view, visit http://gerrit.ovirt.org/8050 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: abandon Gerrit-Change-Id: I25b9c260a7de5f883420a5322f90195b3379e80e Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsland@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com
vdsm-patches@lists.fedorahosted.org
-
abaron＠redhat.com -
Douglas Schilling Landgraf -
ewarszaw＠redhat.com -
smizrahi＠redhat.com -
ybronhei＠redhat.com