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(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>