[PATCH v4 0/5] libxl: update available vcpus map in retrieved configuration

[PATCH v4 0/5] libxl: update available vcpus map in retrieved configuration

[Wei Liu]

See individual patch for detailed changelog.

Wei Liu (5):
  libxl: constify copy and length calculation functions
  libxl: libxl_domain_need_memory shouldn't modify b_info
  libxl: introduce libxl__qmp_query_cpus
  libxl: update vcpus bitmap in retrieved guest config
  libxl: only issue cpu-add call to QEMU for not present CPU

 tools/libxl/gentypes.py      |   4 +-
 tools/libxl/libxl.c          | 147 ++++++++++++++++++++++++++++++++++++++-----
 tools/libxl/libxl.h          |  33 +++++++---
 tools/libxl/libxl_cpuid.c    |   4 +-
 tools/libxl/libxl_genid.c    |   2 +-
 tools/libxl/libxl_internal.h |   5 +-
 tools/libxl/libxl_qmp.c      |  38 +++++++++++
 tools/libxl/libxl_utils.c    |   2 +-
 tools/libxl/libxl_utils.h    |   2 +-
 9 files changed, 204 insertions(+), 33 deletions(-)

-- 
2.1.4


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

[PATCH v4 1/5] libxl: constify copy and length calculation functions

[Wei Liu]

These functions are not supposed to modify the passed in parameters.
Reflect that in function declarations.

Mark the change in APIs in libxl.h

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
---
 tools/libxl/gentypes.py      |  4 ++--
 tools/libxl/libxl.c          | 10 +++++-----
 tools/libxl/libxl.h          | 23 +++++++++++++++--------
 tools/libxl/libxl_cpuid.c    |  4 ++--
 tools/libxl/libxl_genid.c    |  2 +-
 tools/libxl/libxl_internal.h |  2 +-
 tools/libxl/libxl_utils.c    |  2 +-
 tools/libxl/libxl_utils.h    |  2 +-
 8 files changed, 28 insertions(+), 21 deletions(-)

diff --git a/tools/libxl/gentypes.py b/tools/libxl/gentypes.py
index 00816c0..4ea7091 100644
--- a/tools/libxl/gentypes.py
+++ b/tools/libxl/gentypes.py
@@ -544,7 +544,7 @@ if __name__ == '__main__':
         if ty.dispose_fn is not None:
             f.write("%svoid %s(%s);\n" % (ty.hidden(), ty.dispose_fn, ty.make_arg("p")))
         if ty.copy_fn is not None:
-            f.write("%svoid %s(libxl_ctx *ctx, %s, %s);\n" % (ty.hidden(), ty.copy_fn,
+            f.write("%svoid %s(libxl_ctx *ctx, %s, const %s);\n" % (ty.hidden(), ty.copy_fn,
                                               ty.make_arg("dst"), ty.make_arg("src")))
         if ty.init_fn is not None:
             f.write("%svoid %s(%s);\n" % (ty.hidden(), ty.init_fn, ty.make_arg("p")))
@@ -649,7 +649,7 @@ if __name__ == '__main__':
         f.write("\n")
 
     for ty in [t for t in types if t.copy_fn and t.autogenerate_copy_fn]:
-        f.write("void %s(libxl_ctx *ctx, %s, %s)\n" % (ty.copy_fn,
+        f.write("void %s(libxl_ctx *ctx, %s, const %s)\n" % (ty.copy_fn,
                                        ty.make_arg("dst", passby=idl.PASS_BY_REFERENCE),
                                        ty.make_arg("src", passby=idl.PASS_BY_REFERENCE)))
         f.write("{\n")
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 1c81239..0c34d6b 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -234,7 +234,7 @@ void libxl_string_list_dispose(libxl_string_list *psl)
 
 void libxl_string_list_copy(libxl_ctx *ctx,
                             libxl_string_list *dst,
-                            libxl_string_list *src)
+                            const libxl_string_list *src)
 {
     GC_INIT(ctx);
     int i, len;
@@ -266,7 +266,7 @@ int libxl_string_list_length(const libxl_string_list *psl)
     return i;
 }
 
-int libxl_key_value_list_length(libxl_key_value_list *pkvl)
+int libxl_key_value_list_length(const libxl_key_value_list *pkvl)
 {
     int i = 0;
     libxl_key_value_list kvl = *pkvl;
@@ -301,7 +301,7 @@ void libxl_key_value_list_dispose(libxl_key_value_list *pkvl)
 
 void libxl_key_value_list_copy(libxl_ctx *ctx,
                                libxl_key_value_list *dst,
-                               libxl_key_value_list *src)
+                               const libxl_key_value_list *src)
 {
     GC_INIT(ctx);
     int i, len;
@@ -7227,7 +7227,7 @@ out_err:
 
 }
 
-void libxl_hwcap_copy(libxl_ctx *ctx,libxl_hwcap *dst, libxl_hwcap *src)
+void libxl_hwcap_copy(libxl_ctx *ctx,libxl_hwcap *dst, const libxl_hwcap *src)
 {
     int i;
 
@@ -7235,7 +7235,7 @@ void libxl_hwcap_copy(libxl_ctx *ctx,libxl_hwcap *dst, libxl_hwcap *src)
         (*dst)[i] = (*src)[i];
 }
 
-void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, libxl_mac *src)
+void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, const libxl_mac *src)
 {
     int i;
 
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 2c0f868..f2843fd 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -67,6 +67,13 @@
  * the same $(XEN_VERSION) (e.g. throughout a major release).
  */
 
+/* LIBXL_HAVE_CONST_COPY_AND_LENGTH_FUNCTIONS
+ *
+ * If this is defined, the copy functions have constified src parameter and the
+ * length functions accept constified parameter.
+ */
+#define LIBXL_HAVE_CONST_COPY_AND_LENGTH_FUNCTIONS 1
+
 /* LIBXL_HAVE_VNUMA
  *
  * If this is defined the type libxl_vnode_info exists, and a
@@ -839,7 +846,7 @@ typedef uint8_t libxl_mac[6];
 #define LIBXL_MAC_FMT "%02hhx:%02hhx:%02hhx:%02hhx:%02hhx:%02hhx"
 #define LIBXL_MAC_FMTLEN ((2*6)+5) /* 6 hex bytes plus 5 colons */
 #define LIBXL_MAC_BYTES(mac) mac[0], mac[1], mac[2], mac[3], mac[4], mac[5]
-void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, libxl_mac *src);
+void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, const libxl_mac *src);
 
 #if defined(__i386__) || defined(__x86_64__)
 /*
@@ -962,17 +969,17 @@ typedef char **libxl_string_list;
 void libxl_string_list_dispose(libxl_string_list *sl);
 int libxl_string_list_length(const libxl_string_list *sl);
 void libxl_string_list_copy(libxl_ctx *ctx, libxl_string_list *dst,
-                            libxl_string_list *src);
+                            const libxl_string_list *src);
 
 typedef char **libxl_key_value_list;
 void libxl_key_value_list_dispose(libxl_key_value_list *kvl);
-int libxl_key_value_list_length(libxl_key_value_list *kvl);
+int libxl_key_value_list_length(const libxl_key_value_list *kvl);
 void libxl_key_value_list_copy(libxl_ctx *ctx,
                                libxl_key_value_list *dst,
-                               libxl_key_value_list *src);
+                               const libxl_key_value_list *src);
 
 typedef uint32_t libxl_hwcap[8];
-void libxl_hwcap_copy(libxl_ctx *ctx, libxl_hwcap *dst, libxl_hwcap *src);
+void libxl_hwcap_copy(libxl_ctx *ctx, libxl_hwcap *dst, const libxl_hwcap *src);
 
 typedef uint64_t libxl_ev_user;
 
@@ -990,10 +997,10 @@ void libxl_bitmap_dispose(libxl_bitmap *map);
 typedef struct libxl__cpuid_policy libxl_cpuid_policy;
 typedef libxl_cpuid_policy * libxl_cpuid_policy_list;
 void libxl_cpuid_dispose(libxl_cpuid_policy_list *cpuid_list);
-int libxl_cpuid_policy_list_length(libxl_cpuid_policy_list *l);
+int libxl_cpuid_policy_list_length(const libxl_cpuid_policy_list *l);
 void libxl_cpuid_policy_list_copy(libxl_ctx *ctx,
                                   libxl_cpuid_policy_list *dst,
-                                  libxl_cpuid_policy_list *src);
+                                  const libxl_cpuid_policy_list *src);
 
 #define LIBXL_PCI_FUNC_ALL (~0U)
 
@@ -2004,7 +2011,7 @@ int libxl_flask_loadpolicy(libxl_ctx *ctx, void *policy, uint32_t size);
 int libxl_ms_vm_genid_generate(libxl_ctx *ctx, libxl_ms_vm_genid *id);
 bool libxl_ms_vm_genid_is_zero(const libxl_ms_vm_genid *id);
 void libxl_ms_vm_genid_copy(libxl_ctx *ctx, libxl_ms_vm_genid *dst,
-                            libxl_ms_vm_genid *src);
+                            const libxl_ms_vm_genid *src);
 
 #ifdef LIBXL_HAVE_PSR_CMT
 int libxl_psr_cmt_attach(libxl_ctx *ctx, uint32_t domid);
diff --git a/tools/libxl/libxl_cpuid.c b/tools/libxl/libxl_cpuid.c
index fc20157..24591e2 100644
--- a/tools/libxl/libxl_cpuid.c
+++ b/tools/libxl/libxl_cpuid.c
@@ -465,7 +465,7 @@ int libxl__cpuid_policy_list_parse_json(libxl__gc *gc,
     return 0;
 }
 
-int libxl_cpuid_policy_list_length(libxl_cpuid_policy_list *pl)
+int libxl_cpuid_policy_list_length(const libxl_cpuid_policy_list *pl)
 {
     int i = 0;
     libxl_cpuid_policy_list l = *pl;
@@ -480,7 +480,7 @@ int libxl_cpuid_policy_list_length(libxl_cpuid_policy_list *pl)
 
 void libxl_cpuid_policy_list_copy(libxl_ctx *ctx,
                                   libxl_cpuid_policy_list *dst,
-                                  libxl_cpuid_policy_list *src)
+                                  const libxl_cpuid_policy_list *src)
 {
     GC_INIT(ctx);
     int i, j, len;
diff --git a/tools/libxl/libxl_genid.c b/tools/libxl/libxl_genid.c
index f1c4eb7..7f52356 100644
--- a/tools/libxl/libxl_genid.c
+++ b/tools/libxl/libxl_genid.c
@@ -52,7 +52,7 @@ bool libxl_ms_vm_genid_is_zero(const libxl_ms_vm_genid *id)
 }
 
 void libxl_ms_vm_genid_copy(libxl_ctx *ctx, libxl_ms_vm_genid *dst,
-                            libxl_ms_vm_genid *src)
+                            const libxl_ms_vm_genid *src)
 {
     memcpy(dst, src, LIBXL_MS_VM_GENID_LEN);
 }
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index e7ab85d..de77579 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -4095,7 +4095,7 @@ void libxl__update_domain_configuration(libxl__gc *gc,
                                         const libxl_domain_config *src);
 static inline void libxl__update_config_nic(libxl__gc *gc,
                                             libxl_device_nic *dst,
-                                            libxl_device_nic *src)
+                                            const libxl_device_nic *src)
 {
     dst->devid = src->devid;
     dst->nictype = src->nictype;
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 6108d4b..1607d4f 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -1271,7 +1271,7 @@ int libxl_domid_valid_guest(uint32_t domid)
     return domid > 0 && domid < DOMID_FIRST_RESERVED;
 }
 
-void libxl_string_copy(libxl_ctx *ctx, char **dst, char **src)
+void libxl_string_copy(libxl_ctx *ctx, char **dst, char * const*src)
 {
     GC_INIT(ctx);
 
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index 4495417..25773d8 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -167,7 +167,7 @@ int libxl_cpumap_to_nodemap(libxl_ctx *ctx,
     return (s + 1023) / 1024;
 }
 
-void libxl_string_copy(libxl_ctx *ctx, char **dst, char **src);
+void libxl_string_copy(libxl_ctx *ctx, char **dst, char * const*src);
 
 
 #define LIBXL_FILLZERO(object) (memset(&(object), 0, sizeof((object))))
-- 
2.1.4


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

[PATCH v4 2/5] libxl: libxl_domain_need_memory shouldn't modify b_info

[Wei Liu]

This function is used to return the memory needed for a guest. It's not
in a position to modify the b_info passed in (note the _setdefault
function).

Constify the passed in b_info, use a copy to do the calculation. Mark
the change in API in libxl.h.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>

v4: constify b_info and use LIBXL_HAVE_NEED_MEMORY_CONST_B_INFO

v3: new
---
 tools/libxl/libxl.c |  8 +++++++-
 tools/libxl/libxl.h | 10 +++++++++-
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 0c34d6b..51d202f 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -5121,12 +5121,17 @@ int libxl_get_memory_target(libxl_ctx *ctx, uint32_t domid,
     return rc;
 }
 
-int libxl_domain_need_memory(libxl_ctx *ctx, libxl_domain_build_info *b_info,
+int libxl_domain_need_memory(libxl_ctx *ctx,
+                             const libxl_domain_build_info *b_info_in,
                              uint32_t *need_memkb)
 {
     GC_INIT(ctx);
+    libxl_domain_build_info b_info[1];
     int rc;
 
+    libxl_domain_build_info_init(b_info);
+    libxl_domain_build_info_copy(ctx, b_info, b_info_in);
+
     rc = libxl__domain_build_info_setdefault(gc, b_info);
     if (rc) goto out;
 
@@ -5149,6 +5154,7 @@ int libxl_domain_need_memory(libxl_ctx *ctx, libxl_domain_build_info *b_info,
     rc = 0;
 out:
     GC_FREE;
+    libxl_domain_build_info_dispose(b_info);
     return rc;
 
 }
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index f2843fd..48a43ce 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -74,6 +74,13 @@
  */
 #define LIBXL_HAVE_CONST_COPY_AND_LENGTH_FUNCTIONS 1
 
+/* LIBXL_HAVE_DOMAIN_NEED_MEMORY_CONST_B_INFO
+ *
+ * If this is defined, libxl_domain_need_memory no longer modifies
+ * the b_info paseed in.
+ */
+#define LIBXL_HAVE_DOMAIN_NEED_MEMORY_CONST_B_INFO 1
+
 /* LIBXL_HAVE_VNUMA
  *
  * If this is defined the type libxl_vnode_info exists, and a
@@ -1383,7 +1390,8 @@ int libxl_get_memory_target(libxl_ctx *ctx, uint32_t domid, uint32_t *out_target
  * existing programs which use them in roughly the same way as libxl.
  */
 /* how much free memory in the system a domain needs to be built */
-int libxl_domain_need_memory(libxl_ctx *ctx, libxl_domain_build_info *b_info,
+int libxl_domain_need_memory(libxl_ctx *ctx,
+                             const libxl_domain_build_info *b_info_in,
                              uint32_t *need_memkb);
 /* how much free memory is available in the system */
 int libxl_get_free_memory(libxl_ctx *ctx, uint32_t *memkb);
-- 
2.1.4


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

[PATCH v4 3/5] libxl: introduce libxl__qmp_query_cpus

[Wei Liu]

It interrogates QEMU for CPUs and update the bitmap accordingly.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Anthony PERARD <anthony.perard@citrix.com>

v3:
1. Initialise rc in error path.
2. Fix comment in header and a typo in log message.
---
 tools/libxl/libxl_internal.h |  3 +++
 tools/libxl/libxl_qmp.c      | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index de77579..e33c710 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1794,6 +1794,9 @@ _hidden int libxl__qmp_set_global_dirty_log(libxl__gc *gc, int domid, bool enabl
 _hidden int libxl__qmp_insert_cdrom(libxl__gc *gc, int domid, const libxl_device_disk *disk);
 /* Add a virtual CPU */
 _hidden int libxl__qmp_cpu_add(libxl__gc *gc, int domid, int index);
+/* Query the bitmap of CPUs */
+_hidden int libxl__qmp_query_cpus(libxl__gc *gc, int domid,
+                                  libxl_bitmap *map);
 /* Start NBD server */
 _hidden int libxl__qmp_nbd_server_start(libxl__gc *gc, int domid,
                                         const char *host, const char *port);
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index 3eb279a..63c49c5 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -979,6 +979,44 @@ int libxl__qmp_cpu_add(libxl__gc *gc, int domid, int idx)
     return qmp_run_command(gc, domid, "cpu-add", args, NULL, NULL);
 }
 
+static int query_cpus_callback(libxl__qmp_handler *qmp,
+                               const libxl__json_object *response,
+                               void *opaque)
+{
+    libxl_bitmap *map = opaque;
+    unsigned int i;
+    const libxl__json_object *cpu = NULL;
+    int rc;
+    GC_INIT(qmp->ctx);
+
+    libxl_bitmap_set_none(map);
+    for (i = 0; (cpu = libxl__json_array_get(response, i)); i++) {
+        unsigned int idx;
+        const libxl__json_object *o;
+
+        o = libxl__json_map_get("CPU", cpu, JSON_INTEGER);
+        if (!o) {
+            LOG(ERROR, "Failed to retrieve CPU index.");
+            rc = ERROR_FAIL;
+            goto out;
+        }
+
+        idx = libxl__json_object_get_integer(o);
+        libxl_bitmap_set(map, idx);
+    }
+
+    rc = 0;
+out:
+    GC_FREE;
+    return rc;
+}
+
+int libxl__qmp_query_cpus(libxl__gc *gc, int domid, libxl_bitmap *map)
+{
+    return qmp_run_command(gc, domid, "query-cpus", NULL,
+                           query_cpus_callback, map);
+}
+
 int libxl__qmp_nbd_server_start(libxl__gc *gc, int domid,
                                 const char *host, const char *port)
 {
-- 
2.1.4


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

[PATCH v4 4/5] libxl: update vcpus bitmap in retrieved guest config

[Wei Liu]

... because the available vcpu bitmap can change during domain life time
due to cpu hotplug and unplug.

For QEMU upstream, we interrogate QEMU for the number of vcpus. For
others, we look directly into xenstore for information.

Reported-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Anthony PERARD <anthony.perard@citrix.com>

v4:
1. Use libxl__device_model_version_running
2. Move comment

v3:
1. Fix indentation of abort.
2. Use strcmp instead of strncmp.
---
 tools/libxl/libxl.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 90 insertions(+)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 51d202f..3786b09 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -7249,6 +7249,53 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, const libxl_mac *src)
         (*dst)[i] = (*src)[i];
 }
 
+/* For QEMU upstream we always need to provide the number of cpus present to
+ * QEMU whether they are online or not; otherwise QEMU won't accept the saved
+ * state. See implementation of libxl__qmp_query_cpus.
+ */
+static int libxl__update_avail_vcpus_qmp(libxl__gc *gc, uint32_t domid,
+                                         unsigned int max_vcpus,
+                                         libxl_bitmap *map)
+{
+    int rc;
+
+    rc = libxl__qmp_query_cpus(gc, domid, map);
+    if (rc) {
+        LOG(ERROR, "fail to get number of cpus for domain %d", domid);
+        goto out;
+    }
+
+    rc = 0;
+out:
+    return rc;
+}
+
+static int libxl__update_avail_vcpus_xenstore(libxl__gc *gc, uint32_t domid,
+                                              unsigned int max_vcpus,
+                                              libxl_bitmap *map)
+{
+    int rc;
+    unsigned int i;
+    const char *dompath;
+
+    dompath = libxl__xs_get_dompath(gc, domid);
+    if (!dompath) {
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
+    for (i = 0; i < max_vcpus; i++) {
+        const char *path = GCSPRINTF("%s/cpu/%u/availability", dompath, i);
+        const char *content = libxl__xs_read(gc, XBT_NULL, path);
+        if (!strcmp(content, "online"))
+            libxl_bitmap_set(map, i);
+    }
+
+    rc = 0;
+out:
+    return rc;
+}
+
 int libxl_retrieve_domain_configuration(libxl_ctx *ctx, uint32_t domid,
                                         libxl_domain_config *d_config)
 {
@@ -7297,6 +7344,49 @@ int libxl_retrieve_domain_configuration(libxl_ctx *ctx, uint32_t domid,
         libxl_dominfo_dispose(&info);
     }
 
+    /* VCPUs */
+    {
+        libxl_bitmap *map = &d_config->b_info.avail_vcpus;
+        unsigned int max_vcpus = d_config->b_info.max_vcpus;
+        libxl_device_model_version version;
+
+        libxl_bitmap_dispose(map);
+        libxl_bitmap_init(map);
+        libxl_bitmap_alloc(CTX, map, max_vcpus);
+        libxl_bitmap_set_none(map);
+
+        switch (d_config->b_info.type) {
+        case LIBXL_DOMAIN_TYPE_HVM:
+            version = libxl__device_model_version_running(gc, domid);
+            assert(version != LIBXL_DEVICE_MODEL_VERSION_UNKNOWN);
+            switch (version) {
+            case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
+                rc = libxl__update_avail_vcpus_qmp(gc, domid,
+                                                   max_vcpus, map);
+                break;
+            case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
+            case LIBXL_DEVICE_MODEL_VERSION_NONE:
+                rc = libxl__update_avail_vcpus_xenstore(gc, domid,
+                                                        max_vcpus, map);
+                break;
+            default:
+                abort();
+            }
+            break;
+        case LIBXL_DOMAIN_TYPE_PV:
+            rc = libxl__update_avail_vcpus_xenstore(gc, domid,
+                                                    max_vcpus, map);
+            break;
+        default:
+            abort();
+        }
+
+        if (rc) {
+            LOG(ERROR, "fail to update available cpu map for domain %d", domid);
+            goto out;
+        }
+    }
+
     /* Memory limits:
      *
      * Currently there are three memory limits:
-- 
2.1.4


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

[PATCH v4 5/5] libxl: only issue cpu-add call to QEMU for not present CPU

[Wei Liu]

Calculate the final bitmap for CPUs to add to avoid having annoying
error messages complaining those CPUs are already present. Example
message is like (wrapped):

libxl: error: libxl_qmp.c:287:qmp_handle_error_response: received an
error message from QMP server: Unable to add CPU: 0, it already exists

We can also properly handle error from QMP now.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Anthony PERARD <anthony.perard@citrix.com>

v4:
1. update commit log to contain example error message.

v3:
1. Add Anthony's Reviewed-by tag.
---
 tools/libxl/libxl.c | 39 +++++++++++++++++++++++++++++----------
 1 file changed, 29 insertions(+), 10 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 3786b09..e4b0424 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -5756,19 +5756,38 @@ static int libxl__set_vcpuonline_qmp(libxl__gc *gc, uint32_t domid,
                                      libxl_bitmap *cpumap,
                                      const libxl_dominfo *info)
 {
-    int i;
+    int i, rc;
+    libxl_bitmap current_map, final_map;
+
+    libxl_bitmap_init(&current_map);
+    libxl_bitmap_init(&final_map);
+
+    libxl_bitmap_alloc(CTX, &current_map, info->vcpu_max_id + 1);
+    libxl_bitmap_set_none(&current_map);
+    rc = libxl__qmp_query_cpus(gc, domid, &current_map);
+    if (rc) {
+        LOG(ERROR, "failed to query cpus for domain %d", domid);
+        goto out;
+    }
+
+    libxl_bitmap_copy_alloc(CTX, &final_map, cpumap);
 
-    for (i = 0; i <= info->vcpu_max_id; i++) {
-        if (libxl_bitmap_test(cpumap, i)) {
-            /* Return value is ignore because it does not tell anything useful
-             * on the completion of the command.
-             * (For instance, "CPU already plugged-in" give the same return
-             * value as "command not supported".)
-             */
-            libxl__qmp_cpu_add(gc, domid, i);
+    libxl_for_each_set_bit(i, current_map)
+        libxl_bitmap_reset(&final_map, i);
+
+    libxl_for_each_set_bit(i, final_map) {
+        rc = libxl__qmp_cpu_add(gc, domid, i);
+        if (rc) {
+            LOG(ERROR, "failed to add cpu %d to domain %d", i, domid);
+            goto out;
         }
     }
-    return 0;
+
+    rc = 0;
+out:
+    libxl_bitmap_dispose(&current_map);
+    libxl_bitmap_dispose(&final_map);
+    return rc;
 }
 
 int libxl_set_vcpuonline(libxl_ctx *ctx, uint32_t domid, libxl_bitmap *cpumap)
-- 
2.1.4


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

Re: [PATCH v4 1/5] libxl: constify copy and length calculation functions

[Ian Jackson]

Wei Liu writes ("[PATCH v4 1/5] libxl: constify copy and length calculation functions"):
> These functions are not supposed to modify the passed in parameters.
> Reflect that in function declarations.

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

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

Re: [PATCH v4 2/5] libxl: libxl_domain_need_memory shouldn't modify b_info

[Ian Jackson]

Wei Liu writes ("[PATCH v4 2/5] libxl: libxl_domain_need_memory shouldn't modify b_info"):
> This function is used to return the memory needed for a guest. It's not
> in a position to modify the b_info passed in (note the _setdefault
> function).
> 
> Constify the passed in b_info, use a copy to do the calculation. Mark
> the change in API in libxl.h.

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

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

Re: [PATCH v4 4/5] libxl: update vcpus bitmap in retrieved guest config

[Ian Jackson]

Wei Liu writes ("[PATCH v4 4/5] libxl: update vcpus bitmap in retrieved guest config"):
> ... because the available vcpu bitmap can change during domain life time
> due to cpu hotplug and unplug.
> 
> For QEMU upstream, we interrogate QEMU for the number of vcpus. For
> others, we look directly into xenstore for information.
> 
> Reported-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

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

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

Re: [PATCH v4 5/5] libxl: only issue cpu-add call to QEMU for not present CPU

[Ian Jackson]

Wei Liu writes ("[PATCH v4 5/5] libxl: only issue cpu-add call to QEMU for not present CPU"):
> Calculate the final bitmap for CPUs to add to avoid having annoying
> error messages complaining those CPUs are already present. Example
> message is like (wrapped):
> 
> libxl: error: libxl_qmp.c:287:qmp_handle_error_response: received an
> error message from QMP server: Unable to add CPU: 0, it already exists
> 
> We can also properly handle error from QMP now.

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

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

Re: [PATCH v4 4/5] libxl: update vcpus bitmap in retrieved guest config

[Wei Liu]

On Mon, Jul 11, 2016 at 04:56:02PM +0100, Ian Jackson wrote:
> Wei Liu writes ("[PATCH v4 4/5] libxl: update vcpus bitmap in retrieved guest config"):
> > ... because the available vcpu bitmap can change during domain life time
> > due to cpu hotplug and unplug.
> > 
> > For QEMU upstream, we interrogate QEMU for the number of vcpus. For
> > others, we look directly into xenstore for information.
> > 
> > Reported-by: Jan Beulich <jbeulich@suse.com>
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> 
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Sorry I just realise there is one hunk I forgot to commit.

The hunk is (on top of this patch) to use libxl__xs_read_checked

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index e4b0424..e49741d 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -7305,8 +7305,10 @@ static int libxl__update_avail_vcpus_xenstore(libxl__gc *gc, uint32_t domid,
 
     for (i = 0; i < max_vcpus; i++) {
         const char *path = GCSPRINTF("%s/cpu/%u/availability", dompath, i);
-        const char *content = libxl__xs_read(gc, XBT_NULL, path);
-        if (!strcmp(content, "online"))
+        const char *content;
+        rc = libxl__xs_read_checked(gc, XBT_NULL, path, &content);
+        if (rc) goto out;
+        if (content && !strcmp(content, "online"))
             libxl_bitmap_set(map, i);
     }
 
Is it ok to keep your ack?

Wei.

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

Re: [PATCH v4 4/5] libxl: update vcpus bitmap in retrieved guest config

[Ian Jackson]

Wei Liu writes ("Re: [PATCH v4 4/5] libxl: update vcpus bitmap in retrieved guest config"):
> The hunk is (on top of this patch) to use libxl__xs_read_checked

Errr.  Sorry for not spotting that in my review.

> Is it ok to keep your ack?

Yes, thanks.

Ian.

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

Re: [PATCH v4 0/5] libxl: update available vcpus map in retrieved configuration

[Wei Liu]

On Mon, Jul 11, 2016 at 04:49:32PM +0100, Wei Liu wrote:
> See individual patch for detailed changelog.

Series pushed.

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