On Mon, May 16, 2016 at 09:00:17AM -0400, Cole Robinson wrote:
> On 05/16/2016 02:59 AM, Peter Krempa wrote:
> > 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.
> >
>
> Makes sense, thanks for explaining. I'll drop this patch
This comes up time & agin. Do either of you want to submit a patch
putting a nice description of the reason for this in a comment in
code for future reference.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list