| Message ID | 8a1b5962437bfa51e7c8f139673dda4497220527.1463351724.git.crobinso@redhat.com |
|---|---|
| State | New |
| Headers | show |
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 - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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); + } else { virQEMUCapsSet(qemuCaps, flag); } + VIR_FREE(str); } priv->qemuCaps = qemuCaps;