diff mbox

[v1] libxl: remove list callback from device framework

Message ID 1506083335-18754-2-git-send-email-al1img@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Oleksandr Grytsov Sept. 22, 2017, 12:28 p.m. UTC 
From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>

As we have generic functions to get device list
(libxl__device_list) no need to have callback in
the framework. To resolve issue when XS entry
doesn't match device name, device type is
extended with field "entry" which keeps XS entry.

Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
---
 tools/libxl/libxl_checkpoint_device.c |  4 ++--
 tools/libxl/libxl_colo_save.c         |  2 +-
 tools/libxl/libxl_device.c            |  4 ++--
 tools/libxl/libxl_disk.c              | 24 ++++--------------------
 tools/libxl/libxl_domain.c            |  4 ++--
 tools/libxl/libxl_internal.h          | 30 ++++++++++++++++++++++++------
 tools/libxl/libxl_nic.c               |  6 +++---
 tools/libxl/libxl_pci.c               |  2 +-
 tools/libxl/libxl_vdispl.c            | 23 ++---------------------
 tools/libxl/libxl_vtpm.c              | 23 +++--------------------
 10 files changed, 44 insertions(+), 78 deletions(-)

Comments

Wei Liu Sept. 22, 2017, 1:02 p.m. UTC | #1 
On Fri, Sep 22, 2017 at 03:28:55PM +0300, Oleksandr Grytsov wrote:
> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
> 
> As we have generic functions to get device list
> (libxl__device_list) no need to have callback in
> the framework. To resolve issue when XS entry
> doesn't match device name, device type is
> extended with field "entry" which keeps XS entry.
> 
> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>
Oleksandr Grytsov Sept. 22, 2017, 2 p.m. UTC | #2 
On Fri, Sep 22, 2017 at 4:02 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> On Fri, Sep 22, 2017 at 03:28:55PM +0300, Oleksandr Grytsov wrote:
>> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>>
>> As we have generic functions to get device list
>> (libxl__device_list) no need to have callback in
>> the framework. To resolve issue when XS entry
>> doesn't match device name, device type is
>> extended with field "entry" which keeps XS entry.
>>
>> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>
> Acked-by: Wei Liu <wei.liu2@citrix.com>

Shall I resend the patch with your ack?
Wei Liu Sept. 22, 2017, 2:17 p.m. UTC | #3 
On Fri, Sep 22, 2017 at 05:00:44PM +0300, Oleksandr Grytsov wrote:
> On Fri, Sep 22, 2017 at 4:02 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> > On Fri, Sep 22, 2017 at 03:28:55PM +0300, Oleksandr Grytsov wrote:
> >> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
> >>
> >> As we have generic functions to get device list
> >> (libxl__device_list) no need to have callback in
> >> the framework. To resolve issue when XS entry
> >> doesn't match device name, device type is
> >> extended with field "entry" which keeps XS entry.
> >>
> >> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
> >
> > Acked-by: Wei Liu <wei.liu2@citrix.com>
> 
> Shall I resend the patch with your ack?

If this is the final that I can apply, then no; otherwise please resend.
Oleksandr Grytsov Sept. 22, 2017, 2:18 p.m. UTC | #4 
On Fri, Sep 22, 2017 at 5:17 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> On Fri, Sep 22, 2017 at 05:00:44PM +0300, Oleksandr Grytsov wrote:
>> On Fri, Sep 22, 2017 at 4:02 PM, Wei Liu <wei.liu2@citrix.com> wrote:
>> > On Fri, Sep 22, 2017 at 03:28:55PM +0300, Oleksandr Grytsov wrote:
>> >> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>> >>
>> >> As we have generic functions to get device list
>> >> (libxl__device_list) no need to have callback in
>> >> the framework. To resolve issue when XS entry
>> >> doesn't match device name, device type is
>> >> extended with field "entry" which keeps XS entry.
>> >>
>> >> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>> >
>> > Acked-by: Wei Liu <wei.liu2@citrix.com>
>>
>> Shall I resend the patch with your ack?
>
> If this is the final that I can apply, then no; otherwise please resend.

It is final.
diff mbox

Patch

diff --git a/tools/libxl/libxl_checkpoint_device.c b/tools/libxl/libxl_checkpoint_device.c
index ef212e4..f6395dc 100644
--- a/tools/libxl/libxl_checkpoint_device.c
+++ b/tools/libxl/libxl_checkpoint_device.c
@@ -64,11 +64,11 @@  void libxl__checkpoint_devices_setup(libxl__egc *egc,
 
     if (cds->device_kind_flags & (1 << LIBXL__DEVICE_KIND_VIF))
         cds->nics = libxl__device_list(gc, &libxl__nic_devtype, cds->domid,
-                                       "vif", &cds->num_nics);
+                                       &cds->num_nics);
 
     if (cds->device_kind_flags & (1 << LIBXL__DEVICE_KIND_VBD))
         cds->disks = libxl__device_list(gc, &libxl__disk_devtype, cds->domid,
-                                        "vbd", &cds->num_disks);
+                                        &cds->num_disks);
 
     if (cds->num_nics == 0 && cds->num_disks == 0)
         goto out;
diff --git a/tools/libxl/libxl_colo_save.c b/tools/libxl/libxl_colo_save.c
index 43f7dbe..3247cce 100644
--- a/tools/libxl/libxl_colo_save.c
+++ b/tools/libxl/libxl_colo_save.c
@@ -122,7 +122,7 @@  void libxl__colo_save_setup(libxl__egc *egc, libxl__colo_save_state *css)
 
         /* Use this args we can connect to qemu colo-compare */
         cds->nics = libxl__device_list(gc, &libxl__nic_devtype,
-                                       cds->domid, "vif", &cds->num_nics);
+                                       cds->domid, &cds->num_nics);
         if (cds->num_nics > 0) {
             css->cps.checkpoint_host = cds->nics[0].colo_checkpoint_host;
             css->cps.checkpoint_port = cds->nics[0].colo_checkpoint_port;
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 67b7afb..46ca5a4 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -1989,7 +1989,7 @@  out:
 }
 
 void *libxl__device_list(libxl__gc *gc, const struct libxl_device_type *dt,
-                         uint32_t domid, const char* name, int *num)
+                         uint32_t domid, int *num)
 {
     void *r = NULL;
     void *list = NULL;
@@ -2002,7 +2002,7 @@  void *libxl__device_list(libxl__gc *gc, const struct libxl_device_type *dt,
     *num = 0;
 
     libxl_path = GCSPRINTF("%s/device/%s",
-                           libxl__xs_libxl_path(gc, domid), name);
+                           libxl__xs_libxl_path(gc, domid), dt->entry);
 
     dir = libxl__xs_directory(gc, XBT_NULL, libxl_path, &ndirs);
 
diff --git a/tools/libxl/libxl_disk.c b/tools/libxl/libxl_disk.c
index 7111b42..263cb56 100644
--- a/tools/libxl/libxl_disk.c
+++ b/tools/libxl/libxl_disk.c
@@ -622,24 +622,6 @@  out:
     return rc;
 }
 
-libxl_device_disk *libxl_device_disk_list(libxl_ctx *ctx, uint32_t domid, int *num)
-{
-    libxl_device_disk *r;
-
-    GC_INIT(ctx);
-
-    r = libxl__device_list(gc, &libxl__disk_devtype, domid, "vbd", num);
-
-    GC_FREE;
-
-    return r;
-}
-
-void libxl_device_disk_list_free(libxl_device_disk *list, int num)
-{
-    libxl__device_list_free(&libxl__disk_devtype, list, num);
-}
-
 int libxl_device_disk_getinfo(libxl_ctx *ctx, uint32_t domid,
                               libxl_device_disk *disk, libxl_diskinfo *diskinfo)
 {
@@ -741,7 +723,7 @@  int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
         goto out;
     }
 
-    disks = libxl__device_list(gc, &libxl__disk_devtype, domid, "vbd", &num);
+    disks = libxl__device_list(gc, &libxl__disk_devtype, domid, &num);
     for (i = 0; i < num; i++) {
         if (disks[i].is_cdrom && !strcmp(disk->vdev, disks[i].vdev))
         {
@@ -1201,9 +1183,11 @@  static int libxl_device_disk_dm_needed(void *e, unsigned domid)
            elem->backend_domid == domid;
 }
 
+LIBXL_DEFINE_DEVICE_LIST(disk)
+
 #define libxl__device_disk_update_devid NULL
 
-DEFINE_DEVICE_TYPE_STRUCT(disk,
+DEFINE_DEVICE_TYPE_STRUCT_X(disk, disk, vbd,
     .merge       = libxl_device_disk_merge,
     .dm_needed   = libxl_device_disk_dm_needed,
     .from_xenstore = (device_from_xenstore_fn_t)libxl__disk_from_xenstore,
diff --git a/tools/libxl/libxl_domain.c b/tools/libxl/libxl_domain.c
index 08eccd0..0434ab9 100644
--- a/tools/libxl/libxl_domain.c
+++ b/tools/libxl/libxl_domain.c
@@ -1679,11 +1679,11 @@  int libxl_retrieve_domain_configuration(libxl_ctx *ctx, uint32_t domid,
             if (!dt)
                 break;
 
-            if (!dt->list || !dt->compare)
+            if (!dt->compare)
                 continue;
 
             num_dev = libxl__device_type_get_num(dt, d_config);
-            p = dt->list(CTX, domid, &num);
+            p = libxl__device_list(gc, dt, domid, &num);
             if (p == NULL) {
                 LOGD(DEBUG, domid, "No %s from xenstore",
                      dt->type);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index c726595..8b71517 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3461,9 +3461,27 @@  _hidden void libxl__bootloader_run(libxl__egc*, libxl__bootloader_state *st);
     LIBXL_DEFINE_DEVICE_REMOVE_EXT(type, type, remove, 0)               \
     LIBXL_DEFINE_DEVICE_REMOVE_EXT(type, type, destroy, 1)
 
+#define LIBXL_DEFINE_DEVICE_LIST(type)                                  \
+    libxl_device_##type *libxl_device_##type##_list(libxl_ctx *ctx,     \
+                                                    uint32_t domid,     \
+                                                    int *num)           \
+    {                                                                   \
+        libxl_device_##type *r;                                         \
+        GC_INIT(ctx);                                                   \
+        r = libxl__device_list(gc, &libxl__##type##_devtype,            \
+                               domid, num);                             \
+        GC_FREE;                                                        \
+        return r;                                                       \
+    }                                                                   \
+                                                                        \
+    void libxl_device_##type##_list_free(libxl_device_##type *list,     \
+                                         int num)                       \
+    {                                                                   \
+        libxl__device_list_free(&libxl__##type##_devtype, list, num);   \
+    }
+
 typedef void (*device_add_fn_t)(libxl__egc *, libxl__ao *, uint32_t,
                                 libxl_domain_config *, libxl__multidev *);
-typedef void *(*device_list_fn_t)(libxl_ctx *, uint32_t, int *);
 typedef int (*device_set_default_fn_t)(libxl__gc *, uint32_t, void *, bool);
 typedef int (*device_to_device_fn_t)(libxl__gc *, uint32_t, void *,
                                      libxl__device *);
@@ -3483,12 +3501,12 @@  typedef int (*device_set_xenstore_config_fn_t)(libxl__gc *, uint32_t, void *,
 
 struct libxl_device_type {
     char *type;
+    char *entry;
     int skip_attach;   /* Skip entry in domcreate_attach_devices() if 1 */
     int ptr_offset;    /* Offset of device array ptr in libxl_domain_config */
     int num_offset;    /* Offset of # of devices in libxl_domain_config */
     int dev_elem_size; /* Size of one device element in array */
     device_add_fn_t                 add;
-    device_list_fn_t                list;
     device_set_default_fn_t         set_default;
     device_to_device_fn_t           to_device;
     device_init_fn_t                init;
@@ -3503,14 +3521,14 @@  struct libxl_device_type {
     device_set_xenstore_config_fn_t set_xenstore_config;
 };
 
-#define DEFINE_DEVICE_TYPE_STRUCT_X(name, sname, ...)                          \
+#define DEFINE_DEVICE_TYPE_STRUCT_X(name, sname, sentry, ...)                  \
     const struct libxl_device_type libxl__ ## name ## _devtype = {             \
         .type          = #sname,                                               \
+        .entry         = #sentry,                                              \
         .ptr_offset    = offsetof(libxl_domain_config, name ## s),             \
         .num_offset    = offsetof(libxl_domain_config, num_ ## name ## s),     \
         .dev_elem_size = sizeof(libxl_device_ ## sname),                       \
         .add           = libxl__add_ ## name ## s,                             \
-        .list          = (device_list_fn_t)libxl_device_ ## sname ## _list,    \
         .set_default   = (device_set_default_fn_t)                             \
                          libxl__device_ ## sname ## _setdefault,               \
         .to_device     = (device_to_device_fn_t)libxl__device_from_ ## name,   \
@@ -3526,7 +3544,7 @@  struct libxl_device_type {
     }
 
 #define DEFINE_DEVICE_TYPE_STRUCT(name, ...)                                   \
-    DEFINE_DEVICE_TYPE_STRUCT_X(name, name, __VA_ARGS__)
+    DEFINE_DEVICE_TYPE_STRUCT_X(name, name, name, __VA_ARGS__)
 
 static inline void **libxl__device_type_get_ptr(
     const struct libxl_device_type *dt, const libxl_domain_config *d_config)
@@ -4338,7 +4356,7 @@  int libxl__device_add(libxl__gc *gc, uint32_t domid,
  * libxl__device_list_free
  */
 void* libxl__device_list(libxl__gc *gc, const struct libxl_device_type *dt,
-                         uint32_t domid, const char* name, int *num);
+                         uint32_t domid, int *num);
 void libxl__device_list_free(const struct libxl_device_type *dt,
                              void *list, int num);
 #endif
diff --git a/tools/libxl/libxl_nic.c b/tools/libxl/libxl_nic.c
index cf8fd5c..9daecf7 100644
--- a/tools/libxl/libxl_nic.c
+++ b/tools/libxl/libxl_nic.c
@@ -29,7 +29,7 @@  int libxl_mac_to_device_nic(libxl_ctx *ctx, uint32_t domid,
     if (rc)
         return rc;
 
-    nics = libxl__device_list(gc, &libxl__nic_devtype, domid, "vif", &nb);
+    nics = libxl__device_list(gc, &libxl__nic_devtype, domid, &nb);
     if (!nics)
         return ERROR_FAIL;
 
@@ -426,7 +426,7 @@  libxl_device_nic *libxl_device_nic_list(libxl_ctx *ctx, uint32_t domid, int *num
 
     GC_INIT(ctx);
 
-    r = libxl__device_list(gc, &libxl__nic_devtype, domid, "vif", num);
+    r = libxl__device_list(gc, &libxl__nic_devtype, domid, num);
 
     GC_FREE;
 
@@ -540,7 +540,7 @@  LIBXL_DEFINE_DEVICE_ADD(nic)
 LIBXL_DEFINE_DEVICES_ADD(nic)
 LIBXL_DEFINE_DEVICE_REMOVE(nic)
 
-DEFINE_DEVICE_TYPE_STRUCT(nic,
+DEFINE_DEVICE_TYPE_STRUCT_X(nic, nic, vif,
     .update_config = libxl_device_nic_update_config,
     .from_xenstore = (device_from_xenstore_fn_t)libxl__nic_from_xenstore,
     .set_xenstore_config = (device_set_xenstore_config_fn_t)
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index f929e5a..88a55ce 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -1682,7 +1682,7 @@  static int libxl_device_pci_compare(libxl_device_pci *d1,
 
 #define libxl__device_pci_update_devid NULL
 
-DEFINE_DEVICE_TYPE_STRUCT_X(pcidev, pci);
+DEFINE_DEVICE_TYPE_STRUCT_X(pcidev, pci, pci);
 
 /*
  * Local variables:
diff --git a/tools/libxl/libxl_vdispl.c b/tools/libxl/libxl_vdispl.c
index a2371ed..e61ef2d 100644
--- a/tools/libxl/libxl_vdispl.c
+++ b/tools/libxl/libxl_vdispl.c
@@ -94,25 +94,6 @@  static int libxl__set_xenstore_vdispl(libxl__gc *gc, uint32_t domid,
     return 0;
 }
 
-libxl_device_vdispl *libxl_device_vdispl_list(libxl_ctx *ctx, uint32_t domid,
-                                              int *num)
-{
-    libxl_device_vdispl *r;
-
-    GC_INIT(ctx);
-
-    r = libxl__device_list(gc, &libxl__vdispl_devtype, domid, "vdispl", num);
-
-    GC_FREE;
-
-    return r;
-}
-
-void libxl_device_vdispl_list_free(libxl_device_vdispl* list, int num)
-{
-    libxl__device_list_free(&libxl__vdispl_devtype, list, num);
-}
-
 static int libxl__device_vdispl_getconnectors(libxl_ctx *ctx,
                                               const char *path,
                                               libxl_vdisplinfo *info)
@@ -239,8 +220,7 @@  int libxl_devid_to_device_vdispl(libxl_ctx *ctx, uint32_t domid,
 
     libxl_device_vdispl_init(vdispl);
 
-    vdispls = libxl__device_list(gc, &libxl__vdispl_devtype,
-                                 domid, "vdispl", &n);
+    vdispls = libxl__device_list(gc, &libxl__vdispl_devtype, domid, &n);
 
     if (!vdispls) { rc = ERROR_NOTFOUND; goto out; }
 
@@ -267,6 +247,7 @@  LIBXL_DEFINE_DEVICE_ADD(vdispl)
 static LIBXL_DEFINE_DEVICES_ADD(vdispl)
 LIBXL_DEFINE_DEVICE_REMOVE(vdispl)
 static LIBXL_DEFINE_UPDATE_DEVID(vdispl, "vdispl")
+LIBXL_DEFINE_DEVICE_LIST(vdispl)
 
 DEFINE_DEVICE_TYPE_STRUCT(vdispl,
     .update_config = (device_update_config_fn_t)libxl__update_config_vdispl,
diff --git a/tools/libxl/libxl_vtpm.c b/tools/libxl/libxl_vtpm.c
index a06f15d..3f0c563 100644
--- a/tools/libxl/libxl_vtpm.c
+++ b/tools/libxl/libxl_vtpm.c
@@ -104,24 +104,6 @@  static int libxl__vtpm_from_xenstore(libxl__gc *gc, const char *libxl_path,
     return 0;
 }
 
-libxl_device_vtpm *libxl_device_vtpm_list(libxl_ctx *ctx, uint32_t domid, int *num)
-{
-    libxl_device_vtpm *r;
-
-    GC_INIT(ctx);
-
-    r = libxl__device_list(gc, &libxl__vtpm_devtype, domid, "vtpm", num);
-
-    GC_FREE;
-
-    return r;
-}
-
-void libxl_device_vtpm_list_free(libxl_device_vtpm* list, int num)
-{
-    libxl__device_list_free(&libxl__vtpm_devtype, list, num);
-}
-
 int libxl_device_vtpm_getinfo(libxl_ctx *ctx,
                               uint32_t domid,
                               libxl_device_vtpm *vtpm,
@@ -196,7 +178,7 @@  int libxl_devid_to_device_vtpm(libxl_ctx *ctx,
     int nb, i;
     int rc;
 
-    vtpms = libxl__device_list(gc, &libxl__vtpm_devtype, domid, "vtpm", &nb);
+    vtpms = libxl__device_list(gc, &libxl__vtpm_devtype, domid, &nb);
     if (!vtpms)
         return ERROR_FAIL;
 
@@ -231,7 +213,7 @@  int libxl_uuid_to_device_vtpm(libxl_ctx *ctx, uint32_t domid,
     int nb, i;
     int rc;
 
-    vtpms = libxl__device_list(gc, &libxl__vtpm_devtype, domid, "vtpm", &nb);
+    vtpms = libxl__device_list(gc, &libxl__vtpm_devtype, domid, &nb);
     if (!vtpms)
         return ERROR_FAIL;
 
@@ -260,6 +242,7 @@  static void libxl_device_vtpm_update_config(libxl__gc *gc, void *d, void *s)
 LIBXL_DEFINE_DEVICE_ADD(vtpm)
 static LIBXL_DEFINE_DEVICES_ADD(vtpm)
 LIBXL_DEFINE_DEVICE_REMOVE(vtpm)
+LIBXL_DEFINE_DEVICE_LIST(vtpm)
 
 DEFINE_DEVICE_TYPE_STRUCT(vtpm,
     .update_config = libxl_device_vtpm_update_config,