Re: [Xen-devel] [PATCH v5 3/5] libxl: add support for vscsi

[Olaf Hering <olaf@xxxxxxxxx>]

On Wed, May 13, Ian Campbell wrote:

> You've represented this as a libxl_vscsi_dev which is actually a bus
> containing an array of devices, which leads to an API where to remove a
> device you have to provide libxl_device_scsi_remove with a
> libxl_device_vscsi where the individual devices are tagged for remove or
> not, using a field in the dev descriptor. Really the remove API should
> be taking something like what you've called libxl_vscsi_dev.

You are right: at least the remove function should just get a device
(libxl_vscsi_dev) instead of a bus (libxl_device_vscsi) with devices
tagged for remove. As things are implemented now libxl_device_vscsi_list
can be called by libxl itself, so the remove function can just search
the provided vdev and either just remove it or wipe the entire bus. That
means part of xlu_vscsi_detach goes into libxl_device_vscsi_remove.

> There are also issues around updating the json state etc which I think
> are made more complicated because of this (DEVICE_ADD called in dev_rm,
> and a new version of DEVICE_REMOVE etc).

Yes, libxl_device_vscsi_remove has to keep both xenstore and json in
sync.

> Is it important/useful that the user be able to configure/control the
> number (and addresses) of the buses themselves and which devices are on
> which, or can we get away with the pvpci model where the libxl user just
> gives the individual devices and the library internally takes care of
> what buses need to be created?

I do not know if its important for users of vscsi to build a bus with
several devices. Giving each device its own bus would work as well,
which would turn the whole thing into ordinary devices from libxl POV.

No idea where the initial idea came from. Even if the feature-host thing
is used, the raw passthrough of SCSI commands would continue to work.

> If that is not an option then we may need to follow the pvusb model
> (Chunyan and George CC-d in case I've got it wrong) which is to have
> explicit/separate host and device structs in the libxl API and
> associated separate commands to add/remove buses vs add/remove devices
> on those buses.

I think the backend could deal with an empty bus, which has an empty
vscsi-devs/ dir in xenstore. I will think about it.

> I don't think splitting the structs but not the API functions as you hae
> here is something we want to go with.

Not sure how pvusb or pci acutally looks like, but I will see how pvscsi
looks like if libxl gets just libxl_vscsi_dev as input. The bus handling
itself could be done within libxl, so that libxl_device_vscsi becomes
internal to libxl.

Olaf

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