Re: [PATCH v2 2/2] qemu: Refresh RTC adjustment on qemuProcessReconnect

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

 



On Tue, May 03, 2016 at 10:33:43 +0200, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1139766
> 
> Thing is, for some reasons you can have your domain's RTC to be
> in something different than UTC. More weirdly, it's not only time
> zone what you can shift it of, but an arbitrary value. So, if
> domain is configured that way, libvirt will correctly put it onto
> qemu cmd line and moreover track it as this offset changes during
> domain's life time (e.g. because guest OS decides the best thing
> to do is set new time to RTC). Anyway, they way in which this
> tracking is implemented is events. But we've got a problem if
> change in guest's RTC occurs and the daemon is not running. The
> event is lost and we end up reporting invalid value in domain
> XML. Therefore, when the daemon is starting up again and it is
> reconnecting to all running domains, re-fetch their RTC so the
> correct offset value can be computed.
> 
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
>  src/qemu/qemu_process.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 527300a..220a803 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -1992,6 +1992,47 @@ qemuRefreshVirtioChannelState(virQEMUDriverPtr driver,
>      return ret;
>  }
>  
> +static int

Does this even need to return a value, since ... [1]

> +qemuRefreshRTC(virQEMUDriverPtr driver,
> +               virDomainObjPtr vm)
> +{
> +    qemuDomainObjPrivatePtr priv = vm->privateData;
> +    time_t now, then;
> +    struct tm thenbits;
> +    long localOffset;
> +    int ret, rv;
> +
> +    if (vm->def->clock.offset != VIR_DOMAIN_CLOCK_OFFSET_VARIABLE)
> +        return 0;
> +
> +    memset(&thenbits, 0, sizeof(thenbits));
> +    qemuDomainObjEnterMonitor(driver, vm);
> +    now = time(NULL);
> +    rv = qemuMonitorGetRTCTime(priv->mon, &thenbits);
> +    if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +        rv = -1;
> +
> +    if (rv < 0)
> +        goto cleanup;
> +
> +    thenbits.tm_isdst = -1;
> +    if ((then = mktime(&thenbits)) == (time_t) -1) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Unable to convert time"));
> +        goto cleanup;
> +    }
> +
> +    /* Thing is, @now is in local TZ but @then in UTC. */
> +    if (virTimeLocalOffsetFromUTC(&localOffset) < 0)
> +        goto cleanup;
> +
> +    vm->def->clock.data.variable.adjustment = then - now + localOffset;
> +
> +    ret = 0;
> +
> + cleanup:
> +    return ret;
> +}
>  
>  int
>  qemuProcessRefreshBalloonState(virQEMUDriverPtr driver,
> @@ -3673,6 +3714,9 @@ qemuProcessReconnect(void *opaque)
>      if (qemuRefreshVirtioChannelState(driver, obj) < 0)
>          goto error;
>  
> +    /* If querying of guest's RTC failed, report error, but do not kill the domain. */
> +    ignore_value(qemuRefreshRTC(driver, obj));

[1] ... you ignore it?

Do you expect this function to be used anywhere else? If not then it
doesn't make sense to return anything from the func.

ACK with the return value removed.

Peter

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]