[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2 01/10] Add qemuMonitorJSONGetObjectListPaths() method for QMP qom-list command



On 08.07.2013 21:20, John Ferlan wrote:
> Add a new qemuMonitorJSONGetObjectListPaths() method to support invocation
> of the 'qom-list' JSON monitor command with a provided path.
> 
> The returned list of paired data fields of "name" and "type" that can
> be used to peruse QOM configuration data and eventually utilize for the
> balloon statistics.
> 
> The test does a "{"execute":"qom-list", "arguments": { "path": "/"}}" which
> returns "{"return": [{"name": "machine", "type": "child<container>"},
> {"name": "type", "type": "string"}]}" resulting in a return of an array
> of 2 elements with [0].name="machine", [0].type="child<container>".  The [1]
> entry appears to be a header that could be used some day via a command such
> as "virsh qemuobject --list" to format output.
> ---
>  src/qemu/qemu_monitor_json.c | 102 +++++++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_monitor_json.h |  16 +++++++
>  tests/qemumonitorjsontest.c  |  79 +++++++++++++++++++++++++++++++++
>  3 files changed, 197 insertions(+)
> 
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 3383c88..fc2b65f 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -4540,6 +4540,108 @@ cleanup:
>  }
>  
>  
> +int qemuMonitorJSONGetObjectListPaths(qemuMonitorPtr mon,
> +                                      const char *path,
> +                                      qemuMonitorJSONListPathPtr **paths)
> +{
> +    int ret;
> +    virJSONValuePtr cmd;
> +    virJSONValuePtr reply = NULL;
> +    virJSONValuePtr data;
> +    qemuMonitorJSONListPathPtr *pathlist = NULL;
> +    int n = 0;
> +    size_t i;
> +
> +    *paths = NULL;
> +
> +    if (!(cmd = qemuMonitorJSONMakeCommand("qom-list",
> +                                           "s:path", path,
> +                                           NULL)))
> +        return -1;
> +
> +    ret = qemuMonitorJSONCommand(mon, cmd, &reply);
> +
> +    if (ret == 0)
> +        ret = qemuMonitorJSONCheckError(cmd, reply);
> +
> +    if (ret < 0)
> +        goto cleanup;
> +
> +    ret = -1;
> +
> +    if (!(data = virJSONValueObjectGet(reply, "return"))) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("qom-list reply was missing return data"));
> +        goto cleanup;
> +    }
> +
> +    if ((n = virJSONValueArraySize(data)) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("qom-list reply data was not an array"));
> +        goto cleanup;
> +    }
> +
> +    /* null-terminated list */

Do we need NULL terminated list ... [1]

> +    if (VIR_ALLOC_N(pathlist, n + 1) < 0) {
> +        virReportOOMError();

This can be dropped now. But as you've said in one of previous e-mails
of your, you already did that.

> +        goto cleanup;
> +    }
> +
> +    for (i = 0; i < n; i++) {
> +        virJSONValuePtr child = virJSONValueArrayGet(data, i);
> +        const char *tmp;
> +        qemuMonitorJSONListPathPtr info;
> +
> +        if (VIR_ALLOC(info) < 0) {
> +            virReportOOMError();
> +            goto cleanup;
> +        }
> +
> +        pathlist[i] = info;
> +
> +        if (!(tmp = virJSONValueObjectGetString(child, "name"))) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("qom-list reply data was missing 'name'"));
> +            goto cleanup;
> +        }
> +
> +        if (VIR_STRDUP(info->name, tmp) < 0)
> +            goto cleanup;
> +
> +        if (virJSONValueObjectHasKey(child, "type")) {
> +            if (!(tmp = virJSONValueObjectGetString(child, "type"))) {
> +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                               _("qom-list reply has malformed 'type' data"));
> +                goto cleanup;
> +            }
> +            if (VIR_STRDUP(info->type, tmp) < 0)
> +                goto cleanup;
> +        }
> +    }
> +
> +    ret = n;

1: ... if we are returning number of items in array? I'm not saying it's
something wrong, just curious.

> +    *paths = pathlist;
> +
> +cleanup:
> +    if (ret < 0 && pathlist) {
> +        for (i = 0; i < n; i++)
> +            qemuMonitorJSONListPathFree(pathlist[i]);
> +        VIR_FREE(pathlist);
> +    }
> +    virJSONValueFree(cmd);
> +    virJSONValueFree(reply);
> +    return ret;
> +}
> +
> +void qemuMonitorJSONListPathFree(qemuMonitorJSONListPathPtr paths)
> +{
> +    if (!paths)
> +        return;
> +    VIR_FREE(paths->name);
> +    VIR_FREE(paths->type);
> +    VIR_FREE(paths);
> +}
> +
>  int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon,
>                                    const char *type,
>                                    char ***props)
> diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
> index d79b86b..e7ce145 100644
> --- a/src/qemu/qemu_monitor_json.h
> +++ b/src/qemu/qemu_monitor_json.h
> @@ -332,6 +332,22 @@ int qemuMonitorJSONGetKVMState(qemuMonitorPtr mon,
>  int qemuMonitorJSONGetObjectTypes(qemuMonitorPtr mon,
>                                    char ***types)
>      ATTRIBUTE_NONNULL(2);
> +
> +typedef struct _qemuMonitorJSONListPath qemuMonitorJSONListPath;
> +typedef qemuMonitorJSONListPath *qemuMonitorJSONListPathPtr;
> +
> +struct _qemuMonitorJSONListPath {
> +    char *name;
> +    char *type;
> +};
> +
> +int qemuMonitorJSONGetObjectListPaths(qemuMonitorPtr mon,
> +                                      const char *path,
> +                                      qemuMonitorJSONListPathPtr **paths)
> +    ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
> +
> +void qemuMonitorJSONListPathFree(qemuMonitorJSONListPathPtr paths);
> +
>  int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon,
>                                    const char *type,
>                                    char ***props)
> diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
> index acc94ca..380d377 100644
> --- a/tests/qemumonitorjsontest.c
> +++ b/tests/qemumonitorjsontest.c
> @@ -23,6 +23,7 @@
>  #include "testutilsqemu.h"
>  #include "qemumonitortestutils.h"
>  #include "qemu/qemu_conf.h"
> +#include "qemu/qemu_monitor_json.h"
>  #include "virthread.h"
>  #include "virerror.h"
>  #include "virstring.h"
> @@ -595,6 +596,83 @@ cleanup:
>  }
>  
>  
> +/*
> + * This test will request to return a list of paths for "/". It should be
> + * a simple list of 1 real element that being the "machine". The following
> + * is the execution and expected return:
> + *
> + *  {"execute":"qom-list", "arguments": { "path": "/"}}"
> + *  {"return": [{"name": "machine", "type": "child<container>"}, \
> + *              {"name": "type", "type": "string"}]}
> + */
> +static int
> +testQemuMonitorJSONGetListPaths(const void *data)
> +{
> +    const virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
> +    qemuMonitorTestPtr test = qemuMonitorTestNew(true, xmlopt);
> +    int ret = -1;
> +    qemuMonitorJSONListPathPtr *paths;
> +    int npaths = 0;
> +    int i;

And after Daniel's patches, this needs to be size_t i;

> +
> +    if (!test)
> +        return -1;
> +
> +    if (qemuMonitorTestAddItem(test, "qom-list",
> +                               "{ "
> +                               "  \"return\": [ "
> +                               "  {\"name\": \"machine\", "
> +                               "   \"type\": \"child<container>\"}, "
> +                               "  {\"name\": \"type\", "
> +                               "   \"type\": \"string\"} "
> +                               " ]"
> +                               "}") < 0)
> +        goto cleanup;
> +
> +    /* present with path */
> +    if ((npaths = qemuMonitorJSONGetObjectListPaths(
> +                                                qemuMonitorTestGetMonitor(test),
> +                                                "/",
> +                                                &paths)) < 0)
> +        goto cleanup;
> +
> +    if (npaths != 2) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       "npaths was %d, expected 1", npaths);
> +        goto cleanup;
> +    }
> +
> +#define CHECK(i, wantname, wanttype)                                    \
> +    do {                                                                \
> +        if (STRNEQ(paths[i]->name, (wantname))) {                       \
> +            virReportError(VIR_ERR_INTERNAL_ERROR,                      \
> +                           "name was %s, expected %s",                  \
> +                           paths[i]->name, (wantname));                 \
> +            goto cleanup;                                               \
> +        }                                                               \
> +        if (STRNEQ_NULLABLE(paths[i]->type, (wanttype))) {              \
> +            virReportError(VIR_ERR_INTERNAL_ERROR,                      \
> +                           "type was %s, expected %s",                  \
> +                           NULLSTR(paths[i]->type), (wanttype));        \
> +            goto cleanup;                                               \
> +        }                                                               \
> +    } while (0)
> +
> +    CHECK(0, "machine", "child<container>");
> +
> +#undef CHECK
> +
> +    ret = 0;
> +
> +cleanup:
> +    qemuMonitorTestFree(test);
> +    for (i = 0; i < npaths; i++)
> +        qemuMonitorJSONListPathFree(paths[i]);
> +    VIR_FREE(paths);
> +    return ret;
> +}
> +
> +
>  static int
>  mymain(void)
>  {
> @@ -623,6 +701,7 @@ mymain(void)
>      DO_TEST(GetCommands);
>      DO_TEST(GetTPMModels);
>      DO_TEST(GetCommandLineOptionParameters);
> +    DO_TEST(GetListPaths);
>  
>      virObjectUnref(xmlopt);
>  
> 

Michal


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]