[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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |