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

danken at redhat.com danken at redhat.com
Tue May 21 07:57:52 UTC 2013


Dan Kenigsberg has posted comments on this change.

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


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

(3 inline comments)

....................................................
File vdsm/clientIF.py
Line 341:             res = self.irs.teardownImage(drive['domainID'],
Line 342:                                          drive['poolID'], drive['imageID'])
Line 343:         except KeyError:
Line 344:             self.log.warning("Drive is not a vdsm image: %s",
Line 345:                              str(drive), exc_info=True)
calling str() explodes if the drive dict has unicode items. %s is enough.
Line 346:         except TypeError:
Line 347:             if type(drive) not in (str, unicode):
Line 348:                 raise
Line 349:             # paths (strings) are not deactivated


Line 343:         except KeyError:
Line 344:             self.log.warning("Drive is not a vdsm image: %s",
Line 345:                              str(drive), exc_info=True)
Line 346:         except TypeError:
Line 347:             if type(drive) not in (str, unicode):
ask anybody: the way to check stringness is

 isinstance(drive, basestring)
Line 348:                 raise
Line 349:             # paths (strings) are not deactivated
Line 350: 
Line 351:         return res['status']['code']


....................................................
File vdsm/libvirtvm.py
Line 1028: 
Line 1029:     def __getitem__(self, key):
Line 1030:         def __getitem__(self, key):
Line 1031:             try:
Line 1032:                 value = getattr(self, str(key))
why do you str() the key? if key is not a valid attr, let getattr() explode.
Line 1033:             except AttributeError:
Line 1034:                 raise KeyError(key)
Line 1035:             else:
Line 1036:                 return value


--
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: 3
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: Lee Yarwood <lyarwood 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