[Xen-devel] [PATCH 00/15] Some cleanup of libxl

[Xen-devel] [PATCH 00/15] Some cleanup of libxl

[Anthony PERARD]

Hi,

This series contains cleanup patches, ahead of a series that's going to make
more changes to libxl.

Anthony PERARD (15):
  libxl: Rename struct libxl_device_type to libxl__device_type
  libxl: Remove unused variable in libxl__device_pci_add_xenstore
  libxl_pci: Make libxl__create_pci_backend static
  libxl_pci: Constify arg `pcidev' of libxl__device_pci_add_xenstore
  libxl_pci: `starting' is a bool
  libxl_dom_save: Reorder functions for switch_qemu_logdirty
  libxl_dm: Fix initialisation of libxl__stub_dm_spawn_state
  libxl: Comment libxl__dm_spawn_state aboud init and dispose
  libxl_domain: Cleanup libxl__destroy_domid
  libxl_usb: Use proper domid value, from libxl__device
  libxl_usb: Fix wrong usage of asserts
  libxl_usb: Fix libxl_device_usbctrl_getinfo
  libxl: Constify libxl_device_* param of *_getinfo
  libxl_usb: usbctrl, make use of generic device handling functions
  libxl_usb: Use usbctrl instead of usbctrlinfo

 tools/libxl/libxl.h          |  16 +--
 tools/libxl/libxl_console.c  |   2 +-
 tools/libxl/libxl_create.c   |   4 +-
 tools/libxl/libxl_device.c   |  10 +-
 tools/libxl/libxl_disk.c     |   3 +-
 tools/libxl/libxl_dm.c       |   8 +-
 tools/libxl/libxl_dom_save.c | 168 +++++++++++++++++---------------
 tools/libxl/libxl_domain.c   |  20 ++--
 tools/libxl/libxl_internal.c |   2 +-
 tools/libxl/libxl_internal.h |  56 +++++------
 tools/libxl/libxl_nic.c      |   3 +-
 tools/libxl/libxl_pci.c      |  39 ++++----
 tools/libxl/libxl_usb.c      | 183 ++++++++++-------------------------
 tools/libxl/libxl_vdispl.c   |   2 +-
 tools/libxl/libxl_vkb.c      |   2 +-
 tools/libxl/libxl_vsnd.c     |   4 +-
 tools/libxl/libxl_vtpm.c     |   2 +-
 17 files changed, 227 insertions(+), 297 deletions(-)

-- 
Anthony PERARD


Patch series available in this git branch:
https://xenbits.xen.org/git-http/people/aperard/xen-unstable.git br.libxl-cleanup-v1

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

[Xen-devel] [PATCH 01/15] libxl: Rename struct libxl_device_type to libxl__device_type

[Anthony PERARD]

libxl__device_type is internal to libxl, rename it to the internal
only prefix. And eliminate redundant 'struct' keyword, in accord with
the coding style.

No functional changes.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl_create.c   |  4 +--
 tools/libxl/libxl_device.c   | 10 ++++----
 tools/libxl/libxl_dm.c       |  2 +-
 tools/libxl/libxl_domain.c   |  2 +-
 tools/libxl/libxl_internal.c |  2 +-
 tools/libxl/libxl_internal.h | 49 ++++++++++++++++++------------------
 6 files changed, 35 insertions(+), 34 deletions(-)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 03ce166f4f..9f56851be2 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1522,7 +1522,7 @@ static void libxl__add_dtdevs(libxl__egc *egc, libxl__ao *ao, uint32_t domid,
 #define libxl__device_dtdev_update_devid NULL
 static DEFINE_DEVICE_TYPE_STRUCT(dtdev, NONE);
 
-const struct libxl_device_type *device_type_tbl[] = {
+const libxl__device_type *device_type_tbl[] = {
     &libxl__disk_devtype,
     &libxl__nic_devtype,
     &libxl__vtpm_devtype,
@@ -1575,7 +1575,7 @@ static void domcreate_attach_devices(libxl__egc *egc,
     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;
+    const libxl__device_type *dt;
 
     if (ret) {
         LOGD(ERROR, domid, "unable to add %s devices",
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index db6c0203b7..d385f19d85 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -1823,7 +1823,7 @@ int libxl_device_events_handler(libxl_ctx *ctx,
 }
 
 void device_add_domain_config(libxl__gc *gc, libxl_domain_config *d_config,
-                              const struct libxl_device_type *dt, const void *dev)
+                              const libxl__device_type *dt, const void *dev)
 {
     int *num_dev;
     unsigned int i;
@@ -1853,7 +1853,7 @@ void device_add_domain_config(libxl__gc *gc, libxl_domain_config *d_config,
 }
 
 void libxl__device_add_async(libxl__egc *egc, uint32_t domid,
-                             const struct libxl_device_type *dt, void *type,
+                             const libxl__device_type *dt, void *type,
                              libxl__ao_device *aodev)
 {
     STATE_AO_GC(aodev->ao);
@@ -1968,7 +1968,7 @@ void libxl__device_add_async(libxl__egc *egc, uint32_t domid,
 }
 
 int libxl__device_add(libxl__gc *gc, uint32_t domid,
-                      const struct libxl_device_type *dt, void *type)
+                      const libxl__device_type *dt, void *type)
 {
     flexarray_t *back;
     flexarray_t *front, *ro_front;
@@ -2017,7 +2017,7 @@ int libxl__device_add(libxl__gc *gc, uint32_t domid,
     return rc;
 }
 
-void *libxl__device_list(libxl__gc *gc, const struct libxl_device_type *dt,
+void *libxl__device_list(libxl__gc *gc, const libxl__device_type *dt,
                          uint32_t domid, int *num)
 {
     void *r = NULL;
@@ -2068,7 +2068,7 @@ void *libxl__device_list(libxl__gc *gc, const struct libxl_device_type *dt,
     return r;
 }
 
-void libxl__device_list_free(const struct libxl_device_type *dt,
+void libxl__device_list_free(const libxl__device_type *dt,
                              void *list, int num)
 {
     int i;
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index f4fc96415d..348c3fddf5 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -3168,7 +3168,7 @@ int libxl__need_xenpv_qemu(libxl__gc *gc, libxl_domain_config *d_config)
 {
     int idx, i, ret, num;
     uint32_t domid;
-    const struct libxl_device_type *dt;
+    const libxl__device_type *dt;
 
     ret = libxl__get_domid(gc, &domid);
     if (ret) {
diff --git a/tools/libxl/libxl_domain.c b/tools/libxl/libxl_domain.c
index 0ce1ba1327..aee621b0a5 100644
--- a/tools/libxl/libxl_domain.c
+++ b/tools/libxl/libxl_domain.c
@@ -1726,7 +1726,7 @@ int libxl_retrieve_domain_configuration(libxl_ctx *ctx, uint32_t domid,
      *    retrieve from JSON.
      */
     {
-        const struct libxl_device_type *dt;
+        const libxl__device_type *dt;
         int idx;
 
         for (idx = 0;; idx++) {
diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
index f492dae5ff..ce6eb7ab16 100644
--- a/tools/libxl/libxl_internal.c
+++ b/tools/libxl/libxl_internal.c
@@ -552,7 +552,7 @@ void libxl__update_domain_configuration(libxl__gc *gc,
                                         const libxl_domain_config *src)
 {
     int i, idx, num;
-    const struct libxl_device_type *dt;
+    const libxl__device_type *dt;
 
     for (idx = 0;; idx++) {
         dt = device_type_tbl[idx];
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 3be5c644c1..a25bacd7aa 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -192,6 +192,7 @@ typedef struct libxl__ao libxl__ao;
 typedef struct libxl__aop_occurred libxl__aop_occurred;
 typedef struct libxl__osevent_hook_nexus libxl__osevent_hook_nexus;
 typedef struct libxl__osevent_hook_nexi libxl__osevent_hook_nexi;
+typedef struct libxl__device_type libxl__device_type;
 typedef struct libxl__json_object libxl__json_object;
 typedef struct libxl__carefd libxl__carefd;
 
@@ -3713,7 +3714,7 @@ typedef int (*device_set_xenstore_config_fn_t)(libxl__gc *, uint32_t, void *,
                                                flexarray_t *, flexarray_t *,
                                                flexarray_t *);
 
-struct libxl_device_type {
+struct libxl__device_type {
     libxl__device_kind type;
     int skip_attach;   /* Skip entry in domcreate_attach_devices() if 1 */
     int ptr_offset;    /* Offset of device array ptr in libxl_domain_config */
@@ -3735,7 +3736,7 @@ struct libxl_device_type {
 };
 
 #define DEFINE_DEVICE_TYPE_STRUCT_X(name, sname, kind, ...)                    \
-    const struct libxl_device_type libxl__ ## name ## _devtype = {             \
+    const libxl__device_type libxl__ ## name ## _devtype = {                   \
         .type          = LIBXL__DEVICE_KIND_ ## kind,                       \
         .ptr_offset    = offsetof(libxl_domain_config, name ## s),             \
         .num_offset    = offsetof(libxl_domain_config, num_ ## name ## s),     \
@@ -3759,38 +3760,38 @@ struct libxl_device_type {
     DEFINE_DEVICE_TYPE_STRUCT_X(name, name, kind, __VA_ARGS__)
 
 static inline void **libxl__device_type_get_ptr(
-    const struct libxl_device_type *dt, const libxl_domain_config *d_config)
+    const libxl__device_type *dt, const libxl_domain_config *d_config)
 {
     return (void **)((void *)d_config + dt->ptr_offset);
 }
 
 static inline void *libxl__device_type_get_elem(
-    const struct libxl_device_type *dt, const libxl_domain_config *d_config,
+    const libxl__device_type *dt, const libxl_domain_config *d_config,
     int e)
 {
     return *libxl__device_type_get_ptr(dt, d_config) + dt->dev_elem_size * e;
 }
 
 static inline int *libxl__device_type_get_num(
-    const struct libxl_device_type *dt, const libxl_domain_config *d_config)
+    const libxl__device_type *dt, const libxl_domain_config *d_config)
 {
     return (int *)((void *)d_config + dt->num_offset);
 }
 
-extern const struct libxl_device_type libxl__vfb_devtype;
-extern const struct libxl_device_type libxl__vkb_devtype;
-extern const struct libxl_device_type libxl__disk_devtype;
-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;
-extern const struct libxl_device_type libxl__vdispl_devtype;
-extern const struct libxl_device_type libxl__p9_devtype;
-extern const struct libxl_device_type libxl__pvcallsif_devtype;
-extern const struct libxl_device_type libxl__vsnd_devtype;
-
-extern const struct libxl_device_type *device_type_tbl[];
+extern const libxl__device_type libxl__vfb_devtype;
+extern const libxl__device_type libxl__vkb_devtype;
+extern const libxl__device_type libxl__disk_devtype;
+extern const libxl__device_type libxl__nic_devtype;
+extern const libxl__device_type libxl__vtpm_devtype;
+extern const libxl__device_type libxl__usbctrl_devtype;
+extern const libxl__device_type libxl__usbdev_devtype;
+extern const libxl__device_type libxl__pcidev_devtype;
+extern const libxl__device_type libxl__vdispl_devtype;
+extern const libxl__device_type libxl__p9_devtype;
+extern const libxl__device_type libxl__pvcallsif_devtype;
+extern const libxl__device_type libxl__vsnd_devtype;
+
+extern const libxl__device_type *device_type_tbl[];
 
 /*----- Domain destruction -----*/
 
@@ -4568,21 +4569,21 @@ static inline bool libxl__acpi_defbool_val(const libxl_domain_build_info *b_info
  *            (a copy of `dev' will be made)
  */
 void device_add_domain_config(libxl__gc *gc, libxl_domain_config *d_config,
-                              const struct libxl_device_type *dt,
+                              const libxl__device_type *dt,
                               const void *dev);
 
 void libxl__device_add_async(libxl__egc *egc, uint32_t domid,
-                             const struct libxl_device_type *dt, void *type,
+                             const libxl__device_type *dt, void *type,
                              libxl__ao_device *aodev);
 int libxl__device_add(libxl__gc *gc, uint32_t domid,
-                      const struct libxl_device_type *dt, void *type);
+                      const libxl__device_type *dt, void *type);
 
 /* Caller is responsible for freeing the memory by calling
  * libxl__device_list_free
  */
-void* libxl__device_list(libxl__gc *gc, const struct libxl_device_type *dt,
+void* libxl__device_list(libxl__gc *gc, const libxl__device_type *dt,
                          uint32_t domid, int *num);
-void libxl__device_list_free(const struct libxl_device_type *dt,
+void libxl__device_list_free(const libxl__device_type *dt,
                              void *list, int num);
 
 static inline bool libxl__timer_mode_is_default(libxl_timer_mode *tm)
-- 
Anthony PERARD


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

[Xen-devel] [PATCH 02/15] libxl: Remove unused variable in libxl__device_pci_add_xenstore

[Anthony PERARD]

*device isn't used.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl_pci.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 4ec6872798..67677d7b55 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -115,7 +115,6 @@ static int libxl__device_pci_add_xenstore(libxl__gc *gc, uint32_t domid, libxl_d
     char *num_devs, *be_path;
     int num = 0;
     xs_transaction_t t = XBT_NULL;
-    libxl__device *device;
     int rc;
     libxl_domain_config d_config;
     libxl_device_pci pcidev_saved;
@@ -149,9 +148,6 @@ static int libxl__device_pci_add_xenstore(libxl__gc *gc, uint32_t domid, libxl_d
     if (!starting)
         flexarray_append_pair(back, "state", GCSPRINTF("%d", XenbusStateReconfiguring));
 
-    GCNEW(device);
-    libxl__device_from_pcidev(gc, domid, pcidev, device);
-
     lock = libxl__lock_domain_userdata(gc, domid);
     if (!lock) {
         rc = ERROR_LOCK_FAIL;
-- 
Anthony PERARD


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

[Xen-devel] [PATCH 03/15] libxl_pci: Make libxl__create_pci_backend static

[Anthony PERARD]

libxl__create_pci_backend isn't called from outside of libxl_pci
anymore, and it's only useful as part of the pci_add process, so
remove the prototype from libxl_internal.h.

No functional changes.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl_internal.h | 2 --
 tools/libxl/libxl_pci.c      | 4 ++--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index a25bacd7aa..01411382fd 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1541,8 +1541,6 @@ _hidden int libxl__pci_topology_init(libxl__gc *gc,
 /* from libxl_pci */
 
 _hidden int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, int starting);
-_hidden int libxl__create_pci_backend(libxl__gc *gc, uint32_t domid,
-                                      libxl_device_pci *pcidev, int num);
 _hidden int libxl__device_pci_destroy_all(libxl__gc *gc, uint32_t domid);
 _hidden bool libxl__is_igd_vga_passthru(libxl__gc *gc,
                                         const libxl_domain_config *d_config);
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 67677d7b55..0e07f70f29 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -75,8 +75,8 @@ static void libxl__device_from_pcidev(libxl__gc *gc, uint32_t domid,
     device->kind = LIBXL__DEVICE_KIND_PCI;
 }
 
-int libxl__create_pci_backend(libxl__gc *gc, uint32_t domid,
-                              libxl_device_pci *pcidev, int num)
+static int libxl__create_pci_backend(libxl__gc *gc, uint32_t domid,
+                                     libxl_device_pci *pcidev, int num)
 {
     flexarray_t *front = NULL;
     flexarray_t *back = NULL;
-- 
Anthony PERARD


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

[Xen-devel] [PATCH 04/15] libxl_pci: Constify arg `pcidev' of libxl__device_pci_add_xenstore

[Anthony PERARD]

libxl__device_pci_add_xenstore doesn't modify `pcidev', so it can be
constified. Also, we don't need pcidev_saved anymore, so remove the
saved copy. (device_add_domain_config is going to make it's own copy
anyway.)

To achieve this, constify pcidev in all functions that
libxl__device_pci_add_xenstore calls.

No functional changes.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl_pci.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 0e07f70f29..d2d37be75d 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -47,7 +47,10 @@ static void pcidev_struct_fill(libxl_device_pci *pcidev, unsigned int domain,
     pcidev->vdevfn = vdevfn;
 }
 
-static void libxl_create_pci_backend_device(libxl__gc *gc, flexarray_t *back, int num, libxl_device_pci *pcidev)
+static void libxl_create_pci_backend_device(libxl__gc *gc,
+                                            flexarray_t *back,
+                                            int num,
+                                            const libxl_device_pci *pcidev)
 {
     flexarray_append(back, GCSPRINTF("key-%d", num));
     flexarray_append(back, GCSPRINTF(PCI_BDF, pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func));
@@ -64,7 +67,7 @@ static void libxl_create_pci_backend_device(libxl__gc *gc, flexarray_t *back, in
 }
 
 static void libxl__device_from_pcidev(libxl__gc *gc, uint32_t domid,
-                                      libxl_device_pci *pcidev,
+                                      const libxl_device_pci *pcidev,
                                       libxl__device *device)
 {
     device->backend_devid = 0;
@@ -76,7 +79,8 @@ static void libxl__device_from_pcidev(libxl__gc *gc, uint32_t domid,
 }
 
 static int libxl__create_pci_backend(libxl__gc *gc, uint32_t domid,
-                                     libxl_device_pci *pcidev, int num)
+                                     const libxl_device_pci *pcidev,
+                                     int num)
 {
     flexarray_t *front = NULL;
     flexarray_t *back = NULL;
@@ -109,7 +113,10 @@ static int libxl__create_pci_backend(libxl__gc *gc, uint32_t domid,
                                      NULL);
 }
 
-static int libxl__device_pci_add_xenstore(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, int starting)
+static int libxl__device_pci_add_xenstore(libxl__gc *gc,
+                                          uint32_t domid,
+                                          const libxl_device_pci *pcidev,
+                                          int starting)
 {
     flexarray_t *back;
     char *num_devs, *be_path;
@@ -117,12 +124,9 @@ static int libxl__device_pci_add_xenstore(libxl__gc *gc, uint32_t domid, libxl_d
     xs_transaction_t t = XBT_NULL;
     int rc;
     libxl_domain_config d_config;
-    libxl_device_pci pcidev_saved;
     libxl__domain_userdata_lock *lock = NULL;
 
     libxl_domain_config_init(&d_config);
-    libxl_device_pci_init(&pcidev_saved);
-    libxl_device_pci_copy(CTX, &pcidev_saved, pcidev);
 
     be_path = libxl__domain_device_backend_path(gc, 0, domid, 0,
                                                 LIBXL__DEVICE_KIND_PCI);
@@ -158,7 +162,7 @@ static int libxl__device_pci_add_xenstore(libxl__gc *gc, uint32_t domid, libxl_d
     if (rc) goto out;
 
     device_add_domain_config(gc, &d_config, &libxl__pcidev_devtype,
-                             &pcidev_saved);
+                             pcidev);
 
     rc = libxl__dm_check_start(gc, &d_config, domid);
     if (rc) goto out;
@@ -180,7 +184,6 @@ static int libxl__device_pci_add_xenstore(libxl__gc *gc, uint32_t domid, libxl_d
 out:
     libxl__xs_transaction_abort(gc, &t);
     if (lock) libxl__unlock_domain_userdata(lock);
-    libxl_device_pci_dispose(&pcidev_saved);
     libxl_domain_config_dispose(&d_config);
     return rc;
 }
-- 
Anthony PERARD


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

[Xen-devel] [PATCH 05/15] libxl_pci: `starting' is a bool

[Anthony PERARD]

The argument `starting' is used as a boolean, change its type to
reflex that throughout libxl_pci.c.

No functional changes.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl_internal.h |  3 ++-
 tools/libxl/libxl_pci.c      | 14 ++++++++------
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 01411382fd..846227c3cd 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1540,7 +1540,8 @@ _hidden int libxl__pci_topology_init(libxl__gc *gc,
 
 /* from libxl_pci */
 
-_hidden int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, int starting);
+_hidden int libxl__device_pci_add(libxl__gc *gc, uint32_t domid,
+                                  libxl_device_pci *pcidev, bool starting);
 _hidden int libxl__device_pci_destroy_all(libxl__gc *gc, uint32_t domid);
 _hidden bool libxl__is_igd_vga_passthru(libxl__gc *gc,
                                         const libxl_domain_config *d_config);
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index d2d37be75d..f204ac9fb7 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -116,7 +116,7 @@ static int libxl__create_pci_backend(libxl__gc *gc, uint32_t domid,
 static int libxl__device_pci_add_xenstore(libxl__gc *gc,
                                           uint32_t domid,
                                           const libxl_device_pci *pcidev,
-                                          int starting)
+                                          bool starting)
 {
     flexarray_t *back;
     char *num_devs, *be_path;
@@ -983,7 +983,8 @@ static int qemu_pci_add_xenstore(libxl__gc *gc, uint32_t domid,
     return rc;
 }
 
-static int do_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, int starting)
+static int do_pci_add(libxl__gc *gc, uint32_t domid,
+                      libxl_device_pci *pcidev, bool starting)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
     libxl_domain_type type = libxl__domain_type(gc, domid);
@@ -1164,7 +1165,7 @@ int libxl_device_pci_add(libxl_ctx *ctx, uint32_t domid,
 {
     AO_CREATE(ctx, domid, ao_how);
     int rc;
-    rc = libxl__device_pci_add(gc, domid, pcidev, 0);
+    rc = libxl__device_pci_add(gc, domid, pcidev, false);
     libxl__ao_complete(egc, ao, rc);
     return AO_INPROGRESS;
 }
@@ -1186,7 +1187,8 @@ static int libxl_pcidev_assignable(libxl_ctx *ctx, libxl_device_pci *pcidev)
     return i != num;
 }
 
-int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, int starting)
+int libxl__device_pci_add(libxl__gc *gc, uint32_t domid,
+                          libxl_device_pci *pcidev, bool starting)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
     unsigned int orig_vdev, pfunc_mask;
@@ -1241,7 +1243,7 @@ int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcide
     if (stubdomid != 0) {
         libxl_device_pci pcidev_s = *pcidev;
         /* stubdomain is always running by now, even at create time */
-        rc = do_pci_add(gc, stubdomid, &pcidev_s, 0);
+        rc = do_pci_add(gc, stubdomid, &pcidev_s, false);
         if ( rc )
             goto out;
     }
@@ -1294,7 +1296,7 @@ static void libxl__add_pcidevs(libxl__egc *egc, libxl__ao *ao, uint32_t domid,
     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);
+        rc = libxl__device_pci_add(gc, domid, &d_config->pcidevs[i], true);
         if (rc < 0) {
             LOGD(ERROR, domid, "libxl_device_pci_add failed: %d", rc);
             goto out;
-- 
Anthony PERARD


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

[Xen-devel] [PATCH 06/15] libxl_dom_save: Reorder functions for switch_qemu_logdirty

[Anthony PERARD]

There are two differents set of callbacks here, one for
libxl__domain_common_switch_qemu_logdirty,
and one for libxl__domain_suspend_common_switch_qemu_logdirty.

The first set calls the second.

Pure code motion.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---

Notes:
    The check for PVH seems to be in the wrong function:
    It's in libxl__domain_suspend_common_switch_qemu_logdirty(),
    but it should probably be in:
    libxl__domain_common_switch_qemu_logdirty().
    That second function is used by the remus/COLO stuff, but the first is
    only a callback of libxl__domain_save().

 tools/libxl/libxl_dom_save.c | 168 +++++++++++++++++++----------------
 1 file changed, 89 insertions(+), 79 deletions(-)

diff --git a/tools/libxl/libxl_dom_save.c b/tools/libxl/libxl_dom_save.c
index 2e9ec4dbf2..13d08d6dae 100644
--- a/tools/libxl/libxl_dom_save.c
+++ b/tools/libxl/libxl_dom_save.c
@@ -36,11 +36,17 @@ static void domain_save_done(libxl__egc *egc,
  * xenstore_process_logdirty_event
  */
 
+static void domain_suspend_switch_qemu_xen_traditional_logdirty
+                               (libxl__egc *egc, int domid, unsigned enable,
+                                libxl__logdirty_switch *lds);
+static void switch_logdirty_xswatch(libxl__egc *egc, libxl__ev_xswatch*,
+                            const char *watch_path, const char *event_path);
+static void domain_suspend_switch_qemu_xen_logdirty
+                               (libxl__egc *egc, int domid, unsigned enable,
+                                libxl__logdirty_switch *lds);
 static void switch_logdirty_timeout(libxl__egc *egc, libxl__ev_time *ev,
                                     const struct timeval *requested_abs,
                                     int rc);
-static void switch_logdirty_xswatch(libxl__egc *egc, libxl__ev_xswatch*,
-                            const char *watch_path, const char *event_path);
 static void switch_logdirty_done(libxl__egc *egc,
                                  libxl__logdirty_switch *lds, int rc);
 
@@ -51,6 +57,27 @@ void libxl__logdirty_init(libxl__logdirty_switch *lds)
     libxl__ev_time_init(&lds->timeout);
 }
 
+void libxl__domain_common_switch_qemu_logdirty(libxl__egc *egc,
+                                               int domid, unsigned enable,
+                                               libxl__logdirty_switch *lds)
+{
+    STATE_AO_GC(lds->ao);
+
+    switch (libxl__device_model_version_running(gc, domid)) {
+    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
+        domain_suspend_switch_qemu_xen_traditional_logdirty(egc, domid, enable,
+                                                            lds);
+        break;
+    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
+        domain_suspend_switch_qemu_xen_logdirty(egc, domid, enable, lds);
+        break;
+    default:
+        LOGD(ERROR, domid, "logdirty switch failed"
+             ", no valid device model version found, abandoning suspend");
+        lds->callback(egc, lds, ERROR_FAIL);
+    }
+}
+
 static void domain_suspend_switch_qemu_xen_traditional_logdirty
                                (libxl__egc *egc, int domid, unsigned enable,
                                 libxl__logdirty_switch *lds)
@@ -120,83 +147,6 @@ static void domain_suspend_switch_qemu_xen_traditional_logdirty
     switch_logdirty_done(egc,lds,rc);
 }
 
-static void domain_suspend_switch_qemu_xen_logdirty
-                               (libxl__egc *egc, int domid, unsigned enable,
-                                libxl__logdirty_switch *lds)
-{
-    STATE_AO_GC(lds->ao);
-    int rc;
-
-    rc = libxl__qmp_set_global_dirty_log(gc, domid, enable);
-    if (rc)
-        LOGD(ERROR, domid,
-             "logdirty switch failed (rc=%d), abandoning suspend",rc);
-
-    lds->callback(egc, lds, rc);
-}
-
-static void domain_suspend_switch_qemu_logdirty_done
-                        (libxl__egc *egc, libxl__logdirty_switch *lds, int rc)
-{
-    libxl__domain_save_state *dss = CONTAINER_OF(lds, *dss, logdirty);
-
-    if (rc) {
-        dss->rc = rc;
-        libxl__xc_domain_saverestore_async_callback_done(egc,
-                                                         &dss->sws.shs, -1);
-    } else
-        libxl__xc_domain_saverestore_async_callback_done(egc, &dss->sws.shs, 0);
-}
-
-void libxl__domain_suspend_common_switch_qemu_logdirty
-                               (uint32_t domid, unsigned enable, void *user)
-{
-    libxl__save_helper_state *shs = user;
-    libxl__egc *egc = shs->egc;
-    libxl__domain_save_state *dss = shs->caller_state;
-
-    /* Convenience aliases. */
-    libxl__logdirty_switch *const lds = &dss->logdirty;
-
-    if (dss->type == LIBXL_DOMAIN_TYPE_PVH) {
-        domain_suspend_switch_qemu_logdirty_done(egc, lds, 0);
-        return;
-    }
-
-    lds->callback = domain_suspend_switch_qemu_logdirty_done;
-    libxl__domain_common_switch_qemu_logdirty(egc, domid, enable, lds);
-}
-
-void libxl__domain_common_switch_qemu_logdirty(libxl__egc *egc,
-                                               int domid, unsigned enable,
-                                               libxl__logdirty_switch *lds)
-{
-    STATE_AO_GC(lds->ao);
-
-    switch (libxl__device_model_version_running(gc, domid)) {
-    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
-        domain_suspend_switch_qemu_xen_traditional_logdirty(egc, domid, enable,
-                                                            lds);
-        break;
-    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
-        domain_suspend_switch_qemu_xen_logdirty(egc, domid, enable, lds);
-        break;
-    default:
-        LOGD(ERROR, domid, "logdirty switch failed"
-             ", no valid device model version found, abandoning suspend");
-        lds->callback(egc, lds, ERROR_FAIL);
-    }
-}
-static void switch_logdirty_timeout(libxl__egc *egc, libxl__ev_time *ev,
-                                    const struct timeval *requested_abs,
-                                    int rc)
-{
-    libxl__logdirty_switch *lds = CONTAINER_OF(ev, *lds, timeout);
-    STATE_AO_GC(lds->ao);
-    LOG(ERROR,"logdirty switch: wait for device model timed out");
-    switch_logdirty_done(egc,lds,ERROR_FAIL);
-}
-
 static void switch_logdirty_xswatch(libxl__egc *egc, libxl__ev_xswatch *watch,
                             const char *watch_path, const char *event_path)
 {
@@ -251,6 +201,31 @@ static void switch_logdirty_xswatch(libxl__egc *egc, libxl__ev_xswatch *watch,
     }
 }
 
+static void domain_suspend_switch_qemu_xen_logdirty
+                               (libxl__egc *egc, int domid, unsigned enable,
+                                libxl__logdirty_switch *lds)
+{
+    STATE_AO_GC(lds->ao);
+    int rc;
+
+    rc = libxl__qmp_set_global_dirty_log(gc, domid, enable);
+    if (rc)
+        LOGD(ERROR, domid,
+             "logdirty switch failed (rc=%d), abandoning suspend",rc);
+
+    lds->callback(egc, lds, rc);
+}
+
+static void switch_logdirty_timeout(libxl__egc *egc, libxl__ev_time *ev,
+                                    const struct timeval *requested_abs,
+                                    int rc)
+{
+    libxl__logdirty_switch *lds = CONTAINER_OF(ev, *lds, timeout);
+    STATE_AO_GC(lds->ao);
+    LOG(ERROR,"logdirty switch: wait for device model timed out");
+    switch_logdirty_done(egc,lds,ERROR_FAIL);
+}
+
 static void switch_logdirty_done(libxl__egc *egc,
                                  libxl__logdirty_switch *lds,
                                  int rc)
@@ -263,6 +238,41 @@ static void switch_logdirty_done(libxl__egc *egc,
     lds->callback(egc, lds, rc);
 }
 
+static void domain_suspend_switch_qemu_logdirty_done
+                        (libxl__egc *egc, libxl__logdirty_switch *lds, int rc);
+
+void libxl__domain_suspend_common_switch_qemu_logdirty
+                               (uint32_t domid, unsigned enable, void *user)
+{
+    libxl__save_helper_state *shs = user;
+    libxl__egc *egc = shs->egc;
+    libxl__domain_save_state *dss = shs->caller_state;
+
+    /* Convenience aliases. */
+    libxl__logdirty_switch *const lds = &dss->logdirty;
+
+    if (dss->type == LIBXL_DOMAIN_TYPE_PVH) {
+        domain_suspend_switch_qemu_logdirty_done(egc, lds, 0);
+        return;
+    }
+
+    lds->callback = domain_suspend_switch_qemu_logdirty_done;
+    libxl__domain_common_switch_qemu_logdirty(egc, domid, enable, lds);
+}
+
+static void domain_suspend_switch_qemu_logdirty_done
+                        (libxl__egc *egc, libxl__logdirty_switch *lds, int rc)
+{
+    libxl__domain_save_state *dss = CONTAINER_OF(lds, *dss, logdirty);
+
+    if (rc) {
+        dss->rc = rc;
+        libxl__xc_domain_saverestore_async_callback_done(egc,
+                                                         &dss->sws.shs, -1);
+    } else
+        libxl__xc_domain_saverestore_async_callback_done(egc, &dss->sws.shs, 0);
+}
+
 /*----- callbacks, called by xc_domain_save -----*/
 
 /*
-- 
Anthony PERARD


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

[Xen-devel] [PATCH 07/15] libxl_dm: Fix initialisation of libxl__stub_dm_spawn_state

[Anthony PERARD]

sdss->pvqemu wasn't initialiased and disposed of properly.
Also, move the initialisation of sdss->xswait with the rest of the
initialisation of sdss.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl_dm.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 348c3fddf5..5942e70c16 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -2105,8 +2105,11 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
     libxl__domain_build_state *const d_state = sdss->dm.build_state;
     libxl__domain_build_state *const stubdom_state = &sdss->dm_state;
 
+    /* Initialise private part of sdss */
     libxl__domain_build_state_init(stubdom_state);
     dmss_init(&sdss->dm);
+    dmss_init(&sdss->pvqemu);
+    libxl__xswait_init(&sdss->xswait);
 
     if (guest_config->b_info.device_model_version !=
         LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL) {
@@ -2391,8 +2394,6 @@ static void stubdom_pvqemu_cb(libxl__egc *egc,
     STATE_AO_GC(sdss->dm.spawn.ao);
     uint32_t dm_domid = sdss->pvqemu.guest_domid;
 
-    libxl__xswait_init(&sdss->xswait);
-
     if (rc) {
         LOGED(ERROR, sdss->dm.guest_domid,
               "error connecting nics devices");
@@ -2439,6 +2440,7 @@ static void stubdom_xswait_cb(libxl__egc *egc, libxl__xswait_state *xswait,
     libxl__domain_build_state_dispose(&sdss->dm_state);
     libxl__xswait_stop(gc, xswait);
     dmss_dispose(gc, &sdss->dm);
+    dmss_dispose(gc, &sdss->pvqemu);
     sdss->callback(egc, &sdss->dm, rc);
 }
 
-- 
Anthony PERARD


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

[Xen-devel] [PATCH 08/15] libxl: Comment libxl__dm_spawn_state aboud init and dispose

[Anthony PERARD]

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl_internal.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 846227c3cd..0046b30549 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3920,6 +3920,8 @@ typedef struct libxl__dm_spawn_state libxl__dm_spawn_state;
 typedef void libxl__dm_spawn_cb(libxl__egc *egc, libxl__dm_spawn_state*,
                                 int rc /* if !0, error was logged */);
 
+/* Call dmss_init and dmss_dispose to initialise and dispose of
+ * libxl__dm_spawn_state */
 struct libxl__dm_spawn_state {
     /* mixed - spawn.ao must be initialised by user; rest is private: */
     libxl__spawn_state spawn;
-- 
Anthony PERARD


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

[Xen-devel] [PATCH 09/15] libxl_domain: Cleanup libxl__destroy_domid

[Anthony PERARD]

- dom_path isn't used anymore in that function, remove it.
- Use `r' to store return value of external calls.
- Use `CTX', no need for a local `ctx'.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl_domain.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/tools/libxl/libxl_domain.c b/tools/libxl/libxl_domain.c
index aee621b0a5..11a29b235b 100644
--- a/tools/libxl/libxl_domain.c
+++ b/tools/libxl/libxl_domain.c
@@ -1023,14 +1023,13 @@ static void domain_destroy_domid_cb(libxl__egc *egc,
 void libxl__destroy_domid(libxl__egc *egc, libxl__destroy_domid_state *dis)
 {
     STATE_AO_GC(dis->ao);
-    libxl_ctx *ctx = CTX;
     uint32_t domid = dis->domid;
-    char *dom_path;
     int rc, dm_present;
+    int r;
 
     libxl__ev_child_init(&dis->destroyer);
 
-    rc = libxl_domain_info(ctx, NULL, domid);
+    rc = libxl_domain_info(CTX, NULL, domid);
     switch(rc) {
     case 0:
         break;
@@ -1058,17 +1057,12 @@ void libxl__destroy_domid(libxl__egc *egc, libxl__destroy_domid_state *dis)
         abort();
     }
 
-    dom_path = libxl__xs_get_dompath(gc, domid);
-    if (!dom_path) {
-        rc = ERROR_FAIL;
-        goto out;
-    }
-
     if (libxl__device_pci_destroy_all(gc, domid) < 0)
         LOGD(ERROR, domid, "Pci shutdown failed");
-    rc = xc_domain_pause(ctx->xch, domid);
-    if (rc < 0) {
-        LOGEVD(ERROR, rc, domid, "xc_domain_pause failed");
+    r = xc_domain_pause(CTX->xch, domid);
+    if (r < 0) {
+        LOGEVD(ERROR, r, domid, "xc_domain_pause failed");
+        rc = ERROR_FAIL;
     }
 
     if (dm_present) {
-- 
Anthony PERARD


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

[Xen-devel] [PATCH 10/15] libxl_usb: Use proper domid value, from libxl__device

[Anthony PERARD]

ao->domid isn't a reliable way of getting a domid, it might not be set
(this isn't the case here). The right domid value can be found in the
libxl__device (which is the device we want to remove) attached to
libxl__ao_device.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl_usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_usb.c b/tools/libxl/libxl_usb.c
index adb122ac85..3d389c0198 100644
--- a/tools/libxl/libxl_usb.c
+++ b/tools/libxl/libxl_usb.c
@@ -515,7 +515,7 @@ void libxl__initiate_device_usbctrl_remove(libxl__egc *egc,
     libxl_device_usbdev *usbdevs = NULL;
     int num_usbdev = 0;
     int i, rc;
-    uint32_t domid = ao->domid;
+    uint32_t domid = aodev->dev->domid;
     int usbctrl_devid = aodev->dev->devid;
     libxl_device_usbctrl usbctrl;
     libxl_usbctrlinfo usbctrlinfo;
-- 
Anthony PERARD


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

[Xen-devel] [PATCH 11/15] libxl_usb: Fix wrong usage of asserts

[Anthony PERARD]

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl_usb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/libxl_usb.c b/tools/libxl/libxl_usb.c
index 3d389c0198..694870a3c3 100644
--- a/tools/libxl/libxl_usb.c
+++ b/tools/libxl/libxl_usb.c
@@ -148,7 +148,7 @@ static int libxl__device_from_usbctrl(libxl__gc *gc, uint32_t domid,
         break;
     default:
         assert(0); /* can't really happen. */
-        break;
+        return ERROR_INVAL;
     }
     device->devid           = usbctrl->devid;
     device->domid           = domid;
@@ -371,7 +371,7 @@ static int libxl__device_usbctrl_add_hvm(libxl__gc *gc, uint32_t domid,
         break;
     default:
         assert(0); /* Should not be possible. */
-        break;
+        return ERROR_INVAL;
     }
 
     flexarray_append_pair(qmp_args, "id",
-- 
Anthony PERARD


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

[Xen-devel] [PATCH 12/15] libxl_usb: Fix libxl_device_usbctrl_getinfo

[Anthony PERARD]

`usbctrl' is modified in this function which doesn't seems to be
intended, and usbctrlinfo.backend_id was never modified.

Take this opportunity to consify the argument `usbctrl' in libxl API
to avoid similar mistake.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl.h     | 2 +-
 tools/libxl/libxl_usb.c | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 9bacfb97f0..750ef1b7d0 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -1950,7 +1950,7 @@ void libxl_device_usbctrl_list_free(libxl_device_usbctrl *list, int nr);
 
 
 int libxl_device_usbctrl_getinfo(libxl_ctx *ctx, uint32_t domid,
-                                 libxl_device_usbctrl *usbctrl,
+                                 const libxl_device_usbctrl *usbctrl,
                                  libxl_usbctrlinfo *usbctrlinfo);
 
 /* USB Devices */
diff --git a/tools/libxl/libxl_usb.c b/tools/libxl/libxl_usb.c
index 694870a3c3..9851fe8468 100644
--- a/tools/libxl/libxl_usb.c
+++ b/tools/libxl/libxl_usb.c
@@ -641,7 +641,7 @@ libxl_device_usbctrl_list(libxl_ctx *ctx, uint32_t domid, int *num)
 }
 
 int libxl_device_usbctrl_getinfo(libxl_ctx *ctx, uint32_t domid,
-                                 libxl_device_usbctrl *usbctrl,
+                                 const libxl_device_usbctrl *usbctrl,
                                  libxl_usbctrlinfo *usbctrlinfo)
 {
     GC_INIT(ctx);
@@ -678,7 +678,7 @@ int libxl_device_usbctrl_getinfo(libxl_ctx *ctx, uint32_t domid,
         be_path = READ_SUBPATH(libxl_path, "backend");
         usbctrlinfo->backend = libxl__strdup(NOGC, be_path);
         rc = libxl__backendpath_parse_domid(gc, be_path,
-                                            &usbctrl->backend_domid);
+                                            &usbctrlinfo->backend_id);
         if (rc) goto out;
         usbctrlinfo->state = READ_SUBPATH_INT(fe_path, "state");
         usbctrlinfo->evtch = READ_SUBPATH_INT(fe_path, "event-channel");
@@ -691,7 +691,7 @@ int libxl_device_usbctrl_getinfo(libxl_ctx *ctx, uint32_t domid,
     } else {
         usbctrlinfo->ports = READ_SUBPATH_INT(libxl_path, "num-ports");
         usbctrlinfo->version = READ_SUBPATH_INT(libxl_path, "usb-ver");
-        rc = libxl__get_domid(gc, &usbctrl->backend_domid);
+        rc = libxl__get_domid(gc, &usbctrlinfo->backend_id);
         if (rc) goto out;
     }
 
-- 
Anthony PERARD


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

[Xen-devel] [PATCH 13/15] libxl: Constify libxl_device_* param of *_getinfo

[Anthony PERARD]

The libxl_device_TYPE parameter of all the libxl_device_TYPE_getinfo
function seems to be only used as input to find more information to bi
stored in the libxl_TYPEinfo parameter.

Make sure this is always true and constify the input parameter to avoid
further mistake.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl.h         | 14 +++++++-------
 tools/libxl/libxl_console.c |  2 +-
 tools/libxl/libxl_disk.c    |  3 ++-
 tools/libxl/libxl_nic.c     |  3 ++-
 tools/libxl/libxl_vdispl.c  |  2 +-
 tools/libxl/libxl_vkb.c     |  2 +-
 tools/libxl/libxl_vsnd.c    |  4 ++--
 tools/libxl/libxl_vtpm.c    |  2 +-
 8 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 750ef1b7d0..e40546c23a 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -1888,7 +1888,7 @@ libxl_device_disk *libxl_device_disk_list(libxl_ctx *ctx,
 void libxl_device_disk_list_free(libxl_device_disk* list, int num)
                                  LIBXL_EXTERNAL_CALLERS_ONLY;
 int libxl_device_disk_getinfo(libxl_ctx *ctx, uint32_t domid,
-                              libxl_device_disk *disk, libxl_diskinfo *diskinfo)
+                              const libxl_device_disk *disk, libxl_diskinfo *diskinfo)
                               LIBXL_EXTERNAL_CALLERS_ONLY;
 
 /*
@@ -1989,7 +1989,7 @@ libxl_device_nic *libxl_device_nic_list(libxl_ctx *ctx,
 void libxl_device_nic_list_free(libxl_device_nic* list, int num)
                                 LIBXL_EXTERNAL_CALLERS_ONLY;
 int libxl_device_nic_getinfo(libxl_ctx *ctx, uint32_t domid,
-                             libxl_device_nic *nic, libxl_nicinfo *nicinfo)
+                             const libxl_device_nic *nic, libxl_nicinfo *nicinfo)
                              LIBXL_EXTERNAL_CALLERS_ONLY;
 
 /*
@@ -2000,7 +2000,7 @@ libxl_device_channel *libxl_device_channel_list(libxl_ctx *ctx,
                                                 uint32_t domid,
                                                 int *num);
 int libxl_device_channel_getinfo(libxl_ctx *ctx, uint32_t domid,
-                                 libxl_device_channel *channel,
+                                 const libxl_device_channel *channel,
                                  libxl_channelinfo *channelinfo);
 
 /* Virtual TPMs */
@@ -2022,7 +2022,7 @@ libxl_device_vtpm *libxl_device_vtpm_list(libxl_ctx *ctx,
 void libxl_device_vtpm_list_free(libxl_device_vtpm*, int num)
                                  LIBXL_EXTERNAL_CALLERS_ONLY;
 int libxl_device_vtpm_getinfo(libxl_ctx *ctx, uint32_t domid,
-                              libxl_device_vtpm *vtpm, libxl_vtpminfo *vtpminfo)
+                              const libxl_device_vtpm *vtpm, libxl_vtpminfo *vtpminfo)
                               LIBXL_EXTERNAL_CALLERS_ONLY;
 
 /* Virtual displays */
@@ -2045,7 +2045,7 @@ libxl_device_vdispl *libxl_device_vdispl_list(libxl_ctx *ctx,
 void libxl_device_vdispl_list_free(libxl_device_vdispl* list, int num)
                                    LIBXL_EXTERNAL_CALLERS_ONLY;
 int libxl_device_vdispl_getinfo(libxl_ctx *ctx, uint32_t domid,
-                                libxl_device_vdispl *vdispl,
+                                const libxl_device_vdispl *vdispl,
                                 libxl_vdisplinfo *vdisplinfo)
                                 LIBXL_EXTERNAL_CALLERS_ONLY;
 
@@ -2069,7 +2069,7 @@ libxl_device_vsnd *libxl_device_vsnd_list(libxl_ctx *ctx,
 void libxl_device_vsnd_list_free(libxl_device_vsnd* list, int num)
                                  LIBXL_EXTERNAL_CALLERS_ONLY;
 int libxl_device_vsnd_getinfo(libxl_ctx *ctx, uint32_t domid,
-                              libxl_device_vsnd *vsnd,
+                              const libxl_device_vsnd *vsnd,
                               libxl_vsndinfo *vsndlinfo)
                               LIBXL_EXTERNAL_CALLERS_ONLY;
 
@@ -2092,7 +2092,7 @@ libxl_device_vkb *libxl_device_vkb_list(libxl_ctx *ctx,
 void libxl_device_vkb_list_free(libxl_device_vkb* list, int num)
                                 LIBXL_EXTERNAL_CALLERS_ONLY;
 int libxl_device_vkb_getinfo(libxl_ctx *ctx, uint32_t domid,
-                             libxl_device_vkb *vkb,
+                             const libxl_device_vkb *vkb,
                              libxl_vkbinfo *vkbinfo)
                              LIBXL_EXTERNAL_CALLERS_ONLY;
 
diff --git a/tools/libxl/libxl_console.c b/tools/libxl/libxl_console.c
index 36d79f5adc..088a455b52 100644
--- a/tools/libxl/libxl_console.c
+++ b/tools/libxl/libxl_console.c
@@ -566,7 +566,7 @@ libxl_device_channel *libxl_device_channel_list(libxl_ctx *ctx,
 }
 
 int libxl_device_channel_getinfo(libxl_ctx *ctx, uint32_t domid,
-                                 libxl_device_channel *channel,
+                                 const libxl_device_channel *channel,
                                  libxl_channelinfo *channelinfo)
 {
     GC_INIT(ctx);
diff --git a/tools/libxl/libxl_disk.c b/tools/libxl/libxl_disk.c
index bc9e2d5a74..d46e78a75c 100644
--- a/tools/libxl/libxl_disk.c
+++ b/tools/libxl/libxl_disk.c
@@ -598,7 +598,8 @@ int libxl_vdev_to_device_disk(libxl_ctx *ctx, uint32_t domid,
 }
 
 int libxl_device_disk_getinfo(libxl_ctx *ctx, uint32_t domid,
-                              libxl_device_disk *disk, libxl_diskinfo *diskinfo)
+                              const libxl_device_disk *disk,
+                              libxl_diskinfo *diskinfo)
 {
     GC_INIT(ctx);
     char *fe_path, *libxl_path;
diff --git a/tools/libxl/libxl_nic.c b/tools/libxl/libxl_nic.c
index f2b5e02285..07880b39e1 100644
--- a/tools/libxl/libxl_nic.c
+++ b/tools/libxl/libxl_nic.c
@@ -399,7 +399,8 @@ void libxl_device_nic_list_free(libxl_device_nic* list, int num)
 }
 
 int libxl_device_nic_getinfo(libxl_ctx *ctx, uint32_t domid,
-                              libxl_device_nic *nic, libxl_nicinfo *nicinfo)
+                              const libxl_device_nic *nic,
+                              libxl_nicinfo *nicinfo)
 {
     GC_INIT(ctx);
     char *nicpath, *libxl_path;
diff --git a/tools/libxl/libxl_vdispl.c b/tools/libxl/libxl_vdispl.c
index c9cc455176..8ddc8940e9 100644
--- a/tools/libxl/libxl_vdispl.c
+++ b/tools/libxl/libxl_vdispl.c
@@ -152,7 +152,7 @@ static int libxl__device_vdispl_getconnectors(libxl_ctx *ctx,
 }
 
 int libxl_device_vdispl_getinfo(libxl_ctx *ctx, uint32_t domid,
-                                libxl_device_vdispl *vdispl,
+                                const libxl_device_vdispl *vdispl,
                                 libxl_vdisplinfo *info)
 {
     GC_INIT(ctx);
diff --git a/tools/libxl/libxl_vkb.c b/tools/libxl/libxl_vkb.c
index 64e20b772a..26376a7eef 100644
--- a/tools/libxl/libxl_vkb.c
+++ b/tools/libxl/libxl_vkb.c
@@ -270,7 +270,7 @@ int libxl_devid_to_device_vkb(libxl_ctx *ctx, uint32_t domid,
 }
 
 int libxl_device_vkb_getinfo(libxl_ctx *ctx, uint32_t domid,
-                             libxl_device_vkb *vkb,
+                             const libxl_device_vkb *vkb,
                              libxl_vkbinfo *info)
 {
     GC_INIT(ctx);
diff --git a/tools/libxl/libxl_vsnd.c b/tools/libxl/libxl_vsnd.c
index 1151cbc567..0bc5f6dbb1 100644
--- a/tools/libxl/libxl_vsnd.c
+++ b/tools/libxl/libxl_vsnd.c
@@ -563,7 +563,7 @@ static int libxl__device_stream_getinfo(libxl__gc *gc, const char *path,
 }
 
 static int libxl__device_pcm_getinfo(libxl__gc *gc, const char *path,
-                                     libxl_device_vsnd *vsnd,
+                                     const libxl_device_vsnd *vsnd,
                                      libxl_vsndinfo *info)
 {
     int i;
@@ -588,7 +588,7 @@ static int libxl__device_pcm_getinfo(libxl__gc *gc, const char *path,
 }
 
 int libxl_device_vsnd_getinfo(libxl_ctx *ctx, uint32_t domid,
-                              libxl_device_vsnd *vsnd,
+                              const libxl_device_vsnd *vsnd,
                               libxl_vsndinfo *info)
 {
     GC_INIT(ctx);
diff --git a/tools/libxl/libxl_vtpm.c b/tools/libxl/libxl_vtpm.c
index f750395f6f..dd00b267bb 100644
--- a/tools/libxl/libxl_vtpm.c
+++ b/tools/libxl/libxl_vtpm.c
@@ -90,7 +90,7 @@ static int libxl__vtpm_from_xenstore(libxl__gc *gc, const char *libxl_path,
 
 int libxl_device_vtpm_getinfo(libxl_ctx *ctx,
                               uint32_t domid,
-                              libxl_device_vtpm *vtpm,
+                              const libxl_device_vtpm *vtpm,
                               libxl_vtpminfo *vtpminfo)
 {
     GC_INIT(ctx);
-- 
Anthony PERARD


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

[Xen-devel] [PATCH 14/15] libxl_usb: usbctrl, make use of generic device handling functions

[Anthony PERARD]

Two functions in generate `libxl_device_usbctrl' can be replaced by
generic macro:
- libxl_device_usbctrl_list -> LIBXL_DEFINE_DEVICE_LIST
- libxl_devid_to_device_usbctrl -> LIBXL_DEFINE_DEVID_TO_DEVICE

This patch only needs to define `libxl__usbctrl_devtype.from_xenstore'
to makes use of them.

Small change, libxl_devid_to_device_usbctrl doesn't list all usbctrl
anymore before finding the right one.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl_usb.c | 122 +++++++++++-----------------------------
 1 file changed, 32 insertions(+), 90 deletions(-)

diff --git a/tools/libxl/libxl_usb.c b/tools/libxl/libxl_usb.c
index 9851fe8468..de49f8620c 100644
--- a/tools/libxl/libxl_usb.c
+++ b/tools/libxl/libxl_usb.c
@@ -563,81 +563,53 @@ void libxl__initiate_device_usbctrl_remove(libxl__egc *egc,
     return;
 }
 
-libxl_device_usbctrl *
-libxl_device_usbctrl_list(libxl_ctx *ctx, uint32_t domid, int *num)
+static int libxl__usbctrl_from_xenstore(libxl__gc *gc,
+                                        const char *libxl_path,
+                                        libxl_devid devid,
+                                        libxl_device_usbctrl *usbctrl_r)
 {
-    GC_INIT(ctx);
-    libxl_device_usbctrl *usbctrls = NULL;
-    char *libxl_vusbs_path = NULL;
-    char **entry = NULL;
-    unsigned int nentries = 0;
-
-    *num = 0;
-
-    libxl_vusbs_path = GCSPRINTF("%s/device/%s",
-                     libxl__xs_libxl_path(gc, domid),
-                     libxl__device_kind_to_string(LIBXL__DEVICE_KIND_VUSB));
-    entry = libxl__xs_directory(gc, XBT_NULL, libxl_vusbs_path, &nentries);
-
-    if (entry && nentries) {
-        usbctrls = libxl__zalloc(NOGC, sizeof(*usbctrls) * nentries);
-        libxl_device_usbctrl *usbctrl;
-        libxl_device_usbctrl *end = usbctrls + nentries;
-        for (usbctrl = usbctrls;
-             usbctrl < end;
-             usbctrl++, entry++, (*num)++) {
-            const char *tmp, *be_path, *libxl_path;
-            int ret;
-
-            libxl_device_usbctrl_init(usbctrl);
-            usbctrl->devid = atoi(*entry);
+    int rc;
+    const char *tmp;
+    const char *be_path;
 
 #define READ_SUBPATH(path, subpath) ({                                  \
-        ret = libxl__xs_read_checked(gc, XBT_NULL,                      \
+        rc = libxl__xs_read_checked(gc, XBT_NULL,                      \
                                      GCSPRINTF("%s/" subpath, path),    \
                                      &tmp);                             \
-        if (ret) goto out;                                              \
+        if (rc) goto out;                                              \
         (char *)tmp;                                                    \
     })
 
 #define READ_SUBPATH_INT(path, subpath) ({                              \
-        ret = libxl__xs_read_checked(gc, XBT_NULL,                      \
+        rc = libxl__xs_read_checked(gc, XBT_NULL,                      \
                                      GCSPRINTF("%s/" subpath, path),    \
                                      &tmp);                             \
-        if (ret) goto out;                                              \
+        if (rc) goto out;                                              \
         tmp ? atoi(tmp) : -1;                                           \
     })
 
-            libxl_path = GCSPRINTF("%s/%s", libxl_vusbs_path, *entry);
-            libxl_usbctrl_type_from_string(READ_SUBPATH(libxl_path, "type"),
-                                           &usbctrl->type);
-            if (usbctrl->type == LIBXL_USBCTRL_TYPE_DEVICEMODEL) {
-                be_path = libxl_path;
-                ret = libxl__get_domid(gc, &usbctrl->backend_domid);
-            } else {
-                be_path = READ_SUBPATH(libxl_path, "backend");
-                if (!be_path) goto out;
-                ret = libxl__backendpath_parse_domid(gc, be_path,
-                                                     &usbctrl->backend_domid);
-            }
-            if (ret) goto out;
-            usbctrl->version = READ_SUBPATH_INT(be_path, "usb-ver");
-            usbctrl->ports = READ_SUBPATH_INT(be_path, "num-ports");
+    usbctrl_r->devid = devid;
+    libxl_usbctrl_type_from_string(READ_SUBPATH(libxl_path, "type"),
+                                   &usbctrl_r->type);
+    if (usbctrl_r->type == LIBXL_USBCTRL_TYPE_DEVICEMODEL) {
+        be_path = libxl_path;
+        rc = libxl__get_domid(gc, &usbctrl_r->backend_domid);
+    } else {
+        be_path = READ_SUBPATH(libxl_path, "backend");
+        if (!be_path) goto out;
+        rc = libxl__backendpath_parse_domid(gc, be_path,
+                                             &usbctrl_r->backend_domid);
+    }
+    if (rc) goto out;
+    usbctrl_r->version = READ_SUBPATH_INT(be_path, "usb-ver");
+    usbctrl_r->ports = READ_SUBPATH_INT(be_path, "num-ports");
 
 #undef READ_SUBPATH
 #undef READ_SUBPATH_INT
-       }
-    }
-
-    GC_FREE;
-    return usbctrls;
-
 out:
-    LOGD(ERROR, domid, "Unable to list USB Controllers");
-    libxl_device_usbctrl_list_free(usbctrls, *num);
-    GC_FREE;
-    *num = 0;
-    return NULL;
+    if (rc)
+        libxl_device_usbctrl_dispose(usbctrl_r);
+    return rc;
 }
 
 int libxl_device_usbctrl_getinfo(libxl_ctx *ctx, uint32_t domid,
@@ -705,30 +677,6 @@ int libxl_device_usbctrl_getinfo(libxl_ctx *ctx, uint32_t domid,
     return rc;
 }
 
-int libxl_devid_to_device_usbctrl(libxl_ctx *ctx,
-                                  uint32_t domid,
-                                  int devid,
-                                  libxl_device_usbctrl *usbctrl)
-{
-    libxl_device_usbctrl *usbctrls;
-    int nb = 0;
-    int i, rc;
-
-    usbctrls = libxl_device_usbctrl_list(ctx, domid, &nb);
-    if (!usbctrls) return ERROR_FAIL;
-
-    rc = ERROR_FAIL;
-    for (i = 0; i < nb; i++) {
-        if (devid == usbctrls[i].devid) {
-            libxl_device_usbctrl_copy(ctx, usbctrl, &usbctrls[i]);
-            rc = 0;
-            break;
-        }
-    }
-
-    libxl_device_usbctrl_list_free(usbctrls, nb);
-    return rc;
-}
 
 static char *usbdev_busaddr_to_busid(libxl__gc *gc, int bus, int addr)
 {
@@ -1945,15 +1893,6 @@ static int libxl_device_usbdev_compare(const libxl_device_usbdev *d1,
     return COMPARE_USB(d1, d2);
 }
 
-void libxl_device_usbctrl_list_free(libxl_device_usbctrl *list, int nr)
-{
-   int i;
-
-   for (i = 0; i < nr; i++)
-       libxl_device_usbctrl_dispose(&list[i]);
-   free(list);
-}
-
 void libxl_device_usbdev_list_free(libxl_device_usbdev *list, int nr)
 {
    int i;
@@ -1965,7 +1904,10 @@ void libxl_device_usbdev_list_free(libxl_device_usbdev *list, int nr)
 
 #define libxl__device_usbctrl_update_devid NULL
 
+LIBXL_DEFINE_DEVID_TO_DEVICE(usbctrl)
+LIBXL_DEFINE_DEVICE_LIST(usbctrl)
 DEFINE_DEVICE_TYPE_STRUCT(usbctrl, VUSB,
+    .from_xenstore = (device_from_xenstore_fn_t)libxl__usbctrl_from_xenstore,
     .dm_needed = libxl_device_usbctrl_dm_needed
 );
 
-- 
Anthony PERARD


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

[Xen-devel] [PATCH 15/15] libxl_usb: Use usbctrl instead of usbctrlinfo

[Anthony PERARD]

The functions that calls usbctrl_getinfo() only needs information that
can be found in a `libxl_device_usbctrl'. So avoid calling
libxl_device_usbctrl_getinfo and call libxl_devid_to_device_usbctrl
instead. (libxl_device_usbctrl_getinfo needs a `libxl_device_usbctrl'
anyway.)

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl_usb.c | 49 +++++++++++------------------------------
 1 file changed, 13 insertions(+), 36 deletions(-)

diff --git a/tools/libxl/libxl_usb.c b/tools/libxl/libxl_usb.c
index de49f8620c..3d69379c20 100644
--- a/tools/libxl/libxl_usb.c
+++ b/tools/libxl/libxl_usb.c
@@ -518,13 +518,10 @@ void libxl__initiate_device_usbctrl_remove(libxl__egc *egc,
     uint32_t domid = aodev->dev->domid;
     int usbctrl_devid = aodev->dev->devid;
     libxl_device_usbctrl usbctrl;
-    libxl_usbctrlinfo usbctrlinfo;
 
     libxl_device_usbctrl_init(&usbctrl);
-    libxl_usbctrlinfo_init(&usbctrlinfo);
-    usbctrl.devid = usbctrl_devid;
-
-    rc = libxl_device_usbctrl_getinfo(CTX, domid, &usbctrl, &usbctrlinfo);
+    rc = libxl_devid_to_device_usbctrl(CTX, domid, usbctrl_devid,
+                                       &usbctrl);
     if (rc) goto out;
 
     /* Remove usb devices first */
@@ -541,7 +538,7 @@ void libxl__initiate_device_usbctrl_remove(libxl__egc *egc,
         }
     }
 
-    if (usbctrlinfo.type == LIBXL_USBCTRL_TYPE_DEVICEMODEL) {
+    if (usbctrl.type == LIBXL_USBCTRL_TYPE_DEVICEMODEL) {
         rc = libxl__device_usbctrl_del_hvm(gc, domid, usbctrl_devid);
         if (!rc)
             libxl__device_usbctrl_del_xenstore(gc, domid, &usbctrl);
@@ -549,7 +546,6 @@ void libxl__initiate_device_usbctrl_remove(libxl__egc *egc,
     }
 
     libxl_device_usbctrl_dispose(&usbctrl);
-    libxl_usbctrlinfo_dispose(&usbctrlinfo);
 
     /* Remove usbctrl */
     libxl__initiate_device_generic_remove(egc, aodev);
@@ -557,7 +553,6 @@ void libxl__initiate_device_usbctrl_remove(libxl__egc *egc,
 
 out:
     libxl_device_usbctrl_dispose(&usbctrl);
-    libxl_usbctrlinfo_dispose(&usbctrlinfo);
     aodev->rc = rc;
     aodev->callback(egc, aodev);
     return;
@@ -1531,16 +1526,12 @@ static int do_usbdev_add(libxl__gc *gc, uint32_t domid,
     int rc;
     char *busid;
     libxl_device_usbctrl usbctrl;
-    libxl_usbctrlinfo usbctrlinfo;
 
     libxl_device_usbctrl_init(&usbctrl);
-    libxl_usbctrlinfo_init(&usbctrlinfo);
-    usbctrl.devid = usbdev->ctrl;
-
-    rc = libxl_device_usbctrl_getinfo(CTX, domid, &usbctrl, &usbctrlinfo);
+    rc = libxl_devid_to_device_usbctrl(CTX, domid, usbdev->ctrl, &usbctrl);
     if (rc) goto out;
 
-    switch (usbctrlinfo.type) {
+    switch (usbctrl.type) {
     case LIBXL_USBCTRL_TYPE_PV:
         busid = usbdev_busaddr_to_busid(gc, usbdev->u.hostdev.hostbus,
                                         usbdev->u.hostdev.hostaddr);
@@ -1591,7 +1582,6 @@ static int do_usbdev_add(libxl__gc *gc, uint32_t domid,
 
 out:
     libxl_device_usbctrl_dispose(&usbctrl);
-    libxl_usbctrlinfo_dispose(&usbctrlinfo);
     return rc;
 }
 
@@ -1620,21 +1610,19 @@ static void libxl__device_usbdev_add(libxl__egc *egc, uint32_t domid,
     libxl_device_usbdev *assigned;
     int num_assigned;
     libxl_device_usbctrl usbctrl;
-    libxl_usbctrlinfo usbctrlinfo;
 
     libxl_device_usbctrl_init(&usbctrl);
-    libxl_usbctrlinfo_init(&usbctrlinfo);
 
     /* Currently only support adding USB device from Dom0 backend.
      * So, if USB controller is specified, check its backend domain,
      * if it's not Dom0, report error.
      */
     if (usbdev->ctrl != -1) {
-        usbctrl.devid = usbdev->ctrl;
-        rc = libxl_device_usbctrl_getinfo(CTX, domid, &usbctrl, &usbctrlinfo);
+        rc = libxl_devid_to_device_usbctrl(CTX, domid, usbdev->ctrl,
+                                           &usbctrl);
         if (rc) goto out;
 
-        if (usbctrlinfo.backend_id != LIBXL_TOOLSTACK_DOMID) {
+        if (usbctrl.backend_domid != LIBXL_TOOLSTACK_DOMID) {
             LOGD(ERROR, domid,
                  "Don't support adding USB device from non-Dom0 backend");
             rc = ERROR_INVAL;
@@ -1674,7 +1662,6 @@ static void libxl__device_usbdev_add(libxl__egc *egc, uint32_t domid,
 
 out:
     libxl_device_usbctrl_dispose(&usbctrl);
-    libxl_usbctrlinfo_dispose(&usbctrlinfo);
     aodev->rc = rc;
     aodev->callback(egc, aodev);
     return;
@@ -1689,18 +1676,14 @@ static int do_usbdev_remove(libxl__gc *gc, uint32_t domid,
     int rc;
     char *busid;
     libxl_device_usbctrl usbctrl;
-    libxl_usbctrlinfo usbctrlinfo;
 
     libxl_device_usbctrl_init(&usbctrl);
-    libxl_usbctrlinfo_init(&usbctrlinfo);
-    usbctrl.devid = usbdev->ctrl;
-
-    rc = libxl_device_usbctrl_getinfo(CTX, domid, &usbctrl, &usbctrlinfo);
+    rc = libxl_devid_to_device_usbctrl(CTX, domid, usbdev->ctrl, &usbctrl);
     if (rc) goto out;
 
-    switch (usbctrlinfo.type) {
+    switch (usbctrl.type) {
     case LIBXL_USBCTRL_TYPE_PV:
-        busid = usbdev_busid_from_ctrlport(gc, domid, usbdev, usbctrlinfo.type);
+        busid = usbdev_busid_from_ctrlport(gc, domid, usbdev, usbctrl.type);
         if (!busid) {
             rc = ERROR_FAIL;
             goto out;
@@ -1772,7 +1755,6 @@ static int do_usbdev_remove(libxl__gc *gc, uint32_t domid,
 
 out:
     libxl_device_usbctrl_dispose(&usbctrl);
-    libxl_usbctrlinfo_dispose(&usbctrlinfo);
     return rc;
 }
 
@@ -1787,7 +1769,6 @@ static int do_usbdev_remove(libxl__gc *gc, uint32_t domid,
 static int libxl__device_usbdev_remove(libxl__gc *gc, uint32_t domid,
                                        libxl_device_usbdev *usbdev)
 {
-    libxl_usbctrlinfo usbctrlinfo;
     libxl_device_usbctrl usbctrl;
     int rc;
 
@@ -1797,13 +1778,10 @@ static int libxl__device_usbdev_remove(libxl__gc *gc, uint32_t domid,
     }
 
     libxl_device_usbctrl_init(&usbctrl);
-    libxl_usbctrlinfo_init(&usbctrlinfo);
-    usbctrl.devid = usbdev->ctrl;
-
-    rc = libxl_device_usbctrl_getinfo(CTX, domid, &usbctrl, &usbctrlinfo);
+    rc = libxl_devid_to_device_usbctrl(CTX, domid, usbdev->ctrl, &usbctrl);
     if (rc) goto out;
 
-    if (usbctrlinfo.backend_id != LIBXL_TOOLSTACK_DOMID) {
+    if (usbctrl.backend_domid != LIBXL_TOOLSTACK_DOMID) {
         LOGD(ERROR, domid,
              "Don't support removing USB device from non-Dom0 backend");
         rc = ERROR_INVAL;
@@ -1815,7 +1793,6 @@ static int libxl__device_usbdev_remove(libxl__gc *gc, uint32_t domid,
 
 out:
     libxl_device_usbctrl_dispose(&usbctrl);
-    libxl_usbctrlinfo_dispose(&usbctrlinfo);
     return rc;
 }
 
-- 
Anthony PERARD


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

Re: [Xen-devel] [PATCH 01/15] libxl: Rename struct libxl_device_type to libxl__device_type

[Ian Jackson]

Anthony PERARD writes ("[PATCH 01/15] libxl: Rename struct libxl_device_type to libxl__device_type"):
> libxl__device_type is internal to libxl, rename it to the internal
> only prefix. And eliminate redundant 'struct' keyword, in accord with
> the coding style.

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

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

Re: [Xen-devel] [PATCH 02/15] libxl: Remove unused variable in libxl__device_pci_add_xenstore

[Ian Jackson]

Anthony PERARD writes ("[PATCH 02/15] libxl: Remove unused variable in libxl__device_pci_add_xenstore"):
> *device isn't used.

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

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

Re: [Xen-devel] [PATCH 03/15] libxl_pci: Make libxl__create_pci_backend static

[Ian Jackson]

Anthony PERARD writes ("[PATCH 03/15] libxl_pci: Make libxl__create_pci_backend static"):
> libxl__create_pci_backend isn't called from outside of libxl_pci
> anymore, and it's only useful as part of the pci_add process, so
> remove the prototype from libxl_internal.h.
> 
> No functional changes.

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

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

Re: [Xen-devel] [PATCH 04/15] libxl_pci: Constify arg `pcidev' of libxl__device_pci_add_xenstore

[Ian Jackson]

Anthony PERARD writes ("[PATCH 04/15] libxl_pci: Constify arg `pcidev' of libxl__device_pci_add_xenstore"):
> libxl__device_pci_add_xenstore doesn't modify `pcidev', so it can be
> constified. Also, we don't need pcidev_saved anymore, so remove the
> saved copy. (device_add_domain_config is going to make it's own copy
> anyway.)
> 
> To achieve this, constify pcidev in all functions that
> libxl__device_pci_add_xenstore calls.

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

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

Re: [Xen-devel] [PATCH 05/15] libxl_pci: `starting' is a bool

[Ian Jackson]

Anthony PERARD writes ("[PATCH 05/15] libxl_pci: `starting' is a bool"):
> The argument `starting' is used as a boolean, change its type to
> reflex that throughout libxl_pci.c.
> 
> No functional changes.

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

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

Re: [Xen-devel] [PATCH 06/15] libxl_dom_save: Reorder functions for switch_qemu_logdirty

[Ian Jackson]

Anthony PERARD writes ("[PATCH 06/15] libxl_dom_save: Reorder functions for switch_qemu_logdirty"):
> Pure code motion.

I'll trust you on this :-).

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

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

Re: [Xen-devel] [PATCH 07/15] libxl_dm: Fix initialisation of libxl__stub_dm_spawn_state

[Ian Jackson]

Anthony PERARD writes ("[PATCH 07/15] libxl_dm: Fix initialisation of libxl__stub_dm_spawn_state"):
> sdss->pvqemu wasn't initialiased and disposed of properly.
> Also, move the initialisation of sdss->xswait with the rest of the
> initialisation of sdss.

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

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

Re: [Xen-devel] [PATCH 08/15] libxl: Comment libxl__dm_spawn_state aboud init and dispose

[Ian Jackson]

Anthony PERARD writes ("[PATCH 08/15] libxl: Comment libxl__dm_spawn_state aboud init and dispose"):
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

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

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

Re: [Xen-devel] [PATCH 09/15] libxl_domain: Cleanup libxl__destroy_domid

[Ian Jackson]

Anthony PERARD writes ("[PATCH 09/15] libxl_domain: Cleanup libxl__destroy_domid"):
> - dom_path isn't used anymore in that function, remove it.
> - Use `r' to store return value of external calls.
> - Use `CTX', no need for a local `ctx'.

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

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

Re: [Xen-devel] [PATCH 10/15] libxl_usb: Use proper domid value, from libxl__device

[Ian Jackson]

Anthony PERARD writes ("[PATCH 10/15] libxl_usb: Use proper domid value, from libxl__device"):
> ao->domid isn't a reliable way of getting a domid, it might not be set
> (this isn't the case here). The right domid value can be found in the
> libxl__device (which is the device we want to remove) attached to
> libxl__ao_device.

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

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

Re: [Xen-devel] [PATCH 11/15] libxl_usb: Fix wrong usage of asserts

[Ian Jackson]

Anthony PERARD writes ("[PATCH 11/15] libxl_usb: Fix wrong usage of asserts"):
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

I'm not sure why you wouldn't just delete the breaks, rather than
replacing them "return" ?

Thanks,
Ian.

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

Re: [Xen-devel] [PATCH 12/15] libxl_usb: Fix libxl_device_usbctrl_getinfo

[Ian Jackson]

Anthony PERARD writes ("[PATCH 12/15] libxl_usb: Fix libxl_device_usbctrl_getinfo"):
> `usbctrl' is modified in this function which doesn't seems to be
> intended, and usbctrlinfo.backend_id was never modified.
> 
> Take this opportunity to consify the argument `usbctrl' in libxl API
> to avoid similar mistake.

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

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

Re: [Xen-devel] [PATCH 13/15] libxl: Constify libxl_device_* param of *_getinfo

[Ian Jackson]

Anthony PERARD writes ("[PATCH 13/15] libxl: Constify libxl_device_* param of *_getinfo"):
> The libxl_device_TYPE parameter of all the libxl_device_TYPE_getinfo
> function seems to be only used as input to find more information to bi
> stored in the libxl_TYPEinfo parameter.
> 
> Make sure this is always true and constify the input parameter to avoid
> further mistake.

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

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

Re: [Xen-devel] [PATCH 14/15] libxl_usb: usbctrl, make use of generic device handling functions

[Ian Jackson]

Anthony PERARD writes ("[PATCH 14/15] libxl_usb: usbctrl, make use of generic device handling functions"):
> Two functions in generate `libxl_device_usbctrl' can be replaced by
> generic macro:
> - libxl_device_usbctrl_list -> LIBXL_DEFINE_DEVICE_LIST
> - libxl_devid_to_device_usbctrl -> LIBXL_DEFINE_DEVID_TO_DEVICE
> 
> This patch only needs to define `libxl__usbctrl_devtype.from_xenstore'
> to makes use of them.
> 
> Small change, libxl_devid_to_device_usbctrl doesn't list all usbctrl
> anymore before finding the right one.

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

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

Re: [Xen-devel] [PATCH 15/15] libxl_usb: Use usbctrl instead of usbctrlinfo

[Ian Jackson]

Anthony PERARD writes ("[PATCH 15/15] libxl_usb: Use usbctrl instead of usbctrlinfo"):
> The functions that calls usbctrl_getinfo() only needs information that
> can be found in a `libxl_device_usbctrl'. So avoid calling
> libxl_device_usbctrl_getinfo and call libxl_devid_to_device_usbctrl
> instead. (libxl_device_usbctrl_getinfo needs a `libxl_device_usbctrl'
> anyway.)

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

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

Re: [Xen-devel] [PATCH 11/15] libxl_usb: Fix wrong usage of asserts

[Anthony PERARD]

On Tue, Sep 17, 2019 at 05:44:03PM +0100, Ian Jackson wrote:
> Anthony PERARD writes ("[PATCH 11/15] libxl_usb: Fix wrong usage of asserts"):
> > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> 
> I'm not sure why you wouldn't just delete the breaks, rather than
> replacing them "return" ?

Because asserts aren't supposed to be compiled in a release build. I
know that's not the case in libxl...

I could replace the assert(0) by abort() instead, they won't be need for
a break or return after it.

-- 
Anthony PERARD

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

Re: [Xen-devel] [PATCH 11/15] libxl_usb: Fix wrong usage of asserts

[Ian Jackson]

Anthony PERARD writes ("Re: [PATCH 11/15] libxl_usb: Fix wrong usage of asserts"):
> On Tue, Sep 17, 2019 at 05:44:03PM +0100, Ian Jackson wrote:
> > Anthony PERARD writes ("[PATCH 11/15] libxl_usb: Fix wrong usage of asserts"):
> > > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> > 
> > I'm not sure why you wouldn't just delete the breaks, rather than
> > replacing them "return" ?
> 
> Because asserts aren't supposed to be compiled in a release build. I
> know that's not the case in libxl...

Oh I see.

> I could replace the assert(0) by abort() instead, they won't be need for
> a break or return after it.

Yes, please.

Ian.

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

[Xen-devel] [PATCH v2 11/15] libxl_usb: Fix wrong usage of asserts

[Anthony PERARD]

Replace the assert(0) by abort() since the intention in libxl is that
asserts are always compiled in. This patch makes its clear and removes
the need to deal with asserts been compiled out.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---

Notes:
    v2:
    - replace asserts by abort().

 tools/libxl/libxl_usb.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/tools/libxl/libxl_usb.c b/tools/libxl/libxl_usb.c
index 3d389c019822..9f72857d87af 100644
--- a/tools/libxl/libxl_usb.c
+++ b/tools/libxl/libxl_usb.c
@@ -147,8 +147,7 @@ static int libxl__device_from_usbctrl(libxl__gc *gc, uint32_t domid,
         device->backend_kind = LIBXL__DEVICE_KIND_NONE;
         break;
     default:
-        assert(0); /* can't really happen. */
-        break;
+        abort(); /* can't really happen. */
     }
     device->devid           = usbctrl->devid;
     device->domid           = domid;
@@ -370,8 +369,7 @@ static int libxl__device_usbctrl_add_hvm(libxl__gc *gc, uint32_t domid,
         flexarray_append_pair(qmp_args, "p3", GCSPRINTF("%d", usbctrl->ports));
         break;
     default:
-        assert(0); /* Should not be possible. */
-        break;
+        abort(); /* Should not be possible. */
     }
 
     flexarray_append_pair(qmp_args, "id",
-- 
Anthony PERARD


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

Re: [Xen-devel] [PATCH v2 11/15] libxl_usb: Fix wrong usage of asserts

[Ian Jackson]

Anthony PERARD writes ("[PATCH v2 11/15] libxl_usb: Fix wrong usage of asserts"):
> Replace the assert(0) by abort() since the intention in libxl is that
> asserts are always compiled in. This patch makes its clear and removes
> the need to deal with asserts been compiled out.

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

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

Re: [Xen-devel] [PATCH 00/15] Some cleanup of libxl

[Anthony PERARD]

On Fri, Jun 14, 2019 at 12:24:29PM +0100, Anthony PERARD wrote:
> Hi,
> 
> This series contains cleanup patches, ahead of a series that's going to make
> more changes to libxl.

v2 with patch 11 acked and the whole branch rebased pushed to:
https://xenbits.xen.org/git-http/people/aperard/xen-unstable.git br.libxl-cleanup-v2

All patches are acked.

-- 
Anthony PERARD

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