Re: [Qemu-block] [Qemu-devel] [PATCH v4 5/5] raw-posix: Introduce hdev_is_sg()

2015-06-22 Thread Dimitris Aragiorgis
Hello Stefan,

Yes, you are right. Using realpath() is a workaround for supporting
symlinks, as long as they point to a path starting with /dev/sg. I
will remove this reference in the revised version of this patch.

However, it still holds that determining whether a filename is an SG
device or not is based on textual (and hardcoded) examination, i.e.,
whether it starts with /dev/sg.

The point is that a device node is a device node no matter what its
filename or its location in the fs tree is. A device node with  major 1,
minor 3 is the null device whether it's called /dev/null or not.

The proposed patch allows the use of any device node which is capable of
acting as a scsi-generic device; it checks for the required intrinsic
property (does this device support the SG ioctl?) versus trying to
second-guess the nature of the device node by checking its filename.

Similarly, the programming example from SG's documentation does not
check the node of the device node at all:
http://sg.danny.cz/sg/p/sg_v3_ho.html#pexample

/* It is prudent to check we have a sg device by trying an ioctl */
if ((ioctl(sg_fd, SG_GET_VERSION_NUM, k)  0) || (k  3)) {
printf(%s is not an sg device, or old sg driver\n, argv[1]);
return 1;

The above has the advantage that it works with device nodes of any name,
and in any directory.

So I suggest we go with the submitted patch taking into account Eric's
proposal: The code first stat()s the given filename to ensure it is a
character device node, and then it issues the SG_GET_VERSION_NUM and
SG_GET_SCSI_ID ioctl()s.

Thanks,
dimara

* Stefan Hajnoczi stefa...@redhat.com [2015-06-19 13:33:02 +0100]:

 On Wed, May 20, 2015 at 12:57:39PM +0300, Dimitris Aragiorgis wrote:
  This is very fragile, e.g. it fails with symlinks or relative paths.
 
 This is not true since realpath(3) is used to resolve symlinks and
 product an absolute path.
 
 Is this patch really necessary?




signature.asc
Description: Digital signature


Re: [Qemu-block] [Qemu-devel] [PATCH v4 5/5] raw-posix: Introduce hdev_is_sg()

2015-06-22 Thread Stefan Hajnoczi
On Mon, Jun 22, 2015 at 01:18:43PM +0300, Dimitris Aragiorgis wrote:
 So I suggest we go with the submitted patch taking into account Eric's
 proposal: The code first stat()s the given filename to ensure it is a
 character device node, and then it issues the SG_GET_VERSION_NUM and
 SG_GET_SCSI_ID ioctl()s.

That's fine.

Regarding Eric's comment about ioctl number collisions, I've checked
Linux Documentation/ioctl/ioctl-number.txt.  At least under Linux there
is no collision for SG_GET_VERSION_NUM/SG_GET_SCSI_ID.

Stefan


pgpdsPReCd4nS.pgp
Description: PGP signature