| Message ID | m3iq9lvcjr.fsf@blackfin.pond.sub.org |
|---|---|
| State | New |
| Headers | show |
On 02/25/10 11:23, Markus Armbruster wrote: > You're supposed to use scsi-generic for that. Which rejects anything > but /dev/sg*. Well, it isn't *that* easy. The SG_IO ioctl used by scsi-generic works on tons of devices in linux, not only /dev/sg*. I've seen patches floading around which change the check bdrv_is_sg() into "try SG_IO and see if it works", which would allow to use /dev/sda with both scsi-disk and scsi-generic depending on what you want. Which makes alot of sense. Making that change needs some extra care though to avoid existing configurations switching from scsi-disk to scsi-generic unnoticed. cheers, Gerd
On Thu, Feb 25, 2010 at 11:23:52AM +0100, Markus Armbruster wrote: > You're supposed to use scsi-generic for that. Which rejects anything > but /dev/sg*. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > hw/scsi-disk.c | 5 +++++ > 1 files changed, 5 insertions(+), 0 deletions(-) > > diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c > index b2f61fe..ad8eb24 100644 > --- a/hw/scsi-disk.c > +++ b/hw/scsi-disk.c > @@ -1027,6 +1027,11 @@ static int scsi_disk_initfn(SCSIDevice *dev) > } > s->bs = s->qdev.conf.dinfo->bdrv; > > + if (bdrv_is_sg(s->bs)) { > + qemu_error("scsi-disk: unwanted /dev/sg*\n"); > + return -1; Can we make the error message a bit more verbose? E.g. "scsi-disk does not support /dev/sg*, please use *** instead". Where *** tells user what to do. > + } > + > if (bdrv_get_type_hint(s->bs) == BDRV_TYPE_CDROM) { > s->cluster_size = 4; > } else { > -- > 1.6.6 > >
Gerd Hoffmann <kraxel@redhat.com> writes: > On 02/25/10 11:23, Markus Armbruster wrote: >> You're supposed to use scsi-generic for that. Which rejects anything >> but /dev/sg*. > > Well, it isn't *that* easy. The SG_IO ioctl used by scsi-generic > works on tons of devices in linux, not only /dev/sg*. I've seen Yes, the existing check in scsi-generic is pretty cheesy. My patch just makes scsi-disk consistently cheesy ;) > patches floading around which change the check bdrv_is_sg() into "try > SG_IO and see if it works", which would allow to use /dev/sda with > both scsi-disk and scsi-generic depending on what you want. Which > makes alot of sense. > > Making that change needs some extra care though to avoid existing > configurations switching from scsi-disk to scsi-generic unnoticed. Would be nice to have some day.
> On 02/25/10 11:23, Markus Armbruster wrote: > > You're supposed to use scsi-generic for that. Which rejects anything > > but /dev/sg*. > > Well, it isn't *that* easy. The SG_IO ioctl used by scsi-generic works > on tons of devices in linux, not only /dev/sg*. I've seen patches > floading around which change the check bdrv_is_sg() into "try SG_IO and > see if it works", which would allow to use /dev/sda with both scsi-disk > and scsi-generic depending on what you want. Which makes alot of sense. > > Making that change needs some extra care though to avoid existing > configurations switching from scsi-disk to scsi-generic unnoticed. Don't we really want to be testing !bdrv_is_block() ? Paul
On 02/28/10 02:45, Paul Brook wrote: >> On 02/25/10 11:23, Markus Armbruster wrote: >>> You're supposed to use scsi-generic for that. Which rejects anything >>> but /dev/sg*. >> >> Well, it isn't *that* easy. The SG_IO ioctl used by scsi-generic works >> on tons of devices in linux, not only /dev/sg*. I've seen patches >> floading around which change the check bdrv_is_sg() into "try SG_IO and >> see if it works", which would allow to use /dev/sda with both scsi-disk >> and scsi-generic depending on what you want. Which makes alot of sense. >> >> Making that change needs some extra care though to avoid existing >> configurations switching from scsi-disk to scsi-generic unnoticed. > > Don't we really want to be testing !bdrv_is_block() ? That would work for linux. Dunno about the BSD. cheers, Gerd
On 02/25/2010 04:23 AM, Markus Armbruster wrote: > You're supposed to use scsi-generic for that. Which rejects anything > but /dev/sg*. > > Signed-off-by: Markus Armbruster<armbru@redhat.com> > --- > Applied. Thanks. Regards, Anthony Liguori > hw/scsi-disk.c | 5 +++++ > 1 files changed, 5 insertions(+), 0 deletions(-) > > diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c > index b2f61fe..ad8eb24 100644 > --- a/hw/scsi-disk.c > +++ b/hw/scsi-disk.c > @@ -1027,6 +1027,11 @@ static int scsi_disk_initfn(SCSIDevice *dev) > } > s->bs = s->qdev.conf.dinfo->bdrv; > > + if (bdrv_is_sg(s->bs)) { > + qemu_error("scsi-disk: unwanted /dev/sg*\n"); > + return -1; > + } > + > if (bdrv_get_type_hint(s->bs) == BDRV_TYPE_CDROM) { > s->cluster_size = 4; > } else { >
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index b2f61fe..ad8eb24 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -1027,6 +1027,11 @@ static int scsi_disk_initfn(SCSIDevice *dev) } s->bs = s->qdev.conf.dinfo->bdrv; + if (bdrv_is_sg(s->bs)) { + qemu_error("scsi-disk: unwanted /dev/sg*\n"); + return -1; + } + if (bdrv_get_type_hint(s->bs) == BDRV_TYPE_CDROM) { s->cluster_size = 4; } else {
You're supposed to use scsi-generic for that. Which rejects anything but /dev/sg*. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- hw/scsi-disk.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)