On 09.10.2013 05:09, Eric Blake wrote: > On 10/08/2013 03:43 AM, Michal Privoznik wrote: >> https://bugzilla.redhat.com/show_bug.cgi?id=994364 >> >> If user provides domain XML in migration we check if it doesn't break >> ABI via virDomainDefCheckABIStability(). However, if the provided XML >> was generated via virDomainGetXMLDesc(..., VIR_DOMAIN_XML_MIGRATABLE) >> it is missing some devices, e.g. 'pci-root' controller. Hence, the ABI >> stability check fails even though it is stable. >> >> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> >> --- >> src/qemu/qemu_migration.c | 41 +++++++++++++++++++++++++++++++++-------- >> 1 file changed, 33 insertions(+), 8 deletions(-) > > Oh my. How many different ways is this going to bite us? Is it worth > fixing virDomainDefCheckABIStability to do the sanitization itself, > instead of making every single caller across multiple patches repeat the > work? > > What you have seems to work, but I'm really starting to wonder if it is > the best patch. Weak ACK. > Unfortunately, we can't do that simply. virDomainDefCheckABIStability() is defined in domain_conf.c and thus meant to be driver-unaware. I mean, in this patch I'm correctly using qemuDomainDefFormatLive() which on MIGRATABLE flag enabled removes the default usb controller. However, this constraint applies just for the qemu driver. So even if I'd call virDomainDefCopy() within virDomainDefCheckABIStability() to get the MIGRATABLE xml, it is not sufficient for qemu driver. But what about introducing qemuDomainDefCheckABIStability: (writing from the top of my head, doesn't have to necessary compile) qemuDomainDefCheckABIStability(virQEMUDriverPtr driver, virCapsPtr caps, virDomainDefPtr src, virDomainDefPtr dst) { migratableDefStr = qemuDomainDefFormatLive(driver, src, true, false); migratableDef = virDomainDefParseString(migratableDefstr, caps, ...) return virDomainDefCheckABIStability(migratableDef, dst); } And replacing (nearly?) all virDomainDefCheckABIStability() calls under src/qemu/* with the new wrapper? Michal -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list