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

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



On 08/11/2014 07:50 PM, Christoph Hellwig wrote:
On Mon, Aug 11, 2014 at 12:27:29PM +0200, Juergen Gross wrote:
What do you mean with "unusual"? You mean transferring the EH action to
Dom0?

Yes.  Note that hyperv tries something similar and they've run into
timeout issues, you might want to read up the recent thread on that.

+                               } else {
+                                       xenbus_printf(XBT_NIL, dev->nodename,
+                                                     state_str, "%d",
+                                                     XenbusStateConnected);
+                               }

Just print this message in ->slave_configure.

This is calling for problems, I think. xenbus_printf() is not just a
printing function, but it changes an entry in the xenstore. And this
requires locking, switching threads, ...

I doubt doing this while holding SCSI-internal locks is a good idea.

Oh, I thought xenbus_printf was just a logging wrapper.

Doing major work in the slave_* callouts is not a problem, that's what
they were designed for.

Okay.


For the successful case the xenbus_printf should be done in
->slave_configure.  For the failure case you probably want to do it
from ->slave_destroy based on the absence of a flag set in ->slave_configure,
e.g. in slave_configure:

        sdev->hostdata = (void *)1UL;

and in ->slave_destroy:

        if (!sdev->hostdata)

I don't think I'll need the flag. The action is the same if the device
is being destroyed again because of already existing or when it is
really removed.

                ...

although you might see something like this based on external scanning
through procfs/sysfs as mentioned earlier, so please take a look at
how all these corner cases could effect you.

I'll add a check if .slave_configure() and .slave_destroy() are running
in the same task as scsi_add_device() or scsi_remove_device(). This
should rule out all of these corner cases.


Juergen


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.