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