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

Re: [libvirt] [PATCH v1 4/4] qemu: Implement virDomain{Get, Set}Time



On Thu, Feb 13, 2014 at 07:51:45PM +0100, Michal Privoznik wrote:
> One caveat though, qemu-ga is expecting time and returning time
> in nanoseconds. With all the buffering and propagation delay, the
> time is already wrong once it gets to the qemu-ga, but there's
> nothing we can do about it.
>
> Signed-off-by: Michal Privoznik <mprivozn redhat com>
> ---
>  src/qemu/qemu_agent.c  |  81 +++++++++++++++++++++++++++++
>  src/qemu/qemu_agent.h  |   6 +++
>  src/qemu/qemu_driver.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 222 insertions(+)
>
> diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
> index 4a3820c..28f14ea 100644
> --- a/src/qemu/qemu_agent.c
> +++ b/src/qemu/qemu_agent.c
> @@ -1657,3 +1657,84 @@ qemuAgentUpdateCPUInfo(unsigned int nvcpus,
>
>      return 0;
>  }
> +
> +
> +int
> +qemuAgentGetTime(qemuAgentPtr mon,
> +                 long long *time)
> +{
> +    int ret = -1;
> +    unsigned long long json_time;
> +    virJSONValuePtr cmd;
> +    virJSONValuePtr reply = NULL;
> +
> +    cmd = qemuAgentMakeCommand("guest-get-time",
> +                               NULL);
> +    if (!cmd)
> +        return ret;
> +
> +    if (qemuAgentCommand(mon, cmd, &reply,
> +                         VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0)
> +        goto cleanup;
> +
> +    if (!reply || qemuAgentCheckError(cmd, reply) < 0)
> +        goto cleanup;
> +

I don't like that qemu is not that introspectable for us to know
whether it has the 'sync' functionality, because otherwise it will
fail with not-very-descriptive internal error :(  However, I don't see
an easy way out of it.

> +    if (virJSONValueObjectGetNumberUlong(reply, "return", &json_time) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("malformed return value"));
> +        goto cleanup;
> +    }
> +
> +    /* guest agent returns time in nanoseconds,
> +     * we need it in seconds here */
> +    *time = json_time / 1000000000LL;
> +    ret = 0;
> +
> +cleanup:
> +    virJSONValueFree(cmd);
> +    virJSONValueFree(reply);
> +    return ret;
> +}
> +
> +
> +/**
> + * qemuAgentSetTime:
> + * @sync: let guest agent to read domain's RTC (@time is ignored)
> + */
> +int
> +qemuAgentSetTime(qemuAgentPtr mon,
> +                long long time,
> +                bool sync)
> +{
> +    int ret = -1;
> +    virJSONValuePtr cmd;
> +    virJSONValuePtr reply = NULL;
> +
> +    if (sync) {
> +        cmd = qemuAgentMakeCommand("guest-set-time", NULL);
> +    } else {
> +        /* guest agent expect time with nanosecond granularity.

s/expect/expects/

> +         * Impressing. */

s/Impressing/Impressive/ ;-)

Definitely, especially when it takes so long to communicate with him
sometimes :)

> +        unsigned long long json_time = time * 1000000000LL;
> +        cmd = qemuAgentMakeCommand("guest-set-time",
> +                                   "U:time", json_time,
> +                                   NULL);
> +    }
> +
> +    if (!cmd)
> +        return ret;
> +
> +    if (qemuAgentCommand(mon, cmd, &reply,
> +                         VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0)
> +        goto cleanup;
> +
> +    if (!reply || qemuAgentCheckError(cmd, reply) < 0)
> +        goto cleanup;
> +
> +    ret = 0;
> +cleanup:
> +    virJSONValueFree(cmd);
> +    virJSONValueFree(reply);
> +    return ret;
> +}
> diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h
> index 5fbacdb..4618f84 100644
> --- a/src/qemu/qemu_agent.h
> +++ b/src/qemu/qemu_agent.h
> @@ -97,4 +97,10 @@ int qemuAgentSetVCPUs(qemuAgentPtr mon, qemuAgentCPUInfoPtr cpus, size_t ncpus);
>  int qemuAgentUpdateCPUInfo(unsigned int nvcpus,
>                             qemuAgentCPUInfoPtr cpuinfo,
>                             int ncpuinfo);
> +
> +int qemuAgentGetTime(qemuAgentPtr mon,
> +                     long long *time);
> +int qemuAgentSetTime(qemuAgentPtr mon,
> +                     long long time,
> +                     bool sync);
>  #endif /* __QEMU_AGENT_H__ */
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 59e018d..7ed7120 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
[...]
> +static int
> +qemuDomainSetTime(virDomainPtr dom,
> +                  long long set_time,
> +                  const char *timezone,
> +                  unsigned int flags)
> +{
> +    virQEMUDriverPtr driver = dom->conn->privateData;
> +    virDomainObjPtr vm;
> +    int ret = -1;
> +    bool sync = flags & VIR_DOMAIN_TIME_SYNC;
> +    qemuDomainObjPrivatePtr priv;
> +
> +    virCheckFlags(VIR_DOMAIN_TIME_SYNC, ret);
> +
> +    if (timezone) {
> +        virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> +                       _("Supplying timezone is not supported yet"));
> +        return ret;

OK, now I get why you didn't use that variable :)  Are you planning on
adding the possibility into qemu driver or it will be available in
guest agent?  Not that it matters for this patch, just curious.

Rest looks fine,
Martin

Attachment: signature.asc
Description: Digital signature


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