Change in vdsm[master]: vm: Unify checks for vdsm image
sgotliv at redhat.com
sgotliv at redhat.com
Fri Dec 13 15:17:00 UTC 2013
Sergey Gotliv has posted comments on this change.
Change subject: vm: Unify checks for vdsm image
......................................................................
Patch Set 2: Code-Review-1
(1 comment)
I realized that if I am out my -1 is out with me :-).
....................................................
File vdsm/vm.py
Line 88: :type drive: dict or vm.Drive
Line 89: :return: bool
Line 90: """
Line 91: if drive['device'] != 'disk':
Line 92: return False
Currently vdsm image is a PDIV, it has nothing to do with device == disk.
What other devices you know that contain poolID, domainID, imageID and volumeID?
This check of device is unnecessary and earlier today I did a first step to remove that check here http://gerrit.ovirt.org/#/c/22363/.
@Author knows that because he already reviewed it :-). So what is this?
Putting this check in that place won't allow me to remove it step by step, now its like all or nothing and as I explained there "all" is a time consuming in the matter of tests.
Line 93: required = ('volumeID', 'domainID', 'imageID', 'poolID')
Line 94: return all(k in drive for k in required)
Line 95:
Line 96:
--
To view, visit http://gerrit.ovirt.org/22370
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaa109a19aeec56f09ed2e6d64dee636c7779d426
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv at redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list