[PATCH v2 0/4] libxl: add framework for device types

[PATCH v2 0/4] libxl: add framework for device types

[Juergen Gross]

Instead of duplicate coding for each device type (vtpms, usbctrls, ...)
especially on domain creation introduce a framework for that purpose.

I especially found it annoying that e.g. the vtpm callback issued the
error message for a failed attach of nic devices.

Changes in V2:
- added new patch 4 to move more pvusb specific stuff into libxl_pvusb.c
- patch 1: add macro to fill struct libxl__device_type as suggested by
  Ian Jackson
- patch 1: make struct libxl__device_type variables const as requested by
  Ian Jackson

Juergen Gross (4):
  libxl: add framework for device types
  libxl: refactor domcreate_attach_pci() to use device type framework
  libxl: refactor domcreate_attach_dtdev() to use device type framework
  libxl: move DEFINE_DEVICE* macros to libxl_internal.h

 tools/libxl/libxl.c          | 146 +++++------------------
 tools/libxl/libxl_create.c   | 271 +++++++++++++------------------------------
 tools/libxl/libxl_device.c   |  36 ------
 tools/libxl/libxl_internal.h | 127 ++++++++++++++------
 tools/libxl/libxl_pci.c      |  32 +++++
 tools/libxl/libxl_pvusb.c    |  29 +++--
 6 files changed, 255 insertions(+), 386 deletions(-)

-- 
2.6.6


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

[PATCH v2 1/4] libxl: add framework for device types

[Juergen Gross]

Instead of duplicate coding for each device type (vtpms, usbctrls, ...)
especially on domain creation introduce a framework for that purpose.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2: - add macro to fill struct libxl__device_type as suggested by
      Ian Jackson
    - make struct libxl__device_type variables const as requested by
      Ian Jackson
---
 tools/libxl/libxl.c          |   3 +
 tools/libxl/libxl_create.c   | 163 +++++++++++++------------------------------
 tools/libxl/libxl_internal.h |  20 ++++++
 tools/libxl/libxl_pvusb.c    |   4 ++
 4 files changed, 76 insertions(+), 114 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 1c81239..b3deef0 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -7434,6 +7434,9 @@ out:
     return rc;
 }
 
+DEFINE_DEVICE_TYPE_STRUCT(nic);
+DEFINE_DEVICE_TYPE_STRUCT(vtpm);
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 1b99472..5e05f6f 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -742,12 +742,6 @@ static void domcreate_bootloader_done(libxl__egc *egc,
 static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *aodevs,
                                 int ret);
 
-static void domcreate_attach_vtpms(libxl__egc *egc, libxl__multidev *multidev,
-                                   int ret);
-static void domcreate_attach_usbctrls(libxl__egc *egc,
-                                      libxl__multidev *multidev, int ret);
-static void domcreate_attach_usbdevs(libxl__egc *egc, libxl__multidev *multidev,
-                                     int ret);
 static void domcreate_attach_pci(libxl__egc *egc, libxl__multidev *aodevs,
                                  int ret);
 static void domcreate_attach_dtdev(libxl__egc *egc,
@@ -1407,6 +1401,53 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
     domcreate_complete(egc, dcs, ret);
 }
 
+static const struct libxl_device_type *device_type_tbl[] = {
+    &libxl__nic_devtype,
+    &libxl__vtpm_devtype,
+    &libxl__usbctrl_devtype,
+    &libxl__usbdev_devtype,
+};
+
+static void domcreate_attach_devices(libxl__egc *egc,
+                                     libxl__multidev *multidev,
+                                     int ret)
+{
+    libxl__domain_create_state *dcs = CONTAINER_OF(multidev, *dcs, multidev);
+    STATE_AO_GC(dcs->ao);
+    int domid = dcs->guest_domid;
+    libxl_domain_config *const d_config = dcs->guest_config;
+    const struct libxl_device_type *dt;
+
+    if (ret) {
+        LOG(ERROR, "unable to add %s devices",
+            device_type_tbl[dcs->device_type_idx]->type);
+        goto error_out;
+    }
+
+    dcs->device_type_idx++;
+    if (dcs->device_type_idx < ARRAY_SIZE(device_type_tbl)) {
+        dt = device_type_tbl[dcs->device_type_idx];
+        if (*(int *)((void *)d_config + dt->num_offset) > 0) {
+            /* Attach devices */
+            libxl__multidev_begin(ao, &dcs->multidev);
+            dcs->multidev.callback = domcreate_attach_devices;
+            dt->add(egc, ao, domid, d_config, &dcs->multidev);
+            libxl__multidev_prepared(egc, &dcs->multidev, 0);
+            return;
+        }
+
+        domcreate_attach_devices(egc, &dcs->multidev, 0);
+        return;
+    }
+
+    domcreate_attach_pci(egc, multidev, 0);
+    return;
+
+error_out:
+    assert(ret);
+    domcreate_complete(egc, dcs, ret);
+}
+
 static void domcreate_devmodel_started(libxl__egc *egc,
                                        libxl__dm_spawn_state *dmss,
                                        int ret)
@@ -1430,113 +1471,8 @@ static void domcreate_devmodel_started(libxl__egc *egc,
         }
     }
 
-    /* Plug nic interfaces */
-    if (d_config->num_nics > 0) {
-        /* Attach nics */
-        libxl__multidev_begin(ao, &dcs->multidev);
-        dcs->multidev.callback = domcreate_attach_vtpms;
-        libxl__add_nics(egc, ao, domid, d_config, &dcs->multidev);
-        libxl__multidev_prepared(egc, &dcs->multidev, 0);
-        return;
-    }
-
-    domcreate_attach_vtpms(egc, &dcs->multidev, 0);
-    return;
-
-error_out:
-    assert(ret);
-    domcreate_complete(egc, dcs, ret);
-}
-
-static void domcreate_attach_vtpms(libxl__egc *egc,
-                                   libxl__multidev *multidev,
-                                   int ret)
-{
-   libxl__domain_create_state *dcs = CONTAINER_OF(multidev, *dcs, multidev);
-   STATE_AO_GC(dcs->ao);
-   int domid = dcs->guest_domid;
-
-   libxl_domain_config* const d_config = dcs->guest_config;
-
-   if(ret) {
-       LOG(ERROR, "unable to add nic devices");
-       goto error_out;
-   }
-
-    /* Plug vtpm devices */
-   if (d_config->num_vtpms > 0) {
-       /* Attach vtpms */
-       libxl__multidev_begin(ao, &dcs->multidev);
-       dcs->multidev.callback = domcreate_attach_usbctrls;
-       libxl__add_vtpms(egc, ao, domid, d_config, &dcs->multidev);
-       libxl__multidev_prepared(egc, &dcs->multidev, 0);
-       return;
-   }
-
-   domcreate_attach_usbctrls(egc, multidev, 0);
-   return;
-
-error_out:
-   assert(ret);
-   domcreate_complete(egc, dcs, ret);
-}
-
-static void domcreate_attach_usbctrls(libxl__egc *egc,
-                                      libxl__multidev *multidev, int ret)
-{
-    libxl__domain_create_state *dcs = CONTAINER_OF(multidev, *dcs, multidev);
-    STATE_AO_GC(dcs->ao);
-    int domid = dcs->guest_domid;
-
-    libxl_domain_config *const d_config = dcs->guest_config;
-
-    if (ret) {
-        LOG(ERROR, "unable to add vtpm devices");
-        goto error_out;
-    }
-
-    if (d_config->num_usbctrls > 0) {
-        /* Attach usbctrls */
-        libxl__multidev_begin(ao, &dcs->multidev);
-        dcs->multidev.callback = domcreate_attach_usbdevs;
-        libxl__add_usbctrls(egc, ao, domid, d_config, &dcs->multidev);
-        libxl__multidev_prepared(egc, &dcs->multidev, 0);
-        return;
-    }
-
-    domcreate_attach_usbdevs(egc, multidev, 0);
-    return;
-
-error_out:
-    assert(ret);
-    domcreate_complete(egc, dcs, ret);
-}
-
-
-static void domcreate_attach_usbdevs(libxl__egc *egc, libxl__multidev *multidev,
-                                int ret)
-{
-    libxl__domain_create_state *dcs = CONTAINER_OF(multidev, *dcs, multidev);
-    STATE_AO_GC(dcs->ao);
-    int domid = dcs->guest_domid;
-
-    libxl_domain_config *const d_config = dcs->guest_config;
-
-    if (ret) {
-        LOG(ERROR, "unable to add usbctrl devices");
-        goto error_out;
-    }
-
-    if (d_config->num_usbdevs > 0) {
-        /* Attach usbctrls */
-        libxl__multidev_begin(ao, &dcs->multidev);
-        dcs->multidev.callback = domcreate_attach_pci;
-        libxl__add_usbdevs(egc, ao, domid, d_config, &dcs->multidev);
-        libxl__multidev_prepared(egc, &dcs->multidev, 0);
-        return;
-    }
-
-    domcreate_attach_pci(egc, multidev, 0);
+    dcs->device_type_idx = -1;
+    domcreate_attach_devices(egc, &dcs->multidev, 0);
     return;
 
 error_out:
@@ -1556,7 +1492,6 @@ static void domcreate_attach_pci(libxl__egc *egc, libxl__multidev *multidev,
     libxl_domain_config *const d_config = dcs->guest_config;
 
     if (ret) {
-        LOG(ERROR, "unable to add usb devices");
         goto error_out;
     }
 
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index e7ab85d..d16161a 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3389,6 +3389,25 @@ _hidden void libxl__bootloader_init(libxl__bootloader_state *bl);
  * If callback is passed rc==0, will have updated st->info appropriately */
 _hidden void libxl__bootloader_run(libxl__egc*, libxl__bootloader_state *st);
 
+/*----- Generic Device Handling -----*/
+struct libxl_device_type {
+    char *type;
+    int num_offset;   /* Offset of # of devices in libxl_domain_config */
+    void (*add)(libxl__egc *, libxl__ao *, uint32_t, libxl_domain_config *,
+                libxl__multidev *);
+};
+
+#define DEFINE_DEVICE_TYPE_STRUCT(name)                                 \
+    const struct libxl_device_type libxl__ ## name ## _devtype = {      \
+        .type       = #name,                                            \
+        .num_offset = offsetof(libxl_domain_config, num_ ## name ## s), \
+        .add        = libxl__add_ ## name ## s,                         \
+    }
+
+extern const struct libxl_device_type libxl__nic_devtype;
+extern const struct libxl_device_type libxl__vtpm_devtype;
+extern const struct libxl_device_type libxl__usbctrl_devtype;
+extern const struct libxl_device_type libxl__usbdev_devtype;
 /*----- Domain destruction -----*/
 
 /* Domain destruction has been split into two functions:
@@ -3565,6 +3584,7 @@ struct libxl__domain_create_state {
     libxl_asyncprogress_how aop_console_how;
     /* private to domain_create */
     int guest_domid;
+    int device_type_idx;
     const char *colo_proxy_script;
     libxl__domain_build_state build_state;
     libxl__colo_restore_state crs;
diff --git a/tools/libxl/libxl_pvusb.c b/tools/libxl/libxl_pvusb.c
index 885f0d4..5edd206 100644
--- a/tools/libxl/libxl_pvusb.c
+++ b/tools/libxl/libxl_pvusb.c
@@ -1667,6 +1667,10 @@ out:
     GC_FREE;
     return rc;
 }
+
+DEFINE_DEVICE_TYPE_STRUCT(usbctrl);
+DEFINE_DEVICE_TYPE_STRUCT(usbdev);
+
 /*
  * Local variables:
  * mode: C
-- 
2.6.6


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

[PATCH v2 2/4] libxl: refactor domcreate_attach_pci() to use device type framework

[Juergen Gross]

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/libxl/libxl_create.c   | 54 ++++++--------------------------------------
 tools/libxl/libxl_internal.h |  1 +
 tools/libxl/libxl_pci.c      | 32 ++++++++++++++++++++++++++
 3 files changed, 40 insertions(+), 47 deletions(-)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 5e05f6f..354c73f 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -742,10 +742,8 @@ static void domcreate_bootloader_done(libxl__egc *egc,
 static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *aodevs,
                                 int ret);
 
-static void domcreate_attach_pci(libxl__egc *egc, libxl__multidev *aodevs,
-                                 int ret);
-static void domcreate_attach_dtdev(libxl__egc *egc,
-                                   libxl__domain_create_state *dcs);
+static void domcreate_attach_dtdev(libxl__egc *egc, libxl__multidev *multidev,
+                                   int ret);
 
 static void domcreate_console_available(libxl__egc *egc,
                                         libxl__domain_create_state *dcs);
@@ -1406,6 +1404,7 @@ static const struct libxl_device_type *device_type_tbl[] = {
     &libxl__vtpm_devtype,
     &libxl__usbctrl_devtype,
     &libxl__usbdev_devtype,
+    &libxl__pcidev_devtype,
 };
 
 static void domcreate_attach_devices(libxl__egc *egc,
@@ -1440,7 +1439,7 @@ static void domcreate_attach_devices(libxl__egc *egc,
         return;
     }
 
-    domcreate_attach_pci(egc, multidev, 0);
+    domcreate_attach_dtdev(egc, multidev, 0);
     return;
 
 error_out:
@@ -1480,52 +1479,13 @@ error_out:
     domcreate_complete(egc, dcs, ret);
 }
 
-static void domcreate_attach_pci(libxl__egc *egc, libxl__multidev *multidev,
-                                 int ret)
-{
-    libxl__domain_create_state *dcs = CONTAINER_OF(multidev, *dcs, multidev);
-    STATE_AO_GC(dcs->ao);
-    int i;
-    int domid = dcs->guest_domid;
-
-    /* convenience aliases */
-    libxl_domain_config *const d_config = dcs->guest_config;
-
-    if (ret) {
-        goto error_out;
-    }
-
-    for (i = 0; i < d_config->num_pcidevs; i++) {
-        ret = libxl__device_pci_add(gc, domid, &d_config->pcidevs[i], 1);
-        if (ret < 0) {
-            LOG(ERROR, "libxl_device_pci_add failed: %d", ret);
-            goto error_out;
-        }
-    }
-
-    if (d_config->num_pcidevs > 0) {
-        ret = libxl__create_pci_backend(gc, domid, d_config->pcidevs,
-            d_config->num_pcidevs);
-        if (ret < 0) {
-            LOG(ERROR, "libxl_create_pci_backend failed: %d", ret);
-            goto error_out;
-        }
-    }
-
-    domcreate_attach_dtdev(egc, dcs);
-    return;
-
-error_out:
-    assert(ret);
-    domcreate_complete(egc, dcs, ret);
-}
-
 static void domcreate_attach_dtdev(libxl__egc *egc,
-                                   libxl__domain_create_state *dcs)
+                                   libxl__multidev *multidev,
+                                   int ret)
 {
+    libxl__domain_create_state *dcs = CONTAINER_OF(multidev, *dcs, multidev);
     STATE_AO_GC(dcs->ao);
     int i;
-    int ret;
     int domid = dcs->guest_domid;
 
     /* convenience aliases */
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index d16161a..79ce392 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3408,6 +3408,7 @@ extern const struct libxl_device_type libxl__nic_devtype;
 extern const struct libxl_device_type libxl__vtpm_devtype;
 extern const struct libxl_device_type libxl__usbctrl_devtype;
 extern const struct libxl_device_type libxl__usbdev_devtype;
+extern const struct libxl_device_type libxl__pcidev_devtype;
 /*----- Domain destruction -----*/
 
 /* Domain destruction has been split into two functions:
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 236bdd0..9676687 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -1291,6 +1291,36 @@ out:
     return rc;
 }
 
+static void libxl__add_pcidevs(libxl__egc *egc, libxl__ao *ao, uint32_t domid,
+                               libxl_domain_config *d_config,
+                               libxl__multidev *multidev)
+{
+    AO_GC;
+    libxl__ao_device *aodev = libxl__multidev_prepare(multidev);
+    int i, rc = 0;
+
+    for (i = 0; i < d_config->num_pcidevs; i++) {
+        rc = libxl__device_pci_add(gc, domid, &d_config->pcidevs[i], 1);
+        if (rc < 0) {
+            LOG(ERROR, "libxl_device_pci_add failed: %d", rc);
+            goto out;
+        }
+    }
+
+    if (d_config->num_pcidevs > 0) {
+        rc = libxl__create_pci_backend(gc, domid, d_config->pcidevs,
+            d_config->num_pcidevs);
+        if (rc < 0) {
+            LOG(ERROR, "libxl_create_pci_backend failed: %d", rc);
+            goto out;
+        }
+    }
+
+out:
+    aodev->rc = rc;
+    aodev->callback(egc, aodev);
+}
+
 static int qemu_pci_remove_xenstore(libxl__gc *gc, uint32_t domid,
                                     libxl_device_pci *pcidev, int force)
 {
@@ -1668,6 +1698,8 @@ int libxl__grant_vga_iomem_permission(libxl__gc *gc, const uint32_t domid,
     return 0;
 }
 
+DEFINE_DEVICE_TYPE_STRUCT(pcidev);
+
 /*
  * Local variables:
  * mode: C
-- 
2.6.6


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

[PATCH v2 3/4] libxl: refactor domcreate_attach_dtdev() to use device type framework

[Juergen Gross]

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/libxl/libxl_create.c | 68 +++++++++++++++++++++-------------------------
 1 file changed, 31 insertions(+), 37 deletions(-)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 354c73f..828f254 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -742,9 +742,6 @@ static void domcreate_bootloader_done(libxl__egc *egc,
 static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *aodevs,
                                 int ret);
 
-static void domcreate_attach_dtdev(libxl__egc *egc, libxl__multidev *multidev,
-                                   int ret);
-
 static void domcreate_console_available(libxl__egc *egc,
                                         libxl__domain_create_state *dcs);
 
@@ -1399,12 +1396,39 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
     domcreate_complete(egc, dcs, ret);
 }
 
+static void libxl__add_dtdevs(libxl__egc *egc, libxl__ao *ao, uint32_t domid,
+                              libxl_domain_config *d_config,
+                              libxl__multidev *multidev)
+{
+    AO_GC;
+    libxl__ao_device *aodev = libxl__multidev_prepare(multidev);
+    int i, rc = 0;
+
+    for (i = 0; i < d_config->num_dtdevs; i++) {
+        const libxl_device_dtdev *dtdev = &d_config->dtdevs[i];
+
+        LOG(DEBUG, "Assign device \"%s\" to dom%u", dtdev->path, domid);
+        rc = xc_assign_dt_device(CTX->xch, domid, dtdev->path);
+        if (rc < 0) {
+            LOG(ERROR, "xc_assign_dtdevice failed: %d", rc);
+            goto out;
+        }
+    }
+
+out:
+    aodev->rc = rc;
+    aodev->callback(egc, aodev);
+}
+
+static DEFINE_DEVICE_TYPE_STRUCT(dtdev);
+
 static const struct libxl_device_type *device_type_tbl[] = {
     &libxl__nic_devtype,
     &libxl__vtpm_devtype,
     &libxl__usbctrl_devtype,
     &libxl__usbdev_devtype,
     &libxl__pcidev_devtype,
+    &libxl__dtdev_devtype,
 };
 
 static void domcreate_attach_devices(libxl__egc *egc,
@@ -1439,7 +1463,10 @@ static void domcreate_attach_devices(libxl__egc *egc,
         return;
     }
 
-    domcreate_attach_dtdev(egc, multidev, 0);
+    domcreate_console_available(egc, dcs);
+
+    domcreate_complete(egc, dcs, 0);
+
     return;
 
 error_out:
@@ -1479,39 +1506,6 @@ error_out:
     domcreate_complete(egc, dcs, ret);
 }
 
-static void domcreate_attach_dtdev(libxl__egc *egc,
-                                   libxl__multidev *multidev,
-                                   int ret)
-{
-    libxl__domain_create_state *dcs = CONTAINER_OF(multidev, *dcs, multidev);
-    STATE_AO_GC(dcs->ao);
-    int i;
-    int domid = dcs->guest_domid;
-
-    /* convenience aliases */
-    libxl_domain_config *const d_config = dcs->guest_config;
-
-    for (i = 0; i < d_config->num_dtdevs; i++) {
-        const libxl_device_dtdev *dtdev = &d_config->dtdevs[i];
-
-        LOG(DEBUG, "Assign device \"%s\" to dom%u", dtdev->path, domid);
-        ret = xc_assign_dt_device(CTX->xch, domid, dtdev->path);
-        if (ret < 0) {
-            LOG(ERROR, "xc_assign_dtdevice failed: %d", ret);
-            goto error_out;
-        }
-    }
-
-    domcreate_console_available(egc, dcs);
-
-    domcreate_complete(egc, dcs, 0);
-    return;
-
-error_out:
-    assert(ret);
-    domcreate_complete(egc, dcs, ret);
-}
-
 static void domcreate_complete(libxl__egc *egc,
                                libxl__domain_create_state *dcs,
                                int rc)
-- 
2.6.6


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

[PATCH v2 4/4] libxl: move DEFINE_DEVICE* macros to libxl_internal.h

[Juergen Gross]

In order to be able to have all functions related to a device type in
a single source file move the macros used to generate device type
specific functions to libxl_internal.h. Rename the macros as they are
no longer local to a source file. While at it hide device remove and
device destroy in one macro as those are always used in pairs. Move
usage of the macros to the appropriate source files.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/libxl/libxl.c          | 143 +++++++++----------------------------------
 tools/libxl/libxl_device.c   |  36 -----------
 tools/libxl/libxl_internal.h | 106 +++++++++++++++++++++-----------
 tools/libxl/libxl_pvusb.c    |  25 +++++---
 4 files changed, 115 insertions(+), 195 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index b3deef0..68b77b9 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1980,7 +1980,7 @@ out:
 /******************************************************************************/
 
 /* generic callback for devices that only need to set ao_complete */
-static void device_addrm_aocomplete(libxl__egc *egc, libxl__ao_device *aodev)
+void device_addrm_aocomplete(libxl__egc *egc, libxl__ao_device *aodev)
 {
     STATE_AO_GC(aodev->ao);
 
@@ -2055,9 +2055,9 @@ static int libxl__device_from_vtpm(libxl__gc *gc, uint32_t domid,
    return 0;
 }
 
-void libxl__device_vtpm_add(libxl__egc *egc, uint32_t domid,
-                           libxl_device_vtpm *vtpm,
-                           libxl__ao_device *aodev)
+static void libxl__device_vtpm_add(libxl__egc *egc, uint32_t domid,
+                                   libxl_device_vtpm *vtpm,
+                                   libxl__ao_device *aodev)
 {
     STATE_AO_GC(aodev->ao);
     flexarray_t *front;
@@ -2629,8 +2629,9 @@ out:
     return;
 }
 
-void libxl__device_disk_add(libxl__egc *egc, uint32_t domid,
-                           libxl_device_disk *disk, libxl__ao_device *aodev)
+static void libxl__device_disk_add(libxl__egc *egc, uint32_t domid,
+                                   libxl_device_disk *disk,
+                                   libxl__ao_device *aodev)
 {
     device_disk_add(egc, domid, disk, aodev, NULL, NULL);
 }
@@ -3432,8 +3433,9 @@ static int libxl__device_from_nic(libxl__gc *gc, uint32_t domid,
     return 0;
 }
 
-void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
-                           libxl_device_nic *nic, libxl__ao_device *aodev)
+static void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
+                                  libxl_device_nic *nic,
+                                  libxl__ao_device *aodev)
 {
     STATE_AO_GC(aodev->ao);
     flexarray_t *front;
@@ -4298,136 +4300,49 @@ out:
 
 /******************************************************************************/
 
-/* Macro for defining device remove/destroy functions in a compact way */
 /* The following functions are defined:
+ * libxl_device_disk_add
+ * libxl__add_disks
  * libxl_device_disk_remove
  * libxl_device_disk_destroy
+ * libxl_device_nic_add
+ * libxl__add_nics
  * libxl_device_nic_remove
  * libxl_device_nic_destroy
+ * libxl_device_vtpm_add
+ * libxl__add_vtpms
  * libxl_device_vtpm_remove
  * libxl_device_vtpm_destroy
  * libxl_device_vkb_remove
  * libxl_device_vkb_destroy
  * libxl_device_vfb_remove
  * libxl_device_vfb_destroy
- * libxl_device_usbctrl_remove
- * libxl_device_usbctrl_destroy
  */
-#define DEFINE_DEVICE_REMOVE_EXT(type, remtype, removedestroy, f)        \
-    int libxl_device_##type##_##removedestroy(libxl_ctx *ctx,           \
-        uint32_t domid, libxl_device_##type *type,                      \
-        const libxl_asyncop_how *ao_how)                                \
-    {                                                                   \
-        AO_CREATE(ctx, domid, ao_how);                                  \
-        libxl__device *device;                                          \
-        libxl__ao_device *aodev;                                        \
-        int rc;                                                         \
-                                                                        \
-        GCNEW(device);                                                  \
-        rc = libxl__device_from_##type(gc, domid, type, device);        \
-        if (rc != 0) goto out;                                          \
-                                                                        \
-        GCNEW(aodev);                                                   \
-        libxl__prepare_ao_device(ao, aodev);                            \
-        aodev->action = LIBXL__DEVICE_ACTION_REMOVE;                    \
-        aodev->dev = device;                                            \
-        aodev->callback = device_addrm_aocomplete;                      \
-        aodev->force = f;                                               \
-        libxl__initiate_device_##remtype##_remove(egc, aodev);          \
-                                                                        \
-    out:                                                                \
-        if (rc) return AO_CREATE_FAIL(rc);                              \
-        return AO_INPROGRESS;                                           \
-    }
-
-#define DEFINE_DEVICE_REMOVE(type, removedestroy, f) \
-    DEFINE_DEVICE_REMOVE_EXT(type, generic, removedestroy, f)
-
-#define DEFINE_DEVICE_REMOVE_CUSTOM(type, removedestroy, f)  \
-    DEFINE_DEVICE_REMOVE_EXT(type, type, removedestroy, f)
-
-/* Define all remove/destroy functions and undef the macro */
-
-/* disk */
-DEFINE_DEVICE_REMOVE(disk, remove, 0)
-DEFINE_DEVICE_REMOVE(disk, destroy, 1)
-
-/* nic */
-DEFINE_DEVICE_REMOVE(nic, remove, 0)
-DEFINE_DEVICE_REMOVE(nic, destroy, 1)
-
-/* vkb */
-DEFINE_DEVICE_REMOVE(vkb, remove, 0)
-DEFINE_DEVICE_REMOVE(vkb, destroy, 1)
-
-/* vfb */
-
-DEFINE_DEVICE_REMOVE(vfb, remove, 0)
-DEFINE_DEVICE_REMOVE(vfb, destroy, 1)
-
-/* vtpm */
-DEFINE_DEVICE_REMOVE(vtpm, remove, 0)
-DEFINE_DEVICE_REMOVE(vtpm, destroy, 1)
-
-/* usbctrl */
-DEFINE_DEVICE_REMOVE_CUSTOM(usbctrl, remove, 0)
-DEFINE_DEVICE_REMOVE_CUSTOM(usbctrl, destroy, 1)
 
 /* channel/console hotunplug is not implemented. There are 2 possibilities:
  * 1. add support for secondary consoles to xenconsoled
  * 2. dynamically add/remove qemu chardevs via qmp messages. */
 
-#undef DEFINE_DEVICE_REMOVE
-#undef DEFINE_DEVICE_REMOVE_CUSTOM
-#undef DEFINE_DEVICE_REMOVE_EXT
-
-/******************************************************************************/
-
-/* Macro for defining device addition functions in a compact way */
-/* The following functions are defined:
- * libxl_device_disk_add
- * libxl_device_nic_add
- * libxl_device_vtpm_add
- * libxl_device_usbctrl_add
- * libxl_device_usbdev_add
- */
-
-#define DEFINE_DEVICE_ADD(type)                                         \
-    int libxl_device_##type##_add(libxl_ctx *ctx,                       \
-        uint32_t domid, libxl_device_##type *type,                      \
-        const libxl_asyncop_how *ao_how)                                \
-    {                                                                   \
-        AO_CREATE(ctx, domid, ao_how);                                  \
-        libxl__ao_device *aodev;                                        \
-                                                                        \
-        GCNEW(aodev);                                                   \
-        libxl__prepare_ao_device(ao, aodev);                            \
-        aodev->action = LIBXL__DEVICE_ACTION_ADD;                       \
-        aodev->callback = device_addrm_aocomplete;                      \
-        aodev->update_json = true;                                      \
-        libxl__device_##type##_add(egc, domid, type, aodev);            \
-                                                                        \
-        return AO_INPROGRESS;                                           \
-    }
-
-/* Define alladd functions and undef the macro */
-
 /* disk */
-DEFINE_DEVICE_ADD(disk)
+LIBXL_DEFINE_DEVICE_ADD(disk)
+LIBXL_DEFINE_DEVICES_ADD(disk)
+LIBXL_DEFINE_DEVICE_REMOVE(disk)
 
 /* nic */
-DEFINE_DEVICE_ADD(nic)
+LIBXL_DEFINE_DEVICE_ADD(nic)
+LIBXL_DEFINE_DEVICES_ADD(nic)
+LIBXL_DEFINE_DEVICE_REMOVE(nic)
 
 /* vtpm */
-DEFINE_DEVICE_ADD(vtpm)
+LIBXL_DEFINE_DEVICE_ADD(vtpm)
+static LIBXL_DEFINE_DEVICES_ADD(vtpm)
+LIBXL_DEFINE_DEVICE_REMOVE(vtpm)
 
-/* usbctrl */
-DEFINE_DEVICE_ADD(usbctrl)
+/* vkb */
+LIBXL_DEFINE_DEVICE_REMOVE(vkb)
 
-/* usb */
-DEFINE_DEVICE_ADD(usbdev)
-
-#undef DEFINE_DEVICE_ADD
+/* vfb */
+LIBXL_DEFINE_DEVICE_REMOVE(vfb)
 
 /******************************************************************************/
 
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 4717027..200bfad 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -677,42 +677,6 @@ void libxl__multidev_prepared(libxl__egc *egc,
 
 /******************************************************************************/
 
-/* Macro for defining the functions that will add a bunch of disks when
- * inside an async op with multidev.
- * This macro is added to prevent repetition of code.
- *
- * The following functions are defined:
- * libxl__add_disks
- * libxl__add_nics
- * libxl__add_vtpms
- * libxl__add_usbctrls
- * libxl__add_usbs
- */
-
-#define DEFINE_DEVICES_ADD(type)                                        \
-    void libxl__add_##type##s(libxl__egc *egc, libxl__ao *ao, uint32_t domid, \
-                              libxl_domain_config *d_config,            \
-                              libxl__multidev *multidev)                \
-    {                                                                   \
-        AO_GC;                                                          \
-        int i;                                                          \
-        for (i = 0; i < d_config->num_##type##s; i++) {                 \
-            libxl__ao_device *aodev = libxl__multidev_prepare(multidev);  \
-            libxl__device_##type##_add(egc, domid, &d_config->type##s[i], \
-                                       aodev);                          \
-        }                                                               \
-    }
-
-DEFINE_DEVICES_ADD(disk)
-DEFINE_DEVICES_ADD(nic)
-DEFINE_DEVICES_ADD(vtpm)
-DEFINE_DEVICES_ADD(usbctrl)
-DEFINE_DEVICES_ADD(usbdev)
-
-#undef DEFINE_DEVICES_ADD
-
-/******************************************************************************/
-
 int libxl__device_destroy(libxl__gc *gc, libxl__device *dev)
 {
     const char *be_path = libxl__device_backend_path(gc, dev);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 79ce392..00b1f0c 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2379,6 +2379,9 @@ typedef void libxl__device_callback(libxl__egc*, libxl__ao_device*);
  */
 _hidden void libxl__prepare_ao_device(libxl__ao *ao, libxl__ao_device *aodev);
 
+/* generic callback for devices that only need to set ao_complete */
+_hidden void device_addrm_aocomplete(libxl__egc *egc, libxl__ao_device *aodev);
+
 struct libxl__ao_device {
     /* filled in by user */
     libxl__ao *ao;
@@ -2621,26 +2624,6 @@ struct libxl__multidev {
  * xenstore entry afterwards. We have both JSON and xenstore entry,
  * it's a valid state.
  */
-_hidden void libxl__device_disk_add(libxl__egc *egc, uint32_t domid,
-                                    libxl_device_disk *disk,
-                                    libxl__ao_device *aodev);
-
-/* AO operation to connect a nic device */
-_hidden void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
-                                   libxl_device_nic *nic,
-                                   libxl__ao_device *aodev);
-
-_hidden void libxl__device_vtpm_add(libxl__egc *egc, uint32_t domid,
-                                   libxl_device_vtpm *vtpm,
-                                   libxl__ao_device *aodev);
-
-_hidden void libxl__device_usbctrl_add(libxl__egc *egc, uint32_t domid,
-                                       libxl_device_usbctrl *usbctrl,
-                                       libxl__ao_device *aodev);
-
-_hidden void libxl__device_usbdev_add(libxl__egc *egc, uint32_t domid,
-                                      libxl_device_usbdev *usbdev,
-                                      libxl__ao_device *aodev);
 
 /* Internal function to connect a vkb device */
 _hidden int libxl__device_vkb_add(libxl__gc *gc, uint32_t domid,
@@ -2674,10 +2657,6 @@ _hidden void libxl__wait_device_connection(libxl__egc*,
 _hidden void libxl__initiate_device_generic_remove(libxl__egc *egc,
                                                    libxl__ao_device *aodev);
 
-_hidden int libxl__device_from_usbctrl(libxl__gc *gc, uint32_t domid,
-                               libxl_device_usbctrl *usbctrl,
-                               libxl__device *device);
-
 _hidden void libxl__initiate_device_usbctrl_remove(libxl__egc *egc,
                                                    libxl__ao_device *aodev);
 
@@ -3390,6 +3369,73 @@ _hidden void libxl__bootloader_init(libxl__bootloader_state *bl);
 _hidden void libxl__bootloader_run(libxl__egc*, libxl__bootloader_state *st);
 
 /*----- Generic Device Handling -----*/
+#define LIBXL_DEFINE_DEVICE_ADD(type)                                   \
+    int libxl_device_##type##_add(libxl_ctx *ctx,                       \
+        uint32_t domid, libxl_device_##type *type,                      \
+        const libxl_asyncop_how *ao_how)                                \
+    {                                                                   \
+        AO_CREATE(ctx, domid, ao_how);                                  \
+        libxl__ao_device *aodev;                                        \
+                                                                        \
+        GCNEW(aodev);                                                   \
+        libxl__prepare_ao_device(ao, aodev);                            \
+        aodev->action = LIBXL__DEVICE_ACTION_ADD;                       \
+        aodev->callback = device_addrm_aocomplete;                      \
+        aodev->update_json = true;                                      \
+        libxl__device_##type##_add(egc, domid, type, aodev);            \
+                                                                        \
+        return AO_INPROGRESS;                                           \
+    }
+
+#define LIBXL_DEFINE_DEVICES_ADD(type)                                  \
+    void libxl__add_##type##s(libxl__egc *egc, libxl__ao *ao, uint32_t domid, \
+                              libxl_domain_config *d_config,            \
+                              libxl__multidev *multidev)                \
+    {                                                                   \
+        AO_GC;                                                          \
+        int i;                                                          \
+        for (i = 0; i < d_config->num_##type##s; i++) {                 \
+            libxl__ao_device *aodev = libxl__multidev_prepare(multidev);  \
+            libxl__device_##type##_add(egc, domid, &d_config->type##s[i], \
+                                       aodev);                          \
+        }                                                               \
+    }
+
+#define LIBXL_DEFINE_DEVICE_REMOVE_EXT(type, remtype, removedestroy, f) \
+    int libxl_device_##type##_##removedestroy(libxl_ctx *ctx,           \
+        uint32_t domid, libxl_device_##type *type,                      \
+        const libxl_asyncop_how *ao_how)                                \
+    {                                                                   \
+        AO_CREATE(ctx, domid, ao_how);                                  \
+        libxl__device *device;                                          \
+        libxl__ao_device *aodev;                                        \
+        int rc;                                                         \
+                                                                        \
+        GCNEW(device);                                                  \
+        rc = libxl__device_from_##type(gc, domid, type, device);        \
+        if (rc != 0) goto out;                                          \
+                                                                        \
+        GCNEW(aodev);                                                   \
+        libxl__prepare_ao_device(ao, aodev);                            \
+        aodev->action = LIBXL__DEVICE_ACTION_REMOVE;                    \
+        aodev->dev = device;                                            \
+        aodev->callback = device_addrm_aocomplete;                      \
+        aodev->force = f;                                               \
+        libxl__initiate_device_##remtype##_remove(egc, aodev);          \
+                                                                        \
+    out:                                                                \
+        if (rc) return AO_CREATE_FAIL(rc);                              \
+        return AO_INPROGRESS;                                           \
+    }
+
+#define LIBXL_DEFINE_DEVICE_REMOVE(type)                                \
+    LIBXL_DEFINE_DEVICE_REMOVE_EXT(type, generic, remove, 0)            \
+    LIBXL_DEFINE_DEVICE_REMOVE_EXT(type, generic, destroy, 1)
+
+#define LIBXL_DEFINE_DEVICE_REMOVE_CUSTOM(type)                         \
+    LIBXL_DEFINE_DEVICE_REMOVE_EXT(type, type, remove, 0)               \
+    LIBXL_DEFINE_DEVICE_REMOVE_EXT(type, type, destroy, 1)
+
 struct libxl_device_type {
     char *type;
     int num_offset;   /* Offset of # of devices in libxl_domain_config */
@@ -3508,18 +3554,6 @@ _hidden void libxl__add_nics(libxl__egc *egc, libxl__ao *ao, uint32_t domid,
                              libxl_domain_config *d_config,
                              libxl__multidev *multidev);
 
-_hidden void libxl__add_vtpms(libxl__egc *egc, libxl__ao *ao, uint32_t domid,
-                             libxl_domain_config *d_config,
-                             libxl__multidev *multidev);
-
-_hidden void libxl__add_usbctrls(libxl__egc *egc, libxl__ao *ao,
-                                 uint32_t domid, libxl_domain_config *d_config,
-                                 libxl__multidev *multidev);
-
-_hidden void libxl__add_usbdevs(libxl__egc *egc, libxl__ao *ao,
-                                uint32_t domid, libxl_domain_config *d_config,
-                                libxl__multidev *multidev);
-
 /*----- device model creation -----*/
 
 /* First layer; wraps libxl__spawn_spawn. */
diff --git a/tools/libxl/libxl_pvusb.c b/tools/libxl/libxl_pvusb.c
index 5edd206..41ea6bc 100644
--- a/tools/libxl/libxl_pvusb.c
+++ b/tools/libxl/libxl_pvusb.c
@@ -69,9 +69,9 @@ out:
     return rc;
 }
 
-int libxl__device_from_usbctrl(libxl__gc *gc, uint32_t domid,
-                               libxl_device_usbctrl *usbctrl,
-                               libxl__device *device)
+static int libxl__device_from_usbctrl(libxl__gc *gc, uint32_t domid,
+                                      libxl_device_usbctrl *usbctrl,
+                                      libxl__device *device)
 {
     device->backend_devid   = usbctrl->devid;
     device->backend_domid   = usbctrl->backend_domid;
@@ -218,9 +218,9 @@ static char *pvusb_get_device_type(libxl_usbctrl_type type)
  * Before calling this function, aodev should be properly filled:
  * aodev->ao, aodev->callback, aodev->update_json, ...
  */
-void libxl__device_usbctrl_add(libxl__egc *egc, uint32_t domid,
-                               libxl_device_usbctrl *usbctrl,
-                               libxl__ao_device *aodev)
+static void libxl__device_usbctrl_add(libxl__egc *egc, uint32_t domid,
+                                      libxl_device_usbctrl *usbctrl,
+                                      libxl__ao_device *aodev)
 {
     STATE_AO_GC(aodev->ao);
     libxl__device *device;
@@ -263,6 +263,10 @@ out:
     return;
 }
 
+LIBXL_DEFINE_DEVICE_ADD(usbctrl)
+static LIBXL_DEFINE_DEVICES_ADD(usbctrl)
+LIBXL_DEFINE_DEVICE_REMOVE_CUSTOM(usbctrl)
+
 static int libxl__device_usbdev_list_for_usbctrl(libxl__gc *gc, uint32_t domid,
                                                  libxl_devid usbctrl,
                                                  libxl_device_usbdev **usbdevs,
@@ -1423,9 +1427,9 @@ out:
  * Before calling this function, aodev should be properly filled:
  * aodev->ao, aodev->callback, aodev->update_json, ...
  */
-void libxl__device_usbdev_add(libxl__egc *egc, uint32_t domid,
-                              libxl_device_usbdev *usbdev,
-                              libxl__ao_device *aodev)
+static void libxl__device_usbdev_add(libxl__egc *egc, uint32_t domid,
+                                     libxl_device_usbdev *usbdev,
+                                     libxl__ao_device *aodev)
 {
     STATE_AO_GC(aodev->ao);
     int rc;
@@ -1491,6 +1495,9 @@ out:
     return;
 }
 
+LIBXL_DEFINE_DEVICE_ADD(usbdev)
+static LIBXL_DEFINE_DEVICES_ADD(usbdev)
+
 static int do_usbdev_remove(libxl__gc *gc, uint32_t domid,
                             libxl_device_usbdev *usbdev)
 {
-- 
2.6.6


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

Re: [PATCH v2 1/4] libxl: add framework for device types

[Ian Jackson]

Juergen Gross writes ("[PATCH v2 1/4] libxl: add framework for device types"):
> Instead of duplicate coding for each device type (vtpms, usbctrls, ...)
> especially on domain creation introduce a framework for that purpose.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

How very pleasing to see so much code being deleted.

Ian.

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

Re: [PATCH v2 2/4] libxl: refactor domcreate_attach_pci() to use device type framework

[Ian Jackson]

Juergen Gross writes ("[PATCH v2 2/4] libxl: refactor domcreate_attach_pci() to use device type framework"):
> Signed-off-by: Juergen Gross <jgross@suse.com>

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

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

Re: [PATCH v2 3/4] libxl: refactor domcreate_attach_dtdev() to use device type framework

[Ian Jackson]

Juergen Gross writes ("[PATCH v2 3/4] libxl: refactor domcreate_attach_dtdev() to use device type framework"):
> Signed-off-by: Juergen Gross <jgross@suse.com>

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

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

Re: [PATCH v2 4/4] libxl: move DEFINE_DEVICE* macros to libxl_internal.h

[Ian Jackson]

Juergen Gross writes ("[PATCH v2 4/4] libxl: move DEFINE_DEVICE* macros to libxl_internal.h"):
> In order to be able to have all functions related to a device type in
> a single source file move the macros used to generate device type
> specific functions to libxl_internal.h. Rename the macros as they are
> no longer local to a source file. While at it hide device remove and
> device destroy in one macro as those are always used in pairs. Move
> usage of the macros to the appropriate source files.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

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

Re: [PATCH v2 0/4] libxl: add framework for device types

[Ian Jackson]

Juergen Gross writes ("[PATCH v2 0/4] libxl: add framework for device types"):
> Instead of duplicate coding for each device type (vtpms, usbctrls, ...)
> especially on domain creation introduce a framework for that purpose.
> 
> I especially found it annoying that e.g. the vtpm callback issued the
> error message for a failed attach of nic devices.

I was tempted to just commit this, having acked all four, but given
the nature of this series I'm going to wait a bit for further comments
:-).

Regards,
Ian.

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

Re: [PATCH v2 0/4] libxl: add framework for device types

[Wei Liu]

On Fri, Jul 08, 2016 at 06:54:54PM +0100, Ian Jackson wrote:
> Juergen Gross writes ("[PATCH v2 0/4] libxl: add framework for device types"):
> > Instead of duplicate coding for each device type (vtpms, usbctrls, ...)
> > especially on domain creation introduce a framework for that purpose.
> > 
> > I especially found it annoying that e.g. the vtpm callback issued the
> > error message for a failed attach of nic devices.
> 
> I was tempted to just commit this, having acked all four, but given
> the nature of this series I'm going to wait a bit for further comments
> :-).
> 

This series looks good to me.

Wei.

> Regards,
> Ian.

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

Re: [PATCH v2 0/4] libxl: add framework for device types

[Wei Liu]

On Fri, Jul 08, 2016 at 06:54:54PM +0100, Ian Jackson wrote:
> Juergen Gross writes ("[PATCH v2 0/4] libxl: add framework for device types"):
> > Instead of duplicate coding for each device type (vtpms, usbctrls, ...)
> > especially on domain creation introduce a framework for that purpose.
> > 
> > I especially found it annoying that e.g. the vtpm callback issued the
> > error message for a failed attach of nic devices.
> 
> I was tempted to just commit this, having acked all four, but given
> the nature of this series I'm going to wait a bit for further comments
> :-).
> 

Pushed with Ian's ack.

> Regards,
> Ian.

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