|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v4 22/32] libxl_qmp: Handle messages from QEMU
On Fri, Jul 27, 2018 at 03:06:04PM +0100, Anthony PERARD wrote:
> This will handles messages received, and calls callbacks associated with
^ handle
I'm not sure I understand what's 'This' in the context. Would be good
if you could spell out what 'This' refers to IMO.
> the libxl__ev_qmp when the expected response is received.
>
> This also print error messages from QMP on behalf of the callback.
>
> Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> ---
>
> Notes:
> v4:
> Provide an libxl error code to callbacks on error instead of a
> qmp_error_class
>
> tools/libxl/libxl_qmp.c | 116 +++++++++++++++++++++++++++
> tools/libxl/libxl_types.idl | 4 +
> tools/libxl/libxl_types_internal.idl | 8 ++
> 3 files changed, 128 insertions(+)
>
> diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
> index aabf9ad5e7..e649b8054d 100644
> --- a/tools/libxl/libxl_qmp.c
> +++ b/tools/libxl/libxl_qmp.c
> @@ -1330,6 +1330,118 @@ static int qmp_ev_prepare_cmd(libxl__gc *gc,
> return 0;
> }
>
> +/*
> + * Handle messages received from QMP server
> + */
> +
> +static int qmp_error_class_to_libxl_error_code(const libxl__qmp_error_class
> c)
> +{
> + switch (c) {
> + case LIBXL__QMP_ERROR_CLASS_GENERICERROR:
> + return ERROR_QMP_GENERIC_ERROR;
> + case LIBXL__QMP_ERROR_CLASS_COMMANDNOTFOUND:
> + return ERROR_QMP_COMMAND_NOT_FOUND;
> + case LIBXL__QMP_ERROR_CLASS_DEVICENOTACTIVE:
> + return ERROR_QMP_DEVICE_NOT_ACTIVE;
> + case LIBXL__QMP_ERROR_CLASS_DEVICENOTFOUND:
> + return ERROR_QMP_DEVICE_NOT_FOUND;
> + default:
> + abort();
> + }
> +}
> +
> +/* return 1 when a user callback as been called */
> +static int qmp_ev_handle_response(libxl__egc *egc,
> + libxl__ev_qmp *ev,
> + const libxl__json_object *resp,
> + libxl__qmp_message_type type)
> +{
> + EGC_GC;
> + const libxl__json_object *response;
> + const libxl__json_object *o;
> + int rc;
> + int id;
> +
> + o = libxl__json_map_get("id", resp, JSON_INTEGER);
> + if (!o) {
> + const char *error_desc;
> +
> + /* unexpected message, attempt to find an error description */
> + o = libxl__json_map_get("error", resp, JSON_MAP);
> + o = libxl__json_map_get("desc", o, JSON_STRING);
> + error_desc = libxl__json_object_get_string(o);
> + if (error_desc)
> + LOGD(ERROR, ev->domid, "%s", error_desc);
> + else
> + LOGD(ERROR, ev->domid, "Received unexpected message: %s",
> + libxl__json_object_to_json(gc, resp));
> + return 0;
> + }
> +
> + id = libxl__json_object_get_integer(o);
> + if (id != ev->id)
> + return 0;
> +
> + switch (type) {
> + case LIBXL__QMP_MESSAGE_TYPE_RETURN: {
> + response = libxl__json_map_get("return", resp, JSON_ANY);
> + rc = 0;
> + break;
> + }
> + case LIBXL__QMP_MESSAGE_TYPE_ERROR: {
> + const char *s;
> + const libxl__json_object *err;
> + libxl__qmp_error_class error_class;
> +
> + rc = ERROR_FAIL;
> + response = NULL;
> +
> + err = libxl__json_map_get("error", resp, JSON_MAP);
> + o = libxl__json_map_get("class", err, JSON_STRING);
> + s = libxl__json_object_get_string(o);
You could init err and s at definition time.
> + if (s && !libxl__qmp_error_class_from_string(s, &error_class))
> + rc = qmp_error_class_to_libxl_error_code(error_class);
> +
> + o = libxl__json_map_get("desc", err, JSON_STRING);
> + s = libxl__json_object_get_string(o);
> + if (s)
> + LOGD(ERROR, ev->domid, "%s", s);
> +
> + break;
> + }
> + default:
> + abort();
> + }
> +
> + ev->id = -1;
> + ev->callback(egc, ev, response, rc); /* must be last */
> + return 1;
> +}
> +
> +/* return 1 when a user callback as been called */
> +static int qmp_ev_handle_message(libxl__egc *egc,
> + libxl__ev_qmp *ev,
> + const libxl__json_object *resp)
> +{
> + EGC_GC;
> + libxl__qmp_message_type type = qmp_response_type(resp);
> +
> + switch (type) {
> + case LIBXL__QMP_MESSAGE_TYPE_QMP:
> + /* greeting message */
> + return 0;
> + case LIBXL__QMP_MESSAGE_TYPE_RETURN:
> + case LIBXL__QMP_MESSAGE_TYPE_ERROR:
> + return qmp_ev_handle_response(egc, ev, resp, type);
> + case LIBXL__QMP_MESSAGE_TYPE_EVENT:
> + /* Event are ignored */
> + return 0;
> + case LIBXL__QMP_MESSAGE_TYPE_INVALID:
> + return 0;
Might be good to have a 'default' lable here and print some debug
message about receiving an unknown QMP message type?
> + }
> + return 0;
> +}
> +
> /*
> * QMP FD callbacks
> */
> @@ -1432,6 +1544,10 @@ static int qmp_ev_callback_readable(libxl__egc *egc,
> libxl__ev_qmp *ev, int fd)
> end = NULL;
>
> LOG_QMP("JSON object received: %s", libxl__json_object_to_json(gc,
> o));
> +
> + /* Must be last and return when the user callback is called */
> + if (qmp_ev_handle_message(egc, ev, o) == 1)
> + return 0;
> }
> return 0;
> }
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 4a385801ba..e104fea941 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -69,6 +69,10 @@ 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, "QMP_GENERIC_ERROR"), # unspecified qmp error
> + (-27, "QMP_COMMAND_NOT_FOUND"), # the requested command has not been
> found
> + (-28, "QMP_DEVICE_NOT_ACTIVE"), # a device has failed to be become active
> + (-29, "QMP_DEVICE_NOT_FOUND"), # the requested device has not been found
Do we really need such granularity for QMP errors? Isn't it enough to
have a single ERROR_QMP or similar?
Thanks, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |