Change in vdsm[master]: Adding shared raw disk feature.

Igor Lvovsky ilvovsky at redhat.com
Thu Oct 27 09:36:12 UTC 2011


Igor Lvovsky has posted comments on this change.

Change subject: Adding shared raw disk feature.
......................................................................


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

(6 inline comments)

....................................................
Commit Message
Line 10: Use
Line 11: drive=GUID:<guid>,
Line 12: or
Line 13: drive=UUID:<uuid>,
Line 14: Note the mandatory ','
Why it's mandatory?  you can split(',') without it
Line 15: 
Line 16: Change-Id: Ib417bd6423773db382826d6255e8cbeafd333116
Line 17
Line 18


....................................................
File vdsm/clientIF.py
Line 556:                 if res['status']['code']:
Line 557:                     raise vm.VolumeError(drive)
Line 558:                 path = res['path']
Line 559:                 #Another drive dict type
Line 560:             elif drive.has_key("GUID") and os.path.exists(os.path.join("/dev/mapper", drive["GUID"])):
Did we agree about 'drive' changing?
Line 561:                 path = os.path.join("/dev/mapper", drive["GUID"])
Line 562:             elif drive.has_key("UUID"):
Line 563:                 rc, out, err  = storage.misc.execCmd(["blkid", "-U", drive["UUID"]], sudo=False)
Line 564:                 if not out or rc != 0:


Line 558:                 path = res['path']
Line 559:                 #Another drive dict type
Line 560:             elif drive.has_key("GUID") and os.path.exists(os.path.join("/dev/mapper", drive["GUID"])):
Line 561:                 path = os.path.join("/dev/mapper", drive["GUID"])
Line 562:             elif drive.has_key("UUID"):
I think  we saw together that blkid is redundant
Line 563:                 rc, out, err  = storage.misc.execCmd(["blkid", "-U", drive["UUID"]], sudo=False)
Line 564:                 if not out or rc != 0:
Line 565:                     self.log.info("blkid failed for UUID: %s" % drive)
Line 566:                     raise vm.VolumeError(drive)


Line 567:                 else:
Line 568:                     path = out[0]
Line 569:         #For BC sake: a path as a string.
Line 570:         elif not drive:
Line 571:             path = drive
Is drive == None is OK?
Line 572:         elif os.path.exists(drive):
Line 573:             path = drive
Line 574:         self.log.info("prepared volume path: %s" % path)
Line 575:         return path


Line 580:             try:
Line 581:                 result = self.irs.teardownVolume(drive['domainID'], drive['poolID'],
Line 582:                                              drive['imageID'], drive['volumeID'])
Line 583:             except KeyError:
Line 584:                 self.log.info("Avoiding tear down drive %s", str(drive))
why not exception? why not like in _prepareVolumePath?
Line 585: 
Line 586:         return result['status']['code']
Line 587: 
Line 588:     def create(self, vmParams):


....................................................
File vdsm/vm.py
Line 416:                 res = self.cif.irs.getVolumeSize(drive['domainID'],
Line 417:                                      drive['poolID'], drive['imageID'],
Line 418:                                      drive['volumeID'])
Line 419:             except KeyError:
Line 420:                 self.log.info("Ignoring drive %s", str(drive))
Why you need it? We definitely want to raise exception here.
Line 421:             else:
Line 422:                 drive['truesize'] = res['truesize']
Line 423:                 drive['apparentsize'] = res['apparentsize']
Line 424:                 drive['blockDev'] = not self.cif.irs.getStorageDomainInfo(


--
To view, visit http://gerrit.usersys.redhat.com/1064
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib417bd6423773db382826d6255e8cbeafd333116
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo Warszawski <ewarszaw at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky at redhat.com>


More information about the vdsm-patches mailing list