Re: [libvirt] [PATCH] qemu_migration: Check ABI stability against MIGRATABLE xml
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 mpriv...@redhat.com
---
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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu_migration: Check ABI stability against MIGRATABLE xml
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 mpriv...@redhat.com
---
src/qemu/qemu_migration.c | 41 +
1 file changed, 33 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 3a1aab7..65a09b4 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1970,6 +1970,8 @@ static char
virCapsPtr caps = NULL;
unsigned int cookieFlags = QEMU_MIGRATION_COOKIE_LOCKSTATE;
bool abort_on_error = !!(flags VIR_MIGRATE_ABORT_ON_ERROR);
+virDomainDefPtr migratableDef = NULL;
+char *migratableDefStr = NULL;
VIR_DEBUG(driver=%p, vm=%p, xmlin=%s, dname=%s,
cookieout=%p, cookieoutlen=%p, flags=%lx,
@@ -2039,7 +2041,17 @@ static char
VIR_DOMAIN_XML_INACTIVE)))
goto cleanup;
-if (!virDomainDefCheckABIStability(vm-def, def))
+migratableDefStr = qemuDomainDefFormatLive(driver, vm-def, false,
true);
+if (!migratableDefStr)
+goto cleanup;
+
+if (!(migratableDef = virDomainDefParseString(migratableDefStr, caps,
+ driver-xmlopt,
+ QEMU_EXPECTED_VIRT_TYPES,
+
VIR_DOMAIN_XML_INACTIVE)))
+goto cleanup;
+
+if (!virDomainDefCheckABIStability(migratableDef, def))
goto cleanup;
rv = qemuDomainDefFormatLive(driver, def, false, true);
@@ -2051,6 +2063,8 @@ cleanup:
qemuMigrationCookieFree(mig);
virObjectUnref(caps);
virDomainDefFree(def);
+virDomainDefFree(migratableDef);
+VIR_FREE(migratableDefStr);
return rv;
}
@@ -2179,6 +2193,8 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
const char *listenAddr = NULL;
char *migrateFrom = NULL;
bool abort_on_error = !!(flags VIR_MIGRATE_ABORT_ON_ERROR);
+virDomainDefPtr migratableDef = NULL;
+char *migratableDefStr = NULL;
if (virTimeMillisNow(now) 0)
return -1;
@@ -2213,18 +2229,16 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
/* Let migration hook filter domain XML */
if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) {
-char *xml;
int hookret;
-if (!(xml = qemuDomainDefFormatXML(driver, *def,
- VIR_DOMAIN_XML_SECURE |
- VIR_DOMAIN_XML_MIGRATABLE)))
+if (!(migratableDefStr = qemuDomainDefFormatXML(driver, *def,
+VIR_DOMAIN_XML_SECURE |
+
VIR_DOMAIN_XML_MIGRATABLE)))
goto cleanup;
hookret = virHookCall(VIR_HOOK_DRIVER_QEMU, (*def)-name,
VIR_HOOK_QEMU_OP_MIGRATE, VIR_HOOK_SUBOP_BEGIN,
- NULL, xml, xmlout);
-VIR_FREE(xml);
+ NULL, migratableDefStr, xmlout);
if (hookret 0) {
goto cleanup;
@@ -2242,7 +2256,16 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
if (!newdef)
goto cleanup;
-if (!virDomainDefCheckABIStability(*def, newdef)) {
+migratableDef = virDomainDefParseString(migratableDefStr, caps,
+driver-xmlopt,
+
QEMU_EXPECTED_VIRT_TYPES,
+
VIR_DOMAIN_XML_INACTIVE);
+if (!migratableDef) {
+virDomainDefFree(newdef);
+goto cleanup;
+}
+
+if (!virDomainDefCheckABIStability(migratableDef, newdef)) {
virDomainDefFree(newdef);
goto cleanup;
}
@@ -2429,6 +2452,8 @@ cleanup:
qemuDomainEventQueue(driver, event);
qemuMigrationCookieFree(mig);
virObjectUnref(caps);
+virDomainDefFree(migratableDef);
+VIR_FREE(migratableDefStr);
return ret;
stop:
--
1.8.1.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu_migration: Check ABI stability against MIGRATABLE xml
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 mpriv...@redhat.com --- 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. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
