Change in vdsm[master]: Deactivate libvirtVM.Drives()

danken at redhat.com danken at redhat.com
Sun May 19 20:09:45 UTC 2013


Dan Kenigsberg has posted comments on this change.

Change subject: Deactivate libvirtVM.Drives()
......................................................................


Patch Set 2: I would prefer that you didn't submit this

(1 inline comment)

....................................................
File vdsm/clientIF.py
Line 343:                                              drive['poolID'], drive['imageID'])
Line 344:             except KeyError:
Line 345:                 #This drive is not a vdsm image (quartet)
Line 346:                 self.log.info("Avoiding tear down drive %s", str(drive))
Line 347:         elif drive.__class__.__name__ == 'Drive':
who passes a drive object (instead of a drivespec) to this function? shouldn't THEY do the conversion there? Or maybe, add a Drive member function that does this? Adding polymorphism with elifs seems just wrong.

On a side note, I find

 elif drive.__class__ is Drive:

or

 isinstance(drive, Drive)

much prettier.
Line 348:             try:
Line 349:                 res = self.irs.teardownImage(drive.domainID,
Line 350:                                              drive.poolID, drive.imageID)
Line 351:             except AttributeError:


--
To view, visit http://gerrit.ovirt.org/14869
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8ba965f0987037fa5dafc6b3ea6eeb6f01a82436
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Daniel Paikov <paikov at gmail.com>
Gerrit-Reviewer: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list