libxl: New xl interfaces for managing USB devices

RFC PATCH In review Bo Cao 2 participants 2 messages Jun 24, 2014 – Jul 2, 2014

lore

Share & tools

lore

Bo Cao

author

2014-06-24 14:31 UTC #0 lore raw

RFC PATCH 1/1 libxl: New xl interfaces for managing USB devices

Hey, all

This patch is mainly about managing USB devices both for PVUSB and DEVICEMODEL.It mainly consits of two part, USB controller creation and destruction and USB device's attachment and detachment.

For USB controller:
usb-controller-create <domain-id> <usb-version> <number-of-ports> [controller-type] usb-controller-destroy <domain-id> <controller-name>

For USB device:
usb-attach <domain-id> <hostbus.hostdev> [controller_name] usb-detach <domain-id> <hostbus.hostdev>
usb-list <domain-id>

Feel free to give me any commnets.
Thanks!

Simon

/**************************************************************/ Design the new xl interfaces to managing USB devices, both for PVUSB and DEVICEMODEL.

Signed-off-by Simon Cao <[email protected]>

---
CC: George Dunlap <[email protected]>
CC: Ian Jackson <[email protected]>
CC: Ian Campbell <[email protected]>
CC: Pasi Kärkkäinen <[email protected]>

---
 docs/man/xl.pod.1           |   39 +++++++++++++++++++++++++++++++++++++++
 tools/libxl/libxl.h         |   26 ++++++++++++++++++++++++++
 tools/libxl/libxl_types.idl |   27 +++++++++++++++++++++++++++
 tools/libxl/xl_cmdtable.c   |   33 +++++++++++++++++++++++++++++++++
 4 files changed, 125 insertions(+)

docs/man/xl.pod.1 +39 -0

index 30bd4bf..db76ce2 100644
@@ -1194,6 +1194,45 @@ List virtual network interfaces for a domain.
 
 =back
 
+=head2 USB DEVICES
+
+=over 4
+
+=item B<usb-assginable-list>
+
+List all the assignable USB devices.
+
+=item B<usb-assigned-list>
+
+List all the USB devices that has been assigned.
+
+=item B<usb-controller-create> I<usb-version> I<number_of_ports> [I<controller_type>]
+
+Creates a new usb virtual controller for the domain specified by I<domain-id>.
+I<usb-version> describes the version of USB controller to create, i.e., 1, 2 or 3, 
+I<number_of_ports> describers the number of ports in the USB virtual controller, and 
+I<controller_type> describes the type of USB controller, PVUSB or EMULATED.
+
+=item B<usb-controller-destroy> I<domain-id> I<controller-name>
+
+Destroies the virtual USB controller specified by I<controller-name> in I<domain-id>.
+
+=item B<usb-attach> I<domain-id> I<hostbus.hostaddr> [I<controller-name>]
+
+Attaches USB device specified by I<hostbus.hostaddr> to USB controller specified by 
+I<controller-name> in domain I<domain-id>.
+
+=item B<usb-detach> I<domain-id>  I<hostbus.hostaddr>
+
+Detaches the USB device I<hostbus.hostaddr> in domain I<domain-id>.
+
+=item B<usb-list> I<domain-id>
+
+List all the virtual USB controllers and USB devices for the domain 
+specified by I<domain-id>.
+
+= back
+
 =head2 VTPM DEVICES
 
 =over 4

tools/libxl/libxl.h +26 -0

index 17b8a7b..863c86c 100644
@@ -950,6 +950,32 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
                        const libxl_asyncop_how *ao_how)
                        LIBXL_EXTERNAL_CALLERS_ONLY;
 
+/* USB Devices*/
+int libxl_device_usb_controller_create(libxl_ctxlibxl_ctx *ctx, uint32_t domid,
+						 libxl_device_usb_controller *dev,
+                         const libxl_asyncop_how *ao_how)
+                         LIBXL_EXTERNAL_CALLERS_ONLY;
+
+int libxl_device_usb_controller_destroy(libxl_ctxlibxl_ctx *ctx, uint32_t domid,
+                         libxl_device_usb_controller *dev,
+                         const libxl_asyncop_how *ao_how)
+                         LIBXL_EXTERNAL_CALLERS_ONLY;
+
+int libxl_device_usb_add(libxl_ctx *ctx, uint32_t domid,
+                         libxl_device_usb *dev,
+                         const libxl_asyncop_how *ao_how)
+                         LIBXL_EXTERNAL_CALLERS_ONLY;
+
+int libxl_device_usb_remove(libxl_ctx *ctx, uint32_t domid,
+                            libxl_device_usb *dev,
+                            const libxl_asyncop_how *ao_how)
+                            LIBXL_EXTERNAL_CALLERS_ONLY;
+
+libxl_device_usb *libxl_device_usb_list(libxl_ctx *ctx, uint32_t domid,
+                                        int *num)
+                          LIBXL_EXTERNAL_CALLERS_ONLY;
+
+
 /* Network Interfaces */
 int libxl_device_nic_add(libxl_ctx *ctx, uint32_t domid, libxl_device_nic *nic,
                          const libxl_asyncop_how *ao_how)

tools/libxl/libxl_types.idl +27 -0

index f0f6e34..28d896e 100644
@@ -85,6 +85,16 @@ libxl_nic_type = Enumeration("nic_type", [
     (2, "VIF"),
     ])
 
+libxl_usb_controller_type = Enumeration("usb_controller_type",[
+	(0, "AUTO"),
+	(1, "PV"),
+	(2, "DEVICEMODEL"),
+	])
+
+libxl_usb_type = Enumeration("device_usb_type", [
+    (1, "HOSTDEV"),
+    ])
+
 libxl_action_on_shutdown = Enumeration("action_on_shutdown", [
     (1, "DESTROY"),

@@ -448,6 +458,22 @@ libxl_device_pci = Struct("device_pci", [
     ("seize", bool),
     ])
 
+libxl_device_usb_controller = Struct("device_usb_controller", [
+	("name", string),
+	("type", libxl_usb_controller_type),
+	("backend_domid", libxl_domid),
+	("backend_domname",  string),
+	("usb_version", uint8),
+	("num_ports", uint8),
+	])
+
+libxl_device_usb = Struct("device_usb", [
+	("controller_name", string),
+	("type", libxl_usb_type),
+	("bus", uint8),
+	("dev", uint8),
+	]) 
+	 
 libxl_device_vtpm = Struct("device_vtpm", [
     ("backend_domid",    libxl_domid),
     ("backend_domname",  string),

@@ -462,6 +488,7 @@ libxl_domain_config = Struct("domain_config", [
     ("disks", Array(libxl_device_disk, "num_disks")),
     ("nics", Array(libxl_device_nic, "num_nics")),
     ("pcidevs", Array(libxl_device_pci, "num_pcidevs")),
+	("usbs", Array(libxl_device_usb, "num_usbs")),
     ("vfbs", Array(libxl_device_vfb, "num_vfbs")),
     ("vkbs", Array(libxl_device_vkb, "num_vkbs")),
     ("vtpms", Array(libxl_device_vtpm, "num_vtpms")),

tools/libxl/xl_cmdtable.c +33 -0

index 4279b9f..6c0ae9a 100644
@@ -194,6 +194,39 @@ struct cmd_spec cmd_table[] = {
       "Eject a cdrom from a guest's cd drive",
       "<Domain> <VirtualDevice>",
     },
+	{ "usb-assignable-list",
+	  &main_usb_assignable_list, 0, 0,
+	  "List all the assignable USB devices",
+	},
+	{ "usb-assigned-list",
+	  &main_usb_assigned_list. 0, 0,
+	  "List all the USB devices that have been assigned",
+	},
+	{ "usb-controller-create",
+	  &main_usb_controller_create, 1, 1,
+	  "Create a virtual USB controller for a domain",
+	  "<Domain> <UsbVersion['1'[1.1]|'2'[2.0]|'3'[3.0]]> <NumberOfPorts> [ControllerType['pv'|'emu']]",
+	}
+	{ "usb-controller-destroy",
+	  &main_usb_controller_destroy, 0, 1,
+	  "Destory the virtual USB controller specified by <ControllerName> for a domain",
+	  "<Domain> <ControllerName['pv0'|'hci0'|..]>",
+	},
+	{ "usb-attach",
+	  &main_usb_attach, 1, 1,
+	  "Attach a USB device to a domain",
+	  "<Domain> <Hostbus.Hostdev> [ControllerName['pv0'|'hci0']]",
+	},
+	{ "usb-detach",
+	  &main_usb_detach, 0, 1,
+	  "Detach a USB device from a domain",
+	  "<Domain> <Hostbus.Hostdev>",
+	},
+	{ "usb-list",
+	  &main_usb_list, 0, 0,
+	  "List information about USB devices for a domain",
+	  "<Domain>",
+	},
     { "mem-max",
       &main_memmax, 0, 1,
       "Set the maximum amount reservation for a domain",

Signature · 7 lines

1.7.10.4


_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

I'm not really sure we need this one. The only other time we have this is for PCI devices; but that's because PCI devices have an extra step: they must first be seized by pciback, and then assigned to the domain.

There is no equivalent (AFAIK) for USB devices to this; so this would just be duplicating the functionality of lsusb. There's no "pci-list-seizable-devices" to show what devices can be passed to "pci-assignable-add"; you just have to use lspci. We should follow suit here.

docs/man/xl.pod.1 List virtual network interfaces for a domain. · line 1207

Quoted 6 lines

docs/man/xl.pod.1 +6 -0

+
+List all the assignable USB devices.
+
+=item B<usb-assigned-list>
+
+List all the USB devices that has been assigned.

I take it the difference between this one and "usb-list" is that "usb-assigned-list" is meant to list usb devices across all domains, whereas "usb-list" is meant to be for just a single domain?

We don't have this functionality for pci devices; if we want that I think we should probably just implement it as an option to usb-list (e.g., "xl usb-list -a" to list devices across all domains).

docs/man/xl.pod.1 List virtual network interfaces for a domain. · line 1218

Quoted 11 lines

docs/man/xl.pod.1 +11 -0

+
+=item B<usb-controller-create> I<usb-version> I<number_of_ports> [I<controller_type>]
+
+Creates a new usb virtual controller for the domain specified by I<domain-id>.
+I<usb-version> describes the version of USB controller to create, i.e., 1, 2 or 3,
+I<number_of_ports> describers the number of ports in the USB virtual controller, and
+I<controller_type> describes the type of USB controller, PVUSB or EMULATED.
+
+=item B<usb-controller-destroy> I<domain-id> I<controller-name>
+
+Destroies the virtual USB controller specified by I<controller-name> in I<domain-id>.

In keeping with the other device commands, these should be "usb-controller-attach" and "usb-controller-detach". Other than that, looks good.

docs/man/xl.pod.1 List virtual network interfaces for a domain. · line 1227

Quoted 9 lines

docs/man/xl.pod.1 +9 -0

+
+=item B<usb-attach> I<domain-id> I<hostbus.hostaddr> [I<controller-name>]
+
+Attaches USB device specified by I<hostbus.hostaddr> to USB controller specified by
+I<controller-name> in domain I<domain-id>.
+
+=item B<usb-detach> I<domain-id>  I<hostbus.hostaddr>
+
+Detaches the USB device I<hostbus.hostaddr> in domain I<domain-id>.

We want to use the same interface to be able to add other kinds of devices (tablets, mice, keyboards, maybe mass storage) in the future. I think for now this should be something like "hostdev:<hostbus.hostaddr>".

Other than that, looks good.

Quoted 19 lines

docs/man/xl.pod.1 +8 -0

+
+=item B<usb-list> I<domain-id>
+
+List all the virtual USB controllers and USB devices for the domain
+specified by I<domain-id>.
+
+= back
+
  =head2 VTPM DEVICES
 
  =over 4

tools/libxl/libxl.h +0 -0

index 17b8a7b..863c86c 100644
@@ -950,6 +950,32 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
                         const libxl_asyncop_how *ao_how)
                         LIBXL_EXTERNAL_CALLERS_ONLY;

tools/libxl/libxl.h int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk, · line 962

Quoted 10 lines

tools/libxl/libxl.h +10 -0

+/* USB Devices*/
+int libxl_device_usb_controller_create(libxl_ctxlibxl_ctx *ctx, uint32_t domid,
+						 libxl_device_usb_controller *dev,
+                         const libxl_asyncop_how *ao_how)
+                         LIBXL_EXTERNAL_CALLERS_ONLY;
+
+int libxl_device_usb_controller_destroy(libxl_ctxlibxl_ctx *ctx, uint32_t domid,
+                         libxl_device_usb_controller *dev,
+                         const libxl_asyncop_how *ao_how)
+                         LIBXL_EXTERNAL_CALLERS_ONLY;

These should be libxl_device_usb_controller_{add,remove,destroy,list}. Looks good otherwise.

tools/libxl/libxl.h int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk, · line 972

Quoted 10 lines

tools/libxl/libxl.h +10 -0

+
+int libxl_device_usb_add(libxl_ctx *ctx, uint32_t domid,
+                         libxl_device_usb *dev,
+                         const libxl_asyncop_how *ao_how)
+                         LIBXL_EXTERNAL_CALLERS_ONLY;
+
+int libxl_device_usb_remove(libxl_ctx *ctx, uint32_t domid,
+                            libxl_device_usb *dev,
+                            const libxl_asyncop_how *ao_how)
+                            LIBXL_EXTERNAL_CALLERS_ONLY;

You also need libxl_device_usb_destroy().

The difference in theory between remove and destroy is for things like PV disks: remove will remove the disk with the cooperation of the guest, but may fail if the guest is buggy or otherwise doesn't cooperate. Destoy will yank the disk out whether the guest likes it or not (potentially resulting in data corruption).

For a lot of devices (I think pci is like this) there's actually no distinction, and they both map to the same function internally. Not sure what the PVUSB protocol is like in this regard; but in any case you need both functions even if there's no practical difference between the two.

Quoted 32 lines

tools/libxl/libxl.h +6 -0

+
+libxl_device_usb *libxl_device_usb_list(libxl_ctx *ctx, uint32_t domid,
+                                        int *num)
+                          LIBXL_EXTERNAL_CALLERS_ONLY;
+
+
  /* Network Interfaces */
  int libxl_device_nic_add(libxl_ctx *ctx, uint32_t domid, libxl_device_nic *nic,
                           const libxl_asyncop_how *ao_how)

tools/libxl/libxl_types.idl +10 -0

index f0f6e34..28d896e 100644
@@ -85,6 +85,16 @@ libxl_nic_type = Enumeration("nic_type", [
      (2, "VIF"),
      ])
 
+libxl_usb_controller_type = Enumeration("usb_controller_type",[
+	(0, "AUTO"),
+	(1, "PV"),
+	(2, "DEVICEMODEL"),
+	])
+
+libxl_usb_type = Enumeration("device_usb_type", [
+    (1, "HOSTDEV"),
+    ])
+
  libxl_action_on_shutdown = Enumeration("action_on_shutdown", [
      (1, "DESTROY"),

@@ -448,6 +458,22 @@ libxl_device_pci = Struct("device_pci", [
      ("seize", bool),

tools/libxl/libxl_types.idl libxl_device_pci = Struct("device_pci", [ · line 468

Quoted 10 lines

tools/libxl/libxl_types.idl +8 -0

      ])
 
+libxl_device_usb_controller = Struct("device_usb_controller", [
+	("name", string),
+	("type", libxl_usb_controller_type),
+	("backend_domid", libxl_domid),
+	("backend_domname",  string),
+	("usb_version", uint8),
+	("num_ports", uint8),
+	])

Looks good.

Hmm, also just noticed that you're using hard tabs. Xen coding style says you should only be using spaces for indentation; tabs are forbidden.

If you're using emacs, the comment at the end of most source files should do this automatically; if you're using a different editor, you'll have to figure out how to make it use only spaces.

tools/libxl/libxl_types.idl libxl_device_pci = Struct("device_pci", [ · line 475

Quoted 7 lines

tools/libxl/libxl_types.idl +7 -0

+
+libxl_device_usb = Struct("device_usb", [
+	("controller_name", string),
+	("type", libxl_usb_type),
+	("bus", uint8),
+	("dev", uint8),
+	])

Is there a reason you got rid of the KeyedUnion construct?

tools/libxl/libxl_types.idl libxl_domain_config = Struct("domain_config", [ · line 491

Quoted 9 lines

tools/libxl/libxl_types.idl +2 -0

+	
  libxl_device_vtpm = Struct("device_vtpm", [
      ("backend_domid",    libxl_domid),
      ("backend_domname",  string),

@@ -462,6 +488,7 @@ libxl_domain_config = Struct("domain_config", [
      ("disks", Array(libxl_device_disk, "num_disks")),
      ("nics", Array(libxl_device_nic, "num_nics")),
      ("pcidevs", Array(libxl_device_pci, "num_pcidevs")),
+	("usbs", Array(libxl_device_usb, "num_usbs")),

You need a list of controllers as well as a list of usb devices.

(You can still have the domain builder automatically create usb controllers as needed if the list is empty.)

Thanks -- look forward to seeing the code!

-George

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel