All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC v1] libxl: Introduce a template for devices with a controller
@ 2015-05-21 17:07 George Dunlap
  2015-05-21 17:11 ` George Dunlap
                   ` (4 more replies)
  0 siblings, 5 replies; 14+ messages in thread
From: George Dunlap @ 2015-05-21 17:07 UTC (permalink / raw)
  To: xen-devel
  Cc: Juergen Gross, Olaf Hering, Wei Liu, Ian Campbell, George Dunlap,
	Chun Yan Liu, Ian Jackson

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 <george.dunlap@eu.citrix.com>
---
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Ian Jackson <ian.jackson@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Juergen Gross <jgross@suse.com>
CC: Chun Yan Liu <cyliu@suse.com>
CC: Olaf Hering <olaf@aepfle.de>

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 <type>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
  <type>ctrl and <type> 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
  <ctrl,index> as the unique identifier.  (This would mean requiring
  the index/port/lun specification above.)

* libxl_device_<type>_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 <type>, there will be two sets of the
+ * functions, types, and devid namespaces outlined above: one based on
+ * '<type>', and one based on '<type>ctrl'.
+ *
+ * In those cases, libxl_device_<type>ctrl_<function> 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
+ * <type>ctrl libxl_devid namespace.
+ *
+ * Second-level devices which will be attached to a controller will
+ * include in their libxl_device_<type> 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 <type> 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_<type>_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_<type>ctrl_list will list all controllers for the domain.
+ *
+ * libxl_device_<type>_list will list all devices for all controllers
+ * for the domain.  The individual libxl_device_<type> will include
+ * the devid of the controller to which it is attached.
  */
 
 /* Disks */
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH RFC v1] libxl: Introduce a template for devices with a controller
  2015-05-21 17:07 [PATCH RFC v1] libxl: Introduce a template for devices with a controller George Dunlap
@ 2015-05-21 17:11 ` George Dunlap
  2015-05-21 17:28 ` George Dunlap
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 14+ messages in thread
From: George Dunlap @ 2015-05-21 17:11 UTC (permalink / raw)
  To: xen-devel
  Cc: Juergen Gross, Olaf Hering, Wei Liu, Ian Campbell, Chun Yan Liu,
	Ian Jackson

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 <george.dunlap@eu.citrix.com>
> ---
> CC: Ian Campbell <ian.campbell@citrix.com>
> CC: Ian Jackson <ian.jackson@citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Juergen Gross <jgross@suse.com>
> CC: Chun Yan Liu <cyliu@suse.com>
> CC: Olaf Hering <olaf@aepfle.de>
> 
> 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 <type>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
>   <type>ctrl and <type> 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
>   <ctrl,index> as the unique identifier.  (This would mean requiring
>   the index/port/lun specification above.)
> 
> * libxl_device_<type>_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 <type>, there will be two sets of the
> + * functions, types, and devid namespaces outlined above: one based on
> + * '<type>', and one based on '<type>ctrl'.
> + *
> + * In those cases, libxl_device_<type>ctrl_<function> 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
> + * <type>ctrl libxl_devid namespace.
> + *
> + * Second-level devices which will be attached to a controller will
> + * include in their libxl_device_<type> 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 <type> 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_<type>_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_<type>ctrl_list will list all controllers for the domain.
> + *
> + * libxl_device_<type>_list will list all devices for all controllers
> + * for the domain.  The individual libxl_device_<type> will include
> + * the devid of the controller to which it is attached.

Hmm, I also meant to add:

---
For each type, the domain config file will contain a single list of
controllers, and a single list of devices.  libxl will first iterate
through the list adding the controlllers, then iterate through the list
adding each device to the ctrl listed.  If libxl_device_<type>_add
automatically creates controllers as necessary, then it is permissible
for the controller list to be empty and the device list to have devices
(with the ctrl field uninitialized).
---

 -George

>   */
>  
>  /* Disks */
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH RFC v1] libxl: Introduce a template for devices with a controller
  2015-05-21 17:07 [PATCH RFC v1] libxl: Introduce a template for devices with a controller George Dunlap
  2015-05-21 17:11 ` George Dunlap
@ 2015-05-21 17:28 ` George Dunlap
  2015-05-22  4:21 ` Juergen Gross
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 14+ messages in thread
From: George Dunlap @ 2015-05-21 17:28 UTC (permalink / raw)
  To: xen-devel
  Cc: Juergen Gross, Olaf Hering, Wei Liu, Ian Campbell, Chun Yan Liu,
	Ian Jackson

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 <george.dunlap@eu.citrix.com>
> ---
> CC: Ian Campbell <ian.campbell@citrix.com>
> CC: Ian Jackson <ian.jackson@citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Juergen Gross <jgross@suse.com>
> CC: Chun Yan Liu <cyliu@suse.com>
> CC: Olaf Hering <olaf@aepfle.de>
> 
> 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 <type>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
>   <type>ctrl and <type> 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
>   <ctrl,index> as the unique identifier.  (This would mean requiring
>   the index/port/lun specification above.)
> 
> * libxl_device_<type>_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 <type>, there will be two sets of the
> + * functions, types, and devid namespaces outlined above: one based on
> + * '<type>', and one based on '<type>ctrl'.
> + *
> + * In those cases, libxl_device_<type>ctrl_<function> 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
> + * <type>ctrl libxl_devid namespace.
> + *
> + * Second-level devices which will be attached to a controller will
> + * include in their libxl_device_<type> 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 <type> 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_<type>_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_<type>ctrl_list will list all controllers for the domain.
> + *
> + * libxl_device_<type>_list will list all devices for all controllers
> + * for the domain.  The individual libxl_device_<type> 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),
    ])

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH RFC v1] libxl: Introduce a template for devices with a controller
  2015-05-21 17:07 [PATCH RFC v1] libxl: Introduce a template for devices with a controller George Dunlap
  2015-05-21 17:11 ` George Dunlap
  2015-05-21 17:28 ` George Dunlap
@ 2015-05-22  4:21 ` Juergen Gross
  2015-05-26 17:56   ` George Dunlap
  2015-05-27 14:40 ` Ian Campbell
  2015-06-17 11:06 ` Ian Campbell
  4 siblings, 1 reply; 14+ messages in thread
From: Juergen Gross @ 2015-05-22  4:21 UTC (permalink / raw)
  To: George Dunlap, xen-devel
  Cc: Ian Jackson, Olaf Hering, Wei Liu, Ian Campbell, Chun Yan Liu

On 05/21/2015 07: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 <george.dunlap@eu.citrix.com>
> ---
> CC: Ian Campbell <ian.campbell@citrix.com>
> CC: Ian Jackson <ian.jackson@citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Juergen Gross <jgross@suse.com>
> CC: Chun Yan Liu <cyliu@suse.com>
> CC: Olaf Hering <olaf@aepfle.de>
>
> 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 <type>ctrl for the device name,
>    and using "ctrl" as the field name for the devid of the controller
>    in the individual devices.

Hmm, what about "device group" (<type>devgoup)? In the scsi world
"controller" would be one level higher in the hierarchy. And the scsi
controller is at least visible in the configuration syntax "h:c:t:l".
Using "controller" for the "c" in this item and for the "t" internally
could lead to confusion.


Juergen

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH RFC v1] libxl: Introduce a template for devices with a controller
  2015-05-22  4:21 ` Juergen Gross
@ 2015-05-26 17:56   ` George Dunlap
  2015-05-27 10:09     ` Juergen Gross
  0 siblings, 1 reply; 14+ messages in thread
From: George Dunlap @ 2015-05-26 17:56 UTC (permalink / raw)
  To: Juergen Gross
  Cc: Olaf Hering, Wei Liu, Ian Campbell, Chun Yan Liu, xen-devel, Ian Jackson

On Fri, May 22, 2015 at 5:21 AM, Juergen Gross <jgross@suse.com> wrote:
> On 05/21/2015 07: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 <george.dunlap@eu.citrix.com>
>> ---
>> CC: Ian Campbell <ian.campbell@citrix.com>
>> CC: Ian Jackson <ian.jackson@citrix.com>
>> CC: Wei Liu <wei.liu2@citrix.com>
>> CC: Juergen Gross <jgross@suse.com>
>> CC: Chun Yan Liu <cyliu@suse.com>
>> CC: Olaf Hering <olaf@aepfle.de>
>>
>> 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 <type>ctrl for the device name,
>>    and using "ctrl" as the field name for the devid of the controller
>>    in the individual devices.
>
>
> Hmm, what about "device group" (<type>devgoup)? In the scsi world
> "controller" would be one level higher in the hierarchy. And the scsi
> controller is at least visible in the configuration syntax "h:c:t:l".
> Using "controller" for the "c" in this item and for the "t" internally
> could lead to confusion.

OK, so I looked it up[1] and the full address seems to be:
* adapter number / host
* channel number / bus
* id number / target
* LUN

In which case, "controller" would correspond to "adapter / host", right?

In the vscsi world, what levels of what can you make?  I know you
mentioned before that some devices have multiple LUNs, and those need
to be grouped together, with the same LUNs as they do on real
hardware, to work properly -- is that right?

The USB case actually has something somewhat similar:
* USB controller
* USB bus
* USB device
* USB function

So far, there's not really a controller/bus distinction: each
controller has exactly one bus.  When we assign a USB device to a bus,
we automatically go through and assign each function fo that device
individually.

Would it make sense to treat vscsi the same way -- i.e., to make a
"bus", and then attach "targets"s to it, and have the LUNs for any
given target automatically assigned when the target is assigned?

 -George

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH RFC v1] libxl: Introduce a template for devices with a controller
  2015-05-26 17:56   ` George Dunlap
@ 2015-05-27 10:09     ` Juergen Gross
  2015-05-27 13:57       ` George Dunlap
  0 siblings, 1 reply; 14+ messages in thread
From: Juergen Gross @ 2015-05-27 10:09 UTC (permalink / raw)
  To: George Dunlap
  Cc: Olaf Hering, Wei Liu, Ian Campbell, Chun Yan Liu, xen-devel, Ian Jackson

George,

I'm on vacation this and the next week with only limited email access.
So please don't expect fast reaction on any further questions during
this time. :-)

On 05/26/2015 07:56 PM, George Dunlap wrote:
> On Fri, May 22, 2015 at 5:21 AM, Juergen Gross <jgross@suse.com> wrote:
>> On 05/21/2015 07: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 <george.dunlap@eu.citrix.com>
>>> ---
>>> CC: Ian Campbell <ian.campbell@citrix.com>
>>> CC: Ian Jackson <ian.jackson@citrix.com>
>>> CC: Wei Liu <wei.liu2@citrix.com>
>>> CC: Juergen Gross <jgross@suse.com>
>>> CC: Chun Yan Liu <cyliu@suse.com>
>>> CC: Olaf Hering <olaf@aepfle.de>
>>>
>>> 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 <type>ctrl for the device name,
>>>     and using "ctrl" as the field name for the devid of the controller
>>>     in the individual devices.
>>
>>
>> Hmm, what about "device group" (<type>devgoup)? In the scsi world
>> "controller" would be one level higher in the hierarchy. And the scsi
>> controller is at least visible in the configuration syntax "h:c:t:l".
>> Using "controller" for the "c" in this item and for the "t" internally
>> could lead to confusion.
>
> OK, so I looked it up[1] and the full address seems to be:
> * adapter number / host
> * channel number / bus
> * id number / target
> * LUN
>
> In which case, "controller" would correspond to "adapter / host", right?
>
> In the vscsi world, what levels of what can you make?  I know you
> mentioned before that some devices have multiple LUNs, and those need
> to be grouped together, with the same LUNs as they do on real
> hardware, to work properly -- is that right?

Not all of the devices have this requirement, but some.

> The USB case actually has something somewhat similar:
> * USB controller
> * USB bus
> * USB device
> * USB function
>
> So far, there's not really a controller/bus distinction: each
> controller has exactly one bus.  When we assign a USB device to a bus,
> we automatically go through and assign each function fo that device
> individually.
>
> Would it make sense to treat vscsi the same way -- i.e., to make a
> "bus", and then attach "targets"s to it, and have the LUNs for any
> given target automatically assigned when the target is assigned?

As long as it is still possible to assign individual LUNs as well.
If dom0 is controlling e.g. a RAID system you might want to assign
one LUN of a target to domU A and one LUN of the same target to domU B.


Juergen

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH RFC v1] libxl: Introduce a template for devices with a controller
  2015-05-27 10:09     ` Juergen Gross
@ 2015-05-27 13:57       ` George Dunlap
  2015-06-17 11:33         ` Juergen Gross
  0 siblings, 1 reply; 14+ messages in thread
From: George Dunlap @ 2015-05-27 13:57 UTC (permalink / raw)
  To: Juergen Gross
  Cc: Olaf Hering, Wei Liu, Ian Campbell, Chun Yan Liu, xen-devel, Ian Jackson

On 05/27/2015 11:09 AM, Juergen Gross wrote:
> George,
> 
> I'm on vacation this and the next week with only limited email access.
> So please don't expect fast reaction on any further questions during
> this time. :-)

Then quit reading your work e-mail and get back to the important stuff! :-)

>> OK, so I looked it up[1] and the full address seems to be:
>> * adapter number / host
>> * channel number / bus
>> * id number / target
>> * LUN
>>
>> In which case, "controller" would correspond to "adapter / host", right?
>>
>> In the vscsi world, what levels of what can you make?  I know you
>> mentioned before that some devices have multiple LUNs, and those need
>> to be grouped together, with the same LUNs as they do on real
>> hardware, to work properly -- is that right?
> 
> Not all of the devices have this requirement, but some.
> 
>> The USB case actually has something somewhat similar:
>> * USB controller
>> * USB bus
>> * USB device
>> * USB function
>>
>> So far, there's not really a controller/bus distinction: each
>> controller has exactly one bus.  When we assign a USB device to a bus,
>> we automatically go through and assign each function fo that device
>> individually.
>>
>> Would it make sense to treat vscsi the same way -- i.e., to make a
>> "bus", and then attach "targets"s to it, and have the LUNs for any
>> given target automatically assigned when the target is assigned?
> 
> As long as it is still possible to assign individual LUNs as well.
> If dom0 is controlling e.g. a RAID system you might want to assign
> one LUN of a target to domU A and one LUN of the same target to domU B.

OK, so it sounds like in the vscsi case, it would be useful to assign
either an entire target, or an individual LUN.

In the case of assigning a target, you'll want to assign all the LUNs as
well, such that the virtual LUNs mirror the real LUNs.

In the case of assigning a LUN, I assume you'll still need a virtual
target.  Will you be wanting an interface for creating virtual targets,
so that you can assign several real LUNs to the same target?  Or will
you just want one virtual target per LUN if you're not assigning an
entire target?

 -George

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH RFC v1] libxl: Introduce a template for devices with a controller
  2015-05-21 17:07 [PATCH RFC v1] libxl: Introduce a template for devices with a controller George Dunlap
                   ` (2 preceding siblings ...)
  2015-05-22  4:21 ` Juergen Gross
@ 2015-05-27 14:40 ` Ian Campbell
  2015-06-17 11:06 ` Ian Campbell
  4 siblings, 0 replies; 14+ messages in thread
From: Ian Campbell @ 2015-05-27 14:40 UTC (permalink / raw)
  To: George Dunlap
  Cc: Juergen Gross, Olaf Hering, Wei Liu, Chun Yan Liu, xen-devel,
	Ian Jackson

On Thu, 2015-05-21 at 18:07 +0100, 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.

Thanks for taking this on!

> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
> ---
> CC: Ian Campbell <ian.campbell@citrix.com>
> CC: Ian Jackson <ian.jackson@citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Juergen Gross <jgross@suse.com>
> CC: Chun Yan Liu <cyliu@suse.com>
> CC: Olaf Hering <olaf@aepfle.de>
> 
> 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 <type>ctrl for the device name,
>   and using "ctrl" as the field name for the devid of the controller
>   in the individual devices.

Controllers/ctrl is fine, as would Bus/bus. Or paint it pink ;-)

> * 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.

At the moment devid is not universally used, e.g. disk has ("vdev",
string), instead.

For devices (or controllers) where a flat integer devid space doesn't
make sense I think it would be fine to have something else, like a
<type>-specific devid type and a corresponding <type>ctrl specific
ctrlid type, which might be a (small) struct in either case and need not
be uniform across <type> and <type>ctrl nor among different <type>s.

Having this option of a more structured type might also help with the
conversation (in a subthread) regarding where in the
controller/bus/device/function the split between the two id spaces is,
since you aren't constrained to a single int at two levels trying to
describe 4 levels.

Using a string as disks do is probably best not considered as a
precedent here, but could be appropriate on a device-by-device basis.

> * 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
>   <type>ctrl and <type> 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
>   <ctrl,index> as the unique identifier.  (This would mean requiring
>   the index/port/lun specification above.)

I'm not quite sure what you are getting at here, in particular where we
will "have both <type>ctrl and <type> devid namespaces", in the structs
or in the protoptyes.

I was imaging that where the current templates have (ctx, domid, device)
that a controller related function would be (ctx, domid, ctrl) and the
device related function would become (ctx, domid, ctrlid, device). ctrl
and device would contain their respective ctrlid and devid fields.

Perhaps it might be clearer if the proposal included specific function
prototype templates along the lines of the existing
"libxl_device_<type>_add(ctx, domid, device)".

I think you were trying to avoid duplication by presenting this new
scheme as an extension to what is currently written there. It might be
clearer to simply present it as a separate alternative or to list both
possible prototypes next to the current section where things differ in
the two schemes? (and describe it in the text)

Or perhaps to insert some optional parameters into the existing ones
where it makes sense:
        libxl_device_<type>_add(ctx, domid, [ctrlid, ]device)

Not sure which would be least confusing...

> * libxl_device_<type>_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.

I agree with listing everything, I think.

If we find a need to only list per controller ID then we can add an
alternative (list_by_ctrl, device_<type>ctrl_list_devices, or whatever).

> Feedback welcome.

Having said all that I don't seem to have any more comments on the
actual text itself, apart from one typo.

> ---
>  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 <type>, there will be two sets of the
> + * functions, types, and devid namespaces outlined above: one based on
> + * '<type>', and one based on '<type>ctrl'.
> + *
> + * In those cases, libxl_device_<type>ctrl_<function> 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
> + * <type>ctrl libxl_devid namespace.
> + *
> + * Second-level devices which will be attached to a controller will
> + * include in their libxl_device_<type> 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 <type> 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_<type>_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_<type>ctrl_list will list all controllers for the domain.
> + *
> + * libxl_device_<type>_list will list all devices for all controllers
> + * for the domain.  The individual libxl_device_<type> will include
> + * the devid of the controller to which it is attached.

[ijc -- inserted into, hopefully correct, place from following mail]
> For each type, the domain config file will contain a single list of
> controllers, and a single list of devices.  libxl will first iterate
> through the list adding the controlllers, then iterate through the list

"controllers"

> adding each device to the ctrl listed.  If libxl_device_<type>_add
> automatically creates controllers as necessary, then it is permissible
> for the controller list to be empty and the device list to have devices
> (with the ctrl field uninitialized).

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH RFC v1] libxl: Introduce a template for devices with a controller
  2015-05-21 17:07 [PATCH RFC v1] libxl: Introduce a template for devices with a controller George Dunlap
                   ` (3 preceding siblings ...)
  2015-05-27 14:40 ` Ian Campbell
@ 2015-06-17 11:06 ` Ian Campbell
  2015-06-17 11:28   ` Juergen Gross
  2015-06-18  5:18   ` Chun Yan Liu
  4 siblings, 2 replies; 14+ messages in thread
From: Ian Campbell @ 2015-06-17 11:06 UTC (permalink / raw)
  To: George Dunlap
  Cc: Juergen Gross, Olaf Hering, Wei Liu, Chun Yan Liu, xen-devel,
	Ian Jackson

On Thu, 2015-05-21 at 18:07 +0100, 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.

Chun Yan and Jeurgen:

I was hoping we could come to some sort of agreement on this such that
it can be used as the basis for both the pvusb and pvscsi interfaces. As
such your feedback here is important...

> 
> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
> ---
> CC: Ian Campbell <ian.campbell@citrix.com>
> CC: Ian Jackson <ian.jackson@citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Juergen Gross <jgross@suse.com>
> CC: Chun Yan Liu <cyliu@suse.com>
> CC: Olaf Hering <olaf@aepfle.de>
> 
> 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 <type>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
>   <type>ctrl and <type> 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
>   <ctrl,index> as the unique identifier.  (This would mean requiring
>   the index/port/lun specification above.)
> 
> * libxl_device_<type>_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 <type>, there will be two sets of the
> + * functions, types, and devid namespaces outlined above: one based on
> + * '<type>', and one based on '<type>ctrl'.
> + *
> + * In those cases, libxl_device_<type>ctrl_<function> 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
> + * <type>ctrl libxl_devid namespace.
> + *
> + * Second-level devices which will be attached to a controller will
> + * include in their libxl_device_<type> 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 <type> 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_<type>_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_<type>ctrl_list will list all controllers for the domain.
> + *
> + * libxl_device_<type>_list will list all devices for all controllers
> + * for the domain.  The individual libxl_device_<type> will include
> + * the devid of the controller to which it is attached.
>   */
>  
>  /* Disks */

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH RFC v1] libxl: Introduce a template for devices with a controller
  2015-06-17 11:06 ` Ian Campbell
@ 2015-06-17 11:28   ` Juergen Gross
  2015-06-17 13:14     ` Ian Campbell
  2015-06-18  5:18   ` Chun Yan Liu
  1 sibling, 1 reply; 14+ messages in thread
From: Juergen Gross @ 2015-06-17 11:28 UTC (permalink / raw)
  To: Ian Campbell, George Dunlap
  Cc: Ian Jackson, Olaf Hering, Wei Liu, Chun Yan Liu, xen-devel

On 06/17/2015 01:06 PM, Ian Campbell wrote:
> On Thu, 2015-05-21 at 18:07 +0100, 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.
>
> Chun Yan and Jeurgen:
>
> I was hoping we could come to some sort of agreement on this such that
> it can be used as the basis for both the pvusb and pvscsi interfaces. As
> such your feedback here is important...

I already gave some feedback. As everything regarding this feedback
has been discussed:

Acked-by: Juergen Gross <jgross@suse.com>

>
>>
>> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
>> ---
>> CC: Ian Campbell <ian.campbell@citrix.com>
>> CC: Ian Jackson <ian.jackson@citrix.com>
>> CC: Wei Liu <wei.liu2@citrix.com>
>> CC: Juergen Gross <jgross@suse.com>
>> CC: Chun Yan Liu <cyliu@suse.com>
>> CC: Olaf Hering <olaf@aepfle.de>
>>
>> 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 <type>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
>>    <type>ctrl and <type> 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
>>    <ctrl,index> as the unique identifier.  (This would mean requiring
>>    the index/port/lun specification above.)
>>
>> * libxl_device_<type>_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 <type>, there will be two sets of the
>> + * functions, types, and devid namespaces outlined above: one based on
>> + * '<type>', and one based on '<type>ctrl'.
>> + *
>> + * In those cases, libxl_device_<type>ctrl_<function> 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
>> + * <type>ctrl libxl_devid namespace.
>> + *
>> + * Second-level devices which will be attached to a controller will
>> + * include in their libxl_device_<type> 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 <type> 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_<type>_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_<type>ctrl_list will list all controllers for the domain.
>> + *
>> + * libxl_device_<type>_list will list all devices for all controllers
>> + * for the domain.  The individual libxl_device_<type> will include
>> + * the devid of the controller to which it is attached.
>>    */
>>
>>   /* Disks */
>
>
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH RFC v1] libxl: Introduce a template for devices with a controller
  2015-05-27 13:57       ` George Dunlap
@ 2015-06-17 11:33         ` Juergen Gross
  0 siblings, 0 replies; 14+ messages in thread
From: Juergen Gross @ 2015-06-17 11:33 UTC (permalink / raw)
  To: George Dunlap
  Cc: Olaf Hering, Wei Liu, Ian Campbell, Chun Yan Liu, xen-devel, Ian Jackson

On 05/27/2015 03:57 PM, George Dunlap wrote:
> On 05/27/2015 11:09 AM, Juergen Gross wrote:
>> George,
>>
>> I'm on vacation this and the next week with only limited email access.
>> So please don't expect fast reaction on any further questions during
>> this time. :-)
>
> Then quit reading your work e-mail and get back to the important stuff! :-)
>
>>> OK, so I looked it up[1] and the full address seems to be:
>>> * adapter number / host
>>> * channel number / bus
>>> * id number / target
>>> * LUN
>>>
>>> In which case, "controller" would correspond to "adapter / host", right?
>>>
>>> In the vscsi world, what levels of what can you make?  I know you
>>> mentioned before that some devices have multiple LUNs, and those need
>>> to be grouped together, with the same LUNs as they do on real
>>> hardware, to work properly -- is that right?
>>
>> Not all of the devices have this requirement, but some.
>>
>>> The USB case actually has something somewhat similar:
>>> * USB controller
>>> * USB bus
>>> * USB device
>>> * USB function
>>>
>>> So far, there's not really a controller/bus distinction: each
>>> controller has exactly one bus.  When we assign a USB device to a bus,
>>> we automatically go through and assign each function fo that device
>>> individually.
>>>
>>> Would it make sense to treat vscsi the same way -- i.e., to make a
>>> "bus", and then attach "targets"s to it, and have the LUNs for any
>>> given target automatically assigned when the target is assigned?
>>
>> As long as it is still possible to assign individual LUNs as well.
>> If dom0 is controlling e.g. a RAID system you might want to assign
>> one LUN of a target to domU A and one LUN of the same target to domU B.
>
> OK, so it sounds like in the vscsi case, it would be useful to assign
> either an entire target, or an individual LUN.
>
> In the case of assigning a target, you'll want to assign all the LUNs as
> well, such that the virtual LUNs mirror the real LUNs.
>
> In the case of assigning a LUN, I assume you'll still need a virtual
> target.  Will you be wanting an interface for creating virtual targets,
> so that you can assign several real LUNs to the same target?  Or will
> you just want one virtual target per LUN if you're not assigning an
> entire target?

Nearly missed this question.

Hmm, I think the first option would be better. Otherwise it could be
difficult to assign a just created LUN of a target to the same virtual
target while the system is running.


Juergen

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH RFC v1] libxl: Introduce a template for devices with a controller
  2015-06-17 11:28   ` Juergen Gross
@ 2015-06-17 13:14     ` Ian Campbell
  2015-06-18  5:55       ` Olaf Hering
  0 siblings, 1 reply; 14+ messages in thread
From: Ian Campbell @ 2015-06-17 13:14 UTC (permalink / raw)
  To: Juergen Gross
  Cc: Olaf Hering, Wei Liu, George Dunlap, Chun Yan Liu, xen-devel,
	Ian Jackson

On Wed, 2015-06-17 at 13:28 +0200, Juergen Gross wrote:
> On 06/17/2015 01:06 PM, Ian Campbell wrote:
> > On Thu, 2015-05-21 at 18:07 +0100, 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.
> >
> > Chun Yan and Jeurgen:
> >
> > I was hoping we could come to some sort of agreement on this such that
> > it can be used as the basis for both the pvusb and pvscsi interfaces. As
> > such your feedback here is important...
> 
> I already gave some feedback. As everything regarding this feedback
> has been discussed:
> 
> Acked-by: Juergen Gross <jgross@suse.com>

Actually, maybe I really meant to ping Olaf instead/as well since he's
doing the libxl side of pvscsi (you're doing the driver upstreaming,
right?).

So, Olaf, ping...

> 
> >
> >>
> >> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
> >> ---
> >> CC: Ian Campbell <ian.campbell@citrix.com>
> >> CC: Ian Jackson <ian.jackson@citrix.com>
> >> CC: Wei Liu <wei.liu2@citrix.com>
> >> CC: Juergen Gross <jgross@suse.com>
> >> CC: Chun Yan Liu <cyliu@suse.com>
> >> CC: Olaf Hering <olaf@aepfle.de>
> >>
> >> 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 <type>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
> >>    <type>ctrl and <type> 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
> >>    <ctrl,index> as the unique identifier.  (This would mean requiring
> >>    the index/port/lun specification above.)
> >>
> >> * libxl_device_<type>_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 <type>, there will be two sets of the
> >> + * functions, types, and devid namespaces outlined above: one based on
> >> + * '<type>', and one based on '<type>ctrl'.
> >> + *
> >> + * In those cases, libxl_device_<type>ctrl_<function> 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
> >> + * <type>ctrl libxl_devid namespace.
> >> + *
> >> + * Second-level devices which will be attached to a controller will
> >> + * include in their libxl_device_<type> 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 <type> 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_<type>_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_<type>ctrl_list will list all controllers for the domain.
> >> + *
> >> + * libxl_device_<type>_list will list all devices for all controllers
> >> + * for the domain.  The individual libxl_device_<type> will include
> >> + * the devid of the controller to which it is attached.
> >>    */
> >>
> >>   /* Disks */
> >
> >
> >
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH RFC v1] libxl: Introduce a template for devices with a controller
  2015-06-17 11:06 ` Ian Campbell
  2015-06-17 11:28   ` Juergen Gross
@ 2015-06-18  5:18   ` Chun Yan Liu
  1 sibling, 0 replies; 14+ messages in thread
From: Chun Yan Liu @ 2015-06-18  5:18 UTC (permalink / raw)
  To: Ian Campbell, George Dunlap
  Cc: Juergen Gross, Ian Jackson, Olaf Hering, Wei Liu, xen-devel



>>> On 6/17/2015 at 07:06 PM, in message <1434539195.13744.321.camel@citrix.com>,
Ian Campbell <ian.campbell@citrix.com> wrote: 
> On Thu, 2015-05-21 at 18:07 +0100, 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. 
>  
> Chun Yan and Jeurgen: 
>  
> I was hoping we could come to some sort of agreement on this such that 
> it can be used as the basis for both the pvusb and pvscsi interfaces. As 
> such your feedback here is important... 
>  
> >  
> > Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com> 
> > --- 
> > CC: Ian Campbell <ian.campbell@citrix.com> 
> > CC: Ian Jackson <ian.jackson@citrix.com> 
> > CC: Wei Liu <wei.liu2@citrix.com> 
> > CC: Juergen Gross <jgross@suse.com> 
> > CC: Chun Yan Liu <cyliu@suse.com> 
> > CC: Olaf Hering <olaf@aepfle.de> 
> >  
> > 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 <type>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 
> >   <type>ctrl and <type> 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 
> >   <ctrl,index> as the unique identifier.  (This would mean requiring 
> >   the index/port/lun specification above.) 
> >  
> > * libxl_device_<type>_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 <type>, there will be two sets of the 
> > + * functions, types, and devid namespaces outlined above: one based on 
> > + * '<type>', and one based on '<type>ctrl'. 
> > + * 
> > + * In those cases, libxl_device_<type>ctrl_<function> 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 
> > + * <type>ctrl libxl_devid namespace. 
> > + * 
> > + * Second-level devices which will be attached to a controller will 
> > + * include in their libxl_device_<type> 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 <type> devid namespace, and will be used 
> > + * for queries or removals.

All other description is agreed except here:
For pvusb, currently we uses <ctrl, port> instead of devid. It seems to be
more straightforward. To add a USB device info to xenstore, it only writes
the USB busid to controller/port. To remove a USB device, just remove
USB busid from xenstore controller/port.

- Chunyan
 
> > + * 
> > + * 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_<type>_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_<type>ctrl_list will list all controllers for the domain. 
> > + * 
> > + * libxl_device_<type>_list will list all devices for all controllers 
> > + * for the domain.  The individual libxl_device_<type> will include 
> > + * the devid of the controller to which it is attached. 
> >   */ 
> >   
> >  /* Disks */ 
>  
>  
>  
> _______________________________________________ 
> Xen-devel mailing list 
> Xen-devel@lists.xen.org 
> http://lists.xen.org/xen-devel 
>  
>  

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH RFC v1] libxl: Introduce a template for devices with a controller
  2015-06-17 13:14     ` Ian Campbell
@ 2015-06-18  5:55       ` Olaf Hering
  0 siblings, 0 replies; 14+ messages in thread
From: Olaf Hering @ 2015-06-18  5:55 UTC (permalink / raw)
  To: Ian Campbell
  Cc: Juergen Gross, Wei Liu, George Dunlap, Chun Yan Liu, xen-devel,
	Ian Jackson

On Wed, Jun 17, Ian Campbell wrote:

> So, Olaf, ping...

I will return to pvscsi work next week.

Olaf

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2015-06-18  5:55 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-21 17:07 [PATCH RFC v1] libxl: Introduce a template for devices with a controller George Dunlap
2015-05-21 17:11 ` George Dunlap
2015-05-21 17:28 ` George Dunlap
2015-05-22  4:21 ` Juergen Gross
2015-05-26 17:56   ` George Dunlap
2015-05-27 10:09     ` Juergen Gross
2015-05-27 13:57       ` George Dunlap
2015-06-17 11:33         ` Juergen Gross
2015-05-27 14:40 ` Ian Campbell
2015-06-17 11:06 ` Ian Campbell
2015-06-17 11:28   ` Juergen Gross
2015-06-17 13:14     ` Ian Campbell
2015-06-18  5:55       ` Olaf Hering
2015-06-18  5:18   ` Chun Yan Liu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.