Re: [libvirt] [PATCH] qemu: Introduce qemuDomainDefCheckABIStability

2013-10-10 Thread Eric Blake
On 10/10/2013 03:23 AM, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=994364
> 
> Whenever we check for ABI stability, we have new xml (e.g. provided by
> user, or obtained from snapshot, whatever) which we compare to old xml
> and see if ABI won't break. However, if the new xml was produced via
> virDomainGetXMLDesc(..., VIR_DOMAIN_XML_MIGRATABLE) it lacks some
> devices, e.g. 'pci-root' controller. Hence, the ABI stability check
> fails even though it is stable. Moreover, we can't simply fix
> virDomainDefCheckABIStability because removing the correct devices is
> task for the driver. For instance, qemu driver wants to remove the usb
> controller too, while LXC driver doesn't. That's why we need special
> qemu wrapper over virDomainDefCheckABIStability which removes the
> correct devices from domain XML, produces MIGRATABLE xml and calls the
> check ABI stability function.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/qemu/qemu_domain.c| 22 ++
>  src/qemu/qemu_domain.h|  3 +++
>  src/qemu/qemu_driver.c| 10 ++
>  src/qemu/qemu_migration.c |  4 ++--
>  4 files changed, 29 insertions(+), 10 deletions(-)

Yes, this will make life simpler for all callers.  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

[libvirt] [PATCH] qemu: Introduce qemuDomainDefCheckABIStability

2013-10-10 Thread Michal Privoznik
https://bugzilla.redhat.com/show_bug.cgi?id=994364

Whenever we check for ABI stability, we have new xml (e.g. provided by
user, or obtained from snapshot, whatever) which we compare to old xml
and see if ABI won't break. However, if the new xml was produced via
virDomainGetXMLDesc(..., VIR_DOMAIN_XML_MIGRATABLE) it lacks some
devices, e.g. 'pci-root' controller. Hence, the ABI stability check
fails even though it is stable. Moreover, we can't simply fix
virDomainDefCheckABIStability because removing the correct devices is
task for the driver. For instance, qemu driver wants to remove the usb
controller too, while LXC driver doesn't. That's why we need special
qemu wrapper over virDomainDefCheckABIStability which removes the
correct devices from domain XML, produces MIGRATABLE xml and calls the
check ABI stability function.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_domain.c| 22 ++
 src/qemu/qemu_domain.h|  3 +++
 src/qemu/qemu_driver.c| 10 ++
 src/qemu/qemu_migration.c |  4 ++--
 4 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 968e323..f3e2ba1 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2342,3 +2342,25 @@ qemuDomainUpdateDeviceList(virQEMUDriverPtr driver,
 priv->qemuDevices = aliases;
 return 0;
 }
+
+bool
+qemuDomainDefCheckABIStability(virQEMUDriverPtr driver,
+   virDomainDefPtr src,
+   virDomainDefPtr dst)
+{
+virDomainDefPtr migratableDefSrc = NULL;
+virDomainDefPtr migratableDefDst = NULL;
+const int flags = VIR_DOMAIN_XML_SECURE | VIR_DOMAIN_XML_UPDATE_CPU | 
VIR_DOMAIN_XML_MIGRATABLE;
+bool ret = false;
+
+if (!(migratableDefSrc = qemuDomainDefCopy(driver, src, flags)) ||
+!(migratableDefDst = qemuDomainDefCopy(driver, dst, flags)))
+goto cleanup;
+
+ret = virDomainDefCheckABIStability(migratableDefSrc, migratableDefDst);
+
+cleanup:
+virDomainDefFree(migratableDefSrc);
+virDomainDefFree(migratableDefDst);
+return ret;
+}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 21f116c..5d9fab9 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -368,4 +368,7 @@ extern virDomainDefParserConfig 
virQEMUDriverDomainDefParserConfig;
 int qemuDomainUpdateDeviceList(virQEMUDriverPtr driver,
virDomainObjPtr vm);
 
+bool qemuDomainDefCheckABIStability(virQEMUDriverPtr driver,
+virDomainDefPtr src,
+virDomainDefPtr dst);
 #endif /* __QEMU_DOMAIN_H__ */
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c71aecc..8481591 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3077,7 +3077,7 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, 
virDomainPtr dom,
 VIR_DOMAIN_XML_INACTIVE))) {
 goto endjob;
 }
-if (!virDomainDefCheckABIStability(vm->def, def)) {
+if (!qemuDomainDefCheckABIStability(driver, vm->def, def)) {
 virDomainDefFree(def);
 goto endjob;
 }
@@ -12889,7 +12889,6 @@ static int 
qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
 qemuDomainObjPrivatePtr priv;
 int rc;
 virDomainDefPtr config = NULL;
-virDomainDefPtr migratableDef = NULL;
 virQEMUDriverConfigPtr cfg = NULL;
 virCapsPtr caps = NULL;
 
@@ -13006,11 +13005,7 @@ static int 
qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
 /* Transitions 5, 6, 8, 9 */
 /* Check for ABI compatibility. We need to do this check against
  * the migratable XML or it will always fail otherwise */
-if (!(migratableDef = qemuDomainDefCopy(driver, vm->def,
-
VIR_DOMAIN_XML_MIGRATABLE)))
-goto cleanup;
-
-if (config && !virDomainDefCheckABIStability(migratableDef, 
config)) {
+if (config && !qemuDomainDefCheckABIStability(driver, vm->def, 
config)) {
 virErrorPtr err = virGetLastError();
 
 if (!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_FORCE)) {
@@ -13215,7 +13210,6 @@ cleanup:
 }
 if (vm)
 virObjectUnlock(vm);
-virDomainDefFree(migratableDef);
 virObjectUnref(caps);
 virObjectUnref(cfg);
 
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 3a1aab7..59fd263 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2039,7 +2039,7 @@ static char
 VIR_DOMAIN_XML_INACTIVE)))
 goto cleanup;
 
-if (!virDomainDefCheckABIStability(vm->def, def))
+if (!qemuDomainDefCheckABIStability(driver, vm->def, def))
 goto cleanup;
 
 rv = qemuDomainDefFormatLive(driver, def, false, true);
@@ -