Change in vdsm[master]: vm: unify _highWrite and _onAbnormalStop
Dan Kenigsberg has submitted this change and it was merged. Change subject: vm: unify _highWrite and _onAbnormalStop .. vm: unify _highWrite and _onAbnormalStop Both _highWrite and _onAbnormalStop should share the same logic about the volume extension. Change-Id: I4fd790986b81402847b06810529841cfd7bae119 Signed-off-by: Federico Simoncelli Reviewed-on: http://gerrit.ovirt.org/20966 Reviewed-by: Sergey Gotliv Reviewed-by: Ayal Baron --- M vdsm/vm.py 1 file changed, 24 insertions(+), 35 deletions(-) Approvals: Ayal Baron: Looks good to me, approved Federico Simoncelli: Verified Sergey Gotliv: Looks good to me, but someone else must approve -- To view, visit http://gerrit.ovirt.org/20966 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I4fd790986b81402847b06810529841cfd7bae119 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Sergey Gotliv 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]: vm: unify _highWrite and _onAbnormalStop
Ayal Baron has posted comments on this change. Change subject: vm: unify _highWrite and _onAbnormalStop .. Patch Set 7: Code-Review+2 The refactoring here looks fine -- To view, visit http://gerrit.ovirt.org/20966 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4fd790986b81402847b06810529841cfd7bae119 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Sergey Gotliv 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: unify _highWrite and _onAbnormalStop
Sergey Gotliv has posted comments on this change. Change subject: vm: unify _highWrite and _onAbnormalStop .. Patch Set 7: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/20966 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4fd790986b81402847b06810529841cfd7bae119 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Sergey Gotliv 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: unify _highWrite and _onAbnormalStop
oVirt Jenkins CI Server has posted comments on this change. Change subject: vm: unify _highWrite and _onAbnormalStop .. Patch Set 7: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4573/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5373/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5452/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/20966 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4fd790986b81402847b06810529841cfd7bae119 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Sergey Gotliv 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: unify _highWrite and _onAbnormalStop
Federico Simoncelli has posted comments on this change. Change subject: vm: unify _highWrite and _onAbnormalStop .. Patch Set 7: Verified+1 Verified (before the renaming). -- To view, visit http://gerrit.ovirt.org/20966 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4fd790986b81402847b06810529841cfd7bae119 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Sergey Gotliv 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: unify _highWrite and _onAbnormalStop
Nir Soffer has posted comments on this change. Change subject: vm: unify _highWrite and _onAbnormalStop .. Patch Set 6: Rebase? -- To view, visit http://gerrit.ovirt.org/20966 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4fd790986b81402847b06810529841cfd7bae119 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Sergey Gotliv 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: unify _highWrite and _onAbnormalStop
oVirt Jenkins CI Server has posted comments on this change. Change subject: vm: unify _highWrite and _onAbnormalStop .. Patch Set 6: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4559/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5359/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5438/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/20966 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4fd790986b81402847b06810529841cfd7bae119 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Sergey Gotliv 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: unify _highWrite and _onAbnormalStop
Sergey Gotliv has posted comments on this change.
Change subject: vm: unify _highWrite and _onAbnormalStop
..
Patch Set 5:
(2 comments)
I would consider to change the name of the method, except of that looks great.
File vdsm/vm.py
Line 2279: self.log.debug('new rtc offset %s', timeOffset)
Line 2280: with self._confLock:
Line 2281: self.conf['timeOffset'] = timeOffset
Line 2282:
Line 2283: def checkDrivesForExtension(self):
Agree.
Line 2284: extend = []
Line 2285:
Line 2286: for drive in self._devices[DISK_DEVICES]:
Line 2287: if not drive.blockDev or drive.format != 'cow':
Line 2298: drive.volumeID, drive.domainID, drive.apparentsize,
capacity,
Line 2299: alloc, physical)
Line 2300: self.extendDriveVolume(drive)
Line 2301:
Line 2302: return len(extend) > 0
I don't agree here, more than that this patch won't be merged without another,
we all know that, so...
Line 2303:
Line 2304: def extendDriveVolume(self, vmDrive):
Line 2305: if not vmDrive.blockDev:
Line 2306: return
--
To view, visit http://gerrit.ovirt.org/20966
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4fd790986b81402847b06810529841cfd7bae119
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli
Gerrit-Reviewer: Ayal Baron
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Federico Simoncelli
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: Sergey Gotliv
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: unify _highWrite and _onAbnormalStop
Nir Soffer has posted comments on this change. Change subject: vm: unify _highWrite and _onAbnormalStop .. Patch Set 5: (1 comment) File vdsm/vm.py Line 2298: drive.volumeID, drive.domainID, drive.apparentsize, capacity, Line 2299: alloc, physical) Line 2300: self.extendDriveVolume(drive) Line 2301: Line 2302: return len(extend) > 0 I see that there is another patch depending on this, that will use the extend list. However if this patch will be merge without the other then we are left with strange code. The extend list should be added by the other patch. Line 2303: Line 2304: def extendDriveVolume(self, vmDrive): Line 2305: if not vmDrive.blockDev: Line 2306: return -- To view, visit http://gerrit.ovirt.org/20966 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4fd790986b81402847b06810529841cfd7bae119 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Sergey Gotliv 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: unify _highWrite and _onAbnormalStop
Nir Soffer has posted comments on this change.
Change subject: vm: unify _highWrite and _onAbnormalStop
..
Patch Set 5:
(3 comments)
File vdsm/vm.py
Line 2279: self.log.debug('new rtc offset %s', timeOffset)
Line 2280: with self._confLock:
Line 2281: self.conf['timeOffset'] = timeOffset
Line 2282:
Line 2283: def checkDrivesForExtension(self):
This name is misleading - you don't expect that "check" will trigger an
extension.
How about:
def extendDrivesIfNeeded():
One can expect that this method will check which drives should be extended and
extend them.
Line 2284: extend = []
Line 2285:
Line 2286: for drive in self._devices[DISK_DEVICES]:
Line 2287: if not drive.blockDev or drive.format != 'cow':
Line 2298: drive.volumeID, drive.domainID, drive.apparentsize,
capacity,
Line 2299: alloc, physical)
Line 2300: self.extendDriveVolume(drive)
Line 2301:
Line 2302: return len(extend) > 0
Now that we don't have the check for alloc > capacity, why do we need the
extend list?
We can do:
extended = 0
for drive in drives:
skip irelevant drives
get drive info
if drive needs extension:
log
extend it
extended +=1
return extended
Line 2303:
Line 2304: def extendDriveVolume(self, vmDrive):
Line 2305: if not vmDrive.blockDev:
Line 2306: return
Line 4277: self.conf['pauseCode'] = err.upper()
Line 4278: self._guestCpuRunning = False
Line 4279: if err.upper() == 'ENOSPC':
Line 4280: if not self.checkDrivesForExtension():
Line 4281: self.log.info("No VM drives were extended")
Warning or an error?
Line 4282:
Line 4283: def _acpiShutdown(self):
Line 4284:
self._dom.shutdownFlags(libvirt.VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN)
Line 4285:
--
To view, visit http://gerrit.ovirt.org/20966
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4fd790986b81402847b06810529841cfd7bae119
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli
Gerrit-Reviewer: Ayal Baron
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Federico Simoncelli
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: Sergey Gotliv
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: unify _highWrite and _onAbnormalStop
oVirt Jenkins CI Server has posted comments on this change. Change subject: vm: unify _highWrite and _onAbnormalStop .. Patch Set 5: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4540/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5340/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5419/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/20966 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4fd790986b81402847b06810529841cfd7bae119 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Sergey Gotliv 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: unify _highWrite and _onAbnormalStop
Sergey Gotliv has posted comments on this change. Change subject: vm: unify _highWrite and _onAbnormalStop .. Patch Set 4: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/20966 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4fd790986b81402847b06810529841cfd7bae119 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Sergey Gotliv 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: unify _highWrite and _onAbnormalStop
oVirt Jenkins CI Server has posted comments on this change. Change subject: vm: unify _highWrite and _onAbnormalStop .. Patch Set 4: Build Successful http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4472/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5272/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5350/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/20966 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4fd790986b81402847b06810529841cfd7bae119 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Sergey Gotliv 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
