[PATCH v2 0/3] libxl: add support for qemu base pvusb backend

[PATCH v2 0/3] 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 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 (3):
  libxl: make libxl__need_xenpv_qemu() operate on domain config
  libxl: add new pvusb backend "qusb" provided by qemu
  libxl: add domain config parameter to force start of qemu

 docs/man/xl.cfg.pod.5                |  17 +++++-
 tools/libxl/libxl_create.c           |  10 +---
 tools/libxl/libxl_device.c           |   3 +-
 tools/libxl/libxl_dm.c               |  64 +++++++++-------------
 tools/libxl/libxl_internal.h         |   6 +--
 tools/libxl/libxl_pvusb.c            | 102 +++++++++++++++++++++++++++--------
 tools/libxl/libxl_types.idl          |   2 +
 tools/libxl/libxl_types_internal.idl |   1 +
 tools/libxl/xl_cmdimpl.c             |   3 ++
 9 files changed, 131 insertions(+), 77 deletions(-)

-- 
2.6.2


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

[PATCH v2 1/3] 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

[PATCH v2 2/3] 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

[PATCH v2 3/3] 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 possible 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 361e584..767959d 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 v2 0/3] libxl: add support for qemu base pvusb backend

[Juergen Gross]

On 22/03/16 08:29, Juergen Gross wrote:
> 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 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 (3):
>   libxl: make libxl__need_xenpv_qemu() operate on domain config
>   libxl: add new pvusb backend "qusb" provided by qemu
>   libxl: add domain config parameter to force start of qemu

I had a try with detecting a missing device model when adding a device
to an already running domain. Instead of silently failing it would now
print an appropriate error message. This could be changed later to
start the device model in that situation.

Would you like me to send another version of the complete series (the
v2 patches are not changed), or the new patches only?


Juergen

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

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

[Konrad Rzeszutek Wilk]

On Tue, Mar 22, 2016 at 08:29:23AM +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 possible 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.

.. s/devices/PV devices/

As surely PV guests can't use emulated devices. Or could they? That
would be quite interesting in a not kind of fun way.

> 
> 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.

But you are not using this now (late start of QEMU), so why this patch?

> 
> 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 361e584..767959d 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

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

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

[Konrad Rzeszutek Wilk]

On Tue, Mar 22, 2016 at 08:29:22AM +0100, 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>
> ---
>  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;

I believe the CODING STYLE in libxl asks for you to use { } for these
ones.

> +    LOGE(ERROR, "Accessing %s", SYSFS_USBBACK_DRIVER);

Why is this an error?

> +    return -1;
> +}
> +

Otherwise looks OK to me.

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

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

[Juergen Gross]

On 28/03/16 16:50, Konrad Rzeszutek Wilk wrote:
> On Tue, Mar 22, 2016 at 08:29:23AM +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 possible 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.
> 
> .. s/devices/PV devices/
> 
> As surely PV guests can't use emulated devices. Or could they? That
> would be quite interesting in a not kind of fun way.

Would require some work.

> 
>>
>> 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.
> 
> But you are not using this now (late start of QEMU), so why this patch?

This was the reason for the patch: start the device model early in order
to have it running in case it would be needed later.

Juergen

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

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

[Juergen Gross]

On 28/03/16 16:55, Konrad Rzeszutek Wilk wrote:
> On Tue, Mar 22, 2016 at 08:29:22AM +0100, 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>
>> ---
>>  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;
> 
> I believe the CODING STYLE in libxl asks for you to use { } for these
> ones.

No, it doesn't:

Quote from tools/libxl/CODING_STYLE:

5. Block structure

Every indented statement is braced apart from blocks that contain just
one statement.

> 
>> +    LOGE(ERROR, "Accessing %s", SYSFS_USBBACK_DRIVER);
> 
> Why is this an error?

What else? We can't determine whether the driver is loaded or not.
ENOENT is tested above, so it must be something weird.

> 
>> +    return -1;
>> +}
>> +
> 
> Otherwise looks OK to me.

Thanks,

Juergen


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

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

[Konrad Rzeszutek Wilk]

> >> +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;
> > 
> > I believe the CODING STYLE in libxl asks for you to use { } for these
> > ones.
> 
> No, it doesn't:
> 
> Quote from tools/libxl/CODING_STYLE:
> 
> 5. Block structure
> 
> Every indented statement is braced apart from blocks that contain just
> one statement.

You are right. I am so used to 'if (..) else'!
> 
> > 
> >> +    LOGE(ERROR, "Accessing %s", SYSFS_USBBACK_DRIVER);
> > 
> > Why is this an error?
> 
> What else? We can't determine whether the driver is loaded or not.
> ENOENT is tested above, so it must be something weird.

Or it could be EPERM.
> 
> > 
> >> +    return -1;

Which results in the code assuming (the caller of this function)
it is an kernel driver. Is that OK? Or should we bail out completly?

> >> +}
> >> +
> > 
> > Otherwise looks OK to me.
> 
> Thanks,
> 
> Juergen
> 

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

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

[Konrad Rzeszutek Wilk]

On Tue, Mar 29, 2016 at 06:44:32AM +0200, Juergen Gross wrote:
> On 28/03/16 16:50, Konrad Rzeszutek Wilk wrote:
> > On Tue, Mar 22, 2016 at 08:29:23AM +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 possible 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.
> > 
> > .. s/devices/PV devices/
> > 
> > As surely PV guests can't use emulated devices. Or could they? That
> > would be quite interesting in a not kind of fun way.
> 
> Would require some work.
> 
> > 
> >>
> >> 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.
> > 
> > But you are not using this now (late start of QEMU), so why this patch?
> 
> This was the reason for the patch: start the device model early in order
> to have it running in case it would be needed later.


OK. However I am missing something here. My understanding is that the
QEMU Xen PV USB backend is not in the code-base. So why do this if QEMU
won't even support this?

Would it make more sense to wait until QEMU has this support, then
have this patch, or work on making libxl be able to start QEMU later
(during the time QEMU gains the XEn PV USB backend)?

> 
> Juergen

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

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

[Juergen Gross]

On 29/03/16 16:24, Konrad Rzeszutek Wilk wrote:
>>>> +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;
>>>
>>> I believe the CODING STYLE in libxl asks for you to use { } for these
>>> ones.
>>
>> No, it doesn't:
>>
>> Quote from tools/libxl/CODING_STYLE:
>>
>> 5. Block structure
>>
>> Every indented statement is braced apart from blocks that contain just
>> one statement.
> 
> You are right. I am so used to 'if (..) else'!
>>
>>>
>>>> +    LOGE(ERROR, "Accessing %s", SYSFS_USBBACK_DRIVER);
>>>
>>> Why is this an error?
>>
>> What else? We can't determine whether the driver is loaded or not.
>> ENOENT is tested above, so it must be something weird.
> 
> Or it could be EPERM.

Which is weird in this case. :-)

>>
>>>
>>>> +    return -1;
> 
> Which results in the code assuming (the caller of this function)
> it is an kernel driver. Is that OK? Or should we bail out completly?

Have a look in V4 of the patches. :-)


Juergen

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

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

[Juergen Gross]

On 29/03/16 16:27, Konrad Rzeszutek Wilk wrote:
> On Tue, Mar 29, 2016 at 06:44:32AM +0200, Juergen Gross wrote:
>> On 28/03/16 16:50, Konrad Rzeszutek Wilk wrote:
>>> On Tue, Mar 22, 2016 at 08:29:23AM +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 possible 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.
>>>
>>> .. s/devices/PV devices/
>>>
>>> As surely PV guests can't use emulated devices. Or could they? That
>>> would be quite interesting in a not kind of fun way.
>>
>> Would require some work.
>>
>>>
>>>>
>>>> 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.
>>>
>>> But you are not using this now (late start of QEMU), so why this patch?
>>
>> This was the reason for the patch: start the device model early in order
>> to have it running in case it would be needed later.
> 
> 
> OK. However I am missing something here. My understanding is that the
> QEMU Xen PV USB backend is not in the code-base. So why do this if QEMU
> won't even support this?
> 
> Would it make more sense to wait until QEMU has this support, then
> have this patch, or work on making libxl be able to start QEMU later
> (during the time QEMU gains the XEn PV USB backend)?

The qemu patches are sent already. I hope they are being accepted
rather soon.

I'm planning to add support for starting qemu as needed when 4.7 is
out.


Juergen

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