Yeela Kaplan has posted comments on this change.

Change subject: clientIF: rescan devices on failed hotplugDisk
......................................................................


Patch Set 3: (2 inline comments)

....................................................
File vdsm/clientIF.py
Line 275:                 drive['volumeInfo'] = res['info']
Line 276: 
Line 277:             # GUID drive format
Line 278:             elif "GUID" in drive:
Line 279:                 res = self.irs.getDevicesVisibility([drive["GUID"]])
I disagree, 
getDevicesVisibility() will first check if the path: /dev/mapper/guid  exists, 
which tells us if the device is known to multipath.
Only in case it does not exist it will perform the scan.
Therefore adding an os.path.exists is redundant.
Line 280:                 visibility = res['visible']
Line 281:                 if visibility[drive["GUID"]] is False:
Line 282:                     self.log.error("GUID: %s is not visible", 
drive["GUID"])
Line 283:                     raise vm.VolumeError(drive)


Line 276: 
Line 277:             # GUID drive format
Line 278:             elif "GUID" in drive:
Line 279:                 res = self.irs.getDevicesVisibility([drive["GUID"]])
Line 280:                 visibility = res['visible']
In my opinion, it makes the code more readable.
Line 281:                 if visibility[drive["GUID"]] is False:
Line 282:                     self.log.error("GUID: %s is not visible", 
drive["GUID"])
Line 283:                     raise vm.VolumeError(drive)
Line 284: 


--
To view, visit http://gerrit.ovirt.org/13454
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie5252a8ba6bf76c9eda3588c665ad379da3e9cfc
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Eduardo <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Yeela Kaplan <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to