* [PATCH v7 RFC 1/2] libxl: Introduce functions to add and remove USB devices to an HVM guest
[not found] <1401716658-22393-1-git-send-email-george.dunlap@eu.citrix.com>
@ 2014-06-02 13:44 ` George Dunlap
2014-06-18 13:08 ` Ian Campbell
2014-06-02 13:44 ` [PATCH v7 RFC 2/2] xl: Add commands for usb hot-plug George Dunlap
` (4 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: George Dunlap @ 2014-06-02 13:44 UTC (permalink / raw)
To: xen-devel
Cc: Ian Campbell, George Dunlap, sstanisi, Fabio Fantoni,
Anthony Perard, Ian Jackson, Simon (Bo) Cao, Roger Pau Monne
This patch exposes a generic interface which can be expanded in the
future to implement USB for PVUSB, qemu, and stubdoms. It can also be
extended to include other types of USB other than host USB (for example,
tablets, mice, or keyboards).
For each device removed or added, one of two protocols is available:
* PVUSB
* qemu (DEVICEMODEL)
The caller can additionally specify "AUTO", in which case the library will
try to determine the best protocol automatically.
At the moment, the only protocol implemented is DEVICEMODEL, and the only
device type impelmented is HOSTDEV.
This uses the qmp functionality, and is thus only available for
qemu-xen, not qemu-traditional.
Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
---
History:
v7:
- Correct spelling mistake
- Add libxl_device_usb_destroy
- Add backend_domname to device_struct
- Don't have a default backend
- Implement libxl__device_usb_setdefault
- Get rid of "internal" usbdev struct
- Fix author of libxl_usb.c
- Use LOG() consistently
- Don't unnecessarily cast hostbus/hostaddr to uint16 in the USB xenstore path
- Create libxl__domain_usb_init function to put all usb-related xenstore stuff
in one file
- Call libxl__xs_transaction_abort on the failure path of usb_add_xenstore
- Do element-by-element compare rather than memcmp
- Use QMP_PARAMETERS_SPRINTF for id in libxl__qmp_usb_hostdev_{add,remove}
rather than a separate GSPRINTF
- Fix memory leak in failure path of get_assigned_devices()
- Handle non-NULL empty list returned by libxl__xs_directory in get_assigned_devices
- Don't garbage-collect the list when called from libxl_device_usb_list()
v6:
- Return a proper error code if libxl__xs_mkdir fails
- Make casts cuddly
- Add a space after switch statements
v5:
- fix erroneous use of NOGC in qmp functions
- Don't check return of libxl__sprintf in libxl_create.c
- Check return values of new xs actions in libxl_create.c
- Use updated template for xenstore transactions, do checks
- use xs_read_checked
- Fixed if (x) spacing to match coding style
- use GCSFPRINTF
- use libxl__calloc
- Fix comment for LIBXL_HAVE_USB
- use idl-generated *_from_string() and *_to_string() functions
CC: Ian Jackson <ian.jackson@citrix.com>
CC: Roger Pau Monne <roger.pau@citrix.com>
CC: sstanisi@cbnco.com
CC: Fabio Fantoni <fabio.fantoni@m2r.biz>
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Anthony Perard <anthony.perard@citrix.com>
CC: Simon (Bo) Cao <caobosimon@gmail.com>
---
tools/libxl/Makefile | 2 +-
tools/libxl/libxl.c | 3 +-
tools/libxl/libxl.h | 40 +++
tools/libxl/libxl_create.c | 3 +
tools/libxl/libxl_internal.h | 11 +
tools/libxl/libxl_qmp.c | 56 +++++
tools/libxl/libxl_types.idl | 22 ++
tools/libxl/libxl_usb.c | 521 ++++++++++++++++++++++++++++++++++++++++
tools/ocaml/libs/xl/genwrap.py | 1 +
9 files changed, 656 insertions(+), 3 deletions(-)
create mode 100644 tools/libxl/libxl_usb.c
diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index 4cfa275..a50e6cf 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -76,7 +76,7 @@ LIBXL_OBJS = flexarray.o libxl.o libxl_create.o libxl_dm.o libxl_pci.o \
libxl_internal.o libxl_utils.o libxl_uuid.o \
libxl_json.o libxl_aoutils.o libxl_numa.o \
libxl_save_callout.o _libxl_save_msgs_callout.o \
- libxl_qmp.o libxl_event.o libxl_fork.o $(LIBXL_OBJS-y)
+ libxl_qmp.o libxl_event.o libxl_fork.o libxl_usb.o $(LIBXL_OBJS-y)
LIBXL_OBJS += _libxl_types.o libxl_flask.o _libxl_types_internal.o
LIBXL_TESTS += timedereg
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 4ea7abb..7ef5e25 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1767,8 +1767,7 @@ static int libxl__device_nextid(libxl__gc *gc, uint32_t domid, char *device)
return nextid;
}
-static int libxl__resolve_domid(libxl__gc *gc, const char *name,
- uint32_t *domid)
+int libxl__resolve_domid(libxl__gc *gc, const char *name, uint32_t *domid)
{
if (!name)
return 0;
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index c7aa817..963e650 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -82,6 +82,12 @@
#define LIBXL_HAVE_DOMAIN_NODEAFFINITY 1
/*
+ * LIBXL_HAVE_DEVICE_USB indicates the functions for doing hot-plug of
+ * USB devices.
+ */
+#define LIBXL_HAVE_DEVICE_USB 1
+
+/*
* LIBXL_HAVE_BUILDINFO_HVM_VENDOR_DEVICE indicates that the
* libxl_vendor_device field is present in the hvm sections of
* libxl_domain_build_info. This field tells libxl which
@@ -924,6 +930,40 @@ 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
+ *
+ * For each device removed or added, one of these protocols is available:
+ * - PV (i.e., PVUSB)
+ * - DEVICEMODEL (i.e, qemu)
+ *
+ * PV is available for either PV or HVM domains. DEVICEMODEL is only
+ * available for HVM domains. The caller can additionally specify
+ * "AUTO", in which case the library will try to determine the best
+ * protocol automatically.
+ *
+ * At the moment, the only protocol implemented is DEVICEMODEL, and the only
+ * device type implemented is HOSTDEV.
+ *
+ * This uses the qmp functionality, and is thus only available for
+ * qemu-xen, not qemu-traditional.
+ */
+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;
+int libxl_device_usb_destroy(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)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index d015cf4..80a75a4 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -537,6 +537,9 @@ retry_transaction:
xs_rm(ctx->xsh, t, libxl_path);
libxl__xs_mkdir(gc, t, libxl_path, noperm, ARRAY_SIZE(noperm));
+ rc = libxl__domain_usb_init(gc, t, libxl_path, noperm, ARRAY_SIZE(noperm));
+ if (rc) goto out;
+
xs_write(ctx->xsh, t, libxl__sprintf(gc, "%s/vm", dom_path), vm_path, strlen(vm_path));
rc = libxl__domain_rename(gc, *domid, 0, info->name, t);
if (rc)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index c2b73c4..bf394e4 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1031,6 +1031,11 @@ _hidden int libxl__wait_for_backend(libxl__gc *gc, const char *be_path,
const char *state);
_hidden int libxl__nic_type(libxl__gc *gc, libxl__device *dev,
libxl_nic_type *nictype);
+_hidden int libxl__resolve_domid(libxl__gc *gc, const char *name,
+ uint32_t *domid);
+_hidden int libxl__domain_usb_init(libxl__gc *gc, xs_transaction_t t,
+ char *libxl_path, struct xs_permissions *perm,
+ int perm_size);
/*
* For each aggregate type which can be used as an input we provide:
@@ -1056,6 +1061,8 @@ _hidden int libxl__device_vtpm_setdefault(libxl__gc *gc, libxl_device_vtpm *vtpm
_hidden int libxl__device_vfb_setdefault(libxl__gc *gc, libxl_device_vfb *vfb);
_hidden int libxl__device_vkb_setdefault(libxl__gc *gc, libxl_device_vkb *vkb);
_hidden int libxl__device_pci_setdefault(libxl__gc *gc, libxl_device_pci *pci);
+_hidden int libxl__device_usb_setdefault(libxl__gc *gc, libxl_device_usb *usb,
+ uint32_t domid);
_hidden const char *libxl__device_nic_devname(libxl__gc *gc,
uint32_t domid,
@@ -1585,6 +1592,10 @@ _hidden int libxl__qmp_set_global_dirty_log(libxl__gc *gc, int domid, bool enabl
_hidden int libxl__qmp_insert_cdrom(libxl__gc *gc, int domid, const libxl_device_disk *disk);
/* Add a virtual CPU */
_hidden int libxl__qmp_cpu_add(libxl__gc *gc, int domid, int index);
+_hidden int libxl__qmp_usb_add(libxl__gc *gc, int domid,
+ libxl_device_usb *dev);
+_hidden int libxl__qmp_usb_remove(libxl__gc *gc, int domid,
+ libxl_device_usb *dev);
/* close and free the QMP handler */
_hidden void libxl__qmp_close(libxl__qmp_handler *qmp);
/* remove the socket file, if the file has already been removed,
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index 8433e42..56ae368 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -42,6 +42,7 @@
#define QMP_RECEIVE_BUFFER_SIZE 4096
#define PCI_PT_QDEV_ID "pci-pt-%02x_%02x.%01x"
+#define HOST_USB_QDEV_ID "usb-hostdev-%04x.%04x"
typedef int (*qmp_callback_t)(libxl__qmp_handler *qmp,
const libxl__json_object *tree,
@@ -939,6 +940,61 @@ int libxl__qmp_cpu_add(libxl__gc *gc, int domid, int idx)
return qmp_run_command(gc, domid, "cpu-add", args, NULL, NULL);
}
+static int libxl__qmp_usb_hostdev_add(libxl__gc *gc, int domid,
+ libxl_device_usb *usb)
+{
+ libxl__json_object *args = NULL;
+
+ qmp_parameters_add_string(gc, &args, "driver", "usb-host");
+ QMP_PARAMETERS_SPRINTF(&args, "hostbus", "0x%x", usb->u.hostdev.hostbus);
+ QMP_PARAMETERS_SPRINTF(&args, "hostaddr", "0x%x", usb->u.hostdev.hostaddr);
+ QMP_PARAMETERS_SPRINTF(&args, "id", HOST_USB_QDEV_ID,
+ usb->u.hostdev.hostbus, usb->u.hostdev.hostaddr);
+
+ return qmp_run_command(gc, domid, "device_add", args, NULL, NULL);
+}
+
+int libxl__qmp_usb_add(libxl__gc *gc, int domid, libxl_device_usb *usb)
+{
+ int rc;
+ switch (usb->type) {
+ case LIBXL_DEVICE_USB_TYPE_HOSTDEV:
+ rc = libxl__qmp_usb_hostdev_add(gc, domid, usb);
+ break;
+ default:
+ return ERROR_INVAL;
+ }
+ return rc;
+}
+
+
+static int libxl__qmp_usb_hostdev_remove(libxl__gc *gc, int domid,
+ libxl_device_usb *usb)
+{
+ libxl__json_object *args = NULL;
+
+ QMP_PARAMETERS_SPRINTF(&args, "id", HOST_USB_QDEV_ID,
+ usb->u.hostdev.hostbus, usb->u.hostdev.hostaddr);
+
+ return qmp_run_command(gc, domid, "device_del", args, NULL, NULL);
+}
+
+int libxl__qmp_usb_remove(libxl__gc *gc, int domid,
+ libxl_device_usb *usb)
+{
+ int rc;
+ switch (usb->type) {
+ case LIBXL_DEVICE_USB_TYPE_HOSTDEV:
+ rc = libxl__qmp_usb_hostdev_remove(gc, domid, usb);
+ break;
+ default:
+ return ERROR_INVAL;
+ }
+ return rc;
+}
+
+
+
int libxl__qmp_initializations(libxl__gc *gc, uint32_t domid,
const libxl_domain_config *guest_config)
{
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 52f1aa9..ae94ad6 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -457,6 +457,28 @@ libxl_device_vtpm = Struct("device_vtpm", [
("uuid", libxl_uuid),
])
+libxl_device_usb_protocol = Enumeration("usb_protocol", [
+ (0, "AUTO"),
+ (1, "PV"),
+ (2, "DEVICEMODEL"),
+ ])
+
+libxl_device_usb_type = Enumeration("device_usb_type", [
+ (1, "HOSTDEV"),
+ ])
+
+libxl_device_usb = Struct("device_usb", [
+ ("protocol", libxl_device_usb_protocol,
+ {'init_val': 'LIBXL_USB_PROTOCOL_AUTO'}),
+ ("backend_domid", libxl_domid),
+ ("backend_domname", string),
+ ("u", KeyedUnion(None, libxl_device_usb_type, "type",
+ [("hostdev", Struct(None, [
+ ("hostbus", integer),
+ ("hostaddr", integer) ]))
+ ]))
+ ])
+
libxl_domain_config = Struct("domain_config", [
("c_info", libxl_domain_create_info),
("b_info", libxl_domain_build_info),
diff --git a/tools/libxl/libxl_usb.c b/tools/libxl/libxl_usb.c
new file mode 100644
index 0000000..03b6ea1
--- /dev/null
+++ b/tools/libxl/libxl_usb.c
@@ -0,0 +1,521 @@
+/*
+ * Copyright (C) 2009 Citrix Ltd.
+ * Author George Dunlap <george.dunlap@eu.citrix.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; version 2.1 only. with the special
+ * exception on linking described in file LICENSE.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Lesser General Public License for more details.
+ */
+
+#include "libxl_osdeps.h" /* must come before any other headers */
+
+#include "libxl_internal.h"
+
+/*
+ * /libxl/<domid>/usb/<devid>/{type, protocol, backend, [typeinfo]}
+ */
+#define USB_INFO_PATH "%s/usb"
+#define USB_HOSTDEV_ID "hostdev-%04x-%04x"
+
+/*
+ * Just use plain numbers for now. Replace these with strings at some point.
+ */
+static char * hostdev_xs_path(libxl__gc *gc, uint32_t domid,
+ libxl_device_usb *usb)
+{
+ /* FIXME: Assert usb->type here? */
+ return GCSPRINTF(USB_INFO_PATH "/%s",
+ libxl__xs_libxl_path(gc, domid),
+ GCSPRINTF(USB_HOSTDEV_ID,
+ usb->u.hostdev.hostbus,
+ usb->u.hostdev.hostaddr));
+}
+
+static void hostdev_xs_append_params(libxl__gc *gc, libxl_device_usb *usb,
+ flexarray_t *a)
+{
+ /* FIXME: Assert usb->type here? */
+ flexarray_append_pair(a, "hostbus",
+ GCSPRINTF("%d",
+ usb->u.hostdev.hostbus));
+ flexarray_append_pair(a, "hostaddr",
+ GCSPRINTF("%d",
+ usb->u.hostdev.hostaddr));
+}
+
+static int read_hostdev_xenstore_entry(libxl__gc *gc, const char * path,
+ libxl_device_usb *usb)
+{
+ int rc;
+ const char * val;
+
+ rc = libxl__xs_read_checked(gc, XBT_NULL,
+ GCSPRINTF("%s/hostbus", path), &val);
+ if (rc) goto out;
+ usb->u.hostdev.hostbus = atoi(val);
+
+ rc = libxl__xs_read_checked(gc, XBT_NULL,
+ GCSPRINTF("%s/hostaddr", path), &val);
+ if (rc) goto out;
+ usb->u.hostdev.hostaddr = atoi(val);
+
+ rc = 0;
+out:
+ return rc;
+}
+
+static int usb_read_xenstore(libxl__gc *gc, const char * path,
+ libxl_device_usb *usb)
+{
+ const char *val;
+ int rc;
+
+ rc = libxl__xs_read_checked(gc, XBT_NULL,
+ GCSPRINTF("%s/protocol", path), &val);
+ if (rc) goto out;
+ rc = libxl_usb_protocol_from_string(val, &usb->protocol);
+ if (rc) goto out;
+
+ rc = libxl__xs_read_checked(gc, XBT_NULL,
+ GCSPRINTF("%s/backend_domid", path), &val);
+ if (rc) goto out;
+ usb->backend_domid = atoi(val);
+
+ rc = libxl__xs_read_checked(gc, XBT_NULL,
+ GCSPRINTF("%s/type", path), &val);
+ if (rc) goto out;
+ rc = libxl_device_usb_type_from_string(val, &usb->type);
+ if (rc) goto out;
+
+ switch (usb->type) {
+ case LIBXL_DEVICE_USB_TYPE_HOSTDEV:
+ rc = read_hostdev_xenstore_entry(gc, path, usb);
+ if (rc) goto out;
+ break;
+ default:
+ LOG(ERROR, "Internal error (unimplemented type)");
+ goto out;
+ }
+
+ rc = 0;
+out:
+ return rc;
+}
+
+static int usb_add_xenstore(libxl__gc *gc, uint32_t domid,
+ libxl_device_usb *usb)
+{
+ flexarray_t *dev;
+ char *dev_path;
+ xs_transaction_t t = 0;
+ struct xs_permissions noperm[1];
+ int rc = 0;
+
+ noperm[0].id = 0;
+ noperm[0].perms = XS_PERM_NONE;
+
+ dev = flexarray_make(gc, 16, 1);
+
+ flexarray_append_pair(dev, "protocol",
+ GCSPRINTF("%s",
+ libxl_usb_protocol_to_string(usb->protocol)));
+
+ flexarray_append_pair(dev, "backend_domid",
+ GCSPRINTF("%d", usb->backend_domid));
+
+ flexarray_append_pair(dev, "type",
+ GCSPRINTF("%s",
+ libxl_device_usb_type_to_string(usb->type)));
+
+ switch (usb->type) {
+ case LIBXL_DEVICE_USB_TYPE_HOSTDEV:
+ dev_path = hostdev_xs_path(gc, domid, usb);
+ hostdev_xs_append_params(gc, usb, dev);
+ break;
+ default:
+ LOG(ERROR, "Invalid device type: %d", usb->type);
+ return ERROR_FAIL;
+ }
+
+ LOG(DEBUG, "Adding new usb device to xenstore");
+
+ for(;;) {
+ rc = libxl__xs_transaction_start(gc, &t);
+ if (rc) goto out;
+
+ /* Helpfully, mkdir returns 0 on failure, 1 on success */
+ if (!libxl__xs_mkdir(gc, t, dev_path, noperm, ARRAY_SIZE(noperm))) {
+ rc = ERROR_FAIL;
+ goto out;
+ }
+
+ /* And libxl__xs_writev *always* returns 0 no matter what */
+ libxl__xs_writev(gc, t, dev_path,
+ libxl__xs_kvs_of_flexarray(gc, dev, dev->count));
+
+ rc = libxl__xs_transaction_commit(gc, &t);
+ if (!rc) break;
+ if (rc <0) goto out;
+ }
+
+out:
+ libxl__xs_transaction_abort(gc, &t);
+ return rc;
+}
+
+static int usb_remove_xenstore(libxl__gc *gc, uint32_t domid,
+ libxl_device_usb *usb)
+{
+ libxl_ctx *ctx = libxl__gc_owner(gc);
+ char *dev_path;
+
+ LOG(DEBUG, "Removing device from xenstore");
+
+ switch (usb->type) {
+ case LIBXL_DEVICE_USB_TYPE_HOSTDEV:
+ dev_path = hostdev_xs_path(gc, domid, usb);
+ break;
+ default:
+ LOG(ERROR, "Invalid device type: %d", usb->type);
+ return ERROR_FAIL;
+ }
+
+ xs_rm(ctx->xsh, XBT_NULL, dev_path);
+
+ return 0;
+}
+
+int libxl__domain_usb_init(libxl__gc *gc, xs_transaction_t t, char *libxl_path,
+ struct xs_permissions *perm, int perm_size) {
+ int rc;
+ char * libxl_usb_path = GCSPRINTF(USB_INFO_PATH, libxl_path);
+
+ /* Helpfully, libxl__xs_rm_checked() returns 0 on success... */
+ rc = libxl__xs_rm_checked(gc, t, libxl_usb_path);
+ if (rc) goto out;
+
+ /* ...but libxl__xs_mkdir() returns 1 on success, 0 on failure. */
+ if (!libxl__xs_mkdir(gc, t, libxl_usb_path, perm, perm_size)) {
+ rc = ERROR_FAIL;
+ goto out;
+ }
+
+out:
+ return rc;
+}
+
+static int _get_assigned_devices(libxl__gc *gc, libxl__gc *list_gc, uint32_t domid,
+ libxl_device_usb **list, int *num)
+{
+ char **devlist, *dompath;
+ unsigned int nd = 0;
+ int i, rc = 0;
+
+ *list = NULL;
+ *num = 0;
+
+ dompath = GCSPRINTF(USB_INFO_PATH,
+ libxl__xs_libxl_path(gc, domid));
+
+ devlist = libxl__xs_directory(gc, XBT_NULL, dompath, &nd);
+ if (!devlist || !nd)
+ goto out;
+
+ *list = libxl__calloc(list_gc, nd, sizeof(libxl_device_usb));
+
+ for(i = 0; i < nd; i++) {
+ char *path;
+
+ path = GCSPRINTF("%s/%s", dompath, devlist[i]);
+
+ rc = usb_read_xenstore(gc, path, (*list) + i);
+ if (rc) {
+ free(*list);
+ *list = NULL;
+ *num = 0;
+ goto out;
+ }
+ }
+
+ *num = nd;
+
+out:
+ return rc;
+}
+
+/* GC or NOGC indicates whether the list of devices is garbage collected. */
+#define GET_ASSIGNED_DEVICES_GC(gc, domid, list, num) \
+ _get_assigned_devices(gc, gc, domid, list, num)
+#define GET_ASSIGNED_DEVICES_NOGC(gc, domid, list, num)\
+ _get_assigned_devices(gc, NOGC, domid, list, num)
+
+static int is_usbdev_type_hostdev_equal(libxl_device_usb *a,
+ libxl_device_usb *b)
+{
+ return a->u.hostdev.hostbus == b->u.hostdev.hostbus
+ && a->u.hostdev.hostaddr == b->u.hostdev.hostaddr;
+}
+
+static int is_usbdev_in_array(libxl_device_usb *assigned, int num_assigned,
+ libxl_device_usb *dev)
+{
+ int i;
+
+ /* Devices are the same if:
+ * - They have the same backend_domid
+ * - They are of the same type
+ * - Their types match
+ * In theory, someone could try to pass the same device through
+ * via both PVUSB and qemu; not comparing protocol will prevent that.
+ */
+ for(i = 0; i < num_assigned; i++) {
+ if (assigned[i].type != dev->type
+ || assigned[i].backend_domid != dev->backend_domid)
+ continue;
+
+ switch (dev->type) {
+ case LIBXL_DEVICE_USB_TYPE_HOSTDEV:
+ if (!is_usbdev_type_hostdev_equal(dev, assigned+i))
+ continue;
+ }
+
+ return 1;
+ }
+
+ return 0;
+}
+
+/*
+ * USB add
+ */
+static int do_usb_add(libxl__gc *gc, uint32_t domid,
+ libxl_device_usb *usb)
+{
+ int rc;
+
+ switch (usb->protocol) {
+ case LIBXL_USB_PROTOCOL_DEVICEMODEL:
+ switch (libxl__device_model_version_running(gc, domid)) {
+ case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
+ LOG(ERROR, "usb-add not yet implemented for qemu-traditional");
+ return ERROR_INVAL;
+ case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
+ rc = libxl__qmp_usb_add(gc, domid, usb);
+ if (rc)
+ goto out;
+ break;
+ default:
+ return ERROR_INVAL;
+ }
+ break;
+ default:
+ return ERROR_FAIL;
+ }
+
+ rc = usb_add_xenstore(gc, domid, usb);
+out:
+ return rc;
+}
+
+
+int libxl__device_usb_setdefault(libxl__gc *gc, libxl_device_usb *usb, uint32_t domid)
+{
+ int rc;
+ libxl_domain_type domtype = libxl__domain_type(gc, domid);
+
+ rc = libxl__resolve_domid(gc, usb->backend_domname, &usb->backend_domid);
+ if (rc < 0) return rc;
+
+ if (usb->protocol == LIBXL_USB_PROTOCOL_AUTO) {
+ if (domtype == LIBXL_DOMAIN_TYPE_PV) {
+ usb->protocol = LIBXL_USB_PROTOCOL_PV;
+ } else if (domtype == LIBXL_DOMAIN_TYPE_HVM) {
+ /* FIXME: See if we can detect PV frontend */
+ usb->protocol = LIBXL_USB_PROTOCOL_DEVICEMODEL;
+ }
+ }
+
+ return rc;
+}
+
+static int libxl__device_usb_add(libxl__gc *gc, uint32_t domid,
+ libxl_device_usb *usb)
+{
+ libxl_ctx *ctx = libxl__gc_owner(gc);
+ libxl_device_usb *assigned;
+ int rc = ERROR_FAIL, num_assigned;
+ libxl_domid dm_domid = libxl_get_stubdom_id(ctx, domid);
+
+ rc = libxl__device_usb_setdefault(gc, usb, domid);
+ if (rc < 0) goto out;
+
+ /* Check to make sure we're doing something that's implemented */
+ if (usb->protocol != LIBXL_USB_PROTOCOL_DEVICEMODEL) {
+ rc = ERROR_FAIL;
+ LOG(ERROR, "Protocol not implemented");
+ goto out;
+ }
+
+ if (dm_domid != 0) {
+ rc = ERROR_FAIL;
+ LOG(ERROR, "Stubdoms not yet supported");
+ goto out;
+ }
+
+ /* Double-check to make sure it's not already assigned */
+ rc = GET_ASSIGNED_DEVICES_GC(gc, domid, &assigned, &num_assigned);
+ if (rc) {
+ LOG(ERROR, "cannot determine if device is assigned,"
+ " refusing to continue");
+ goto out;
+ }
+ if (is_usbdev_in_array(assigned, num_assigned, usb)) {
+ LOG(ERROR, "USB device already attached to a domain");
+ rc = ERROR_FAIL;
+ goto out;
+ }
+
+ /* Do the add */
+ if (do_usb_add(gc, domid, usb))
+ rc = ERROR_FAIL;
+
+out:
+ return rc;
+}
+
+int libxl_device_usb_add(libxl_ctx *ctx, uint32_t domid,
+ libxl_device_usb *usb,
+ const libxl_asyncop_how *ao_how)
+{
+ AO_CREATE(ctx, domid, ao_how);
+ int rc;
+ rc = libxl__device_usb_add(gc, domid, usb);
+ libxl__ao_complete(egc, ao, rc);
+ return AO_INPROGRESS;
+}
+
+/*
+ * USB remove
+ */
+static int do_usb_remove(libxl__gc *gc, uint32_t domid,
+ libxl_device_usb *usb)
+{
+ int rc;
+
+ switch (usb->protocol) {
+ case LIBXL_USB_PROTOCOL_DEVICEMODEL:
+ switch (libxl__device_model_version_running(gc, domid)) {
+ case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
+ LOG(ERROR, "usb-remove not yet implemented for qemu-traditional");
+ return ERROR_INVAL;
+ case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
+ rc = libxl__qmp_usb_remove(gc, domid, usb);
+ if (rc)
+ goto out;
+ break;
+ default:
+ return ERROR_INVAL;
+ }
+ break;
+ default:
+ return ERROR_FAIL;
+ }
+
+ rc = usb_remove_xenstore(gc, domid, usb);
+out:
+ return rc;
+}
+static int libxl__device_usb_remove_common(libxl__gc *gc, uint32_t domid,
+ libxl_device_usb *usb)
+{
+ libxl_ctx *ctx = libxl__gc_owner(gc);
+ libxl_device_usb *assigned;
+ int rc = ERROR_FAIL, num_assigned;
+ libxl_domid dm_domid = libxl_get_stubdom_id(ctx, domid);
+
+ rc = libxl__device_usb_setdefault(gc, usb, domid);
+ if (rc < 0) goto out;
+
+ /* Check to make sure we're doing something that's impemented */
+ if (usb->protocol != LIBXL_USB_PROTOCOL_DEVICEMODEL) {
+ rc = ERROR_FAIL;
+ LOG(ERROR, "Protocol not implemented");
+ goto out;
+ }
+
+ if (dm_domid != 0) {
+ rc = ERROR_FAIL;
+ LOG(ERROR, "Stubdoms not yet supported");
+ goto out;
+ }
+
+ /* Double-check to make sure it's not already assigned */
+ rc = GET_ASSIGNED_DEVICES_GC(gc, domid, &assigned, &num_assigned);
+ if (rc) {
+ LOG(ERROR, "cannot determine if device is assigned,"
+ " refusing to continue");
+ goto out;
+ }
+ if (!is_usbdev_in_array(assigned, num_assigned, usb)) {
+ LOG(ERROR, "USB device doesn't seem to be attached to the domain");
+ rc = ERROR_FAIL;
+ goto out;
+ }
+
+ /* Do the remove */
+ if (do_usb_remove(gc, domid, usb))
+ rc = ERROR_FAIL;
+
+out:
+ return rc;
+}
+
+int libxl_device_usb_remove(libxl_ctx *ctx, uint32_t domid,
+ libxl_device_usb *usb,
+ const libxl_asyncop_how *ao_how)
+{
+ AO_CREATE(ctx, domid, ao_how);
+ int rc;
+
+ rc = libxl__device_usb_remove_common(gc, domid, usb);
+
+ libxl__ao_complete(egc, ao, rc);
+ return AO_INPROGRESS;
+}
+
+int libxl_device_usb_destroy(libxl_ctx *ctx, uint32_t domid,
+ libxl_device_usb *usb,
+ const libxl_asyncop_how *ao_how)
+{
+ AO_CREATE(ctx, domid, ao_how);
+ int rc;
+
+ rc = libxl__device_usb_remove_common(gc, domid, usb);
+
+ libxl__ao_complete(egc, ao, rc);
+ return AO_INPROGRESS;
+}
+
+libxl_device_usb *libxl_device_usb_list(libxl_ctx *ctx,
+ uint32_t domid, int *num)
+{
+ GC_INIT(ctx);
+ libxl_device_usb *usb = NULL;
+ int rc;
+
+ rc = GET_ASSIGNED_DEVICES_NOGC(gc, domid, &usb, num);
+ if (rc) {
+ LOG(ERROR, "Could not get assigned devices");
+ goto out;
+ }
+
+out:
+ GC_FREE;
+ return usb;
+}
diff --git a/tools/ocaml/libs/xl/genwrap.py b/tools/ocaml/libs/xl/genwrap.py
index 5e43831..09f7370 100644
--- a/tools/ocaml/libs/xl/genwrap.py
+++ b/tools/ocaml/libs/xl/genwrap.py
@@ -501,6 +501,7 @@ if __name__ == '__main__':
blacklist = [
"cpupoolinfo",
"vcpuinfo",
+ "device_usb"
]
for t in blacklist:
--
1.7.9.5
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v7 RFC 2/2] xl: Add commands for usb hot-plug
[not found] <1401716658-22393-1-git-send-email-george.dunlap@eu.citrix.com>
2014-06-02 13:44 ` [PATCH v7 RFC 1/2] libxl: Introduce functions to add and remove USB devices to an HVM guest George Dunlap
@ 2014-06-02 13:44 ` George Dunlap
2014-06-05 10:14 ` [PATCH v7 RFC 0/2] libxl USB prototype and design discussion Daniel P. Berrange
` (3 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: George Dunlap @ 2014-06-02 13:44 UTC (permalink / raw)
To: xen-devel
Cc: Ian Campbell, George Dunlap, sstanisi, Fabio Fantoni,
Anthony Perard, Ian Jackson, Simon (Bo) Cao, Roger Pau Monne
Add xl commands for USB hot-plug. Domain creation and config file
support not yet added.
Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
---
v7:
- Don't check for invalid return values for find_domain()
- Use {foo}_to_string in usb_list()
- call dispose on list elements to avoid leaks
v6:
- Rename to usb-attach and usb-detach to fit better w/ other device
add/remove commands
- Use positional arguments, to fit better w/ other device add/remove
commands
- Change specifier to "hostdev:xx.yy", looking forward to a day when
we may be able to accept something like "tablet"
- Style fix-ups
v5:
- Remove extraneous is_hex
- Replace is_dec with ctype.h isdigit()
- Replace domid -1 with INVALID_DOMID
- Fix up a couple of mistaken function names in strings
- Copy CTYPE macro from libxl_internal.h
- Fix to usb-list manpage (use -d domid)
CC: Ian Jackson <ian.jackson@citrix.com>
CC: Roger Pau Monne <roger.pau@citrix.com>
CC: sstanisi@cbnco.com
CC: Fabio Fantoni <fabio.fantoni@m2r.biz>
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Anthony Perard <anthony.perard@citrix.com>
CC: Simon (Bo) Cao <caobosimon@gmail.com>
---
docs/man/xl.pod.1 | 30 ++++++++
tools/libxl/xl.h | 21 ++++++
tools/libxl/xl_cmdimpl.c | 182 +++++++++++++++++++++++++++++++++++++++++++++
tools/libxl/xl_cmdtable.c | 15 ++++
4 files changed, 248 insertions(+)
diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index 30bd4bf..e04a4a8 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -1194,6 +1194,36 @@ List virtual network interfaces for a domain.
=back
+=head2 USB DEVICES
+
+=over 4
+
+=item B<usb-attach> I<domain-id> I<hostdev:hostbus.hostaddr>
+
+Passes through the host USB device specified by I<hostbus.hostaddr>. At
+the moment this will only work for HVM domains via qemu.
+
+The best way to find out the information for the device is typically using
+lsusb.
+
+This command is only available for domains using qemu-xen, not
+qemu-traditional.
+
+=item B<usb-detach> I<domain-id> I<hostdev:hosbus.hostaddr>
+
+Remove the host USB device from I<domain-id> which is specified
+by <hostbus.hostaddr>. This command only works for devices added
+with usb-add; not for those specified in the config file.
+
+This command is only available for domains using qemu-xen, not
+qemu-traditional.
+
+=item B<usb-list> I<domain-id>
+
+Show host USB devices assigned to the guest.
+
+=back
+
=head2 VTPM DEVICES
=over 4
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index 10a2e66..2f1f612 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -35,6 +35,9 @@ int main_info(int argc, char **argv);
int main_sharing(int argc, char **argv);
int main_cd_eject(int argc, char **argv);
int main_cd_insert(int argc, char **argv);
+int main_usb_attach(int argc, char **argv);
+int main_usb_detach(int argc, char **argv);
+int main_usb_list(int argc, char **argv);
int main_console(int argc, char **argv);
int main_vncviewer(int argc, char **argv);
int main_pcilist(int argc, char **argv);
@@ -183,6 +186,24 @@ extern void printf_info_sexp(int domid, libxl_domain_config *d_config);
#define XL_GLOBAL_CONFIG XEN_CONFIG_DIR "/xl.conf"
#define XL_LOCK_FILE XEN_LOCK_DIR "/xl"
+/*
+ * int CTYPE(ISFOO, char c);
+ * int CTYPE(toupper, char c);
+ * int CTYPE(tolower, char c);
+ *
+ * This is necessary because passing a simple char to a ctype.h
+ * is forbidden. ctype.h macros take ints derived from _unsigned_ chars.
+ *
+ * If you have a char which might be EOF then you should already have
+ * it in an int representing an unsigned char, and you can use the
+ * <ctype.h> macros directly. This generally happens only with values
+ * from fgetc et al.
+ *
+ * For any value known to be a character (eg, anything that came from
+ * a char[]), use CTYPE.
+ */
+#define CTYPE(isfoo,c) (isfoo((unsigned char)(c)))
+
#endif /* XL_H */
/*
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 5195914..7973b4c 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -2771,6 +2771,188 @@ int main_cd_insert(int argc, char **argv)
return 0;
}
+static int parse_usb_specifier(libxl_device_usb *dev, char *s)
+{
+ char *devtype, * hostbus, *hostaddr, *p;
+
+ /*
+ * General format:
+ * <type>[:<type-specific options>]
+ */
+ p = s;
+ devtype = strsep(&p, ":");
+
+ if (strcmp(devtype, "hostdev")) {
+ fprintf(stderr, "Unknown device type: %s\n", devtype);
+ return -1;
+ }
+
+ hostbus = strsep(&p, ".");
+
+ hostaddr = p;
+
+ if (!hostbus) {
+ fprintf(stderr, "Device type %s requires hostaddr\n",
+ devtype);
+ return -1;
+ }
+
+ if (!hostaddr) {
+ fprintf(stderr, "Device type %s requires hostaddr\n",
+ devtype);
+ return -1;
+ }
+
+ /* Make sure they look right */
+ for (p=hostbus; *p; p++) {
+ if (!CTYPE(isdigit,*p)) {
+ fprintf(stderr, "Bad hostbus: %s\n", hostbus);
+ return -1;
+ }
+ }
+
+ for (p=hostaddr; *p; p++) {
+ if (!CTYPE(isdigit,*p)) {
+ fprintf(stderr, "Bad hostaddr: %s\n", hostaddr);
+ return -1;
+ }
+ }
+
+ dev->type = LIBXL_DEVICE_USB_TYPE_HOSTDEV;
+
+ dev->u.hostdev.hostbus = atoi(hostbus);
+ dev->u.hostdev.hostaddr = atoi(hostaddr);
+
+ return 0;
+}
+
+static int usb_attach(uint32_t domid, char * device)
+{
+ libxl_device_usb usbdev;
+ int rc;
+
+ libxl_device_usb_init(&usbdev);
+
+ if (parse_usb_specifier(&usbdev, device)<0) {
+ rc = ERROR_FAIL;
+ goto out;
+ }
+
+ rc = libxl_device_usb_add(ctx, domid, &usbdev, NULL);
+ if (rc<0)
+ fprintf(stderr, "libxl_device_usb_add failed.\n");
+
+ libxl_device_usb_dispose(&usbdev);
+
+out:
+ return rc;
+}
+
+int main_usb_attach(int argc, char **argv)
+{
+ uint32_t domid = INVALID_DOMID;
+ int opt = 0, rc;
+ char *device = NULL;
+
+ SWITCH_FOREACH_OPT(opt, "", NULL, "usb-attach", 2) {
+ /* No options */
+ }
+
+ domid = find_domain(argv[optind]);
+ device = argv[optind + 1];
+
+ rc = usb_attach(domid, device);
+ if (rc<0)
+ return 1;
+ else
+ return 0;
+}
+
+static int usb_detach(uint32_t domid, libxl_device_usb_type type,
+ char * device)
+{
+ libxl_device_usb usbdev;
+ int rc;
+
+ libxl_device_usb_init(&usbdev);
+
+ if (parse_usb_specifier(&usbdev, device)<0) {
+ rc = ERROR_FAIL;
+ goto out;
+ }
+
+ rc = libxl_device_usb_remove(ctx, domid, &usbdev, NULL);
+ if (rc<0)
+ fprintf(stderr, "libxl_device_usb_remove failed.\n");
+
+ libxl_device_usb_dispose(&usbdev);
+
+out:
+ return rc;
+}
+
+int main_usb_detach(int argc, char **argv)
+{
+ uint32_t domid = INVALID_DOMID;
+ int opt = 0, rc;
+ char *device = NULL;
+ int type = 0;
+
+ SWITCH_FOREACH_OPT(opt, "", NULL, "usb-detach", 2) {
+ /* No options */
+ }
+
+ domid = find_domain(argv[optind]);
+ device = argv[optind + 1];
+
+ rc = usb_detach(domid, type, device);
+ if (rc < 0)
+ return 1;
+ else
+ return 0;
+}
+
+static void usb_list(uint32_t domid)
+{
+ libxl_device_usb *dev;
+ int num, i;
+
+ dev = libxl_device_usb_list(ctx, domid, &num);
+ if (dev == NULL)
+ return;
+ printf("protocol backend type device\n");
+ for (i = 0; i < num; i++) {
+ printf("%10s ",
+ libxl_usb_protocol_to_string(dev[i].protocol));
+ printf("%7d ", dev[i].backend_domid);
+ printf("%7s ", libxl_device_usb_type_to_string(dev[i].type));
+ if (dev[i].type == LIBXL_DEVICE_USB_TYPE_HOSTDEV)
+ printf("%03d.%03d",
+ dev[i].u.hostdev.hostbus,
+ dev[i].u.hostdev.hostaddr);
+ printf("\n");
+ libxl_device_usb_dispose(&dev[i]);
+ }
+ free(dev);
+}
+
+
+int main_usb_list(int argc, char **argv)
+{
+ uint32_t domid = INVALID_DOMID;
+ int opt;
+
+ SWITCH_FOREACH_OPT(opt, "", NULL, "usb-list", 1) {
+ /* No options */
+ }
+
+ domid = find_domain(argv[optind]);
+ usb_list(domid);
+ return 0;
+}
+
+
+
int main_console(int argc, char **argv)
{
uint32_t domid;
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index 4279b9f..c97796b 100644
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -194,6 +194,21 @@ struct cmd_spec cmd_table[] = {
"Eject a cdrom from a guest's cd drive",
"<Domain> <VirtualDevice>",
},
+ { "usb-attach",
+ &main_usb_attach, 1, 1,
+ "Hot-plug a usb device to a domain.",
+ "<Domain> <hostdev:<hostbus.hostaddr>>",
+ },
+ { "usb-detach",
+ &main_usb_detach, 1, 1,
+ "Hot-unplug a usb device from a domain.",
+ "<Domain> <hostdev:hostbus.hostaddr>",
+ },
+ { "usb-list",
+ &main_usb_list, 0, 0,
+ "List usb devices for a domain",
+ "<Domain>",
+ },
{ "mem-max",
&main_memmax, 0, 1,
"Set the maximum amount reservation for a domain",
--
1.7.9.5
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v7 RFC 0/2] libxl USB prototype and design discussion
[not found] <1401716658-22393-1-git-send-email-george.dunlap@eu.citrix.com>
2014-06-02 13:44 ` [PATCH v7 RFC 1/2] libxl: Introduce functions to add and remove USB devices to an HVM guest George Dunlap
2014-06-02 13:44 ` [PATCH v7 RFC 2/2] xl: Add commands for usb hot-plug George Dunlap
@ 2014-06-05 10:14 ` Daniel P. Berrange
[not found] ` <20140605101438.GD19077@redhat.com>
` (2 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Daniel P. Berrange @ 2014-06-05 10:14 UTC (permalink / raw)
To: George Dunlap
Cc: Ian Campbell, LibVirt Development List, Jim Fehlig, xen-devel,
sstanisi, Fabio Fantoni, Anthony Perard, Ian Jackson,
Simon (Bo) Cao, Roger Pau Monne
On Mon, Jun 02, 2014 at 02:44:16PM +0100, George Dunlap wrote:
> == Libvirt's interface ==
>
> I've had a brief look at libvirt's USB interface, and learned a bit
> about libvirt's general approach to things at the Xen Hackathon last
> week. One of the goals of libvirt is to be able to specify the
> virtual hardware in enough detail to keep it from changing when you
> upgrade the hypervisor, so that certain proprietary operating systems
> which are sensitive to this kind of thing continue to work.
>
> Instead of specifying a controller name, you specify a controller
> index (which is different than qemu). But instead of specifying a USB
> version number, you specify a model for the USB controller (which
> happens to be the exact name that qemu uses): "piix3-uhci",
> "piix4-uhci", "ehci", "ich9-ehci1", "ich9-uhci1", "ich9-uhci2",
> "ich9-uhci3", "vt82c686b-uhci", "pci-ohci" or "nec-xhci".
>
> When specifying a USB device, libvirt has the concept of an "address"
> where you will plug it into. Here is what the page says about it:
>
> "USB addresses have the following additional attributes: bus (a hex
> value between 0 and 0xfff, inclusive), and port (a dotted notation of
> up to four octets, such as 1.2 or 2.1.3.1)."
>
> It's not exactly clear to me what those numbers mean. But after
> chatting with Daniel Berrange at the Hackathon, it looks like the
> "bus" in the address corresponds to the "index" in the controller
> specification. It also appears that this "index" is internal to
> libvirt and is not exposed to the guest: so it should in theory be
> possible to have index 0 be a Xen PVUSB controller, 1 be an emulated
> qemu controller, 3 be an emulated PVUSB controller, and so on.
This structure/concept is something that libvirt uses more generally
than just USB. Any kind of device which has further devices attached
as a child is represented as a "controller" in libvirt. eg PCI bus,
PCI bridge, USB controller, USB hub, IDE controller, SCSI controller
etc, etc. Any device then has an <address/> element whose 'type'
attribute specifies what type of controller it connects to, and which
maps to the specific <controller> element via its index number.
The validate attributes for the <address> vary depending on the address
type / controller type.
You are correct that the index is completely internal to libvirt,
not exposed to QEMU or the guest. So in USB case the 'bus' attribute
on <address type='usb'/> maps to the <controller> index for the USB
controller.
WRT to choosing the type of controller, libvirt always aims to use
the name of a real hardware model if there is one, since this is
something that can be standardized across hypervisors if they're
emulating the same underlying hardware. The specific model name
we pick is first-come-first-served. So if choose 'piix3-uhci'
based on QEMU name and later we add VMWare support and it has
called it 'piix3-usb', we'd still use 'piix3-uhci' in libvirt
XML and re-write to the VMWare specific name internally. Or the
opposite if we standardized on the VMWare name first and later
wnated to add QEMU support.
In the case of totally invented paravirt devices, we'll come up
with a name that seems most appropriate for the hypervisor which
invented the device type.
> == Draft design proposal ==
>
> I've given it some thought, and based on the below is a suggested
> design for people to have a go at.
>
> Basic idea: Specify named controllers. It's controllers that are PV
> or emulated, and may have a backend domain. When adding devices,
> specify which controller to attach it to. Allow most things to have
> intelligent defaults.
>
> * libxl functions
>
> usb_controller_add
> usb_controller_remove/destroy
> usb_controller_list
> device struct:
> name # If empty, default to hciN/pvN, where N starts at 0
> type = {PV, EMULATED, AUTO}
> backend_{domname,domid} # PV only
> usbversion = {1, 2, 3}
> numports # default 16
So with this kind of syntax, libvirt is going to have to try to
map a model name 'piix3-uhci' to a pair of type,usbversion
eg type=emulated,usbversion=1. We'd likely give the paravirt
controller model names of xenusb1, xenusb2, xenusb3 for USB-1
USB-2 and USB-3 respectively, and again map these to type=pv,usbversion=N.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v7 RFC 0/2] libxl USB prototype and design discussion
[not found] ` <20140605101438.GD19077@redhat.com>
@ 2014-06-05 15:04 ` George Dunlap
0 siblings, 0 replies; 15+ messages in thread
From: George Dunlap @ 2014-06-05 15:04 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: Ian Campbell, LibVirt Development List, Jim Fehlig, xen-devel,
Stefan, Fabio Fantoni, Anthony Perard, Ian Jackson,
Simon (Bo) Cao, Roger Pau Monne
On Thu, Jun 5, 2014 at 11:14 AM, Daniel P. Berrange <berrange@redhat.com> wrote:
> On Mon, Jun 02, 2014 at 02:44:16PM +0100, George Dunlap wrote:
>> == Libvirt's interface ==
>>
>> I've had a brief look at libvirt's USB interface, and learned a bit
>> about libvirt's general approach to things at the Xen Hackathon last
>> week. One of the goals of libvirt is to be able to specify the
>> virtual hardware in enough detail to keep it from changing when you
>> upgrade the hypervisor, so that certain proprietary operating systems
>> which are sensitive to this kind of thing continue to work.
>>
>> Instead of specifying a controller name, you specify a controller
>> index (which is different than qemu). But instead of specifying a USB
>> version number, you specify a model for the USB controller (which
>> happens to be the exact name that qemu uses): "piix3-uhci",
>> "piix4-uhci", "ehci", "ich9-ehci1", "ich9-uhci1", "ich9-uhci2",
>> "ich9-uhci3", "vt82c686b-uhci", "pci-ohci" or "nec-xhci".
>>
>> When specifying a USB device, libvirt has the concept of an "address"
>> where you will plug it into. Here is what the page says about it:
>>
>> "USB addresses have the following additional attributes: bus (a hex
>> value between 0 and 0xfff, inclusive), and port (a dotted notation of
>> up to four octets, such as 1.2 or 2.1.3.1)."
>>
>> It's not exactly clear to me what those numbers mean. But after
>> chatting with Daniel Berrange at the Hackathon, it looks like the
>> "bus" in the address corresponds to the "index" in the controller
>> specification. It also appears that this "index" is internal to
>> libvirt and is not exposed to the guest: so it should in theory be
>> possible to have index 0 be a Xen PVUSB controller, 1 be an emulated
>> qemu controller, 3 be an emulated PVUSB controller, and so on.
>
>
> This structure/concept is something that libvirt uses more generally
> than just USB. Any kind of device which has further devices attached
> as a child is represented as a "controller" in libvirt. eg PCI bus,
> PCI bridge, USB controller, USB hub, IDE controller, SCSI controller
> etc, etc. Any device then has an <address/> element whose 'type'
> attribute specifies what type of controller it connects to, and which
> maps to the specific <controller> element via its index number.
> The validate attributes for the <address> vary depending on the address
> type / controller type.
>
> You are correct that the index is completely internal to libvirt,
> not exposed to QEMU or the guest. So in USB case the 'bus' attribute
> on <address type='usb'/> maps to the <controller> index for the USB
> controller.
>
> WRT to choosing the type of controller, libvirt always aims to use
> the name of a real hardware model if there is one, since this is
> something that can be standardized across hypervisors if they're
> emulating the same underlying hardware. The specific model name
> we pick is first-come-first-served. So if choose 'piix3-uhci'
> based on QEMU name and later we add VMWare support and it has
> called it 'piix3-usb', we'd still use 'piix3-uhci' in libvirt
> XML and re-write to the VMWare specific name internally. Or the
> opposite if we standardized on the VMWare name first and later
> wnated to add QEMU support.
That makes sense.
>
> In the case of totally invented paravirt devices, we'll come up
> with a name that seems most appropriate for the hypervisor which
> invented the device type.
>
>> == Draft design proposal ==
>>
>> I've given it some thought, and based on the below is a suggested
>> design for people to have a go at.
>>
>> Basic idea: Specify named controllers. It's controllers that are PV
>> or emulated, and may have a backend domain. When adding devices,
>> specify which controller to attach it to. Allow most things to have
>> intelligent defaults.
>>
>> * libxl functions
>>
>> usb_controller_add
>> usb_controller_remove/destroy
>> usb_controller_list
>> device struct:
>> name # If empty, default to hciN/pvN, where N starts at 0
>> type = {PV, EMULATED, AUTO}
>> backend_{domname,domid} # PV only
>> usbversion = {1, 2, 3}
>> numports # default 16
>
> So with this kind of syntax, libvirt is going to have to try to
> map a model name 'piix3-uhci' to a pair of type,usbversion
> eg type=emulated,usbversion=1.
Right -- if we went with an interface where you cannot specify via
libxl exactly which controller qemu uses, then we would try to make
sure that repeated invocations of the same parameters got the same
controller, so that things don't change under the guest OS's feet. So
it should be OK for libvirt to magically know that "{ .type=EMULATED,
.usbversion=1}" would map to 'piix3-uhci', and use that to implement
libvirt's 'piix3-uhci' model. Anything not exposed by libxl would
have to just be unimplemented until the interface was extended to
allow them.
At the moment the mapping exposed by the "usbversion" seems to be: {1,
2, 3} -> {piix3-uhci, ich9-usb-ehci1, nec-usb-xhci}. Does that seem
like a reasonable choice?
Personally I wouldn't mind being able to either pass in "usbversion",
or a string requesting a specific controller name -- but I think IanJ
would prefer to keep those details secret.
> We'd likely give the paravirt
> controller model names of xenusb1, xenusb2, xenusb3 for USB-1
> USB-2 and USB-3 respectively, and again map these to type=pv,usbversion=N.
That sounds reasonable.
-George
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v7 RFC 0/2] libxl USB prototype and design discussion
[not found] <1401716658-22393-1-git-send-email-george.dunlap@eu.citrix.com>
` (3 preceding siblings ...)
[not found] ` <20140605101438.GD19077@redhat.com>
@ 2014-06-18 12:57 ` Ian Campbell
[not found] ` <1403096222.32540.14.camel@kazak.uk.xensource.com>
5 siblings, 0 replies; 15+ messages in thread
From: Ian Campbell @ 2014-06-18 12:57 UTC (permalink / raw)
To: George Dunlap
Cc: Daniel P. Berrange, LibVirt Development List, Jim Fehlig,
xen-devel, sstanisi, Fabio Fantoni, Anthony Perard, Ian Jackson,
Simon (Bo) Cao, Roger Pau Monne
On Mon, 2014-06-02 at 14:44 +0100, George Dunlap wrote:
> == Open questions ==
>
> Those are things I think I know; there are a couple of pertinent
> factual questions which I'm not sure of:
>
> * Is it possible to specify PVUSB controllers and attach USB devices
> to them before the guest is up and running (i.e., before the frontend
> is connected)? It looks like xend had a syntax for specifying virtual
> controllers and attaching virtual devices, so it seems like it should
> be possible.
Unless the PVUSB drivers are radically different to other PV devices
this ought to be possible and should just work. (Essentially you just
preload the backend with all the settings and htey get handled when the
f.e. turns up)
> * Is it possible to connect a USB 1.1 device to a PVUSB controller
> which has been specified 2.0, or would there have to be a separate
> virtual controller for each?
I'm a bit surprised that PVUSB exposes the concept of a version to the
FE at all. I suppose there is some USBish reason why the f.e. would
care.
But I don't know the answer to your question.
> * Is it possible for the toolstack to tell if dom0 (or whatever the
> specified backend domain) has PVUSB support before starting the guest?
After creating the nodes with state == XenbusStateInitialising (1) the
toolstack waits for the backend to move to XenbusStateInitWait (2)
before continuing, with a timeout. So you will detect this in a
controlled way. You can't tell before try the setup though since the
driver might be autoloaded.
(Assuming pvusbback is the same as everything else)
> * Is it possible for the toolstack to tell if domU has a working and
> connected PVUSB front-end?
It can observe the state variable being 4 I suppose. Why do you need to
know?
> * Do we want to be able to create virtual hubs for qemu-backed
> controllers at some point in the future? Is there any groundwork we
> want to lay for that?
qemu-backed emulated or PV controllers?
I don't think emulated would make sense for a PV guest and if qemu
wanted to be a PV backend it would have to implement the usual xenstore
watches etc.
I suppose a backend type a la libxl_device_disk's = phy|tap|qdisk might
be needed for this, but I think you can probably add that in a
compatible way in the future if necessary.
> == Design questions ==
>
> Then based on that, we have several design questions.
>
> * How much of the "controller" thing should be exposed via libxl? Via xl?
>
> * This series only allows you to specify the "protocol", either PV or
> DEVICE_MODEL. Would it be better, for instance, to get rid of that,
> and instead allow the user to specify the creation of USB controllers,
> allow them to have a type of "HCI (or emulated)" or "PV", and allow
> the user to specify which controller to attach a specific device to?
>
> * How much "smarts" should we have in the libxl / xl about creating
> emulated/virtual controllers and of what kinds?
>
> * How much / what kind of "smarts" should be in libxl / xl about
> choosing a controller to plug a device into?
Dunno * 4. Your proposed design looked ok to me.
> * What about config file syntax? Should we try to reuse / extend the
> current config syntax, or create a new one? Should the new one allow
> the specification of controllers? Should it perhaps *require* the
> specification of controllers?
We should at least strive for any existing xm config files which use USB
to continue working, but that needn't imply that the preferred xl syntax
looks that way.
Of course if the xm syntax is horribly terribly broken then we might
make a concious choice not to carry it forward, but the default should
be compatibility.
> For reference, here are some example config snippets from the current
> xl / xm config files:
>
> -- snip --
> # HVM USB
> usb=1
> usbdevice=['tablet','host:4.3']
>
> # HVM USB, not compatible with the above
> usbversion=3
>
> # xend's PVUSB config file; this creates one virtual controller, then
> # plugs hostdev 1.8 into port 1
> vusb=['usbver=2, numports=4, port_1=1-8']
Oh my. That last one is quite exciting.
> -- snip --
>
> Given that, here is a potential config file format:
>
> -- snip --
> # Create two controllers, one pv, one emulated
> usbcontroller=['type=pv,name=pv0,usbversion=2,numports=4',
> 'type=emul,name=hci0,usbversion=2']
>
> # Create a controller with the defaults; PV for PV guests, emul for HVM guests
> usbcontroller=['']
>
> # Same as above, but defaulting to version 2
> usbversion=2
>
> # I think we should be able to automatically detect which format to
> # use; so I think we should re-use usbdevice. I could be convinced otherwise.
> usbdevice=['type=tablet','type=hostdev,hostaddr=4.3,bus=pv0']
Does this require that the usbcontroller have been specified?
I think it would be good if xl would by default create some number of
appropriate controllers without my having to specify them explicitly,
iow just saying usbdevice=[...] should be enough.
I'm not saying that you shouldn't support more specific syntax for
people who want more control, just that it shouldn't be required to do
so.
(I'm just talking xl here, at the libxl layer I think it would be fine
to require them to be explicit).
> * Rather than having to specify a controller, automatically hot-plug
> controllers as-needed.
At the xl level I think this would be good.
Ian.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v7 RFC 1/2] libxl: Introduce functions to add and remove USB devices to an HVM guest
2014-06-02 13:44 ` [PATCH v7 RFC 1/2] libxl: Introduce functions to add and remove USB devices to an HVM guest George Dunlap
@ 2014-06-18 13:08 ` Ian Campbell
2014-06-18 13:22 ` George Dunlap
0 siblings, 1 reply; 15+ messages in thread
From: Ian Campbell @ 2014-06-18 13:08 UTC (permalink / raw)
To: George Dunlap
Cc: xen-devel, sstanisi, Fabio Fantoni, Anthony Perard, Ian Jackson,
Simon (Bo) Cao, Roger Pau Monne
On Mon, 2014-06-02 at 14:44 +0100, George Dunlap wrote:
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> index c7aa817..963e650 100644
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -82,6 +82,12 @@
> #define LIBXL_HAVE_DOMAIN_NODEAFFINITY 1
>
> /*
> + * LIBXL_HAVE_DEVICE_USB indicates the functions for doing hot-plug of
> + * USB devices.
> + */
> +#define LIBXL_HAVE_DEVICE_USB 1
> +
> +/*
> * LIBXL_HAVE_BUILDINFO_HVM_VENDOR_DEVICE indicates that the
> * libxl_vendor_device field is present in the hvm sections of
> * libxl_domain_build_info. This field tells libxl which
> @@ -924,6 +930,40 @@ 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
> + *
> + * For each device removed or added, one of these protocols is available:
> + * - PV (i.e., PVUSB)
> + * - DEVICEMODEL (i.e, qemu)
> + *
> + * PV is available for either PV or HVM domains. DEVICEMODEL is only
> + * available for HVM domains. The caller can additionally specify
> + * "AUTO", in which case the library will try to determine the best
> + * protocol automatically.
> + *
> + * At the moment, the only protocol implemented is DEVICEMODEL, and the only
> + * device type implemented is HOSTDEV.
> + *
> + * This uses the qmp functionality, and is thus only available for
> + * qemu-xen, not qemu-traditional.
> + */
> +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;
> +int libxl_device_usb_destroy(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;
No _getinfo? (Might only make sense with the PV stuff I guess)
> +
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 52f1aa9..ae94ad6 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -457,6 +457,28 @@ libxl_device_vtpm = Struct("device_vtpm", [
> ("uuid", libxl_uuid),
> ])
>
> +libxl_device_usb_protocol = Enumeration("usb_protocol", [
> + (0, "AUTO"),
> + (1, "PV"),
> + (2, "DEVICEMODEL"),
> + ])
> +
> +libxl_device_usb_type = Enumeration("device_usb_type", [
> + (1, "HOSTDEV"),
> + ])
> +
> +libxl_device_usb = Struct("device_usb", [
> + ("protocol", libxl_device_usb_protocol,
> + {'init_val': 'LIBXL_USB_PROTOCOL_AUTO'}),
FYI you can omit the init_val here since it is inherent in the
libxl_device_usb_protocol as you've defined it.
(if the default were non-zero you'd want to specify it, in general we
don't)
> + ("backend_domid", libxl_domid),
> + ("backend_domname", string),
> + ("u", KeyedUnion(None, libxl_device_usb_type, "type",
> + [("hostdev", Struct(None, [
> + ("hostbus", integer),
> + ("hostaddr", integer) ]))
No need to express the host topology I think (because you can build that
from the bus,addr tuples)?
> + ]))
> + ])
> +
> libxl_domain_config = Struct("domain_config", [
> ("c_info", libxl_domain_create_info),
> ("b_info", libxl_domain_build_info),
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v7 RFC 0/2] libxl USB prototype and design discussion
[not found] ` <1403096222.32540.14.camel@kazak.uk.xensource.com>
@ 2014-06-18 13:15 ` George Dunlap
2014-06-18 14:04 ` George Dunlap
[not found] ` <53A19C67.3070809@eu.citrix.com>
2 siblings, 0 replies; 15+ messages in thread
From: George Dunlap @ 2014-06-18 13:15 UTC (permalink / raw)
To: Ian Campbell
Cc: Daniel P. Berrange, LibVirt Development List, Jim Fehlig,
xen-devel, sstanisi, Fabio Fantoni, Anthony Perard, Ian Jackson,
Simon (Bo) Cao, Roger Pau Monne
On 06/18/2014 01:57 PM, Ian Campbell wrote:
>> * Is it possible for the toolstack to tell if domU has a working and
>> connected PVUSB front-end?
> It can observe the state variable being 4 I suppose. Why do you need to
> know?
It might be nice to be able to create both a pv and an emulated
controller in the config file, and then have "xl usb-attach [foo]" to
automatically plug it into the PV controller if the PV frontend seems to
be up, and into the emulated controller if it doesn't seem to be up.
But that doesn't change the elements of the interface, just what the
default would be if the controller field is empty.
>> * Do we want to be able to create virtual hubs for qemu-backed
>> controllers at some point in the future? Is there any groundwork we
>> want to lay for that?
> qemu-backed emulated or PV controllers?
>
> I don't think emulated would make sense for a PV guest and if qemu
> wanted to be a PV backend it would have to implement the usual xenstore
> watches etc.
I mean, emulate an actual USB hub -- you know, it plugs into your USB
controller and you can plug other USB devices into it.
I'm inclined not to bother with it at this point.
>> -- snip --
>>
>> Given that, here is a potential config file format:
>>
>> -- snip --
>> # Create two controllers, one pv, one emulated
>> usbcontroller=['type=pv,name=pv0,usbversion=2,numports=4',
>> 'type=emul,name=hci0,usbversion=2']
>>
>> # Create a controller with the defaults; PV for PV guests, emul for HVM guests
>> usbcontroller=['']
>>
>> # Same as above, but defaulting to version 2
>> usbversion=2
>>
>> # I think we should be able to automatically detect which format to
>> # use; so I think we should re-use usbdevice. I could be convinced otherwise.
>> usbdevice=['type=tablet','type=hostdev,hostaddr=4.3,bus=pv0']
> Does this require that the usbcontroller have been specified?
>
> I think it would be good if xl would by default create some number of
> appropriate controllers without my having to specify them explicitly,
> iow just saying usbdevice=[...] should be enough.
>
> I'm not saying that you shouldn't support more specific syntax for
> people who want more control, just that it shouldn't be required to do
> so.
>
> (I'm just talking xl here, at the libxl layer I think it would be fine
> to require them to be explicit).
That makes sense.
>> * Rather than having to specify a controller, automatically hot-plug
>> controllers as-needed.
> At the xl level I think this would be good.
OK, sounds good. Thanks for the input.
-George
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v7 RFC 1/2] libxl: Introduce functions to add and remove USB devices to an HVM guest
2014-06-18 13:08 ` Ian Campbell
@ 2014-06-18 13:22 ` George Dunlap
2014-06-18 13:49 ` Ian Campbell
0 siblings, 1 reply; 15+ messages in thread
From: George Dunlap @ 2014-06-18 13:22 UTC (permalink / raw)
To: Ian Campbell
Cc: xen-devel, sstanisi, Fabio Fantoni, Anthony Perard, Ian Jackson,
Simon (Bo) Cao, Roger Pau Monne
On 06/18/2014 02:08 PM, Ian Campbell wrote:
> On Mon, 2014-06-02 at 14:44 +0100, George Dunlap wrote:
>> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
>> index c7aa817..963e650 100644
>> --- a/tools/libxl/libxl.h
>> +++ b/tools/libxl/libxl.h
>> @@ -82,6 +82,12 @@
>> #define LIBXL_HAVE_DOMAIN_NODEAFFINITY 1
>>
>> /*
>> + * LIBXL_HAVE_DEVICE_USB indicates the functions for doing hot-plug of
>> + * USB devices.
>> + */
>> +#define LIBXL_HAVE_DEVICE_USB 1
>> +
>> +/*
>> * LIBXL_HAVE_BUILDINFO_HVM_VENDOR_DEVICE indicates that the
>> * libxl_vendor_device field is present in the hvm sections of
>> * libxl_domain_build_info. This field tells libxl which
>> @@ -924,6 +930,40 @@ 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
>> + *
>> + * For each device removed or added, one of these protocols is available:
>> + * - PV (i.e., PVUSB)
>> + * - DEVICEMODEL (i.e, qemu)
>> + *
>> + * PV is available for either PV or HVM domains. DEVICEMODEL is only
>> + * available for HVM domains. The caller can additionally specify
>> + * "AUTO", in which case the library will try to determine the best
>> + * protocol automatically.
>> + *
>> + * At the moment, the only protocol implemented is DEVICEMODEL, and the only
>> + * device type implemented is HOSTDEV.
>> + *
>> + * This uses the qmp functionality, and is thus only available for
>> + * qemu-xen, not qemu-traditional.
>> + */
>> +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;
>> +int libxl_device_usb_destroy(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;
> No _getinfo? (Might only make sense with the PV stuff I guess)
IIRC the pattern I saw for other devices was:
* libxl_device_$FOO struct is used to add, remove, destroy
* libl_device_$FOO_list returns an array of libxl_device_$FOO
* libl_device_$FOO_getinfo is only used when there's information you
need which is not in libxl_device_$FOO struct (and hence isn't returned
by _list).
>> + ("backend_domid", libxl_domid),
>> + ("backend_domname", string),
>> + ("u", KeyedUnion(None, libxl_device_usb_type, "type",
>> + [("hostdev", Struct(None, [
>> + ("hostbus", integer),
>> + ("hostaddr", integer) ]))
> No need to express the host topology I think (because you can build that
> from the bus,addr tuples)?
I don't really follow. You mean, we can drop 'host' from the last two
elements, and just call them "bus" and "addr"?
-George
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v7 RFC 1/2] libxl: Introduce functions to add and remove USB devices to an HVM guest
2014-06-18 13:22 ` George Dunlap
@ 2014-06-18 13:49 ` Ian Campbell
2014-06-18 13:58 ` George Dunlap
0 siblings, 1 reply; 15+ messages in thread
From: Ian Campbell @ 2014-06-18 13:49 UTC (permalink / raw)
To: George Dunlap
Cc: xen-devel, sstanisi, Fabio Fantoni, Anthony Perard, Ian Jackson,
Simon (Bo) Cao, Roger Pau Monne
On Wed, 2014-06-18 at 14:22 +0100, George Dunlap wrote:
> On 06/18/2014 02:08 PM, Ian Campbell wrote:
> > On Mon, 2014-06-02 at 14:44 +0100, George Dunlap wrote:
> >> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> >> index c7aa817..963e650 100644
> >> --- a/tools/libxl/libxl.h
> >> +++ b/tools/libxl/libxl.h
> >> @@ -82,6 +82,12 @@
> >> #define LIBXL_HAVE_DOMAIN_NODEAFFINITY 1
> >>
> >> /*
> >> + * LIBXL_HAVE_DEVICE_USB indicates the functions for doing hot-plug of
> >> + * USB devices.
> >> + */
> >> +#define LIBXL_HAVE_DEVICE_USB 1
> >> +
> >> +/*
> >> * LIBXL_HAVE_BUILDINFO_HVM_VENDOR_DEVICE indicates that the
> >> * libxl_vendor_device field is present in the hvm sections of
> >> * libxl_domain_build_info. This field tells libxl which
> >> @@ -924,6 +930,40 @@ 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
> >> + *
> >> + * For each device removed or added, one of these protocols is available:
> >> + * - PV (i.e., PVUSB)
> >> + * - DEVICEMODEL (i.e, qemu)
> >> + *
> >> + * PV is available for either PV or HVM domains. DEVICEMODEL is only
> >> + * available for HVM domains. The caller can additionally specify
> >> + * "AUTO", in which case the library will try to determine the best
> >> + * protocol automatically.
> >> + *
> >> + * At the moment, the only protocol implemented is DEVICEMODEL, and the only
> >> + * device type implemented is HOSTDEV.
> >> + *
> >> + * This uses the qmp functionality, and is thus only available for
> >> + * qemu-xen, not qemu-traditional.
> >> + */
> >> +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;
> >> +int libxl_device_usb_destroy(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;
> > No _getinfo? (Might only make sense with the PV stuff I guess)
>
> IIRC the pattern I saw for other devices was:
> * libxl_device_$FOO struct is used to add, remove, destroy
> * libl_device_$FOO_list returns an array of libxl_device_$FOO
> * libl_device_$FOO_getinfo is only used when there's information you
> need which is not in libxl_device_$FOO struct (and hence isn't returned
> by _list).
Right, getinfo is runtime stuff, like the evtchn and shared ring (or
stats, I guess). Hence probably only for PV unless there is something
like that for the emulated stuff.
>
>
> >> + ("backend_domid", libxl_domid),
> >> + ("backend_domname", string),
> >> + ("u", KeyedUnion(None, libxl_device_usb_type, "type",
> >> + [("hostdev", Struct(None, [
> >> + ("hostbus", integer),
> >> + ("hostaddr", integer) ]))
> > No need to express the host topology I think (because you can build that
> > from the bus,addr tuples)?
>
> I don't really follow. You mean, we can drop 'host' from the last two
> elements, and just call them "bus" and "addr"?
Gah, I started writing one thing and then reunderstood usb and wrote
half another.
What I was trying to say is that you don't need hostaddr to describe the
full USB topology path to the device because the (bus,addr) tuple you've
given already does so (because each hub effectively creates a new bus
number, so they all look like toplevel buses in this representation).
But you could drop the host prefix, yes.
Ian.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v7 RFC 1/2] libxl: Introduce functions to add and remove USB devices to an HVM guest
2014-06-18 13:49 ` Ian Campbell
@ 2014-06-18 13:58 ` George Dunlap
2014-06-18 14:30 ` Ian Campbell
0 siblings, 1 reply; 15+ messages in thread
From: George Dunlap @ 2014-06-18 13:58 UTC (permalink / raw)
To: Ian Campbell
Cc: xen-devel, sstanisi, Fabio Fantoni, Anthony Perard, Ian Jackson,
Simon (Bo) Cao, Roger Pau Monne
On 06/18/2014 02:49 PM, Ian Campbell wrote:
>>>> + ("backend_domid", libxl_domid),
>>>> + ("backend_domname", string),
>>>> + ("u", KeyedUnion(None, libxl_device_usb_type, "type",
>>>> + [("hostdev", Struct(None, [
>>>> + ("hostbus", integer),
>>>> + ("hostaddr", integer) ]))
>>> No need to express the host topology I think (because you can build that
>>> from the bus,addr tuples)?
>> I don't really follow. You mean, we can drop 'host' from the last two
>> elements, and just call them "bus" and "addr"?
> Gah, I started writing one thing and then reunderstood usb and wrote
> half another.
>
> What I was trying to say is that you don't need hostaddr to describe the
> full USB topology path to the device because the (bus,addr) tuple you've
> given already does so (because each hub effectively creates a new bus
> number, so they all look like toplevel buses in this representation).
You seem to be saying that something is redundant, or that there's extra
information somewhere; but as there are only two bits of data (bus and
addr), and agree that I need both, I'm having a hard time telling what
you think is redundant / could be removed..
"hostdev" is an element of the union; so the structure should unpack
like this:
struct {
libxl_domid backend_domid;
char * backend_domname;
libxl_usb_device_type type;
union u {
struct {
int hostbus, hostaddr;
} hostdev;
};
};
At the moment, "type" can only be "hostdev"; but I'm envisioning in the
future that "type" might be "tablet", "mouse", "keyboard", maybe
"mass-storage", and that the union would have more elements.
Does that clear things up? Or am I totally confused? :-)
-George
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v7 RFC 0/2] libxl USB prototype and design discussion
[not found] ` <1403096222.32540.14.camel@kazak.uk.xensource.com>
2014-06-18 13:15 ` George Dunlap
@ 2014-06-18 14:04 ` George Dunlap
[not found] ` <53A19C67.3070809@eu.citrix.com>
2 siblings, 0 replies; 15+ messages in thread
From: George Dunlap @ 2014-06-18 14:04 UTC (permalink / raw)
To: Ian Campbell
Cc: Daniel P. Berrange, LibVirt Development List, Jim Fehlig,
xen-devel, sstanisi, Fabio Fantoni, Anthony Perard, Ian Jackson,
Simon (Bo) Cao, Roger Pau Monne
On 06/18/2014 01:57 PM, Ian Campbell wrote:
>> * Is it possible to connect a USB 1.1 device to a PVUSB controller
>> which has been specified 2.0, or would there have to be a separate
>> virtual controller for each?
> I'm a bit surprised that PVUSB exposes the concept of a version to the
> FE at all. I suppose there is some USBish reason why the f.e. would
> care.
>
> But I don't know the answer to your question.
Simon,
Can you put "Test different USB version devices with different PVUSB bus
version numbers" on your to-do list?
(i.e., find USB 1.1, 2.0, and 3.0 devices, and try each of them with
usbversion set to 1, 2, and 3).
Thanks!
-George
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v7 RFC 1/2] libxl: Introduce functions to add and remove USB devices to an HVM guest
2014-06-18 13:58 ` George Dunlap
@ 2014-06-18 14:30 ` Ian Campbell
2014-06-18 14:47 ` George Dunlap
0 siblings, 1 reply; 15+ messages in thread
From: Ian Campbell @ 2014-06-18 14:30 UTC (permalink / raw)
To: George Dunlap
Cc: xen-devel, sstanisi, Fabio Fantoni, Anthony Perard, Ian Jackson,
Simon (Bo) Cao, Roger Pau Monne
On Wed, 2014-06-18 at 14:58 +0100, George Dunlap wrote:
> On 06/18/2014 02:49 PM, Ian Campbell wrote:
> >>>> + ("backend_domid", libxl_domid),
> >>>> + ("backend_domname", string),
> >>>> + ("u", KeyedUnion(None, libxl_device_usb_type, "type",
> >>>> + [("hostdev", Struct(None, [
> >>>> + ("hostbus", integer),
> >>>> + ("hostaddr", integer) ]))
> >>> No need to express the host topology I think (because you can build that
> >>> from the bus,addr tuples)?
> >> I don't really follow. You mean, we can drop 'host' from the last two
> >> elements, and just call them "bus" and "addr"?
> > Gah, I started writing one thing and then reunderstood usb and wrote
> > half another.
> >
> > What I was trying to say is that you don't need hostaddr to describe the
> > full USB topology path to the device because the (bus,addr) tuple you've
> > given already does so (because each hub effectively creates a new bus
> > number, so they all look like toplevel buses in this representation).
>
> You seem to be saying that something is redundant, or that there's extra
> information somewhere; but as there are only two bits of data (bus and
> addr), and agree that I need both, I'm having a hard time telling what
> you think is redundant / could be removed..
I'm actually saying the opposite (or rather asking for confirmation that
this is the case). i.e. one might expect that *more* information would
be needed here (i.e. the full bus topology/path to a device via multiple
hubs etc) but in fact this is not needed because the topology is not
visible in this numbering scheme (each potential fork in the path
results in a whole new bus number).
Sorry, I could have been clearer about this.
> "hostdev" is an element of the union; so the structure should unpack
> like this:
> struct {
> libxl_domid backend_domid;
> char * backend_domname;
> libxl_usb_device_type type;
> union u {
> struct {
> int hostbus, hostaddr;
> } hostdev;
> };
> };
>
> At the moment, "type" can only be "hostdev"; but I'm envisioning in the
> future that "type" might be "tablet", "mouse", "keyboard", maybe
> "mass-storage", and that the union would have more elements.
This seems sensible. You brought up dropping the hostdev prefix from the
bus and addr (because you thought I was ;-)), which seemed sensible when
you think of the code which accesses these fields i.e.
dev->u.hostdev.hostbus vs dev->u.hostdev.bus. Up to you whether you want
to repeat "host" in that.
> Does that clear things up? Or am I totally confused? :-)
I think you are remarkably unconfused given how confusing I'm being...
Ian.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v7 RFC 1/2] libxl: Introduce functions to add and remove USB devices to an HVM guest
2014-06-18 14:30 ` Ian Campbell
@ 2014-06-18 14:47 ` George Dunlap
0 siblings, 0 replies; 15+ messages in thread
From: George Dunlap @ 2014-06-18 14:47 UTC (permalink / raw)
To: Ian Campbell
Cc: xen-devel, sstanisi, Fabio Fantoni, Anthony Perard, Ian Jackson,
Simon (Bo) Cao, Roger Pau Monne
On 06/18/2014 03:30 PM, Ian Campbell wrote:
> On Wed, 2014-06-18 at 14:58 +0100, George Dunlap wrote:
>> On 06/18/2014 02:49 PM, Ian Campbell wrote:
>>>>>> + ("backend_domid", libxl_domid),
>>>>>> + ("backend_domname", string),
>>>>>> + ("u", KeyedUnion(None, libxl_device_usb_type, "type",
>>>>>> + [("hostdev", Struct(None, [
>>>>>> + ("hostbus", integer),
>>>>>> + ("hostaddr", integer) ]))
>>>>> No need to express the host topology I think (because you can build that
>>>>> from the bus,addr tuples)?
>>>> I don't really follow. You mean, we can drop 'host' from the last two
>>>> elements, and just call them "bus" and "addr"?
>>> Gah, I started writing one thing and then reunderstood usb and wrote
>>> half another.
>>>
>>> What I was trying to say is that you don't need hostaddr to describe the
>>> full USB topology path to the device because the (bus,addr) tuple you've
>>> given already does so (because each hub effectively creates a new bus
>>> number, so they all look like toplevel buses in this representation).
>> You seem to be saying that something is redundant, or that there's extra
>> information somewhere; but as there are only two bits of data (bus and
>> addr), and agree that I need both, I'm having a hard time telling what
>> you think is redundant / could be removed..
> I'm actually saying the opposite (or rather asking for confirmation that
> this is the case). i.e. one might expect that *more* information would
> be needed here (i.e. the full bus topology/path to a device via multiple
> hubs etc) but in fact this is not needed because the topology is not
> visible in this numbering scheme (each potential fork in the path
> results in a whole new bus number).
>
> Sorry, I could have been clearer about this.
Ah, I see. Yes, that's right: <bus,addr> is sufficient to specify a
single device.
To qemu you can either specify <bus,addr> or <vendorid:productid>.
Unfortunately both have their advantages and disavantages. <bus,addr>
will specify a unique device, but exactly which device that is may
change across reboots, so you have to check each time that you're still
passing in your USB stick rather than your webcam.
On the other hang, <vid,pid> will allow you to pass in a specific kind
of device no matter where it's plugged in... as long as there's only one
of them. If you have two webcams / usb devices of the same kind, you're
out of luck, and you have to fall back to <bus,addr> again.
I considered allowing the user to specify either one, just as you can
with qemu. (That might mean having libxl do the <vid,pid> -> <bus,addr>
resolution for PVUSB.) I'm open to opinions on that one.
-George
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v7 RFC 0/2] libxl USB prototype and design discussion
[not found] ` <53A19C67.3070809@eu.citrix.com>
@ 2014-06-30 14:41 ` Simon Cao
0 siblings, 0 replies; 15+ messages in thread
From: Simon Cao @ 2014-06-30 14:41 UTC (permalink / raw)
To: George Dunlap
Cc: Daniel P. Berrange, Ian Campbell, LibVirt Development List,
Jim Fehlig, xen-devel, sstanisi, Fabio Fantoni, Anthony Perard,
Ian Jackson, Roger Pau Monne
[-- Attachment #1.1: Type: text/plain, Size: 833 bytes --]
okay, I will do my best to test all the three devices.
Bo.
2014-06-18 22:04 GMT+08:00 George Dunlap <george.dunlap@eu.citrix.com>:
> On 06/18/2014 01:57 PM, Ian Campbell wrote:
>
>> * Is it possible to connect a USB 1.1 device to a PVUSB controller
>>> which has been specified 2.0, or would there have to be a separate
>>> virtual controller for each?
>>>
>> I'm a bit surprised that PVUSB exposes the concept of a version to the
>> FE at all. I suppose there is some USBish reason why the f.e. would
>> care.
>>
>> But I don't know the answer to your question.
>>
>
> Simon,
>
> Can you put "Test different USB version devices with different PVUSB bus
> version numbers" on your to-do list?
>
> (i.e., find USB 1.1, 2.0, and 3.0 devices, and try each of them with
> usbversion set to 1, 2, and 3).
>
> Thanks!
>
> -George
>
>
[-- Attachment #1.2: Type: text/html, Size: 1564 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v7 RFC 0/2] libxl USB prototype and design discussion
@ 2014-06-02 13:44 George Dunlap
0 siblings, 0 replies; 15+ messages in thread
From: George Dunlap @ 2014-06-02 13:44 UTC (permalink / raw)
To: xen-devel
Cc: Daniel P. Berrange, Ian Campbell, LibVirt Development List,
Jim Fehlig, sstanisi, Fabio Fantoni, Anthony Perard, Ian Jackson,
Simon (Bo) Cao, Roger Pau Monne
Hey all!
So here I finally have a rebased the HVM USB hotplug series from last
year. I went through and addressed all of IanC's technical comments
that I could; so it should be in much better shape than it was.
However, one of the unfinished threads of the conversation at that
time was about the interface. In particular since Simon Bo is now
working on the PVUSB side of things, I thought it would be good to
review the situation so we can get an interface we're happy with.
In order to really answer the questions, I went through and looked at
the underlying capabilities of QEMU and PVUSB, at the existing libxl
and xend interfaces for those two, and also at the libvirt interface.
I summarize what I've found here; but if you want a TLDR version, just
skip to the bottom, where it says "Draft Design Proposal". But before
asking any questions or making any criticisms, please go back and read
the appropriate summary section.
References are at the very bottom of the e-mail.
This can be found at the following address:
git://xenbits.xen.org/people/gdunlap/xen.git out/hvm-usb-hotplug-v7-RFC
== Capabilities of QEMU ==
Qemu has the additional ability to not only pass through host devices,
but other kinds of devices -- tablets, mice, keyboards, drives, hubs,
and so on. As far as I know, PVUSB can only do host device
pass-through at the moment. (Although you could certainly imagine
that changing; if qemu was taught how to become a PVUSB back-end, for
example.)
qemu has two ways of specifying usb devices. The "legacy" method can
be specified on the qemu command-line or via the qemu monitor. It is
limited to USB 1.1. The new "qdevice" method can be used either on
the command-line or via qmp.
qemu-traditional only supports the legacy method. qemu-xen still
supports the legacy method via command-line, but we should try to
start using the qdevice method if we can.
qemu-xen has the ability to create virtual USB 2.0 or 3.0 controllers.
These can be named, and it is possible to specify (to a certain level)
which controller to plug a device into. These can be created either
via the command-line, or hot-plugged via qmp.
qemu also seems to be willing to shift things around behind the scenes
to be able to handle your request; shifting around the USB topology,
for instance. It will also do "helpful" things; for instance, if you
specify that you want to plug a device into a USB 2.0 controller, and
the device is only 1.1 capable, it will ignore what you asked and plug
it into the 1.1 controller.
== Capabilities of PVUSB ==
PVUSB also requires you to first create a virtual controller, before
attaching host devices to it. There is some flexibility in how to
create these, and you can create more than one and specify which
virtual bus to attach the host device. You seem to be able to specify
the USB version number when you specify the controller as well. You
can also specify the number of ports for a device; I'm not sure what
the maximum number of ports per controller is, or why you would ever
really want to have less than the maximum number of ports.
It looks like in the xm interface, when you create a virtual
controller it is assigned an index, starting at 0; and this is the
index that is used when specifying where to plug in a device.
(Simon, can you please correct this if I'm wrong, and add anything
important that I missed?)
== libxl/xl interface ==
At the moment, libxl/xl only support USB at domain creation time.
For HVM guests, we have two incompatible sets of options. usb=1 and
usbdevice=[] use the "old-style" qemu interface on the qemu
command-line. The "old-style" interface can only be used to specify
USB 1.1 devices. We also have "usbversion=[foo]", which uses the
"new-style" device specification commands. These new specification
commands *can* be specified either on the qemu command-line or via
qmp; but at the moment libxl specifies them on the command-line.
The patch series attached specifes using the new-style interface via
qmp. (This seems to work properly with the various controllers no
matter how they were created.) In theory, we should be able to attach
these devices during domain creation, after qemu has been started but
before the guest is running.
Obviously, we would ideally like for the user not to have to worry
about a lot of this complexity, and just say, "Can you pass this
device through to the guest? Thanks."
Another thing to consider in the design space is config file and
start-up behavior.
== Libvirt's interface ==
I've had a brief look at libvirt's USB interface, and learned a bit
about libvirt's general approach to things at the Xen Hackathon last
week. One of the goals of libvirt is to be able to specify the
virtual hardware in enough detail to keep it from changing when you
upgrade the hypervisor, so that certain proprietary operating systems
which are sensitive to this kind of thing continue to work.
Instead of specifying a controller name, you specify a controller
index (which is different than qemu). But instead of specifying a USB
version number, you specify a model for the USB controller (which
happens to be the exact name that qemu uses): "piix3-uhci",
"piix4-uhci", "ehci", "ich9-ehci1", "ich9-uhci1", "ich9-uhci2",
"ich9-uhci3", "vt82c686b-uhci", "pci-ohci" or "nec-xhci".
When specifying a USB device, libvirt has the concept of an "address"
where you will plug it into. Here is what the page says about it:
"USB addresses have the following additional attributes: bus (a hex
value between 0 and 0xfff, inclusive), and port (a dotted notation of
up to four octets, such as 1.2 or 2.1.3.1)."
It's not exactly clear to me what those numbers mean. But after
chatting with Daniel Berrange at the Hackathon, it looks like the
"bus" in the address corresponds to the "index" in the controller
specification. It also appears that this "index" is internal to
libvirt and is not exposed to the guest: so it should in theory be
possible to have index 0 be a Xen PVUSB controller, 1 be an emulated
qemu controller, 3 be an emulated PVUSB controller, and so on.
== Open questions ==
Those are things I think I know; there are a couple of pertinent
factual questions which I'm not sure of:
* Is it possible to specify PVUSB controllers and attach USB devices
to them before the guest is up and running (i.e., before the frontend
is connected)? It looks like xend had a syntax for specifying virtual
controllers and attaching virtual devices, so it seems like it should
be possible.
* Is it possible to connect a USB 1.1 device to a PVUSB controller
which has been specified 2.0, or would there have to be a separate
virtual controller for each?
* Is it possible for the toolstack to tell if dom0 (or whatever the
specified backend domain) has PVUSB support before starting the guest?
* Is it possible for the toolstack to tell if domU has a working and
connected PVUSB front-end?
* Do we want to be able to create virtual hubs for qemu-backed
controllers at some point in the future? Is there any groundwork we
want to lay for that?
== Design questions ==
Then based on that, we have several design questions.
* How much of the "controller" thing should be exposed via libxl? Via xl?
* This series only allows you to specify the "protocol", either PV or
DEVICE_MODEL. Would it be better, for instance, to get rid of that,
and instead allow the user to specify the creation of USB controllers,
allow them to have a type of "HCI (or emulated)" or "PV", and allow
the user to specify which controller to attach a specific device to?
* How much "smarts" should we have in the libxl / xl about creating
emulated/virtual controllers and of what kinds?
* How much / what kind of "smarts" should be in libxl / xl about
choosing a controller to plug a device into?
* What about config file syntax? Should we try to reuse / extend the
current config syntax, or create a new one? Should the new one allow
the specification of controllers? Should it perhaps *require* the
specification of controllers?
== Draft design proposal ==
I've given it some thought, and based on the below is a suggested
design for people to have a go at.
Basic idea: Specify named controllers. It's controllers that are PV
or emulated, and may have a backend domain. When adding devices,
specify which controller to attach it to. Allow most things to have
intelligent defaults.
* libxl functions
usb_controller_add
usb_controller_remove/destroy
usb_controller_list
device struct:
name # If empty, default to hciN/pvN, where N starts at 0
type = {PV, EMULATED, AUTO}
backend_{domname,domid} # PV only
usbversion = {1, 2, 3}
numports # default 16
If type==AUTO, then it will be PV for a PV domain, EMULATED for an HVM domain.
usb_add
usb_remove/destroy
usb_list
device struct:
controller_name # If empty, choose one of the controllers.
type
hostdev
hostbus,hostaddr
Note: I've removed backend from the device struct, as that will be
based on the controller.
* Storing information about virtual USB controllers / devices
qemu does not at the moment have a way to query for plugged-in
devices. I'm not sure if PVUSB does. Store information about both
USB controllers and USB devices created with libxl in xenstore,
somewhat similar to the system in the attached patches.
* Domain creation
I think what we add to the domain creation libxl calls will be obvious
from how we design the config file interface.
For reference, here are some example config snippets from the current
xl / xm config files:
-- snip --
# HVM USB
usb=1
usbdevice=['tablet','host:4.3']
# HVM USB, not compatible with the above
usbversion=3
# xend's PVUSB config file; this creates one virtual controller, then
# plugs hostdev 1.8 into port 1
vusb=['usbver=2, numports=4, port_1=1-8']
-- snip --
Given that, here is a potential config file format:
-- snip --
# Create two controllers, one pv, one emulated
usbcontroller=['type=pv,name=pv0,usbversion=2,numports=4',
'type=emul,name=hci0,usbversion=2']
# Create a controller with the defaults; PV for PV guests, emul for HVM guests
usbcontroller=['']
# Same as above, but defaulting to version 2
usbversion=2
# I think we should be able to automatically detect which format to
# use; so I think we should re-use usbdevice. I could be convinced otherwise.
usbdevice=['type=tablet','type=hostdev,hostaddr=4.3,bus=pv0']
-- snip --
Other ideas:
* To make the interface closer to libvirt's, instead of specifying PV
/ EMULATED and usbversion, just specify a model. Then create
"pvusb-v1", "pvusb-v2", &c for PVUSB hubs with the various versions,
and detect automatically whether to use the PV or the qemu path. (NB
that at the libvirt level, to fit with their syntax, we'd probably end
up creating a "xenpvusbN" model in libvirt that would translate down
to whatever the appropriate thing is in libxl.)
* Rather than having to specify a controller, automatically hot-plug
controllers as-needed.
* Include an optional port number into which we will plug the USB
device.
Thoughts?
-George
== References ==
Last time this was posted:
http://thread.gmane.org/gmane.comp.emulators.xen.devel/156796
PVUSB references:
http://doc.opensuse.org/products/draft/SLES/SLES-xen_sd_draft/cha.xen.config.html#sec.xen.config.pvusb
Libvirt references:
http://wiki.libvirt.org/page/QEMUSwitchToLibvirt#-usbdevice
https://access.redhat.com/site/documentation/en-US/Red_Hat_Enterprise_Linux/6/html/Virtualization_Administration_Guide/sect-Attaching_and_updating_a_device_with_virsh.html
http://libvirt.org/formatdomain.html
http://libvirt.org/formatdomain.html#elementsControllers
CC: Ian Jackson <ian.jackson@citrix.com>
CC: Roger Pau Monne <roger.pau@citrix.com>
CC: sstanisi@cbnco.com
CC: Fabio Fantoni <fabio.fantoni@m2r.biz>
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Anthony Perard <anthony.perard@citrix.com>
CC: Simon (Bo) Cao <caobosimon@gmail.com>
CC: LibVirt Development List <libvir-list@redhat.com>
CC: Jim Fehlig <jfehlig@suse.com>
CC: Daniel P. Berrange <berrange@redhat.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2014-06-30 14:41 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1401716658-22393-1-git-send-email-george.dunlap@eu.citrix.com>
2014-06-02 13:44 ` [PATCH v7 RFC 1/2] libxl: Introduce functions to add and remove USB devices to an HVM guest George Dunlap
2014-06-18 13:08 ` Ian Campbell
2014-06-18 13:22 ` George Dunlap
2014-06-18 13:49 ` Ian Campbell
2014-06-18 13:58 ` George Dunlap
2014-06-18 14:30 ` Ian Campbell
2014-06-18 14:47 ` George Dunlap
2014-06-02 13:44 ` [PATCH v7 RFC 2/2] xl: Add commands for usb hot-plug George Dunlap
2014-06-05 10:14 ` [PATCH v7 RFC 0/2] libxl USB prototype and design discussion Daniel P. Berrange
[not found] ` <20140605101438.GD19077@redhat.com>
2014-06-05 15:04 ` George Dunlap
2014-06-18 12:57 ` Ian Campbell
[not found] ` <1403096222.32540.14.camel@kazak.uk.xensource.com>
2014-06-18 13:15 ` George Dunlap
2014-06-18 14:04 ` George Dunlap
[not found] ` <53A19C67.3070809@eu.citrix.com>
2014-06-30 14:41 ` Simon Cao
2014-06-02 13:44 George Dunlap
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.