On Fri, Aug 16, 2019 at 02:22:58PM +0200, Markus Armbruster wrote: > Erik Skultety <eskul...@redhat.com> writes: > > > On Fri, Aug 16, 2019 at 08:10:20AM +0200, Markus Armbruster wrote: > >> Eduardo Habkost <ehabk...@redhat.com> writes: > >> > >> > We have this issue reported when using libvirt to hotplug CPUs: > >> > https://bugzilla.redhat.com/show_bug.cgi?id=1741451 > >> > > >> > Basically, libvirt is not copying die-id from > >> > query-hotpluggable-cpus, but die-id is now mandatory. > >> > >> Uh-oh, "is now mandatory": making an optional property mandatory is an > >> incompatible change. When did we do that? Commit hash, please. > >> > >> [...] > >> > > > > I don't even see it as being optional ever - the property wasn't even > > recognized before commit 176d2cda0de introduced it as mandatory. > > Compatibility break. > > Commit 176d2cda0de is in v4.1.0. If I had learned about it a bit > earlier, I would've argued for a last minute fix or a revert. Now we > have a regression in the release. > > Eduardo, I think this fix should go into v4.1.1. Please add cc: > qemu-stable.
I did it in v2. > > How can we best avoid such compatibility breaks to slip in undetected? > > A static checker would be nice. For vmstate, we have > scripts/vmstate-static-checker.py. Not sure it's used. I don't think this specific bug would be detected with a static checker. "die-id is mandatory" is not something that can be extracted by looking at QOM data structures. The new rule was being enforced by the hotplug handler callbacks, and the hotplug handler call tree is a bit complex (too complex for my taste, but I digress). We could have detected this with a simple CPU hotplug automated test case, though. Or with a very simple -device test case like the one I have submitted with this patch. This was detected by libvirt automated test cases. It would be nice if this was run during the -rc stage and not only after the 4.1.0 release, though. I don't know details of the test job. Danilo, Mirek, Yash: do you know how this bug was detected, and what we could do to run the same test jobs in upstream QEMU release candidates? -- Eduardo