[libvirt] [PATCH 4/4] qemu: query command line options in QMP
Osier Yang
jyang at redhat.com
Mon May 13 08:35:08 UTC 2013
On 27/04/13 05:01, Eric Blake wrote:
> Ever since the conversion to using only QMP for probing features
> of qemu 1.2 and newer, we have been unable to detect features
> that are added only by additional command line options. For
> example, we'd like to know if '-machine mem-merge=on' (added
> in qemu 1.5) is present. To do this, we will take advantage
> of qemu 1.5's query-command-line-parameters QMP call [1].
>
> This patch wires up the framework for probing the command results;
> if the QMP command is missing, or if a particular command line
> option does not output any parameters (for example, -net uses
> an old-style parser, so it shows up with no parameters), we
> silently treat that command as having no results.
>
> [1] https://lists.gnu.org/archive/html/qemu-devel/2013-04/msg05180.html
>
> * src/qemu/qemu_monitor.h (qemuMonitorGetOptions)
> (qemuMonitorSetOptions)
> (qemuMonitorGetCommandLineOptionParameters): New functions.
> * src/qemu/qemu_monitor_json.h
> (qemuMonitorJSONGetCommandLineOptionParameters): Likewise.
> * src/qemu/qemu_monitor.c (_qemuMonitor): Add cache field.
> (qemuMonitorDispose): Clean it.
> (qemuMonitorGetCommandLineOptionParameters): Implement new function.
> * src/qemu/qemu_monitor_json.c
> (qemuMonitorJSONGetCommandLineOptionParameters): Likewise.
> (testQemuMonitorJSONGetCommandLineParameters): Test it.
>
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
> src/qemu/qemu_monitor.c | 41 +++++++++++++++
> src/qemu/qemu_monitor.h | 9 +++-
> src/qemu/qemu_monitor_json.c | 123 +++++++++++++++++++++++++++++++++++++++++++
> src/qemu/qemu_monitor_json.h | 4 ++
> tests/qemumonitorjsontest.c | 103 ++++++++++++++++++++++++++++++++++++
> 5 files changed, 279 insertions(+), 1 deletion(-)
>
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index 5f714d6..fffd4cd 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -78,6 +78,9 @@ struct _qemuMonitor {
>
> bool json;
> bool waitGreeting;
> +
> + /* cache of query-command-line-options results */
> + virJSONValuePtr options;
> };
>
> static virClassPtr qemuMonitorClass;
> @@ -242,6 +245,7 @@ static void qemuMonitorDispose(void *obj)
> (mon->cb->destroy)(mon, mon->vm);
> virCondDestroy(&mon->notify);
> VIR_FREE(mon->buffer);
> + virJSONValueFree(mon->options);
> }
>
>
> @@ -911,6 +915,18 @@ cleanup:
> }
>
>
> +virJSONValuePtr
> +qemuMonitorGetOptions(qemuMonitorPtr mon)
> +{
> + return mon->options;
> +}
> +
> +void
> +qemuMonitorSetOptions(qemuMonitorPtr mon, virJSONValuePtr options)
> +{
> + mon->options = options;
> +}
> +
> int qemuMonitorHMPCommandWithFd(qemuMonitorPtr mon,
> const char *cmd,
> int scm_fd,
> @@ -3333,6 +3349,31 @@ int qemuMonitorGetEvents(qemuMonitorPtr mon,
> }
>
>
> +/* Collect the parameters associated with a given command line option.
> + * Return count of known parameters or -1 on error. */
> +int
> +qemuMonitorGetCommandLineOptionParameters(qemuMonitorPtr mon,
> + const char *option,
> + char ***params)
> +{
> + VIR_DEBUG("mon=%p option=%s params=%p", mon, option, params);
> +
> + if (!mon) {
> + virReportError(VIR_ERR_INVALID_ARG, "%s",
> + _("monitor must not be NULL"));
> + return -1;
> + }
> +
> + if (!mon->json) {
> + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> + _("JSON monitor is required"));
> + return -1;
> + }
> +
> + return qemuMonitorJSONGetCommandLineOptionParameters(mon, option, params);
> +}
> +
> +
> int qemuMonitorGetKVMState(qemuMonitorPtr mon,
> bool *enabled,
> bool *present)
> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> index 527da0a..8f9c182 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -162,12 +162,16 @@ int qemuMonitorSetCapabilities(qemuMonitorPtr mon);
>
> int qemuMonitorSetLink(qemuMonitorPtr mon,
> const char *name,
> - enum virDomainNetInterfaceLinkState state) ;
> + enum virDomainNetInterfaceLinkState state);
>
> /* These APIs are for use by the internal Text/JSON monitor impl code only */
> char *qemuMonitorNextCommandID(qemuMonitorPtr mon);
> int qemuMonitorSend(qemuMonitorPtr mon,
> qemuMonitorMessagePtr msg);
> +virJSONValuePtr qemuMonitorGetOptions(qemuMonitorPtr mon)
> + ATTRIBUTE_NONNULL(1);
> +void qemuMonitorSetOptions(qemuMonitorPtr mon, virJSONValuePtr options)
> + ATTRIBUTE_NONNULL(1);
> int qemuMonitorHMPCommandWithFd(qemuMonitorPtr mon,
> const char *cmd,
> int scm_fd,
> @@ -664,6 +668,9 @@ int qemuMonitorGetCommands(qemuMonitorPtr mon,
> char ***commands);
> int qemuMonitorGetEvents(qemuMonitorPtr mon,
> char ***events);
> +int qemuMonitorGetCommandLineOptionParameters(qemuMonitorPtr mon,
> + const char *option,
> + char ***params);
>
> int qemuMonitorGetKVMState(qemuMonitorPtr mon,
> bool *enabled,
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 341b295..0423ec7 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -4275,6 +4275,129 @@ cleanup:
> }
>
>
> +int
> +qemuMonitorJSONGetCommandLineOptionParameters(qemuMonitorPtr mon,
> + const char *option,
> + char ***params)
> +{
> + int ret;
> + virJSONValuePtr cmd;
> + virJSONValuePtr reply = NULL;
> + virJSONValuePtr data = NULL;
> + virJSONValuePtr array = NULL;
> + char **paramlist = NULL;
> + int n = 0;
> + size_t i;
> +
> + *params = NULL;
> +
> + /* query-command-line-options has fixed output for a given qemu
> + * binary; but since callers want to query parameters for one
> + * option at a time, we cache the option list from qemu. */
> + if (!qemuMonitorGetOptions(mon)) {
> + if (!(cmd = qemuMonitorJSONMakeCommand("query-command-line-options",
> + NULL)))
> + return -1;
> +
> + ret = qemuMonitorJSONCommand(mon, cmd, &reply);
> +
> + if (ret == 0) {
> + if (qemuMonitorJSONHasError(reply, "CommandNotFound"))
> + goto cleanup;
> + ret = qemuMonitorJSONCheckError(cmd, reply);
> + }
> +
> + if (ret < 0)
> + goto cleanup;
> +
> + if (virJSONValueObjectRemoveKey(reply, "return", &array) <= 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("query-command-line-options reply was missing "
> + "return data"));
> + goto cleanup;
> + }
> + qemuMonitorSetOptions(mon, array);
> + }
> +
> + ret = -1;
> +
> + array = qemuMonitorGetOptions(mon);
No need to get it when the options is cached. How about:
if (!(array = qemuMonitorGetOptions(mon))) {
.....
}
> + if ((n = virJSONValueArraySize(array)) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("query-command-line-options reply data was not "
s/was/is/, ? I'm sure that I don't want to talk about English with you
though. :-)
> + "an array"));
> + goto cleanup;
> + }
> +
> + for (i = 0 ; i < n ; i++) {
> + virJSONValuePtr child = virJSONValueArrayGet(array, i);
> + const char *tmp;
> +
> + if (!(tmp = virJSONValueObjectGetString(child, "option"))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("query-command-line-options reply data was "
Likewise.
> + "missing 'option'"));
> + goto cleanup;
> + }
> + if (STREQ(tmp, option)) {
> + data = virJSONValueObjectGet(child, "parameters");
> + break;
> + }
> + }
> +
> + if (!data) {
> + /* Option not found; return 0 parameters rather than an error. */
> + ret = 0;
> + goto cleanup;
> + }
> +
> + if ((n = virJSONValueArraySize(data)) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("query-command-line-options parameter data was not "
Likewise.
> + "an array"));
> + goto cleanup;
> + }
> +
> + /* null-terminated list */
> + if (VIR_ALLOC_N(paramlist, n + 1) < 0) {
> + virReportOOMError();
> + goto cleanup;
> + }
> +
> + for (i = 0 ; i < n ; i++) {
> + virJSONValuePtr child = virJSONValueArrayGet(data, i);
> + const char *tmp;
> +
> + if (!(tmp = virJSONValueObjectGetString(child, "name"))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("query-command-line-options parameter data was "
Likwise.
> + "missing 'name'"));
> + goto cleanup;
> + }
> +
> + if (!(paramlist[i] = strdup(tmp))) {
This has to be changed to use VIR_STRDUP.
> + virReportOOMError();
> + goto cleanup;
> + }
> + }
> +
> + ret = n;
> + *params = paramlist;
> +
> +cleanup:
> + /* If we failed before getting the JSON array of options, we (try)
> + * to cache an empty array to speed up the next failure. */
> + if (!qemuMonitorGetOptions(mon))
> + qemuMonitorSetOptions(mon, virJSONValueNewArray());
> +
> + if (ret < 0)
> + virStringFreeList(paramlist);
> + virJSONValueFree(cmd);
> + virJSONValueFree(reply);
> + return ret;
> +}
> +
> +
> int qemuMonitorJSONGetKVMState(qemuMonitorPtr mon,
> bool *enabled,
> bool *present)
> diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
> index 5ee0d84..74e2476 100644
> --- a/src/qemu/qemu_monitor_json.h
> +++ b/src/qemu/qemu_monitor_json.h
> @@ -319,6 +319,10 @@ int qemuMonitorJSONGetCommands(qemuMonitorPtr mon,
> int qemuMonitorJSONGetEvents(qemuMonitorPtr mon,
> char ***events)
> ATTRIBUTE_NONNULL(2);
> +int qemuMonitorJSONGetCommandLineOptionParameters(qemuMonitorPtr mon,
> + const char *option,
> + char ***params)
> + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
>
> int qemuMonitorJSONGetKVMState(qemuMonitorPtr mon,
> bool *enabled,
> diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
> index 6f42598..c7f0536 100644
> --- a/tests/qemumonitorjsontest.c
> +++ b/tests/qemumonitorjsontest.c
> @@ -494,6 +494,108 @@ cleanup:
>
>
> static int
> +testQemuMonitorJSONGetCommandLineOptionParameters(const void *data)
> +{
> + const virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
> + qemuMonitorTestPtr test = qemuMonitorTestNew(true, xmlopt);
> + int ret = -1;
> + char **params = NULL;
> + int nparams = 0;
> +
> + if (!test)
> + return -1;
> +
> + if (qemuMonitorTestAddItem(test, "query-command-line-options",
> + "{ "
> + " \"return\": [ "
> + " {\"parameters\": [], \"option\": \"acpi\" },"
> + " {\"parameters\": ["
> + " {\"name\": \"romfile\", "
> + " \"type\": \"string\"}, "
> + " {\"name\": \"bootindex\", "
> + " \"type\": \"number\"}], "
> + " \"option\": \"option-rom\"}"
> + " ]"
> + "}") < 0)
> + goto cleanup;
> +
> + /* present with params */
> + if ((nparams = qemuMonitorGetCommandLineOptionParameters(qemuMonitorTestGetMonitor(test),
> + "option-rom",
> + ¶ms)) < 0)
> + goto cleanup;
> +
> + if (nparams != 2) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + "nparams %d is not 2", nparams);
This will produce something like "4 is not 2", which is obvious and
confused. :/
"'nparams' is not 2" might be better.
> + goto cleanup;
> + }
> +
> +#define CHECK(i, wantname) \
> + do { \
> + if (STRNEQ(params[i], (wantname))) { \
> + virReportError(VIR_ERR_INTERNAL_ERROR, \
> + "name %s is not %s", \
> + params[i], (wantname)); \
> + goto cleanup; \
> + } \
> + } while (0)
> +
> + CHECK(0, "romfile");
> + CHECK(1, "bootindex");
> +
> +#undef CHECK
> +
> + virStringFreeList(params);
> + params = NULL;
> +
> + /* present but empty */
> + if ((nparams = qemuMonitorGetCommandLineOptionParameters(qemuMonitorTestGetMonitor(test),
> + "acpi",
> + ¶ms)) < 0)
> + goto cleanup;
> +
> + if (nparams != 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + "nparams %d is not 0", nparams);
Likewise.
> + goto cleanup;
> + }
> + if (params && params[0]) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + "unexpected array contents");
> + goto cleanup;
> + }
> +
> + virStringFreeList(params);
> + params = NULL;
> +
> + /* no such option */
> + if ((nparams = qemuMonitorGetCommandLineOptionParameters(qemuMonitorTestGetMonitor(test),
> + "foobar",
> + ¶ms)) < 0)
> + goto cleanup;
> +
> + if (nparams != 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + "nparams %d is not 0", nparams);
Likewise.
ACK with the nits fixed.
More information about the libvir-list
mailing list