From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [PATCH RFC v1] libxl: Introduce a template for devices with a controller Date: Thu, 21 May 2015 18:28:01 +0100 Message-ID: <555E15A1.10907@eu.citrix.com> References: <1432228052-15667-1-git-send-email-george.dunlap@eu.citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1432228052-15667-1-git-send-email-george.dunlap@eu.citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: xen-devel@lists.xen.org Cc: Juergen Gross , Olaf Hering , Wei Liu , Ian Campbell , Chun Yan Liu , Ian Jackson List-Id: xen-devel@lists.xenproject.org On 05/21/2015 06:07 PM, George Dunlap wrote: > We have several outstanding patch series which add devices that have > two levels: a controller and individual devices attached to that > controller. > > In the interest of consistency, this patch introduces a section that > sketches out a template for interfaces for such devices. > > Signed-off-by: George Dunlap > --- > CC: Ian Campbell > CC: Ian Jackson > CC: Wei Liu > CC: Juergen Gross > CC: Chun Yan Liu > CC: Olaf Hering > > So, this is definitely RFC -- I tried to spec things out in a way that > made sense, but I often just chose something that I thought would be a > sensible starting point for discussion. > > This spec looks a lot more like the PVUSB spec than the PVSCSI spec, > in part because I think the PVUSB spec has already had a lot more > thought that's gone into it. > > A couple of random points to discuss: > > * Calling things "controllers", using ctrl for the device name, > and using "ctrl" as the field name for the devid of the controller > in the individual devices. > > * I've said that having an index (port, lun, whatever) is optional. > Do we want to make that requred? Do we want it to have a consistent > name? In the case of emulated USB, we can't really specify to qemu > what port the device gets attached to, so I'm tempted to say it's > not required; but even there we could always give it a port number > just for name's sake. > > * Naming sub-devices. We need to have a way to uniquely name both > controllers and subdevices. Here I've said that we will have both > ctrl and devid namespaces, mainly because in the > previous point I opted not to require an index. Another option > would be not to have another devid namespace, but to use > as the unique identifier. (This would mean requiring > the index/port/lun specification above.) > > * libxl_device__list listing devices across all controllers. I > think this is the most practical thing to do, but one could imagine > wanting to query by controller ID instead. > > Feedback welcome. > --- > tools/libxl/libxl.h | 46 ++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 46 insertions(+) > > diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h > index 2ed7194..d757845 100644 > --- a/tools/libxl/libxl.h > +++ b/tools/libxl/libxl.h > @@ -1234,6 +1234,52 @@ void libxl_vtpminfo_list_free(libxl_vtpminfo *, int nr_vtpms); > * > * This function does not interact with the guest and therefore > * cannot block on the guest. > + * > + * Controllers > + * ----------- > + * > + * Most devices are treated individually. Some devices however, like > + * USB or SCSI, inherently have the need to have "busses" or > + * "controllers" to which individual devices can be attached. > + * > + * In that case, for each , there will be two sets of the > + * functions, types, and devid namespaces outlined above: one based on > + * '', and one based on 'ctrl'. > + * > + * In those cases, libxl_device_ctrl_ will act more or > + * less like top-level non-bus devices: they will either create or > + * accept a libxl_devid which will be unique within the > + * ctrl libxl_devid namespace. > + * > + * Second-level devices which will be attached to a controller will > + * include in their libxl_device_ a field called ctrl, which > + * will be the libxl_devid of the corresponding controller. It may also > + * include an index onto that bus, that represents (for example) a USB > + * port or a SCSI LUN. > + * > + * These second-level devices will also have their own devid which > + * will be unique within the devid namespace, and will be used > + * for queries or removals. > + * > + * In the case where there are multiple different ways to implement a > + * given device -- for instance, one which is fully PV and one which > + * uses an emulator -- the controller will contain a field which > + * specifies what type of implementation is used. The implementations > + * of individual devices will be known by the controller to which they are > + * attached. > + * > + * If libxl_device__add receives an uninitialized ctrl devid, it > + * may return an error. Or it may (but is not required to) choose to > + * automatically choose a suitable controller to which to attach the > + * new device. It may also (but is not required to) automatically > + * create a new controller if no suitable controllers exist. > + * Individual devices should document their behavior. > + * > + * libxl_device_ctrl_list will list all controllers for the domain. > + * > + * libxl_device__list will list all devices for all controllers > + * for the domain. The individual libxl_device_ will include > + * the devid of the controller to which it is attached. > */ So just for concreteness, here is a somewhat "dumb" conversion of the most recently-posted pvscsi IDL using this template: libxl_vscsi_pdev_type = Enumeration("vscsi_pdev_type", [ (0, "INVALID"), (1, "HCTL"), (2, "WWN"), ]) libxl_vscsi_hctl = Struct("vscsi_hctl", [ ("hst", uint32), ("chn", uint32), ("tgt", uint32), ("lun", uint32), ]) libxl_vscsi_pdev = Struct("vscsi_pdev", [ ("p_devname", string), ("u", KeyedUnion(None, libxl_vscsi_pdev_type, "type", [ ("invalid", None), ("hctl", Struct(None, [("m", libxl_vscsi_hctl)])), ("wwn", Struct(None, [("m", string)])), ])), ]) libxl_device_vscsi = Struct("device_vscsi", [ ("ctrl", libxl_devid), ("devid", libxl_devid), ("pdev", libxl_vscsi_pdev), ("vdev", libxl_vscsi_hctl), ]) libxl_device_vscsictrl = Struct("device_vscsictrl", [ ("backend_domid", libxl_domid), ("devid", libxl_devid), ("hst", uint32), ("next_vscsi_dev_id", libxl_devid), ("feature_host", libxl_defbool), ])