qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v3 2/4] scsi: Allow SCSI devices to override the


From: Stefan Hajnoczi
Subject: Re: [Qemu-devel] [PATCH v3 2/4] scsi: Allow SCSI devices to override the removable bit
Date: Mon, 24 Jan 2011 14:47:54 +0000
User-agent: Mutt/1.5.20 (2009-06-14)

On Fri, Jan 21, 2011 at 06:26:14PM +0100, Markus Armbruster wrote:
> Stefan Hajnoczi <address@hidden> writes:
> 
> > Some SCSI devices may wish to override the removable bit.  Add support
> > for a qdev property on the SCSI device.
> 
> I find this description a bit misleading.  The qdev property is added in
> 1/4.  Here, you merely extend scsi_bus_legacy_add_drive() to provide
> access to it.  Its primary users (-drive if=scsi & friends) don't use
> that access.  But there's another user: usb_msd_initfn()[*], and 3/4
> will make that one use the access.
> 
> I guess I'd squash 2+3 together, but that's strictly a matter of taste.

You're right, the description is poor.  Sorry about that.

> >
> > Signed-off-by: Stefan Hajnoczi <address@hidden>
> > ---
> >  hw/pci-hotplug.c |    2 +-
> >  hw/scsi-bus.c    |    8 ++++++--
> >  hw/scsi.h        |    3 ++-
> >  hw/usb-msd.c     |    2 +-
> >  4 files changed, 10 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
> > index 716133c..270a982 100644
> > --- a/hw/pci-hotplug.c
> > +++ b/hw/pci-hotplug.c
> > @@ -90,7 +90,7 @@ static int scsi_hot_add(Monitor *mon, DeviceState 
> > *adapter,
> >       * specified).
> >       */
> >      dinfo->unit = qemu_opt_get_number(dinfo->opts, "unit", -1);
> > -    scsidev = scsi_bus_legacy_add_drive(scsibus, dinfo->bdrv, dinfo->unit);
> > +    scsidev = scsi_bus_legacy_add_drive(scsibus, dinfo->bdrv, dinfo->unit, 
> > false);
> >      if (!scsidev) {
> >          return -1;
> >      }
> > diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
> > index 7febb86..ceeb4ec 100644
> > --- a/hw/scsi-bus.c
> > +++ b/hw/scsi-bus.c
> > @@ -87,7 +87,8 @@ void scsi_qdev_register(SCSIDeviceInfo *info)
> >  }
> >  
> >  /* handle legacy '-drive if=scsi,...' cmd line args */
> > -SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockDriverState 
> > *bdrv, int unit)
> > +SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockDriverState *bdrv,
> > +                                      int unit, bool removable)
> >  {
> >      const char *driver;
> >      DeviceState *dev;
> > @@ -95,6 +96,9 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, 
> > BlockDriverState *bdrv, int
> >      driver = bdrv_is_sg(bdrv) ? "scsi-generic" : "scsi-disk";
> >      dev = qdev_create(&bus->qbus, driver);
> >      qdev_prop_set_uint32(dev, "scsi-id", unit);
> > +    if (qdev_prop_exists(dev, "removable")) {
> 
> Isn't this just a funky way to check for "scsi-disk"?
> 
> Removable gets silently ignored for -device usb-storage with a scsi
> generic drive.  Worth nothing in 4/4.

Yes, it only applies for scsi-disk and I'll note that in 4/4.

Stefan



reply via email to

[Prev in Thread] Current Thread [Next in Thread]