This patch series enables libxl to set dirty log on QEMU upstream during
migration through a new QMP command.
The success of the call depends on the presence of the specific QMP command
xen-set-global-dirty-log in QEMU. Patches for this command have been sent.
Also, a recent patch to libxl will probably prevent this from working.
Anthony PERARD (6):
libxl_json: Export json_object related function.
libxl_json: Introduce libxl__json_object_to_yajl_gen.
libxl_qmp: Introduces helpers to create an argument list.
libxl_qmp: Use qmp_parameters_* functions for param list of a QMP
command.
libxl_qmp: Introduce libxl__qmp_set_global_dirty_log.
libxl_dom: Call the right switch logdirty for the right DM.
tools/libxl/libxl_dom.c | 41 ++++++++-
tools/libxl/libxl_internal.h | 10 ++
tools/libxl/libxl_json.c | 97 ++++++++++++++++---
tools/libxl/libxl_qmp.c | 207 ++++++++++++++++++++++++++++++++----------
4 files changed, 289 insertions(+), 66 deletions(-)
--
Anthony PERARD
Anthony PERARD
2012-Jul-25 17:04 UTC
[PATCH 1/6] libxl_json: Export json_object related function.
Export libxl__json_object_alloc and libxl__json_object_append_to to use them in
a later patch.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
tools/libxl/libxl_internal.h | 5 +++++
tools/libxl/libxl_json.c | 32 ++++++++++++++++----------------
2 files changed, 21 insertions(+), 16 deletions(-)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index f832fbd..c233416 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1465,6 +1465,11 @@ static inline long long
libxl__json_object_get_integer(const libxl__json_object
return -1;
}
+_hidden libxl__json_object *libxl__json_object_alloc(libxl__gc *gc,
+ libxl__json_node_type
type);
+_hidden int libxl__json_object_append_to(libxl__gc *gc,
+ libxl__json_object *obj,
+ libxl__json_object *dst);
_hidden libxl__json_object *libxl__json_array_get(const libxl__json_object *o,
int i);
_hidden
diff --git a/tools/libxl/libxl_json.c b/tools/libxl/libxl_json.c
index e870606..9e302d5 100644
--- a/tools/libxl/libxl_json.c
+++ b/tools/libxl/libxl_json.c
@@ -205,7 +205,7 @@ yajl_gen_status libxl__string_gen_json(yajl_gen hand,
* libxl__json_object helper functions
*/
-static libxl__json_object *json_object_alloc(libxl__gc *gc,
+libxl__json_object *libxl__json_object_alloc(libxl__gc *gc,
libxl__json_node_type type)
{
libxl__json_object *obj;
@@ -236,7 +236,7 @@ static libxl__json_object *json_object_alloc(libxl__gc *gc,
return obj;
}
-static int json_object_append_to(libxl__gc *gc,
+int libxl__json_object_append_to(libxl__gc *gc,
libxl__json_object *obj,
libxl__json_object *dst)
{
@@ -393,10 +393,10 @@ static int json_callback_null(void *opaque)
DEBUG_GEN(ctx, null);
- if ((obj = json_object_alloc(ctx->gc, JSON_NULL)) == NULL)
+ if ((obj = libxl__json_object_alloc(ctx->gc, JSON_NULL)) == NULL)
return 0;
- if (json_object_append_to(ctx->gc, obj, ctx->current) == -1) {
+ if (libxl__json_object_append_to(ctx->gc, obj, ctx->current) == -1) {
libxl__json_object_free(ctx->gc, obj);
return 0;
}
@@ -411,11 +411,11 @@ static int json_callback_boolean(void *opaque, int
boolean)
DEBUG_GEN_VALUE(ctx, bool, boolean);
- if ((obj = json_object_alloc(ctx->gc,
+ if ((obj = libxl__json_object_alloc(ctx->gc,
boolean ? JSON_TRUE : JSON_FALSE)) == NULL)
return 0;
- if (json_object_append_to(ctx->gc, obj, ctx->current) == -1) {
+ if (libxl__json_object_append_to(ctx->gc, obj, ctx->current) == -1) {
libxl__json_object_free(ctx->gc, obj);
return 0;
}
@@ -448,7 +448,7 @@ static int json_callback_number(void *opaque, const char *s,
libxl_yajl_length l
goto error;
}
- if ((obj = json_object_alloc(ctx->gc, JSON_DOUBLE)) == NULL)
+ if ((obj = libxl__json_object_alloc(ctx->gc, JSON_DOUBLE)) == NULL)
return 0;
obj->u.d = d;
} else {
@@ -458,7 +458,7 @@ static int json_callback_number(void *opaque, const char *s,
libxl_yajl_length l
goto error;
}
- if ((obj = json_object_alloc(ctx->gc, JSON_INTEGER)) == NULL)
+ if ((obj = libxl__json_object_alloc(ctx->gc, JSON_INTEGER)) == NULL)
return 0;
obj->u.i = i;
}
@@ -466,7 +466,7 @@ static int json_callback_number(void *opaque, const char *s,
libxl_yajl_length l
error:
/* If the conversion fail, we just store the original string. */
- if ((obj = json_object_alloc(ctx->gc, JSON_NUMBER)) == NULL)
+ if ((obj = libxl__json_object_alloc(ctx->gc, JSON_NUMBER)) == NULL)
return 0;
t = malloc(len + 1);
@@ -481,7 +481,7 @@ error:
obj->u.string = t;
out:
- if (json_object_append_to(ctx->gc, obj, ctx->current) == -1) {
+ if (libxl__json_object_append_to(ctx->gc, obj, ctx->current) == -1) {
libxl__json_object_free(ctx->gc, obj);
return 0;
}
@@ -508,13 +508,13 @@ static int json_callback_string(void *opaque, const
unsigned char *str,
strncpy(t, (const char *) str, len);
t[len] = 0;
- if ((obj = json_object_alloc(ctx->gc, JSON_STRING)) == NULL) {
+ if ((obj = libxl__json_object_alloc(ctx->gc, JSON_STRING)) == NULL) {
free(t);
return 0;
}
obj->u.string = t;
- if (json_object_append_to(ctx->gc, obj, ctx->current) == -1) {
+ if (libxl__json_object_append_to(ctx->gc, obj, ctx->current) == -1) {
libxl__json_object_free(ctx->gc, obj);
return 0;
}
@@ -573,11 +573,11 @@ static int json_callback_start_map(void *opaque)
DEBUG_GEN(ctx, map_open);
- if ((obj = json_object_alloc(ctx->gc, JSON_MAP)) == NULL)
+ if ((obj = libxl__json_object_alloc(ctx->gc, JSON_MAP)) == NULL)
return 0;
if (ctx->current) {
- if (json_object_append_to(ctx->gc, obj, ctx->current) == -1) {
+ if (libxl__json_object_append_to(ctx->gc, obj, ctx->current) ==
-1) {
libxl__json_object_free(ctx->gc, obj);
return 0;
}
@@ -615,11 +615,11 @@ static int json_callback_start_array(void *opaque)
DEBUG_GEN(ctx, array_open);
- if ((obj = json_object_alloc(ctx->gc, JSON_ARRAY)) == NULL)
+ if ((obj = libxl__json_object_alloc(ctx->gc, JSON_ARRAY)) == NULL)
return 0;
if (ctx->current) {
- if (json_object_append_to(ctx->gc, obj, ctx->current) == -1) {
+ if (libxl__json_object_append_to(ctx->gc, obj, ctx->current) ==
-1) {
libxl__json_object_free(ctx->gc, obj);
return 0;
}
--
Anthony PERARD
Anthony PERARD
2012-Jul-25 17:04 UTC
[PATCH 2/6] libxl_json: Introduce libxl__json_object_to_yajl_gen.
This function converts a libxl__json_object to yajl by calling every yajl_gen_*
function on a preallocated yajl_gen hand.
This helps to integrate a json_object into an already existing yajl_gen tree.
This function is used in a later patch.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
tools/libxl/libxl_internal.h | 3 ++
tools/libxl/libxl_json.c | 65 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 68 insertions(+), 0 deletions(-)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index c233416..2176778 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1478,6 +1478,9 @@ libxl__json_map_node *libxl__json_map_node_get(const
libxl__json_object *o,
_hidden const libxl__json_object *libxl__json_map_get(const char *key,
const libxl__json_object *o,
libxl__json_node_type expected_type);
+_hidden yajl_status libxl__json_object_to_yajl_gen(libxl__gc *gc,
+ yajl_gen hand,
+ libxl__json_object *param);
_hidden void libxl__json_object_free(libxl__gc *gc, libxl__json_object *obj);
_hidden libxl__json_object *libxl__json_parse(libxl__gc *gc, const char *s);
diff --git a/tools/libxl/libxl_json.c b/tools/libxl/libxl_json.c
index 9e302d5..a93d3ce 100644
--- a/tools/libxl/libxl_json.c
+++ b/tools/libxl/libxl_json.c
@@ -381,6 +381,71 @@ const libxl__json_object *libxl__json_map_get(const char
*key,
return NULL;
}
+yajl_status libxl__json_object_to_yajl_gen(libxl__gc *gc,
+ yajl_gen hand,
+ libxl__json_object *obj)
+{
+ int index = 0;
+ yajl_status rc;
+
+ if (obj == NULL)
+ return -1;
+ switch (obj->type) {
+ case JSON_NULL:
+ return yajl_gen_null(hand);
+ case JSON_TRUE:
+ return yajl_gen_bool(hand, true);
+ case JSON_FALSE:
+ return yajl_gen_bool(hand, false);
+ case JSON_INTEGER:
+ return yajl_gen_integer(hand, obj->u.i);
+ case JSON_DOUBLE:
+ return yajl_gen_double(hand, obj->u.d);
+ case JSON_NUMBER:
+ return yajl_gen_number(hand, obj->u.string,
strlen(obj->u.string));
+ case JSON_STRING:
+ return libxl__yajl_gen_asciiz(hand, obj->u.string);
+ case JSON_MAP: {
+ libxl__json_map_node *node = NULL;
+
+ rc = yajl_gen_map_open(hand);
+ if (rc != yajl_status_ok)
+ return rc;
+ for (index = 0; index < obj->u.map->count; index++) {
+ if (flexarray_get(obj->u.map, index, (void**)&node) != 0)
+ break;
+
+ rc = libxl__yajl_gen_asciiz(hand, node->map_key);
+ if (rc != yajl_status_ok)
+ return rc;
+ rc = libxl__json_object_to_yajl_gen(gc, hand, node->obj);
+ if (rc != yajl_status_ok)
+ return rc;
+ }
+ return yajl_gen_map_close(hand);
+ }
+ case JSON_ARRAY: {
+ libxl__json_object *node = NULL;
+
+ rc = yajl_gen_array_open(hand);
+ if (rc != yajl_status_ok)
+ return rc;
+ for (index = 0; index < obj->u.array->count; index++) {
+ if (flexarray_get(obj->u.array, index, (void**)&node) != 0)
+ break;
+ rc = libxl__json_object_to_yajl_gen(gc, hand, node);
+ if (rc != yajl_status_ok)
+ return rc;
+ }
+ return yajl_gen_array_close(hand);
+ }
+ case JSON_ERROR:
+ case JSON_ANY:
+ default:
+ return -1;
+ }
+}
+
/*
* JSON callbacks
--
Anthony PERARD
Anthony PERARD
2012-Jul-25 17:04 UTC
[PATCH 3/6] libxl_qmp: Introduces helpers to create an argument list.
Those functions will be used to create a "list" of parameters that
contain more
than just strings. This list is converted by qmp_send to a string to be sent to
QEMU.
Those functions will be used in the next two patches, so right now there are
not compiled.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
tools/libxl/libxl_qmp.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 86 insertions(+), 0 deletions(-)
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index e33b130..edbd3b4 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -623,6 +623,92 @@ static void qmp_free_handler(libxl__qmp_handler *qmp)
free(qmp);
}
+#if 0
+/*
+ * QMP Parameters Helpers
+ * If a function fail to allocate memomy for one more parameters, then
+ * the param is free, even if it''s come from a caller.
+ */
+static libxl__json_object *qmp_parameters_common(libxl__gc *gc,
+ libxl__json_object *param,
+ const char *name,
+ libxl__json_object *obj)
+{
+ // check that every caller of this free the tree !
+ libxl__json_map_node *arg = NULL;
+ if (!param) {
+ param = libxl__json_object_alloc(gc, JSON_MAP);
+ if (!param)
+ return NULL;
+ }
+ arg = calloc(1, sizeof (libxl__json_map_node));
+ if (!arg)
+ goto error_nomem;
+
+ if (flexarray_append(param->u.map, arg) == 2) {
+ free(arg);
+ goto error_nomem;
+ }
+
+ arg->map_key = strdup(name);
+ if (!arg->map_key)
+ goto error_nomem;
+
+ arg->obj = obj;
+
+ return param;
+error_nomem:
+ libxl__json_object_free(gc, param);
+ return NULL;
+}
+
+static libxl__json_object *qmp_parameters_add_string(libxl__gc *gc,
+ libxl__json_object *param,
+ const char *name,
+ const char *argument)
+{
+ libxl__json_object *obj;
+
+ obj = libxl__json_object_alloc(gc, JSON_STRING);
+ if (!obj)
+ goto error_nomem;
+ obj->u.string = strdup(argument);
+ if (!obj->u.string)
+ goto error_nomem;
+
+ param = qmp_parameters_common(gc, param, name, obj);
+ if (!param)
+ goto error_nomem;
+
+ return param;
+error_nomem:
+ libxl__json_object_free(gc, param);
+ libxl__json_object_free(gc, obj);
+ return NULL;
+}
+
+static libxl__json_object *qmp_parameters_add_bool(libxl__gc *gc,
+ libxl__json_object *param,
+ const char *name, bool b)
+{
+ libxl__json_object *obj;
+
+ obj = libxl__json_object_alloc(gc, JSON_TRUE);
+ if (!obj)
+ goto error_nomem;
+
+ param = qmp_parameters_common(gc, param, name, obj);
+ if (!param)
+ goto error_nomem;
+
+ return param;
+error_nomem:
+ libxl__json_object_free(gc, param);
+ libxl__json_object_free(gc, obj);
+ return NULL;
+}
+#endif
+
/*
* API
*/
--
Anthony PERARD
Anthony PERARD
2012-Jul-25 17:04 UTC
[PATCH 4/6] libxl_qmp: Use qmp_parameters_* functions for param list of a QMP command.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
tools/libxl/libxl_qmp.c | 100 +++++++++++++++++++++++-----------------------
1 files changed, 50 insertions(+), 50 deletions(-)
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index edbd3b4..8bd56cf 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -78,7 +78,7 @@ struct libxl__qmp_handler {
};
static int qmp_send(libxl__qmp_handler *qmp,
- const char *cmd, libxl_key_value_list *args,
+ const char *cmd, libxl__json_object *args,
qmp_callback_t callback, void *opaque,
qmp_request_context *context);
@@ -502,7 +502,7 @@ static int qmp_next(libxl__gc *gc, libxl__qmp_handler *qmp)
}
static char *qmp_send_prepare(libxl__gc *gc, libxl__qmp_handler *qmp,
- const char *cmd, libxl_key_value_list *args,
+ const char *cmd, libxl__json_object *args,
qmp_callback_t callback, void *opaque,
qmp_request_context *context)
{
@@ -526,7 +526,7 @@ static char *qmp_send_prepare(libxl__gc *gc,
libxl__qmp_handler *qmp,
yajl_gen_integer(hand, ++qmp->last_id_used);
if (args) {
libxl__yajl_gen_asciiz(hand, "arguments");
- libxl_key_value_list_gen_json(hand, args);
+ libxl__json_object_to_yajl_gen(gc, hand, args);
}
yajl_gen_map_close(hand);
@@ -560,7 +560,7 @@ out:
}
static int qmp_send(libxl__qmp_handler *qmp,
- const char *cmd, libxl_key_value_list *args,
+ const char *cmd, libxl__json_object *args,
qmp_callback_t callback, void *opaque,
qmp_request_context *context)
{
@@ -588,7 +588,7 @@ out:
}
static int qmp_synchronous_send(libxl__qmp_handler *qmp, const char *cmd,
- libxl_key_value_list *args,
+ libxl__json_object *args,
qmp_callback_t callback, void *opaque,
int ask_timeout)
{
@@ -623,7 +623,6 @@ static void qmp_free_handler(libxl__qmp_handler *qmp)
free(qmp);
}
-#if 0
/*
* QMP Parameters Helpers
* If a function fail to allocate memomy for one more parameters, then
@@ -687,6 +686,7 @@ error_nomem:
return NULL;
}
+#if 0
static libxl__json_object *qmp_parameters_add_bool(libxl__gc *gc,
libxl__json_object *param,
const char *name, bool b)
@@ -835,8 +835,7 @@ out:
int libxl__qmp_pci_add(libxl__gc *gc, int domid, libxl_device_pci *pcidev)
{
libxl__qmp_handler *qmp = NULL;
- flexarray_t *parameters = NULL;
- libxl_key_value_list args = NULL;
+ libxl__json_object *args = NULL;
char *hostaddr = NULL;
int rc = 0;
@@ -849,56 +848,63 @@ int libxl__qmp_pci_add(libxl__gc *gc, int domid,
libxl_device_pci *pcidev)
if (!hostaddr)
return -1;
- parameters = flexarray_make(6, 1);
- flexarray_append_pair(parameters, "driver",
"xen-pci-passthrough");
- flexarray_append_pair(parameters, "id",
+ args = qmp_parameters_add_string(gc, NULL, "driver",
"xen-pci-passthrough");
+ if (!args)
+ goto error_nomem;
+ args = qmp_parameters_add_string(gc, args, "id",
libxl__sprintf(gc, PCI_PT_QDEV_ID,
pcidev->bus, pcidev->dev,
pcidev->func));
- flexarray_append_pair(parameters, "hostaddr", hostaddr);
+ if (!args)
+ goto error_nomem;
+ args = qmp_parameters_add_string(gc, args, "hostaddr", hostaddr);
+ if (!args)
+ goto error_nomem;
if (pcidev->vdevfn) {
- flexarray_append_pair(parameters, "addr",
+ args = qmp_parameters_add_string(gc, args, "addr",
libxl__sprintf(gc, "%x.%x",
PCI_SLOT(pcidev->vdevfn),
PCI_FUNC(pcidev->vdevfn)));
+ if (!args)
+ goto error_nomem;
}
- args = libxl__xs_kvs_of_flexarray(gc, parameters, parameters->count);
- if (!args)
- return -1;
- rc = qmp_synchronous_send(qmp, "device_add", &args,
+ rc = qmp_synchronous_send(qmp, "device_add", args,
NULL, NULL, qmp->timeout);
if (rc == 0) {
rc = qmp_synchronous_send(qmp, "query-pci", NULL,
pci_add_callback, pcidev, qmp->timeout);
}
- flexarray_free(parameters);
+ libxl__json_object_free(gc, args);
libxl__qmp_close(qmp);
return rc;
+error_nomem:
+ libxl__qmp_close(qmp);
+ return ERROR_NOMEM;
}
static int qmp_device_del(libxl__gc *gc, int domid, char *id)
{
libxl__qmp_handler *qmp = NULL;
- flexarray_t *parameters = NULL;
- libxl_key_value_list args = NULL;
+ libxl__json_object *args = NULL;
int rc = 0;
qmp = libxl__qmp_initialize(gc, domid);
if (!qmp)
return ERROR_FAIL;
- parameters = flexarray_make(2, 1);
- flexarray_append_pair(parameters, "id", id);
- args = libxl__xs_kvs_of_flexarray(gc, parameters, parameters->count);
- if (!args)
- return ERROR_NOMEM;
+ args = qmp_parameters_add_string(gc, NULL, "id", id);
+ if (!args) {
+ rc = ERROR_NOMEM;
+ goto out;
+ }
- rc = qmp_synchronous_send(qmp, "device_del", &args,
+ rc = qmp_synchronous_send(qmp, "device_del", args,
NULL, NULL, qmp->timeout);
+ libxl__json_object_free(gc, args);
- flexarray_free(parameters);
+out:
libxl__qmp_close(qmp);
return rc;
}
@@ -916,31 +922,23 @@ int libxl__qmp_pci_del(libxl__gc *gc, int domid,
libxl_device_pci *pcidev)
int libxl__qmp_save(libxl__gc *gc, int domid, const char *filename)
{
libxl__qmp_handler *qmp = NULL;
- flexarray_t *parameters = NULL;
- libxl_key_value_list args = NULL;
+ libxl__json_object *args = NULL;
int rc = 0;
qmp = libxl__qmp_initialize(gc, domid);
if (!qmp)
return ERROR_FAIL;
- parameters = flexarray_make(2, 1);
- if (!parameters) {
- rc = ERROR_NOMEM;
- goto out;
- }
- flexarray_append_pair(parameters, "filename", (char *)filename);
- args = libxl__xs_kvs_of_flexarray(gc, parameters, parameters->count);
+ args = qmp_parameters_add_string(gc, NULL, "filename", (char
*)filename);
if (!args) {
rc = ERROR_NOMEM;
- goto out2;
+ goto out;
}
- rc = qmp_synchronous_send(qmp, "xen-save-devices-state",
&args,
+ rc = qmp_synchronous_send(qmp, "xen-save-devices-state", args,
NULL, NULL, qmp->timeout);
+ libxl__json_object_free(gc, args);
-out2:
- flexarray_free(parameters);
out:
libxl__qmp_close(qmp);
return rc;
@@ -949,23 +947,25 @@ out:
static int qmp_change(libxl__gc *gc, libxl__qmp_handler *qmp,
char *device, char *target, char *arg)
{
- flexarray_t *parameters = NULL;
- libxl_key_value_list args = NULL;
+ libxl__json_object *args = NULL;
int rc = 0;
- parameters = flexarray_make(6, 1);
- flexarray_append_pair(parameters, "device", device);
- flexarray_append_pair(parameters, "target", target);
- if (arg)
- flexarray_append_pair(parameters, "arg", arg);
- args = libxl__xs_kvs_of_flexarray(gc, parameters, parameters->count);
+ args = qmp_parameters_add_string(gc, NULL, "device", device);
if (!args)
return ERROR_NOMEM;
+ args = qmp_parameters_add_string(gc, args, "target", target);
+ if (!args)
+ return ERROR_NOMEM;
+ if (arg) {
+ args = qmp_parameters_add_string(gc, args, "arg", arg);
+ if (!args)
+ return ERROR_NOMEM;
+ }
- rc = qmp_synchronous_send(qmp, "change", &args,
+ rc = qmp_synchronous_send(qmp, "change", args,
NULL, NULL, qmp->timeout);
+ libxl__json_object_free(gc, args);
- flexarray_free(parameters);
return rc;
}
--
Anthony PERARD
Anthony PERARD
2012-Jul-25 17:04 UTC
[PATCH 5/6] libxl_qmp: Introduce libxl__qmp_set_global_dirty_log.
This function will enable or disable the global dirty log on QEMU, used during
a migration.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
tools/libxl/libxl_internal.h | 2 ++
tools/libxl/libxl_qmp.c | 27 +++++++++++++++++++++++++--
2 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 2176778..5af4f5c 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1353,6 +1353,8 @@ _hidden int libxl__qmp_stop(libxl__gc *gc, int domid);
_hidden int libxl__qmp_resume(libxl__gc *gc, int domid);
/* Save current QEMU state into fd. */
_hidden int libxl__qmp_save(libxl__gc *gc, int domid, const char *filename);
+/* Set dirty bitmap logging status */
+_hidden int libxl__qmp_set_global_dirty_log(libxl__gc *gc, int domid, bool
enable);
/* close and free the QMP handler */
_hidden void libxl__qmp_close(libxl__qmp_handler *qmp);
/* remove the socket file, if the file has already been removed,
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index 8bd56cf..2c5559f 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -686,7 +686,6 @@ error_nomem:
return NULL;
}
-#if 0
static libxl__json_object *qmp_parameters_add_bool(libxl__gc *gc,
libxl__json_object *param,
const char *name, bool b)
@@ -707,7 +706,6 @@ error_nomem:
libxl__json_object_free(gc, obj);
return NULL;
}
-#endif
/*
* API
@@ -1001,6 +999,31 @@ int libxl__qmp_resume(libxl__gc *gc, int domid)
return rc;
}
+int libxl__qmp_set_global_dirty_log(libxl__gc *gc, int domid, bool enable)
+{
+ libxl__qmp_handler *qmp = NULL;
+ libxl__json_object *args = NULL;
+ int rc = 0;
+
+ qmp = libxl__qmp_initialize(gc, domid);
+ if (!qmp)
+ return ERROR_FAIL;
+
+ args = qmp_parameters_add_bool(gc, NULL, "enable", enable);
+ if (!args) {
+ rc = ERROR_NOMEM;
+ goto out;
+ }
+
+ rc = qmp_synchronous_send(qmp, "xen-set-global-dirty-log", args,
+ NULL, NULL, qmp->timeout);
+ libxl__json_object_free(gc, args);
+
+out:
+ libxl__qmp_close(qmp);
+ return rc;
+}
+
int libxl__qmp_initializations(libxl__gc *gc, uint32_t domid,
const libxl_domain_config *guest_config)
{
--
Anthony PERARD
Anthony PERARD
2012-Jul-25 17:04 UTC
[PATCH 6/6] libxl_dom: Call the right switch logdirty for the right DM.
This patch dispatch the switch logdirty call depending on which device model
version is running.
The call to qemu-xen is synchrone, not like the one to qemu-xen-traditional.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
tools/libxl/libxl_dom.c | 41 ++++++++++++++++++++++++++++++++++++++++-
1 files changed, 40 insertions(+), 1 deletions(-)
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index cecda9b..ee7dc62 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -561,7 +561,7 @@ static void logdirty_init(libxl__logdirty_switch *lds)
libxl__ev_time_init(&lds->timeout);
}
-void libxl__domain_suspend_common_switch_qemu_logdirty
+static void domain_suspend_switch_qemu_xen_traditional_logdirty
(int domid, unsigned enable, void *user)
{
libxl__save_helper_state *shs = user;
@@ -631,6 +631,45 @@ void libxl__domain_suspend_common_switch_qemu_logdirty
switch_logdirty_done(egc,dss,-1);
}
+static void domain_suspend_switch_qemu_xen_logdirty
+ (int domid, unsigned enable, void *user)
+{
+ libxl__save_helper_state *shs = user;
+ libxl__egc *egc = shs->egc;
+ libxl__domain_suspend_state *dss = CONTAINER_OF(shs, *dss, shs);
+ STATE_AO_GC(dss->ao);
+ int rc;
+
+ rc = libxl__qmp_set_global_dirty_log(gc, domid, enable);
+ if (!rc) {
+ libxl__xc_domain_saverestore_async_callback_done(egc, shs, 0);
+ } else {
+ LOG(ERROR,"logdirty switch failed (rc=%d), aborting
suspend",rc);
+ libxl__xc_domain_saverestore_async_callback_done(egc, shs, -1);
+ }
+}
+
+void libxl__domain_suspend_common_switch_qemu_logdirty
+ (int domid, unsigned enable, void *user)
+{
+ libxl__save_helper_state *shs = user;
+ libxl__egc *egc = shs->egc;
+ libxl__domain_suspend_state *dss = CONTAINER_OF(shs, *dss, shs);
+ STATE_AO_GC(dss->ao);
+
+ switch (libxl__device_model_version_running(gc, domid)) {
+ case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
+ domain_suspend_switch_qemu_xen_traditional_logdirty(domid, enable,
user);
+ break;
+ case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
+ domain_suspend_switch_qemu_xen_logdirty(domid, enable, user);
+ break;
+ default:
+ LOG(ERROR,"logdirty switch failed"
+ ", no valid device model version found, aborting
suspend");
+ libxl__xc_domain_saverestore_async_callback_done(egc, shs, -1);
+ }
+}
static void switch_logdirty_timeout(libxl__egc *egc, libxl__ev_time *ev,
const struct timeval *requested_abs)
{
--
Anthony PERARD
Ian Jackson
2012-Jul-31 12:50 UTC
Re: [PATCH 1/6] libxl_json: Export json_object related function.
Anthony PERARD writes ("[PATCH 1/6] libxl_json: Export json_object related
function."):> Export libxl__json_object_alloc and libxl__json_object_append_to to
> use them in a later patch.
Now that we have decided that allocation failures will be fatal, this
function should perhaps be fixed not to use the libxl allocation
helpers with NOGC, and its call sites changed to assume success ?
Ian.
Ian Jackson
2012-Jul-31 14:19 UTC
Re: [PATCH 2/6] libxl_json: Introduce libxl__json_object_to_yajl_gen.
Anthony PERARD writes ("[PATCH 2/6] libxl_json: Introduce
libxl__json_object_to_yajl_gen."):> This function converts a libxl__json_object to yajl by calling every
> yajl_gen_* function on a preallocated yajl_gen hand.
I''m slightly concerned at the amount of boilerplateish code
we''re
introducing here. Forgive my utter ignorance but is there not some
more automatic way of handling all this ?
At least it looks reasonably generic so if there is no better way to
do it then fine.
I have some more detailed comments:
...> +yajl_status libxl__json_object_to_yajl_gen(libxl__gc *gc,
> + yajl_gen hand,
> + libxl__json_object *obj)
> +{
> + int index = 0;
> + yajl_status rc;
> +
> + if (obj == NULL)
> + return -1;
Is this an expected situation ? Surely we should crash if we pass
NULL rather than silently inventing an error code ?
> + default:
> + return -1;
When might this occur ?
Ian.
Ian Jackson
2012-Jul-31 14:27 UTC
Re: [PATCH 3/6] libxl_qmp: Introduces helpers to create an argument list.
Anthony PERARD writes ("[PATCH 3/6] libxl_qmp: Introduces helpers to create
an argument list."):> diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
> index e33b130..edbd3b4 100644
> --- a/tools/libxl/libxl_qmp.c
> +++ b/tools/libxl/libxl_qmp.c
> @@ -623,6 +623,92 @@ static void qmp_free_handler(libxl__qmp_handler *qmp)
> free(qmp);
> }
>
> +#if 0
> +/*
> + * QMP Parameters Helpers
> + * If a function fail to allocate memomy for one more parameters, then
> + * the param is free, even if it''s come from a caller.
Allocation failures are supposed to be fatal nowadays, so this case
doesn''t need to exit.
> +static libxl__json_object *qmp_parameters_common(libxl__gc *gc,
> + libxl__json_object
*param,
> + const char *name,
> + libxl__json_object *obj)
> +{
> + // check that every caller of this free the tree !
We use /* ... */. I''m not sure what the purpose of this comment is.
Do you mean "this is a fixme" ? Or is that intended to be a doc
comment saying that callers end up owning some object ? (The return
value?) Why is this object not from the gc ?
> + arg = calloc(1, sizeof (libxl__json_map_node));
These allcations should all use the appropriate libxl helpers,
probably. (Throughout all your patches.)
I''m not sure exactly what this function _does_, though. It prepares
some "common" parameters for a qmp call, evidently, but what common
parameters ?
OTOH the code seems to add a single parameter to the map ?
> +static libxl__json_object *qmp_parameters_add_string(libxl__gc *gc,
> + libxl__json_object
*param,
> + const char *name,
> + const char *argument)
> +{
> + libxl__json_object *obj;
> +
> + obj = libxl__json_object_alloc(gc, JSON_STRING);
> + if (!obj)
> + goto error_nomem;
> + obj->u.string = strdup(argument);
> + if (!obj->u.string)
> + goto error_nomem;
> +
> + param = qmp_parameters_common(gc, param, name, obj);
> + if (!param)
> + goto error_nomem;
> +
> + return param;
> +error_nomem:
> + libxl__json_object_free(gc, param);
> + libxl__json_object_free(gc, obj);
> + return NULL;
> +}
This is very similar to the same function for bool. Perhaps it would
be better to separate out the type-specific object creation ?
Ian.
Ian Jackson
2012-Jul-31 14:36 UTC
Re: [PATCH 4/6] libxl_qmp: Use qmp_parameters_* functions for param list of a QMP command.
Anthony PERARD writes ("[PATCH 4/6] libxl_qmp: Use qmp_parameters_*
functions for param list of a QMP command."):> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Thanks:
> - parameters = flexarray_make(6, 1);
> - flexarray_append_pair(parameters, "driver",
"xen-pci-passthrough");
> - flexarray_append_pair(parameters, "id",
> + args = qmp_parameters_add_string(gc, NULL, "driver",
"xen-pci-passthrough");
> + if (!args)
> + goto error_nomem;
This is a rather clumsy use pattern, threading args through all the
time. If nothing else it might lead to mistakes because the first
call takes NULL whereas subsequent calls take args:
> + args = qmp_parameters_add_string(gc, args, "id",
> libxl__sprintf(gc, PCI_PT_QDEV_ID,
> pcidev->bus, pcidev->dev,
> pcidev->func));
How about inventing functions that would allow
libxl__json_object *args = NULL;
libxl__qmp_param_add(gc, &args, "id", libxl__qmp_sprintf(gc,
....);
with perhaps a helper macro that allows
QMP_PARAM_SPRINTF(args, "id", PCI_PT_QDEV_ID,
pcidev->bus, pcidev->dev, pcidev->func));
?
And again there''s a lot of unnecessary error handling.
Ian.
Ian Jackson
2012-Jul-31 14:37 UTC
Re: [PATCH 5/6] libxl_qmp: Introduce libxl__qmp_set_global_dirty_log.
Anthony PERARD writes ("[PATCH 5/6] libxl_qmp: Introduce
libxl__qmp_set_global_dirty_log."):> This function will enable or disable the global dirty log on QEMU, used
during
> a migration.
Again we have the possible concurrent use of the qmp to contend with.
Also:
> +int libxl__qmp_set_global_dirty_log(libxl__gc *gc, int domid, bool enable)
> +{
> + libxl__qmp_handler *qmp = NULL;
> + libxl__json_object *args = NULL;
> + int rc = 0;
> +
> + qmp = libxl__qmp_initialize(gc, domid);
> + if (!qmp)
> + return ERROR_FAIL;
> +
> + args = qmp_parameters_add_bool(gc, NULL, "enable", enable);
> + if (!args) {
> + rc = ERROR_NOMEM;
> + goto out;
> + }
> +
> + rc = qmp_synchronous_send(qmp, "xen-set-global-dirty-log",
args,
> + NULL, NULL, qmp->timeout);
> + libxl__json_object_free(gc, args);
> +
> +out:
> + libxl__qmp_close(qmp);
> + return rc;
> +}
This pattern which is starting to appear multiple times could perhaps
be encapsulated in a function ?
Ian.
Ian Jackson
2012-Jul-31 14:40 UTC
Re: [PATCH 6/6] libxl_dom: Call the right switch logdirty for the right DM.
Anthony PERARD writes ("[PATCH 6/6] libxl_dom: Call the right switch
logdirty for the right DM."):> This patch dispatch the switch logdirty call depending on which device
model
> version is running.
...> -void libxl__domain_suspend_common_switch_qemu_logdirty
> +static void domain_suspend_switch_qemu_xen_traditional_logdirty
> (int domid, unsigned enable, void *user)
If you make these functions internal you should not have them each
separately upcast the void*. They should take the shs as a typed
pointer.
Apart from that it looks sane.
Thanks,
Ian.