-
Notifications
You must be signed in to change notification settings - Fork 92
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CA-174892: scsiutil.getSCSIid() fails if multipath device is given #266
Conversation
getdev() resolves all symlinks and returns the block device's name. In the case of multipath devices I believe it's always 'dm-#'. |
That doesn't mean other block devices couldn't contain the substring "dm-" in their names. These would mistakenly be taken as device mappers, whereas comparing the major number is foolproof. |
Felipe, this is not a way to test if a device is a device mapper device, this is just to avoid stripping numbers for devices like dm-0, where the number in this case is not a partition. Calling a device dm-* is a device mapper thing, according to udev rules. |
@kostaslamda, I still think that checking for the name is fine but I agree with Felipe that searching for a substring "dm-" is not sufficient. "starting with" is a stronger one. |
When getSCSIid() is passed a multipath device (e.g. /dev/dm-2) it fails, as rawdev() strips any numbers in the end of the device name. Firstly, rawdev() is modified to return device names that contain 'dm-' untouched. Secondly, getSCSIid() uses 'path' with the new version of the command, and only searches for the raw device name in the fallback call. Signed-off-by: Kostas Ladopoulos <Konstantinos.Ladopoulos@citrix.com>
OK |
Kostas, I am not entirely sure but I think this fix is superseded by #269 |
At a first glance, it seems that #269 has the exact opposite functionality; you supply a SCSI id and you get back all the block devices associated with that id. |
Nope, it is exactly for the same thing: it makes sure a device mapper is never passed, making your fix moot. |
I disaggree. You can remove if you want, but it will just break stuff, without providing any improvement whatsoever. |
If you disagree just explain why. |
getSCSIid() takes a block device path as input and returns its SCSI id. Different paths can resolve to the same SCSI id (partitions, multipathed block device, symlinks). getSCSIid() queries a block device about its SCSI id. get_devices_by_SCSIid() takes a SCSI id as input and returns a list of block devices. #269 is an 'ls' on '/dev/disk/by-scsid/', removing the 'mapper' entry if it exists. |
Thanks for the explanation about what the functions do, even though I am pretty sure I know both the functions very well.. I was just asking you to verify that that fix did not have any impact on your patch, for example, What was not clear if the fix in #269 was somehow improving the situation there, because it While this conversation was taking place I checked myself if that was the case. |
When getSCSIid() is passed a multipath device (e.g. /dev/dm-2) it fails, as rawdev() strips any numbers in the end of the device name. Firstly, rawdev() is modified to return device names that contain 'dm-' untouched. Secondly, getSCSIid() uses 'path' with the new version of the command, and only searches for the raw device name in the fallback call. Reviewed-by: Germano Percossi <germano.percossi@citrix.com> GitHub: closes xapi-project#266
When getSCSIid() is passed a multipath device (e.g. /dev/dm-2) it fails,
as rawdev() strips any numbers in the end of the device name. Firstly,
rawdev() is modified to return device names that contain 'dm-' untouched.
Secondly, getSCSIid() uses 'path' with the new version of the command,
and only searches for the raw device name in the fallback call.
Signed-off-by: Kostas Ladopoulos Konstantinos.Ladopoulos@citrix.com