[PATCH v3 0/5] libxl: add support for qemu base pvusb backend

[PATCH v3 0/5] libxl: add support for qemu base pvusb backend

[Juergen Gross]

This patch series is meant to be applied on top of Chunyan's series
to support pvusb in libxl.

It is adding support for an alternative pvusb backend "qusb" via qemu.

Changes in V3:
- added new patches 3 and 4 to at least report failure in case no device
  model is running when adding devices to a domain requiring a dm.

Changes in V2:
- patch 1: Return false if libxl__get_domid() fails as requested by
  George Dunlap
- Swapped patches 2 and 3 as former patch 2 has been questioned to make
  sense for 4.7. This will remove an obstacle for former patch 3 to go in.

Juergen Gross (5):
  libxl: make libxl__need_xenpv_qemu() operate on domain config
  libxl: add new pvusb backend "qusb" provided by qemu
  libxl: add service function to check whether device model is running
  libxl: check for dynamic device model start required
  libxl: add domain config parameter to force start of qemu

 docs/man/xl.cfg.pod.5                |  17 +++++-
 tools/libxl/libxl.c                  |  16 +++++-
 tools/libxl/libxl_create.c           |  10 +---
 tools/libxl/libxl_device.c           |   3 +-
 tools/libxl/libxl_dm.c               |  88 +++++++++++++++-------------
 tools/libxl/libxl_internal.h         |  10 ++--
 tools/libxl/libxl_pci.c              |   3 +
 tools/libxl/libxl_pvusb.c            | 108 +++++++++++++++++++++++++++--------
 tools/libxl/libxl_types.idl          |   2 +
 tools/libxl/libxl_types_internal.idl |   1 +
 tools/libxl/xl_cmdimpl.c             |   3 +
 11 files changed, 181 insertions(+), 80 deletions(-)

-- 
2.6.2


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH v3 1/5] libxl: make libxl__need_xenpv_qemu() operate on domain config

[Juergen Gross]

libxl__need_xenpv_qemu() is called with configuration data for console,
vfbs, disks and channels today in order to evaluate the need for
starting a device model for a pv domain.

The console data is local to the caller and setup in a way to never
require a device model. All other data is taken from the domain config
structure.

In order to support other device backends via qemu change the interface
of libxl__need_xenpv_qemu() to take the domain config structure as
input instead of the single device arrays.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2: Return false if libxl__get_domid() fails as requested by George Dunlap
---
 tools/libxl/libxl_create.c   |  9 +------
 tools/libxl/libxl_dm.c       | 57 ++++++++++++--------------------------------
 tools/libxl/libxl_internal.h |  5 +---
 3 files changed, 17 insertions(+), 54 deletions(-)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 61b5c01..0e2b0a0 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1304,7 +1304,6 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
     }
     case LIBXL_DOMAIN_TYPE_PV:
     {
-        int need_qemu = 0;
         libxl__device_console console;
         libxl__device device;
 
@@ -1314,17 +1313,11 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
         }
 
         init_console_info(gc, &console, 0);
-
-        need_qemu = libxl__need_xenpv_qemu(gc, 1, &console,
-                d_config->num_vfbs, d_config->vfbs,
-                d_config->num_disks, &d_config->disks[0],
-                d_config->num_channels, &d_config->channels[0]);
-
         console.backend_domid = state->console_domid;
         libxl__device_console_add(gc, domid, &console, state, &device);
         libxl__device_console_dispose(&console);
 
-        if (need_qemu) {
+        if (libxl__need_xenpv_qemu(gc, d_config)) {
             dcs->dmss.dm.guest_domid = domid;
             libxl__spawn_local_dm(egc, &dcs->dmss.dm);
             return;
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 4aca38e..897f3f9 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -2107,61 +2107,34 @@ int libxl__destroy_device_model(libxl__gc *gc, uint32_t domid)
                 GCSPRINTF("/local/domain/%d/image/device-model-pid", domid));
 }
 
-int libxl__need_xenpv_qemu(libxl__gc *gc,
-        int nr_consoles, libxl__device_console *consoles,
-        int nr_vfbs, libxl_device_vfb *vfbs,
-        int nr_disks, libxl_device_disk *disks,
-        int nr_channels, libxl_device_channel *channels)
+int libxl__need_xenpv_qemu(libxl__gc *gc, libxl_domain_config *d_config)
 {
     int i, ret = 0;
     uint32_t domid;
 
-    /*
-     * qemu is required in order to support 2 or more consoles. So switch all
-     * backends to qemu if this is the case
-     */
-    if (nr_consoles > 1) {
-        for (i = 0; i < nr_consoles; i++)
-            consoles[i].consback = LIBXL__CONSOLE_BACKEND_IOEMU;
-        ret = 1;
+    if (libxl__get_domid(gc, &domid))
         goto out;
-    }
 
-    for (i = 0; i < nr_consoles; i++) {
-        if (consoles[i].consback == LIBXL__CONSOLE_BACKEND_IOEMU) {
-            ret = 1;
-            goto out;
-        }
-    }
-
-    if (nr_vfbs > 0) {
+    if (d_config->num_vfbs > 0) {
         ret = 1;
         goto out;
     }
 
-    if (nr_disks > 0) {
-        ret = libxl__get_domid(gc, &domid);
-        if (ret) goto out;
-        for (i = 0; i < nr_disks; i++) {
-            if (disks[i].backend == LIBXL_DISK_BACKEND_QDISK &&
-                disks[i].backend_domid == domid) {
-                ret = 1;
-                goto out;
-            }
+    for (i = 0; i < d_config->num_disks; i++) {
+        if (d_config->disks[i].backend == LIBXL_DISK_BACKEND_QDISK &&
+            d_config->disks[i].backend_domid == domid) {
+            ret = 1;
+            goto out;
         }
     }
 
-    if (nr_channels > 0) {
-        ret = libxl__get_domid(gc, &domid);
-        if (ret) goto out;
-        for (i = 0; i < nr_channels; i++) {
-            if (channels[i].backend_domid == domid) {
-                /* xenconsoled is limited to the first console only.
-                   Until this restriction is removed we must use qemu for
-                   secondary consoles which includes all channels. */
-                ret = 1;
-                goto out;
-            }
+    for (i = 0; i < d_config->num_channels; i++) {
+        if (d_config->channels[i].backend_domid == domid) {
+            /* xenconsoled is limited to the first console only.
+               Until this restriction is removed we must use qemu for
+               secondary consoles which includes all channels. */
+            ret = 1;
+            goto out;
         }
     }
 
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 345a764..fc7bdab 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1616,10 +1616,7 @@ _hidden int libxl__domain_build(libxl__gc *gc,
 _hidden const char *libxl__domain_device_model(libxl__gc *gc,
                                         const libxl_domain_build_info *info);
 _hidden int libxl__need_xenpv_qemu(libxl__gc *gc,
-        int nr_consoles, libxl__device_console *consoles,
-        int nr_vfbs, libxl_device_vfb *vfbs,
-        int nr_disks, libxl_device_disk *disks,
-        int nr_channels, libxl_device_channel *channels);
+                                   libxl_domain_config *d_config);
 
 /*
  * This function will fix reserved device memory conflict
-- 
2.6.2


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v3 1/5] libxl: make libxl__need_xenpv_qemu() operate on domain config

[Wei Liu]

On Wed, Mar 23, 2016 at 01:24:03PM +0100, Juergen Gross wrote:
> libxl__need_xenpv_qemu() is called with configuration data for console,
> vfbs, disks and channels today in order to evaluate the need for
> starting a device model for a pv domain.
> 
> The console data is local to the caller and setup in a way to never
> require a device model. All other data is taken from the domain config
> structure.
> 
> In order to support other device backends via qemu change the interface
> of libxl__need_xenpv_qemu() to take the domain config structure as
> input instead of the single device arrays.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
> V2: Return false if libxl__get_domid() fails as requested by George Dunlap

This is a bug that should be addressed properly -- returning true or
false doesn't make sense in the context of this function.

It would make sense to push calling libxl__get_domid to the sole caller,
then expose a new parameter called backend_id for
libxl__need_xenpv_qemu. A patch like that would also be easily
backportable. What do you think?

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v3 1/5] libxl: make libxl__need_xenpv_qemu() operate on domain config

[Juergen Gross]

On 24/03/16 21:28, Wei Liu wrote:
> On Wed, Mar 23, 2016 at 01:24:03PM +0100, Juergen Gross wrote:
>> libxl__need_xenpv_qemu() is called with configuration data for console,
>> vfbs, disks and channels today in order to evaluate the need for
>> starting a device model for a pv domain.
>>
>> The console data is local to the caller and setup in a way to never
>> require a device model. All other data is taken from the domain config
>> structure.
>>
>> In order to support other device backends via qemu change the interface
>> of libxl__need_xenpv_qemu() to take the domain config structure as
>> input instead of the single device arrays.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>> V2: Return false if libxl__get_domid() fails as requested by George Dunlap
> 
> This is a bug that should be addressed properly -- returning true or
> false doesn't make sense in the context of this function.
> 
> It would make sense to push calling libxl__get_domid to the sole caller,
> then expose a new parameter called backend_id for
> libxl__need_xenpv_qemu. A patch like that would also be easily
> backportable. What do you think?

Hmm, right now there is only one caller. Patch 4 adds another one. I'd
rather return a negative value for an error, and 0 or 1 indicating the
device model isn't or is to be started.

Juergen


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH v3 2/5] libxl: add new pvusb backend "qusb" provided by qemu

[Juergen Gross]

Add a new pvusb backend type "qusb" which is provided by qemu. It can
be selected either by specifying the type directly in the configuration
or it is selected automatically by libxl in case there is no "usbback"
driver loaded.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 docs/man/xl.cfg.pod.5                |  11 +++-
 tools/libxl/libxl_device.c           |   3 +-
 tools/libxl/libxl_dm.c               |   8 +++
 tools/libxl/libxl_internal.h         |   1 +
 tools/libxl/libxl_pvusb.c            | 102 +++++++++++++++++++++++++++--------
 tools/libxl/libxl_types.idl          |   1 +
 tools/libxl/libxl_types_internal.idl |   1 +
 7 files changed, 101 insertions(+), 26 deletions(-)

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index ec739cc..a4cc1b3 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -737,8 +737,15 @@ Possible B<KEY>s are:
 
 =item B<type=TYPE>
 
-Specifies the usb controller type.  Currently only 'pv' and 'auto'
-are supported.
+Specifies the usb controller type.
+
+"pv" denotes a kernel based pvusb backend.
+
+"qusb" specifies a qemu base backend for pvusb.
+
+"auto" (the default) determines whether a kernel based backend is installed.
+If this is the case, "pv" is selected, "qusb" will be selected if no kernel
+backend is currently available.
 
 =item B<version=VERSION>
 
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 4ced9b6..eba3087 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -680,7 +680,8 @@ void libxl__devices_destroy(libxl__egc *egc, libxl__devices_remove_state *drs)
                 aodev->action = LIBXL__DEVICE_ACTION_REMOVE;
                 aodev->dev = dev;
                 aodev->force = drs->force;
-                if (dev->backend_kind == LIBXL__DEVICE_KIND_VUSB)
+                if (dev->backend_kind == LIBXL__DEVICE_KIND_VUSB ||
+                    dev->backend_kind == LIBXL__DEVICE_KIND_QUSB)
                     libxl__initiate_device_usbctrl_remove(egc, aodev);
                 else
                     libxl__initiate_device_generic_remove(egc, aodev);
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 897f3f9..361e584 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -2138,6 +2138,14 @@ int libxl__need_xenpv_qemu(libxl__gc *gc, libxl_domain_config *d_config)
         }
     }
 
+    for (i = 0; i < d_config->num_usbctrls; i++) {
+       if (d_config->usbctrls[i].type == LIBXL_USBCTRL_TYPE_QUSB &&
+           d_config->usbctrls[i].backend_domid == domid) {
+            ret = 1;
+            goto out;
+        }
+    }
+
 out:
     return ret;
 }
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index fc7bdab..2db8b1b 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -487,6 +487,7 @@ typedef struct {
 #define QEMU_BACKEND(dev) (\
     (dev)->backend_kind == LIBXL__DEVICE_KIND_QDISK || \
     (dev)->backend_kind == LIBXL__DEVICE_KIND_VFB || \
+    (dev)->backend_kind == LIBXL__DEVICE_KIND_QUSB || \
     (dev)->backend_kind == LIBXL__DEVICE_KIND_VKBD)
 
 #define XC_PCI_BDF             "0x%x, 0x%x, 0x%x, 0x%x"
diff --git a/tools/libxl/libxl_pvusb.c b/tools/libxl/libxl_pvusb.c
index 5f92628..7200ead 100644
--- a/tools/libxl/libxl_pvusb.c
+++ b/tools/libxl/libxl_pvusb.c
@@ -22,6 +22,21 @@
 
 #define USBHUB_CLASS_CODE 9
 
+static int usbback_is_loaded(libxl__gc *gc)
+{
+    int r;
+    struct stat st;
+
+    r = lstat(SYSFS_USBBACK_DRIVER, &st);
+
+    if (r == 0)
+        return 1;
+    if (r < 0 && errno == ENOENT)
+        return 0;
+    LOGE(ERROR, "Accessing %s", SYSFS_USBBACK_DRIVER);
+    return -1;
+}
+
 static int libxl__device_usbctrl_setdefault(libxl__gc *gc, uint32_t domid,
                                             libxl_device_usbctrl *usbctrl)
 {
@@ -36,7 +51,8 @@ static int libxl__device_usbctrl_setdefault(libxl__gc *gc, uint32_t domid,
 
     if (usbctrl->type == LIBXL_USBCTRL_TYPE_AUTO) {
         if (domtype == LIBXL_DOMAIN_TYPE_PV) {
-            usbctrl->type = LIBXL_USBCTRL_TYPE_PV;
+            usbctrl->type = usbback_is_loaded(gc) ? LIBXL_USBCTRL_TYPE_PV
+                                                  : LIBXL_USBCTRL_TYPE_QUSB;
         } else if (domtype == LIBXL_DOMAIN_TYPE_HVM) {
             /* FIXME: See if we can detect PV frontend */
             usbctrl->type = LIBXL_USBCTRL_TYPE_DEVICEMODEL;
@@ -54,7 +70,9 @@ int libxl__device_from_usbctrl(libxl__gc *gc, uint32_t domid,
 {
     device->backend_devid   = usbctrl->devid;
     device->backend_domid   = usbctrl->backend_domid;
-    device->backend_kind    = LIBXL__DEVICE_KIND_VUSB;
+    device->backend_kind    = (usbctrl->type == LIBXL_USBCTRL_TYPE_PV)
+                              ? LIBXL__DEVICE_KIND_VUSB
+                              : LIBXL__DEVICE_KIND_QUSB;
     device->devid           = usbctrl->devid;
     device->domid           = domid;
     device->kind            = LIBXL__DEVICE_KIND_VUSB;
@@ -64,9 +82,9 @@ int libxl__device_from_usbctrl(libxl__gc *gc, uint32_t domid,
 
 /* Add usbctrl information to xenstore.
  *
- * Adding a usb controller will add a new 'vusb' device in xenstore, and
- * add corresponding frontend, backend information to it. According to
- * "update_json", decide wether to update json config file.
+ * Adding a usb controller will add a new 'qusb' or 'vusb' device in xenstore,
+ * and add corresponding frontend, backend information to it. According to
+ * "update_json", decide whether to update json config file.
  */
 static int libxl__device_usbctrl_add_xenstore(libxl__gc *gc, uint32_t domid,
                                               libxl_device_usbctrl *usbctrl,
@@ -159,6 +177,18 @@ out:
     return rc;
 }
 
+static char *pvusb_get_device_type(libxl_usbctrl_type type)
+{
+    switch (type) {
+    case LIBXL_USBCTRL_TYPE_PV:
+        return "vusb";
+    case LIBXL_USBCTRL_TYPE_QUSB:
+        return "qusb";
+    default:
+        return NULL;
+    }
+}
+
 /* AO operation to add a usb controller.
  *
  * Generally, it does:
@@ -190,7 +220,8 @@ void libxl__device_usbctrl_add(libxl__egc *egc, uint32_t domid,
         }
     }
 
-    if (usbctrl->type != LIBXL_USBCTRL_TYPE_PV) {
+    if (usbctrl->type != LIBXL_USBCTRL_TYPE_PV &&
+        usbctrl->type != LIBXL_USBCTRL_TYPE_QUSB) {
         LOG(ERROR, "Unsupported USB controller type");
         rc = ERROR_FAIL;
         goto out;
@@ -252,7 +283,8 @@ void libxl__initiate_device_usbctrl_remove(libxl__egc *egc,
     rc = libxl_device_usbctrl_getinfo(CTX, domid, &usbctrl, &usbctrlinfo);
     if (rc) goto out;
 
-    if (usbctrlinfo.type != LIBXL_USBCTRL_TYPE_PV) {
+    if (usbctrlinfo.type != LIBXL_USBCTRL_TYPE_PV &&
+        usbctrlinfo.type != LIBXL_USBCTRL_TYPE_QUSB) {
         LOG(ERROR, "Unsupported USB controller type");
         rc = ERROR_FAIL;
         goto out;
@@ -293,6 +325,7 @@ static const char *vusb_be_from_xs_fe(libxl__gc *gc, const char *fe_path,
     const char *be_path;
     int r;
     uint32_t be_domid, fe_domid;
+    char be_type[16];
 
     r = libxl__xs_read_checked(gc, XBT_NULL, GCSPRINTF("%s/backend", fe_path),
                                &be_path);
@@ -300,10 +333,10 @@ static const char *vusb_be_from_xs_fe(libxl__gc *gc, const char *fe_path,
 
     /* Check to see that it has the proper form, and that fe_domid ==
      * target domid */
-    r = sscanf(be_path, "/local/domain/%d/backend/vusb/%d",
-               &be_domid, &fe_domid);
+    r = sscanf(be_path, "/local/domain/%d/backend/%15[^/]/%d",
+               &be_domid, be_type, &fe_domid);
 
-    if (r != 2 || fe_domid != tgt_domid) {
+    if (r != 3 || fe_domid != tgt_domid) {
         LOG(ERROR, "Malformed backend, refusing to use");
         return NULL;
     }
@@ -740,8 +773,9 @@ libxl__device_usbdev_set_default_usbctrl(libxl__gc *gc, uint32_t domid,
         for (j = 0; j < usbctrls[i].ports; j++) {
             const char *path, *tmp;
 
-            path = GCSPRINTF("%s/backend/vusb/%d/%d/port/%d",
+            path = GCSPRINTF("%s/backend/%s/%d/%d/port/%d",
                              libxl__xs_get_dompath(gc, LIBXL_TOOLSTACK_DOMID),
+                             pvusb_get_device_type(usbctrls[i].type),
                              domid, usbctrls[i].devid, j + 1);
             rc = libxl__xs_read_checked(gc, XBT_NULL, path, &tmp);
             if (rc) goto out;
@@ -883,11 +917,12 @@ out:
 
 /* Add usb information to xenstore
  *
- * Adding a usb device won't create new 'vusb' device, but only write
+ * Adding a usb device won't create new 'qusb'/'vusb' device, but only write
  * the device busid to the controller:port in xenstore.
  */
 static int libxl__device_usbdev_add_xenstore(libxl__gc *gc, uint32_t domid,
                                              libxl_device_usbdev *usbdev,
+                                             libxl_usbctrl_type type,
                                              bool update_json)
 {
     char *be_path, *busid;
@@ -931,8 +966,9 @@ static int libxl__device_usbdev_add_xenstore(libxl__gc *gc, uint32_t domid,
             if (rc) goto out;
         }
 
-        be_path = GCSPRINTF("%s/backend/vusb/%d/%d/port/%d",
+        be_path = GCSPRINTF("%s/backend/%s/%d/%d/port/%d",
                             libxl__xs_get_dompath(gc, LIBXL_TOOLSTACK_DOMID),
+                            pvusb_get_device_type(type),
                             domid, usbdev->ctrl, usbdev->port);
 
         LOG(DEBUG, "Adding usb device %s to xenstore: controller %d, port %d",
@@ -956,12 +992,14 @@ out:
 }
 
 static int libxl__device_usbdev_remove_xenstore(libxl__gc *gc, uint32_t domid,
-                                                libxl_device_usbdev *usbdev)
+                                                libxl_device_usbdev *usbdev,
+                                                libxl_usbctrl_type type)
 {
     char *be_path;
 
-    be_path = GCSPRINTF("%s/backend/vusb/%d/%d/port/%d",
+    be_path = GCSPRINTF("%s/backend/%s/%d/%d/port/%d",
                         libxl__xs_get_dompath(gc, LIBXL_TOOLSTACK_DOMID),
+                        pvusb_get_device_type(type),
                         domid, usbdev->ctrl, usbdev->port);
 
     LOG(DEBUG, "Removing usb device from xenstore: controller %d, port %d",
@@ -971,12 +1009,14 @@ static int libxl__device_usbdev_remove_xenstore(libxl__gc *gc, uint32_t domid,
 }
 
 static char *usbdev_busid_from_ctrlport(libxl__gc *gc, uint32_t domid,
-                                        libxl_device_usbdev *usbdev)
+                                        libxl_device_usbdev *usbdev,
+                                        libxl_usbctrl_type type)
 {
     return libxl__xs_read(gc, XBT_NULL,
-                          GCSPRINTF("%s/backend/vusb/%d/%d/port/%d",
+                          GCSPRINTF("%s/backend/%s/%d/%d/port/%d",
                               libxl__xs_get_dompath(gc, LIBXL_TOOLSTACK_DOMID),
-                          domid, usbdev->ctrl, usbdev->port));
+                              pvusb_get_device_type(type),
+                              domid, usbdev->ctrl, usbdev->port));
 }
 
 /* get original driver path of usb interface, stored in @drvpath */
@@ -1333,15 +1373,25 @@ static int do_usbdev_add(libxl__gc *gc, uint32_t domid,
             goto out;
         }
 
-        rc = libxl__device_usbdev_add_xenstore(gc, domid, usbdev, update_json);
+        rc = libxl__device_usbdev_add_xenstore(gc, domid, usbdev,
+                                               LIBXL_USBCTRL_TYPE_PV,
+                                               update_json);
         if (rc) goto out;
 
         rc = usbback_dev_assign(gc, busid);
         if (rc) {
-            libxl__device_usbdev_remove_xenstore(gc, domid, usbdev);
+            libxl__device_usbdev_remove_xenstore(gc, domid, usbdev,
+                                                 LIBXL_USBCTRL_TYPE_PV);
             goto out;
         }
         break;
+    case LIBXL_USBCTRL_TYPE_QUSB:
+        rc = libxl__device_usbdev_add_xenstore(gc, domid, usbdev,
+                                               LIBXL_USBCTRL_TYPE_QUSB,
+                                               update_json);
+        if (rc) goto out;
+
+        break;
     case LIBXL_USBCTRL_TYPE_DEVICEMODEL:
     default:
         LOG(ERROR, "Unsupported usb controller type");
@@ -1458,7 +1508,7 @@ static int do_usbdev_remove(libxl__gc *gc, uint32_t domid,
 
     switch (usbctrlinfo.type) {
     case LIBXL_USBCTRL_TYPE_PV:
-        busid = usbdev_busid_from_ctrlport(gc, domid, usbdev);
+        busid = usbdev_busid_from_ctrlport(gc, domid, usbdev, usbctrlinfo.type);
         if (!busid) {
             rc = ERROR_FAIL;
             goto out;
@@ -1483,7 +1533,8 @@ static int do_usbdev_remove(libxl__gc *gc, uint32_t domid,
             goto out;
         }
 
-        rc = libxl__device_usbdev_remove_xenstore(gc, domid, usbdev);
+        rc = libxl__device_usbdev_remove_xenstore(gc, domid, usbdev,
+                                                  LIBXL_USBCTRL_TYPE_PV);
         if (rc) {
             LOG(ERROR, "Error removing device from guest."
                 " Try running usbdev-detach again.");
@@ -1499,6 +1550,12 @@ static int do_usbdev_remove(libxl__gc *gc, uint32_t domid,
         }
 
         break;
+    case LIBXL_USBCTRL_TYPE_QUSB:
+        rc = libxl__device_usbdev_remove_xenstore(gc, domid, usbdev,
+                                                  LIBXL_USBCTRL_TYPE_QUSB);
+        if (rc) goto out;
+
+        break;
     case LIBXL_USBCTRL_TYPE_DEVICEMODEL:
     default:
         LOG(ERROR, "Unsupported usb controller type");
@@ -1583,7 +1640,6 @@ int libxl_ctrlport_to_device_usbdev(libxl_ctx *ctx,
     dompath = libxl__xs_get_dompath(gc, domid);
 
     fe_path = GCSPRINTF("%s/device/vusb/%d", dompath, ctrl);
-
     be_path = vusb_be_from_xs_fe(gc, fe_path, domid);
     if (!be_path) {
         rc = ERROR_FAIL;
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 59b183c..304aa11 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -618,6 +618,7 @@ libxl_usbctrl_type = Enumeration("usbctrl_type", [
     (0, "AUTO"),
     (1, "PV"),
     (2, "DEVICEMODEL"),
+    (3, "QUSB"),
     ])
 
 libxl_usbdev_type = Enumeration("usbdev_type", [
diff --git a/tools/libxl/libxl_types_internal.idl b/tools/libxl/libxl_types_internal.idl
index 696f5f8..177f9b7 100644
--- a/tools/libxl/libxl_types_internal.idl
+++ b/tools/libxl/libxl_types_internal.idl
@@ -23,6 +23,7 @@ libxl__device_kind = Enumeration("device_kind", [
     (7, "CONSOLE"),
     (8, "VTPM"),
     (9, "VUSB"),
+    (10, "QUSB"),
     ])
 
 libxl__console_backend = Enumeration("console_backend", [
-- 
2.6.2


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v3 2/5] libxl: add new pvusb backend "qusb" provided by qemu

[Chun Yan Liu]

>>> On 3/23/2016 at 08:24 PM, in message
<1458735847-9448-3-git-send-email-jgross@suse.com>, Juergen Gross
<jgross@suse.com> wrote: 
> Add a new pvusb backend type "qusb" which is provided by qemu. It can 
> be selected either by specifying the type directly in the configuration 
> or it is selected automatically by libxl in case there is no "usbback" 
> driver loaded. 
>  
> Signed-off-by: Juergen Gross <jgross@suse.com> 
> --- 
>  docs/man/xl.cfg.pod.5                |  11 +++- 
>  tools/libxl/libxl_device.c           |   3 +- 
>  tools/libxl/libxl_dm.c               |   8 +++ 
>  tools/libxl/libxl_internal.h         |   1 + 
>  tools/libxl/libxl_pvusb.c            | 102 +++++++++++++++++++++++++++-------- 
>  tools/libxl/libxl_types.idl          |   1 + 
>  tools/libxl/libxl_types_internal.idl |   1 + 
>  7 files changed, 101 insertions(+), 26 deletions(-) 
>  
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5 
> index ec739cc..a4cc1b3 100644 
> --- a/docs/man/xl.cfg.pod.5 
> +++ b/docs/man/xl.cfg.pod.5 
> @@ -737,8 +737,15 @@ Possible B<KEY>s are: 
>   
>  =item B<type=TYPE> 
>   
> -Specifies the usb controller type.  Currently only 'pv' and 'auto' 
> -are supported. 
> +Specifies the usb controller type. 
> + 
> +"pv" denotes a kernel based pvusb backend. 
> + 
> +"qusb" specifies a qemu base backend for pvusb. 
> + 
> +"auto" (the default) determines whether a kernel based backend is  
> installed. 
> +If this is the case, "pv" is selected, "qusb" will be selected if no kernel 
> +backend is currently available. 
>   
>  =item B<version=VERSION> 
>   
> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c 
> index 4ced9b6..eba3087 100644 
> --- a/tools/libxl/libxl_device.c 
> +++ b/tools/libxl/libxl_device.c 
> @@ -680,7 +680,8 @@ void libxl__devices_destroy(libxl__egc *egc,  
> libxl__devices_remove_state *drs) 
>                  aodev->action = LIBXL__DEVICE_ACTION_REMOVE; 
>                  aodev->dev = dev; 
>                  aodev->force = drs->force; 
> -                if (dev->backend_kind == LIBXL__DEVICE_KIND_VUSB) 
> +                if (dev->backend_kind == LIBXL__DEVICE_KIND_VUSB || 
> +                    dev->backend_kind == LIBXL__DEVICE_KIND_QUSB) 
>                      libxl__initiate_device_usbctrl_remove(egc, aodev); 
>                  else 
>                      libxl__initiate_device_generic_remove(egc, aodev); 
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c 
> index 897f3f9..361e584 100644 
> --- a/tools/libxl/libxl_dm.c 
> +++ b/tools/libxl/libxl_dm.c 
> @@ -2138,6 +2138,14 @@ int libxl__need_xenpv_qemu(libxl__gc *gc,  
> libxl_domain_config *d_config) 
>          } 
>      } 
>   
> +    for (i = 0; i < d_config->num_usbctrls; i++) { 
> +       if (d_config->usbctrls[i].type == LIBXL_USBCTRL_TYPE_QUSB && 
> +           d_config->usbctrls[i].backend_domid == domid) { 
> +            ret = 1; 
> +            goto out; 
> +        } 
> +    } 
> + 
>  out: 
>      return ret; 
>  } 
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h 
> index fc7bdab..2db8b1b 100644 
> --- a/tools/libxl/libxl_internal.h 
> +++ b/tools/libxl/libxl_internal.h 
> @@ -487,6 +487,7 @@ typedef struct { 
>  #define QEMU_BACKEND(dev) (\ 
>      (dev)->backend_kind == LIBXL__DEVICE_KIND_QDISK || \ 
>      (dev)->backend_kind == LIBXL__DEVICE_KIND_VFB || \ 
> +    (dev)->backend_kind == LIBXL__DEVICE_KIND_QUSB || \ 
>      (dev)->backend_kind == LIBXL__DEVICE_KIND_VKBD) 
>   
>  #define XC_PCI_BDF             "0x%x, 0x%x, 0x%x, 0x%x" 
> diff --git a/tools/libxl/libxl_pvusb.c b/tools/libxl/libxl_pvusb.c 
> index 5f92628..7200ead 100644 
> --- a/tools/libxl/libxl_pvusb.c 
> +++ b/tools/libxl/libxl_pvusb.c 
> @@ -22,6 +22,21 @@ 
>   
>  #define USBHUB_CLASS_CODE 9 
>   
> +static int usbback_is_loaded(libxl__gc *gc) 
> +{ 
> +    int r; 
> +    struct stat st; 
> + 
> +    r = lstat(SYSFS_USBBACK_DRIVER, &st); 
> + 
> +    if (r == 0) 
> +        return 1; 
> +    if (r < 0 && errno == ENOENT) 
> +        return 0; 
> +    LOGE(ERROR, "Accessing %s", SYSFS_USBBACK_DRIVER); 
> +    return -1; 
> +} 
> + 
>  static int libxl__device_usbctrl_setdefault(libxl__gc *gc, uint32_t domid, 
>                                              libxl_device_usbctrl *usbctrl) 
>  { 
> @@ -36,7 +51,8 @@ static int libxl__device_usbctrl_setdefault(libxl__gc *gc,  
> uint32_t domid, 
>   
>      if (usbctrl->type == LIBXL_USBCTRL_TYPE_AUTO) { 
>          if (domtype == LIBXL_DOMAIN_TYPE_PV) { 
> -            usbctrl->type = LIBXL_USBCTRL_TYPE_PV; 
> +            usbctrl->type = usbback_is_loaded(gc) ? LIBXL_USBCTRL_TYPE_PV 
The condition should be (usbback_is_loaded(gc) > 0)?
usbback_is_loaded(gc) < 0 means lstat error, cannot determine if the
usbback driver is loaded.

Otherwise, it looks good!

-Chunyan

> +                                                  :  
> LIBXL_USBCTRL_TYPE_QUSB; 
>          } else if (domtype == LIBXL_DOMAIN_TYPE_HVM) { 
>              /* FIXME: See if we can detect PV frontend */ 
>              usbctrl->type = LIBXL_USBCTRL_TYPE_DEVICEMODEL; 
> @@ -54,7 +70,9 @@ int libxl__device_from_usbctrl(libxl__gc *gc, uint32_t  
> domid, 
>  { 
>      device->backend_devid   = usbctrl->devid; 
>      device->backend_domid   = usbctrl->backend_domid; 
> -    device->backend_kind    = LIBXL__DEVICE_KIND_VUSB; 
> +    device->backend_kind    = (usbctrl->type == LIBXL_USBCTRL_TYPE_PV) 
> +                              ? LIBXL__DEVICE_KIND_VUSB 
> +                              : LIBXL__DEVICE_KIND_QUSB; 
>      device->devid           = usbctrl->devid; 
>      device->domid           = domid; 
>      device->kind            = LIBXL__DEVICE_KIND_VUSB; 
> @@ -64,9 +82,9 @@ int libxl__device_from_usbctrl(libxl__gc *gc, uint32_t  
> domid, 
>   
>  /* Add usbctrl information to xenstore. 
>   * 
> - * Adding a usb controller will add a new 'vusb' device in xenstore, and 
> - * add corresponding frontend, backend information to it. According to 
> - * "update_json", decide wether to update json config file. 
> + * Adding a usb controller will add a new 'qusb' or 'vusb' device in  
> xenstore, 
> + * and add corresponding frontend, backend information to it. According to 
> + * "update_json", decide whether to update json config file. 
>   */ 
>  static int libxl__device_usbctrl_add_xenstore(libxl__gc *gc, uint32_t  
> domid, 
>                                                libxl_device_usbctrl  
> *usbctrl, 
> @@ -159,6 +177,18 @@ out: 
>      return rc; 
>  } 
>   
> +static char *pvusb_get_device_type(libxl_usbctrl_type type) 
> +{ 
> +    switch (type) { 
> +    case LIBXL_USBCTRL_TYPE_PV: 
> +        return "vusb"; 
> +    case LIBXL_USBCTRL_TYPE_QUSB: 
> +        return "qusb"; 
> +    default: 
> +        return NULL; 
> +    } 
> +} 
> + 
>  /* AO operation to add a usb controller. 
>   * 
>   * Generally, it does: 
> @@ -190,7 +220,8 @@ void libxl__device_usbctrl_add(libxl__egc *egc, uint32_t  
> domid, 
>          } 
>      } 
>   
> -    if (usbctrl->type != LIBXL_USBCTRL_TYPE_PV) { 
> +    if (usbctrl->type != LIBXL_USBCTRL_TYPE_PV && 
> +        usbctrl->type != LIBXL_USBCTRL_TYPE_QUSB) { 
>          LOG(ERROR, "Unsupported USB controller type"); 
>          rc = ERROR_FAIL; 
>          goto out; 
> @@ -252,7 +283,8 @@ void libxl__initiate_device_usbctrl_remove(libxl__egc  
> *egc, 
>      rc = libxl_device_usbctrl_getinfo(CTX, domid, &usbctrl, &usbctrlinfo); 
>      if (rc) goto out; 
>   
> -    if (usbctrlinfo.type != LIBXL_USBCTRL_TYPE_PV) { 
> +    if (usbctrlinfo.type != LIBXL_USBCTRL_TYPE_PV && 
> +        usbctrlinfo.type != LIBXL_USBCTRL_TYPE_QUSB) { 
>          LOG(ERROR, "Unsupported USB controller type"); 
>          rc = ERROR_FAIL; 
>          goto out; 
> @@ -293,6 +325,7 @@ static const char *vusb_be_from_xs_fe(libxl__gc *gc,  
> const char *fe_path, 
>      const char *be_path; 
>      int r; 
>      uint32_t be_domid, fe_domid; 
> +    char be_type[16]; 
>   
>      r = libxl__xs_read_checked(gc, XBT_NULL, GCSPRINTF("%s/backend",  
> fe_path), 
>                                 &be_path); 
> @@ -300,10 +333,10 @@ static const char *vusb_be_from_xs_fe(libxl__gc *gc,  
> const char *fe_path, 
>   
>      /* Check to see that it has the proper form, and that fe_domid == 
>       * target domid */ 
> -    r = sscanf(be_path, "/local/domain/%d/backend/vusb/%d", 
> -               &be_domid, &fe_domid); 
> +    r = sscanf(be_path, "/local/domain/%d/backend/%15[^/]/%d", 
> +               &be_domid, be_type, &fe_domid); 
>   
> -    if (r != 2 || fe_domid != tgt_domid) { 
> +    if (r != 3 || fe_domid != tgt_domid) { 
>          LOG(ERROR, "Malformed backend, refusing to use"); 
>          return NULL; 
>      } 
> @@ -740,8 +773,9 @@ libxl__device_usbdev_set_default_usbctrl(libxl__gc *gc,  
> uint32_t domid, 
>          for (j = 0; j < usbctrls[i].ports; j++) { 
>              const char *path, *tmp; 
>   
> -            path = GCSPRINTF("%s/backend/vusb/%d/%d/port/%d", 
> +            path = GCSPRINTF("%s/backend/%s/%d/%d/port/%d", 
>                               libxl__xs_get_dompath(gc,  
> LIBXL_TOOLSTACK_DOMID), 
> +                             pvusb_get_device_type(usbctrls[i].type), 
>                               domid, usbctrls[i].devid, j + 1); 
>              rc = libxl__xs_read_checked(gc, XBT_NULL, path, &tmp); 
>              if (rc) goto out; 
> @@ -883,11 +917,12 @@ out: 
>   
>  /* Add usb information to xenstore 
>   * 
> - * Adding a usb device won't create new 'vusb' device, but only write 
> + * Adding a usb device won't create new 'qusb'/'vusb' device, but only  
> write 
>   * the device busid to the controller:port in xenstore. 
>   */ 
>  static int libxl__device_usbdev_add_xenstore(libxl__gc *gc, uint32_t domid, 
>                                               libxl_device_usbdev *usbdev, 
> +                                             libxl_usbctrl_type type, 
>                                               bool update_json) 
>  { 
>      char *be_path, *busid; 
> @@ -931,8 +966,9 @@ static int libxl__device_usbdev_add_xenstore(libxl__gc  
> *gc, uint32_t domid, 
>              if (rc) goto out; 
>          } 
>   
> -        be_path = GCSPRINTF("%s/backend/vusb/%d/%d/port/%d", 
> +        be_path = GCSPRINTF("%s/backend/%s/%d/%d/port/%d", 
>                              libxl__xs_get_dompath(gc,  
> LIBXL_TOOLSTACK_DOMID), 
> +                            pvusb_get_device_type(type), 
>                              domid, usbdev->ctrl, usbdev->port); 
>   
>          LOG(DEBUG, "Adding usb device %s to xenstore: controller %d, port  
> %d", 
> @@ -956,12 +992,14 @@ out: 
>  } 
>   
>  static int libxl__device_usbdev_remove_xenstore(libxl__gc *gc, uint32_t  
> domid, 
> -                                                libxl_device_usbdev *usbdev) 
> +                                                libxl_device_usbdev  
> *usbdev, 
> +                                                libxl_usbctrl_type type) 
>  { 
>      char *be_path; 
>   
> -    be_path = GCSPRINTF("%s/backend/vusb/%d/%d/port/%d", 
> +    be_path = GCSPRINTF("%s/backend/%s/%d/%d/port/%d", 
>                          libxl__xs_get_dompath(gc, LIBXL_TOOLSTACK_DOMID), 
> +                        pvusb_get_device_type(type), 
>                          domid, usbdev->ctrl, usbdev->port); 
>   
>      LOG(DEBUG, "Removing usb device from xenstore: controller %d, port %d", 
> @@ -971,12 +1009,14 @@ static int 
> libxl__device_usbdev_remove_xenstore(libxl__gc *gc, uint32_t domid, 
>  } 
>   
>  static char *usbdev_busid_from_ctrlport(libxl__gc *gc, uint32_t domid, 
> -                                        libxl_device_usbdev *usbdev) 
> +                                        libxl_device_usbdev *usbdev, 
> +                                        libxl_usbctrl_type type) 
>  { 
>      return libxl__xs_read(gc, XBT_NULL, 
> -                          GCSPRINTF("%s/backend/vusb/%d/%d/port/%d", 
> +                          GCSPRINTF("%s/backend/%s/%d/%d/port/%d", 
>                                libxl__xs_get_dompath(gc,  
> LIBXL_TOOLSTACK_DOMID), 
> -                          domid, usbdev->ctrl, usbdev->port)); 
> +                              pvusb_get_device_type(type), 
> +                              domid, usbdev->ctrl, usbdev->port)); 
>  } 
>   
>  /* get original driver path of usb interface, stored in @drvpath */ 
> @@ -1333,15 +1373,25 @@ static int do_usbdev_add(libxl__gc *gc, uint32_t  
> domid, 
>              goto out; 
>          } 
>   
> -        rc = libxl__device_usbdev_add_xenstore(gc, domid, usbdev,  
> update_json); 
> +        rc = libxl__device_usbdev_add_xenstore(gc, domid, usbdev, 
> +                                               LIBXL_USBCTRL_TYPE_PV, 
> +                                               update_json); 
>          if (rc) goto out; 
>   
>          rc = usbback_dev_assign(gc, busid); 
>          if (rc) { 
> -            libxl__device_usbdev_remove_xenstore(gc, domid, usbdev); 
> +            libxl__device_usbdev_remove_xenstore(gc, domid, usbdev, 
> +                                                 LIBXL_USBCTRL_TYPE_PV); 
>              goto out; 
>          } 
>          break; 
> +    case LIBXL_USBCTRL_TYPE_QUSB: 
> +        rc = libxl__device_usbdev_add_xenstore(gc, domid, usbdev, 
> +                                               LIBXL_USBCTRL_TYPE_QUSB, 
> +                                               update_json); 
> +        if (rc) goto out; 
> + 
> +        break; 
>      case LIBXL_USBCTRL_TYPE_DEVICEMODEL: 
>      default: 
>          LOG(ERROR, "Unsupported usb controller type"); 
> @@ -1458,7 +1508,7 @@ static int do_usbdev_remove(libxl__gc *gc, uint32_t  
> domid, 
>   
>      switch (usbctrlinfo.type) { 
>      case LIBXL_USBCTRL_TYPE_PV: 
> -        busid = usbdev_busid_from_ctrlport(gc, domid, usbdev); 
> +        busid = usbdev_busid_from_ctrlport(gc, domid, usbdev,  
> usbctrlinfo.type); 
>          if (!busid) { 
>              rc = ERROR_FAIL; 
>              goto out; 
> @@ -1483,7 +1533,8 @@ static int do_usbdev_remove(libxl__gc *gc, uint32_t  
> domid, 
>              goto out; 
>          } 
>   
> -        rc = libxl__device_usbdev_remove_xenstore(gc, domid, usbdev); 
> +        rc = libxl__device_usbdev_remove_xenstore(gc, domid, usbdev, 
> +                                                  LIBXL_USBCTRL_TYPE_PV); 
>          if (rc) { 
>              LOG(ERROR, "Error removing device from guest." 
>                  " Try running usbdev-detach again."); 
> @@ -1499,6 +1550,12 @@ static int do_usbdev_remove(libxl__gc *gc, uint32_t  
> domid, 
>          } 
>   
>          break; 
> +    case LIBXL_USBCTRL_TYPE_QUSB: 
> +        rc = libxl__device_usbdev_remove_xenstore(gc, domid, usbdev, 
> +                                                  LIBXL_USBCTRL_TYPE_QUSB); 
> +        if (rc) goto out; 
> + 
> +        break; 
>      case LIBXL_USBCTRL_TYPE_DEVICEMODEL: 
>      default: 
>          LOG(ERROR, "Unsupported usb controller type"); 
> @@ -1583,7 +1640,6 @@ int libxl_ctrlport_to_device_usbdev(libxl_ctx *ctx, 
>      dompath = libxl__xs_get_dompath(gc, domid); 
>   
>      fe_path = GCSPRINTF("%s/device/vusb/%d", dompath, ctrl); 
> - 
>      be_path = vusb_be_from_xs_fe(gc, fe_path, domid); 
>      if (!be_path) { 
>          rc = ERROR_FAIL; 
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl 
> index 59b183c..304aa11 100644 
> --- a/tools/libxl/libxl_types.idl 
> +++ b/tools/libxl/libxl_types.idl 
> @@ -618,6 +618,7 @@ libxl_usbctrl_type = Enumeration("usbctrl_type", [ 
>      (0, "AUTO"), 
>      (1, "PV"), 
>      (2, "DEVICEMODEL"), 
> +    (3, "QUSB"), 
>      ]) 
>   
>  libxl_usbdev_type = Enumeration("usbdev_type", [ 
> diff --git a/tools/libxl/libxl_types_internal.idl  
> b/tools/libxl/libxl_types_internal.idl 
> index 696f5f8..177f9b7 100644 
> --- a/tools/libxl/libxl_types_internal.idl 
> +++ b/tools/libxl/libxl_types_internal.idl 
> @@ -23,6 +23,7 @@ libxl__device_kind = Enumeration("device_kind", [ 
>      (7, "CONSOLE"), 
>      (8, "VTPM"), 
>      (9, "VUSB"), 
> +    (10, "QUSB"), 
>      ]) 
>   
>  libxl__console_backend = Enumeration("console_backend", [ 
 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v3 2/5] libxl: add new pvusb backend "qusb" provided by qemu

[Juergen Gross]

On 25/03/16 03:23, Chun Yan Liu wrote:
> 
> 
>>>> On 3/23/2016 at 08:24 PM, in message
> <1458735847-9448-3-git-send-email-jgross@suse.com>, Juergen Gross
> <jgross@suse.com> wrote: 
>> Add a new pvusb backend type "qusb" which is provided by qemu. It can 
>> be selected either by specifying the type directly in the configuration 
>> or it is selected automatically by libxl in case there is no "usbback" 
>> driver loaded. 
>>  
>> Signed-off-by: Juergen Gross <jgross@suse.com> 
>> --- 
>>  docs/man/xl.cfg.pod.5                |  11 +++- 
>>  tools/libxl/libxl_device.c           |   3 +- 
>>  tools/libxl/libxl_dm.c               |   8 +++ 
>>  tools/libxl/libxl_internal.h         |   1 + 
>>  tools/libxl/libxl_pvusb.c            | 102 +++++++++++++++++++++++++++-------- 
>>  tools/libxl/libxl_types.idl          |   1 + 
>>  tools/libxl/libxl_types_internal.idl |   1 + 
>>  7 files changed, 101 insertions(+), 26 deletions(-) 
>>  
>> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h 
>> index fc7bdab..2db8b1b 100644 
>> --- a/tools/libxl/libxl_internal.h 
>> +++ b/tools/libxl/libxl_internal.h 
>> @@ -22,6 +22,21 @@ 
>>   
>>  #define USBHUB_CLASS_CODE 9 
>>   
>> +static int usbback_is_loaded(libxl__gc *gc) 
>> +{ 
>> +    int r; 
>> +    struct stat st; 
>> + 
>> +    r = lstat(SYSFS_USBBACK_DRIVER, &st); 
>> + 
>> +    if (r == 0) 
>> +        return 1; 
>> +    if (r < 0 && errno == ENOENT) 
>> +        return 0; 
>> +    LOGE(ERROR, "Accessing %s", SYSFS_USBBACK_DRIVER); 
>> +    return -1; 
>> +} 
>> + 
>>  static int libxl__device_usbctrl_setdefault(libxl__gc *gc, uint32_t domid, 
>>                                              libxl_device_usbctrl *usbctrl) 
>>  { 
>> @@ -36,7 +51,8 @@ static int libxl__device_usbctrl_setdefault(libxl__gc *gc,  
>> uint32_t domid, 
>>   
>>      if (usbctrl->type == LIBXL_USBCTRL_TYPE_AUTO) { 
>>          if (domtype == LIBXL_DOMAIN_TYPE_PV) { 
>> -            usbctrl->type = LIBXL_USBCTRL_TYPE_PV; 
>> +            usbctrl->type = usbback_is_loaded(gc) ? LIBXL_USBCTRL_TYPE_PV 
> The condition should be (usbback_is_loaded(gc) > 0)?
> usbback_is_loaded(gc) < 0 means lstat error, cannot determine if the
> usbback driver is loaded.

Good point. I think in error case I should rather abort the operation.

Thoughts?


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v3 2/5] libxl: add new pvusb backend "qusb" provided by qemu

[Chun Yan Liu]

>>> On 3/25/2016 at 02:29 PM, in message <56F4DADE.4040200@suse.com>, Juergen Gross
<jgross@suse.com> wrote: 
> On 25/03/16 03:23, Chun Yan Liu wrote: 
> >  
> >  
> >>>> On 3/23/2016 at 08:24 PM, in message 
> > <1458735847-9448-3-git-send-email-jgross@suse.com>, Juergen Gross 
> > <jgross@suse.com> wrote:  
> >> Add a new pvusb backend type "qusb" which is provided by qemu. It can  
> >> be selected either by specifying the type directly in the configuration  
> >> or it is selected automatically by libxl in case there is no "usbback"  
> >> driver loaded.  
> >>   
> >> Signed-off-by: Juergen Gross <jgross@suse.com>  
> >> ---  
> >>  docs/man/xl.cfg.pod.5                |  11 +++-  
> >>  tools/libxl/libxl_device.c           |   3 +-  
> >>  tools/libxl/libxl_dm.c               |   8 +++  
> >>  tools/libxl/libxl_internal.h         |   1 +  
> >>  tools/libxl/libxl_pvusb.c            | 102  
> +++++++++++++++++++++++++++--------  
> >>  tools/libxl/libxl_types.idl          |   1 +  
> >>  tools/libxl/libxl_types_internal.idl |   1 +  
> >>  7 files changed, 101 insertions(+), 26 deletions(-)  
> >>   
> >> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h  
> >> index fc7bdab..2db8b1b 100644  
> >> --- a/tools/libxl/libxl_internal.h  
> >> +++ b/tools/libxl/libxl_internal.h  
> >> @@ -22,6 +22,21 @@  
> >>    
> >>  #define USBHUB_CLASS_CODE 9  
> >>    
> >> +static int usbback_is_loaded(libxl__gc *gc)  
> >> +{  
> >> +    int r;  
> >> +    struct stat st;  
> >> +  
> >> +    r = lstat(SYSFS_USBBACK_DRIVER, &st);  
> >> +  
> >> +    if (r == 0)  
> >> +        return 1;  
> >> +    if (r < 0 && errno == ENOENT)  
> >> +        return 0;  
> >> +    LOGE(ERROR, "Accessing %s", SYSFS_USBBACK_DRIVER);  
> >> +    return -1;  
> >> +}  
> >> +  
> >>  static int libxl__device_usbctrl_setdefault(libxl__gc *gc, uint32_t domid,  
>  
> >>                                              libxl_device_usbctrl *usbctrl)  
>  
> >>  {  
> >> @@ -36,7 +51,8 @@ static int libxl__device_usbctrl_setdefault(libxl__gc  
> *gc,   
> >> uint32_t domid,  
> >>    
> >>      if (usbctrl->type == LIBXL_USBCTRL_TYPE_AUTO) {  
> >>          if (domtype == LIBXL_DOMAIN_TYPE_PV) {  
> >> -            usbctrl->type = LIBXL_USBCTRL_TYPE_PV;  
> >> +            usbctrl->type = usbback_is_loaded(gc) ? LIBXL_USBCTRL_TYPE_PV  
> > The condition should be (usbback_is_loaded(gc) > 0)? 
> > usbback_is_loaded(gc) < 0 means lstat error, cannot determine if the 
> > usbback driver is loaded. 
>  
> Good point. I think in error case I should rather abort the operation. 
>  
> Thoughts? 

I think it's OK. In another case when we check if a USB device is assigned
or not, we call get_assigned_devices(), it that function fails, we cannot
determine if a USB device is assigned or not, the handling is just abort.
So I think here if cannot determine usbback is loaded or not, we can
also abort and report error directly.

Chunyan


>  
>  
> Juergen 
>  



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH v3 3/5] libxl: add service function to check whether device model is running

[Juergen Gross]

Add an internal service function to check for a running device model.
This can be used later when adding devices to a domain requiring a
device model for either printing an error message or starting the
device model in case it is not already running.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/libxl/libxl.c    |  4 +---
 tools/libxl/libxl_dm.c | 10 ++++++++++
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 3471c4c..dcd0951 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1532,7 +1532,6 @@ void libxl__destroy_domid(libxl__egc *egc, libxl__destroy_domid_state *dis)
     libxl_ctx *ctx = CTX;
     uint32_t domid = dis->domid;
     char *dom_path;
-    char *pid;
     int rc, dm_present;
 
     libxl__ev_child_init(&dis->destroyer);
@@ -1555,8 +1554,7 @@ void libxl__destroy_domid(libxl__egc *egc, libxl__destroy_domid_state *dis)
         }
         /* fall through */
     case LIBXL_DOMAIN_TYPE_PV:
-        pid = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("/local/domain/%d/image/device-model-pid", domid));
-        dm_present = (pid != NULL);
+        dm_present = libxl__dm_active(gc, domid);
         break;
     case LIBXL_DOMAIN_TYPE_INVALID:
         rc = ERROR_FAIL;
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 361e584..bffb8f8 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -2150,6 +2150,16 @@ out:
     return ret;
 }
 
+int libxl__dm_active(libxl__gc *gc, uint32_t domid)
+{
+    char *pid, *path;
+
+    path = GCSPRINTF("/local/domain/%d/image/device-model-pid", domid);
+    pid = libxl__xs_read(gc, XBT_NULL, path);
+
+    return pid != NULL;
+}
+
 /*
  * Local variables:
  * mode: C
-- 
2.6.2


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v3 3/5] libxl: add service function to check whether device model is running

[Wei Liu]

On Wed, Mar 23, 2016 at 01:24:05PM +0100, Juergen Gross wrote:
> Add an internal service function to check for a running device model.
> This can be used later when adding devices to a domain requiring a
> device model for either printing an error message or starting the
> device model in case it is not already running.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH v3 4/5] libxl: check for dynamic device model start required

[Juergen Gross]

Add a service routine checking whether a device model must be started
after adding a device to a domain.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/libxl/libxl.c          | 12 ++++++++++++
 tools/libxl/libxl_dm.c       | 14 ++++++++++++++
 tools/libxl/libxl_internal.h |  4 ++++
 tools/libxl/libxl_pci.c      |  3 +++
 tools/libxl/libxl_pvusb.c    |  6 ++++++
 5 files changed, 39 insertions(+)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index dcd0951..2b4e36f 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2084,6 +2084,9 @@ void libxl__device_vtpm_add(libxl__egc *egc, uint32_t domid,
         if (rc) goto out;
 
         DEVICE_ADD(vtpm, vtpms, domid, &vtpm_saved, COMPARE_DEVID, &d_config);
+
+        rc = libxl__dm_check_start(gc, &d_config, domid);
+        if (rc) goto out;
     }
 
     for (;;) {
@@ -2388,6 +2391,9 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
         if (rc) goto out;
 
         DEVICE_ADD(disk, disks, domid, &disk_saved, COMPARE_DISK, &d_config);
+
+        rc = libxl__dm_check_start(gc, &d_config, domid);
+        if (rc) goto out;
     }
 
     for (;;) {
@@ -2928,6 +2934,9 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
 
     DEVICE_ADD(disk, disks, domid, &disk_saved, COMPARE_DISK, &d_config);
 
+    rc = libxl__dm_check_start(gc, &d_config, domid);
+    if (rc) goto out;
+
     if (dm_ver == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
         rc = libxl__qmp_insert_cdrom(gc, domid, disk);
         if (rc) goto out;
@@ -3354,6 +3363,9 @@ void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
         if (rc) goto out;
 
         DEVICE_ADD(nic, nics, domid, &nic_saved, COMPARE_DEVID, &d_config);
+
+        rc = libxl__dm_check_start(gc, &d_config, domid);
+        if (rc) goto out;
     }
 
     for (;;) {
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index bffb8f8..78c46674 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -2160,6 +2160,20 @@ int libxl__dm_active(libxl__gc *gc, uint32_t domid)
     return pid != NULL;
 }
 
+int libxl__dm_check_start(libxl__gc *gc, libxl_domain_config *d_config,
+                          uint32_t domid)
+{
+    if (libxl__dm_active(gc, domid))
+        return 0;
+
+    if (!libxl__need_xenpv_qemu(gc, d_config))
+        return 0;
+
+    LOG(ERROR, "device model required but not running");
+
+    return ERROR_FAIL;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 2db8b1b..9708a46 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1618,6 +1618,10 @@ _hidden const char *libxl__domain_device_model(libxl__gc *gc,
                                         const libxl_domain_build_info *info);
 _hidden int libxl__need_xenpv_qemu(libxl__gc *gc,
                                    libxl_domain_config *d_config);
+_hidden int libxl__dm_active(libxl__gc *gc, uint32_t domid);
+_hidden int libxl__dm_check_start(libxl__gc *gc,
+                                  libxl_domain_config *d_config,
+                                  uint32_t domid);
 
 /*
  * This function will fix reserved device memory conflict
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index dc10cb7..300fd4d 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -169,6 +169,9 @@ static int libxl__device_pci_add_xenstore(libxl__gc *gc, uint32_t domid, libxl_d
 
     DEVICE_ADD(pci, pcidevs, domid, &pcidev_saved, COMPARE_PCI, &d_config);
 
+    rc = libxl__dm_check_start(gc, &d_config, domid);
+    if (rc) goto out;
+
     for (;;) {
         rc = libxl__xs_transaction_start(gc, &t);
         if (rc) goto out;
diff --git a/tools/libxl/libxl_pvusb.c b/tools/libxl/libxl_pvusb.c
index 7200ead..976e4c7 100644
--- a/tools/libxl/libxl_pvusb.c
+++ b/tools/libxl/libxl_pvusb.c
@@ -139,6 +139,9 @@ static int libxl__device_usbctrl_add_xenstore(libxl__gc *gc, uint32_t domid,
 
         DEVICE_ADD(usbctrl, usbctrls, domid, &usbctrl_saved,
                    COMPARE_USBCTRL, &d_config);
+
+        rc = libxl__dm_check_start(gc, &d_config, domid);
+        if (rc) goto out;
     }
 
     for (;;) {
@@ -955,6 +958,9 @@ static int libxl__device_usbdev_add_xenstore(libxl__gc *gc, uint32_t domid,
 
         DEVICE_ADD(usbdev, usbdevs, domid, &usbdev_saved,
                    COMPARE_USB, &d_config);
+
+        rc = libxl__dm_check_start(gc, &d_config, domid);
+        if (rc) goto out;
     }
 
     for (;;) {
-- 
2.6.2


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v3 4/5] libxl: check for dynamic device model start required

[Chun Yan Liu]

>>> On 3/23/2016 at 08:24 PM, in message
<1458735847-9448-5-git-send-email-jgross@suse.com>, Juergen Gross
<jgross@suse.com> wrote: 
> Add a service routine checking whether a device model must be started 
> after adding a device to a domain. 
>  
> Signed-off-by: Juergen Gross <jgross@suse.com> 
> --- 
>  tools/libxl/libxl.c          | 12 ++++++++++++ 
>  tools/libxl/libxl_dm.c       | 14 ++++++++++++++ 
>  tools/libxl/libxl_internal.h |  4 ++++ 
>  tools/libxl/libxl_pci.c      |  3 +++ 
>  tools/libxl/libxl_pvusb.c    |  6 ++++++ 
>  5 files changed, 39 insertions(+) 
>  
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c 
> index dcd0951..2b4e36f 100644 
> --- a/tools/libxl/libxl.c 
> +++ b/tools/libxl/libxl.c 
> @@ -2084,6 +2084,9 @@ void libxl__device_vtpm_add(libxl__egc *egc, uint32_t  
> domid, 
>          if (rc) goto out; 
>   
>          DEVICE_ADD(vtpm, vtpms, domid, &vtpm_saved, COMPARE_DEVID,  
> &d_config); 
> + 
> +        rc = libxl__dm_check_start(gc, &d_config, domid); 
> +        if (rc) goto out; 
>      }

Why is this check put inside the if (aodev->update_json) {  }? I think it's a common
check, so should move outside.

- Chunyan 
 
>   
>      for (;;) { 
> @@ -2388,6 +2391,9 @@ static void device_disk_add(libxl__egc *egc, uint32_t  
> domid, 
>          if (rc) goto out; 
>   
>          DEVICE_ADD(disk, disks, domid, &disk_saved, COMPARE_DISK, &d_config); 
> + 
> +        rc = libxl__dm_check_start(gc, &d_config, domid); 
> +        if (rc) goto out; 
>      } 
>   
>      for (;;) { 
> @@ -2928,6 +2934,9 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid,  
> libxl_device_disk *disk, 
>   
>      DEVICE_ADD(disk, disks, domid, &disk_saved, COMPARE_DISK, &d_config); 
>   
> +    rc = libxl__dm_check_start(gc, &d_config, domid); 
> +    if (rc) goto out; 
> + 
>      if (dm_ver == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) { 
>          rc = libxl__qmp_insert_cdrom(gc, domid, disk); 
>          if (rc) goto out; 
> @@ -3354,6 +3363,9 @@ void libxl__device_nic_add(libxl__egc *egc, uint32_t  
> domid, 
>          if (rc) goto out; 
>   
>          DEVICE_ADD(nic, nics, domid, &nic_saved, COMPARE_DEVID, &d_config); 
> + 
> +        rc = libxl__dm_check_start(gc, &d_config, domid); 
> +        if (rc) goto out; 
>      } 
>   
>      for (;;) { 
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c 
> index bffb8f8..78c46674 100644 
> --- a/tools/libxl/libxl_dm.c 
> +++ b/tools/libxl/libxl_dm.c 
> @@ -2160,6 +2160,20 @@ int libxl__dm_active(libxl__gc *gc, uint32_t domid) 
>      return pid != NULL; 
>  } 
>   
> +int libxl__dm_check_start(libxl__gc *gc, libxl_domain_config *d_config, 
> +                          uint32_t domid) 
> +{ 
> +    if (libxl__dm_active(gc, domid)) 
> +        return 0; 
> + 
> +    if (!libxl__need_xenpv_qemu(gc, d_config)) 
> +        return 0; 
> + 
> +    LOG(ERROR, "device model required but not running"); 
> + 
> +    return ERROR_FAIL; 
> +} 
> + 
>  /* 
>   * Local variables: 
>   * mode: C 
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h 
> index 2db8b1b..9708a46 100644 
> --- a/tools/libxl/libxl_internal.h 
> +++ b/tools/libxl/libxl_internal.h 
> @@ -1618,6 +1618,10 @@ _hidden const char  
> *libxl__domain_device_model(libxl__gc *gc, 
>                                          const libxl_domain_build_info  
> *info); 
>  _hidden int libxl__need_xenpv_qemu(libxl__gc *gc, 
>                                     libxl_domain_config *d_config); 
> +_hidden int libxl__dm_active(libxl__gc *gc, uint32_t domid); 
> +_hidden int libxl__dm_check_start(libxl__gc *gc, 
> +                                  libxl_domain_config *d_config, 
> +                                  uint32_t domid); 
>   
>  /* 
>   * This function will fix reserved device memory conflict 
> diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c 
> index dc10cb7..300fd4d 100644 
> --- a/tools/libxl/libxl_pci.c 
> +++ b/tools/libxl/libxl_pci.c 
> @@ -169,6 +169,9 @@ static int libxl__device_pci_add_xenstore(libxl__gc *gc,  
> uint32_t domid, libxl_d 
>   
>      DEVICE_ADD(pci, pcidevs, domid, &pcidev_saved, COMPARE_PCI, &d_config); 
>   
> +    rc = libxl__dm_check_start(gc, &d_config, domid); 
> +    if (rc) goto out; 
> + 
>      for (;;) { 
>          rc = libxl__xs_transaction_start(gc, &t); 
>          if (rc) goto out; 
> diff --git a/tools/libxl/libxl_pvusb.c b/tools/libxl/libxl_pvusb.c 
> index 7200ead..976e4c7 100644 
> --- a/tools/libxl/libxl_pvusb.c 
> +++ b/tools/libxl/libxl_pvusb.c 
> @@ -139,6 +139,9 @@ static int libxl__device_usbctrl_add_xenstore(libxl__gc  
> *gc, uint32_t domid, 
>   
>          DEVICE_ADD(usbctrl, usbctrls, domid, &usbctrl_saved, 
>                     COMPARE_USBCTRL, &d_config); 
> + 
> +        rc = libxl__dm_check_start(gc, &d_config, domid); 
> +        if (rc) goto out; 
>      } 
>   
>      for (;;) { 
> @@ -955,6 +958,9 @@ static int libxl__device_usbdev_add_xenstore(libxl__gc  
> *gc, uint32_t domid, 
>   
>          DEVICE_ADD(usbdev, usbdevs, domid, &usbdev_saved, 
>                     COMPARE_USB, &d_config); 
> + 
> +        rc = libxl__dm_check_start(gc, &d_config, domid); 
> +        if (rc) goto out; 
>      } 
>   
>      for (;;) { 
 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v3 4/5] libxl: check for dynamic device model start required

[Juergen Gross]

On 25/03/16 03:06, Chun Yan Liu wrote:
> 
> 
>>>> On 3/23/2016 at 08:24 PM, in message
> <1458735847-9448-5-git-send-email-jgross@suse.com>, Juergen Gross
> <jgross@suse.com> wrote: 
>> Add a service routine checking whether a device model must be started 
>> after adding a device to a domain. 
>>  
>> Signed-off-by: Juergen Gross <jgross@suse.com> 
>> --- 
>>  tools/libxl/libxl.c          | 12 ++++++++++++ 
>>  tools/libxl/libxl_dm.c       | 14 ++++++++++++++ 
>>  tools/libxl/libxl_internal.h |  4 ++++ 
>>  tools/libxl/libxl_pci.c      |  3 +++ 
>>  tools/libxl/libxl_pvusb.c    |  6 ++++++ 
>>  5 files changed, 39 insertions(+) 
>>  
>> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c 
>> index dcd0951..2b4e36f 100644 
>> --- a/tools/libxl/libxl.c 
>> +++ b/tools/libxl/libxl.c 
>> @@ -2084,6 +2084,9 @@ void libxl__device_vtpm_add(libxl__egc *egc, uint32_t  
>> domid, 
>>          if (rc) goto out; 
>>   
>>          DEVICE_ADD(vtpm, vtpms, domid, &vtpm_saved, COMPARE_DEVID,  
>> &d_config); 
>> + 
>> +        rc = libxl__dm_check_start(gc, &d_config, domid); 
>> +        if (rc) goto out; 
>>      }
> 
> Why is this check put inside the if (aodev->update_json) {  }? I think it's a common
> check, so should move outside.

It is the only case where the check makes sense: update_json isn't set
when we are just creating the domain, in which case the test for the
device model needed is already in place. When the device is added to an
already running domain update_json will always be true.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v3 4/5] libxl: check for dynamic device model start required

[Chun Yan Liu]

>>> On 3/25/2016 at 02:25 PM, in message <56F4D9D6.8030300@suse.com>, Juergen Gross
<jgross@suse.com> wrote: 
> On 25/03/16 03:06, Chun Yan Liu wrote: 
> >  
> >  
> >>>> On 3/23/2016 at 08:24 PM, in message 
> > <1458735847-9448-5-git-send-email-jgross@suse.com>, Juergen Gross 
> > <jgross@suse.com> wrote:  
> >> Add a service routine checking whether a device model must be started  
> >> after adding a device to a domain.  
> >>   
> >> Signed-off-by: Juergen Gross <jgross@suse.com>  
> >> ---  
> >>  tools/libxl/libxl.c          | 12 ++++++++++++  
> >>  tools/libxl/libxl_dm.c       | 14 ++++++++++++++  
> >>  tools/libxl/libxl_internal.h |  4 ++++  
> >>  tools/libxl/libxl_pci.c      |  3 +++  
> >>  tools/libxl/libxl_pvusb.c    |  6 ++++++  
> >>  5 files changed, 39 insertions(+)  
> >>   
> >> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c  
> >> index dcd0951..2b4e36f 100644  
> >> --- a/tools/libxl/libxl.c  
> >> +++ b/tools/libxl/libxl.c  
> >> @@ -2084,6 +2084,9 @@ void libxl__device_vtpm_add(libxl__egc *egc, uint32_t  
>   
> >> domid,  
> >>          if (rc) goto out;  
> >>    
> >>          DEVICE_ADD(vtpm, vtpms, domid, &vtpm_saved, COMPARE_DEVID,   
> >> &d_config);  
> >> +  
> >> +        rc = libxl__dm_check_start(gc, &d_config, domid);  
> >> +        if (rc) goto out;  
> >>      } 
> >  
> > Why is this check put inside the if (aodev->update_json) {  }? I think it's  
> a common 
> > check, so should move outside. 
>  
> It is the only case where the check makes sense: update_json isn't set 
> when we are just creating the domain, in which case the test for the 
> device model needed is already in place.

See. That's OK then.

Thanks,
Chunyan
 
> When the device is added to an 
> already running domain update_json will always be true. 
>  
>  
> Juergen 
>  



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH v3 5/5] libxl: add domain config parameter to force start of qemu

[Juergen Gross]

Today the device model (qemu) is started for a pv domain only in case
a device requiring qemu is specified in the domain configuration
(qdisk, vfb, channel). If there is no such device the device model
isn't started and hence it is impossible to add such a device to the
domain later.

Add a domain configuration parameter to specify the device model is
to be started in any case. This will enable adding devices with a
qemu based backend later.

While the optimal solution would be to start the device model
automatically when needed this would require some major rework of
libxl at multiple places.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 docs/man/xl.cfg.pod.5       | 6 ++++++
 tools/libxl/libxl_create.c  | 1 +
 tools/libxl/libxl_dm.c      | 5 +++++
 tools/libxl/libxl_types.idl | 1 +
 tools/libxl/xl_cmdimpl.c    | 3 +++
 5 files changed, 16 insertions(+)

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index a4cc1b3..a3611a6 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -1956,6 +1956,12 @@ xen-qemudepriv-domid$domid or xen-qemudepriv-shared or root.
 Please note that running QEMU as non-root causes migration and PCI
 passthrough not to work properly.
 
+=item B<device_model_always=BOOLEAN>
+
+If true, start the device model for paravirtualized domains even if this isn't
+required according to the configured devices. This allows to add such devices
+later when the domain is already running.
+
 =back
 
 =head2 Keymaps
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 0e2b0a0..52a0a2f 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -73,6 +73,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
         return ERROR_INVAL;
     }
 
+    libxl_defbool_setdefault(&b_info->device_model_always, false);
     libxl_defbool_setdefault(&b_info->device_model_stubdomain, false);
 
     if (libxl_defbool_val(b_info->device_model_stubdomain) &&
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 78c46674..87d454c 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -2115,6 +2115,11 @@ int libxl__need_xenpv_qemu(libxl__gc *gc, libxl_domain_config *d_config)
     if (libxl__get_domid(gc, &domid))
         goto out;
 
+    if (libxl_defbool_val(d_config->b_info.device_model_always)) {
+        ret = 1;
+        goto out;
+    }
+
     if (d_config->num_vfbs > 0) {
         ret = 1;
         goto out;
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 304aa11..8ff9050 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -451,6 +451,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
     ("device_model_ssidref", uint32),
     ("device_model_ssid_label", string),
     ("device_model_user", string),
+    ("device_model_always", libxl_defbool),
 
     # extra parameters pass directly to qemu, NULL terminated
     ("extra",            libxl_string_list),
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index a3610fc..0fdca73 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -2269,6 +2269,9 @@ skip_usbdev:
     }
 
     /* parse device model arguments, this works for pv, hvm and stubdom */
+    xlu_cfg_get_defbool (config, "device_model_always",
+                         &b_info->device_model_always, 0);
+
     if (!xlu_cfg_get_string (config, "device_model", &buf, 0)) {
         fprintf(stderr,
                 "WARNING: ignoring device_model directive.\n"
-- 
2.6.2


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v3 5/5] libxl: add domain config parameter to force start of qemu

[Wei Liu]

On Wed, Mar 23, 2016 at 01:24:07PM +0100, Juergen Gross wrote:
> Today the device model (qemu) is started for a pv domain only in case
> a device requiring qemu is specified in the domain configuration
> (qdisk, vfb, channel). If there is no such device the device model
> isn't started and hence it is impossible to add such a device to the
> domain later.
> 
> Add a domain configuration parameter to specify the device model is
> to be started in any case. This will enable adding devices with a
> qemu based backend later.
> 
> While the optimal solution would be to start the device model
> automatically when needed this would require some major rework of
> libxl at multiple places.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

I can't say I like this.

Libxl can be made smart enough to detect whether starting a device model
is required (as you already did in previous patch) then start qemu on
demand.

There is a xen-attach mode in qemu, and libxl already has machinery for
starting qemu. I suspect what you need to do is refactoring existing
code and expose the right parameter for qemu. Then you wouldn't need
this user visible option at all. You would also save the resources for
always having an idle qemu in toolstack domain.

Let me know if this is to vague.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v3 5/5] libxl: add domain config parameter to force start of qemu

[Wei Liu]

On Thu, Mar 24, 2016 at 08:28:45PM +0000, Wei Liu wrote:
> On Wed, Mar 23, 2016 at 01:24:07PM +0100, Juergen Gross wrote:
> > Today the device model (qemu) is started for a pv domain only in case
> > a device requiring qemu is specified in the domain configuration
> > (qdisk, vfb, channel). If there is no such device the device model
> > isn't started and hence it is impossible to add such a device to the
> > domain later.
> > 
> > Add a domain configuration parameter to specify the device model is
> > to be started in any case. This will enable adding devices with a
> > qemu based backend later.
> > 
> > While the optimal solution would be to start the device model
> > automatically when needed this would require some major rework of
> > libxl at multiple places.
> > 
> > Signed-off-by: Juergen Gross <jgross@suse.com>
> 
> I can't say I like this.
> 
> Libxl can be made smart enough to detect whether starting a device model
> is required (as you already did in previous patch) then start qemu on
> demand.
> 
> There is a xen-attach mode in qemu, and libxl already has machinery for
> starting qemu. I suspect what you need to do is refactoring existing
> code and expose the right parameter for qemu. Then you wouldn't need
> this user visible option at all. You would also save the resources for
> always having an idle qemu in toolstack domain.
> 
> Let me know if this is to vague.
> 

Actually I don't think this is your problem. I would consider this a bug
in libxl for not starting qemu on demand. I can take a stab at it, but
that will need to wait until next release.

I think the functionality you care about  can live without this patch,
but I'm not sure if it would become too cumbersome to use.

Wei.

> Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v3 5/5] libxl: add domain config parameter to force start of qemu

[Juergen Gross]

On 24/03/16 21:35, Wei Liu wrote:
> On Thu, Mar 24, 2016 at 08:28:45PM +0000, Wei Liu wrote:
>> On Wed, Mar 23, 2016 at 01:24:07PM +0100, Juergen Gross wrote:
>>> Today the device model (qemu) is started for a pv domain only in case
>>> a device requiring qemu is specified in the domain configuration
>>> (qdisk, vfb, channel). If there is no such device the device model
>>> isn't started and hence it is impossible to add such a device to the
>>> domain later.
>>>
>>> Add a domain configuration parameter to specify the device model is
>>> to be started in any case. This will enable adding devices with a
>>> qemu based backend later.
>>>
>>> While the optimal solution would be to start the device model
>>> automatically when needed this would require some major rework of
>>> libxl at multiple places.
>>>
>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>
>> I can't say I like this.
>>
>> Libxl can be made smart enough to detect whether starting a device model
>> is required (as you already did in previous patch) then start qemu on
>> demand.
>>
>> There is a xen-attach mode in qemu, and libxl already has machinery for
>> starting qemu. I suspect what you need to do is refactoring existing
>> code and expose the right parameter for qemu. Then you wouldn't need
>> this user visible option at all. You would also save the resources for
>> always having an idle qemu in toolstack domain.
>>
>> Let me know if this is to vague.
>>
> 
> Actually I don't think this is your problem. I would consider this a bug
> in libxl for not starting qemu on demand. I can take a stab at it, but
> that will need to wait until next release.
> 
> I think the functionality you care about  can live without this patch,
> but I'm not sure if it would become too cumbersome to use.

The "normal" block backend is qdisk these days. So normally the device
model will be running. And in many cases a pvusb device will be assigned
via the xl config of the domain which will start the device model
automatically.

I just wanted to make clear there is a potential problem and presented
a solution for this which I could implement in time for 4.7.


Juergen


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v3 5/5] libxl: add domain config parameter to force start of qemu

[Juergen Gross]

On 24/03/16 21:28, Wei Liu wrote:
> On Wed, Mar 23, 2016 at 01:24:07PM +0100, Juergen Gross wrote:
>> Today the device model (qemu) is started for a pv domain only in case
>> a device requiring qemu is specified in the domain configuration
>> (qdisk, vfb, channel). If there is no such device the device model
>> isn't started and hence it is impossible to add such a device to the
>> domain later.
>>
>> Add a domain configuration parameter to specify the device model is
>> to be started in any case. This will enable adding devices with a
>> qemu based backend later.
>>
>> While the optimal solution would be to start the device model
>> automatically when needed this would require some major rework of
>> libxl at multiple places.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
> 
> I can't say I like this.
> 
> Libxl can be made smart enough to detect whether starting a device model
> is required (as you already did in previous patch) then start qemu on
> demand.
> 
> There is a xen-attach mode in qemu, and libxl already has machinery for
> starting qemu. I suspect what you need to do is refactoring existing
> code and expose the right parameter for qemu. Then you wouldn't need
> this user visible option at all. You would also save the resources for
> always having an idle qemu in toolstack domain.
> 
> Let me know if this is to vague.

No, that's okay. I think I'll drop this patch for now and start a clean
attempt after 4.7 is out.

Juergen


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel