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 <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Eduardo <[email protected]>
Gerrit-Reviewer: Igor Lvovsky <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://fedorahosted.org/mailman/listinfo/vdsm-patches
