[libvirt] [PATCH] qemu: Don't lose VM runtime state on libvirt downgrade

[Peter Krempa]

On Sun, May 15, 2016 at 18:35:24 -0400, Cole Robinson wrote:
> Run libvirtd from git with latest qemu, start a VM, stop libvirtd.
> Run an older libvirtd version an you may see an error like:

We explicitly don't support downgrades. It will be very hard to handle
this correctly ... let me explain:

[...]

> ---
>  src/qemu/qemu_domain.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index b0eb3b6..dbf8124 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -1411,18 +1411,18 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt,
>              goto error;
>  
>          for (i = 0; i < n; i++) {
> +            int flag;
>              char *str = virXMLPropString(nodes[i], "name");
> -            if (str) {
> -                int flag = virQEMUCapsTypeFromString(str);
> -                if (flag < 0) {
> -                    virReportError(VIR_ERR_INTERNAL_ERROR,
> -                                   _("Unknown qemu capabilities flag %s"), str);
> -                    VIR_FREE(str);
> -                    goto error;
> -                }
> -                VIR_FREE(str);
> +            if (!str)
> +                continue;
> +
> +            flag = virQEMUCapsTypeFromString(str);
> +            if (flag < 0) {
> +                VIR_WARN("Unknown qemu capabilities flag %s", str);

At this point the VM was created with a set of capabilities known by
the newer libvirt version which may change the behavior in a way where
only the new code can handle it.

One of recent examples would be QEMU_CAPS_NAME_DEBUG_THREADS which
broke one of the APIs returning stats about the thread due to change
in the naming. The API was fixed along with the addition of the
capability. If any previous version would contain this code the API
would start to fail after a downgrade.

> +            } else {
>                  virQEMUCapsSet(qemuCaps, flag);
>              }
> +            VIR_FREE(str);
>          }

NACK to this approach. If you want to fix the disk corruption issue
which is legitimate you need to kill the running VM process with missing
capabilities. Making silently ignore new caps is asking for trouble and
complications in the long run since we'd need to start to be forward
compatible.

One of the troublesome approaches could be introduced by
QEMU_CAPS_OBJECT_SECRET which needs different handling in the hotplug
code (not yet implemented).

Also this would create a false sense of that we actually do support
downgrades which I don't think we ever want to do.

Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160516/ec00a5e5/attachment-0001.sig>