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

[PATCH v5 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 V5:
- added new patch 2 as requested by George Dunlap

Changes in V4:
- dropped patch 5
- patch 1: Return (negative) error value in case of failure, 0 or 1 else
- patch 2: Bail out in case of usbback_is_loaded() error as requested by
  Chun Yan Liu

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 query function for backend support by device model
  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

 docs/man/xl.cfg.pod.5                |  11 +++-
 tools/libxl/libxl.c                  |  16 ++++-
 tools/libxl/libxl_create.c           |  12 ++--
 tools/libxl/libxl_device.c           |   3 +-
 tools/libxl/libxl_dm.c               | 115 +++++++++++++++++++++------------
 tools/libxl/libxl_internal.h         |  15 +++--
 tools/libxl/libxl_pci.c              |   3 +
 tools/libxl/libxl_pvusb.c            | 122 ++++++++++++++++++++++++++++-------
 tools/libxl/libxl_types.idl          |   1 +
 tools/libxl/libxl_types_internal.idl |   1 +
 10 files changed, 218 insertions(+), 81 deletions(-)

-- 
2.6.2


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

[PATCH v5 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>
---
V4: Return (negative) error value in case of failure, 0 or 1 else

V2: Return false if libxl__get_domid() fails as requested by George Dunlap
---
 tools/libxl/libxl_create.c   | 12 +++------
 tools/libxl/libxl_dm.c       | 60 +++++++++++++-------------------------------
 tools/libxl/libxl_internal.h |  5 +---
 3 files changed, 23 insertions(+), 54 deletions(-)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 61b5c01..0681103 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,14 @@ 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) {
+        ret = libxl__need_xenpv_qemu(gc, d_config);
+        if (ret < 0)
+            goto error_out;
+        if (ret) {
             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 cfda24c..0d88c37 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -2113,61 +2113,37 @@ 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;
+    int i, ret;
     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;
+    ret = libxl__get_domid(gc, &domid);
+    if (ret) {
+        LOG(ERROR, "unable to get domain id");
         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

[PATCH v5 2/5] libxl: add query function for backend support by device model

[Juergen Gross]

Add a function to query whether the device model is supporting a
specific backend type. The device model is writing the supported
backend types to Xenstore on startup. The new query function checks
for the appropriate entry to be present.

As not all versions of qemu are capable to indicate support of
specific backends the query function is to be called with an indicator
whether the default return value should be "supported" (in case qemu
doesn't know set any support information) or "not supported".

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/libxl/libxl_dm.c       | 19 +++++++++++++++++++
 tools/libxl/libxl_internal.h |  5 +++++
 2 files changed, 24 insertions(+)

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 0d88c37..7d9abbe 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1920,6 +1920,25 @@ out:
         device_model_spawn_outcome(egc, dmss, rc);
 }
 
+bool libxl__query_qemu_backend(libxl__gc *gc, uint32_t domid,
+                               uint32_t backend_id, const char *type, bool def)
+{
+    char *path;
+    char **dir;
+    unsigned int n;
+
+    path = GCSPRINTF("%s/device-model/%u/backends",
+                     libxl__xs_get_dompath(gc, backend_id), domid);
+    dir = libxl__xs_directory(gc, XBT_NULL, path, &n);
+    if (!dir)
+        return def;
+
+    path = GCSPRINTF("%s/device-model/%u/backends/%s",
+                     libxl__xs_get_dompath(gc, backend_id), domid, type);
+    dir = libxl__xs_directory(gc, XBT_NULL, path, &n);
+
+    return !!dir;
+}
 
 static void device_model_confirm(libxl__egc *egc, libxl__spawn_state *spawn,
                                  const char *xsdata)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index fc7bdab..c06ffc0 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1617,6 +1617,11 @@ _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 bool libxl__query_qemu_backend(libxl__gc *gc,
+                                       uint32_t domid,
+                                       uint32_t backend_id,
+                                       const char *type,
+                                       bool def);
 
 /*
  * 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

[PATCH v5 3/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>
---
V4: bail out in case of usbback_is_loaded() error as requested by
    Chun Yan Liu
---
 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            | 116 ++++++++++++++++++++++++++++-------
 tools/libxl/libxl_types.idl          |   1 +
 tools/libxl/libxl_types_internal.idl |   1 +
 7 files changed, 115 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 7d9abbe..8f5c4e6 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -2166,6 +2166,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 c06ffc0..20b6122 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..68e5673 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 ERROR_FAIL;
+}
+
 static int libxl__device_usbctrl_setdefault(libxl__gc *gc, uint32_t domid,
                                             libxl_device_usbctrl *usbctrl)
 {
@@ -36,7 +51,11 @@ 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;
+            rc = usbback_is_loaded(gc);
+            if (rc < 0)
+                goto out;
+            usbctrl->type = rc ? 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;
@@ -45,6 +64,8 @@ static int libxl__device_usbctrl_setdefault(libxl__gc *gc, uint32_t domid,
 
     rc = libxl__resolve_domid(gc, usbctrl->backend_domname,
                               &usbctrl->backend_domid);
+
+out:
     return rc;
 }
 
@@ -54,7 +75,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 +87,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,
@@ -121,6 +144,15 @@ static int libxl__device_usbctrl_add_xenstore(libxl__gc *gc, uint32_t domid,
 
         DEVICE_ADD(usbctrl, usbctrls, domid, &usbctrl_saved,
                    COMPARE_USBCTRL, &d_config);
+
+        if (usbctrl->type == LIBXL_USBCTRL_TYPE_QUSB) {
+            if (!libxl__query_qemu_backend(gc, domid, usbctrl->backend_domid,
+                                           "qusb", false)) {
+                LOG(ERROR, "backend type not supported by device model");
+                rc = ERROR_FAIL;
+                goto out;
+            }
+        }
     }
 
     for (;;) {
@@ -159,6 +191,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 +234,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 +297,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 +339,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 +347,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 +787,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 +931,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 +980,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 +1006,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 +1023,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 +1387,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 +1522,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 +1547,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 +1564,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 +1654,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

[PATCH v5 4/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>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl.c          |  4 +---
 tools/libxl/libxl_dm.c       | 10 ++++++++++
 tools/libxl/libxl_internal.h |  1 +
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 5c473e7..bb6a689 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 8f5c4e6..e60e125 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -2178,6 +2178,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
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 20b6122..144c715 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1623,6 +1623,7 @@ _hidden bool libxl__query_qemu_backend(libxl__gc *gc,
                                        uint32_t backend_id,
                                        const char *type,
                                        bool def);
+_hidden int libxl__dm_active(libxl__gc *gc, uint32_t domid);
 
 /*
  * 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

[PATCH v5 5/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       | 22 ++++++++++++++++++++++
 tools/libxl/libxl_internal.h |  3 +++
 tools/libxl/libxl_pci.c      |  3 +++
 tools/libxl/libxl_pvusb.c    |  6 ++++++
 5 files changed, 46 insertions(+)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index bb6a689..6d4b5b4 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 e60e125..0eb8ddd 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -2188,6 +2188,28 @@ 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)
+{
+    int rc;
+
+    if (libxl__dm_active(gc, domid))
+        return 0;
+
+    rc = libxl__need_xenpv_qemu(gc, d_config);
+    if (rc < 0)
+        goto out;
+
+    if (!rc)
+        return 0;
+
+    LOG(ERROR, "device model required but not running");
+    rc = ERROR_FAIL;
+
+out:
+    return rc;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 144c715..f61eaf0 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1624,6 +1624,9 @@ _hidden bool libxl__query_qemu_backend(libxl__gc *gc,
                                        const char *type,
                                        bool def);
 _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 68e5673..c9b7c4a 100644
--- a/tools/libxl/libxl_pvusb.c
+++ b/tools/libxl/libxl_pvusb.c
@@ -145,6 +145,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;
+
         if (usbctrl->type == LIBXL_USBCTRL_TYPE_QUSB) {
             if (!libxl__query_qemu_backend(gc, domid, usbctrl->backend_domid,
                                            "qusb", false)) {
@@ -969,6 +972,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 v5 1/5] libxl: make libxl__need_xenpv_qemu() operate on domain config

[Wei Liu]

On Wed, Mar 30, 2016 at 02:05:54PM +0200, 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>
> ---
> V4: Return (negative) error value in case of failure, 0 or 1 else
> 

I think this is a backport candidate -- can you please split it out
from the rest of changes?

Other than that, the code looks good.

> V2: Return false if libxl__get_domid() fails as requested by George Dunlap
> ---
>  tools/libxl/libxl_create.c   | 12 +++------
>  tools/libxl/libxl_dm.c       | 60 +++++++++++++-------------------------------
>  tools/libxl/libxl_internal.h |  5 +---
>  3 files changed, 23 insertions(+), 54 deletions(-)
> 
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 61b5c01..0681103 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,14 @@ 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) {
> +        ret = libxl__need_xenpv_qemu(gc, d_config);
> +        if (ret < 0)
> +            goto error_out;
> +        if (ret) {
>              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 cfda24c..0d88c37 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -2113,61 +2113,37 @@ 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)

Maybe add a comment here about return values?

> +int libxl__need_xenpv_qemu(libxl__gc *gc, libxl_domain_config *d_config)
>  {

Wei.

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

Re: [PATCH v5 2/5] libxl: add query function for backend support by device model

[George Dunlap]

On 30/03/16 13:05, Juergen Gross wrote:
> Add a function to query whether the device model is supporting a
> specific backend type. The device model is writing the supported
> backend types to Xenstore on startup. The new query function checks
> for the appropriate entry to be present.
> 
> As not all versions of qemu are capable to indicate support of
> specific backends the query function is to be called with an indicator
> whether the default return value should be "supported" (in case qemu
> doesn't know set any support information) or "not supported".
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Has this approach (writing backend capabilities into xenstore) been
agreed on the qemu side?  It's significantly different than what's been
done so far wrt qemu feature discovery, right?

I suppose we can just revert this whole series if they end up
disagreeing with this approach, but I think that's something we'd like
to avoid.  (Although I suppose that's ultimately the release manager's
call.)

 -George

> ---
>  tools/libxl/libxl_dm.c       | 19 +++++++++++++++++++
>  tools/libxl/libxl_internal.h |  5 +++++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 0d88c37..7d9abbe 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -1920,6 +1920,25 @@ out:
>          device_model_spawn_outcome(egc, dmss, rc);
>  }
>  
> +bool libxl__query_qemu_backend(libxl__gc *gc, uint32_t domid,
> +                               uint32_t backend_id, const char *type, bool def)
> +{
> +    char *path;
> +    char **dir;
> +    unsigned int n;
> +
> +    path = GCSPRINTF("%s/device-model/%u/backends",
> +                     libxl__xs_get_dompath(gc, backend_id), domid);
> +    dir = libxl__xs_directory(gc, XBT_NULL, path, &n);
> +    if (!dir)
> +        return def;
> +
> +    path = GCSPRINTF("%s/device-model/%u/backends/%s",
> +                     libxl__xs_get_dompath(gc, backend_id), domid, type);
> +    dir = libxl__xs_directory(gc, XBT_NULL, path, &n);
> +
> +    return !!dir;
> +}
>  
>  static void device_model_confirm(libxl__egc *egc, libxl__spawn_state *spawn,
>                                   const char *xsdata)
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index fc7bdab..c06ffc0 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -1617,6 +1617,11 @@ _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 bool libxl__query_qemu_backend(libxl__gc *gc,
> +                                       uint32_t domid,
> +                                       uint32_t backend_id,
> +                                       const char *type,
> +                                       bool def);
>  
>  /*
>   * This function will fix reserved device memory conflict
> 


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

Re: [PATCH v5 2/5] libxl: add query function for backend support by device model

[Wei Liu]

On Wed, Mar 30, 2016 at 02:05:55PM +0200, Juergen Gross wrote:
> Add a function to query whether the device model is supporting a
> specific backend type. The device model is writing the supported
> backend types to Xenstore on startup. The new query function checks
> for the appropriate entry to be present.
> 
> As not all versions of qemu are capable to indicate support of
> specific backends the query function is to be called with an indicator
> whether the default return value should be "supported" (in case qemu
> doesn't know set any support information) or "not supported".
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

The code itself looks straightforward enough.

But note that this is a new protocol that needs to be supported
essentially forever. I've CC QEMU maintainers for their input.

This also means even if we get this in for 4.7 we can't essentially
benefit from it until this protocol is implemented in upstream QEMU. At
this point I don't think I would take this particular patch for 4.7.

(I haven't checked if the rest depends on this one though)

Wei.

> ---
>  tools/libxl/libxl_dm.c       | 19 +++++++++++++++++++
>  tools/libxl/libxl_internal.h |  5 +++++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 0d88c37..7d9abbe 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -1920,6 +1920,25 @@ out:
>          device_model_spawn_outcome(egc, dmss, rc);
>  }
>  
> +bool libxl__query_qemu_backend(libxl__gc *gc, uint32_t domid,
> +                               uint32_t backend_id, const char *type, bool def)
> +{
> +    char *path;
> +    char **dir;
> +    unsigned int n;
> +
> +    path = GCSPRINTF("%s/device-model/%u/backends",
> +                     libxl__xs_get_dompath(gc, backend_id), domid);
> +    dir = libxl__xs_directory(gc, XBT_NULL, path, &n);
> +    if (!dir)
> +        return def;
> +
> +    path = GCSPRINTF("%s/device-model/%u/backends/%s",
> +                     libxl__xs_get_dompath(gc, backend_id), domid, type);
> +    dir = libxl__xs_directory(gc, XBT_NULL, path, &n);
> +
> +    return !!dir;
> +}
>  
>  static void device_model_confirm(libxl__egc *egc, libxl__spawn_state *spawn,
>                                   const char *xsdata)
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index fc7bdab..c06ffc0 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -1617,6 +1617,11 @@ _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 bool libxl__query_qemu_backend(libxl__gc *gc,
> +                                       uint32_t domid,
> +                                       uint32_t backend_id,
> +                                       const char *type,
> +                                       bool def);
>  
>  /*
>   * 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 v5 2/5] libxl: add query function for backend support by device model

[Juergen Gross]

On 30/03/16 15:44, George Dunlap wrote:
> On 30/03/16 13:05, Juergen Gross wrote:
>> Add a function to query whether the device model is supporting a
>> specific backend type. The device model is writing the supported
>> backend types to Xenstore on startup. The new query function checks
>> for the appropriate entry to be present.
>>
>> As not all versions of qemu are capable to indicate support of
>> specific backends the query function is to be called with an indicator
>> whether the default return value should be "supported" (in case qemu
>> doesn't know set any support information) or "not supported".
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
> 
> Has this approach (writing backend capabilities into xenstore) been
> agreed on the qemu side?  It's significantly different than what's been
> done so far wrt qemu feature discovery, right?

I wanted to make sure we agree on the approach. The qemu patch is
trivial and I'll send it as soon as I've got the "go" on Xen side.
The patch is just moving some functions to another source to be able
to reuse those and it adds 4 lines of code. That's all.

TBH: This is the natural way to do the feature discovery for Xen. It is
simple and even today qemu is writing it's "running" state into Xenstore
which is some kind of "feature" as well. And we can be sure that once
qemu is started for a domain we'll have the features it is offering at
a place where we can read them easily as often as we want. I don't think
any other solution would just work for stubdom, driver domains etc.
without adding special code for those cases.

> I suppose we can just revert this whole series if they end up
> disagreeing with this approach, but I think that's something we'd like
> to avoid.  (Although I suppose that's ultimately the release manager's
> call.)

Chicken and egg again. :-)


Juergen

> 
>  -George
> 
>> ---
>>  tools/libxl/libxl_dm.c       | 19 +++++++++++++++++++
>>  tools/libxl/libxl_internal.h |  5 +++++
>>  2 files changed, 24 insertions(+)
>>
>> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>> index 0d88c37..7d9abbe 100644
>> --- a/tools/libxl/libxl_dm.c
>> +++ b/tools/libxl/libxl_dm.c
>> @@ -1920,6 +1920,25 @@ out:
>>          device_model_spawn_outcome(egc, dmss, rc);
>>  }
>>  
>> +bool libxl__query_qemu_backend(libxl__gc *gc, uint32_t domid,
>> +                               uint32_t backend_id, const char *type, bool def)
>> +{
>> +    char *path;
>> +    char **dir;
>> +    unsigned int n;
>> +
>> +    path = GCSPRINTF("%s/device-model/%u/backends",
>> +                     libxl__xs_get_dompath(gc, backend_id), domid);
>> +    dir = libxl__xs_directory(gc, XBT_NULL, path, &n);
>> +    if (!dir)
>> +        return def;
>> +
>> +    path = GCSPRINTF("%s/device-model/%u/backends/%s",
>> +                     libxl__xs_get_dompath(gc, backend_id), domid, type);
>> +    dir = libxl__xs_directory(gc, XBT_NULL, path, &n);
>> +
>> +    return !!dir;
>> +}
>>  
>>  static void device_model_confirm(libxl__egc *egc, libxl__spawn_state *spawn,
>>                                   const char *xsdata)
>> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
>> index fc7bdab..c06ffc0 100644
>> --- a/tools/libxl/libxl_internal.h
>> +++ b/tools/libxl/libxl_internal.h
>> @@ -1617,6 +1617,11 @@ _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 bool libxl__query_qemu_backend(libxl__gc *gc,
>> +                                       uint32_t domid,
>> +                                       uint32_t backend_id,
>> +                                       const char *type,
>> +                                       bool def);
>>  
>>  /*
>>   * This function will fix reserved device memory conflict
>>
> 
> 


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

Re: [PATCH v5 2/5] libxl: add query function for backend support by device model

[Juergen Gross]

On 30/03/16 15:53, Wei Liu wrote:
> On Wed, Mar 30, 2016 at 02:05:55PM +0200, Juergen Gross wrote:
>> Add a function to query whether the device model is supporting a
>> specific backend type. The device model is writing the supported
>> backend types to Xenstore on startup. The new query function checks
>> for the appropriate entry to be present.
>>
>> As not all versions of qemu are capable to indicate support of
>> specific backends the query function is to be called with an indicator
>> whether the default return value should be "supported" (in case qemu
>> doesn't know set any support information) or "not supported".
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
> 
> The code itself looks straightforward enough.
> 
> But note that this is a new protocol that needs to be supported
> essentially forever. I've CC QEMU maintainers for their input.

Okay, I'll send the qemu patch now.

> This also means even if we get this in for 4.7 we can't essentially
> benefit from it until this protocol is implemented in upstream QEMU. At
> this point I don't think I would take this particular patch for 4.7.
> 
> (I haven't checked if the rest depends on this one though)

George asked me to add some kind of feature test (qusb support by qemu).
So I added it and yes, patch 3 depends on this one.


Juergen

> 
> Wei.
> 
>> ---
>>  tools/libxl/libxl_dm.c       | 19 +++++++++++++++++++
>>  tools/libxl/libxl_internal.h |  5 +++++
>>  2 files changed, 24 insertions(+)
>>
>> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>> index 0d88c37..7d9abbe 100644
>> --- a/tools/libxl/libxl_dm.c
>> +++ b/tools/libxl/libxl_dm.c
>> @@ -1920,6 +1920,25 @@ out:
>>          device_model_spawn_outcome(egc, dmss, rc);
>>  }
>>  
>> +bool libxl__query_qemu_backend(libxl__gc *gc, uint32_t domid,
>> +                               uint32_t backend_id, const char *type, bool def)
>> +{
>> +    char *path;
>> +    char **dir;
>> +    unsigned int n;
>> +
>> +    path = GCSPRINTF("%s/device-model/%u/backends",
>> +                     libxl__xs_get_dompath(gc, backend_id), domid);
>> +    dir = libxl__xs_directory(gc, XBT_NULL, path, &n);
>> +    if (!dir)
>> +        return def;
>> +
>> +    path = GCSPRINTF("%s/device-model/%u/backends/%s",
>> +                     libxl__xs_get_dompath(gc, backend_id), domid, type);
>> +    dir = libxl__xs_directory(gc, XBT_NULL, path, &n);
>> +
>> +    return !!dir;
>> +}
>>  
>>  static void device_model_confirm(libxl__egc *egc, libxl__spawn_state *spawn,
>>                                   const char *xsdata)
>> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
>> index fc7bdab..c06ffc0 100644
>> --- a/tools/libxl/libxl_internal.h
>> +++ b/tools/libxl/libxl_internal.h
>> @@ -1617,6 +1617,11 @@ _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 bool libxl__query_qemu_backend(libxl__gc *gc,
>> +                                       uint32_t domid,
>> +                                       uint32_t backend_id,
>> +                                       const char *type,
>> +                                       bool def);
>>  
>>  /*
>>   * 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 v5 1/5] libxl: make libxl__need_xenpv_qemu() operate on domain config

[Juergen Gross]

On 30/03/16 15:38, Wei Liu wrote:
> On Wed, Mar 30, 2016 at 02:05:54PM +0200, 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>
>> ---
>> V4: Return (negative) error value in case of failure, 0 or 1 else
>>
> 
> I think this is a backport candidate -- can you please split it out
> from the rest of changes?

Sure.

> 
> Other than that, the code looks good.
> 
>> V2: Return false if libxl__get_domid() fails as requested by George Dunlap
>> ---
>>  tools/libxl/libxl_create.c   | 12 +++------
>>  tools/libxl/libxl_dm.c       | 60 +++++++++++++-------------------------------
>>  tools/libxl/libxl_internal.h |  5 +---
>>  3 files changed, 23 insertions(+), 54 deletions(-)
>>
>> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
>> index 61b5c01..0681103 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,14 @@ 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) {
>> +        ret = libxl__need_xenpv_qemu(gc, d_config);
>> +        if (ret < 0)
>> +            goto error_out;
>> +        if (ret) {
>>              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 cfda24c..0d88c37 100644
>> --- a/tools/libxl/libxl_dm.c
>> +++ b/tools/libxl/libxl_dm.c
>> @@ -2113,61 +2113,37 @@ 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)
> 
> Maybe add a comment here about return values?

Yes, can do.


Juergen

> 
>> +int libxl__need_xenpv_qemu(libxl__gc *gc, libxl_domain_config *d_config)
>>  {
> 
> Wei.
> 


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

Re: [PATCH v5 2/5] libxl: add query function for backend support by device model

[Wei Liu]

On Wed, Mar 30, 2016 at 04:02:20PM +0200, Juergen Gross wrote:
> On 30/03/16 15:53, Wei Liu wrote:
> > On Wed, Mar 30, 2016 at 02:05:55PM +0200, Juergen Gross wrote:
> >> Add a function to query whether the device model is supporting a
> >> specific backend type. The device model is writing the supported
> >> backend types to Xenstore on startup. The new query function checks
> >> for the appropriate entry to be present.
> >>
> >> As not all versions of qemu are capable to indicate support of
> >> specific backends the query function is to be called with an indicator
> >> whether the default return value should be "supported" (in case qemu
> >> doesn't know set any support information) or "not supported".
> >>
> >> Signed-off-by: Juergen Gross <jgross@suse.com>
> > 
> > The code itself looks straightforward enough.
> > 
> > But note that this is a new protocol that needs to be supported
> > essentially forever. I've CC QEMU maintainers for their input.
> 
> Okay, I'll send the qemu patch now.
> 

And also a patch to docs/misc/xenstore-paths.markdown please.

Wei.

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

Re: [PATCH v5 2/5] libxl: add query function for backend support by device model

[George Dunlap]

On 30/03/16 14:53, Wei Liu wrote:
> On Wed, Mar 30, 2016 at 02:05:55PM +0200, Juergen Gross wrote:
>> Add a function to query whether the device model is supporting a
>> specific backend type. The device model is writing the supported
>> backend types to Xenstore on startup. The new query function checks
>> for the appropriate entry to be present.
>>
>> As not all versions of qemu are capable to indicate support of
>> specific backends the query function is to be called with an indicator
>> whether the default return value should be "supported" (in case qemu
>> doesn't know set any support information) or "not supported".
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
> 
> The code itself looks straightforward enough.
> 
> But note that this is a new protocol that needs to be supported
> essentially forever. I've CC QEMU maintainers for their input.

FWIW, libxl only needs to support this forever if qemu ever actually
provides it.  If no version of qemu ever ships with this check, then we
can't break anything by removing it.

This is what I was saying in my other comment -- we could just check
this in, and back out the whole series if the qemu side gets NACK'ed.
Even if we release with this, there's no real harm done, as the qusb
code will simply never be activated.

> This also means even if we get this in for 4.7 we can't essentially
> benefit from it until this protocol is implemented in upstream QEMU. At
> this point I don't think I would take this particular patch for 4.7.

qemu has a faster release cycle than us, right?  If we have support in
principle for this approach, people who want qusb support can always use
a newer qemu release.

OTOH, if we check this in then the qemu folks can either take this exact
approach, or something completely incompatible -- they can't make minor
modifications.

 -George

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

Re: [PATCH v5 2/5] libxl: add query function for backend support by device model

[Juergen Gross]

On 30/03/16 16:09, Wei Liu wrote:
> On Wed, Mar 30, 2016 at 04:02:20PM +0200, Juergen Gross wrote:
>> On 30/03/16 15:53, Wei Liu wrote:
>>> On Wed, Mar 30, 2016 at 02:05:55PM +0200, Juergen Gross wrote:
>>>> Add a function to query whether the device model is supporting a
>>>> specific backend type. The device model is writing the supported
>>>> backend types to Xenstore on startup. The new query function checks
>>>> for the appropriate entry to be present.
>>>>
>>>> As not all versions of qemu are capable to indicate support of
>>>> specific backends the query function is to be called with an indicator
>>>> whether the default return value should be "supported" (in case qemu
>>>> doesn't know set any support information) or "not supported".
>>>>
>>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>>
>>> The code itself looks straightforward enough.
>>>
>>> But note that this is a new protocol that needs to be supported
>>> essentially forever. I've CC QEMU maintainers for their input.
>>
>> Okay, I'll send the qemu patch now.
>>
> 
> And also a patch to docs/misc/xenstore-paths.markdown please.

Aah, sure.


Juergen


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

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

[Wei Liu]

On Wed, Mar 30, 2016 at 02:05:56PM +0200, Juergen Gross 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>
> ---
> V4: bail out in case of usbback_is_loaded() error as requested by
>     Chun Yan Liu
> ---
[...]
>  /* 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,
> @@ -121,6 +144,15 @@ static int libxl__device_usbctrl_add_xenstore(libxl__gc *gc, uint32_t domid,
>  
>          DEVICE_ADD(usbctrl, usbctrls, domid, &usbctrl_saved,
>                     COMPARE_USBCTRL, &d_config);
> +
> +        if (usbctrl->type == LIBXL_USBCTRL_TYPE_QUSB) {
> +            if (!libxl__query_qemu_backend(gc, domid, usbctrl->backend_domid,
> +                                           "qusb", false)) {

This needs to be sorted out.

And this is maybe a rather dumb question: the xenstore paths for qusb
and kernel backend are the same? What is the likelihood that one
deviates from the other? Note that this is not suggesting that you
over-engineer current code, it's just something that needs clarifying.

Wei.

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

Re: [PATCH v5 2/5] libxl: add query function for backend support by device model

[Wei Liu]

On Wed, Mar 30, 2016 at 04:11:39PM +0200, Juergen Gross wrote:
> On 30/03/16 16:09, Wei Liu wrote:
> > On Wed, Mar 30, 2016 at 04:02:20PM +0200, Juergen Gross wrote:
> >> On 30/03/16 15:53, Wei Liu wrote:
> >>> On Wed, Mar 30, 2016 at 02:05:55PM +0200, Juergen Gross wrote:
> >>>> Add a function to query whether the device model is supporting a
> >>>> specific backend type. The device model is writing the supported
> >>>> backend types to Xenstore on startup. The new query function checks
> >>>> for the appropriate entry to be present.
> >>>>
> >>>> As not all versions of qemu are capable to indicate support of
> >>>> specific backends the query function is to be called with an indicator
> >>>> whether the default return value should be "supported" (in case qemu
> >>>> doesn't know set any support information) or "not supported".
> >>>>
> >>>> Signed-off-by: Juergen Gross <jgross@suse.com>
> >>>
> >>> The code itself looks straightforward enough.
> >>>
> >>> But note that this is a new protocol that needs to be supported
> >>> essentially forever. I've CC QEMU maintainers for their input.
> >>
> >> Okay, I'll send the qemu patch now.
> >>
> > 
> > And also a patch to docs/misc/xenstore-paths.markdown please.
> 
> Aah, sure.
> 

Actually wait, I think all QEMU nodes are either under documented or I
looked at the wrong place.

Anthony, how should we document the protocol between QEMU and toolstack?
Did I look at the wrong place?

Wei.

> 
> Juergen
> 

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

Re: [PATCH v5 2/5] libxl: add query function for backend support by device model

[Wei Liu]

On Wed, Mar 30, 2016 at 03:10:45PM +0100, George Dunlap wrote:
> On 30/03/16 14:53, Wei Liu wrote:
> > On Wed, Mar 30, 2016 at 02:05:55PM +0200, Juergen Gross wrote:
> >> Add a function to query whether the device model is supporting a
> >> specific backend type. The device model is writing the supported
> >> backend types to Xenstore on startup. The new query function checks
> >> for the appropriate entry to be present.
> >>
> >> As not all versions of qemu are capable to indicate support of
> >> specific backends the query function is to be called with an indicator
> >> whether the default return value should be "supported" (in case qemu
> >> doesn't know set any support information) or "not supported".
> >>
> >> Signed-off-by: Juergen Gross <jgross@suse.com>
> > 
> > The code itself looks straightforward enough.
> > 
> > But note that this is a new protocol that needs to be supported
> > essentially forever. I've CC QEMU maintainers for their input.
> 
> FWIW, libxl only needs to support this forever if qemu ever actually
> provides it.  If no version of qemu ever ships with this check, then we
> can't break anything by removing it.
> 

That's true. But the code is actually not very relevant, I think the
protocol shall be written down somewhere in a canonical document --
that's something that we can't easily revert -- unless we don't document
it, then I'm not sure what state should this feature be in or how do we
advertise it in our release note.

Wei.

> This is what I was saying in my other comment -- we could just check
> this in, and back out the whole series if the qemu side gets NACK'ed.
> Even if we release with this, there's no real harm done, as the qusb
> code will simply never be activated.
> 
> > This also means even if we get this in for 4.7 we can't essentially
> > benefit from it until this protocol is implemented in upstream QEMU. At
> > this point I don't think I would take this particular patch for 4.7.
> 
> qemu has a faster release cycle than us, right?  If we have support in
> principle for this approach, people who want qusb support can always use
> a newer qemu release.
> 
> OTOH, if we check this in then the qemu folks can either take this exact
> approach, or something completely incompatible -- they can't make minor
> modifications.
> 
>  -George

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

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

[Juergen Gross]

On 30/03/16 16:19, Wei Liu wrote:
> On Wed, Mar 30, 2016 at 02:05:56PM +0200, Juergen Gross 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>
>> ---
>> V4: bail out in case of usbback_is_loaded() error as requested by
>>     Chun Yan Liu
>> ---
> [...]
>>  /* 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,
>> @@ -121,6 +144,15 @@ static int libxl__device_usbctrl_add_xenstore(libxl__gc *gc, uint32_t domid,
>>  
>>          DEVICE_ADD(usbctrl, usbctrls, domid, &usbctrl_saved,
>>                     COMPARE_USBCTRL, &d_config);
>> +
>> +        if (usbctrl->type == LIBXL_USBCTRL_TYPE_QUSB) {
>> +            if (!libxl__query_qemu_backend(gc, domid, usbctrl->backend_domid,
>> +                                           "qusb", false)) {
> 
> This needs to be sorted out.

What do you mean?

> And this is maybe a rather dumb question: the xenstore paths for qusb
> and kernel backend are the same? What is the likelihood that one
> deviates from the other? Note that this is not suggesting that you
> over-engineer current code, it's just something that needs clarifying.

There is an existing pvusb kernel backend implementation in kernel-xen
of openSUSE and SLE. The port of that implementation hasn't been
accepted in Linux upstream as it was regarded to fit better in qemu.

So the qemu base backend is designed in a way to be compatible to the
already existing kernel backend. Future enhancements will be made on the
qemu based backend only (as far as SUSE is involved).

So the frontend related Xenstore paths are the same, while the backend
paths are different ("vusb" vs. "qusb"). And yes, I've tested the same
domU with both backends.


Juergen


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

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

[Wei Liu]

On Wed, Mar 30, 2016 at 04:34:20PM +0200, Juergen Gross wrote:
> On 30/03/16 16:19, Wei Liu wrote:
> > On Wed, Mar 30, 2016 at 02:05:56PM +0200, Juergen Gross 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>
> >> ---
> >> V4: bail out in case of usbback_is_loaded() error as requested by
> >>     Chun Yan Liu
> >> ---
> > [...]
> >>  /* 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,
> >> @@ -121,6 +144,15 @@ static int libxl__device_usbctrl_add_xenstore(libxl__gc *gc, uint32_t domid,
> >>  
> >>          DEVICE_ADD(usbctrl, usbctrls, domid, &usbctrl_saved,
> >>                     COMPARE_USBCTRL, &d_config);
> >> +
> >> +        if (usbctrl->type == LIBXL_USBCTRL_TYPE_QUSB) {
> >> +            if (!libxl__query_qemu_backend(gc, domid, usbctrl->backend_domid,
> >> +                                           "qusb", false)) {
> > 
> > This needs to be sorted out.
> 
> What do you mean?
> 

Nothing -- it just meant it depends on a previous patch.

> > And this is maybe a rather dumb question: the xenstore paths for qusb
> > and kernel backend are the same? What is the likelihood that one
> > deviates from the other? Note that this is not suggesting that you
> > over-engineer current code, it's just something that needs clarifying.
> 
> There is an existing pvusb kernel backend implementation in kernel-xen
> of openSUSE and SLE. The port of that implementation hasn't been
> accepted in Linux upstream as it was regarded to fit better in qemu.
> 
> So the qemu base backend is designed in a way to be compatible to the
> already existing kernel backend. Future enhancements will be made on the
> qemu based backend only (as far as SUSE is involved).
> 
> So the frontend related Xenstore paths are the same, while the backend
> paths are different ("vusb" vs. "qusb"). And yes, I've tested the same
> domU with both backends.
> 

Cool, that makes sense.

Wei.

> 
> Juergen
> 

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

Re: [PATCH v5 2/5] libxl: add query function for backend support by device model

[Anthony PERARD]

On Wed, Mar 30, 2016 at 03:26:23PM +0100, Wei Liu wrote:
> On Wed, Mar 30, 2016 at 04:11:39PM +0200, Juergen Gross wrote:
> > On 30/03/16 16:09, Wei Liu wrote:
> > > On Wed, Mar 30, 2016 at 04:02:20PM +0200, Juergen Gross wrote:
> > >> On 30/03/16 15:53, Wei Liu wrote:
> > >>> On Wed, Mar 30, 2016 at 02:05:55PM +0200, Juergen Gross wrote:
> > >>>> Add a function to query whether the device model is supporting a
> > >>>> specific backend type. The device model is writing the supported
> > >>>> backend types to Xenstore on startup. The new query function checks
> > >>>> for the appropriate entry to be present.
> > >>>>
> > >>>> As not all versions of qemu are capable to indicate support of
> > >>>> specific backends the query function is to be called with an indicator
> > >>>> whether the default return value should be "supported" (in case qemu
> > >>>> doesn't know set any support information) or "not supported".
> > >>>>
> > >>>> Signed-off-by: Juergen Gross <jgross@suse.com>
> > >>>
> > >>> The code itself looks straightforward enough.
> > >>>
> > >>> But note that this is a new protocol that needs to be supported
> > >>> essentially forever. I've CC QEMU maintainers for their input.
> > >>
> > >> Okay, I'll send the qemu patch now.
> > >>
> > > 
> > > And also a patch to docs/misc/xenstore-paths.markdown please.
> > 
> > Aah, sure.
> > 
> 
> Actually wait, I think all QEMU nodes are either under documented or I
> looked at the wrong place.

I don't think there is anything missing, in term of PV protocol, which go
through xenstore. Is there a node in particular that you are thinking of?

> Anthony, how should we document the protocol between QEMU and toolstack?
> Did I look at the wrong place?

I think xenstore-paths.markdown would be a good place to document this
protocol.

-- 
Anthony PERARD

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

Re: [PATCH v5 2/5] libxl: add query function for backend support by device model

[Wei Liu]

On Wed, Mar 30, 2016 at 04:55:42PM +0100, Anthony PERARD wrote:
> On Wed, Mar 30, 2016 at 03:26:23PM +0100, Wei Liu wrote:
> > On Wed, Mar 30, 2016 at 04:11:39PM +0200, Juergen Gross wrote:
> > > On 30/03/16 16:09, Wei Liu wrote:
> > > > On Wed, Mar 30, 2016 at 04:02:20PM +0200, Juergen Gross wrote:
> > > >> On 30/03/16 15:53, Wei Liu wrote:
> > > >>> On Wed, Mar 30, 2016 at 02:05:55PM +0200, Juergen Gross wrote:
> > > >>>> Add a function to query whether the device model is supporting a
> > > >>>> specific backend type. The device model is writing the supported
> > > >>>> backend types to Xenstore on startup. The new query function checks
> > > >>>> for the appropriate entry to be present.
> > > >>>>
> > > >>>> As not all versions of qemu are capable to indicate support of
> > > >>>> specific backends the query function is to be called with an indicator
> > > >>>> whether the default return value should be "supported" (in case qemu
> > > >>>> doesn't know set any support information) or "not supported".
> > > >>>>
> > > >>>> Signed-off-by: Juergen Gross <jgross@suse.com>
> > > >>>
> > > >>> The code itself looks straightforward enough.
> > > >>>
> > > >>> But note that this is a new protocol that needs to be supported
> > > >>> essentially forever. I've CC QEMU maintainers for their input.
> > > >>
> > > >> Okay, I'll send the qemu patch now.
> > > >>
> > > > 
> > > > And also a patch to docs/misc/xenstore-paths.markdown please.
> > > 
> > > Aah, sure.
> > > 
> > 
> > Actually wait, I think all QEMU nodes are either under documented or I
> > looked at the wrong place.
> 
> I don't think there is anything missing, in term of PV protocol, which go
> through xenstore. Is there a node in particular that you are thinking of?
> 

The node for video ram? That's what I was looking for. Then I realised
it was probably not documented but I wasn't very sure.

> > Anthony, how should we document the protocol between QEMU and toolstack?
> > Did I look at the wrong place?
> 
> I think xenstore-paths.markdown would be a good place to document this
> protocol.
> 
> -- 
> Anthony PERARD

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

Re: [PATCH v5 2/5] libxl: add query function for backend support by device model

[Anthony PERARD]

On Wed, Mar 30, 2016 at 04:59:22PM +0100, Wei Liu wrote:
> On Wed, Mar 30, 2016 at 04:55:42PM +0100, Anthony PERARD wrote:
> > On Wed, Mar 30, 2016 at 03:26:23PM +0100, Wei Liu wrote:
> > > On Wed, Mar 30, 2016 at 04:11:39PM +0200, Juergen Gross wrote:
> > > > On 30/03/16 16:09, Wei Liu wrote:
> > > > > And also a patch to docs/misc/xenstore-paths.markdown please.
> > > > 
> > > > Aah, sure.
> > > > 
> > > 
> > > Actually wait, I think all QEMU nodes are either under documented or I
> > > looked at the wrong place.
> > 
> > I don't think there is anything missing, in term of PV protocol, which go
> > through xenstore. Is there a node in particular that you are thinking of?
> > 
> 
> The node for video ram? That's what I was looking for. Then I realised
> it was probably not documented but I wasn't very sure.

Yes, this seams to be missing from the documentation.

-- 
Anthony PERARD

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

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

[Wei Liu]

On Wed, Mar 30, 2016 at 04:34:20PM +0200, Juergen Gross wrote:
> On 30/03/16 16:19, Wei Liu wrote:
> > On Wed, Mar 30, 2016 at 02:05:56PM +0200, Juergen Gross 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>
> >> ---
> >> V4: bail out in case of usbback_is_loaded() error as requested by
> >>     Chun Yan Liu
> >> ---
> > [...]
> >>  /* 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,
> >> @@ -121,6 +144,15 @@ static int libxl__device_usbctrl_add_xenstore(libxl__gc *gc, uint32_t domid,
> >>  
> >>          DEVICE_ADD(usbctrl, usbctrls, domid, &usbctrl_saved,
> >>                     COMPARE_USBCTRL, &d_config);
> >> +
> >> +        if (usbctrl->type == LIBXL_USBCTRL_TYPE_QUSB) {
> >> +            if (!libxl__query_qemu_backend(gc, domid, usbctrl->backend_domid,
> >> +                                           "qusb", false)) {
> > 
> > This needs to be sorted out.
> 
> What do you mean?
> 
> > And this is maybe a rather dumb question: the xenstore paths for qusb
> > and kernel backend are the same? What is the likelihood that one
> > deviates from the other? Note that this is not suggesting that you
> > over-engineer current code, it's just something that needs clarifying.
> 
> There is an existing pvusb kernel backend implementation in kernel-xen
> of openSUSE and SLE. The port of that implementation hasn't been
> accepted in Linux upstream as it was regarded to fit better in qemu.
> 
> So the qemu base backend is designed in a way to be compatible to the
> already existing kernel backend. Future enhancements will be made on the
> qemu based backend only (as far as SUSE is involved).
> 
> So the frontend related Xenstore paths are the same, while the backend
> paths are different ("vusb" vs. "qusb"). And yes, I've tested the same
> domU with both backends.
> 

I forgot to ask -- When you repost, can you add such information to
commit message? Thanks.

> 
> Juergen
> 

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

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

[Juergen Gross]

On 30/03/16 18:16, Wei Liu wrote:
> On Wed, Mar 30, 2016 at 04:34:20PM +0200, Juergen Gross wrote:
>> On 30/03/16 16:19, Wei Liu wrote:
>>> On Wed, Mar 30, 2016 at 02:05:56PM +0200, Juergen Gross 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>
>>>> ---
>>>> V4: bail out in case of usbback_is_loaded() error as requested by
>>>>     Chun Yan Liu
>>>> ---
>>> [...]
>>>>  /* 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,
>>>> @@ -121,6 +144,15 @@ static int libxl__device_usbctrl_add_xenstore(libxl__gc *gc, uint32_t domid,
>>>>  
>>>>          DEVICE_ADD(usbctrl, usbctrls, domid, &usbctrl_saved,
>>>>                     COMPARE_USBCTRL, &d_config);
>>>> +
>>>> +        if (usbctrl->type == LIBXL_USBCTRL_TYPE_QUSB) {
>>>> +            if (!libxl__query_qemu_backend(gc, domid, usbctrl->backend_domid,
>>>> +                                           "qusb", false)) {
>>>
>>> This needs to be sorted out.
>>
>> What do you mean?
>>
>>> And this is maybe a rather dumb question: the xenstore paths for qusb
>>> and kernel backend are the same? What is the likelihood that one
>>> deviates from the other? Note that this is not suggesting that you
>>> over-engineer current code, it's just something that needs clarifying.
>>
>> There is an existing pvusb kernel backend implementation in kernel-xen
>> of openSUSE and SLE. The port of that implementation hasn't been
>> accepted in Linux upstream as it was regarded to fit better in qemu.
>>
>> So the qemu base backend is designed in a way to be compatible to the
>> already existing kernel backend. Future enhancements will be made on the
>> qemu based backend only (as far as SUSE is involved).
>>
>> So the frontend related Xenstore paths are the same, while the backend
>> paths are different ("vusb" vs. "qusb"). And yes, I've tested the same
>> domU with both backends.
>>
> 
> I forgot to ask -- When you repost, can you add such information to
> commit message? Thanks.

Sure.


Juergen


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