Re: [Xen-devel] [PATCH V2 2/4] Introduce xen-scsifront module

[Juergen Gross <jgross@xxxxxxxx>]

On 07/29/2014 04:57 PM, Juergen Gross wrote:
> On 07/29/2014 03:53 PM, Christoph Hellwig wrote:
> > > +        switch (op) {
> > > +        case VSCSIFRONT_OP_ADD_LUN:
> > > +            if (device_state == XenbusStateInitialised) {
> > > +                sdev = scsi_device_lookup(info->host, chn, tgt,
> > > +                              lun);
> > > +                if (sdev) {
> > > +                    dev_err(&dev->dev,
> > > +                        "Device already in use.\n");
> > > +                    scsi_device_put(sdev);
> > > +                    xenbus_printf(XBT_NIL, dev->nodename,
> > > +                              state_str, "%d",
> > > +                              XenbusStateClosed);
> > > +                } else {
> > > +                    scsi_add_device(info->host, chn, tgt,
> > > +                            lun);
> > > +                    xenbus_printf(XBT_NIL, dev->nodename,
> > > +                              state_str, "%d",
> > > +                              XenbusStateConnected);
> > > +                }
> > > +            }
> > > +            break;
> > scsi_add_device handles an already existing device just fine, and unlike
> > this construct isn't racy.
> Okay. I'll change it.
Hmm, I looked into scsi_add_device(). It seems as if the caller can't
distinguish between a new created and an already existing device. Am I
missing something?

The race is not existing: scsi_add_device() (and scsi_remove_device()
as well) for this scsi_host is called in scsifront_do_lun_hotplug()
only, and this function is always called in the same thread (xenbus
watch). A comment seems to be a good idea.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel