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

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

 



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

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux