Change in vdsm[master]: getIsoList() returns dict with files metadata
Federico Simoncelli has posted comments on this change. Change subject: getIsoList() returns dict with files metadata .. Patch Set 7: Code-Review+1 Nice! This works well with the recent db/ui changes! -- To view, visit http://gerrit.ovirt.org/19544 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0b28488a81cec756188ed763e4489b8a39b2b05d Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Eduardo ewars...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: hsm: [wip] protect deleteImage with the spm lock
Federico Simoncelli has uploaded a new change for review. Change subject: hsm: [wip] protect deleteImage with the spm lock .. hsm: [wip] protect deleteImage with the spm lock Change-Id: Iac5b4b0f71de6a12de34513d4fafb295b701306c Signed-off-by: Federico Simoncelli fsimo...@redhat.com --- M vdsm/storage/hsm.py M vdsm/storage/sp.py 2 files changed, 8 insertions(+), 2 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/95/19795/1 diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py index 4f5a8a7..c4350d6 100644 --- a/vdsm/storage/hsm.py +++ b/vdsm/storage/hsm.py @@ -1506,7 +1506,7 @@ force parameter is deprecated and not evaluated. # vars.task.setDefaultException(se.ChangeMeError(%s % args)) -self.getPool(spUUID) # Validates that the pool is connected. WHY? +pool = self.getPool(spUUID) # Validates pool connection. WHY? dom = sdCache.produce(sdUUID=sdUUID) vars.task.getExclusiveLock(STORAGE, imgUUID) @@ -1541,7 +1541,7 @@ if fakeTUUID: tParams = dom.produceVolume(imgUUID, fakeTUUID).\ getVolumeParams() -dom.deleteImage(sdUUID, imgUUID, volsByImg) +pool.deleteImage(dom, imgUUID, volsByImg) # This is a hack to keep the interface consistent # We currently have race conditions in delete image, to quickly fix # this we delete images in the synchronous state. This only works diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py index 8bab500..7242006 100644 --- a/vdsm/storage/sp.py +++ b/vdsm/storage/sp.py @@ -2020,6 +2020,12 @@ sdCache.produce(sdUUID).produceVolume(imgUUID, volUUID).delete( postZero=postZero, force=force) +def deleteImage(self, domain, imgUUID, volsByImg): + +TODO: describe method + +domain.deleteImage(domain.sdUUID, imgUUID, volsByImg) + def setMaxHostID(self, spUUID, maxID): Set maximum host ID -- To view, visit http://gerrit.ovirt.org/19795 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iac5b4b0f71de6a12de34513d4fafb295b701306c Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: hsm: [wip] protect deleteImage with the spm lock
Federico Simoncelli has posted comments on this change. Change subject: hsm: [wip] protect deleteImage with the spm lock .. Patch Set 1: (3 comments) File vdsm/storage/hsm.py Line 1540: else: Line 1541: if fakeTUUID: Line 1542: tParams = dom.produceVolume(imgUUID, fakeTUUID).\ Line 1543: getVolumeParams() Line 1544: pool.deleteImage(dom, imgUUID, volsByImg) Even though this is the bare minimum, we can try to investigate better the comment here below and move this block (lines 1534-1556) to sp.py. Line 1545: # This is a hack to keep the interface consistent Line 1546: # We currently have race conditions in delete image, to quickly fix Line 1547: # this we delete images in the synchronous state. This only works Line 1548: # because Engine does not send two requests at a time. This hack is File vdsm/storage/sp.py Line 2019: for volUUID in volumes: Line 2020: sdCache.produce(sdUUID).produceVolume(imgUUID, volUUID).delete( Line 2021: postZero=postZero, force=force) Line 2022: Line 2023: def deleteImage(self, domain, imgUUID, volsByImg): We can use the domain object here because we don't schedule it as a task. Might worth commenting. I'm also fine with using sdUUID and produce the domain here again. Line 2024: Line 2025: TODO: describe method Line 2026: Line 2027: domain.deleteImage(domain.sdUUID, imgUUID, volsByImg) Line 2023: def deleteImage(self, domain, imgUUID, volsByImg): Line 2024: Line 2025: TODO: describe method Line 2026: Line 2027: domain.deleteImage(domain.sdUUID, imgUUID, volsByImg) Why do we need to pass the sdUUID? The domain object says it all. Also volsByImg are redundant but I understand that getting rid of them would require a larger refactoring (e.g lvm commands can be applied by to multiple lvs using tags, eg: lvremove @IU_imgUUID, provided that we're able to filter also by vg). Line 2028: Line 2029: def setMaxHostID(self, spUUID, maxID): Line 2030: Line 2031: Set maximum host ID -- To view, visit http://gerrit.ovirt.org/19795 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iac5b4b0f71de6a12de34513d4fafb295b701306c Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: hsm: [wip] protect deleteImage with the spm lock
Federico Simoncelli has posted comments on this change. Change subject: hsm: [wip] protect deleteImage with the spm lock .. Patch Set 1: Verified+1 The SPM locking is used. -- To view, visit http://gerrit.ovirt.org/19795 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iac5b4b0f71de6a12de34513d4fafb295b701306c Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vm: add the transient disk support
Federico Simoncelli has posted comments on this change. Change subject: vm: add the transient disk support .. Patch Set 8: (1 comment) File vdsm/vm.py Line 3318:volume %s, drive['volumeID'], exc_info=True) Line 3319: finally: Line 3320: os.close(transientHandle) Line 3321: Line 3322: drive['path'] = transientPath drive['format'] = 'cow' Line 3323: Line 3324: def _removeTransientDisk(self, drive): Line 3325: if getattr(drive, 'transient', False): Line 3326: os.unlink(drive.path) -- To view, visit http://gerrit.ovirt.org/18326 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3dfec35e324c47d8c86a965947e3ae4ae48c7524 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepa...@linux.vnet.ibm.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Deepak C Shetty deepa...@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Itamar Heim ih...@redhat.com Gerrit-Reviewer: Liron Ar lara...@redhat.com Gerrit-Reviewer: Timothy Asir tjeya...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vm: add the transient disk support
Federico Simoncelli has posted comments on this change. Change subject: vm: add the transient disk support .. Patch Set 8: (1 comment) File lib/vdsm/tool/transient.py Line 70: # one access has been found, fuser returns zero. we can discard the Line 71: # return code. Line 72: open_transient_images = set(x.rsplit(:, 1)[0] for x in cmd_out) Line 73: Line 74: for image_path in transient_images - open_transient_images: absolute path vs relative path Line 75: # NOTE: This could cause a race with the creation of a virtual Line 76: # machine with a transient disk (if vdsm is running). Line 77: try: Line 78: os.unlink(image_path) -- To view, visit http://gerrit.ovirt.org/18326 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3dfec35e324c47d8c86a965947e3ae4ae48c7524 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepa...@linux.vnet.ibm.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Deepak C Shetty deepa...@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Itamar Heim ih...@redhat.com Gerrit-Reviewer: Liron Ar lara...@redhat.com Gerrit-Reviewer: Timothy Asir tjeya...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vm: shared attribute backward compatibility
Federico Simoncelli has posted comments on this change. Change subject: vm: shared attribute backward compatibility .. Patch Set 7: Verified+1 Partially verified. I didn't spot any error on the shared attribute but migration fails because the path of the disks path have changed between ovirt-3.3 and master (/var/run...). I also tested the exclusive case (no regressions here, lease acquired). -- To view, visit http://gerrit.ovirt.org/19509 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4d377635b0687baccc69b203cb3fbe8dbf573169 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm: add support for ram/vram on QXL device
Federico Simoncelli has posted comments on this change. Change subject: vdsm: add support for ram/vram on QXL device .. Patch Set 7: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/19361 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I71cc51acf22dbce66373009f0c6ec8e2022a4f87 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: Better Saggi bettersa...@gmail.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Peter V. Saveliev p...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vm: add the transient disk support
Federico Simoncelli has posted comments on this change. Change subject: vm: add the transient disk support .. Patch Set 7: (7 comments) File lib/vdsm/tool/transient.py Line 29: SELINUX_VIRT_IMAGE_LABEL = system_u:object_r:virt_image_t:s0 Line 30: Line 31: _fuser = CommandPath( Line 32: fuser, Line 33: /sbin/fuser, # Fedora, EL6 I can try to unify them later. Given where the other one is defined (storage instead of utils) and the changes that I'd need, it's too risky. Line 34: ) Line 35: Line 36: Line 37: @expose(setup-transient-repository) File vdsm/vm.py Line 2492: with self._volPrepareLock: Line 2493: for drive in drives: Line 2494: try: Line 2495: self._removeTransientDisk(drive) Line 2496: except: Do you really want to risk another teardownVolumePath issue? Catching an OSError would be fiddling with the (current) internal implementation of _removeTransientDisk. Line 2497: self.log.error(Drive transient volume deletion failed Line 2498:for drive %s, drive, exc_info=True) Line 2499: try: Line 2500: self.cif.teardownVolumePath(drive) Line 2493: for drive in drives: Line 2494: try: Line 2495: self._removeTransientDisk(drive) Line 2496: except: Line 2497: self.log.error(Drive transient volume deletion failed Done Line 2498:for drive %s, drive, exc_info=True) Line 2499: try: Line 2500: self.cif.teardownVolumePath(drive) Line 2501: except: Line 3299: qemuImg.FORMAT.RAW Line 3300: ) Line 3301: Line 3302: transientHandle, transientPath = tempfile.mkstemp( Line 3303: dir=constants.P_VDSM_TRANSIENT, Done Line 3304: prefix=%s-%s. % (drive['domainID'], drive['volumeID'])) Line 3305: Line 3306: try: Line 3307: qemuImg.create(transientPath, format=qemuImg.FORMAT.QCOW2, Line 3303: dir=constants.P_VDSM_TRANSIENT, Line 3304: prefix=%s-%s. % (drive['domainID'], drive['volumeID'])) Line 3305: Line 3306: try: Line 3307: qemuImg.create(transientPath, format=qemuImg.FORMAT.QCOW2, Fully transient disk? I think we should just get rid of few validations and checks along the way. Line 3308:backing=drive['path'], backingFormat=driveFormat) Line 3309: os.fchmod(transientHandle, 0660) Line 3310: except: Line 3311: os.unlink(transientPath) # Closing after deletion is correct Line 3694: Line 3695: if vmDrive.hasVolumeLeases: Line 3696: return errCode['noimpl'] Line 3697: Line 3698: if vmDrive.transientDisk: leftover Line 3699: return errCode['transientErr'] Line 3700: Line 3701: vmDevName = vmDrive.name Line 3702: Line 3967: if srcDrive.hasVolumeLeases: Line 3968: return errCode['noimpl'] Line 3969: Line 3970: if srcDrive.transientDisk: Line 3971: return errCode['transientErr'] If I understand correctly what you're suggesting is to get disk replica to work with transient disks. To replicate the disk you need to access both source and destination from the same host (and on the contrary the transient layer is local). Line 3972: Line 3973: try: Line 3974: self._setDiskReplica(srcDrive, dstDisk) Line 3975: except: -- To view, visit http://gerrit.ovirt.org/18326 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3dfec35e324c47d8c86a965947e3ae4ae48c7524 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepa...@linux.vnet.ibm.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Deepak C Shetty deepa...@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Itamar Heim ih...@redhat.com Gerrit-Reviewer: Liron Ar lara...@redhat.com Gerrit-Reviewer: Timothy Asir tjeya...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vm: add the transient disk support
Federico Simoncelli has posted comments on this change. Change subject: vm: add the transient disk support .. Patch Set 8: (1 comment) File lib/vdsm/tool/transient.py Line 63: daemon is running) Line 64: Line 65: transient_images = set(os.listdir(TRANSIENT_DISKS_REPO)) Line 66: Line 67: cmd_ret, cmd_out, cmd_err = execCmd([_fuser.cmd] + transient_images) Yes I already addressed this locally. I thought it was on your build as well. Line 68: # According to: fuser returns a non-zero return code if none of the Line 69: # specified files is accessed or in case of a fatal error. If at least Line 70: # one access has been found, fuser returns zero. we can discard the Line 71: # return code. -- To view, visit http://gerrit.ovirt.org/18326 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3dfec35e324c47d8c86a965947e3ae4ae48c7524 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepa...@linux.vnet.ibm.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Deepak C Shetty deepa...@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Itamar Heim ih...@redhat.com Gerrit-Reviewer: Liron Ar lara...@redhat.com Gerrit-Reviewer: Timothy Asir tjeya...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm: add support for ram/vram on QXL device
Federico Simoncelli has posted comments on this change. Change subject: vdsm: add support for ram/vram on QXL device .. Patch Set 6: Code-Review+1 It seems straight forward (and at first sight it seems not to break live migration), anyway I am not familiar enough with this part in order to give a +2 at this stage. Can you verify it in the meantime? -- To view, visit http://gerrit.ovirt.org/19361 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I71cc51acf22dbce66373009f0c6ec8e2022a4f87 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: Better Saggi bettersa...@gmail.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Peter V. Saveliev p...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Shahar Havivi shav...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sd: use the correct default for LEASE_TIME_SEC
Federico Simoncelli has uploaded a new change for review. Change subject: sd: use the correct default for LEASE_TIME_SEC .. sd: use the correct default for LEASE_TIME_SEC Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1011505 Change-Id: I7d0e3939241120656513d47d1bbb1cacaef0ae83 Signed-off-by: Federico Simoncelli fsimo...@redhat.com --- M vdsm/storage/blockSD.py M vdsm/storage/fileSD.py 2 files changed, 2 insertions(+), 2 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/67/19567/1 diff --git a/vdsm/storage/blockSD.py b/vdsm/storage/blockSD.py index 0867ad3..d27f331 100644 --- a/vdsm/storage/blockSD.py +++ b/vdsm/storage/blockSD.py @@ -564,7 +564,7 @@ sd.DMDK_LOCK_RENEWAL_INTERVAL_SEC: sd.DEFAULT_LEASE_PARAMS[ sd.DMDK_LOCK_RENEWAL_INTERVAL_SEC], sd.DMDK_LEASE_TIME_SEC: sd.DEFAULT_LEASE_PARAMS[ -sd.DMDK_LOCK_RENEWAL_INTERVAL_SEC], +sd.DMDK_LEASE_TIME_SEC], sd.DMDK_IO_OP_TIMEOUT_SEC: sd.DEFAULT_LEASE_PARAMS[ sd.DMDK_IO_OP_TIMEOUT_SEC], sd.DMDK_LEASE_RETRIES: sd.DEFAULT_LEASE_PARAMS[ diff --git a/vdsm/storage/fileSD.py b/vdsm/storage/fileSD.py index ee1255f..8e788a5 100644 --- a/vdsm/storage/fileSD.py +++ b/vdsm/storage/fileSD.py @@ -227,7 +227,7 @@ sd.DMDK_LOCK_RENEWAL_INTERVAL_SEC: sd.DEFAULT_LEASE_PARAMS[sd.DMDK_LOCK_RENEWAL_INTERVAL_SEC], sd.DMDK_LEASE_TIME_SEC: sd.DEFAULT_LEASE_PARAMS[ -sd.DMDK_LOCK_RENEWAL_INTERVAL_SEC], +sd.DMDK_LEASE_TIME_SEC], sd.DMDK_IO_OP_TIMEOUT_SEC: sd.DEFAULT_LEASE_PARAMS[sd.DMDK_IO_OP_TIMEOUT_SEC], sd.DMDK_LEASE_RETRIES: -- To view, visit http://gerrit.ovirt.org/19567 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I7d0e3939241120656513d47d1bbb1cacaef0ae83 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vm: ignore missing shared attribute
Federico Simoncelli has uploaded a new change for review. Change subject: vm: ignore missing shared attribute .. vm: ignore missing shared attribute Change-Id: I4d377635b0687baccc69b203cb3fbe8dbf573169 Signed-off-by: Federico Simoncelli fsimo...@redhat.com --- M vdsm/vm.py 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/09/19509/1 diff --git a/vdsm/vm.py b/vdsm/vm.py index f356c08..f4d7f13 100644 --- a/vdsm/vm.py +++ b/vdsm/vm.py @@ -1341,7 +1341,7 @@ @property def hasVolumeLeases(self): -if self.shared != DRIVE_SHARED_TYPE.EXCLUSIVE: +if getattr(self, shared, None) != DRIVE_SHARED_TYPE.EXCLUSIVE: return False for volInfo in getattr(self, volumeChain, []): -- To view, visit http://gerrit.ovirt.org/19509 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I4d377635b0687baccc69b203cb3fbe8dbf573169 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: fileVolume: improve size checks in _extendSizeRaw
Federico Simoncelli has uploaded a new change for review. Change subject: fileVolume: improve size checks in _extendSizeRaw .. fileVolume: improve size checks in _extendSizeRaw Change-Id: I87695d67bd912f084e99314cff4ce42fd9d4cd2c Signed-off-by: Federico Simoncelli fsimo...@redhat.com --- M vdsm/storage/fileVolume.py 1 file changed, 6 insertions(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/08/19508/1 diff --git a/vdsm/storage/fileVolume.py b/vdsm/storage/fileVolume.py index 4525b0e..33d97d9 100644 --- a/vdsm/storage/fileVolume.py +++ b/vdsm/storage/fileVolume.py @@ -578,7 +578,12 @@ # No real sanity checks here, they should be included in the calling # function/method. We just validate the sizes to be consistent since # they're computed and used in the pre-allocated case. -if (newSizeBytes = curSizeBytes): +if newSizeBytes == curSizeBytes: +return # Nothing to do +elif curSizeBytes = 0: +raise se.StorageException( +Volume size is impossible: %s % curSizeBytes) +elif newSizeBytes curSizeBytes: raise se.VolumeResizeValueError(newSize) if self.getVolType() == volume.PREALLOCATED_VOL: -- To view, visit http://gerrit.ovirt.org/19508 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I87695d67bd912f084e99314cff4ce42fd9d4cd2c Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vm: add the transient disk support
Federico Simoncelli has posted comments on this change. Change subject: vm: add the transient disk support .. Patch Set 7: (1 comment) File vdsm/vm.py Line 3384: # We lookup the drive by UUIDs since the image path could Line 3385: # be transient. Line 3386: drive = self._findDriveByUUIDs(diskParams) Line 3387: else: Line 3388: drive = self._findDriveByPath(diskParams['path']) This doesn't mix well in case we want a transient LUN disk. Line 3389: except LookupError: Line 3390: self.log.error(Hotunplug disk failed - Disk not found: %s, Line 3391:diskParams) Line 3392: return {'status': { -- To view, visit http://gerrit.ovirt.org/18326 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3dfec35e324c47d8c86a965947e3ae4ae48c7524 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepa...@linux.vnet.ibm.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Deepak C Shetty deepa...@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Itamar Heim ih...@redhat.com Gerrit-Reviewer: Liron Ar lara...@redhat.com Gerrit-Reviewer: Timothy Asir tjeya...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: blockVolume: round up volume size for createVolume
Federico Simoncelli has uploaded a new change for review. Change subject: blockVolume: round up volume size for createVolume .. blockVolume: round up volume size for createVolume The logical volume size granularity used in lvm(.py) is 1Mb, therefore the size in sectors should be aligned. When the size is particularly small this also prevents failing lvcreate calls with size 0m. Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1009086 Change-Id: I5ae3d918ea4e7ca91383c3a3184a338cb40c4c27 Signed-off-by: Federico Simoncelli fsimo...@redhat.com --- M vdsm/storage/blockVolume.py 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/90/19390/1 diff --git a/vdsm/storage/blockVolume.py b/vdsm/storage/blockVolume.py index 88632ef..badf2c1 100644 --- a/vdsm/storage/blockVolume.py +++ b/vdsm/storage/blockVolume.py @@ -174,7 +174,7 @@ if preallocate == volume.SPARSE_VOL: volSize = %s % config.get(irs, volume_utilization_chunk_mb) else: -volSize = %s % (size / 2 / 1024) +volSize = %s % ((size + SECTORS_TO_MB - 1) / SECTORS_TO_MB) lvm.createLV(dom.sdUUID, volUUID, volSize, activate=True, initialTag=TAG_VOL_UNINIT) -- To view, visit http://gerrit.ovirt.org/19390 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I5ae3d918ea4e7ca91383c3a3184a338cb40c4c27 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Fix parent volume rollback after failure to create snapshot
Federico Simoncelli has posted comments on this change. Change subject: Fix parent volume rollback after failure to create snapshot .. Patch Set 7: Code-Review+1 (1 comment) File vdsm/storage/volume.py Line 334: pvolUUID) Line 335: pvol.prepare() Line 336: try: Line 337: pvol.recheckIfLeaf() Line 338: except Exception: Don't kill me :-) ...but I was wondering... maybe this is unnecessary (the except block) ...feel free to address it somewhere else Line 339: cls.log.error(Unexpected error, exc_info=True) Line 340: finally: Line 341: pvol.teardown(sdUUID, pvolUUID) Line 342: -- To view, visit http://gerrit.ovirt.org/18205 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I11948aacad21008ffbb4cb23013bf363e60bdb9b Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Eduardo ewars...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: glusterSD: BZ 988299: Deduce gluster volume name from domain...
Federico Simoncelli has posted comments on this change. Change subject: glusterSD: BZ 988299: Deduce gluster volume name from domain metadata .. Patch Set 1: Code-Review-1 (2 comments) -1 for visibility File vdsm/storage/glusterVolume.py Line 18: Line 19: Line 20: # Get the pristine remote path (hostname:volname) from MD, Line 21: # so that we don't end up using the mangled volume and/or hostnames. Line 22: rpath = sdCache.produce(self.sdUUID).getMetaParam(REMOTE_PATH) Would it be possible to override getRemotePath() in glusterSD? Line 23: rpath_list = rpath.rsplit(:, 1) Line 24: volfileServer = rpath_list[0] Line 25: # Check for leading '/', since its perfectly valid to say Line 26: # hostname:/volname for gluster mounts. Line 22: rpath = sdCache.produce(self.sdUUID).getMetaParam(REMOTE_PATH) Line 23: rpath_list = rpath.rsplit(:, 1) Line 24: volfileServer = rpath_list[0] Line 25: # Check for leading '/', since its perfectly valid to say Line 26: # hostname:/volname for gluster mounts. Maybe this part will go away when we'll use glusterSD.getRemotePath() Line 27: if rpath_list[1][0] == '/': Line 28: volname = rpath_list[1][1:] Line 29: else: Line 30: volname = rpath_list[1] -- To view, visit http://gerrit.ovirt.org/19219 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibfc8af4aa7ab028a7a6f0e691eae1c89f5148ee9 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepa...@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Deepak C Shetty deepa...@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: update lease parameters on masterMigrate
Federico Simoncelli has posted comments on this change. Change subject: sp: update lease parameters on masterMigrate .. Patch Set 2: Verified+1 Verified migrating the master from one SD to another. -- To view, visit http://gerrit.ovirt.org/19176 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e5935e8a7081c391702f63d450621650769bb92 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: update lease parameters on masterMigrate
Federico Simoncelli has posted comments on this change. Change subject: sp: update lease parameters on masterMigrate .. Patch Set 3: Verified+1 Commit change only. -- To view, visit http://gerrit.ovirt.org/19176 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e5935e8a7081c391702f63d450621650769bb92 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Fix parent volume rollback after failure to create snapshot
Federico Simoncelli has posted comments on this change. Change subject: Fix parent volume rollback after failure to create snapshot .. Patch Set 6: Code-Review+1 (2 comments) Small nits File vdsm/storage/volume.py Line 262: parent = self.getVolumePath() Line 263: parent_format = fmt2str(self.getFormat()) Line 264: # We should use parent's relative path instead of full path Line 265: parent = os.path.join(os.path.basename(os.path.dirname(parent)), Line 266: os.path.basename(parent)) Lines 261-266 can be taken out of the try block (they don't need the volume to be prepared). Line 267: createVolume(parent, parent_format, dst_path, Line 268: size, volFormat, preallocate) Line 269: except Exception as e: Line 270: self.log.error(Volume.clone: can't clone: %s to %s % Line 269: except Exception as e: Line 270: self.log.error(Volume.clone: can't clone: %s to %s % Line 271:(self.volumePath, dst_path)) Line 272: raise se.CannotCloneVolume(self.volumePath, dst_path, str(e)) Line 273: else: No need for an else block. You can just move this after finally (no need for the volume to be prepared). Line 274: if self.isLeaf(): Line 275: taskName = parent volume rollback: + self.volUUID Line 276: vars.task.pushRecovery( Line 277: task.Recovery(taskName, volume, Volume, -- To view, visit http://gerrit.ovirt.org/18205 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I11948aacad21008ffbb4cb23013bf363e60bdb9b Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Eduardo ewars...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: hsm: fix updateVolumeSize for qcow2 volumes
Federico Simoncelli has uploaded a new change for review. Change subject: hsm: fix updateVolumeSize for qcow2 volumes .. hsm: fix updateVolumeSize for qcow2 volumes Change-Id: I08aa24277d11595e82c93b979c2c34fcf9d09c38 Signed-off-by: Federico Simoncelli fsimo...@redhat.com --- M vdsm/storage/hsm.py 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/79/19279/1 diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py index 744a855..6210912 100644 --- a/vdsm/storage/hsm.py +++ b/vdsm/storage/hsm.py @@ -696,7 +696,7 @@ if volFormat != volume.COW_FORMAT: # This method is used only with COW volumes (see docstring), # for RAW volumes we just return the volume size. -return dict(size=str(self.getVolumeSize(bs=1))) +return dict(size=str(volToExtend.getVolumeSize(bs=1))) qemuImgFormat = volume.fmt2str(volume.COW_FORMAT) -- To view, visit http://gerrit.ovirt.org/19279 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I08aa24277d11595e82c93b979c2c34fcf9d09c38 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.3]: imageSharing: return proper size in httpGetSize
Hello Dan Kenigsberg, I'd like you to do a code review. Please visit http://gerrit.ovirt.org/19284 to review the following change. Change subject: imageSharing: return proper size in httpGetSize .. imageSharing: return proper size in httpGetSize When importing images from OpenStack Glance the Content-Length is 0 so we need to override the value with the content of the custom header X-Image-Meta-Size. Change-Id: Icbcc601767d7f5b044b9e0e32f35abccdfb5746b Signed-off-by: Federico Simoncelli fsimo...@redhat.com Reviewed-on: http://gerrit.ovirt.org/19222 Reviewed-by: Dan Kenigsberg dan...@redhat.com --- M vdsm/storage/imageSharing.py 1 file changed, 10 insertions(+), 3 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/84/19284/1 diff --git a/vdsm/storage/imageSharing.py b/vdsm/storage/imageSharing.py index 5db0a4e..26f299a 100644 --- a/vdsm/storage/imageSharing.py +++ b/vdsm/storage/imageSharing.py @@ -29,11 +29,18 @@ headers = curlImgWrap.head(methodArgs.get('url'), methodArgs.get(headers, {})) +size = None + if 'Content-Length' in headers: size = int(headers['Content-Length']) -elif 'X-Image-Meta-Size' in headers: -size = int(headers['X-Image-Meta-Size']) -else: + +# OpenStack Glance returns Content-Length = 0 so we need to +# override the value with the content of the custom header +# X-Image-Meta-Size. +if 'X-Image-Meta-Size' in headers: +size = max(size, int(headers['X-Image-Meta-Size'])) + +if size is None: raise RuntimeError(Unable to determine image size) return size -- To view, visit http://gerrit.ovirt.org/19284 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Icbcc601767d7f5b044b9e0e32f35abccdfb5746b Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.3 Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.3]: imageSharing: return proper size in httpGetSize
Federico Simoncelli has submitted this change and it was merged. Change subject: imageSharing: return proper size in httpGetSize .. imageSharing: return proper size in httpGetSize When importing images from OpenStack Glance the Content-Length is 0 so we need to override the value with the content of the custom header X-Image-Meta-Size. Change-Id: Icbcc601767d7f5b044b9e0e32f35abccdfb5746b Signed-off-by: Federico Simoncelli fsimo...@redhat.com Reviewed-on: http://gerrit.ovirt.org/19222 Reviewed-by: Dan Kenigsberg dan...@redhat.com Reviewed-on: http://gerrit.ovirt.org/19284 --- M vdsm/storage/imageSharing.py 1 file changed, 10 insertions(+), 3 deletions(-) Approvals: Federico Simoncelli: Verified; Looks good to me, approved -- To view, visit http://gerrit.ovirt.org/19284 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: Icbcc601767d7f5b044b9e0e32f35abccdfb5746b Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.3 Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.3]: imageSharing: return proper size in httpGetSize
Federico Simoncelli has posted comments on this change. Change subject: imageSharing: return proper size in httpGetSize .. Patch Set 1: Verified+1 Code-Review+2 Same as master. -- To view, visit http://gerrit.ovirt.org/19284 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icbcc601767d7f5b044b9e0e32f35abccdfb5746b Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.3 Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Fix parent volume rollback after failure to create snapshot
Federico Simoncelli has posted comments on this change. Change subject: Fix parent volume rollback after failure to create snapshot .. Patch Set 6: (1 comment) File vdsm/storage/volume.py Line 262: parent = self.getVolumePath() Line 263: parent_format = fmt2str(self.getFormat()) Line 264: # We should use parent's relative path instead of full path Line 265: parent = os.path.join(os.path.basename(os.path.dirname(parent)), Line 266: os.path.basename(parent)) Anything could raise an exception there (KeyError in fmt2str, etc.). I'm not a fan of exceptions translation/masking. The CannotCloneVolume exception is not helping the engine: what difference it makes at what step we failed? We weren't able to create a volume (which required cloning, but that's not interesting). Even more, if we translate it to CannotCloneVolume it is even misleading as the real error would be a metadata corruption (as the size is not a base-10 integer or the format is unknown). For as much as I know the exception translation could be removed: def clone(...): (prepare the needed variables) self.prepare(rw=False) try: createVolume(...) finally: self.teardown(...) if self.isLeaf(): ... Line 267: createVolume(parent, parent_format, dst_path, Line 268: size, volFormat, preallocate) Line 269: except Exception as e: Line 270: self.log.error(Volume.clone: can't clone: %s to %s % -- To view, visit http://gerrit.ovirt.org/18205 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I11948aacad21008ffbb4cb23013bf363e60bdb9b Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Eduardo ewars...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vm: add the transient disk support
Federico Simoncelli has posted comments on this change. Change subject: vm: add the transient disk support .. Patch Set 6: (2 comments) File vdsm/vm.py Line 1424: # To handle legacy and removable drives. Line 1425: return False Line 1426: Line 1427: def _customize(self): Line 1428: if self.transientDisk: Note to self: remember to check this. Line 1429: # Force the cache to be writethrough, which is qemu's default. Line 1430: # This is done to ensure that we don't ever use cache=none for Line 1431: # transient disks, since we create them in /var/run/vdsm which Line 1432: # may end up on tmpfs and don't support O_DIRECT, and qemu uses Line 3398: # We lookup the drive by UUIDs since the image path could be Line 3399: # transient. Line 3400: drive = self._findDriveByUUIDs(diskParams) Line 3401: else: Line 3402: drive = self._findDriveByPath(diskParams['path']) Catch LookupError and transform to errCode['hotunplugDisk']. Line 3403: Line 3404: if drive.hasVolumeLeases: Line 3405: return errCode['noimpl'] Line 3406: -- To view, visit http://gerrit.ovirt.org/18326 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3dfec35e324c47d8c86a965947e3ae4ae48c7524 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepa...@linux.vnet.ibm.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Deepak C Shetty deepa...@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Itamar Heim ih...@redhat.com Gerrit-Reviewer: Liron Ar lara...@redhat.com Gerrit-Reviewer: Timothy Asir tjeya...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imageSharing: return proper size in httpGetSize
Federico Simoncelli has uploaded a new change for review. Change subject: imageSharing: return proper size in httpGetSize .. imageSharing: return proper size in httpGetSize When importing images from OpenStack Glance the Content-Length is 0 so we need to override the value with the content of the custom header X-Image-Meta-Size. Change-Id: Icbcc601767d7f5b044b9e0e32f35abccdfb5746b Signed-off-by: Federico Simoncelli fsimo...@redhat.com --- M vdsm/storage/imageSharing.py 1 file changed, 10 insertions(+), 3 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/22/19222/1 diff --git a/vdsm/storage/imageSharing.py b/vdsm/storage/imageSharing.py index 5db0a4e..26f299a 100644 --- a/vdsm/storage/imageSharing.py +++ b/vdsm/storage/imageSharing.py @@ -29,11 +29,18 @@ headers = curlImgWrap.head(methodArgs.get('url'), methodArgs.get(headers, {})) +size = None + if 'Content-Length' in headers: size = int(headers['Content-Length']) -elif 'X-Image-Meta-Size' in headers: -size = int(headers['X-Image-Meta-Size']) -else: + +# OpenStack Glance returns Content-Length = 0 so we need to +# override the value with the content of the custom header +# X-Image-Meta-Size. +if 'X-Image-Meta-Size' in headers: +size = max(size, int(headers['X-Image-Meta-Size'])) + +if size is None: raise RuntimeError(Unable to determine image size) return size -- To view, visit http://gerrit.ovirt.org/19222 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Icbcc601767d7f5b044b9e0e32f35abccdfb5746b Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: imageSharing: return proper size in httpGetSize
Federico Simoncelli has posted comments on this change. Change subject: imageSharing: return proper size in httpGetSize .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.ovirt.org/19222 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icbcc601767d7f5b044b9e0e32f35abccdfb5746b Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: update lease parameters on masterMigrate
Federico Simoncelli has uploaded a new change for review. Change subject: sp: update lease parameters on masterMigrate .. sp: update lease parameters on masterMigrate In this patch: - update lease parameters on masterMigrate - relax the default parameters Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=999343 Change-Id: I0e5935e8a7081c391702f63d450621650769bb92 Signed-off-by: Federico Simoncelli fsimo...@redhat.com --- M vdsm/storage/sd.py M vdsm/storage/sp.py 2 files changed, 10 insertions(+), 2 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/76/19176/1 diff --git a/vdsm/storage/sd.py b/vdsm/storage/sd.py index 68cfda3..fb266d2 100644 --- a/vdsm/storage/sd.py +++ b/vdsm/storage/sd.py @@ -123,9 +123,9 @@ DEFAULT_LEASE_PARAMS = {DMDK_LOCK_POLICY: ON, DMDK_LEASE_RETRIES: 3, -DMDK_LEASE_TIME_SEC: 30, +DMDK_LEASE_TIME_SEC: 60, DMDK_LOCK_RENEWAL_INTERVAL_SEC: 5, -DMDK_IO_OP_TIMEOUT_SEC: 1} +DMDK_IO_OP_TIMEOUT_SEC: 10} MASTER_FS_DIR = 'master' VMS_DIR = 'vms' diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py index 38eda39..801784f 100644 --- a/vdsm/storage/sp.py +++ b/vdsm/storage/sp.py @@ -847,6 +847,13 @@ newMD.changeLeaseParams(leaseParams) @unsecured +def _copyLeaseParameters(srcDomain, dstDomain): +leaseParams = srcDomain.getLeaseParams() +self.log.info(Updating lease parameters for domain %s to %s, + srcDomain.sdUUID, leaseParams) +dstDomain.changeLeaseParams(leaseParams) + +@unsecured def masterMigrate(self, sdUUID, msdUUID, masterVersion): self.log.info(sdUUID=%s spUUID=%s msdUUID=%s, sdUUID, self.spUUID, msdUUID) @@ -859,6 +866,7 @@ self._refreshDomainLinks(newmsd) curmsd.invalidateMetadata() self._convertDomain(newmsd, curmsd.getFormat()) +self._copyLeaseParameters(curmsd, newmsd) # new 'master' should be in 'active' status domList = self.getDomains() -- To view, visit http://gerrit.ovirt.org/19176 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I0e5935e8a7081c391702f63d450621650769bb92 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: update lease parameters on masterMigrate
Federico Simoncelli has posted comments on this change. Change subject: sp: update lease parameters on masterMigrate .. Patch Set 1: (1 comment) File vdsm/storage/sp.py Line 1006: if dom.isData() and domVers mstVers: Line 1007: raise se.MixedSDVersionError(dom.sdUUID, domVers, Line 1008: self.masterDomain.sdUUID, Line 1009: mstVers) Line 1010: We can add another: self._copyLeaseParameters(self.masterDomain, dom) but it looks redundant to me. Line 1011: dom.attach(self.spUUID) Line 1012: domains[sdUUID] = sd.DOM_ATTACHED_STATUS Line 1013: self.setMetaParam(PMDK_DOMAINS, domains) Line 1014: self._refreshDomainLinks(dom) -- To view, visit http://gerrit.ovirt.org/19176 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e5935e8a7081c391702f63d450621650769bb92 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: update lease parameters on masterMigrate
Federico Simoncelli has posted comments on this change. Change subject: sp: update lease parameters on masterMigrate .. Patch Set 2: (1 comment) File vdsm/storage/sp.py Line 1006: if dom.isData() and domVers mstVers: Line 1007: raise se.MixedSDVersionError(dom.sdUUID, domVers, Line 1008: self.masterDomain.sdUUID, Line 1009: mstVers) Line 1010: We can add another: self._copyLeaseParameters(self.masterDomain, dom) but it looks redundant to me. Line 1011: dom.attach(self.spUUID) Line 1012: domains[sdUUID] = sd.DOM_ATTACHED_STATUS Line 1013: self.setMetaParam(PMDK_DOMAINS, domains) Line 1014: self._refreshDomainLinks(dom) -- To view, visit http://gerrit.ovirt.org/19176 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e5935e8a7081c391702f63d450621650769bb92 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.3]: vdsmd.init: Add service-is-managed in shutdown_conflicting_srv
Federico Simoncelli has submitted this change and it was merged. Change subject: vdsmd.init: Add service-is-managed in shutdown_conflicting_srv .. vdsmd.init: Add service-is-managed in shutdown_conflicting_srv We should not throw a stack trace if a service does not exist Change-Id: I8916a73c446ea2f296b66f5fcf133b07c2d7a66c Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1006842 Signed-off-by: Douglas Schilling Landgraf dougsl...@redhat.com Reviewed-on: http://gerrit.ovirt.org/19197 Reviewed-by: Dan Kenigsberg dan...@redhat.com --- M vdsm/vdsmd.init.in 1 file changed, 1 insertion(+), 0 deletions(-) Approvals: Douglas Schilling Landgraf: Verified Dan Kenigsberg: Looks good to me, approved -- To view, visit http://gerrit.ovirt.org/19197 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I8916a73c446ea2f296b66f5fcf133b07c2d7a66c Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.3 Gerrit-Owner: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: automat...@ovirt.org ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: lib: add create function to the qemuImg module
Federico Simoncelli has posted comments on this change. Change subject: lib: add create function to the qemuImg module .. Patch Set 2: Verified+1 In my current use case it works (backup api). Feel free to merge if you need this in other patches too. In the future I'll send a patch to clean up other places that should be using this function instead of a direct call to qemu-img. -- To view, visit http://gerrit.ovirt.org/18935 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I69522cd6231e511d891815248fab9a15cbc987e5 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm-tool: handle missing sanlock pid file
Federico Simoncelli has uploaded a new change for review. Change subject: vdsm-tool: handle missing sanlock pid file .. vdsm-tool: handle missing sanlock pid file Change-Id: I711d9b413415ca9372520e6bb25cf6d39ff571a3 Signed-off-by: Federico Simoncelli fsimo...@redhat.com --- M lib/vdsm/tool/sanlock.py 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/12/19112/1 diff --git a/lib/vdsm/tool/sanlock.py b/lib/vdsm/tool/sanlock.py index c64c874..650a5cf 100644 --- a/lib/vdsm/tool/sanlock.py +++ b/lib/vdsm/tool/sanlock.py @@ -38,13 +38,13 @@ supplementary groups. -sanlock_pid = open(SANLOCK_PID, r).readline().strip() - try: +sanlock_pid = open(SANLOCK_PID, r).readline().strip() sanlock_status = open(PROC_STATUS_PATH % sanlock_pid, r) except IOError as e: if e.errno == os.errno.ENOENT: return 0 # service is not running, returning +raise for status_line in sanlock_status: if status_line.startswith(PROC_STATUS_GROUPS): -- To view, visit http://gerrit.ovirt.org/19112 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I711d9b413415ca9372520e6bb25cf6d39ff571a3 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm-tool: handle missing sanlock pid file
Federico Simoncelli has posted comments on this change. Change subject: vdsm-tool: handle missing sanlock pid file .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.ovirt.org/19112 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I711d9b413415ca9372520e6bb25cf6d39ff571a3 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vm: small refactor for _normalizeVdsmImg
Federico Simoncelli has uploaded a new change for review. Change subject: vm: small refactor for _normalizeVdsmImg .. vm: small refactor for _normalizeVdsmImg Change-Id: Ie68292eee4b82fbe8527e3960739979cfe117dfa Signed-off-by: Federico Simoncelli fsimo...@redhat.com --- M vdsm/vm.py 1 file changed, 18 insertions(+), 17 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/57/19157/1 diff --git a/vdsm/vm.py b/vdsm/vm.py index 92d274e..2605f24 100644 --- a/vdsm/vm.py +++ b/vdsm/vm.py @@ -1779,25 +1779,26 @@ break return str(idx) -def _normalizeVdsmImg(self, drv): -drv['reqsize'] = drv.get('reqsize', '0') # Backward compatible -if 'device' not in drv: -drv['device'] = 'disk' +def _normalizeVdsmImg(self, drive): +drive['device'] = drive.get('device', 'disk') # Disk by default +drive['reqsize'] = drive.get('reqsize', '0') # Backward compatible -if drv['device'] == 'disk': -res = self.cif.irs.getVolumeSize(drv['domainID'], drv['poolID'], - drv['imageID'], drv['volumeID']) -try: -drv['truesize'] = res['truesize'] -drv['apparentsize'] = res['apparentsize'] -except KeyError: -self.log.error(Unable to get volume size for %s, - drv['volumeID'], exc_info=True) -raise RuntimeError(Volume %s is corrupted or missing % - drv['volumeID']) +if drive['device'] == 'disk': +volInfo = self.cif.irs.getVolumeInfo( +drive['domainID'], drive['poolID'], drive['imageID'], +drive['volumeID']) + +if volInfo.get('status', {}).get('code', -1): +self.log.error( +Unable to get volume info for %s, drive['volumeID']) +raise RuntimeError( +Volume %s is corrupted or missing % drive['volumeID']) + +drive['truesize'] = volInfo['info']['truesize'] +drive['apparentsize'] = volInfo['info']['apparentsize'] else: -drv['truesize'] = 0 -drv['apparentsize'] = 0 +drive['truesize'] = 0 +drive['apparentsize'] = 0 @classmethod def _normalizeDriveSharedAttribute(self, drive): -- To view, visit http://gerrit.ovirt.org/19157 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ie68292eee4b82fbe8527e3960739979cfe117dfa Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: hsm: fix sloppy rebase of one shot prepare
Federico Simoncelli has uploaded a new change for review. Change subject: hsm: fix sloppy rebase of one shot prepare .. hsm: fix sloppy rebase of one shot prepare A sloppy rebase of c072945 One shot prepare removed the changes introduced by cef2d5b vm: extend shared property to support locking. Change-Id: I8f0de78bb9865c93dc40f51c1fcc9b6b29cef516 Signed-off-by: Federico Simoncelli fsimo...@redhat.com --- M vdsm/storage/hsm.py 1 file changed, 6 insertions(+), 7 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/49/19049/1 diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py index f6ba303..8adfda9 100644 --- a/vdsm/storage/hsm.py +++ b/vdsm/storage/hsm.py @@ -3242,14 +3242,13 @@ 'volumeID': volUUID, 'path': path, 'volType': path} -if config.getboolean('irs', 'use_volume_leases'): -leasePath, leaseOffset = dom.getVolumeLease(imgUUID, volUUID) +leasePath, leaseOffset = dom.getVolumeLease(imgUUID, volUUID) -if leasePath and leaseOffset is not None: -volInfo.update({ - 'leasePath': leasePath, - 'leaseOffset': leaseOffset, - }) +if leasePath and type(leaseOffset) in (int, long): +volInfo.update({ +'leasePath': leasePath, +'leaseOffset': leaseOffset, +}) imgVolumesInfo.append(volInfo) if volUUID == leafUUID: -- To view, visit http://gerrit.ovirt.org/19049 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I8f0de78bb9865c93dc40f51c1fcc9b6b29cef516 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: hsm: fix sloppy rebase of one shot prepare
Federico Simoncelli has posted comments on this change. Change subject: hsm: fix sloppy rebase of one shot prepare .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.ovirt.org/19049 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8f0de78bb9865c93dc40f51c1fcc9b6b29cef516 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm-tool: fix qemu-sanlock configuration
Federico Simoncelli has uploaded a new change for review. Change subject: vdsm-tool: fix qemu-sanlock configuration .. vdsm-tool: fix qemu-sanlock configuration Fixing the logic about qemu-sanlock configuration: if sanlock is enabled then qemu-sanlock.conf needs to be updated. Change-Id: I3d2a6b05fb8fbf0ef2e9599fd1a740a7091680d4 Signed-off-by: Federico Simoncelli fsimo...@redhat.com --- M lib/vdsm/tool/libvirt_configure.sh.in 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/94/19094/1 diff --git a/lib/vdsm/tool/libvirt_configure.sh.in b/lib/vdsm/tool/libvirt_configure.sh.in index c4f56a9..2ab5334 100755 --- a/lib/vdsm/tool/libvirt_configure.sh.in +++ b/lib/vdsm/tool/libvirt_configure.sh.in @@ -30,7 +30,7 @@ QCONF=@sysconfdir@/libvirt/qemu.conf LDCONF=@sysconfdir@/sysconfig/libvirtd QLCONF=@sysconfdir@/libvirt/qemu-sanlock.conf -[ ${ENABLE_LIBVIRT_SANLOCK} = yes ] QLCONF=/dev/null +[ ${ENABLE_LIBVIRT_SANLOCK} = yes ] || QLCONF=/dev/null # trigger for reconfiguration FORCE_RECONFIGURE=@VDSMLIBDIR@/reconfigure -- To view, visit http://gerrit.ovirt.org/19094 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I3d2a6b05fb8fbf0ef2e9599fd1a740a7091680d4 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Adding more qemuImg commands
Federico Simoncelli has abandoned this change. Change subject: Adding more qemuImg commands .. Abandoned These changes were slowly introduced by other patches. -- To view, visit http://gerrit.ovirt.org/2612 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: abandon Gerrit-Change-Id: I1fbe822da8544ece4fcce9cb5b15a39a9bb35a64 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: lib: add create function to the qemuImg module
Federico Simoncelli has uploaded a new change for review. Change subject: lib: add create function to the qemuImg module .. lib: add create function to the qemuImg module Change-Id: I69522cd6231e511d891815248fab9a15cbc987e5 Signed-off-by: Federico Simoncelli fsimo...@redhat.com --- M lib/vdsm/qemuImg.py 1 file changed, 23 insertions(+), 0 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/35/18935/1 diff --git a/lib/vdsm/qemuImg.py b/lib/vdsm/qemuImg.py index a2a5682..e9a328c 100644 --- a/lib/vdsm/qemuImg.py +++ b/lib/vdsm/qemuImg.py @@ -88,6 +88,29 @@ return info +def create(image, size=None, format=None, backing=None, backingFormat=None): +cmd = [_qemuimg.cmd, create] + +if format: +cmd.extend((-f, format)) + +if backing: +cmd.extend((-b, backing)) + +if backingFormat: +cmd.extend((-F, backingFormat)) + +cmd.append(image) + +if size: +cmd.append(int(size)) + +rc, out, err = utils.execCmd(cmd) + +if rc != 0: +raise QImgError(rc, out, err) + + def check(image, format=None): cmd = [_qemuimg.cmd, check] -- To view, visit http://gerrit.ovirt.org/18935 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I69522cd6231e511d891815248fab9a15cbc987e5 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: lib: add create function to the qemuImg module
Federico Simoncelli has posted comments on this change. Change subject: lib: add create function to the qemuImg module .. Patch Set 1: (1 comment) File lib/vdsm/qemuImg.py Line 87: Line 88: return info Line 89: Line 90: Line 91: def create(image, size=None, format=None, backing=None, backingFormat=None): On the contrary, I don't think it's awkward, it's just how qemu-img works. The mandatory arguments are image and size: qemuImg.create(myimage.img, 1024**3) The format by default is raw, so you can omit it, but if you want you can still specify it easily: qemuImg.create(myimage.img, 1024**3, qemuImg.FORMAT.QCOW2) Now you might say that you prefer always being explicit about the format, that's a good practice, agreed, but it has nothing to do with writing a wrapper where the goal is to expose the exact behavior of the command. The only reason for size being optional is that it can be automatically detected when a backing file is specified: qemuImg.create(myimage.img, ..., backing=mybacking.img) Now, the reason for not checking the size/backing presence is that we don't want to re-write checks, we'll rely on qemu-img failing (triggering the QImgError exception). Line 92: cmd = [_qemuimg.cmd, create] Line 93: Line 94: if format: Line 95: cmd.extend((-f, format)) -- To view, visit http://gerrit.ovirt.org/18935 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I69522cd6231e511d891815248fab9a15cbc987e5 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: spec: remove shared glusterfs requirements
Federico Simoncelli has posted comments on this change. Change subject: spec: remove shared glusterfs requirements .. Patch Set 1: Verified+1 verified with: $ rpm -qRp vdsm-gluster-4.12.0-102.gitde6f9a2.el6_4.noarch.rpm vdsm = 4.12.0-102.gitde6f9a2.el6_4 glusterfs-server python-magic ... $ rpm -qRp vdsm-4.12.0-102.gitde6f9a2.el6_4.x86_64.rpm ... glusterfs = 3.4.0 glusterfs-api glusterfs-cli glusterfs-fuse glusterfs-rdma ... -- To view, visit http://gerrit.ovirt.org/18408 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6cdbf9a0bc97ce0bf42b0b7647a2641717442d58 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Aravinda VK avish...@redhat.com Gerrit-Reviewer: Bala.FA barum...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Deepak C Shetty deepa...@linux.vnet.ibm.com Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vm: extend shared property to support locking
Federico Simoncelli has posted comments on this change. Change subject: vm: extend shared property to support locking .. Patch Set 7: Verified+1 -- To view, visit http://gerrit.ovirt.org/17714 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9429ead45caac1178957a33393642817db59508f Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Arik Hadas aha...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Eduardo ewars...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Greg Padgett gpadg...@redhat.com Gerrit-Reviewer: Itamar Heim ih...@redhat.com Gerrit-Reviewer: Sandro Bonazzola sbona...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vm: extend shared property to support locking
Federico Simoncelli has posted comments on this change. Change subject: vm: extend shared property to support locking .. Patch Set 8: Verified+1 No relevant changes. -- To view, visit http://gerrit.ovirt.org/17714 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9429ead45caac1178957a33393642817db59508f Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Arik Hadas aha...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Eduardo ewars...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Greg Padgett gpadg...@redhat.com Gerrit-Reviewer: Itamar Heim ih...@redhat.com Gerrit-Reviewer: Sandro Bonazzola sbona...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Removing has_systemd from sysv script
Federico Simoncelli has posted comments on this change. Change subject: Removing has_systemd from sysv script .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/18806 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ief9be5503c84e2cf960e722e5097dbdc05b888a6 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Maor Lipchuk mlipc...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: storage: kill ongoing operations on exit
Federico Simoncelli has posted comments on this change. Change subject: storage: kill ongoing operations on exit .. Patch Set 4: Verified+1 Quickly re-verified. -- To view, visit http://gerrit.ovirt.org/18074 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic6ff67cb687494099e332d2d7b1eb3a293d1b4e4 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: storage: kill ongoing operations on exit
Federico Simoncelli has posted comments on this change. Change subject: storage: kill ongoing operations on exit .. Patch Set 5: Verified+1 Minor change -- To view, visit http://gerrit.ovirt.org/18074 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic6ff67cb687494099e332d2d7b1eb3a293d1b4e4 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: storage: kill ongoing operations on exit
Federico Simoncelli has posted comments on this change. Change subject: storage: kill ongoing operations on exit .. Patch Set 3: Verified+1 Verified using this patch regularly for more than a week. I also verified that a (long) running dd command (zeroImage) is now terminated when vdsm is killed. -- To view, visit http://gerrit.ovirt.org/18074 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic6ff67cb687494099e332d2d7b1eb3a293d1b4e4 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Packaging: VDSM v4.12.0 packaged on Ubuntu
Federico Simoncelli has posted comments on this change. Change subject: Packaging: VDSM v4.12.0 packaged on Ubuntu .. Patch Set 3: Code-Review-1 (1 comment) File debian/patches/drop-libvirt-sanlock.patch Line 1: libvirt-sanlock provided in latest libvirt needs libaudit0, but policycoreutils needs libaudit1. Line 2: In these release we drop libvirt-sanlock and related feature. Yes, we're on the verge of finally using sanlock for volume leases (hosted engine). This is definitely a show stopper. Line 3: --- a/lib/vdsm/tool/libvirt_configure.sh.in Line 4: +++ b/lib/vdsm/tool/libvirt_configure.sh.in Line 5: @@ -235,9 +235,9 @@ Line 6: fi -- To view, visit http://gerrit.ovirt.org/18443 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iab62aeadfa65baef3f0f4e6da2592fa0ff451c6f Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Packaging: VDSM v4.12.0 packaged on Ubuntu
Federico Simoncelli has posted comments on this change. Change subject: Packaging: VDSM v4.12.0 packaged on Ubuntu .. Patch Set 3: -Code-Review (1 comment) File debian/patches/drop-libvirt-sanlock.patch Line 1: libvirt-sanlock provided in latest libvirt needs libaudit0, but policycoreutils needs libaudit1. Line 2: In these release we drop libvirt-sanlock and related feature. Ah sorry I missed that this would be for debian only. Maybe removing libvirt-sanlock from this initial debian integration could be ok. Anyway it's something that debian will have to deal with in the future. The important thing is that sanlock is provided for the SPM resource (domain v3). Line 3: --- a/lib/vdsm/tool/libvirt_configure.sh.in Line 4: +++ b/lib/vdsm/tool/libvirt_configure.sh.in Line 5: @@ -235,9 +235,9 @@ Line 6: fi -- To view, visit http://gerrit.ovirt.org/18443 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iab62aeadfa65baef3f0f4e6da2592fa0ff451c6f Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm: Add tuneBlockDevIo interface
Federico Simoncelli has posted comments on this change. Change subject: vdsm: Add tuneBlockDevIo interface .. Patch Set 14: Code-Review-1 (3 comments) File vdsm/vm.py Line 4326: # persist the target value to make it consistent after recovery Line 4327: self.saveState() Line 4328: return {'status': doneCode} Line 4329: Line 4330: def _getTuneBlkDevIoInfo(self): This might be useless. More info below. Line 4331: # Only tuned disk IO information is returned. Line 4332: results = {} Line 4333: for device in self.conf['devices']: Line 4334: if device.get('device') == 'disk' and device.get('name'): Line 4350: invalidParamNames = ', '.join(invalidParams) Line 4351: raise ValueError('Parameter %s name(s) are invalid' % Line 4352: invalidParamNames) Line 4353: Line 4354: def _checkIoTuneCategories(self, params): I am not a big fan of checks re-writes. If this is already checked by libvirt then let's just wait for it to fail when we call setBlockIoTune. Line 4355: categories = (bytes, iops) Line 4356: for category in categories: Line 4357: if params.get('total_' + category + '_sec', 0) and \ Line 4358: (params.get('read_' + category + '_sec', 0) or Line 4406: errMsg = 'Get new block I/O Tune params failed after setting' Line 4407: self.log.exception(errMsg) Line 4408: return getErrCode('tuneBlkDevIoErr', errMsg) Line 4409: else: Line 4410: for device in self.conf['devices']: I'm not sure I like this because if we crash between setBlockIoTune and saveState loop we basically lose this information. Best thing here would probably be add blockIoTune in the vm stats thread (and forget about persisting these values). Therefore _getTuneBlkDevIoInfo might be useless and could be removed. Line 4411: if device.get('name') == dev or device.get('path') == dev: Line 4412: device.setdefault('specParams', {}) Line 4413: device['specParams']['ioTune'] = info Line 4414: -- To view, visit http://gerrit.ovirt.org/14394 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icb33510a081d221af0f69d4dd2d55adf0b79efd2 Gerrit-PatchSet: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mei Liu liu...@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke a...@us.ibm.com Gerrit-Reviewer: Doron Fediuck dfedi...@redhat.com Gerrit-Reviewer: Eduardo ewars...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Mark Wu wu...@linux.vnet.ibm.com Gerrit-Reviewer: Mei Liu liu...@linux.vnet.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: storage: kill ongoing operations on exit
Federico Simoncelli has posted comments on this change. Change subject: storage: kill ongoing operations on exit .. Patch Set 3: (1 comment) File lib/vdsm/utils.py Line 458: Line 459: def execCmd(command, sudo=False, cwd=None, data=None, raw=False, logErr=True, Line 460: printable=None, env=None, sync=True, nice=None, ioclass=None, Line 461: ioclassdata=None, setsid=False, execCmdLogger=logging.root, Line 462: killOnExit=False): Why do you think death is more thread-specific than exit? After all the pthread lib has pthread_exit not pthread_die. If you think that death is better because it both includes exits and, well, process deaths (crashes) then we could consider: * killOnDeath * stopOnExit/stopOnDeath * terminateOnExit/terminateOnDeath Anyway my point here is that deathSignal really looks confusing (let's leave it for BetterPopen where in fact we specify a signal to use). Line 463: Line 464: Executes an external command, optionally via sudo. Line 465: Line 466: if ioclass is not None: -- To view, visit http://gerrit.ovirt.org/18074 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic6ff67cb687494099e332d2d7b1eb3a293d1b4e4 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Making setStorageDomainDescription the SPM command
Federico Simoncelli has posted comments on this change. Change subject: Making setStorageDomainDescription the SPM command .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/17198 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idca7b7b6642c222d88a6cd2a94d4033c0c3ef70b Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Eduardo ewars...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vm: extend shared property to support locking
Federico Simoncelli has posted comments on this change. Change subject: vm: extend shared property to support locking .. Patch Set 5: (1 comment) File vdsm/vm.py Line 1486: if not self.hasVolumeLeases: Line 1487: return # empty items generator Line 1488: Line 1489: # NOTE: at the moment we are generating the lease only for the leaf Line 1490: # when libvirt will support shared leases this will loop over all Finish comment. Line 1491: for volInfo in self.volumeChain[-1:]: Line 1492: lease = XMLElement('lease') Line 1493: lease.appendChildWithArgs('key', text=volInfo['volumeID']) Line 1494: lease.appendChildWithArgs('lockspace', -- To view, visit http://gerrit.ovirt.org/17714 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9429ead45caac1178957a33393642817db59508f Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Arik Hadas aha...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Eduardo ewars...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Greg Padgett gpadg...@redhat.com Gerrit-Reviewer: Itamar Heim ih...@redhat.com Gerrit-Reviewer: Sandro Bonazzola sbona...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add transient disk support
Federico Simoncelli has posted comments on this change. Change subject: Add transient disk support .. Patch Set 2: (3 comments) File vdsm/vm.py Line 1391: return False Line 1392: Line 1393: @property Line 1394: def blockDev(self): Line 1395: if self.networkDev or self.transientDisk: This should be automatic. Isn't self._blockDev = utils.isBlockDevice(self.path) going to be false anyway? Line 1396: return False Line 1397: Line 1398: if self._blockDev is None: Line 1399: try: Line 1418: # transient disks, since we create them in /var/run/vdsm which Line 1419: # may end up on tmpfs and don't support O_DIRECT, and qemu uses Line 1420: # O_DIRECT when cache=none and hence hotplug might fail with Line 1421: # error that one can take eternity to debug the reason behind it! Line 1422: self.cache = writethrough You probably need to update also the format to be always qcow2 (not sure if this is the relevant place though). Line 1423: Line 1424: # Set the path to the trPath. Ideally this should have worked, but Line 1425: # this changes the drives's path attribute and hotunplug fails to Line 1426: # to find the disk!. Alternative is to use the trPath as part of Line 3299: Line 3300: parentFmt = fmt2str(name2type(volInfo['info']['format'])) Line 3301: trPath = self._getTransientDiskPath(vpath) Line 3302: Line 3303: createVolume(parent, parentFmt, trPath, size, COW_FORMAT, I'm not sure we want this here. I was thinking of using a straight qemuImg.create(...). Line 3304: SPARSE_VOL, useRelPath=False) Line 3305: Line 3306: sdUUID = drive['domainID'] Line 3307: oop.getProcessPool(sdUUID).os.chmod(trPath, 0660) -- To view, visit http://gerrit.ovirt.org/18326 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3dfec35e324c47d8c86a965947e3ae4ae48c7524 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepa...@linux.vnet.ibm.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Deepak C Shetty deepa...@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Itamar Heim ih...@redhat.com Gerrit-Reviewer: Liron Ar lara...@redhat.com Gerrit-Reviewer: Timothy Asir tjeya...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsmd: remove reconfigure sanlock on 'reconfigure' verb
Federico Simoncelli has posted comments on this change. Change subject: vdsmd: remove reconfigure sanlock on 'reconfigure' verb .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/18466 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I61a4e99396b15e67b2c9385b654e981a1d7c3429 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsmd: remove reconfigure sanlock on 'reconfigure' verb
Federico Simoncelli has submitted this change and it was merged. Change subject: vdsmd: remove reconfigure sanlock on 'reconfigure' verb .. vdsmd: remove reconfigure sanlock on 'reconfigure' verb reconfigure sanlock is perform at every restart, so there is no need for performing that on reconfigure target. if it is required, we need vdsm-tool configure or similar verb, as systemd does not support custom targets. Change-Id: I61a4e99396b15e67b2c9385b654e981a1d7c3429 Signed-off-by: Alon Bar-Lev alo...@redhat.com Reviewed-on: http://gerrit.ovirt.org/18466 Reviewed-by: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Reviewed-by: Federico Simoncelli fsimo...@redhat.com --- M vdsm/vdsmd.init.in 1 file changed, 0 insertions(+), 1 deletion(-) Approvals: Alon Bar-Lev: Verified Federico Simoncelli: Looks good to me, approved Zhou Zheng Sheng: Looks good to me, but someone else must approve -- To view, visit http://gerrit.ovirt.org/18466 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I61a4e99396b15e67b2c9385b654e981a1d7c3429 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Fix rollback after failure to create snapshot
Federico Simoncelli has posted comments on this change. Change subject: Fix rollback after failure to create snapshot .. Patch Set 2: Code-Review-1 (5 comments) File vdsm/storage/volume.py Line 248: raise se.MergeSnapshotsError(self.volUUID) Line 249: self.setParent(backingVol) Line 250: self.recheckIfLeaf() Line 251: Line 252: def clone(self, dst_image_dir, dst_volUUID, volFormat, preallocate): This method could use a little bit of refactoring. I know it's not your fault and it's not strictly related to the bug you're fixing. Feel free to address my suggestions in a different patch. Line 253: Line 254: Clone self volume to the specified dst_image_dir/dst_volUUID Line 255: Line 256: dst_path = None Line 252: def clone(self, dst_image_dir, dst_volUUID, volFormat, preallocate): Line 253: Line 254: Clone self volume to the specified dst_image_dir/dst_volUUID Line 255: Line 256: dst_path = None Let's move here all the things that doesn't require the volume to be prepared, e.g.: dst_path = os.path.join(dst_image_dir, dst_volUUID) size = int(self.getMetaParam(SIZE)) ... Line 257: try: Line 258: self.prepare(rw=False) Line 259: dst_path = os.path.join(dst_image_dir, dst_volUUID) Line 260: self.log.debug(Volume.clone: %s to %s % Line 254: Clone self volume to the specified dst_image_dir/dst_volUUID Line 255: Line 256: dst_path = None Line 257: try: Line 258: self.prepare(rw=False) No need to protect self.prepare(rw=False) in the try block. Line 259: dst_path = os.path.join(dst_image_dir, dst_volUUID) Line 260: self.log.debug(Volume.clone: %s to %s % Line 261:(self.volumePath, dst_path)) Line 262: size = int(self.getMetaParam(SIZE)) Line 266: parent = os.path.join(os.path.basename(os.path.dirname(parent)), Line 267: os.path.basename(parent)) Line 268: createVolume(parent, parent_format, dst_path, Line 269: size, volFormat, preallocate) Line 270: self.teardown(self.sdUUID, self.volUUID) Why to repeat it twice (here) and in the Exception block, let's use finally. Line 271: except Exception as e: Line 272: self.teardown(self.sdUUID, self.volUUID) Line 273: self.log.error(Volume.clone: can't clone: %s to %s % Line 274:(self.volumePath, dst_path)) Line 313: def refreshVolume(self): Line 314: Line 315: Refresh volume Line 316: Line 317: pass Given the flow where clone is used, you cannot remove the rollback. Looking at both blockVolume._create(...) and fileVolume._create(...) the clone call is part of the (child) volume creation. Now if you succeed to setInternal() (on volParent) but then you crash, the child volume will be removed (rollbacks: startCreateVolumeRollback, halfbakedVolumeRollback), but what is going to set volParent to a leaf again? Line 318: Line 319: @classmethod Line 320: def startCreateVolumeRollback(cls, taskObj, sdUUID, imgUUID, volUUID): Line 321: cls.log.info(startCreateVolumeRollback: sdUUID=%s imgUUID=%s -- To view, visit http://gerrit.ovirt.org/18205 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I11948aacad21008ffbb4cb23013bf363e60bdb9b Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Eduardo ewars...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Remove redundant check that causes lvm cache to refresh ever...
Federico Simoncelli has posted comments on this change. Change subject: Remove redundant check that causes lvm cache to refresh every volume creation .. Patch Set 4: Code-Review+2 (1 comment) Minor comment and waiting for verification. File vdsm/storage/remoteFileHandler.py Line 352: if creatExcl: Line 353: flags |= os.O_EXCL Line 354: Line 355: fd = os.open(path, flags) Line 356: with os.fdopen(fd, 'w') as f: This looks fine (as os.fdopen should take care of closing also the fd) anyway an alternate form using the file descriptor is: fd = os.open(path, flags) try: if mode is not None: os.fchmod(fd, mode) os.ftruncate(fd, size) finally: os.close(fd) Use (or keep) the one you prefer. Line 357: if mode is not None: Line 358: os.chmod(path, mode) Line 359: f.truncate(size) Line 360: -- To view, visit http://gerrit.ovirt.org/18274 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6f3b6ca8313070d0345b5f76ebb0b3d9772d14f Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Removing validate sd from spm verbs which calls ckvg and ref...
Federico Simoncelli has posted comments on this change. Change subject: Removing validate sd from spm verbs which calls ckvg and refreshes the domain metadata on the spm even though the spm already controls the changes (i.e. a lot of redundant time is wasted) .. Patch Set 4: Code-Review-1 (2 comments) File vdsm/storage/hsm.py Line 1670: def uploadImage(self, methodArgs, spUUID, sdUUID, imgUUID, volUUID=None): Line 1671: Line 1672: Upload an image to a remote endpoint using the specified method and Line 1673: methodArgs. Line 1674: As in the other cases this should remain as: sdCache.produce(sdUUID) in order to fail early before the task is started. We know it's racy but either we remove it from the other commands as well or we keep it (no reason to make this different). Line 1675: pool = self.getPool(spUUID) Line 1676: # NOTE: this could become an hsm task Line 1677: self._spmSchedule(spUUID, uploadImage, pool.uploadImage, Line 1678: methodArgs, sdUUID, imgUUID, volUUID) Line 1681: def downloadImage(self, methodArgs, spUUID, sdUUID, imgUUID, volUUID=None): Line 1682: Line 1683: Download an image from a remote endpoint using the specified method Line 1684: and methodArgs. Line 1685: As in the other cases this should remain as: sdCache.produce(sdUUID) in order to fail early before the task is started. We know it's racy but either we remove it from the other commands as well or we keep it (no reason to make this different). Line 1686: pool = self.getPool(spUUID) Line 1687: # NOTE: this could become an hsm task, in such case the LV extension Line 1688: # required to prepare the destination should go through the mailbox. Line 1689: self._spmSchedule(spUUID, downloadImage, pool.downloadImage, -- To view, visit http://gerrit.ovirt.org/18275 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibf597a9e53dad04ad7a01fc1d5ceb8a180a542f2 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Removing validate sd from spm verbs which calls ckvg and ref...
Federico Simoncelli has posted comments on this change. Change subject: Removing validate sd from spm verbs which calls ckvg and refreshes the domain metadata on the spm even though the spm already controls the changes (i.e. a lot of redundant time is wasted) .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/18275 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibf597a9e53dad04ad7a01fc1d5ceb8a180a542f2 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: hsm: deleteImage fails because of wrong dictionary use
Federico Simoncelli has posted comments on this change. Change subject: hsm: deleteImage fails because of wrong dictionary use .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/17383 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I81f9a5aa63c0914e3b934046454df64ccd39c269 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Eduardo ewars...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Removing validate sd from spm verbs which calls ckvg and ref...
Federico Simoncelli has posted comments on this change. Change subject: Removing validate sd from spm verbs which calls ckvg and refreshes the domain metadata on the spm even though the spm already controls the changes (i.e. a lot of redundant time is wasted) .. Patch Set 6: Code-Review+2 Same -- To view, visit http://gerrit.ovirt.org/18275 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibf597a9e53dad04ad7a01fc1d5ceb8a180a542f2 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Remove redundant check that causes lvm cache to refresh ever...
Federico Simoncelli has posted comments on this change. Change subject: Remove redundant check that causes lvm cache to refresh every volume creation .. Patch Set 5: (1 comment) File vdsm/storage/fileVolume.py Line 129: oop.getProcessPool(dom.sdUUID).truncateFile(volPath, sizeBytes, Line 130: creatExcl=True) Line 131: except OSError as e: Line 132: if e.errno == errno.EEXIST: Line 133: raise se.VolumeAlreadyExists(volUUID) You need to add another raise to fail in other cases as well. I think you need this for blockVolumes too. Line 134: if preallocate == volume.PREALLOCATED_VOL: Line 135: try: Line 136: # ddWatchCopy expects size to be in bytes Line 137: misc.ddWatchCopy(/dev/zero, volPath, -- To view, visit http://gerrit.ovirt.org/18274 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6f3b6ca8313070d0345b5f76ebb0b3d9772d14f Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: spec: remove shared glusterfs requirements
Federico Simoncelli has uploaded a new change for review. Change subject: spec: remove shared glusterfs requirements .. spec: remove shared glusterfs requirements Since we now require glusterfs in the main package we can now remove the same dependencies from the gluster rpm. Change-Id: I6cdbf9a0bc97ce0bf42b0b7647a2641717442d58 Signed-off-by: Federico Simoncelli fsimo...@redhat.com --- M vdsm.spec.in 1 file changed, 0 insertions(+), 3 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/08/18408/1 diff --git a/vdsm.spec.in b/vdsm.spec.in index 74ad331..9666cc6 100644 --- a/vdsm.spec.in +++ b/vdsm.spec.in @@ -525,10 +525,7 @@ BuildArch: noarch Requires: %{name} = %{version}-%{release} -Requires: glusterfs = 3.4.0 Requires: glusterfs-server -Requires: glusterfs-fuse -Requires: glusterfs-rdma Requires: python-magic %description gluster -- To view, visit http://gerrit.ovirt.org/18408 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I6cdbf9a0bc97ce0bf42b0b7647a2641717442d58 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm.spec: BZ 988299: Fix GlusterFS RPM dep to support Glust...
Federico Simoncelli has posted comments on this change. Change subject: vdsm.spec: BZ 988299: Fix GlusterFS RPM dep to support GlusterFS SD .. Patch Set 1: Code-Review+2 (1 comment) Given that the requirements are almost the same of vdsm-gluster, looks good to me. File vdsm.spec.in Line 199: %endif Line 200: Line 201: # GlusterFS client-side RPMs needed for Gluster SD Line 202: %if 0%{?with_gluster} Line 203: Requires: glusterfs = 3.4.0 Given that we are now adding these requirements in the main package we might want to clean them up from the gluster rpm. Since that might be controversial and we're already late to get this in, I sent an additional patch here: http://gerrit.ovirt.org/#/c/18408 Discussion can continue there. Line 204: Requires: glusterfs-cli Line 205: Requires: glusterfs-api Line 206: Requires: glusterfs-fuse Line 207: Requires: glusterfs-rdma -- To view, visit http://gerrit.ovirt.org/17994 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If648feec92ed33f24f45550f016ac132a25e6923 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepa...@linux.vnet.ibm.com Gerrit-Reviewer: Andrew Cathrow and...@cathrow.com Gerrit-Reviewer: Bala.FA barum...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Deepak C Shetty deepa...@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Vijay Bellur vbel...@gmail.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm.spec: BZ 988299: Fix GlusterFS RPM dep to support Glust...
Federico Simoncelli has posted comments on this change. Change subject: vdsm.spec: BZ 988299: Fix GlusterFS RPM dep to support GlusterFS SD .. Patch Set 1: Code-Review+1 Temporarily removing +2 as we're still debating on how to deliver gluster 3.4 to el6. -- To view, visit http://gerrit.ovirt.org/17994 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If648feec92ed33f24f45550f016ac132a25e6923 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepa...@linux.vnet.ibm.com Gerrit-Reviewer: Andrew Cathrow and...@cathrow.com Gerrit-Reviewer: Bala.FA barum...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Deepak C Shetty deepa...@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Vijay Bellur vbel...@gmail.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm.spec: BZ 988299: Fix GlusterFS RPM dep to support Glust...
Federico Simoncelli has posted comments on this change. Change subject: vdsm.spec: BZ 988299: Fix GlusterFS RPM dep to support GlusterFS SD .. Patch Set 1: (1 comment) File vdsm.spec.in Line 199: %endif Line 200: Line 201: # GlusterFS client-side RPMs needed for Gluster SD Line 202: %if 0%{?with_gluster} Line 203: Requires: glusterfs = 3.4.0 As far as I see at line 527, vdsm-gluster requires vdsm: Requires: %{name} = %{version}-%{release} Line 204: Requires: glusterfs-cli Line 205: Requires: glusterfs-api Line 206: Requires: glusterfs-fuse Line 207: Requires: glusterfs-rdma -- To view, visit http://gerrit.ovirt.org/17994 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If648feec92ed33f24f45550f016ac132a25e6923 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepa...@linux.vnet.ibm.com Gerrit-Reviewer: Andrew Cathrow and...@cathrow.com Gerrit-Reviewer: Bala.FA barum...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Deepak C Shetty deepa...@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Vijay Bellur vbel...@gmail.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm.spec: BZ 988299: Fix GlusterFS RPM dep to support Glust...
Federico Simoncelli has posted comments on this change. Change subject: vdsm.spec: BZ 988299: Fix GlusterFS RPM dep to support GlusterFS SD .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/17994 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If648feec92ed33f24f45550f016ac132a25e6923 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepa...@linux.vnet.ibm.com Gerrit-Reviewer: Andrew Cathrow and...@cathrow.com Gerrit-Reviewer: Bala.FA barum...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Deepak C Shetty deepa...@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Vijay Bellur vbel...@gmail.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.3]: vdsm.spec: BZ 988299: Fix GlusterFS RPM dep to support Glust...
Federico Simoncelli has posted comments on this change. Change subject: vdsm.spec: BZ 988299: Fix GlusterFS RPM dep to support GlusterFS SD .. Patch Set 1: Verified+1 Code-Review+2 Same as master -- To view, visit http://gerrit.ovirt.org/18426 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If648feec92ed33f24f45550f016ac132a25e6923 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.3 Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Bala.FA barum...@redhat.com Gerrit-Reviewer: Deepak C Shetty deepa...@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm.spec: BZ 988299: Fix GlusterFS RPM dep to support Glust...
Federico Simoncelli has submitted this change and it was merged. Change subject: vdsm.spec: BZ 988299: Fix GlusterFS RPM dep to support GlusterFS SD .. vdsm.spec: BZ 988299: Fix GlusterFS RPM dep to support GlusterFS SD This patch fixes the GlusterFS RPM dependency needed by VDSM to support Gluster Storage Domain feature. Recent GlusterFS RPM re-packaging effort to better separate the client and server side bits and availability of qemu with libgfapi support mark the pre-req for this patch. These pre-req are now available in F18/F19/RHEL6/Centos6 and above in the appropriate distro specific repos. Also removing F17 dep as its End of life now. BZ 988299 has more details, with pointers to the community discussions around these. Change-Id: If648feec92ed33f24f45550f016ac132a25e6923 Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=988299 Signed-off-by: Deepak C Shetty deepa...@linux.vnet.ibm.com Reviewed-on: http://gerrit.ovirt.org/17994 Reviewed-by: Bala.FA barum...@redhat.com Reviewed-by: Federico Simoncelli fsimo...@redhat.com --- M vdsm.spec.in 1 file changed, 10 insertions(+), 1 deletion(-) Approvals: Bala.FA: Looks good to me, but someone else must approve Federico Simoncelli: Looks good to me, approved Deepak C Shetty: Verified -- To view, visit http://gerrit.ovirt.org/17994 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: If648feec92ed33f24f45550f016ac132a25e6923 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Deepak C Shetty deepa...@linux.vnet.ibm.com Gerrit-Reviewer: Andrew Cathrow and...@cathrow.com Gerrit-Reviewer: Bala.FA barum...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Deepak C Shetty deepa...@linux.vnet.ibm.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Vijay Bellur vbel...@gmail.com Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Remove redundant check that causes lvm cache to refresh ever...
Federico Simoncelli has posted comments on this change. Change subject: Remove redundant check that causes lvm cache to refresh every volume creation .. Patch Set 2: (1 comment) File vdsm/storage/remoteFileHandler.py Line 349: Line 350: def truncateFile(path, size, mode=None, creatExcl=False): Line 351: flags = os.O_EXCL | os.O_CREAT if creatExcl else w Line 352: Line 353: with os.open(path, flags) as f: os.open returns a file descriptor, I don't think it can be used in this way Line 354: if mode is not None: Line 355: os.chmod(path, mode) Line 356: f.truncate(size) Line 357: -- To view, visit http://gerrit.ovirt.org/18274 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6f3b6ca8313070d0345b5f76ebb0b3d9772d14f Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Remove redundant check that causes lvm cache to refresh ever...
Federico Simoncelli has posted comments on this change. Change subject: Remove redundant check that causes lvm cache to refresh every volume creation .. Patch Set 1: (1 comment) File vdsm/storage/volume.py Line 401: Line 402: cls.validateCreateVolumeParams(volFormat, preallocate, srcVolUUID) Line 403: Line 404: dom = sdCache.produce(sdUUID) Line 405: imgPath = image.Image(repoPath).create(sdUUID, imgUUID) That is enough only for sparse volumes: if preallocate == volume.SPARSE_VOL: # Sparse = regular file oop.getProcessPool(dom.sdUUID).truncateFile(volPath, sizeBytes) else: try: # ddWatchCopy expects size to be in bytes misc.ddWatchCopy(/dev/zero, volPath, vars.task.aborting, sizeBytes) ... Anyway I'm sure we can do something like: oop.getProcessPool(dom.sdUUID).truncateFile(volPath, sizeBytes, ...) if preallocate == volume.PREALLOCATED_VOL: try: # ddWatchCopy expects size to be in bytes misc.ddWatchCopy(/dev/zero, volPath, vars.task.aborting, sizeBytes) ... About truncateFile, I'm not sure if we want to expose the posix flags, maybe we could just add an creatExcl bool: def truncateFile(path, size, mode=None, creatExcl=False): ... Anyway either way is ok with me. Line 406: Line 407: volPath = os.path.join(imgPath, volUUID) Line 408: volParent = None Line 409: volType = type2name(LEAF_VOL) -- To view, visit http://gerrit.ovirt.org/18274 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6f3b6ca8313070d0345b5f76ebb0b3d9772d14f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: After exception on _recoverExistingVms thread we should try ...
Federico Simoncelli has posted comments on this change. Change subject: After exception on _recoverExistingVms thread we should try again .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/17617 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I80745e04c7cce50e4cd8ce12628131b76d0659a7 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Barak Azulay bazu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: mooli tayer mta...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: After exception on _recoverExistingVms thread we should try ...
Federico Simoncelli has submitted this change and it was merged. Change subject: After exception on _recoverExistingVms thread we should try again .. After exception on _recoverExistingVms thread we should try again Currently we fail the process on exception and gets stuck on recovery. This might be because libvirt exception during first connect. Anyhow, we should continue try to recover to start serving vdsm requests. Change-Id: I80745e04c7cce50e4cd8ce12628131b76d0659a7 Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=991091 Signed-off-by: Yaniv Bronhaim ybron...@redhat.com Reviewed-on: http://gerrit.ovirt.org/17617 Reviewed-by: Michal Skrivanek michal.skriva...@redhat.com Reviewed-by: Federico Simoncelli fsimo...@redhat.com --- M vdsm/clientIF.py 1 file changed, 13 insertions(+), 6 deletions(-) Approvals: Yaniv Bronhaim: Verified Federico Simoncelli: Looks good to me, approved Michal Skrivanek: Looks good to me, but someone else must approve -- To view, visit http://gerrit.ovirt.org/17617 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I80745e04c7cce50e4cd8ce12628131b76d0659a7 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Barak Azulay bazu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Martin Polednik mpole...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: mooli tayer mta...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Making setStorageDomainDescription the SPM command
Federico Simoncelli has posted comments on this change. Change subject: Making setStorageDomainDescription the SPM command .. Patch Set 4: Code-Review-1 (1 comment) I dropped an idea to setStorageDomainDescription with shared lock instead of exclusive due to Eduardo review. Eduardo's review mentioned the verb not being an SPM verb and therefore race-prone. That has nothing to do with the getSharedLock/getExcelusiveLock in setStorageDomainDescription. File vdsm/storage/hsm.py Line 2703: vars.task.setDefaultException( Line 2704: se.StorageDomainActionError( Line 2705: sdUUID=%s, description=%s % (sdUUID, description))) Line 2706: dom = sdCache.produce(sdUUID=sdUUID) Line 2707: vars.task.getExclusiveLock(STORAGE, sdUUID) vars.task.getSharedLock(STORAGE, sdUUID) Line 2708: Line 2709: pool = self.getPool(dom.getPools()[0]) Line 2710: pool.validatePoolSD(sdUUID) Line 2711: pool.setSDDescription(sdUUID, description) -- To view, visit http://gerrit.ovirt.org/17198 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idca7b7b6642c222d88a6cd2a94d4033c0c3ef70b Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Eduardo ewars...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Remove redundant check that causes lvm cache to refresh ever...
Federico Simoncelli has posted comments on this change. Change subject: Remove redundant check that causes lvm cache to refresh every volume creation .. Patch Set 1: (1 comment) File vdsm/storage/volume.py Line 420: srcImgUUID = imgUUID Line 421: Line 422: volParent = cls(repoPath, sdUUID, srcImgUUID, srcVolUUID) Line 423: # Override the size with the size of the parent Line 424: size = volParent.getSize() Yes, I just spoke with Ayal and he said that he moved it by mistake. Line 425: Line 426: if imgUUID != srcImgUUID: Line 427: volParent.share(imgPath) Line 428: volParent = cls(repoPath, sdUUID, imgUUID, srcVolUUID) -- To view, visit http://gerrit.ovirt.org/18274 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6f3b6ca8313070d0345b5f76ebb0b3d9772d14f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: One shot prepare.
Federico Simoncelli has posted comments on this change. Change subject: One shot prepare. .. Patch Set 10: (2 comments) File vdsm/storage/blockSD.py Line 1033: Creates a compatiblity layer for vdsm and qemu-img use. Line 1034: Line 1035: srcImgPath: Dir where the image volumes are. Line 1036: Line 1037: sdRunDir = os.path.join(/var/run/vdsm/storage, self.sdUUID) I'm sure Dan won't mind if we use the Path definitions section in constants.py.in. Line 1038: imgRunDir = os.path.join(sdRunDir, imgUUID) Line 1039: fileUtils.createdir(imgRunDir) Line 1040: for volUUID in volUUIDs: Line 1041: srcVol = os.path.join(srcImgPath, volUUID) Line 1061: If the image is based on a template image it will be activated. Line 1062: Line 1063: volUUIDs = sd.getVolsOfImage(allVols, imgUUID).keys() Line 1064: lvm.activateLVs(self.sdUUID, volUUIDs) Line 1065: vgDir = os.path.join(/dev, self.sdUUID) For this you could just provide a getDomainDevPath method in this class. Line 1066: return self.linkRunImage(vgDir, imgUUID, volUUIDs) Line 1067: Line 1068: def getVolumeLease(self, imgUUID, volUUID): Line 1069: -- To view, visit http://gerrit.ovirt.org/4220 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id47db9c53199385dd5d08586e4782ea23885eb72 Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo ewars...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Eduardo ewars...@redhat.com Gerrit-Reviewer: Elad Ben Aharon eladba1...@gmail.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Greg Padgett gpadg...@redhat.com Gerrit-Reviewer: Haim Ateya hat...@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com Gerrit-Reviewer: Sandro Bonazzola sbona...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: One shot prepare.
Federico Simoncelli has posted comments on this change. Change subject: One shot prepare. .. Patch Set 10: (1 comment) File vdsm/clientIF.py Line 257: Line 258: volPath = res['path'] Line 259: # drive['volsInfo'] is unsorted! Line 260: drive['volumeInfo'] = res['imgVolumesInfo'] Line 261: drive['volumeInfo'] = res['info'] Don't forget to fix all the volumeChain uses in vm.py (and other files if any) Line 262: Line 263: # GUID drive format Line 264: elif GUID in drive: Line 265: visible = self.irs.scanDevicesVisibility([drive[GUID]]) -- To view, visit http://gerrit.ovirt.org/4220 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id47db9c53199385dd5d08586e4782ea23885eb72 Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo ewars...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Eduardo ewars...@redhat.com Gerrit-Reviewer: Elad Ben Aharon eladba1...@gmail.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Greg Padgett gpadg...@redhat.com Gerrit-Reviewer: Haim Ateya hat...@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com Gerrit-Reviewer: Sandro Bonazzola sbona...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Removing validate sd from spm verbs which calls ckvg and ref...
Federico Simoncelli has posted comments on this change. Change subject: Removing validate sd from spm verbs which calls ckvg and refreshes the domain metadata on the spm even though the spm already controls the changes (i.e. a lot of redundant time is wasted) .. Patch Set 1: (1 comment) File vdsm/storage/hsm.py Line 1857: except se.StoragePoolUnknown: Line 1858: pool = sp.StoragePool(spUUID, self.taskMng) Line 1859: else: Line 1860: raise se.StoragePoolConnected(spUUID) Line 1861: I don't think it's that risky. validateSdUUID is a misleading method as it's hiding its real purpose (in some cases) which is reloading the domain metadata. Since you're already converting most of the calls from self.validateSdUUID(...) to sdCache.produce(...), the remaining ones are: * validateSdUUID call added recently and missed during rebase * validateSdUUID call used to refresh the metadata * unknown use of validateSdUUID (what you're afraid to change) Now, the methods that are still using validateSdUUID are: * updateVolumeSize - unrelated to domain metadata refresh (probably one that was missed during the rebase) * refreshStoragePool - looks clearly like a refresh metadata call * uploadImage / downloadImage - missed during the rebase (safe to change) * reconstructMaster - looks clearly like a refresh metadata call * getStorageDomainInfo - make sense to keep it as a refresh metadata call What I was suggesting was to rename validateSdUUID to something more explicit (e.g. refreshDomainMetadata) and use it only where relevant (refreshStoragePool, reconstructMaster, getStorageDomainInfo). Line 1862: self.validateSdUUID(masterDom) Line 1863: Line 1864: if hostId is not None: Line 1865: misc.validateN(hostId, 'hostId') -- To view, visit http://gerrit.ovirt.org/18275 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibf597a9e53dad04ad7a01fc1d5ceb8a180a542f2 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: define.py: remove %s from imageErr
Federico Simoncelli has posted comments on this change. Change subject: define.py: remove %s from imageErr .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/18230 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia2b8534c34e8efb9d4af3c438a0a0ecc99cf16f0 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Remove redundant check that causes lvm cache to refresh ever...
Federico Simoncelli has posted comments on this change. Change subject: Remove redundant check that causes lvm cache to refresh every volume creation .. Patch Set 1: Code-Review-1 Marking for visibility -- To view, visit http://gerrit.ovirt.org/18274 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6f3b6ca8313070d0345b5f76ebb0b3d9772d14f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Remove redundant check that causes lvm cache to refresh ever...
Federico Simoncelli has posted comments on this change. Change subject: Remove redundant check that causes lvm cache to refresh every volume creation .. Patch Set 1: (3 comments) File vdsm/storage/volume.py Line 397: 'preallocate' - Preallocate / Sparse Line 398: 'diskType' - enum (API.Image.DiskTypes) Line 399: 'srcImgUUID' - source image UUID Line 400: 'srcVolUUID' - source volume UUID Line 401: Nice, we finally get rid of these. It was on my todo list since a long time! Line 402: cls.validateCreateVolumeParams(volFormat, preallocate, srcVolUUID) Line 403: Line 404: dom = sdCache.produce(sdUUID) Line 405: imgPath = image.Image(repoPath).create(sdUUID, imgUUID) Line 401: Line 402: cls.validateCreateVolumeParams(volFormat, preallocate, srcVolUUID) Line 403: Line 404: dom = sdCache.produce(sdUUID) Line 405: imgPath = image.Image(repoPath).create(sdUUID, imgUUID) Removing the volumeExists check might be safe on block domains but what about file domains? It looks to me that this wouldn't prevent you from (partially or completely) wiping an existing volume. Line 406: Line 407: volPath = os.path.join(imgPath, volUUID) Line 408: volParent = None Line 409: volType = type2name(LEAF_VOL) Line 420: srcImgUUID = imgUUID Line 421: Line 422: volParent = cls(repoPath, sdUUID, srcImgUUID, srcVolUUID) Line 423: # Override the size with the size of the parent Line 424: size = volParent.getSize() Any reason to move this? Line 425: Line 426: if imgUUID != srcImgUUID: Line 427: volParent.share(imgPath) Line 428: volParent = cls(repoPath, sdUUID, imgUUID, srcVolUUID) -- To view, visit http://gerrit.ovirt.org/18274 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6f3b6ca8313070d0345b5f76ebb0b3d9772d14f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Removing validate sd from spm verbs which calls ckvg and ref...
Federico Simoncelli has posted comments on this change. Change subject: Removing validate sd from spm verbs which calls ckvg and refreshes the domain metadata on the spm even though the spm already controls the changes (i.e. a lot of redundant time is wasted) .. Patch Set 1: Code-Review-1 (6 comments) Marking for visibility File vdsm/storage/hsm.py Line 683: Line 684: This synchronous method is intended to be used only with COW volumes Line 685: where the size can be updated simply changing the qcow2 header. Line 686: Line 687: newSizeBytes = int(newSize) Is there any reason for this to be left with validateSdUUID? Line 688: domain = self.validateSdUUID(sdUUID) Line 689: volToExtend = domain.produceVolume(imgUUID, volUUID) Line 690: volPath = volToExtend.getVolumePath() Line 691: volFormat = volToExtend.getFormat() Line 869: return Line 870: Line 871: pool = self.getPool(spUUID) Line 872: Line 873: try: Maybe this is worthy of the validateSdUUID. The point here is that what's *hidden* inside validateSdUUID is a metadata reload (see validate in blockSD/fileSD). What would be best here is to separate the metadata reload in an explicit verb (reloadMetadata?). Line 874: self.validateSdUUID(msdUUID) Line 875: pool.refresh(msdUUID, masterVersion) Line 876: except: Line 877: self._disconnectPool(pool, pool.id, pool.scsiKey, False) Line 1671: Line 1672: Upload an image to a remote endpoint using the specified method and Line 1673: methodArgs. Line 1674: Line 1675: self.validateSdUUID(sdUUID) Is there any reason for this to be left with validateSdUUID? Line 1676: pool = self.getPool(spUUID) Line 1677: # NOTE: this could become an hsm task Line 1678: self._spmSchedule(spUUID, uploadImage, pool.uploadImage, Line 1679: methodArgs, sdUUID, imgUUID, volUUID) Line 1683: Line 1684: Download an image from a remote endpoint using the specified method Line 1685: and methodArgs. Line 1686: Line 1687: self.validateSdUUID(sdUUID) Is there any reason for this to be left with validateSdUUID? Line 1688: pool = self.getPool(spUUID) Line 1689: # NOTE: this could become an hsm task, in such case the LV extension Line 1690: # required to prepare the destination should go through the mailbox. Line 1691: self._spmSchedule(spUUID, downloadImage, pool.downloadImage, Line 1857: except se.StoragePoolUnknown: Line 1858: pool = sp.StoragePool(spUUID, self.taskMng) Line 1859: else: Line 1860: raise se.StoragePoolConnected(spUUID) Line 1861: This is probably the same as refresh. We want to keep it only to force the reload of the metadata. Line 1862: self.validateSdUUID(masterDom) Line 1863: Line 1864: if hostId is not None: Line 1865: misc.validateN(hostId, 'hostId') Line 2717: :returns: a dict containing the information about the domain. Line 2718: :rtype: dict Line 2719: Line 2720: vars.task.setDefaultException( Line 2721: se.StorageDomainActionError(sdUUID=%s % sdUUID)) Again fresh metadata, or just an overlook? Line 2722: dom = self.validateSdUUID(sdUUID) Line 2723: # getSharedLock(connectionsResource...) Line 2724: Line 2725: vars.task.getSharedLock(STORAGE, sdUUID) -- To view, visit http://gerrit.ovirt.org/18275 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibf597a9e53dad04ad7a01fc1d5ceb8a180a542f2 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: define.py: remove %s from imageErr
Federico Simoncelli has submitted this change and it was merged. Change subject: define.py: remove %s from imageErr .. define.py: remove %s from imageErr imageErr untypically has an %s in the error message. To make things worse, most flows do not fill it, producing messages which are difficult to understand. This patch removes the aforementioned %s. Change-Id: Ia2b8534c34e8efb9d4af3c438a0a0ecc99cf16f0 Bug-Url: https://bugzilla.redhat.com/960635 Signed-off-by: Allon Mureinik amure...@redhat.com Reviewed-on: http://gerrit.ovirt.org/18230 Reviewed-by: Federico Simoncelli fsimo...@redhat.com --- M lib/vdsm/define.py M vdsm/vm.py 2 files changed, 3 insertions(+), 5 deletions(-) Approvals: Federico Simoncelli: Looks good to me, approved Allon Mureinik: Verified -- To view, visit http://gerrit.ovirt.org/18230 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ia2b8534c34e8efb9d4af3c438a0a0ecc99cf16f0 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Handling ssl error specifically instead of printing all stac...
Federico Simoncelli has posted comments on this change. Change subject: Handling ssl error specifically instead of printing all stacktrace .. Patch Set 2: Code-Review-1 (1 comment) Let's just use the regular log string formatting and merge this. File vdsm/BindingXMLRPC.py Line 67: while self._enabled: Line 68: try: Line 69: self.server.handle_request() Line 70: except SSLError as e: Line 71: self.log.error(xml-rpc SSL error: {0}. from {1}.format( I love the idea of this patch. But I don't think it's the right moment to stop using the string formatting provided by log.error(...). Line 72:e.message, self.server.lastClient)) Line 73: except Exception as e: Line 74: if e[0] != EINTR: Line 75: self.log.error(xml-rpc handler exception, -- To view, visit http://gerrit.ovirt.org/17917 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I47556f62aef6f63a66fa47ecb4d693082da4dee1 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: mooli tayer mta...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Handling ssl error specifically instead of printing all stac...
Federico Simoncelli has posted comments on this change. Change subject: Handling ssl error specifically instead of printing all stacktrace .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/17917 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I47556f62aef6f63a66fa47ecb4d693082da4dee1 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: mooli tayer mta...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: hsm: deleteImage fails because of wrong dictionary use
Federico Simoncelli has posted comments on this change. Change subject: hsm: deleteImage fails because of wrong dictionary use .. Patch Set 1: Code-Review-1 (2 comments) File vdsm/storage/hsm.py Line 1525: # hence no need to create fake template if postZero is true. Line 1526: self._spmSchedule(spUUID, zeroImage_%s % imgUUID, dom.zeroImage, Line 1527: sdUUID, imgUUID, volsByImg) Line 1528: else: Line 1529: dom.deleteImage(sdUUID, imgUUID, volsByImg) Do we want to optimize in case the image is already a fake template? vol.getLegality() == volume.FAKE_VOL In such case we can avoid to delete and recreate another fake volume. Line 1530: # This is a hack to keep the interface consistent Line 1531: # We currently have race conditions in delete image, to quickly fix Line 1532: # this we delete images in the synchronous state. This only works Line 1533: # because Engine does not send two requests at a time. This hack is Line 1536: # an eliminate all race conditions Line 1537: if needFake: Line 1538: img = image.Image(os.path.join(self.storage_repository, Line 1539:spUUID)) Line 1540: tName = volsByImg.keys()[0] This might work but it's potentially wrong. It works only because you have a dictionary with 1 key (but this is rather implicit) otherwise a random key would be selected. We could try to do some cleanup, remove the (unnecessary) isTemplateWithChildren/needFake and get the fake volume uuid from the loop at line 1508: fakeVolUUID = None for k, v in volsByImg.iteritems(): if len(v.imgs) 1 and v.imgs[0] == imgUUID: if dom.isBackup(): fakeVolUUID = k else: raise se.CannotDeleteSharedVolume(...) ... if fakeVolUUID is not None: ... tParams = dom.produceVolume(imgUUID, fakeVolUUID).getVolumeParams() ... Line 1541: tParams = dom.produceVolume(imgUUID, tName).getVolumeParams() Line 1542: img.createFakeTemplate(sdUUID=sdUUID, volParams=tParams) Line 1543: self._spmSchedule(spUUID, deleteImage_%s % imgUUID, lambda: True) Line 1544: -- To view, visit http://gerrit.ovirt.org/17383 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I81f9a5aa63c0914e3b934046454df64ccd39c269 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Eduardo ewars...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: One shot prepare.
Federico Simoncelli has posted comments on this change. Change subject: One shot prepare. .. Patch Set 10: Code-Review-1 (6 comments) File vdsm/storage/blockSD.py Line 1033: Creates a compatiblity layer for vdsm and qemu-img use. Line 1034: Line 1035: srcImgPath: Dir where the image volumes are. Line 1036: Line 1037: sdRunDir = os.path.join(/var/run/vdsm/storage, self.sdUUID) This should be a constant. Line 1038: imgRunDir = os.path.join(sdRunDir, imgUUID) Line 1039: fileUtils.createdir(imgRunDir) Line 1040: for volUUID in volUUIDs: Line 1041: srcVol = os.path.join(srcImgPath, volUUID) Line 1061: If the image is based on a template image it will be activated. Line 1062: Line 1063: volUUIDs = sd.getVolsOfImage(allVols, imgUUID).keys() Line 1064: lvm.activateLVs(self.sdUUID, volUUIDs) Line 1065: vgDir = os.path.join(/dev, self.sdUUID) os.path.join(/dev, ... is already appearing so many times in this file that it makes me think that we need to encapsulate it. Line 1066: return self.linkRunImage(vgDir, imgUUID, volUUIDs) Line 1067: Line 1068: def getVolumeLease(self, imgUUID, volUUID): Line 1069: File vdsm/storage/fileSD.py Line 423: Creates a compatiblity layer for vdsm and qemu-img use. Line 424: Line 425: srcImgPath: Dir where the image volumes are. Line 426: Line 427: sdRunDir = os.path.join(/var/run/vdsm/storage, self.sdUUID) This should be a constant. Line 428: fileUtils.createdir(sdRunDir) Line 429: imgRunDir = os.path.join(sdRunDir, imgUUID) Line 430: try: Line 431: os.symlink(srcImgPath, imgRunDir) Line 430: try: Line 431: os.symlink(srcImgPath, imgRunDir) Line 432: except OSError as e: Line 433: if e.errno == errno.EEXIST: Line 434: self.log.debug(img run dir already exists: %s, imgRunDir) This implicitly assumes that the same storage domain cannot be accessed using different nfs hosts (you could use vdsm.utils.forceLink). Line 435: else: Line 436: self.log.error(Failed to create img run dir: %s, imgRunDir) Line 437: raise Line 438: Line 435: else: Line 436: self.log.error(Failed to create img run dir: %s, imgRunDir) Line 437: raise Line 438: Line 439: return imgRunDir I see some code/logic duplication here in linkRunImage (same as blockSD). Maybe we can extract it in some common method/function. Line 440: Line 441: def activateVolumes(self, imgUUID, allVols): Line 442: Line 443: Activate all the volumes listed in volUUIDs Line 446: # set. We have to set it here if we want qemu-kvm to write to old Line 447: # NFS volumes. In theory it is necessary to fix the permission Line 448: # of the leaf only but to not introduce an additional requirement Line 449: # (ordered volUUIDs) we fix them all. Line 450: imgDir = os.path.join(self.mountpoint, self.sdUUID, sd.DOMAIN_IMAGES, Another solution to the stale symlinks would be using the /rhev/spuuid/sduuid path here instead of self.mountpoint/sduuid. Line 451: imgUUID) Line 452: volUUIDs = sd.getVolsOfImage(allVols, imgUUID).keys() Line 453: volPaths = tuple(os.path.join(imgDir, v) for v in volUUIDs) Line 454: for volPath in volPaths: -- To view, visit http://gerrit.ovirt.org/4220 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id47db9c53199385dd5d08586e4782ea23885eb72 Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo ewars...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Eduardo ewars...@redhat.com Gerrit-Reviewer: Elad Ben Aharon eladba1...@gmail.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Greg Padgett gpadg...@redhat.com Gerrit-Reviewer: Haim Ateya hat...@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com Gerrit-Reviewer: Sandro Bonazzola sbona...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: storage: kill ongoing operations on exit
Federico Simoncelli has posted comments on this change. Change subject: storage: kill ongoing operations on exit .. Patch Set 1: (1 comment) File lib/vdsm/utils.py Line 517: return [l[:-1] if l.endswith('\n') else l for l in lines] Line 518: Line 519: Line 520: def watchCmd(command, stop, cwd=None, data=None, recoveryCallback=None, Line 521: nice=None, ioclass=None, execCmdLogger=logging.root, right, but the other mistake is in 'execCmd' where it should be False by default. Line 522: killOnExit=False): Line 523: Line 524: Executes an external command, optionally via sudo with stop abilities. Line 525: -- To view, visit http://gerrit.ovirt.org/18074 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic6ff67cb687494099e332d2d7b1eb3a293d1b4e4 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: One shot prepare.
Federico Simoncelli has posted comments on this change. Change subject: One shot prepare. .. Patch Set 10: (1 comment) File vdsm/clientIF.py Line 257: Line 258: volPath = res['path'] Line 259: # drive['volsInfo'] is unsorted! Line 260: drive['volumeInfo'] = res['imgVolumesInfo'] Line 261: drive['volumeInfo'] = res['info'] This must be addressed ASAP as it's preventing (all) the leases to be passed to the vm. Line 262: Line 263: # GUID drive format Line 264: elif GUID in drive: Line 265: visible = self.irs.scanDevicesVisibility([drive[GUID]]) -- To view, visit http://gerrit.ovirt.org/4220 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id47db9c53199385dd5d08586e4782ea23885eb72 Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo ewars...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Eduardo ewars...@redhat.com Gerrit-Reviewer: Elad Ben Aharon eladba1...@gmail.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Greg Padgett gpadg...@redhat.com Gerrit-Reviewer: Haim Ateya hat...@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com Gerrit-Reviewer: Sandro Bonazzola sbona...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: hsm: deleteImage fails because of wrong dictionary use
Federico Simoncelli has posted comments on this change. Change subject: hsm: deleteImage fails because of wrong dictionary use .. Patch Set 1: (2 comments) File vdsm/storage/hsm.py Line 1525: # hence no need to create fake template if postZero is true. Line 1526: self._spmSchedule(spUUID, zeroImage_%s % imgUUID, dom.zeroImage, Line 1527: sdUUID, imgUUID, volsByImg) Line 1528: else: Line 1529: dom.deleteImage(sdUUID, imgUUID, volsByImg) A fake volume is a small void volume, there is no real gain optimizing here. Check out what createFakeTemplate and its createVolume do. It looks to me like there's a lot to optimize out. Anyway you wouldn't have to check for FAKE_VOL every time, but only if needFake == True: if needFake == True and vol.getLegality() == volume.FAKE_VOL: self._spmSchedule(spUUID, deleteImage_%s % imgUUID, lambda: True) return # done Line 1530: # This is a hack to keep the interface consistent Line 1531: # We currently have race conditions in delete image, to quickly fix Line 1532: # this we delete images in the synchronous state. This only works Line 1533: # because Engine does not send two requests at a time. This hack is Line 1536: # an eliminate all race conditions Line 1537: if needFake: Line 1538: img = image.Image(os.path.join(self.storage_repository, Line 1539:spUUID)) Line 1540: tName = volsByImg.keys()[0] Can't be more than one volume in a template image since ovirt images require that the template image is composed by a unique volume. Yes, I stated that as first thing, It works only because you have a dictionary with 1 key (but this is rather implicit) For performance reasons we avoid to read MD multiple times and we want to simplify the logic as far we can. The complexity is a product of the ovirt images layout. Irrelevant here as the number of operations are the same. I'm in fact trying to simplify the logic here removing two unnecessary flags. Line 1541: tParams = dom.produceVolume(imgUUID, tName).getVolumeParams() Line 1542: img.createFakeTemplate(sdUUID=sdUUID, volParams=tParams) Line 1543: self._spmSchedule(spUUID, deleteImage_%s % imgUUID, lambda: True) Line 1544: -- To view, visit http://gerrit.ovirt.org/17383 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I81f9a5aa63c0914e3b934046454df64ccd39c269 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Eduardo ewars...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Yeela Kaplan ykap...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: storage: kill ongoing operations on exit
Federico Simoncelli has uploaded a new change for review. Change subject: storage: kill ongoing operations on exit .. storage: kill ongoing operations on exit Some storage operations must be forcibly interrupted when the vdsm (parent) process exits. Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=713434 Change-Id: Ic6ff67cb687494099e332d2d7b1eb3a293d1b4e4 Signed-off-by: Federico Simoncelli fsimo...@redhat.com --- M lib/vdsm/utils.py M vdsm/storage/blockSD.py M vdsm/storage/curlImgWrap.py M vdsm/storage/misc.py M vdsm/storage/sp.py M vdsm/storage/storage_mailbox.py M vdsm/storage/volume.py 7 files changed, 35 insertions(+), 21 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/74/18074/1 diff --git a/lib/vdsm/utils.py b/lib/vdsm/utils.py index 538498f..8890160 100644 --- a/lib/vdsm/utils.py +++ b/lib/vdsm/utils.py @@ -458,7 +458,8 @@ def execCmd(command, sudo=False, cwd=None, data=None, raw=False, logErr=True, printable=None, env=None, sync=True, nice=None, ioclass=None, -ioclassdata=None, setsid=False, execCmdLogger=logging.root): +ioclassdata=None, setsid=False, execCmdLogger=logging.root, +killOnExit=True): Executes an external command, optionally via sudo. @@ -485,7 +486,8 @@ cmdline = repr(subprocess.list2cmdline(printable)) execCmdLogger.debug(%s (cwd %s), cmdline, cwd) -p = BetterPopen(command, close_fds=True, cwd=cwd, env=env) +p = BetterPopen(command, close_fds=True, cwd=cwd, env=env, +deathSignal=(signal.SIGKILL if killOnExit else 0)) p = AsyncProc(p) if not sync: if data is not None: @@ -516,12 +518,14 @@ def watchCmd(command, stop, cwd=None, data=None, recoveryCallback=None, - nice=None, ioclass=None, execCmdLogger=logging.root): + nice=None, ioclass=None, execCmdLogger=logging.root, + killOnExit=False): Executes an external command, optionally via sudo with stop abilities. proc = execCmd(command, sudo=False, cwd=cwd, data=data, sync=False, - nice=nice, ioclass=ioclass, execCmdLogger=execCmdLogger) + nice=nice, ioclass=ioclass, execCmdLogger=execCmdLogger, + killOnExit=killOnExit) if recoveryCallback: recoveryCallback(proc) diff --git a/vdsm/storage/blockSD.py b/vdsm/storage/blockSD.py index e8b2980..baca437 100644 --- a/vdsm/storage/blockSD.py +++ b/vdsm/storage/blockSD.py @@ -204,7 +204,8 @@ of=%s % lvm.lvPath(sdUUID, volUUID), bs=%s % BS, count=%s % count] p = misc.execCmd(cmd, sudo=False, sync=False, - nice=utils.NICENESS.HIGH, ioclass=utils.IOCLASS.IDLE) + nice=utils.NICENESS.HIGH, ioclass=utils.IOCLASS.IDLE, + killOnExit=True) return p @@ -1084,7 +1085,7 @@ # If there is a journal already tune2fs will do nothing, indicating # this condition only with exit code. However, we do not really care. cmd = [constants.EXT_TUNE2FS, -j, masterfsdev] -misc.execCmd(cmd, sudo=True) +misc.execCmd(cmd, sudo=True, killOnExit=True) masterMount = mount.Mount(masterfsdev, masterDir) @@ -1274,7 +1275,7 @@ Create a special file system to store VM data cmd = [constants.EXT_MKFS, -q, -j, -E, nodiscard, dev] -rc = misc.execCmd(cmd, sudo=True)[0] +rc = misc.execCmd(cmd, sudo=True, killOnExit=True)[0] if rc != 0: raise se.MkfsError(dev) diff --git a/vdsm/storage/curlImgWrap.py b/vdsm/storage/curlImgWrap.py index 2f63311..e9b574e 100644 --- a/vdsm/storage/curlImgWrap.py +++ b/vdsm/storage/curlImgWrap.py @@ -63,7 +63,7 @@ cmd = [constants.EXT_CURL_IMG_WRAP, --download] cmd.extend(_headersToOptions(headers) + [path, url]) -rc, out, err = utils.execCmd(cmd) +rc, out, err = utils.execCmd(cmd, killOnExit=True) if rc != 0: raise CurlError(rc, out, err) @@ -73,7 +73,7 @@ cmd = [constants.EXT_CURL_IMG_WRAP, --upload] cmd.extend(_headersToOptions(headers) + [path, url]) -rc, out, err = utils.execCmd(cmd) +rc, out, err = utils.execCmd(cmd, killOnExit=True) if rc != 0: raise CurlError(rc, out, err) diff --git a/vdsm/storage/misc.py b/vdsm/storage/misc.py index fe3a870..013eded 100644 --- a/vdsm/storage/misc.py +++ b/vdsm/storage/misc.py @@ -367,12 +367,14 @@ if not stop: (rc, out, err) = execCmd(cmd, sudo=False, nice=utils.NICENESS.HIGH, - ioclass=utils.IOCLASS.IDLE) + ioclass=utils.IOCLASS.IDLE, + killOnExit=True) else: (rc, out, err) = watchCmd(cmd, stop=stop, recoveryCallback=recoveryCallback,
Change in vdsm[master]: Handling signals interrupts, using pidofproc and prevent loc...
Federico Simoncelli has posted comments on this change. Change subject: Handling signals interrupts, using pidofproc and prevent lock over systemd .. Patch Set 6: Code-Review+2 a comment about a future use of flock is good enough -- To view, visit http://gerrit.ovirt.org/17926 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9b58672957840c9dc5d13dce6af8de1717a9cb2d Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Elad Ben Aharon eladba1...@gmail.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ohad Basan oba...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: mooli tayer mta...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Handling signals interrupts, using pidofproc and prevent loc...
Federico Simoncelli has submitted this change and it was merged. Change subject: Handling signals interrupts, using pidofproc and prevent lock over systemd .. Handling signals interrupts, using pidofproc and prevent lock over systemd Under start lock, if signal is received, we should clean the lock file and stop the instance. During stop we should not stop the process until respawn and vdsm instances are cleared. Instead of checking the existence of respawn.pid and vdsmd.pid files, using pidofproc checks if the processes are actually run. The deletion of the respawn.pid file is redundant and leads to conflicts. Over systemd we don't require to use locking at all. Systemd takes care of that. Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=994912 Change-Id: I9b58672957840c9dc5d13dce6af8de1717a9cb2d Signed-off-by: Yaniv Bronhaim ybron...@redhat.com Reviewed-on: http://gerrit.ovirt.org/17926 Tested-by: Elad Ben Aharon eladba1...@gmail.com Reviewed-by: Saggi Mizrahi smizr...@redhat.com Reviewed-by: Federico Simoncelli fsimo...@redhat.com --- M vdsm/vdsmd.init.in 1 file changed, 26 insertions(+), 6 deletions(-) Approvals: Elad Ben Aharon: Verified Yaniv Bronhaim: Verified Saggi Mizrahi: Looks good to me, but someone else must approve Federico Simoncelli: Looks good to me, approved -- To view, visit http://gerrit.ovirt.org/17926 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I9b58672957840c9dc5d13dce6af8de1717a9cb2d Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: Alon Bar-Lev alo...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Elad Ben Aharon eladba1...@gmail.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ohad Basan oba...@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizr...@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: mooli tayer mta...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: image: use successor volume size when merging
Federico Simoncelli has posted comments on this change. Change subject: image: use successor volume size when merging .. Patch Set 2: Verified+1 Code-Review+2 -- To view, visit http://gerrit.ovirt.org/17093 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic0bfd2ffa080bf89709ae543043b52aad27eb759 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Eduardo ewars...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: image: use successor volume size when merging
Federico Simoncelli has submitted this change and it was merged. Change subject: image: use successor volume size when merging .. image: use successor volume size when merging Volumes in the same image chain might have different capacity since the introduction of the disk resize feature. This means that when we merge volumes the ancestor should get the new size from the successor in order to be able to contain the additional data that we are collapsing. Change-Id: Ic0bfd2ffa080bf89709ae543043b52aad27eb759 Signed-off-by: Federico Simoncelli fsimo...@redhat.com Reviewed-on: http://gerrit.ovirt.org/17093 Reviewed-by: Eduardo ewars...@redhat.com --- M vdsm/storage/image.py 1 file changed, 6 insertions(+), 1 deletion(-) Approvals: Federico Simoncelli: Verified; Looks good to me, approved Eduardo: Looks good to me, but someone else must approve -- To view, visit http://gerrit.ovirt.org/17093 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ic0bfd2ffa080bf89709ae543043b52aad27eb759 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Ayal Baron aba...@redhat.com Gerrit-Reviewer: Eduardo ewars...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Sergey Gotliv sgot...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.3]: image: use successor volume size when merging
Federico Simoncelli has posted comments on this change. Change subject: image: use successor volume size when merging .. Patch Set 1: Verified+1 Code-Review+2 Same as master -- To view, visit http://gerrit.ovirt.org/18025 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic0bfd2ffa080bf89709ae543043b52aad27eb759 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.3 Gerrit-Owner: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Eduardo ewars...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches