diff mbox

[v5,5/5] raw-posix: Introduce hdev_is_sg()

Message ID 1435056300-14924-6-git-send-email-dimara@arrikto.com
State New
Headers show

Commit Message

Dimitris Aragiorgis June 23, 2015, 10:45 a.m. UTC 
Until now, an SG device was identified only by checking if its path
started with "/dev/sg". Then, hdev_open() would set the bs->sg flag
accordingly. The patch relies on the actual properties of the device
instead of the specified file path.

To this end, test for an SG device (e.g. /dev/sg0) by ensuring that
all of the following holds:

 - The specified file name corresponds to a character device
 - The device supports the SG_GET_VERSION_NUM ioctl
 - The device supports the SG_GET_SCSI_ID ioctl

Signed-off-by: Dimitris Aragiorgis <dimara@arrikto.com>
---
 block/raw-posix.c |   39 ++++++++++++++++++++++++++++-----------
 1 file changed, 28 insertions(+), 11 deletions(-)

Comments

Stefan Hajnoczi June 23, 2015, 2:03 p.m. UTC | #1 
On Tue, Jun 23, 2015 at 01:45:00PM +0300, Dimitris Aragiorgis wrote:
> +static bool hdev_is_sg(BlockDriverState *bs)
> +{
> +
> +#if defined(__linux__)
> +
> +    struct stat st;
> +    struct sg_scsi_id scsiid;
> +    int sg_version;
> +
> +    if (stat(bs->filename, &st) >= 0 && S_ISCHR(st.st_mode) &&
> +        !bdrv_ioctl(bs, SG_GET_VERSION_NUM, &sg_version) &&
> +        !bdrv_ioctl(bs, SG_GET_SCSI_ID, &scsiid)) {
> +        DPRINTF("SG device found: type=%d, version=%d\n",
> +            scsiid.scsi_type, sg_version);
> +        return true;
> +    }

If you respin this series, please use fstat() instead of stat() since we
already have the file descriptor open.  That ensures the stat is really
for the same file as the one we already have open (it avoids the race
condition).

I don't see a practical danger in using stat() for the time being:

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
diff mbox

Patch

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 69ae251..942cda3 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -57,6 +57,7 @@ 
 #include <linux/fd.h>
 #include <linux/fs.h>
 #include <linux/hdreg.h>
+#include <scsi/sg.h>
 #ifdef __s390__
 #include <asm/dasd.h>
 #endif
@@ -2084,15 +2085,38 @@  static void hdev_parse_filename(const char *filename, QDict *options,
     qdict_put_obj(options, "filename", QOBJECT(qstring_from_str(filename)));
 }
 
+static bool hdev_is_sg(BlockDriverState *bs)
+{
+
+#if defined(__linux__)
+
+    struct stat st;
+    struct sg_scsi_id scsiid;
+    int sg_version;
+
+    if (stat(bs->filename, &st) >= 0 && S_ISCHR(st.st_mode) &&
+        !bdrv_ioctl(bs, SG_GET_VERSION_NUM, &sg_version) &&
+        !bdrv_ioctl(bs, SG_GET_SCSI_ID, &scsiid)) {
+        DPRINTF("SG device found: type=%d, version=%d\n",
+            scsiid.scsi_type, sg_version);
+        return true;
+    }
+
+#endif
+
+    return false;
+}
+
 static int hdev_open(BlockDriverState *bs, QDict *options, int flags,
                      Error **errp)
 {
     BDRVRawState *s = bs->opaque;
     Error *local_err = NULL;
     int ret;
-    const char *filename = qdict_get_str(options, "filename");
 
 #if defined(__APPLE__) && defined(__MACH__)
+    const char *filename = qdict_get_str(options, "filename");
+
     if (strstart(filename, "/dev/cdrom", NULL)) {
         kern_return_t kernResult;
         io_iterator_t mediaIterator;
@@ -2121,16 +2145,6 @@  static int hdev_open(BlockDriverState *bs, QDict *options, int flags,
 #endif
 
     s->type = FTYPE_FILE;
-#if defined(__linux__)
-    {
-        char resolved_path[ MAXPATHLEN ], *temp;
-
-        temp = realpath(filename, resolved_path);
-        if (temp && strstart(temp, "/dev/sg", NULL)) {
-            bs->sg = 1;
-        }
-    }
-#endif
 
     ret = raw_open_common(bs, options, flags, 0, &local_err);
     if (ret < 0) {
@@ -2140,6 +2154,9 @@  static int hdev_open(BlockDriverState *bs, QDict *options, int flags,
         return ret;
     }
 
+    /* Since this does ioctl the device must be already opened */
+    bs->sg = hdev_is_sg(bs);
+
     if (flags & BDRV_O_RDWR) {
         ret = check_hdev_writable(s);
         if (ret < 0) {