[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 1/2] libxl: Introduce functions to add and remove host USB devices to an HVM guest

On 03/19/2013 01:14 PM, Roger Pau Monne wrote:
On 19/03/13 13:09, George Dunlap wrote:
This uses the qmp functionality, and is thus only available for qemu-xen,
not qemu-traditional.

Devices must be removed by "id", an identifying string, which must be
specified when the device is created.  The caller can either pass one
in to request; if none is passed in, then libxl will choose one and pass
it back to the caller.

qemu will reject duplicate ids.  There is a small possibility that the
libxl-chosen id my collide with a previously created one, in which
case the add will fail.  It would be nice if the library could
automatically modify it until it found a unique one, but at the moment
qmp_run_command() doesn't return which error the command failed by, so
the caller can't tell that the command failed because of a duplicate

Since it's additional work, and it's not clear that the situation can
actually happen in practice, I'm considering it a "maybe do later".

Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
  tools/libxl/libxl.c          |   85 ++++++++++++++++++++++++++++++++++++++++++
  tools/libxl/libxl.h          |   32 ++++++++++++++++
  tools/libxl/libxl_internal.h |    3 ++
  tools/libxl/libxl_qmp.c      |   45 ++++++++++++++++++++++
  tools/libxl/libxl_types.idl  |    8 ++++
  5 files changed, 173 insertions(+)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 572c2c6..34b648e 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2498,6 +2498,91 @@ out:
      return AO_INPROGRESS;

+int libxl_hvm_host_usb_add(libxl_ctx *ctx, uint32_t domid,
+                           libxl_device_host_usb *dev)

Would it make sense to just call this function libxl_device_usb_add and
fail if guest type is not HVM?

If we later add usb support to PV guests, I would prefer to avoid having
another libxl_pv_host_usb_add or libxl_pvh_host_usb_add.

I had thought about something like that, but the basic problem is that HVM guests can use PVUSB as well. I don't think libxl is the right place to be deciding whether to use qemu or PVUSB if both are available.

Also you should add a "const libxl_asyncop_how *ao_how", even if this is
not an async op right now, we might want to make it async in the future.

OK -- I'll try to copy libxl_insert_cdrom(), since that seems to do the "fake async" thing.

+    GC_INIT(ctx);
+    int rc, dm_ver;
+    libxl_domain_type type = libxl__domain_type(gc, domid);
+    if (type == LIBXL_DOMAIN_TYPE_INVALID) {
+        rc = ERROR_FAIL;
+        goto out;
+    }
+    if (type != LIBXL_DOMAIN_TYPE_HVM) {
+        LOG(ERROR, "hvm-host-usb-add requires an HVM domain");
+        rc = ERROR_INVAL;
+        goto out;
+    }
+    if (libxl_get_stubdom_id(ctx, domid) != 0) {
+        LOG(ERROR, "hvm-host-usb-add doesn't work for stub domains");
+        rc = ERROR_INVAL;
+        goto out;
+    }
+    dm_ver = libxl__device_model_version_running(gc, domid);
+    if (dm_ver == -1) {
+        LOG(ERROR, "cannot determine device model version");
+        rc = ERROR_FAIL;
+        goto out;
+    }
+        rc = libxl__qmp_host_usb_add(gc, domid, dev);
+    } else {
+        LOG(ERROR, "hvm-host-usb-add not yet implemented for 
+        rc = ERROR_FAIL;
+    }
+    GC_FREE;
+    return rc;
+int libxl_hvm_host_usb_del(libxl_ctx *ctx, uint32_t domid,
+                           const char * id)

Same here, libxl_device_usb_remove will probably be a better name to
keep in sync with the current device functions, and it's also missing a
"const libxl_asyncop_how *ao_how".

Is it possible to pass a libxl_device_host_usb *dev instead of an id? So
that the function resembles to the other _remove/_destroy functions.

The only way qmp allows you to remove a device is via id. And at the moment there is no way via qmp to list USB devices to find out which ones might have an id.

I suppose if we allow the user to pass *dev though, then usb_del could either use dev->id (if it exists), or re-construct the default id and try to remove it if not.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.