* [PATCH v8 01/17] libxl: Enhance libxl__sendmsg_fds to deal with EINTR and EWOULDBLOCK
2019-01-04 15:30 [PATCH v8 00/17] libxl: Enable save/restore/migration of a restricted QEMU + libxl__ev_qmp_* Anthony PERARD
@ 2019-01-04 15:30 ` Anthony PERARD
2019-01-04 15:30 ` [PATCH v8 02/17] libxl_qmp: Separate QMP message generation from qmp_send_prepare Anthony PERARD
` (15 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: Anthony PERARD @ 2019-01-04 15:30 UTC (permalink / raw)
To: xen-devel; +Cc: Anthony PERARD, Ian Jackson, Wei Liu
This patch change the behavior of libxl__sendmsg_fds to retry sendmsg on
EINTR error and return an error on short writes.
This patch allow a caller of libxl__sendmsg_fds to deal with EWOULDBLOCK
and short writes. The function now requires to send only 1 byte of data
so that when dealing with non-blocking fds a EWOULDBLOCK error would
mean that the fds haven't been sent yet. Current caller already send
only 1 byte.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
Notes:
v8:
Acked, but:
- there is a follow-up patch to get rid of datalen param.
- commit message have been modified a bit to talk about short
write in general.
v7:
always assert datalen == 1, but only fail when sendmsg haven't send
everything (r != datalen)
check sendmsg return value on success as well (check for short write)
tools/libxl/libxl_internal.h | 5 ++++-
tools/libxl/libxl_utils.c | 25 ++++++++++++++++++++-----
2 files changed, 24 insertions(+), 6 deletions(-)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 67f3f6ac76..aa9059ffec 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1876,7 +1876,10 @@ _hidden void libxl__qmp_cleanup(libxl__gc *gc, uint32_t domid);
_hidden int libxl__qmp_initializations(libxl__gc *gc, uint32_t domid,
const libxl_domain_config *guest_config);
-/* on failure, logs */
+/* `datalen` should be 1 byte
+ * When dealing with a non-blocking fd, it returns
+ * ERROR_NOT_READY on EWOULDBLOCK
+ * logs on other failures. */
int libxl__sendmsg_fds(libxl__gc *gc, int carrier,
const void *data, size_t datalen,
int nfds, const int fds[], const char *what);
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 5854717b11..d53db8c37d 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -1070,6 +1070,8 @@ int libxl__sendmsg_fds(libxl__gc *gc, int carrier,
struct iovec iov;
int r;
+ assert(datalen == 1);
+
iov.iov_base = (void*)data;
iov.iov_len = datalen;
@@ -1088,11 +1090,24 @@ int libxl__sendmsg_fds(libxl__gc *gc, int carrier,
msg.msg_controllen = cmsg->cmsg_len;
- r = sendmsg(carrier, &msg, 0);
- if (r < 0) {
- LOGE(ERROR, "failed to send fd-carrying message (%s)", what);
- return ERROR_FAIL;
- }
+ while (1) {
+ r = sendmsg(carrier, &msg, 0);
+ if (r < 0) {
+ if (errno == EINTR)
+ continue;
+ if (errno == EWOULDBLOCK) {
+ return ERROR_NOT_READY;
+ }
+ LOGE(ERROR, "failed to send fd-carrying message (%s)", what);
+ return ERROR_FAIL;
+ }
+ if (r != datalen) {
+ LOG(ERROR, "sendmsg have written %d instead of %ld",
+ r, datalen);
+ return ERROR_FAIL;
+ }
+ break;
+ };
return 0;
}
--
Anthony PERARD
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v8 02/17] libxl_qmp: Separate QMP message generation from qmp_send_prepare
2019-01-04 15:30 [PATCH v8 00/17] libxl: Enable save/restore/migration of a restricted QEMU + libxl__ev_qmp_* Anthony PERARD
2019-01-04 15:30 ` [PATCH v8 01/17] libxl: Enhance libxl__sendmsg_fds to deal with EINTR and EWOULDBLOCK Anthony PERARD
@ 2019-01-04 15:30 ` Anthony PERARD
2019-01-04 15:30 ` [PATCH v8 03/17] libxl_qmp: Change qmp_qemu_check_version to compare version Anthony PERARD
` (14 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: Anthony PERARD @ 2019-01-04 15:30 UTC (permalink / raw)
To: xen-devel; +Cc: Anthony PERARD, Ian Jackson, Wei Liu
.. to be able to re-use qmp_prepare_cmd with libxl__ev_qmp.
This patch also add the QMP end of command '\r\n' into the generated
string as every caller will needs this.
There should be no functional change.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
Notes:
v8:
Acked
v7:
got rid of len_r in qmp_prepare_cmd parameters
callers will need to call strlen.
The returned value (`ret`) is now allocated within `gc` (instead of
NOGC)
v6:
comment about ownership of buf
use lib__sprintf instead of two strncpy
v5:
rename qmp_prepare_qmp_cmd to qmp_prepare_cmd
fix coding style
tools/libxl/libxl_qmp.c | 49 ++++++++++++++++++++++++++---------------
1 file changed, 31 insertions(+), 18 deletions(-)
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index 6a5c997546..45a2cc423b 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -571,17 +571,16 @@ static int qmp_next(libxl__gc *gc, libxl__qmp_handler *qmp)
return rc;
}
-static char *qmp_send_prepare(libxl__gc *gc, libxl__qmp_handler *qmp,
- const char *cmd, libxl__json_object *args,
- qmp_callback_t callback, void *opaque,
- qmp_request_context *context)
+static char *qmp_prepare_cmd(libxl__gc *gc, const char *cmd,
+ const libxl__json_object *args,
+ int id)
{
- const unsigned char *buf = NULL;
- char *ret = NULL;
- libxl_yajl_length len = 0;
+ yajl_gen hand = NULL;
+ /* memory for 'buf' is owned by 'hand' */
+ const unsigned char *buf;
+ libxl_yajl_length len;
yajl_gen_status s;
- yajl_gen hand;
- callback_id_pair *elm = NULL;
+ char *ret = NULL;
hand = libxl_yajl_gen_alloc(NULL);
@@ -598,7 +597,7 @@ static char *qmp_send_prepare(libxl__gc *gc, libxl__qmp_handler *qmp,
libxl__yajl_gen_asciiz(hand, "execute");
libxl__yajl_gen_asciiz(hand, cmd);
libxl__yajl_gen_asciiz(hand, "id");
- yajl_gen_integer(hand, ++qmp->last_id_used);
+ yajl_gen_integer(hand, id);
if (args) {
libxl__yajl_gen_asciiz(hand, "arguments");
libxl__json_object_to_yajl_gen(gc, hand, args);
@@ -607,7 +606,27 @@ static char *qmp_send_prepare(libxl__gc *gc, libxl__qmp_handler *qmp,
s = yajl_gen_get_buf(hand, &buf, &len);
- if (s) {
+ if (s != yajl_gen_status_ok)
+ goto out;
+
+ ret = libxl__sprintf(gc, "%*.*s\r\n", (int)len, (int)len, buf);
+
+out:
+ yajl_gen_free(hand);
+ return ret;
+}
+
+static char *qmp_send_prepare(libxl__gc *gc, libxl__qmp_handler *qmp,
+ const char *cmd, libxl__json_object *args,
+ qmp_callback_t callback, void *opaque,
+ qmp_request_context *context)
+{
+ char *buf;
+ callback_id_pair *elm;
+
+ buf = qmp_prepare_cmd(gc, cmd, args, ++qmp->last_id_used);
+
+ if (!buf) {
LOGD(ERROR, qmp->domid, "Failed to generate a qmp command");
goto out;
}
@@ -623,13 +642,10 @@ static char *qmp_send_prepare(libxl__gc *gc, libxl__qmp_handler *qmp,
elm->context = context;
LIBXL_STAILQ_INSERT_TAIL(&qmp->callback_list, elm, next);
- ret = libxl__strndup(gc, (const char*)buf, len);
-
LOGD(DEBUG, qmp->domid, "next qmp command: '%s'", buf);
out:
- yajl_gen_free(hand);
- return ret;
+ return buf;
}
static int qmp_send(libxl__qmp_handler *qmp,
@@ -650,9 +666,6 @@ static int qmp_send(libxl__qmp_handler *qmp,
if (libxl_write_exactly(qmp->ctx, qmp->qmp_fd, buf, strlen(buf),
"QMP command", "QMP socket"))
goto out;
- if (libxl_write_exactly(qmp->ctx, qmp->qmp_fd, "\r\n", 2,
- "CRLF", "QMP socket"))
- goto out;
rc = qmp->last_id_used;
out:
--
Anthony PERARD
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v8 03/17] libxl_qmp: Change qmp_qemu_check_version to compare version
2019-01-04 15:30 [PATCH v8 00/17] libxl: Enable save/restore/migration of a restricted QEMU + libxl__ev_qmp_* Anthony PERARD
2019-01-04 15:30 ` [PATCH v8 01/17] libxl: Enhance libxl__sendmsg_fds to deal with EINTR and EWOULDBLOCK Anthony PERARD
2019-01-04 15:30 ` [PATCH v8 02/17] libxl_qmp: Separate QMP message generation from qmp_send_prepare Anthony PERARD
@ 2019-01-04 15:30 ` Anthony PERARD
2019-01-04 15:30 ` [PATCH v8 04/17] libxl: Add wrapper around libxl__json_object_to_json JSON Anthony PERARD
` (13 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: Anthony PERARD @ 2019-01-04 15:30 UTC (permalink / raw)
To: xen-devel; +Cc: Anthony PERARD, Ian Jackson, Wei Liu
This patch makes the function simpler to read. It also add the ability
for a caller to tell if QEMU is newer or have the exact version.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
Notes:
v6:
new patch
tools/libxl/libxl_qmp.c | 28 +++++++++++++++++++++-------
1 file changed, 21 insertions(+), 7 deletions(-)
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index 45a2cc423b..73f2202b4f 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -392,13 +392,27 @@ static int qmp_handle_response(libxl__gc *gc, libxl__qmp_handler *qmp,
return 0;
}
-static bool qmp_qemu_check_version(libxl__qmp_handler *qmp, int major,
- int minor, int micro)
+/*
+ * return values:
+ * < 0 if qemu's version < asked version
+ * = 0 if qemu's version == asked version
+ * > 0 if qemu's version > asked version
+ */
+static int qmp_qemu_compare_version(libxl__qmp_handler *qmp, int major,
+ int minor, int micro)
{
- return qmp->version.major > major ||
- (qmp->version.major == major &&
- (qmp->version.minor > minor ||
- (qmp->version.minor == minor && qmp->version.micro >= micro)));
+#define CHECK_VERSION(level) do { \
+ if (qmp->version.level > (level)) return +1; \
+ if (qmp->version.level < (level)) return -1; \
+} while (0)
+
+ CHECK_VERSION(major);
+ CHECK_VERSION(minor);
+ CHECK_VERSION(micro);
+
+#undef CHECK_VERSION
+
+ return 0;
}
/*
@@ -1012,7 +1026,7 @@ int libxl__qmp_save(libxl__gc *gc, int domid, const char *filename, bool live)
/* live parameter was added to QEMU 2.11. It signal QEMU that the save
* operation is for a live migration rather that for taking a snapshot. */
- if (qmp_qemu_check_version(qmp, 2, 11, 0))
+ if (qmp_qemu_compare_version(qmp, 2, 11, 0) >= 0)
qmp_parameters_add_bool(gc, &args, "live", live);
rc = qmp_synchronous_send(qmp, "xen-save-devices-state", args,
--
Anthony PERARD
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v8 04/17] libxl: Add wrapper around libxl__json_object_to_json JSON
2019-01-04 15:30 [PATCH v8 00/17] libxl: Enable save/restore/migration of a restricted QEMU + libxl__ev_qmp_* Anthony PERARD
` (2 preceding siblings ...)
2019-01-04 15:30 ` [PATCH v8 03/17] libxl_qmp: Change qmp_qemu_check_version to compare version Anthony PERARD
@ 2019-01-04 15:30 ` Anthony PERARD
2019-01-04 15:30 ` [PATCH v8 05/17] libxl: Design of an async API to issue QMP commands to QEMU Anthony PERARD
` (12 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: Anthony PERARD @ 2019-01-04 15:30 UTC (permalink / raw)
To: xen-devel; +Cc: Anthony PERARD, Ian Jackson, Wei Liu
That wrapper is going to be used to safely log a json_object, as
libxl__json_object_to_json return NULL on error. In the error case,
JSON() will return an invalid json string.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
Notes:
v8:
Acked
v7:
new patch
There are no user yet because the first users is going to be in
"libxl_qmp: Implementation of libxl__ev_qmp_*" which is already a
huge patch, that 3 lines might get lost.
tools/libxl/libxl_internal.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index aa9059ffec..16e6eb6c5e 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2046,6 +2046,9 @@ _hidden libxl__json_object *libxl__json_parse(libxl__gc *gc_opt, const char *s);
_hidden char *libxl__json_object_to_json(libxl__gc *gc,
const libxl__json_object *args);
+/* Always return a valid string, but invalid json on error. */
+#define JSON(o) \
+ (libxl__json_object_to_json(gc, (o)) ? : "<invalid-json-object>")
/* Based on /local/domain/$domid/dm-version xenstore key
* default is qemu xen traditional */
--
Anthony PERARD
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v8 05/17] libxl: Design of an async API to issue QMP commands to QEMU
2019-01-04 15:30 [PATCH v8 00/17] libxl: Enable save/restore/migration of a restricted QEMU + libxl__ev_qmp_* Anthony PERARD
` (3 preceding siblings ...)
2019-01-04 15:30 ` [PATCH v8 04/17] libxl: Add wrapper around libxl__json_object_to_json JSON Anthony PERARD
@ 2019-01-04 15:30 ` Anthony PERARD
2019-01-04 15:30 ` [PATCH v8 06/17] libxl_qmp: Implementation of libxl__ev_qmp_* Anthony PERARD
` (11 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: Anthony PERARD @ 2019-01-04 15:30 UTC (permalink / raw)
To: xen-devel; +Cc: Anthony PERARD, Ian Jackson, Wei Liu
All the functions will be implemented in later patches.
This patch includes the API that libxl can use to send QMP commands to
QEMU.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
Notes:
v7:
acked, but with:
fd field renamed to payload_fd
libxl__ao ao field added
added to _send that callback isn't called synchronously.
v6:
use libxl_domid type for domid instead of plain uin32_t
avoid the work "chained", rewrite the paragraph about sending one
cmd after another
Rewrite the comment about the callback, and explain that on error,
the `ev` may be Idle or may still be Connected
Change the carefd to a simple int (field cfd -> fd)
v5:
some changes in the comment
tools/libxl/libxl_internal.h | 76 +++++++++++++++++++++++++++++++++++-
1 file changed, 74 insertions(+), 2 deletions(-)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 16e6eb6c5e..fadafa5f11 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -192,6 +192,8 @@ 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__json_object libxl__json_object;
+typedef struct libxl__carefd libxl__carefd;
typedef struct libxl__domain_create_state libxl__domain_create_state;
typedef void libxl__domain_create_cb(struct libxl__egc *egc,
@@ -355,6 +357,76 @@ struct libxl__ev_child {
LIBXL_LIST_ENTRY(struct libxl__ev_child) entry;
};
+/*
+ * QMP asynchronous calls
+ *
+ * This facility allows a command to be sent to QEMU, and the response
+ * to be handed to a callback function.
+ *
+ * Commands can be submited one after an other with the same
+ * connection (e.g. the result from the "add-fd" command need to be
+ * use in a follow-up command before disconnecting from QMP). A
+ * libxl__ev_qmp can be reused when the callback is been called in
+ * order to use the same connection.
+ *
+ * Only one connection at a time can be made to one QEMU, so avoid
+ * keeping a libxl__ev_qmp Connected for to long and call
+ * libxl__ev_qmp_dispose as soon as it is not needed anymore.
+ *
+ * Possible states of a libxl__ev_qmp:
+ * Undefined
+ * Might contain anything.
+ * Idle
+ * Struct contents are defined enough to pass to any
+ * libxl__ev_qmp_* function.
+ * The struct does not contain references to any allocated private
+ * resources so can be thrown away.
+ * Active
+ * Currently waiting for the callback to be called.
+ * _dispose must be called to reclaim resources.
+ * Connected
+ * Struct contain allocated ressources.
+ * Calling _send() with this same ev will use the same QMP connection.
+ * _dispose() must be called to reclaim resources.
+ *
+ * libxl__ev_qmp_init: Undefined/Idle -> Idle
+ *
+ * libxl__ev_qmp_send: Idle/Connected -> Active (on error: Idle)
+ * Sends a command to QEMU.
+ * callback will be called when a response is received or when an
+ * error as occured.
+ * callback isn't called synchronously.
+ *
+ * libxl__ev_qmp_dispose: Connected/Active/Idle -> Idle
+ *
+ * callback: When called: Active -> Connected (on error: Idle/Connected)
+ * When called, ev is Connected and can be reused or disposed of.
+ * On error, the callback is called with response == NULL and the
+ * error code in rc. The new state of ev depending on the value of rc:
+ * - rc == ERROR_QMP_*: This is an error associated with the cmd to
+ * run, ev is Connected.
+ * - otherwise: An other error happend, ev is now Idle.
+ * The callback is only called once.
+ */
+typedef struct libxl__ev_qmp libxl__ev_qmp;
+typedef void libxl__ev_qmp_callback(libxl__egc *egc, libxl__ev_qmp *ev,
+ const libxl__json_object *response,
+ int rc);
+
+_hidden void libxl__ev_qmp_init(libxl__ev_qmp *ev);
+_hidden int libxl__ev_qmp_send(libxl__gc *gc, libxl__ev_qmp *ev,
+ const char *cmd, libxl__json_object *args);
+_hidden void libxl__ev_qmp_dispose(libxl__gc *gc, libxl__ev_qmp *ev);
+
+struct libxl__ev_qmp {
+ /* caller should include this in their own struct */
+ /* caller must fill these in, and they must all remain valid */
+ libxl__ao *ao;
+ libxl_domid domid;
+ libxl__ev_qmp_callback *callback;
+ int payload_fd; /* set to send a fd with the command, -1 otherwise */
+};
+
/*
* evgen structures, which are the state we use for generating
@@ -1909,7 +1981,7 @@ typedef enum {
JSON_ANY = 255 /* this is a mask of all values above, adjust as needed */
} libxl__json_node_type;
-typedef struct libxl__json_object {
+struct libxl__json_object {
libxl__json_node_type type;
union {
bool b;
@@ -1922,7 +1994,7 @@ typedef struct libxl__json_object {
flexarray_t *map;
} u;
struct libxl__json_object *parent;
-} libxl__json_object;
+};
typedef int (*libxl__json_parse_callback)(libxl__gc *gc,
libxl__json_object *o,
--
Anthony PERARD
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v8 06/17] libxl_qmp: Implementation of libxl__ev_qmp_*
2019-01-04 15:30 [PATCH v8 00/17] libxl: Enable save/restore/migration of a restricted QEMU + libxl__ev_qmp_* Anthony PERARD
` (4 preceding siblings ...)
2019-01-04 15:30 ` [PATCH v8 05/17] libxl: Design of an async API to issue QMP commands to QEMU Anthony PERARD
@ 2019-01-04 15:30 ` Anthony PERARD
2019-01-11 12:06 ` Ian Jackson
2019-01-04 15:30 ` [PATCH v8 07/17] libxl_exec: Add libxl__spawn_initiate_failure Anthony PERARD
` (10 subsequent siblings)
16 siblings, 1 reply; 24+ messages in thread
From: Anthony PERARD @ 2019-01-04 15:30 UTC (permalink / raw)
To: xen-devel; +Cc: Anthony PERARD, Ian Jackson, Wei Liu
This patch implement the API libxl__ev_qmp documented in the previous
patch, "libxl: Design of an async API to issue QMP commands to QEMU".
Since this API is to interact with QEMU via the QMP protocol, it also
implement a QMP client. The specification for the QEMU Machine Protocol
(QMP) can be found in the QEMU repository at:
https://git.qemu.org/?p=qemu.git;a=blob_plain;f=docs/interop/qmp-spec.txt
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
Notes:
v8:
- using STATE_AO_GC everywhere
- call qmp_ev_ensure_reading_writing from qmp_ev_set_state instead of
other places. But add a call to qmp_ev_ensure_reading_writing when the
tx_buf is updated (data been transmitted to QEMU)
- assert return value of write().
- fix implementation doc of rx_buf (can be free when used)
- fix doc of qmp_ev_ensure_reading_writing
- reorder qmp_ev_parse_error_messages and qmp_ev_handle_message
v7:
- Make use of the new `ao` field filled by the caller
(no more free(tx_buf))
- Have reworked the state table to be more accurate, easier to read and
with better desc of the waiting_reply state with sub-states as well as
better description of the state of the ev_fd `efd`.
- There is state transition changes, now we have cap.neg ->
waiting_reply (before it was cap.neg -> connected -> waiting_reply),
that makes the internal connected state the same as the external
Connected state.
- there are now 3 different id variable instead of a single one, the
currently sent one `id`, the next one `next_id`, and the one
associated with the caller's command `msg_id`, `id` will always be the
for the next expected message from QEMU.
- qmp_ev_fd_callback now check for POLLERR, and read async error with
getsockopt(SO_ERROR).
- qmp_ev_callback_readable takes care once again to also parse messages.
But it now first attempt to read messages that would be in the rx_buf
before reading from the socket.
- Some functions state changes have been updated,
qmp_ev_ensure_reading_writing, qmp_ev_set_state,
qmp_ev_callback_writable, qmp_ev_callback_readable
- Have cleaned up qmp_ev_ensure_reading_writing
- Have cleaned up qmp_error_class_to_libxl_error_code, and log unknown
error classes.
- In qmp_ev_callback_readable, the size increase of the buffer have been
reworked to update both rx_buf_size and rx_buf at the same time, and
remove the use of max()
- Return value of qmp_prepare_cmd calles is checked and logged on error.
- comments in RHS of struct
- New qmp_ev_tx_buf_clear func
- Add a link to the QMP spec.
- update callers of qmp_prepare_cmd, which doesn't provide the string
lenght anymore also remove ev->msg_len
- rename internal fields qmp_cfd, qmp_efd, qmp_state to cfd, efd and
state.
- change qmp_ev_connect to only allow disconnect on entry
- squash qmp_ev_prepare_cmd into libxl__ev_qmp_send
- reduce max rx buffer size to 1M
query-vcpus with 71 cpus active yield 14484 bytes.
- use JSON to print libxl__json_object
v6.2:
Add definition of the internal broken state
updated comments about states
v6.1:
Adding some comment about possible internal state changes
v6:
This is a squash of 7 commits on the previous version:
- libxl_qmp: Connect to QMP socket
- libxl_qmp: Implement fd callback and read data
- libxl_qmp: Parse JSON input from QMP
- libxl_qmp: Prepare the command to be sent
- libxl_qmp: Handle write to QMP socket
- libxl_qmp: Handle messages from QEMU
- libxl_qmp: Respond to QMP greeting
General rework of the implementation.
Added more comment, with a description of allowed internal states.
Check for EINPROGRESS after connect().
Read until EWOULDBLOCK.
Handle EWOULDBLOCK on write and sendmsg.
Using memmem instead of strstr.
Using memmove instead of having an offset in rx_buf.
Rework buffer allocation
Don't feed \r into json parser anymore
Add a check for a maximum RX buffer size
Added more error messages
New error code ERROR_PROTOCOL_ERROR_QMP
Rewrite conversion of QMP ErrorClass to libxl_error code
Added helpers: qmp_ev_ensure_reading_writing, qmp_ev_set_state
Split some functions, squash others
Added ev->msg* to store generated user command as tx_buf is used during
connection (for qmp_capabilities)
Remove qmp_state_greeting
Added qmp_state_waiting_reply
v5:
nits
use a define instead of a static int for QMP_CAPABILITY_NEGOCIATION_MSGID
use a switch in qmp_ev_callback_writable to check qmp_state
Add a description of the different value of libxl__qmp_state enum.
some cleanup
remove read loop that only handled EINTR, simply return
initialize len and s at declaration time
remove old comment
rename buf_fd to send_fd
Adding default:abort() in qmp_ev_handle_message.
v4:
remove use of a linked list of receive buffer, and use realloc instead.
simplification of the patch due to use of a single allocated space for the
receive buffer.
tools/libxl/libxl_internal.h | 34 ++
tools/libxl/libxl_qmp.c | 739 +++++++++++++++++++++++++++++++++++
tools/libxl/libxl_types.idl | 6 +
3 files changed, 779 insertions(+)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index fadafa5f11..be18d4f341 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -418,6 +418,19 @@ _hidden int libxl__ev_qmp_send(libxl__gc *gc, libxl__ev_qmp *ev,
const char *cmd, libxl__json_object *args);
_hidden void libxl__ev_qmp_dispose(libxl__gc *gc, libxl__ev_qmp *ev);
+typedef enum {
+ /* initial state */
+ qmp_state_disconnected = 1,
+ /* connected to QMP socket, waiting for greeting message */
+ qmp_state_connecting,
+ /* qmp_capabilities command sent, waiting for reply */
+ qmp_state_capability_negotiation,
+ /* sending user's cmd and waiting for reply */
+ qmp_state_waiting_reply,
+ /* ready to send commands */
+ qmp_state_connected,
+} libxl__qmp_state;
+
struct libxl__ev_qmp {
/* caller should include this in their own struct */
/* caller must fill these in, and they must all remain valid */
@@ -425,6 +438,27 @@ struct libxl__ev_qmp {
libxl_domid domid;
libxl__ev_qmp_callback *callback;
int payload_fd; /* set to send a fd with the command, -1 otherwise */
+
+ /*
+ * remaining fields are private to libxl_ev_qmp_*
+ */
+
+ libxl__carefd *cfd;
+ libxl__ev_fd efd;
+ libxl__qmp_state state;
+ int id;
+ int next_id; /* next id to use */
+ /* receive buffer */
+ char *rx_buf;
+ size_t rx_buf_size; /* current allocated size */
+ size_t rx_buf_used; /* actual data in the buffer */
+ /* sending buffer */
+ char *tx_buf;
+ size_t tx_buf_len; /* tx_buf size */
+ size_t tx_buf_off; /* already sent */
+ /* The message to send when ready */
+ char *msg;
+ int msg_id;
};
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index 73f2202b4f..f2ab08620a 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -75,11 +75,18 @@
# define DEBUG_REPORT_RECEIVED(dom, buf, len) ((void)0)
#endif
+#ifdef DEBUG_QMP_CLIENT
+# define LOG_QMP(f, ...) LOGD(DEBUG, ev->domid, f, ##__VA_ARGS__)
+#else
+# define LOG_QMP(f, ...)
+#endif
+
/*
* QMP types & constant
*/
#define QMP_RECEIVE_BUFFER_SIZE 4096
+#define QMP_MAX_SIZE_RX_BUF MB(1)
#define PCI_PT_QDEV_ID "pci-pt-%02x_%02x.%01x"
/*
@@ -1307,6 +1314,738 @@ int libxl__qmp_initializations(libxl__gc *gc, uint32_t domid,
return ret;
}
+/* ------------ Implementation of libxl__ev_qmp ---------------- */
+
+/*
+ * Possible internal state compared to qmp_state:
+ *
+ * qmp_state External cfd efd id rx_buf* tx_buf* msg*
+ * disconnected Idle NULL Idle reset free free free
+ * connecting Active open IN reset used free set
+ * cap.neg Active open IN|OUT sent used cap_neg set
+ * cap.neg Active open IN sent used free set
+ * connected Connected open IN any used free free
+ * waiting_reply Active open IN|OUT sent used free set
+ * waiting_reply Active open IN|OUT sent used user's free
+ * waiting_reply Active open IN sent used free free
+ * broken[1] none[2] any Active any any any any
+ *
+ * [1] When an internal function return an error, it can leave ev_qmp in a
+ * `broken` state but only if the caller is another internal function.
+ * That `broken` needs to be cleaned up, e.i. transitionned to the
+ * `disconnected` state, before the control of ev_qmp is released outsides
+ * of ev_qmp implementation.
+ *
+ * [2] This internal state should not be visible externally, see [1].
+ *
+ * Possible buffers states:
+ * - receiving buffer:
+ * free used
+ * rx_buf NULL NULL or allocated
+ * rx_buf_size 0 allocation size of `rx_buf`
+ * rx_buf_used 0 <= rx_buf_size, actual data in the buffer
+ * - transmitting buffer:
+ * free used
+ * tx_buf NULL contains data
+ * tx_buf_len 0 size of data
+ * tx_buf_off 0 <= tx_buf_len, data already sent
+ * - queued user command:
+ * free set
+ * msg NULL contains data
+ * msg_id 0 id assoctiated with the command in `msg`
+ *
+ * - Allowed internal state transition:
+ * disconnected -> connecting
+ * connection -> capability_negotiation
+ * capability_negotiation/connected -> waiting_reply
+ * waiting_reply -> connected
+ * any -> broken
+ * broken -> disconnected
+ * any -> disconnected
+ *
+ * The QEMU Machine Protocol (QMP) specification can be found in the QEMU
+ * repository:
+ * https://git.qemu.org/?p=qemu.git;a=blob_plain;f=docs/interop/qmp-spec.txt
+ */
+
+/* prototypes */
+
+static void qmp_ev_fd_callback(libxl__egc *egc, libxl__ev_fd *ev_fd,
+ int fd, short events, short revents);
+static int qmp_ev_callback_writable(libxl__gc *gc,
+ libxl__ev_qmp *ev, int fd);
+static int qmp_ev_callback_readable(libxl__egc *egc,
+ libxl__ev_qmp *ev, int fd);
+static int qmp_ev_get_next_msg(libxl__egc *egc, libxl__ev_qmp *ev,
+ libxl__json_object **o_r);
+static int qmp_ev_handle_message(libxl__egc *egc,
+ libxl__ev_qmp *ev,
+ const libxl__json_object *resp);
+
+/* helpers */
+
+static void qmp_ev_ensure_reading_writing(libxl__gc *gc, libxl__ev_qmp *ev)
+ /* Update the state of `efd` to match the permited state
+ * on entry: !disconnected */
+{
+ short events = POLLIN;
+
+ if (ev->tx_buf)
+ events |= POLLOUT;
+ else if ((ev->state == qmp_state_waiting_reply) && ev->msg)
+ events |= POLLOUT;
+
+ libxl__ev_fd_modify(gc, &ev->efd, events);
+}
+
+static void qmp_ev_set_state(libxl__gc *gc, libxl__ev_qmp *ev,
+ libxl__qmp_state new_state)
+ /* on entry: !broken and !disconnected */
+{
+ switch (new_state) {
+ case qmp_state_disconnected:
+ break;
+ case qmp_state_connecting:
+ assert(ev->state == qmp_state_disconnected);
+ break;
+ case qmp_state_capability_negotiation:
+ assert(ev->state == qmp_state_connecting);
+ break;
+ case qmp_state_waiting_reply:
+ assert(ev->state == qmp_state_capability_negotiation ||
+ ev->state == qmp_state_connected);
+ break;
+ case qmp_state_connected:
+ assert(ev->state == qmp_state_waiting_reply);
+ break;
+ }
+
+ ev->state = new_state;
+
+ qmp_ev_ensure_reading_writing(gc, ev);
+}
+
+static void qmp_ev_tx_buf_clear(libxl__ev_qmp *ev)
+{
+ ev->tx_buf = NULL;
+ ev->tx_buf_len = 0;
+ ev->tx_buf_off = 0;
+}
+
+static int qmp_error_class_to_libxl_error_code(libxl__gc *gc,
+ const char *eclass)
+{
+ const libxl_enum_string_table *t = libxl_error_string_table;
+ const char skip[] = "QMP_";
+ const size_t skipl = sizeof(skip) - 1;
+
+ /* compare "QMP_GENERIC_ERROR" from libxl_error to "GenericError"
+ * generated by the QMP server */
+
+ for (; t->s; t++) {
+ const char *s = eclass;
+ const char *se = t->s;
+ if (strncasecmp(t->s, skip, skipl))
+ continue;
+ se += skipl;
+ while (*s && *se) {
+ /* skip underscores */
+ if (*se == '_') {
+ se++;
+ continue;
+ }
+ if (tolower(*s) != tolower(*se))
+ break;
+ s++, se++;
+ }
+ if (!*s && !*se)
+ return t->v;
+ }
+
+ LOG(ERROR, "Unknown QMP error class '%s'", eclass);
+ return ERROR_UNKNOWN_QMP_ERROR;
+}
+
+/* Setup connection */
+
+static int qmp_ev_connect(libxl__gc *gc, libxl__ev_qmp *ev)
+ /* disconnected -> connecting but with `msg` free
+ * on error: broken */
+{
+ int fd;
+ int rc, r;
+ struct sockaddr_un un;
+ const char *qmp_socket_path;
+
+ assert(ev->state == qmp_state_disconnected);
+
+ qmp_socket_path = libxl__qemu_qmp_path(gc, ev->domid);
+
+ LOGD(DEBUG, ev->domid, "Connecting to %s", qmp_socket_path);
+
+ libxl__carefd_begin();
+ fd = socket(AF_UNIX, SOCK_STREAM, 0);
+ ev->cfd = libxl__carefd_opened(CTX, fd);
+ if (!ev->cfd) {
+ LOGED(ERROR, ev->domid, "socket() failed");
+ rc = ERROR_FAIL;
+ goto out;
+ }
+ rc = libxl_fd_set_nonblock(CTX, libxl__carefd_fd(ev->cfd), 1);
+ if (rc)
+ goto out;
+
+ rc = libxl__prepare_sockaddr_un(gc, &un, qmp_socket_path,
+ "QMP socket");
+ if (rc)
+ goto out;
+
+ r = connect(libxl__carefd_fd(ev->cfd),
+ (struct sockaddr *) &un, sizeof(un));
+ if (r && errno != EINPROGRESS) {
+ LOGED(ERROR, ev->domid, "Failed to connect to QMP socket %s",
+ qmp_socket_path);
+ rc = ERROR_FAIL;
+ goto out;
+ }
+
+ rc = libxl__ev_fd_register(gc, &ev->efd, qmp_ev_fd_callback,
+ libxl__carefd_fd(ev->cfd), POLLIN);
+ if (rc)
+ goto out;
+
+ qmp_ev_set_state(gc, ev, qmp_state_connecting);
+
+ return 0;
+
+out:
+ return rc;
+}
+
+/* QMP FD callbacks */
+
+static void qmp_ev_fd_callback(libxl__egc *egc, libxl__ev_fd *ev_fd,
+ int fd, short events, short revents)
+ /* On entry, ev_fd is (of course) Active. The ev_qmp may be in any
+ * state where this is permitted. qmp_ev_fd_callback will do the work
+ * necessary to make progress, depending on the current state, and make
+ * the appropriate state transitions and callbacks. */
+{
+ libxl__ev_qmp *ev = CONTAINER_OF(ev_fd, *ev, efd);
+ STATE_AO_GC(ev->ao);
+ int rc;
+
+ if (revents & (POLLHUP|POLLERR)) {
+ int r;
+ int error_val = 0;
+ socklen_t opt_len = sizeof(error_val);
+
+ r = getsockopt(fd, SOL_SOCKET, SO_ERROR, &error_val, &opt_len);
+ if (r)
+ LOGED(ERROR, ev->domid, "getsockopt failed");
+ if (!r && error_val) {
+ errno = error_val;
+ LOGED(ERROR, ev->domid, "error on QMP socket");
+ } else {
+ LOGD(ERROR, ev->domid,
+ "received POLLHUP|POLLERR from QMP socket");
+ }
+ rc = ERROR_PROTOCOL_ERROR_QMP;
+ goto error;
+ }
+
+ if (revents & ~(POLLIN|POLLOUT)) {
+ LOGD(ERROR, ev->domid,
+ "unexpected poll event 0x%x on QMP socket (expected POLLIN "
+ "and/or POLLOUT)",
+ revents);
+ rc = ERROR_FAIL;
+ goto error;
+ }
+
+ if (revents & POLLOUT) {
+ rc = qmp_ev_callback_writable(gc, ev, fd);
+ if (rc)
+ goto error;
+ }
+
+ if (revents & POLLIN) {
+ rc = qmp_ev_callback_readable(egc, ev, fd);
+ if (rc < 0)
+ goto error;
+ if (rc == 1) {
+ /* user callback has been called */
+ return;
+ }
+ }
+
+ return;
+
+error:
+ assert(rc);
+
+ LOGD(ERROR, ev->domid,
+ "Error happened with the QMP connection to QEMU");
+
+ /* On error, deallocate all private ressources */
+ libxl__ev_qmp_dispose(gc, ev);
+
+ /* And tell libxl__ev_qmp user about the error */
+ ev->callback(egc, ev, NULL, rc); /* must be last */
+}
+
+static int qmp_ev_callback_writable(libxl__gc *gc,
+ libxl__ev_qmp *ev, int fd)
+ /* on entry: !disconnected
+ * on return, one of these state transition:
+ * waiting_reply (with msg set) -> waiting_reply (with msg free)
+ * tx_buf set -> same state or tx_buf free
+ * on error: broken */
+{
+ int rc;
+ ssize_t r;
+
+ if (ev->state == qmp_state_waiting_reply) {
+ if (ev->msg) {
+ assert(!ev->tx_buf);
+ ev->tx_buf = ev->msg;
+ ev->tx_buf_len = strlen(ev->msg);
+ ev->tx_buf_off = 0;
+ ev->id = ev->msg_id;
+ ev->msg = NULL;
+ ev->msg_id = 0;
+ }
+ }
+
+ assert(ev->tx_buf);
+
+ LOG_QMP("sending: '%.*s'", (int)ev->tx_buf_len, ev->tx_buf);
+
+ /*
+ * We will send a file descriptor associated with a command on the
+ * first byte of this command.
+ */
+ if (ev->state == qmp_state_waiting_reply &&
+ ev->payload_fd >= 0 &&
+ ev->tx_buf_off == 0) {
+
+ rc = libxl__sendmsg_fds(gc, fd, ev->tx_buf, 1,
+ 1, &ev->payload_fd, "QMP socket");
+ /* Check for EWOULDBLOCK, and return to try again later */
+ if (rc == ERROR_NOT_READY)
+ return 0;
+ if (rc)
+ return rc;
+ ev->tx_buf_off++;
+ }
+
+ while (ev->tx_buf_off < ev->tx_buf_len) {
+ ssize_t max_write = ev->tx_buf_len - ev->tx_buf_off;
+ r = write(fd, ev->tx_buf + ev->tx_buf_off, max_write);
+ if (r < 0) {
+ if (errno == EINTR)
+ continue;
+ if (errno == EWOULDBLOCK)
+ break;
+ LOGED(ERROR, ev->domid, "failed to write to QMP socket");
+ return ERROR_FAIL;
+ }
+ assert(r >= 0 && r <= max_write);
+ ev->tx_buf_off += r;
+ }
+
+ if (ev->tx_buf_off == ev->tx_buf_len)
+ qmp_ev_tx_buf_clear(ev);
+
+ qmp_ev_ensure_reading_writing(gc, ev);
+
+ return 0;
+}
+
+static int qmp_ev_callback_readable(libxl__egc *egc,
+ libxl__ev_qmp *ev, int fd)
+ /*
+ * Return values:
+ * < 0 libxl error code
+ * 0 success
+ * 1 success, but a user callback has been called,
+ * `ev` should not be used anymore.
+ *
+ * This function will update the rx buffer and possibly update
+ * ev->state:
+ * connecting -> capability_negotiation
+ * capability_negotiation -> waiting_reply
+ * waiting_reply -> connected
+ * on error: broken
+ */
+{
+ STATE_AO_GC(ev->ao);
+ int rc;
+ ssize_t r;
+
+ while (1) {
+ while (1) {
+ libxl__json_object *o = NULL;
+
+ /* parse rx buffer to find one json object */
+ rc = qmp_ev_get_next_msg(egc, ev, &o);
+ if (rc == ERROR_NOTFOUND)
+ break;
+ else if (rc)
+ return rc;
+
+ /* Must be last and return when the user callback is called */
+ rc = qmp_ev_handle_message(egc, ev, o);
+ if (rc)
+ /* returns both rc values -ERROR_* and 1 */
+ return rc;
+ }
+
+ /* Check if the buffer still have space, or increase size */
+ if (ev->rx_buf_size - ev->rx_buf_used < QMP_RECEIVE_BUFFER_SIZE) {
+ size_t newsize = ev->rx_buf_size * 2 + QMP_RECEIVE_BUFFER_SIZE;
+
+ if (newsize > QMP_MAX_SIZE_RX_BUF) {
+ LOGD(ERROR, ev->domid,
+ "QMP receive buffer is too big (%ld > %lld)",
+ newsize, QMP_MAX_SIZE_RX_BUF);
+ return ERROR_BUFFERFULL;
+ }
+ ev->rx_buf_size = newsize;
+ ev->rx_buf = libxl__realloc(gc, ev->rx_buf, ev->rx_buf_size);
+ }
+
+ r = read(fd, ev->rx_buf + ev->rx_buf_used,
+ ev->rx_buf_size - ev->rx_buf_used);
+ if (r < 0) {
+ if (errno == EINTR)
+ continue;
+ if (errno == EWOULDBLOCK)
+ break;
+ LOGED(ERROR, ev->domid, "error reading QMP socket");
+ return ERROR_FAIL;
+ }
+
+ if (r == 0) {
+ LOGD(ERROR, ev->domid, "Unexpected EOF on QMP socket");
+ return ERROR_PROTOCOL_ERROR_QMP;
+ }
+
+ LOG_QMP("received %ldB: '%.*s'", r,
+ (int)r, ev->rx_buf + ev->rx_buf_used);
+
+ ev->rx_buf_used += r;
+ assert(ev->rx_buf_used <= ev->rx_buf_size);
+ }
+
+ return 0;
+}
+
+/* Handle messages received from QMP server */
+
+static int qmp_ev_get_next_msg(libxl__egc *egc, libxl__ev_qmp *ev,
+ libxl__json_object **o_r)
+ /* Find a JSON object and store it in o_r.
+ * return ERROR_NOTFOUND if no object is found.
+ *
+ * !disconnected -> same state (with rx buffer updated)
+ */
+{
+ STATE_AO_GC(ev->ao);
+ size_t len;
+ char *end = NULL;
+ const char eom[] = "\r\n";
+ const size_t eoml = sizeof(eom) - 1;
+ libxl__json_object *o = NULL;
+
+ if (!ev->rx_buf_used)
+ return ERROR_NOTFOUND;
+
+ /* Search for the end of a QMP message: "\r\n" */
+ end = memmem(ev->rx_buf, ev->rx_buf_used, eom, eoml);
+ if (!end)
+ return ERROR_NOTFOUND;
+ len = (end - ev->rx_buf) + eoml;
+
+ LOG_QMP("parsing %luB: '%.*s'", len, (int)len, ev->rx_buf);
+
+ /* Replace \r by \0 so that libxl__json_parse can use strlen */
+ ev->rx_buf[len - eoml] = '\0';
+ o = libxl__json_parse(gc, ev->rx_buf);
+
+ if (!o) {
+ LOGD(ERROR, ev->domid, "Parse error");
+ return ERROR_PROTOCOL_ERROR_QMP;
+ }
+
+ ev->rx_buf_used -= len;
+ memmove(ev->rx_buf, ev->rx_buf + len, ev->rx_buf_used);
+
+ LOG_QMP("JSON object received: %s", JSON(o));
+
+ *o_r = o;
+
+ return 0;
+}
+
+static int qmp_ev_parse_error_messages(libxl__egc *egc,
+ libxl__ev_qmp *ev,
+ const libxl__json_object *resp);
+
+static int qmp_ev_handle_message(libxl__egc *egc,
+ libxl__ev_qmp *ev,
+ const libxl__json_object *resp)
+ /*
+ * This function will handle every messages sent by the QMP server.
+ * Return values:
+ * < 0 libxl error code
+ * 0 success
+ * 1 success, but a user callback has been called,
+ * `ev` should not be used anymore.
+ *
+ * Possible state changes:
+ * connecting -> capability_negotiation
+ * capability_negotiation -> waiting_reply
+ * waiting_reply -> waiting_reply/connected
+ *
+ * on error: broken
+ */
+{
+ STATE_AO_GC(ev->ao);
+ int id;
+ char *buf;
+ int rc = 0;
+ const libxl__json_object *o;
+ const libxl__json_object *response;
+ libxl__qmp_message_type type = qmp_response_type(resp);
+
+ switch (type) {
+ case LIBXL__QMP_MESSAGE_TYPE_QMP:
+ /* greeting message */
+
+ if (ev->state != qmp_state_connecting) {
+ LOGD(ERROR, ev->domid,
+ "Unexpected greeting message received");
+ return ERROR_PROTOCOL_ERROR_QMP;
+ }
+
+ /* Prepare next message to send */
+ assert(!ev->tx_buf);
+ ev->id = ev->next_id++;
+ buf = qmp_prepare_cmd(gc, "qmp_capabilities", NULL, ev->id);
+ if (!buf) {
+ LOGD(ERROR, ev->domid,
+ "Failed to generate qmp_capabilities command");
+ return ERROR_FAIL;
+ }
+ ev->tx_buf = buf;
+ ev->tx_buf_len = strlen(buf);
+ ev->tx_buf_off = 0;
+ qmp_ev_set_state(gc, ev, qmp_state_capability_negotiation);
+
+ return 0;
+
+ case LIBXL__QMP_MESSAGE_TYPE_RETURN:
+ case LIBXL__QMP_MESSAGE_TYPE_ERROR:
+ /*
+ * Reply to a command (success/error) or server error
+ *
+ * In this cases, we are parsing two possibles responses:
+ * - success:
+ * { "return": json-value, "id": int }
+ * - error:
+ * { "error": { "class": string, "desc": string }, "id": int }
+ */
+
+ o = libxl__json_map_get("id", resp, JSON_INTEGER);
+ if (!o) {
+ /*
+ * If "id" isn't present, an error occur on the server before
+ * it has read the "id" provided by libxl.
+ *
+ * We deliberately squash all errors into
+ * ERROR_PROTOCOL_ERROR_QMP as qmp_ev_parse_error_messages may
+ * also return ERROR_QMP_* but those are reserved for errors
+ * return by the caller's command.
+ */
+ qmp_ev_parse_error_messages(egc, ev, resp);
+ return ERROR_PROTOCOL_ERROR_QMP;
+ }
+
+ id = libxl__json_object_get_integer(o);
+
+ if (id != ev->id) {
+ LOGD(ERROR, ev->domid,
+ "Message from QEMU with unexpected id %d: %s",
+ id, JSON(resp));
+ return ERROR_PROTOCOL_ERROR_QMP;
+ }
+
+ switch (ev->state) {
+ case qmp_state_capability_negotiation:
+ if (type != LIBXL__QMP_MESSAGE_TYPE_RETURN) {
+ LOGD(ERROR, ev->domid,
+ "Error during capability negotiation: %s",
+ JSON(resp));
+ return ERROR_PROTOCOL_ERROR_QMP;
+ }
+ qmp_ev_set_state(gc, ev, qmp_state_waiting_reply);
+ return 0;
+ case qmp_state_waiting_reply:
+ if (type == LIBXL__QMP_MESSAGE_TYPE_RETURN) {
+ response = libxl__json_map_get("return", resp, JSON_ANY);
+ rc = 0;
+ } else {
+ /* error message */
+ response = NULL;
+ rc = qmp_ev_parse_error_messages(egc, ev, resp);
+ }
+ qmp_ev_set_state(gc, ev, qmp_state_connected);
+ ev->callback(egc, ev, response, rc); /* must be last */
+ return 1;
+ default:
+ LOGD(ERROR, ev->domid, "Unexpected message: %s", JSON(resp));
+ return ERROR_PROTOCOL_ERROR_QMP;
+ }
+ return 0;
+
+ case LIBXL__QMP_MESSAGE_TYPE_EVENT:
+ /* Events are ignored */
+ return 0;
+
+ case LIBXL__QMP_MESSAGE_TYPE_INVALID:
+ LOGD(ERROR, ev->domid, "Unexpected message received: %s",
+ JSON(resp));
+ return ERROR_PROTOCOL_ERROR_QMP;
+
+ default:
+ abort();
+ }
+
+ return 0;
+}
+
+static int qmp_ev_parse_error_messages(libxl__egc *egc,
+ libxl__ev_qmp *ev,
+ const libxl__json_object *resp)
+ /* no state change */
+{
+ STATE_AO_GC(ev->ao);
+ int rc;
+ const char *s;
+ const libxl__json_object *o;
+ const libxl__json_object *err;
+
+ /*
+ * { "error": { "class": string, "desc": string } }
+ */
+
+ err = libxl__json_map_get("error", resp, JSON_MAP);
+
+ o = libxl__json_map_get("class", err, JSON_STRING);
+ if (!o) {
+ LOGD(ERROR, ev->domid,
+ "Protocol error: missing 'class' member in error message");
+ return ERROR_PROTOCOL_ERROR_QMP;
+ }
+ s = libxl__json_object_get_string(o);
+ if (s)
+ rc = qmp_error_class_to_libxl_error_code(gc, s);
+ else
+ rc = ERROR_PROTOCOL_ERROR_QMP;
+
+ o = libxl__json_map_get("desc", err, JSON_STRING);
+ if (!o) {
+ LOGD(ERROR, ev->domid,
+ "Protocol error: missing 'desc' member in error message");
+ return ERROR_PROTOCOL_ERROR_QMP;
+ }
+ s = libxl__json_object_get_string(o);
+ if (s)
+ LOGD(ERROR, ev->domid, "%s", s);
+ else
+ LOGD(ERROR, ev->domid, "Received unexpected error: %s",
+ JSON(resp));
+ return rc;
+}
+
+/*
+ * libxl__ev_qmp_*
+ */
+
+void libxl__ev_qmp_init(libxl__ev_qmp *ev)
+ /* disconnected -> disconnected */
+{
+ /* Start with an message ID that is obviously generated by libxl
+ * "xlq\0" */
+ ev->next_id = 0x786c7100;
+
+ ev->cfd = NULL;
+ libxl__ev_fd_init(&ev->efd);
+ ev->state = qmp_state_disconnected;
+ ev->id = 0;
+
+ ev->rx_buf = NULL;
+ ev->rx_buf_size = ev->rx_buf_used = 0;
+ qmp_ev_tx_buf_clear(ev);
+
+ ev->msg = NULL;
+ ev->msg_id = 0;
+}
+
+int libxl__ev_qmp_send(libxl__gc *unused_gc, libxl__ev_qmp *ev,
+ const char *cmd, libxl__json_object *args)
+ /* disconnected -> connecting
+ * connected -> waiting_reply (with msg set)
+ * on error: disconnected */
+{
+ STATE_AO_GC(ev->ao);
+ int rc;
+
+ LOGD(DEBUG, ev->domid, " ev %p, cmd '%s'", ev, cmd);
+
+ assert(ev->state == qmp_state_disconnected ||
+ ev->state == qmp_state_connected);
+ assert(cmd);
+
+ /* Connect to QEMU if not already connected */
+ if (ev->state == qmp_state_disconnected) {
+ rc = qmp_ev_connect(gc, ev);
+ if (rc)
+ goto error;
+ }
+
+ /* Prepare user command */
+ ev->msg_id = ev->next_id++;
+ ev->msg = qmp_prepare_cmd(gc, cmd, args, ev->msg_id);
+ if (!ev->msg) {
+ LOGD(ERROR, ev->domid, "Failed to generate caller's command %s",
+ cmd);
+ rc = ERROR_FAIL;
+ goto error;
+ }
+ if (ev->state == qmp_state_connected) {
+ qmp_ev_set_state(gc, ev, qmp_state_waiting_reply);
+ }
+
+ return 0;
+
+error:
+ libxl__ev_qmp_dispose(gc, ev);
+ return rc;
+}
+
+void libxl__ev_qmp_dispose(libxl__gc *gc, libxl__ev_qmp *ev)
+ /* * -> disconnected */
+{
+ LOGD(DEBUG, ev->domid, " ev %p", ev);
+
+ libxl__ev_fd_deregister(gc, &ev->efd);
+ libxl__carefd_close(ev->cfd);
+
+ libxl__ev_qmp_init(ev);
+}
+
/*
* Local variables:
* mode: C
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 141c46e42a..212b00a677 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -69,6 +69,12 @@ libxl_error = Enumeration("error", [
(-23, "NOTFOUND"),
(-24, "DOMAIN_DESTROYED"), # Target domain ceased to exist during op
(-25, "FEATURE_REMOVED"), # For functionality that has been removed
+ (-26, "PROTOCOL_ERROR_QMP"),
+ (-27, "UNKNOWN_QMP_ERROR"),
+ (-28, "QMP_GENERIC_ERROR"), # unspecified qmp error
+ (-29, "QMP_COMMAND_NOT_FOUND"), # the requested command has not been found
+ (-30, "QMP_DEVICE_NOT_ACTIVE"), # a device has failed to be become active
+ (-31, "QMP_DEVICE_NOT_FOUND"), # the requested device has not been found
], value_namespace = "")
libxl_domain_type = Enumeration("domain_type", [
--
Anthony PERARD
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v8 06/17] libxl_qmp: Implementation of libxl__ev_qmp_*
2019-01-04 15:30 ` [PATCH v8 06/17] libxl_qmp: Implementation of libxl__ev_qmp_* Anthony PERARD
@ 2019-01-11 12:06 ` Ian Jackson
2019-01-11 12:16 ` Anthony PERARD
0 siblings, 1 reply; 24+ messages in thread
From: Ian Jackson @ 2019-01-11 12:06 UTC (permalink / raw)
To: Anthony PERARD; +Cc: xen-devel, Wei Liu
Anthony PERARD writes ("[PATCH v8 06/17] libxl_qmp: Implementation of libxl__ev_qmp_*"):
> This patch implement the API libxl__ev_qmp documented in the previous
> patch, "libxl: Design of an async API to issue QMP commands to QEMU".
>
> Since this API is to interact with QEMU via the QMP protocol, it also
> implement a QMP client. The specification for the QEMU Machine Protocol
> (QMP) can be found in the QEMU repository at:
> https://git.qemu.org/?p=qemu.git;a=blob_plain;f=docs/interop/qmp-spec.txt
Thanks. I have re-reviewed this and I have only this one question:
> + while (ev->tx_buf_off < ev->tx_buf_len) {
> + ssize_t max_write = ev->tx_buf_len - ev->tx_buf_off;
> + r = write(fd, ev->tx_buf + ev->tx_buf_off, max_write);
> + if (r < 0) {
> + if (errno == EINTR)
> + continue;
> + if (errno == EWOULDBLOCK)
> + break;
> + LOGED(ERROR, ev->domid, "failed to write to QMP socket");
> + return ERROR_FAIL;
> + }
> + assert(r >= 0 && r <= max_write);
Doesn't this want to be
+ assert(r > 0 && r <= max_write);
?
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v8 06/17] libxl_qmp: Implementation of libxl__ev_qmp_*
2019-01-11 12:06 ` Ian Jackson
@ 2019-01-11 12:16 ` Anthony PERARD
2019-01-11 12:21 ` Ian Jackson
0 siblings, 1 reply; 24+ messages in thread
From: Anthony PERARD @ 2019-01-11 12:16 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel, Wei Liu
On Fri, Jan 11, 2019 at 12:06:07PM +0000, Ian Jackson wrote:
> Anthony PERARD writes ("[PATCH v8 06/17] libxl_qmp: Implementation of libxl__ev_qmp_*"):
> > This patch implement the API libxl__ev_qmp documented in the previous
> > patch, "libxl: Design of an async API to issue QMP commands to QEMU".
> >
> > Since this API is to interact with QEMU via the QMP protocol, it also
> > implement a QMP client. The specification for the QEMU Machine Protocol
> > (QMP) can be found in the QEMU repository at:
> > https://git.qemu.org/?p=qemu.git;a=blob_plain;f=docs/interop/qmp-spec.txt
>
> Thanks. I have re-reviewed this and I have only this one question:
>
> > + while (ev->tx_buf_off < ev->tx_buf_len) {
> > + ssize_t max_write = ev->tx_buf_len - ev->tx_buf_off;
> > + r = write(fd, ev->tx_buf + ev->tx_buf_off, max_write);
> > + if (r < 0) {
> > + if (errno == EINTR)
> > + continue;
> > + if (errno == EWOULDBLOCK)
> > + break;
> > + LOGED(ERROR, ev->domid, "failed to write to QMP socket");
> > + return ERROR_FAIL;
> > + }
> > + assert(r >= 0 && r <= max_write);
>
> Doesn't this want to be
> + assert(r > 0 && r <= max_write);
Yes, I think that would be fine. I guess that would catch issue where
max_write is 0 (so trying to write 0 bytes).
Thanks,
--
Anthony PERARD
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v8 06/17] libxl_qmp: Implementation of libxl__ev_qmp_*
2019-01-11 12:16 ` Anthony PERARD
@ 2019-01-11 12:21 ` Ian Jackson
0 siblings, 0 replies; 24+ messages in thread
From: Ian Jackson @ 2019-01-11 12:21 UTC (permalink / raw)
To: Anthony PERARD; +Cc: xen-devel, Wei Liu
Anthony PERARD writes ("Re: [PATCH v8 06/17] libxl_qmp: Implementation of libxl__ev_qmp_*"):
> On Fri, Jan 11, 2019 at 12:06:07PM +0000, Ian Jackson wrote:
> > Doesn't this want to be
> > + assert(r > 0 && r <= max_write);
>
> Yes, I think that would be fine. I guess that would catch issue where
> max_write is 0 (so trying to write 0 bytes).
I was thinking of situations where the open-file referred to by fd has
been put into a funny state which leads to write() returning 0.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v8 07/17] libxl_exec: Add libxl__spawn_initiate_failure
2019-01-04 15:30 [PATCH v8 00/17] libxl: Enable save/restore/migration of a restricted QEMU + libxl__ev_qmp_* Anthony PERARD
` (5 preceding siblings ...)
2019-01-04 15:30 ` [PATCH v8 06/17] libxl_qmp: Implementation of libxl__ev_qmp_* Anthony PERARD
@ 2019-01-04 15:30 ` Anthony PERARD
2019-01-04 15:30 ` [PATCH v8 08/17] libxl: Add init/dispose of for libxl__domain_build_state Anthony PERARD
` (9 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: Anthony PERARD @ 2019-01-04 15:30 UTC (permalink / raw)
To: xen-devel; +Cc: Anthony PERARD, Ian Jackson, Wei Liu
This function can be used by user of libxl__spawn_* when they setup a
notification other than xenstore. The parent can already report success
via libxl__spawn_initiate_detach(), this new function can be used for
failure instead of waiting for the timeout.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
Notes:
v8:
Acked
v7:
call spawn_fail from libxl__spawn_initiate_failure.
modify spawn_fail to set ss->rc only once.
Fix description to tell caller of libxl__spawn_initiate_failure that
they have to log.
use an `egc` instead of `gc` with libxl__spawn_initiate_failure.
v6:
long line fix
typos fixed
fix leak of internal state into external doc
if the function is call multiple times, set rc only the first time.
v5:
fix typos
tools/libxl/libxl_exec.c | 11 ++++++++++-
tools/libxl/libxl_internal.h | 23 ++++++++++++++++++++++-
2 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/tools/libxl/libxl_exec.c b/tools/libxl/libxl_exec.c
index 02e6c917f0..47c9c8f1ba 100644
--- a/tools/libxl/libxl_exec.c
+++ b/tools/libxl/libxl_exec.c
@@ -373,13 +373,22 @@ void libxl__spawn_initiate_detach(libxl__gc *gc, libxl__spawn_state *ss)
spawn_detach(gc, ss);
}
+void libxl__spawn_initiate_failure(libxl__egc *egc, libxl__spawn_state *ss,
+ int rc)
+/* The spawn state must be Attached on entry and will be Attached Failed
+ * on return. */
+{
+ spawn_fail(egc, ss, rc);
+}
+
static void spawn_fail(libxl__egc *egc, libxl__spawn_state *ss, int rc)
/* Caller must have logged. Must be last thing in calling function,
* as it may make the callback. Precondition: Attached or Detaching. */
{
EGC_GC;
assert(rc);
- ss->rc = rc;
+ if (!ss->rc)
+ ss->rc = rc;
spawn_detach(gc, ss);
}
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index be18d4f341..9e7e4dcae9 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1567,7 +1567,8 @@ _hidden void libxl__spawn_init(libxl__spawn_state*);
*
* The inner child must soon exit or exec. It must also soon exit or
* notify the parent of its successful startup by writing to the
- * xenstore path xspath.
+ * xenstore path xspath OR via other means that the parent will have
+ * to set up.
*
* The user (in the parent) will be called back (confirm_cb) every
* time that xenstore path is modified.
@@ -1623,6 +1624,26 @@ _hidden int libxl__spawn_spawn(libxl__egc *egc, libxl__spawn_state *spawn);
*/
_hidden void libxl__spawn_initiate_detach(libxl__gc *gc, libxl__spawn_state*);
+/*
+ * libxl__spawn_initiate_failure - Propagate failure from the caller to the
+ * callee.
+ *
+ * Works by killing the intermediate process from spawn_spawn.
+ * After this function returns, a failure will be reported.
+ *
+ * This is not synchronous: there will be a further callback when
+ * the detach is complete.
+ *
+ * Caller must have logged a failure reason.
+ *
+ * The spawn state must be Attached on entry and will remain Attached. It
+ * is possible for a spawn to fail for multiple reasons, for example
+ * call(s) to libxl__spawn_initiate_failure and also for some other reason.
+ * In that case the first rc value from any source will take precedence.
+ */
+_hidden void libxl__spawn_initiate_failure(libxl__egc *egc,
+ libxl__spawn_state *ss, int rc);
+
/*
* If successful, this should return 0.
*
--
Anthony PERARD
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v8 08/17] libxl: Add init/dispose of for libxl__domain_build_state
2019-01-04 15:30 [PATCH v8 00/17] libxl: Enable save/restore/migration of a restricted QEMU + libxl__ev_qmp_* Anthony PERARD
` (6 preceding siblings ...)
2019-01-04 15:30 ` [PATCH v8 07/17] libxl_exec: Add libxl__spawn_initiate_failure Anthony PERARD
@ 2019-01-04 15:30 ` Anthony PERARD
2019-01-04 15:30 ` [PATCH v8 09/17] libxl_dm: Pre-open QMP socket for QEMU Anthony PERARD
` (8 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: Anthony PERARD @ 2019-01-04 15:30 UTC (permalink / raw)
To: xen-devel; +Cc: Anthony PERARD, Ian Jackson, Wei Liu
These two new functions libxl__domain_build_state_{init,dispose} should
be called every time a new libxl__domain_build_state comes to existance.
There seems to be two of them, one with the domain creation machinery,
and one in the stub_dm_spawn.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
Notes:
v8:
Acked
v7:
new patch
tools/libxl/libxl_create.c | 14 ++++++++++++--
tools/libxl/libxl_dm.c | 3 +++
tools/libxl/libxl_internal.h | 3 +++
3 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 891175b15b..4ca25f56a6 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -761,6 +761,16 @@ static int store_libxl_entry(libxl__gc *gc, uint32_t domid,
libxl_device_model_version_to_string(b_info->device_model_version));
}
+void libxl__domain_build_state_init(libxl__domain_build_state *state)
+{
+}
+
+void libxl__domain_build_state_dispose(libxl__domain_build_state *state)
+{
+ libxl__file_reference_unmap(&state->pv_kernel);
+ libxl__file_reference_unmap(&state->pv_ramdisk);
+}
+
/*----- main domain creation -----*/
/* We have a linear control flow; only one event callback is
@@ -823,6 +833,7 @@ static void initiate_domain_create(libxl__egc *egc,
const int restore_fd = dcs->restore_fd;
domid = dcs->domid_soft_reset;
+ libxl__domain_build_state_init(&dcs->build_state);
if (d_config->c_info.ssid_label) {
char *s = d_config->c_info.ssid_label;
@@ -1595,8 +1606,7 @@ static void domcreate_complete(libxl__egc *egc,
libxl_domain_config *const d_config = dcs->guest_config;
libxl_domain_config *d_config_saved = &dcs->guest_config_saved;
- libxl__file_reference_unmap(&dcs->build_state.pv_kernel);
- libxl__file_reference_unmap(&dcs->build_state.pv_ramdisk);
+ libxl__domain_build_state_dispose(&dcs->build_state);
if (!rc && d_config->b_info.exec_ssidref)
rc = xc_flask_relabel_domain(CTX->xch, dcs->guest_domid, d_config->b_info.exec_ssidref);
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index be493cf9f2..200310083f 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -2045,6 +2045,8 @@ 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;
+ libxl__domain_build_state_init(stubdom_state);
+
if (guest_config->b_info.device_model_version !=
LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL) {
ret = ERROR_INVAL;
@@ -2373,6 +2375,7 @@ static void stubdom_xswait_cb(libxl__egc *egc, libxl__xswait_state *xswait,
if (strcmp(p, "running"))
return;
out:
+ libxl__domain_build_state_dispose(&sdss->dm_state);
libxl__xswait_stop(gc, xswait);
sdss->callback(egc, &sdss->dm, rc);
}
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 9e7e4dcae9..88c01ff118 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1265,6 +1265,9 @@ typedef struct {
uint32_t clock_frequency;
} libxl__domain_build_state;
+_hidden void libxl__domain_build_state_init(libxl__domain_build_state *s);
+_hidden void libxl__domain_build_state_dispose(libxl__domain_build_state *s);
+
_hidden int libxl__build_pre(libxl__gc *gc, uint32_t domid,
libxl_domain_config * const d_config,
libxl__domain_build_state *state);
--
Anthony PERARD
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v8 09/17] libxl_dm: Pre-open QMP socket for QEMU
2019-01-04 15:30 [PATCH v8 00/17] libxl: Enable save/restore/migration of a restricted QEMU + libxl__ev_qmp_* Anthony PERARD
` (7 preceding siblings ...)
2019-01-04 15:30 ` [PATCH v8 08/17] libxl: Add init/dispose of for libxl__domain_build_state Anthony PERARD
@ 2019-01-04 15:30 ` Anthony PERARD
2019-01-04 15:30 ` [PATCH v8 10/17] libxl: Add dmss_init/dispose for libxl__dm_spawn_state Anthony PERARD
` (7 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: Anthony PERARD @ 2019-01-04 15:30 UTC (permalink / raw)
To: xen-devel; +Cc: Anthony PERARD, Ian Jackson, Wei Liu
This patch moves the creation of the QMP unix socket from QEMU to libxl.
But libxl doesn't rely on this yet.
When starting QEMU with dm_restrict=1, pre-open the QMP socket before
exec QEMU. That socket will be useful to find out if QEMU is ready, and
pre-opening it means that libxl can connect to it without waiting for
QEMU to create it.
The pre-opening is conditional, based on the use of dm_restrict
because it is using a new command line option of QEMU, and dm_restrict
support in QEMU is newer.
-chardev socket,fd=X is available with QEMU 2.12, since commit:
> char: allow passing pre-opened socket file descriptor at startup
> 0935700f8544033ebbd41e1f13cd528f8a58d24d
dm_restrict is available in QEMU 3.0.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
Notes:
v8:
acked
v7:
move domain_build_state init/dispose to a different patch
and close dm_monitor_fd in that new dispose function
assert that dm_monitor_fd should be -1 when using old qemu
v6:
move dm_monitor_fd into libxl_domain_build_info (or d_state)
-> move the creation of the socket into libxl__spawn_local_dm
instead of libxl__build_device_model_args
Use libxl_domid type instead of int for libxl__pre_open_qmp_socket()
Check function calls (bind and listen) return value in a separate statement.
typo and other coding style issue fixes
v5:
use libxl__remove_file
few changes in coding style
remove stale includes (sys/socket, sys/un) which are now in libxl_internal.h
v4:
separate the logic to open a socket into a function.
Use libxl__prepare_sockaddr_un() to check path size
tools/libxl/libxl_create.c | 5 +++
tools/libxl/libxl_dm.c | 71 ++++++++++++++++++++++++++++++++++--
tools/libxl/libxl_internal.h | 1 +
3 files changed, 73 insertions(+), 4 deletions(-)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 4ca25f56a6..89f4766a27 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -763,12 +763,17 @@ static int store_libxl_entry(libxl__gc *gc, uint32_t domid,
void libxl__domain_build_state_init(libxl__domain_build_state *state)
{
+ state->dm_monitor_fd = -1;
}
void libxl__domain_build_state_dispose(libxl__domain_build_state *state)
{
libxl__file_reference_unmap(&state->pv_kernel);
libxl__file_reference_unmap(&state->pv_ramdisk);
+ if (state->dm_monitor_fd >= 0) {
+ close(state->dm_monitor_fd);
+ state->dm_monitor_fd = -1;
+ }
}
/*----- main domain creation -----*/
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 200310083f..b9921eb885 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -670,6 +670,8 @@ static int libxl__build_device_model_args_old(libxl__gc *gc,
dm_args = flexarray_make(gc, 16, 1);
dm_envs = flexarray_make(gc, 16, 1);
+ assert(state->dm_monitor_fd == -1);
+
libxl__set_qemu_env_for_xsa_180(gc, dm_envs);
flexarray_vappend(dm_args, dm,
@@ -1100,6 +1102,51 @@ static char *qemu_disk_ide_drive_string(libxl__gc *gc, const char *target_path,
return drive;
}
+static int libxl__pre_open_qmp_socket(libxl__gc *gc, libxl_domid domid,
+ int *fd_r)
+{
+ int rc, r;
+ int fd;
+ struct sockaddr_un un;
+ const char *path = libxl__qemu_qmp_path(gc, domid);
+
+ fd = socket(AF_UNIX, SOCK_STREAM, 0);
+ if (fd < 0) {
+ LOGED(ERROR, domid, "socket() failed");
+ return ERROR_FAIL;
+ }
+
+ rc = libxl__prepare_sockaddr_un(gc, &un, path, "QEMU's QMP socket");
+ if (rc)
+ goto out;
+
+ rc = libxl__remove_file(gc, path);
+ if (rc)
+ goto out;
+
+ r = bind(fd, (struct sockaddr *) &un, sizeof(un));
+ if (r < 0) {
+ LOGED(ERROR, domid, "bind('%s') failed", path);
+ rc = ERROR_FAIL;
+ goto out;
+ }
+
+ r = listen(fd, 1);
+ if (r < 0) {
+ LOGED(ERROR, domid, "listen() failed");
+ rc = ERROR_FAIL;
+ goto out;
+ }
+
+ *fd_r = fd;
+ rc = 0;
+
+out:
+ if (rc && fd >= 0)
+ close(fd);
+ return rc;
+}
+
static int libxl__build_device_model_args_new(libxl__gc *gc,
const char *dm, int guest_domid,
const libxl_domain_config *guest_config,
@@ -1132,10 +1179,16 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
GCSPRINTF("%d", guest_domid), NULL);
flexarray_append(dm_args, "-chardev");
- flexarray_append(dm_args,
- GCSPRINTF("socket,id=libxl-cmd,"
- "path=%s,server,nowait",
- libxl__qemu_qmp_path(gc, guest_domid)));
+ if (state->dm_monitor_fd >= 0) {
+ flexarray_append(dm_args,
+ GCSPRINTF("socket,id=libxl-cmd,fd=%d,server,nowait",
+ state->dm_monitor_fd));
+ } else {
+ flexarray_append(dm_args,
+ GCSPRINTF("socket,id=libxl-cmd,"
+ "path=%s,server,nowait",
+ libxl__qemu_qmp_path(gc, guest_domid)));
+ }
flexarray_append(dm_args, "-no-shutdown");
flexarray_append(dm_args, "-mon");
@@ -2437,6 +2490,16 @@ void libxl__spawn_local_dm(libxl__egc *egc, libxl__dm_spawn_state *dmss)
if (rc)
goto out;
+ if (b_info->device_model_version
+ == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN &&
+ libxl_defbool_val(b_info->dm_restrict)) {
+ /* If we have to use dm_restrict, QEMU needs to be new enough
+ * and will have the new interface where we can pre-open the
+ * QMP socket. */
+ rc = libxl__pre_open_qmp_socket(gc, domid, &state->dm_monitor_fd);
+ if (rc) goto out;
+ }
+
rc = libxl__build_device_model_args(gc, dm, domid, guest_config,
&args, &envs, state,
&dm_state_fd);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 88c01ff118..83bc384d3e 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1240,6 +1240,7 @@ typedef struct {
char *console_tty;
char *saved_state;
+ int dm_monitor_fd;
libxl__file_reference pv_kernel;
libxl__file_reference pv_ramdisk;
--
Anthony PERARD
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v8 10/17] libxl: Add dmss_init/dispose for libxl__dm_spawn_state
2019-01-04 15:30 [PATCH v8 00/17] libxl: Enable save/restore/migration of a restricted QEMU + libxl__ev_qmp_* Anthony PERARD
` (8 preceding siblings ...)
2019-01-04 15:30 ` [PATCH v8 09/17] libxl_dm: Pre-open QMP socket for QEMU Anthony PERARD
@ 2019-01-04 15:30 ` Anthony PERARD
2019-01-04 15:30 ` [PATCH v8 11/17] libxl: QEMU startup sync based on QMP Anthony PERARD
` (6 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: Anthony PERARD @ 2019-01-04 15:30 UTC (permalink / raw)
To: xen-devel; +Cc: Anthony PERARD, Ian Jackson, Wei Liu
These two functions, dmss_init and dmss_dispose, need to be called to
initialise the private parts of a libxl__dm_spawn_state (dmss) as well
as dispose of them before giving back control to a caller.
There are 3 functions that can start using a dmss, the classic
libxl__spawn_local_dm, the one for stubdom libxl__spawn_stub_dm and
libxl__spawn_qdisk_backend. But there are only 2 exit path as
libxl__spawn_qdisk_backend is using libxl__spawn_local_dm functions.
These two new functions are empty but will be used shortly.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
Notes:
v8:
Acked
v7:
new patch
tools/libxl/libxl_dm.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index b9921eb885..4000f47545 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -2061,6 +2061,14 @@ static int libxl__write_stub_dmargs(libxl__gc *gc,
return 0;
}
+static void dmss_init(libxl__dm_spawn_state *dmss)
+{
+}
+
+static void dmss_dispose(libxl__gc *gc, libxl__dm_spawn_state *dmss)
+{
+}
+
static void spawn_stubdom_pvqemu_cb(libxl__egc *egc,
libxl__dm_spawn_state *stubdom_dmss,
int rc);
@@ -2099,6 +2107,7 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
libxl__domain_build_state *const stubdom_state = &sdss->dm_state;
libxl__domain_build_state_init(stubdom_state);
+ dmss_init(&sdss->dm);
if (guest_config->b_info.device_model_version !=
LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL) {
@@ -2430,6 +2439,7 @@ static void stubdom_xswait_cb(libxl__egc *egc, libxl__xswait_state *xswait,
out:
libxl__domain_build_state_dispose(&sdss->dm_state);
libxl__xswait_stop(gc, xswait);
+ dmss_dispose(gc, &sdss->dm);
sdss->callback(egc, &sdss->dm, rc);
}
@@ -2471,6 +2481,8 @@ void libxl__spawn_local_dm(libxl__egc *egc, libxl__dm_spawn_state *dmss)
const char *dm;
int dm_state_fd = -1;
+ dmss_init(dmss);
+
if (libxl_defbool_val(b_info->device_model_stubdomain)) {
abort();
}
@@ -2690,6 +2702,7 @@ static void device_model_spawn_outcome(libxl__egc *egc,
}
out:
+ dmss_dispose(gc, dmss);
dmss->callback(egc, dmss, rc);
}
@@ -2702,6 +2715,8 @@ void libxl__spawn_qdisk_backend(libxl__egc *egc, libxl__dm_spawn_state *dmss)
int logfile_w, null = -1, rc;
uint32_t domid = dmss->guest_domid;
+ dmss_init(dmss);
+
/* Always use qemu-xen as device model */
dm = qemu_xen_path(gc);
@@ -2766,6 +2781,7 @@ void libxl__spawn_qdisk_backend(libxl__egc *egc, libxl__dm_spawn_state *dmss)
rc = 0;
out:
+ dmss_dispose(gc, dmss);
if (logfile_w >= 0) close(logfile_w);
if (null >= 0) close(null);
/* callback on error only, success goes via dmss->spawn.*_cb */
--
Anthony PERARD
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v8 11/17] libxl: QEMU startup sync based on QMP
2019-01-04 15:30 [PATCH v8 00/17] libxl: Enable save/restore/migration of a restricted QEMU + libxl__ev_qmp_* Anthony PERARD
` (9 preceding siblings ...)
2019-01-04 15:30 ` [PATCH v8 10/17] libxl: Add dmss_init/dispose for libxl__dm_spawn_state Anthony PERARD
@ 2019-01-04 15:30 ` Anthony PERARD
2019-01-04 15:30 ` [PATCH v8 12/17] libxl_qmp: Store advertised QEMU version in libxl__ev_qmp Anthony PERARD
` (5 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: Anthony PERARD @ 2019-01-04 15:30 UTC (permalink / raw)
To: xen-devel; +Cc: Anthony PERARD, Ian Jackson, Wei Liu, Roger Pau Monné
This is only activated when dm_restrict=1, as explained in a previous
patch "libxl_dm: Pre-open QMP socket for QEMU"
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
Notes:
v8:
Acked
v7:
fixed _dispose call in device_model_spawn_outcome
move qmp field in libxl__dm_spawn_state into the private section
added description about the new QEMU_API error value
v6:
invent ERROR_QEMU_API
return better rc: ERROR_QEMU_API or ERROR_NOT_READY
enhance log messages (debug and error)
v5:
removed empty success branch in device_model_qmp_cb()
call libxl__ev_qmp_init() earlier in libxl__spawn_local_dm.
otherwise the error path would use an uninitialised
libxl__ev_qmp.
v4:
moved to libxl__dm_spawn_* from libxl__spawn_*
tools/libxl/libxl_dm.c | 53 ++++++++++++++++++++++++++++++++++++
tools/libxl/libxl_internal.h | 1 +
tools/libxl/libxl_types.idl | 1 +
3 files changed, 55 insertions(+)
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 4000f47545..20d811be03 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -2063,10 +2063,12 @@ static int libxl__write_stub_dmargs(libxl__gc *gc,
static void dmss_init(libxl__dm_spawn_state *dmss)
{
+ libxl__ev_qmp_init(&dmss->qmp);
}
static void dmss_dispose(libxl__gc *gc, libxl__dm_spawn_state *dmss)
{
+ libxl__ev_qmp_dispose(gc, &dmss->qmp);
}
static void spawn_stubdom_pvqemu_cb(libxl__egc *egc,
@@ -2451,6 +2453,9 @@ static void device_model_startup_failed(libxl__egc *egc,
int rc);
static void device_model_detached(libxl__egc *egc,
libxl__spawn_state *spawn);
+static void device_model_qmp_cb(libxl__egc *egc, libxl__ev_qmp *ev,
+ const libxl__json_object *response,
+ int rc);
/* our "next step" function, called from those callbacks and elsewhere */
static void device_model_spawn_outcome(libxl__egc *egc,
@@ -2605,6 +2610,17 @@ void libxl__spawn_local_dm(libxl__egc *egc, libxl__dm_spawn_state *dmss)
spawn->failure_cb = device_model_startup_failed;
spawn->detached_cb = device_model_detached;
+ if (state->dm_monitor_fd >= 0) {
+ /* There is a valid QMP socket available now,
+ * use it to find out when QEMU is ready */
+ dmss->qmp.ao = ao;
+ dmss->qmp.callback = device_model_qmp_cb;
+ dmss->qmp.domid = domid;
+ dmss->qmp.payload_fd = -1;
+ rc = libxl__ev_qmp_send(gc, &dmss->qmp, "query-status", NULL);
+ if (rc) goto out_close;
+ }
+
rc = libxl__spawn_spawn(egc, spawn);
if (rc < 0)
goto out_close;
@@ -2678,6 +2694,43 @@ static void device_model_detached(libxl__egc *egc,
device_model_spawn_outcome(egc, dmss, 0);
}
+static void device_model_qmp_cb(libxl__egc *egc, libxl__ev_qmp *ev,
+ const libxl__json_object *response,
+ int rc)
+{
+ EGC_GC;
+ libxl__dm_spawn_state *dmss = CONTAINER_OF(ev, *dmss, qmp);
+ const libxl__json_object *o;
+ const char *status;
+
+ libxl__ev_qmp_dispose(gc, ev);
+
+ if (rc)
+ goto failed;
+
+ o = libxl__json_map_get("status", response, JSON_STRING);
+ if (!o) {
+ LOGD(ERROR, ev->domid,
+ "Missing 'status' in response to 'query-status'");
+ LOGD(DEBUG, ev->domid, ".. instead, got: %s", JSON(response));
+ rc = ERROR_QEMU_API;
+ goto failed;
+ }
+ status = libxl__json_object_get_string(o);
+ if (strcmp(status, "running")) {
+ LOGD(ERROR, ev->domid, "Unexpected QEMU status: %s", status);
+ rc = ERROR_NOT_READY;
+ goto failed;
+ }
+
+ libxl__spawn_initiate_detach(gc, &dmss->spawn);
+ return;
+
+failed:
+ LOGD(ERROR, ev->domid, "QEMU did not start properly, rc=%d", rc);
+ libxl__spawn_initiate_failure(egc, &dmss->spawn, rc);
+}
+
static void device_model_spawn_outcome(libxl__egc *egc,
libxl__dm_spawn_state *dmss,
int rc)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 83bc384d3e..87a58b3ac3 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3934,6 +3934,7 @@ typedef void libxl__dm_spawn_cb(libxl__egc *egc, libxl__dm_spawn_state*,
struct libxl__dm_spawn_state {
/* mixed - spawn.ao must be initialised by user; rest is private: */
libxl__spawn_state spawn;
+ libxl__ev_qmp qmp;
/* filled in by user, must remain valid: */
uint32_t guest_domid; /* domain being served */
libxl_domain_config *guest_config;
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 212b00a677..b685ac47ac 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -75,6 +75,7 @@ libxl_error = Enumeration("error", [
(-29, "QMP_COMMAND_NOT_FOUND"), # the requested command has not been found
(-30, "QMP_DEVICE_NOT_ACTIVE"), # a device has failed to be become active
(-31, "QMP_DEVICE_NOT_FOUND"), # the requested device has not been found
+ (-32, "QEMU_API"), # QEMU's replies don't contains expected members
], value_namespace = "")
libxl_domain_type = Enumeration("domain_type", [
--
Anthony PERARD
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v8 12/17] libxl_qmp: Store advertised QEMU version in libxl__ev_qmp
2019-01-04 15:30 [PATCH v8 00/17] libxl: Enable save/restore/migration of a restricted QEMU + libxl__ev_qmp_* Anthony PERARD
` (10 preceding siblings ...)
2019-01-04 15:30 ` [PATCH v8 11/17] libxl: QEMU startup sync based on QMP Anthony PERARD
@ 2019-01-04 15:30 ` Anthony PERARD
2019-01-04 15:30 ` [PATCH v8 13/17] libxl: Change libxl__domain_suspend_device_model() to be async Anthony PERARD
` (4 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: Anthony PERARD @ 2019-01-04 15:30 UTC (permalink / raw)
To: xen-devel; +Cc: Anthony PERARD, Ian Jackson, Wei Liu
This will be used in a later patch.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
Notes:
v8:
Acked
v7:
Add do{}while around the MACRO.
formating nits changes.
v6:
new local macro GRAB_VERSION
better definition of qemu_version field in libxl_internal.h
v5:
initialise qemu_version struct in libxl__ev_qmp_init
tools/libxl/libxl_internal.h | 8 ++++++++
tools/libxl/libxl_qmp.c | 25 +++++++++++++++++++++++++
2 files changed, 33 insertions(+)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 87a58b3ac3..62492e13d7 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -439,6 +439,14 @@ struct libxl__ev_qmp {
libxl__ev_qmp_callback *callback;
int payload_fd; /* set to send a fd with the command, -1 otherwise */
+ /* read-only when Connected
+ * and not to be accessed by the caller otherwise */
+ struct {
+ int major;
+ int minor;
+ int micro;
+ } qemu_version;
+
/*
* remaining fields are private to libxl_ev_qmp_*
*/
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index f2ab08620a..9b4a07d622 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -1829,6 +1829,27 @@ static int qmp_ev_handle_message(libxl__egc *egc,
return ERROR_PROTOCOL_ERROR_QMP;
}
+ /*
+ * Store advertised QEMU version
+ * { "QMP": { "version": {
+ * "qemu": { "major": int, "minor": int, "micro": int } } } }
+ */
+ o = libxl__json_map_get("QMP", resp, JSON_MAP);
+ o = libxl__json_map_get("version", o, JSON_MAP);
+ o = libxl__json_map_get("qemu", o, JSON_MAP);
+#define GRAB_VERSION(level) do { \
+ ev->qemu_version.level = libxl__json_object_get_integer( \
+ libxl__json_map_get(#level, o, JSON_INTEGER)); \
+ } while (0)
+ GRAB_VERSION(major);
+ GRAB_VERSION(minor);
+ GRAB_VERSION(micro);
+#undef GRAB_VERSION
+ LOGD(DEBUG, ev->domid, "QEMU version: %d.%d.%d",
+ ev->qemu_version.major,
+ ev->qemu_version.minor,
+ ev->qemu_version.micro);
+
/* Prepare next message to send */
assert(!ev->tx_buf);
ev->id = ev->next_id++;
@@ -1991,6 +2012,10 @@ void libxl__ev_qmp_init(libxl__ev_qmp *ev)
ev->msg = NULL;
ev->msg_id = 0;
+
+ ev->qemu_version.major = -1;
+ ev->qemu_version.minor = -1;
+ ev->qemu_version.micro = -1;
}
int libxl__ev_qmp_send(libxl__gc *unused_gc, libxl__ev_qmp *ev,
--
Anthony PERARD
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v8 13/17] libxl: Change libxl__domain_suspend_device_model() to be async
2019-01-04 15:30 [PATCH v8 00/17] libxl: Enable save/restore/migration of a restricted QEMU + libxl__ev_qmp_* Anthony PERARD
` (11 preceding siblings ...)
2019-01-04 15:30 ` [PATCH v8 12/17] libxl_qmp: Store advertised QEMU version in libxl__ev_qmp Anthony PERARD
@ 2019-01-04 15:30 ` Anthony PERARD
2019-01-04 15:30 ` [PATCH v8 14/17] libxl: Re-implement domain_suspend_device_model using libxl__ev_qmp Anthony PERARD
` (3 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: Anthony PERARD @ 2019-01-04 15:30 UTC (permalink / raw)
To: xen-devel; +Cc: Anthony PERARD, Ian Jackson, Wei Liu
This create an extra step for the two call sites of the function.
libxl__domain_suspend_device_model() in this patch gets an extra error
variable (there is ret and rc), but ret goes away in the next patch.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
libxl_domain_soft_reset() haven't been tested, as it doesn't appear to
possible to call the function from xl.
---
Notes:
v8:
Acked
v6:
fix multiple way to report errors,
libxl__domain_suspend_device_model will now only report via
callbacks, and return void
add rc in libxl__domain_suspend_device_model (ret isn't a proper rc
as libxl__qmp_save don't return one)
tools/libxl/libxl_create.c | 35 +++++++++++++++++++++-----------
tools/libxl/libxl_dom_suspend.c | 36 ++++++++++++++++++++-------------
tools/libxl/libxl_internal.h | 7 +++++--
3 files changed, 50 insertions(+), 28 deletions(-)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 89f4766a27..a4e74a5cd2 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1774,6 +1774,9 @@ static void domain_soft_reset_cb(libxl__egc *egc,
domcreate_complete(egc, &cdcs->dcs, rc);
}
+static void soft_reset_dm_suspended(libxl__egc *egc,
+ libxl__domain_suspend_state *dsps,
+ int rc);
static int do_domain_soft_reset(libxl_ctx *ctx,
libxl_domain_config *d_config,
uint32_t domid_soft_reset,
@@ -1856,11 +1859,24 @@ static int do_domain_soft_reset(libxl_ctx *ctx,
goto out;
}
- rc = libxl__domain_suspend_device_model(gc, &dss->dsps);
- if (rc) {
- LOGD(ERROR, domid_soft_reset, "failed to suspend device model.");
- goto out;
- }
+ dss->dsps.ao = ao;
+ dss->dsps.callback_device_model_done = soft_reset_dm_suspended;
+ libxl__domain_suspend_device_model(egc, &dss->dsps); /* must be last */
+
+ return AO_INPROGRESS;
+
+ out:
+ return AO_CREATE_FAIL(rc);
+}
+
+static void soft_reset_dm_suspended(libxl__egc *egc,
+ libxl__domain_suspend_state *dsps,
+ int rc)
+{
+ STATE_AO_GC(dsps->ao);
+ libxl__domain_soft_reset_state *srs =
+ CONTAINER_OF(dsps, *srs, dss.dsps);
+ libxl__app_domain_create_state *cdcs = &srs->cdcs;
/*
* Ask all backends to disconnect by removing the domain from
@@ -1868,18 +1884,13 @@ static int do_domain_soft_reset(libxl_ctx *ctx,
* xenstore again with probably different store/console/...
* channels.
*/
- xs_release_domain(ctx->xsh, cdcs->dcs.domid_soft_reset);
+ xs_release_domain(CTX->xsh, cdcs->dcs.domid_soft_reset);
srs->dds.ao = ao;
- srs->dds.domid = domid_soft_reset;
+ srs->dds.domid = cdcs->dcs.domid_soft_reset;
srs->dds.callback = domain_soft_reset_cb;
srs->dds.soft_reset = true;
libxl__domain_destroy(egc, &srs->dds);
-
- return AO_INPROGRESS;
-
- out:
- return AO_CREATE_FAIL(rc);
}
static void domain_create_cb(libxl__egc *egc,
diff --git a/tools/libxl/libxl_dom_suspend.c b/tools/libxl/libxl_dom_suspend.c
index 1e904bae8a..f8ff5cf0c5 100644
--- a/tools/libxl/libxl_dom_suspend.c
+++ b/tools/libxl/libxl_dom_suspend.c
@@ -68,10 +68,12 @@ int libxl__domain_suspend_init(libxl__egc *egc,
/*----- callbacks, called by xc_domain_save -----*/
-int libxl__domain_suspend_device_model(libxl__gc *gc,
+void libxl__domain_suspend_device_model(libxl__egc *egc,
libxl__domain_suspend_state *dsps)
{
+ STATE_AO_GC(dsps->ao);
int ret = 0;
+ int rc = 0;
uint32_t const domid = dsps->domid;
const char *const filename = dsps->dm_savefile;
@@ -83,18 +85,29 @@ int libxl__domain_suspend_device_model(libxl__gc *gc,
break;
}
case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
- if (libxl__qmp_stop(gc, domid))
- return ERROR_FAIL;
+ ret = libxl__qmp_stop(gc, domid);
+ if (ret) {
+ rc = ERROR_FAIL;
+ goto out;
+ }
/* Save DM state into filename */
ret = libxl__qmp_save(gc, domid, filename, dsps->live);
- if (ret)
+ if (ret) {
+ rc = ERROR_FAIL;
unlink(filename);
+ goto out;
+ }
break;
default:
- return ERROR_INVAL;
+ rc = ERROR_INVAL;
+ goto out;
}
- return ret;
+out:
+ if (rc)
+ LOGD(ERROR, dsps->domid,
+ "failed to suspend device model, rc=%d", rc);
+ dsps->callback_device_model_done(egc, dsps, rc); /* must be last */
}
static void domain_suspend_common_wait_guest(libxl__egc *egc,
@@ -371,20 +384,15 @@ static void domain_suspend_common_guest_suspended(libxl__egc *egc,
libxl__domain_suspend_state *dsps)
{
STATE_AO_GC(dsps->ao);
- int rc;
libxl__ev_evtchn_cancel(gc, &dsps->guest_evtchn);
libxl__ev_xswatch_deregister(gc, &dsps->guest_watch);
libxl__ev_time_deregister(gc, &dsps->guest_timeout);
if (dsps->type == LIBXL_DOMAIN_TYPE_HVM) {
- rc = libxl__domain_suspend_device_model(gc, dsps);
- if (rc) {
- LOGD(ERROR, dsps->domid,
- "libxl__domain_suspend_device_model failed ret=%d", rc);
- domain_suspend_common_done(egc, dsps, rc);
- return;
- }
+ dsps->callback_device_model_done = domain_suspend_common_done;
+ libxl__domain_suspend_device_model(egc, dsps); /* must be last */
+ return;
}
domain_suspend_common_done(egc, dsps, 0);
}
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 62492e13d7..5a18937562 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3435,6 +3435,8 @@ struct libxl__domain_suspend_state {
libxl__ev_time guest_timeout;
const char *dm_savefile;
+ void (*callback_device_model_done)(libxl__egc*,
+ struct libxl__domain_suspend_state*, int rc);
void (*callback_common_done)(libxl__egc*,
struct libxl__domain_suspend_state*, int ok);
};
@@ -4076,8 +4078,9 @@ static inline bool libxl__save_helper_inuse(const libxl__save_helper_state *shs)
return libxl__ev_child_inuse(&shs->child);
}
-/* Each time the dm needs to be saved, we must call suspend and then save */
-_hidden int libxl__domain_suspend_device_model(libxl__gc *gc,
+/* Each time the dm needs to be saved, we must call suspend and then save
+ * calls dsps->callback_device_model_done when done */
+_hidden void libxl__domain_suspend_device_model(libxl__egc *egc,
libxl__domain_suspend_state *dsps);
_hidden const char *libxl__device_model_savefile(libxl__gc *gc, uint32_t domid);
--
Anthony PERARD
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v8 14/17] libxl: Re-implement domain_suspend_device_model using libxl__ev_qmp
2019-01-04 15:30 [PATCH v8 00/17] libxl: Enable save/restore/migration of a restricted QEMU + libxl__ev_qmp_* Anthony PERARD
` (12 preceding siblings ...)
2019-01-04 15:30 ` [PATCH v8 13/17] libxl: Change libxl__domain_suspend_device_model() to be async Anthony PERARD
@ 2019-01-04 15:30 ` Anthony PERARD
2019-01-04 15:30 ` [PATCH v8 15/17] libxl: Remove unused arg from libxl__sendmsg_fds Anthony PERARD
` (2 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: Anthony PERARD @ 2019-01-04 15:30 UTC (permalink / raw)
To: xen-devel; +Cc: Anthony PERARD, Ian Jackson, Wei Liu
The re-implementation is done because we want to be able to send the
file description that QEMU can use to save its state. When QEMU is
restricted, it would not be able to write to a path.
This replace both libxl__qmp_stop() and libxl__qmp_save().
qmp_qemu_check_version() was only used by libxl__qmp_save(), so it is
replace by a version using libxl__ev_qmp instead.
Coding style fixed in libxl__domain_suspend_device_model() for the
return value.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
Notes:
v8:
Acked
v7:
use libxl__remove_file instead of unlink.
comment that libxl__qmp_suspend_save can callback synchronously
v6:
extract open call from libxl__carefd_opened to respect coding style
libxl__qmp_suspend_save now always report success/error
via dsps->callback_device_model_done
v5:
rename goto 'out' label to 'error', as it is use only for errors.
re-add/keep the comment about the "live" parameter in dm_state_fd_ready
use libxl__remove_file instead of plain unlink
v4:
This patch replace the patch "libxl_qmp: Have QEMU save its state to a file
descriptor" from previous version of the serie.
It uses libxl__ev_qmp instead.
tools/libxl/libxl_dom_suspend.c | 19 +---
tools/libxl/libxl_internal.h | 11 ++-
tools/libxl/libxl_qmp.c | 160 +++++++++++++++++++++++++-------
3 files changed, 139 insertions(+), 51 deletions(-)
diff --git a/tools/libxl/libxl_dom_suspend.c b/tools/libxl/libxl_dom_suspend.c
index f8ff5cf0c5..d1af3a6573 100644
--- a/tools/libxl/libxl_dom_suspend.c
+++ b/tools/libxl/libxl_dom_suspend.c
@@ -34,6 +34,7 @@ int libxl__domain_suspend_init(libxl__egc *egc,
libxl__ev_evtchn_init(&dsps->guest_evtchn);
libxl__ev_xswatch_init(&dsps->guest_watch);
libxl__ev_time_init(&dsps->guest_timeout);
+ libxl__ev_qmp_init(&dsps->qmp);
if (type == LIBXL_DOMAIN_TYPE_INVALID) goto out;
dsps->type = type;
@@ -72,7 +73,6 @@ void libxl__domain_suspend_device_model(libxl__egc *egc,
libxl__domain_suspend_state *dsps)
{
STATE_AO_GC(dsps->ao);
- int ret = 0;
int rc = 0;
uint32_t const domid = dsps->domid;
const char *const filename = dsps->dm_savefile;
@@ -85,19 +85,9 @@ void libxl__domain_suspend_device_model(libxl__egc *egc,
break;
}
case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
- ret = libxl__qmp_stop(gc, domid);
- if (ret) {
- rc = ERROR_FAIL;
- goto out;
- }
- /* Save DM state into filename */
- ret = libxl__qmp_save(gc, domid, filename, dsps->live);
- if (ret) {
- rc = ERROR_FAIL;
- unlink(filename);
- goto out;
- }
- break;
+ /* calls dsps->callback_device_model_done when done */
+ libxl__qmp_suspend_save(egc, dsps); /* must be last */
+ return;
default:
rc = ERROR_INVAL;
goto out;
@@ -406,6 +396,7 @@ static void domain_suspend_common_done(libxl__egc *egc,
libxl__ev_evtchn_cancel(gc, &dsps->guest_evtchn);
libxl__ev_xswatch_deregister(gc, &dsps->guest_watch);
libxl__ev_time_deregister(gc, &dsps->guest_timeout);
+ libxl__ev_qmp_dispose(gc, &dsps->qmp);
dsps->callback_common_done(egc, dsps, rc);
}
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 5a18937562..77a8cd5aa5 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1963,13 +1963,8 @@ _hidden int libxl__qmp_pci_del(libxl__gc *gc, int domid,
libxl_device_pci *pcidev);
/* Resume hvm domain */
_hidden int libxl__qmp_system_wakeup(libxl__gc *gc, int domid);
-/* Suspend QEMU. */
-_hidden int libxl__qmp_stop(libxl__gc *gc, int domid);
/* Resume QEMU. */
_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,
- bool live);
/* Load current QEMU state from file. */
_hidden int libxl__qmp_restore(libxl__gc *gc, int domid, const char *filename);
/* Set dirty bitmap logging status */
@@ -3433,6 +3428,7 @@ struct libxl__domain_suspend_state {
libxl__xswait_state pvcontrol;
libxl__ev_xswatch guest_watch;
libxl__ev_time guest_timeout;
+ libxl__ev_qmp qmp;
const char *dm_savefile;
void (*callback_device_model_done)(libxl__egc*,
@@ -3444,6 +3440,11 @@ int libxl__domain_suspend_init(libxl__egc *egc,
libxl__domain_suspend_state *dsps,
libxl_domain_type type);
+/* calls dsps->callback_device_model_done when done
+ * may synchronously calls this callback */
+_hidden void libxl__qmp_suspend_save(libxl__egc *egc,
+ libxl__domain_suspend_state *dsps);
+
struct libxl__domain_save_state {
/* set by caller of libxl__domain_save */
libxl__ao *ao;
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index 9b4a07d622..02ba156df9 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -405,12 +405,12 @@ static int qmp_handle_response(libxl__gc *gc, libxl__qmp_handler *qmp,
* = 0 if qemu's version == asked version
* > 0 if qemu's version > asked version
*/
-static int qmp_qemu_compare_version(libxl__qmp_handler *qmp, int major,
- int minor, int micro)
+static int qmp_ev_qemu_compare_version(libxl__ev_qmp *ev, int major,
+ int minor, int micro)
{
#define CHECK_VERSION(level) do { \
- if (qmp->version.level > (level)) return +1; \
- if (qmp->version.level < (level)) return -1; \
+ if (ev->qemu_version.level > (level)) return +1; \
+ if (ev->qemu_version.level < (level)) return -1; \
} while (0)
CHECK_VERSION(major);
@@ -1019,29 +1019,6 @@ int libxl__qmp_system_wakeup(libxl__gc *gc, int domid)
return qmp_run_command(gc, domid, "system_wakeup", NULL, NULL, NULL);
}
-int libxl__qmp_save(libxl__gc *gc, int domid, const char *filename, bool live)
-{
- libxl__json_object *args = NULL;
- libxl__qmp_handler *qmp = NULL;
- int rc;
-
- qmp = libxl__qmp_initialize(gc, domid);
- if (!qmp)
- return ERROR_FAIL;
-
- qmp_parameters_add_string(gc, &args, "filename", (char *)filename);
-
- /* live parameter was added to QEMU 2.11. It signal QEMU that the save
- * operation is for a live migration rather that for taking a snapshot. */
- if (qmp_qemu_compare_version(qmp, 2, 11, 0) >= 0)
- qmp_parameters_add_bool(gc, &args, "live", live);
-
- rc = qmp_synchronous_send(qmp, "xen-save-devices-state", args,
- NULL, NULL, qmp->timeout);
- libxl__qmp_close(qmp);
- return rc;
-}
-
int libxl__qmp_restore(libxl__gc *gc, int domid, const char *state_file)
{
libxl__json_object *args = NULL;
@@ -1070,11 +1047,6 @@ static int qmp_change(libxl__gc *gc, libxl__qmp_handler *qmp,
return rc;
}
-int libxl__qmp_stop(libxl__gc *gc, int domid)
-{
- return qmp_run_command(gc, domid, "stop", NULL, NULL, NULL);
-}
-
int libxl__qmp_resume(libxl__gc *gc, int domid)
{
return qmp_run_command(gc, domid, "cont", NULL, NULL, NULL);
@@ -1314,6 +1286,130 @@ int libxl__qmp_initializations(libxl__gc *gc, uint32_t domid,
return ret;
}
+
+/*
+ * Functions using libxl__ev_qmp
+ */
+
+static void dm_stopped(libxl__egc *egc, libxl__ev_qmp *ev,
+ const libxl__json_object *response, int rc);
+static void dm_state_fd_ready(libxl__egc *egc, libxl__ev_qmp *ev,
+ const libxl__json_object *response, int rc);
+static void dm_state_saved(libxl__egc *egc, libxl__ev_qmp *ev,
+ const libxl__json_object *response, int rc);
+
+/* calls dsps->callback_device_model_done when done */
+void libxl__qmp_suspend_save(libxl__egc *egc,
+ libxl__domain_suspend_state *dsps)
+{
+ EGC_GC;
+ int rc;
+ libxl__ev_qmp *ev = &dsps->qmp;
+
+ ev->ao = dsps->ao;
+ ev->domid = dsps->domid;
+ ev->callback = dm_stopped;
+ ev->payload_fd = -1;
+
+ rc = libxl__ev_qmp_send(gc, ev, "stop", NULL);
+ if (rc)
+ goto error;
+
+ return;
+
+error:
+ dsps->callback_device_model_done(egc, dsps, rc);
+}
+
+static void dm_stopped(libxl__egc *egc, libxl__ev_qmp *ev,
+ const libxl__json_object *response, int rc)
+{
+ EGC_GC;
+ libxl__domain_suspend_state *dsps = CONTAINER_OF(ev, *dsps, qmp);
+ const char *const filename = dsps->dm_savefile;
+
+ if (rc)
+ goto error;
+
+ ev->payload_fd = open(filename, O_WRONLY | O_CREAT, 0600);
+ if (ev->payload_fd < 0) {
+ LOGED(ERROR, ev->domid,
+ "Failed to open file %s for QEMU", filename);
+ rc = ERROR_FAIL;
+ goto error;
+ }
+
+ ev->callback = dm_state_fd_ready;
+ rc = libxl__ev_qmp_send(gc, ev, "add-fd", NULL);
+ if (rc)
+ goto error;
+
+ return;
+
+error:
+ if (ev->payload_fd >= 0) {
+ close(ev->payload_fd);
+ libxl__remove_file(gc, filename);
+ ev->payload_fd = -1;
+ }
+ dsps->callback_device_model_done(egc, dsps, rc);
+}
+
+static void dm_state_fd_ready(libxl__egc *egc, libxl__ev_qmp *ev,
+ const libxl__json_object *response, int rc)
+{
+ EGC_GC;
+ int fdset;
+ const libxl__json_object *o;
+ libxl__json_object *args = NULL;
+ libxl__domain_suspend_state *dsps = CONTAINER_OF(ev, *dsps, qmp);
+
+ close(ev->payload_fd);
+ ev->payload_fd = -1;
+
+ if (rc)
+ goto error;
+
+ o = libxl__json_map_get("fdset-id", response, JSON_INTEGER);
+ if (!o) {
+ rc = ERROR_QEMU_API;
+ goto error;
+ }
+ fdset = libxl__json_object_get_integer(o);
+
+ ev->callback = dm_state_saved;
+
+ /* The `live` parameter was added to QEMU 2.11. It signals QEMU that
+ * the save operation is for a live migration rather than for taking a
+ * snapshot. */
+ if (qmp_ev_qemu_compare_version(ev, 2, 11, 0) >= 0)
+ qmp_parameters_add_bool(gc, &args, "live", dsps->live);
+ QMP_PARAMETERS_SPRINTF(&args, "filename", "/dev/fdset/%d", fdset);
+ rc = libxl__ev_qmp_send(gc, ev, "xen-save-devices-state", args);
+ if (rc)
+ goto error;
+
+ return;
+
+error:
+ assert(rc);
+ libxl__remove_file(gc, dsps->dm_savefile);
+ dsps->callback_device_model_done(egc, dsps, rc);
+}
+
+static void dm_state_saved(libxl__egc *egc, libxl__ev_qmp *ev,
+ const libxl__json_object *response, int rc)
+{
+ EGC_GC;
+ libxl__domain_suspend_state *dsps = CONTAINER_OF(ev, *dsps, qmp);
+
+ if (rc)
+ libxl__remove_file(gc, dsps->dm_savefile);
+
+ dsps->callback_device_model_done(egc, dsps, rc);
+}
+
+
/* ------------ Implementation of libxl__ev_qmp ---------------- */
/*
--
Anthony PERARD
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v8 15/17] libxl: Remove unused arg from libxl__sendmsg_fds
2019-01-04 15:30 [PATCH v8 00/17] libxl: Enable save/restore/migration of a restricted QEMU + libxl__ev_qmp_* Anthony PERARD
` (13 preceding siblings ...)
2019-01-04 15:30 ` [PATCH v8 14/17] libxl: Re-implement domain_suspend_device_model using libxl__ev_qmp Anthony PERARD
@ 2019-01-04 15:30 ` Anthony PERARD
2019-01-04 16:04 ` Ian Jackson
2019-01-04 15:30 ` [PATCH v8 16/17] libxl_json: Remove libxl__json_object_append_to from header Anthony PERARD
2019-01-04 15:30 ` [PATCH v8 17/17] libxl: Add comments to libxl__json_*get* functions Anthony PERARD
16 siblings, 1 reply; 24+ messages in thread
From: Anthony PERARD @ 2019-01-04 15:30 UTC (permalink / raw)
To: xen-devel; +Cc: Anthony PERARD, Ian Jackson, Wei Liu, Anthony PERARD
From: Anthony PERARD <anthony.perard@gmail.com>
Now that `datalen' needs to be 1, we can remove it. Also change `data'
parameter to be a singe byte.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
Notes:
v8:
new patch, just a follow-up
tools/libxl/libxl_aoutils.c | 2 +-
tools/libxl/libxl_internal.h | 4 ++--
tools/libxl/libxl_qmp.c | 2 +-
tools/libxl/libxl_utils.c | 7 +++----
4 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c
index 9e493cd487..e24e4eed53 100644
--- a/tools/libxl/libxl_aoutils.c
+++ b/tools/libxl/libxl_aoutils.c
@@ -477,7 +477,7 @@ int libxl__openptys(libxl__openpty_state *op,
r = openpty(&ptyfds[i][0], &ptyfds[i][1], NULL, termp, winp);
if (r) { LOGE(ERROR,"openpty failed"); _exit(-1); }
}
- rc = libxl__sendmsg_fds(gc, sockets[1], "",1,
+ rc = libxl__sendmsg_fds(gc, sockets[1], '\0',
2*count, &ptyfds[0][0], "ptys");
if (rc) { LOGE(ERROR,"sendmsg to parent failed"); _exit(-1); }
_exit(0);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 77a8cd5aa5..c4968ca135 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2010,12 +2010,12 @@ _hidden void libxl__qmp_cleanup(libxl__gc *gc, uint32_t domid);
_hidden int libxl__qmp_initializations(libxl__gc *gc, uint32_t domid,
const libxl_domain_config *guest_config);
-/* `datalen` should be 1 byte
+/* `data' should contain a byte to send.
* When dealing with a non-blocking fd, it returns
* ERROR_NOT_READY on EWOULDBLOCK
* logs on other failures. */
int libxl__sendmsg_fds(libxl__gc *gc, int carrier,
- const void *data, size_t datalen,
+ const char data,
int nfds, const int fds[], const char *what);
/* Insists on receiving exactly nfds and datalen. On failure, logs
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index 02ba156df9..131d94fb29 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -1725,7 +1725,7 @@ static int qmp_ev_callback_writable(libxl__gc *gc,
ev->payload_fd >= 0 &&
ev->tx_buf_off == 0) {
- rc = libxl__sendmsg_fds(gc, fd, ev->tx_buf, 1,
+ rc = libxl__sendmsg_fds(gc, fd, ev->tx_buf[ev->tx_buf_off],
1, &ev->payload_fd, "QMP socket");
/* Check for EWOULDBLOCK, and return to try again later */
if (rc == ERROR_NOT_READY)
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index d53db8c37d..e123baa632 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -1061,18 +1061,17 @@ void libxl_vcpuinfo_list_free(libxl_vcpuinfo *list, int nr)
}
int libxl__sendmsg_fds(libxl__gc *gc, int carrier,
- const void *data, size_t datalen,
+ const char data,
int nfds, const int fds[], const char *what) {
struct msghdr msg = { 0 };
struct cmsghdr *cmsg;
size_t spaceneeded = nfds * sizeof(fds[0]);
char control[CMSG_SPACE(spaceneeded)];
+ const size_t datalen = 1;
struct iovec iov;
int r;
- assert(datalen == 1);
-
- iov.iov_base = (void*)data;
+ iov.iov_base = (void*)&data;
iov.iov_len = datalen;
/* compose the message */
--
Anthony PERARD
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v8 16/17] libxl_json: Remove libxl__json_object_append_to from header
2019-01-04 15:30 [PATCH v8 00/17] libxl: Enable save/restore/migration of a restricted QEMU + libxl__ev_qmp_* Anthony PERARD
` (14 preceding siblings ...)
2019-01-04 15:30 ` [PATCH v8 15/17] libxl: Remove unused arg from libxl__sendmsg_fds Anthony PERARD
@ 2019-01-04 15:30 ` Anthony PERARD
2019-01-04 16:03 ` Ian Jackson
2019-01-04 15:30 ` [PATCH v8 17/17] libxl: Add comments to libxl__json_*get* functions Anthony PERARD
16 siblings, 1 reply; 24+ messages in thread
From: Anthony PERARD @ 2019-01-04 15:30 UTC (permalink / raw)
To: xen-devel; +Cc: Anthony PERARD, Ian Jackson, Wei Liu
It isn't possible to use libxl__json_object_append_to() outside of
libxl_json.c as there is no way to allocate a struct libxl__yajl_ctx.
So also remove libxl__yajl_ctx typedef from the internal header.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
Notes:
v8:
New follow-up patch
tools/libxl/libxl_internal.h | 5 -----
tools/libxl/libxl_json.c | 9 +++++----
2 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index c4968ca135..f1e5c2c980 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2071,8 +2071,6 @@ typedef struct {
libxl__json_object *obj;
} libxl__json_map_node;
-typedef struct libxl__yajl_ctx libxl__yajl_ctx;
-
static inline bool libxl__json_object_is_null(const libxl__json_object *o)
{
return o != NULL && o->type == JSON_NULL;
@@ -2159,9 +2157,6 @@ static inline long long libxl__json_object_get_integer(const libxl__json_object
*/
_hidden libxl__json_object *libxl__json_object_alloc(libxl__gc *gc_opt,
libxl__json_node_type type);
-_hidden int libxl__json_object_append_to(libxl__gc *gc_opt,
- libxl__json_object *obj,
- libxl__yajl_ctx *ctx);
_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 cd78313df2..9b8ef2cab9 100644
--- a/tools/libxl/libxl_json.c
+++ b/tools/libxl/libxl_json.c
@@ -23,7 +23,7 @@
/* #define DEBUG_ANSWER */
-struct libxl__yajl_ctx {
+typedef struct libxl__yajl_ctx {
libxl__gc *gc;
yajl_handle hand;
libxl__json_object *head;
@@ -31,7 +31,7 @@ struct libxl__yajl_ctx {
#ifdef DEBUG_ANSWER
yajl_gen g;
#endif
-};
+} libxl__yajl_ctx;
#ifdef DEBUG_ANSWER
#if YAJL_VERSION < 20000
@@ -463,8 +463,9 @@ libxl__json_object *libxl__json_object_alloc(libxl__gc *gc,
return obj;
}
-int libxl__json_object_append_to(libxl__gc *gc, libxl__json_object *obj,
- libxl__yajl_ctx *ctx)
+static int libxl__json_object_append_to(libxl__gc *gc,
+ libxl__json_object *obj,
+ libxl__yajl_ctx *ctx)
{
libxl__json_object *dst = ctx->current;
--
Anthony PERARD
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v8 17/17] libxl: Add comments to libxl__json_*get* functions
2019-01-04 15:30 [PATCH v8 00/17] libxl: Enable save/restore/migration of a restricted QEMU + libxl__ev_qmp_* Anthony PERARD
` (15 preceding siblings ...)
2019-01-04 15:30 ` [PATCH v8 16/17] libxl_json: Remove libxl__json_object_append_to from header Anthony PERARD
@ 2019-01-04 15:30 ` Anthony PERARD
2019-01-11 11:55 ` Ian Jackson
16 siblings, 1 reply; 24+ messages in thread
From: Anthony PERARD @ 2019-01-04 15:30 UTC (permalink / raw)
To: xen-devel; +Cc: Anthony PERARD, Ian Jackson, Wei Liu
This comments that libxl__json_object_get_* and libxl__json_*_get
functions accept the libxl__json_object parameter to be NULL.
libxl__json_object_to_json also works with NULL.
This also move libxl__json_object_alloc declaration closer to similar
functions, and closer to libxl__json_object_free.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
Notes:
v8:
New follow-up patch
tools/libxl/libxl_internal.h | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index f1e5c2c980..8674b3ed47 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2104,6 +2104,10 @@ static inline bool libxl__json_object_is_array(const libxl__json_object *o)
return o != NULL && o->type == JSON_ARRAY;
}
+/*
+ * `o` may be NULL for all libxl__json_object_get_* functions.
+ */
+
static inline bool libxl__json_object_get_bool(const libxl__json_object *o)
{
if (libxl__json_object_is_bool(o))
@@ -2152,11 +2156,9 @@ static inline long long libxl__json_object_get_integer(const libxl__json_object
}
/*
- * NOGC can be used with those json_object functions, but the
- * libxl__json_object* will need to be freed with libxl__json_object_free.
+ * `o` may be NULL for the following libxl__json_*_get functions.
*/
-_hidden libxl__json_object *libxl__json_object_alloc(libxl__gc *gc_opt,
- libxl__json_node_type type);
+
_hidden libxl__json_object *libxl__json_array_get(const libxl__json_object *o,
int i);
_hidden
@@ -2165,6 +2167,13 @@ 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);
+
+/*
+ * NOGC can be used with those json_object functions, but the
+ * libxl__json_object* will need to be freed with libxl__json_object_free.
+ */
+_hidden libxl__json_object *libxl__json_object_alloc(libxl__gc *gc_opt,
+ libxl__json_node_type type);
_hidden yajl_status libxl__json_object_to_yajl_gen(libxl__gc *gc_opt,
yajl_gen hand,
const libxl__json_object *param);
@@ -2173,6 +2182,7 @@ _hidden void libxl__json_object_free(libxl__gc *gc_opt,
_hidden libxl__json_object *libxl__json_parse(libxl__gc *gc_opt, const char *s);
+/* `args` may be NULL */
_hidden char *libxl__json_object_to_json(libxl__gc *gc,
const libxl__json_object *args);
/* Always return a valid string, but invalid json on error. */
--
Anthony PERARD
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 24+ messages in thread