Patchwork Curious about relaxation of bdrv_is_sg() for scsi-generic

login
register
mail settings
Submitter Markus Armbruster
Date April 15, 2013, 12:41 p.m.
Message ID <87y5ckx8xs.fsf@blackfin.pond.sub.org>
Download mbox | patch
Permalink /patch/236580/
State New
Headers show

Comments

Markus Armbruster - April 15, 2013, 12:41 p.m.
Commit 9839245 "ISCSI: Add SCSI passthrough via scsi-generic to
libiscsi" contains this hunk:


Why could we relax the "must be sg" requirement here?  I know it's been
a while...
Paolo Bonzini - April 15, 2013, 1:08 p.m.
Il 15/04/2013 14:41, Markus Armbruster ha scritto:
> Commit 9839245 "ISCSI: Add SCSI passthrough via scsi-generic to
> libiscsi" contains this hunk:
> 
> diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
> index d856d23..8d51060 100644
> --- a/hw/scsi-generic.c
> +++ b/hw/scsi-generic.c
> @@ -400,12 +400,6 @@ static int scsi_generic_initfn(SCSIDevice *s)
>          return -1;
>      }
>  
> -    /* check we are really using a /dev/sg* file */
> -    if (!bdrv_is_sg(s->conf.bs)) {
> -        error_report("not /dev/sg*");
> -        return -1;
> -    }
> -
>      if (bdrv_get_on_error(s->conf.bs, 0) != BLOCK_ERR_STOP_ENOSPC) {
>          error_report("Device doesn't support drive option werror");
>          return -1;
> 
> Why could we relax the "must be sg" requirement here?  I know it's been
> a while...

It's for two reasons.

First, and related to this patch, because an iscsi://foo/bar device now
can be used for scsi-generic too.  Then you can use libiscsi to connect
to a tape or media changer.

Second, simply because it was an artificial restriction.  You can use
scsi-generic with disks or CD-ROMs, and it is much simpler to use
/dev/sda or /dev/sr0 than to figure out what is the matching /dev/sgN
character device.

The important test is not on the path.  It is done a few lines below:

    if (bdrv_ioctl(s->conf.bs, SG_GET_VERSION_NUM, &sg_version) < 0) {
        error_report("scsi generic interface not supported");
        return -1;
    }

Paolo

Patch

diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index d856d23..8d51060 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -400,12 +400,6 @@  static int scsi_generic_initfn(SCSIDevice *s)
         return -1;
     }
 
-    /* check we are really using a /dev/sg* file */
-    if (!bdrv_is_sg(s->conf.bs)) {
-        error_report("not /dev/sg*");
-        return -1;
-    }
-
     if (bdrv_get_on_error(s->conf.bs, 0) != BLOCK_ERR_STOP_ENOSPC) {
         error_report("Device doesn't support drive option werror");
         return -1;