Change in vdsm[master]: virt: Move Vm._getUnderlyingDriveInfo() out of Vm
gerrit-hooks has posted comments on this change. Change subject: virt: Move Vm._getUnderlyingDriveInfo() out of Vm .. Patch Set 14: * Update tracker: IGNORE, no Bug-Url found * Set MODIFIED::IGNORE, no Bug-Url found. -- To view, visit https://gerrit.ovirt.org/53677 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6b372f86b82da7422727f3d084b9afc5505a289 Gerrit-PatchSet: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: Move Vm._getUnderlyingDriveInfo() out of Vm
Dan Kenigsberg has submitted this change and it was merged. Change subject: virt: Move Vm._getUnderlyingDriveInfo() out of Vm .. virt: Move Vm._getUnderlyingDriveInfo() out of Vm This generally follows the Sound device example, but involves a bit more code. Change-Id: Ib6b372f86b82da7422727f3d084b9afc5505a289 Signed-off-by: Milan Zamazal Reviewed-on: https://gerrit.ovirt.org/53677 Continuous-Integration: Jenkins CI Reviewed-by: Nir Soffer Reviewed-by: Francesco Romani --- M tests/devices/data/testComplexVm.xml M vdsm/virt/vm.py M vdsm/virt/vmdevices/storage.py 3 files changed, 115 insertions(+), 106 deletions(-) Approvals: Nir Soffer: Looks good to me, but someone else must approve Jenkins CI: Passed CI tests Francesco Romani: Looks good to me, approved Milan Zamazal: Verified -- To view, visit https://gerrit.ovirt.org/53677 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ib6b372f86b82da7422727f3d084b9afc5505a289 Gerrit-PatchSet: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: Move Vm._getUnderlyingDriveInfo() out of Vm
Milan Zamazal has posted comments on this change. Change subject: virt: Move Vm._getUnderlyingDriveInfo() out of Vm .. Patch Set 11: (1 comment) https://gerrit.ovirt.org/#/c/53677/11/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 1723: devices = self._devices Line 1724: vmdevices.network.Interface.update_device_info( Line 1725: self, devices[hwclass.NIC]) Line 1726: vmdevices.storage.Drive.update_device_info( Line 1727: self, devices[hwclass.DISK]) > I don't like them because updating the conf/devices from xml should not be > I don't like them because updating the conf/devices from xml should not be > the responsibility of the Device class. I see. I think it's desirable to group code by devices. Most device classes just create and process domain XML and it wouldn't be good to perform those two transformations in separate places. Properties of a given device should be handled in a single place, as a form of encapsulation. > Also in this design the code for handling the xml is spread all over the > place, instead of one module handling this task. We should generally improve XML handling, it's mess. If we can do that then this shouldn't be a problem. Line 1728: vmdevices.core.Sound.update_device_info( Line 1729: self, devices[hwclass.SOUND]) Line 1730: vmdevices.core.Video.update_device_info( Line 1731: self, devices[hwclass.VIDEO]) -- To view, visit https://gerrit.ovirt.org/53677 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6b372f86b82da7422727f3d084b9afc5505a289 Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: Move Vm._getUnderlyingDriveInfo() out of Vm
Francesco Romani has posted comments on this change. Change subject: virt: Move Vm._getUnderlyingDriveInfo() out of Vm .. Patch Set 13: Code-Review+2 -- To view, visit https://gerrit.ovirt.org/53677 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6b372f86b82da7422727f3d084b9afc5505a289 Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: Move Vm._getUnderlyingDriveInfo() out of Vm
Nir Soffer has posted comments on this change. Change subject: virt: Move Vm._getUnderlyingDriveInfo() out of Vm .. Patch Set 13: Code-Review+1 Thanks, looks better now. -- To view, visit https://gerrit.ovirt.org/53677 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6b372f86b82da7422727f3d084b9afc5505a289 Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: Move Vm._getUnderlyingDriveInfo() out of Vm
Nir Soffer has posted comments on this change. Change subject: virt: Move Vm._getUnderlyingDriveInfo() out of Vm .. Patch Set 11: (1 comment) https://gerrit.ovirt.org/#/c/53677/11/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 1723: devices = self._devices Line 1724: vmdevices.network.Interface.update_device_info( Line 1725: self, devices[hwclass.NIC]) Line 1726: vmdevices.storage.Drive.update_device_info( Line 1727: self, devices[hwclass.DISK]) > Why don't you like them? Because they are class methods or for another reas I don't like them because updating the conf/devices from xml should not be the responsibility of the Device class. Classes should be responsible for creating instances. Also in this design the code for handling the xml is spread all over the place, instead of one module handling this task. Line 1728: vmdevices.core.Sound.update_device_info( Line 1729: self, devices[hwclass.SOUND]) Line 1730: vmdevices.core.Video.update_device_info( Line 1731: self, devices[hwclass.VIDEO]) -- To view, visit https://gerrit.ovirt.org/53677 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6b372f86b82da7422727f3d084b9afc5505a289 Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: Move Vm._getUnderlyingDriveInfo() out of Vm
Milan Zamazal has posted comments on this change. Change subject: virt: Move Vm._getUnderlyingDriveInfo() out of Vm .. Patch Set 13: Verified+1 -- To view, visit https://gerrit.ovirt.org/53677 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6b372f86b82da7422727f3d084b9afc5505a289 Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: Move Vm._getUnderlyingDriveInfo() out of Vm
Milan Zamazal has posted comments on this change.
Change subject: virt: Move Vm._getUnderlyingDriveInfo() out of Vm
..
Patch Set 11:
(3 comments)
https://gerrit.ovirt.org/#/c/53677/11/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 1723: devices = self._devices
Line 1724: vmdevices.network.Interface.update_device_info(
Line 1725: self, devices[hwclass.NIC])
Line 1726: vmdevices.storage.Drive.update_device_info(
Line 1727: self, devices[hwclass.DISK])
> I don't like these class methods, but it seems to be too late now after you
Why don't you like them? Because they are class methods or for another reason?
We can make changes in followup patches.
Line 1728: vmdevices.core.Sound.update_device_info(
Line 1729: self, devices[hwclass.SOUND])
Line 1730: vmdevices.core.Video.update_device_info(
Line 1731: self, devices[hwclass.VIDEO])
https://gerrit.ovirt.org/#/c/53677/11/vdsm/virt/vmdevices/storage.py
File vdsm/virt/vmdevices/storage.py:
Line 443: dom = ET.fromstring(xml_string)
Line 444: return bool(dom.findall(self._xpath))
Line 445:
Line 446: @classmethod
Line 447: def _getDriveIdentification(cls, dom):
> I don't see any use of the cls argument - why is this a classmethod?
Done
Line 448: sources = dom.getElementsByTagName('source')
Line 449: if sources:
Line 450: devPath = (sources[0].getAttribute('file') or
Line 451:sources[0].getAttribute('dev') or
Line 457: alias =
dom.getElementsByTagName('alias')[0].getAttribute('name')
Line 458: return alias, devPath, name
Line 459:
Line 460: @classmethod
Line 461: def update_device_info(cls, vm, device_conf):
> Same, I don't see any use of this class, so it should be a function in this
Each device type has its own update_device_info method, so it should be in the
corresponding class. We can argue whether it should be classmethod or
staticmethod, but conceptually it's not a separate function.
Line 462: # FIXME! We need to gather as much info as possible from the
libvirt.
Line 463: # In the future we can return this real data to management
instead of
Line 464: # vm's conf
Line 465: for x in vm.domain.get_device_elements('disk'):
--
To view, visit https://gerrit.ovirt.org/53677
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib6b372f86b82da7422727f3d084b9afc5505a289
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal
Gerrit-Reviewer: Adam Litke
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik
Gerrit-Reviewer: Milan Zamazal
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: gerrit-hooks
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: Move Vm._getUnderlyingDriveInfo() out of Vm
gerrit-hooks has posted comments on this change. Change subject: virt: Move Vm._getUnderlyingDriveInfo() out of Vm .. Patch Set 13: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/53677 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6b372f86b82da7422727f3d084b9afc5505a289 Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: Move Vm._getUnderlyingDriveInfo() out of Vm
gerrit-hooks has posted comments on this change. Change subject: virt: Move Vm._getUnderlyingDriveInfo() out of Vm .. Patch Set 12: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/53677 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6b372f86b82da7422727f3d084b9afc5505a289 Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: Move Vm._getUnderlyingDriveInfo() out of Vm
gerrit-hooks has posted comments on this change. Change subject: virt: Move Vm._getUnderlyingDriveInfo() out of Vm .. Patch Set 11: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/53677 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6b372f86b82da7422727f3d084b9afc5505a289 Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: Move Vm._getUnderlyingDriveInfo() out of Vm
Nir Soffer has posted comments on this change.
Change subject: virt: Move Vm._getUnderlyingDriveInfo() out of Vm
..
Patch Set 11:
(3 comments)
https://gerrit.ovirt.org/#/c/53677/11/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 1723: devices = self._devices
Line 1724: vmdevices.network.Interface.update_device_info(
Line 1725: self, devices[hwclass.NIC])
Line 1726: vmdevices.storage.Drive.update_device_info(
Line 1727: self, devices[hwclass.DISK])
I don't like these class methods, but it seems to be too late now after you
created so many of them.
Line 1728: vmdevices.core.Sound.update_device_info(
Line 1729: self, devices[hwclass.SOUND])
Line 1730: vmdevices.core.Video.update_device_info(
Line 1731: self, devices[hwclass.VIDEO])
https://gerrit.ovirt.org/#/c/53677/11/vdsm/virt/vmdevices/storage.py
File vdsm/virt/vmdevices/storage.py:
Line 443: dom = ET.fromstring(xml_string)
Line 444: return bool(dom.findall(self._xpath))
Line 445:
Line 446: @classmethod
Line 447: def _getDriveIdentification(cls, dom):
I don't see any use of the cls argument - why is this a classmethod?
This looks like utility for extracting drive info from xml, so I think it is
better use as a function in this module.
Also, it would be nice to use lower case names when you move this method.
Finally, class methods should be grouped at the start of the class, not spread
randomly.
Line 448: sources = dom.getElementsByTagName('source')
Line 449: if sources:
Line 450: devPath = (sources[0].getAttribute('file') or
Line 451:sources[0].getAttribute('dev') or
Line 457: alias =
dom.getElementsByTagName('alias')[0].getAttribute('name')
Line 458: return alias, devPath, name
Line 459:
Line 460: @classmethod
Line 461: def update_device_info(cls, vm, device_conf):
Same, I don't see any use of this class, so it should be a function in this
module.
Line 462: # FIXME! We need to gather as much info as possible from the
libvirt.
Line 463: # In the future we can return this real data to management
instead of
Line 464: # vm's conf
Line 465: for x in vm.domain.get_device_elements('disk'):
--
To view, visit https://gerrit.ovirt.org/53677
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib6b372f86b82da7422727f3d084b9afc5505a289
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal
Gerrit-Reviewer: Adam Litke
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik
Gerrit-Reviewer: Milan Zamazal
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: gerrit-hooks
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: Move Vm._getUnderlyingDriveInfo() out of Vm
Francesco Romani has posted comments on this change. Change subject: virt: Move Vm._getUnderlyingDriveInfo() out of Vm .. Patch Set 11: Code-Review+1 waiting for ACK from Nir (or other storage dev) -- To view, visit https://gerrit.ovirt.org/53677 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6b372f86b82da7422727f3d084b9afc5505a289 Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: Move Vm._getUnderlyingDriveInfo() out of Vm
Milan Zamazal has posted comments on this change. Change subject: virt: Move Vm._getUnderlyingDriveInfo() out of Vm .. Patch Set 11: Verified+1 Verified by running a VM from Engine, migrating it to another host and back, and stopping it. -- To view, visit https://gerrit.ovirt.org/53677 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6b372f86b82da7422727f3d084b9afc5505a289 Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: Move Vm._getUnderlyingDriveInfo() out of Vm
gerrit-hooks has posted comments on this change. Change subject: virt: Move Vm._getUnderlyingDriveInfo() out of Vm .. Patch Set 10: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/53677 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6b372f86b82da7422727f3d084b9afc5505a289 Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: Move Vm._getUnderlyingDriveInfo() out of Vm
gerrit-hooks has posted comments on this change. Change subject: virt: Move Vm._getUnderlyingDriveInfo() out of Vm .. Patch Set 9: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/53677 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6b372f86b82da7422727f3d084b9afc5505a289 Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: Move Vm._getUnderlyingDriveInfo() out of Vm
gerrit-hooks has posted comments on this change. Change subject: virt: Move Vm._getUnderlyingDriveInfo() out of Vm .. Patch Set 8: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/53677 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6b372f86b82da7422727f3d084b9afc5505a289 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: Move Vm._getUnderlyingDriveInfo() out of Vm
Francesco Romani has posted comments on this change. Change subject: virt: Move Vm._getUnderlyingDriveInfo() out of Vm .. Patch Set 7: Code-Review+2 question answered, seems ok -> raising score -- To view, visit https://gerrit.ovirt.org/53677 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6b372f86b82da7422727f3d084b9afc5505a289 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: Move Vm._getUnderlyingDriveInfo() out of Vm
Francesco Romani has posted comments on this change. Change subject: virt: Move Vm._getUnderlyingDriveInfo() out of Vm .. Patch Set 7: (1 comment) https://gerrit.ovirt.org/#/c/53677/7/tests/devices/data/testComplexVm.xml File tests/devices/data/testComplexVm.xml: Line 142: /dev/random Line 143: Line 144: Line 145: Line 146: > This piece of XML is needed to get the moved code at least partially covere ok, fine for me Line 147: Line 148: Line 149: Line 150: -- To view, visit https://gerrit.ovirt.org/53677 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6b372f86b82da7422727f3d084b9afc5505a289 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: Move Vm._getUnderlyingDriveInfo() out of Vm
Milan Zamazal has posted comments on this change. Change subject: virt: Move Vm._getUnderlyingDriveInfo() out of Vm .. Patch Set 7: (1 comment) https://gerrit.ovirt.org/#/c/53677/7/tests/devices/data/testComplexVm.xml File tests/devices/data/testComplexVm.xml: Line 142: /dev/random Line 143: Line 144: Line 145: Line 146: > Could you please remind me why this is needed? This piece of XML is needed to get the moved code at least partially covered by the tests. If it is not present the top loop in Drive.update_device_info just skips. Line 147: Line 148: Line 149: Line 150: -- To view, visit https://gerrit.ovirt.org/53677 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6b372f86b82da7422727f3d084b9afc5505a289 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: Move Vm._getUnderlyingDriveInfo() out of Vm
Francesco Romani has posted comments on this change. Change subject: virt: Move Vm._getUnderlyingDriveInfo() out of Vm .. Patch Set 7: Code-Review+1 (1 comment) partial ACK because of one inline question (but looks good overall) https://gerrit.ovirt.org/#/c/53677/7/tests/devices/data/testComplexVm.xml File tests/devices/data/testComplexVm.xml: Line 142: /dev/random Line 143: Line 144: Line 145: Line 146: Could you please remind me why this is needed? Line 147: Line 148: Line 149: Line 150: -- To view, visit https://gerrit.ovirt.org/53677 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6b372f86b82da7422727f3d084b9afc5505a289 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: Move Vm._getUnderlyingDriveInfo() out of Vm
gerrit-hooks has posted comments on this change. Change subject: virt: Move Vm._getUnderlyingDriveInfo() out of Vm .. Patch Set 7: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/53677 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6b372f86b82da7422727f3d084b9afc5505a289 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: Move Vm._getUnderlyingDriveInfo() out of Vm
gerrit-hooks has posted comments on this change. Change subject: virt: Move Vm._getUnderlyingDriveInfo() out of Vm .. Patch Set 6: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/53677 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6b372f86b82da7422727f3d084b9afc5505a289 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: Move Vm._getUnderlyingDriveInfo() out of Vm
gerrit-hooks has posted comments on this change. Change subject: virt: Move Vm._getUnderlyingDriveInfo() out of Vm .. Patch Set 5: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/53677 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib6b372f86b82da7422727f3d084b9afc5505a289 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
