Dan Kenigsberg has posted comments on this change.
Change subject: Change device ownership for VM's drives.
......................................................................
Patch Set 4: I would prefer that you didn't submit this
(6 inline comments)
mostly minor comments
....................................................
File vdsm/storage/hsm.py
Line 2401: def public_appropriateVolume(self, guid, thiefId):
appropriateDevice
please fix docstring format
Line 2409: def public_rmUdevRules(self, thiefId):
inappropriateDevices? yieldDevices?
add docstring, and hint that this is an internal vdsm API.
....................................................
File vdsm/supervdsmServer.py
Line 42: UDEV_RULE_FILE_DIR = "/etc/udev/rules.d/"
these constants are not interesting to anyone outside. Please name them as such (_)
Line 164: raise OSError("Could not trigger change for device %s, \
first arg to OSError must be an errno.
Line 168: def appropriateVolume(self, guid, thiefId):
I think you appropriateDevice, not "volume".
Line 178: os.listdir(UDEV_RULE_FILE_DIR) if
I think a plain glob could be safer and clearer
(now someonme could pass empty thiefId, and remove everybody's rules. hmm, glob is
worse, as '*' could do the same. how about a complete regexp? )
--
To view, visit
http://gerrit.ovirt.org/904
To unsubscribe, visit
http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I7859635b8abed67fe006886efd500eb9ac9f3b7c
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>