[libvirt] [PATCH v3 3/9] qemu: add helper for getting guest users

[Michal Privoznik]

On 8/23/19 6:31 PM, Jonathon Jongsma wrote:
> This function fetches the list of logged-in users from the qemu agent
> and adds them to a list of typed parameters so that they can be used
> internally in libvirt.
> 
> Also add some basic tests for the function.
> 
> Signed-off-by: Jonathon Jongsma <jjongsma at redhat.com>
> ---
>   src/qemu/qemu_agent.c |  91 +++++++++++++++++++++++
>   src/qemu/qemu_agent.h |   2 +
>   tests/qemuagenttest.c | 168 ++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 261 insertions(+)
> 
> diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
> index 361db299a5..963a4b9359 100644
> --- a/src/qemu/qemu_agent.c
> +++ b/src/qemu/qemu_agent.c
> @@ -2240,3 +2240,94 @@ qemuAgentSetUserPassword(qemuAgentPtr mon,
>       VIR_FREE(password64);
>       return ret;
>   }
> +
> +int
> +qemuAgentGetUsers(qemuAgentPtr mon,
> +                  virTypedParameterPtr *params,
> +                  int *nparams,
> +                  int *maxparams)
> +{
> +    int ret = -1;
> +    size_t i;
> +    virJSONValuePtr cmd;
> +    virJSONValuePtr reply = NULL;
> +    virJSONValuePtr data = NULL;

We can use VIR_AUTOPTR() and drop the explicit virJSONValueFree() calls 
at the cleanup label which then in turn gets needless as we can 'return 
-1' directly instead of 'goto cleanup'.

> +    size_t ndata;
> +    const char *strvalue;
> +
> +    if (!(cmd = qemuAgentMakeCommand("guest-get-users", NULL)))
> +        return -1;
> +
> +    if (qemuAgentCommand(mon, cmd, &reply, true,
> +                         VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0)
> +        goto cleanup;
> +
> +    if (!(data = virJSONValueObjectGetArray(reply, "return"))) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("guest-get-users reply was missing return data"));
> +        goto cleanup;
> +    }
> +
> +    if (!virJSONValueIsArray(data)) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Malformed guest-get-users data array"));
> +        goto cleanup;
> +    }
> +
> +    ndata = virJSONValueArraySize(data);
> +
> +    if (virTypedParamsAddUInt(params, nparams, maxparams,
> +                              "user.count", ndata) < 0)
> +        goto cleanup;
> +
> +    for (i = 0; i < ndata; i++) {
> +        char param_name[VIR_TYPED_PARAM_FIELD_LENGTH];
> +        virJSONValuePtr entry = virJSONValueArrayGet(data, i);
> +
> +        if (!entry) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("array element missing in guest-get-users return "
> +                             "value"));
> +            goto cleanup;
> +        }
> +
> +        if (!(strvalue = virJSONValueObjectGetString(entry, "user"))) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("'user' missing in reply of guest-get-users"));
> +            goto cleanup;
> +        }
> +
> +        snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, "user.%zu.name", i);
> +        if (virTypedParamsAddString(params, nparams, maxparams,
> +                                    param_name, strvalue) < 0)
> +            goto cleanup;
> +
> +        /* 'domain' is only present for windows guests */
> +        if ((strvalue = virJSONValueObjectGetString(entry, "domain"))) {
> +            snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> +                     "user.%zu.domain", i);
> +            if (virTypedParamsAddString(params, nparams, maxparams,
> +                                        param_name, strvalue) < 0)
> +                goto cleanup;
> +        }
> +
> +        double logintime;
> +        if (virJSONValueObjectGetNumberDouble(entry, "login-time", &logintime) < 0) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("'login-time' missing in reply of guest-get-users"));
> +            goto cleanup;
> +        }
> +        snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> +                 "user.%zu.login-time", i);
> +        if (virTypedParamsAddULLong(params, nparams, maxparams,
> +                                    param_name, logintime * 1000) < 0)
> +            goto cleanup;
> +    }
> +
> +    ret = ndata;
> +
> + cleanup:
> +    virJSONValueFree(cmd);
> +    virJSONValueFree(reply);
> +    return ret;
> +}
> diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h
> index 6ae9fe54da..05621b521a 100644
> --- a/src/qemu/qemu_agent.h
> +++ b/src/qemu/qemu_agent.h
> @@ -120,3 +120,5 @@ int qemuAgentSetUserPassword(qemuAgentPtr mon,
>                                const char *user,
>                                const char *password,
>                                bool crypted);
> +
> +int qemuAgentGetUsers(qemuAgentPtr mon, virTypedParameterPtr *params, int *nparams, int *maxparams);

Very loooong line ;-)

> diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c
> index 2f79986207..f2936a59f0 100644
> --- a/tests/qemuagenttest.c
> +++ b/tests/qemuagenttest.c
> @@ -902,6 +902,173 @@ testQemuAgentGetInterfaces(const void *data)
>       return ret;
>   }
>   
> +static const char testQemuAgentUsersResponse[] =
> +    "{\"return\": "
> +    "   ["
> +    "       {\"user\": \"test\","
> +    "        \"login-time\": 1561739203.584038"
> +    "       },"
> +    "       {\"user\": \"test2\","
> +    "        \"login-time\": 1561739229.190697"
> +    "       }"
> +    "   ]"
> +    "}";
> +
> +static const char testQemuAgentUsersResponse2[] =
> +    "{\"return\": "
> +    "   ["
> +    "       {\"user\": \"test\","
> +    "        \"domain\": \"DOMAIN\","
> +    "        \"login-time\": 1561739203.584038"
> +    "       }"
> +    "   ]"
> +    "}";
> +
> +static int getUserInfo(virTypedParameterPtr params, int nparams, size_t nth,
> +                       const char **username, const char **domain,
> +                       unsigned long long *logintime)
> +{
> +    char param_name[VIR_TYPED_PARAM_FIELD_LENGTH];
> +
> +    snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> +             "user.%zu.name", nth);
> +    if (username &&
> +        virTypedParamsGetString(params, nparams, param_name, username) < 0)
> +        return -1;
> +
> +    snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> +             "user.%zu.domain", nth);
> +    if (domain &&
> +        virTypedParamsGetString(params, nparams, param_name, domain) < 0)
> +        return -1;
> +
> +    snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> +             "user.%zu.login-time", nth);
> +    if (logintime &&
> +        virTypedParamsGetULLong(params, nparams, param_name, logintime) < 0)
> +        return -1;
> +
> +    return 0;

This function can be renamed to checkUserInfo() and it can check the 
values directly. It saves us couple of more lines.

> +}
> +

Michal