[libvirt] [PATCH 1/2] qemu: Fix checking of ABI stability when restoring external checkpoints

[Ján Tomko]

On 09/16/2013 02:36 PM, Peter Krempa wrote:
> External checkpoints have a bug in the implementation where they use the
> normal definition instead of the "migratable" one. This causes errors
> when the snapshot is being reverted using the workaround method via
> qemuDomainRestoreFlags() with a custom XML. This issue was introduced
> when commit 07966f6a8b5ccb5bb4c716b25deb8ba2e572cc67 changed the code to
> compare "migratable" XMLs from the user as we should have used
> migratable in the image too.
> 
> This patch adds a compatibility layer, so that fixing the snapshot code
> won't make existing snapshots fail to load.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1008340
> ---
>  src/qemu/qemu_driver.c | 22 +++++++++++++++++++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index ae1948f..22497f0 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -5251,14 +5251,30 @@ qemuDomainSaveImageOpen(virQEMUDriverPtr driver,
>              goto error;
> 
>          newdef = qemuDomainDefCopy(driver, def2, VIR_DOMAIN_XML_MIGRATABLE);
> -        virDomainDefFree(def2);

You removed the freeing of def2 here and only free it on errors.

> -        if (!newdef)
> +        if (!newdef) {
> +            virDomainDefFree(def2);
>              goto error;
> +        }
> 
>          if (!virDomainDefCheckABIStability(def, newdef)) {
>              virDomainDefFree(newdef);
> -            goto error;
> +            virResetLastError();
> +
> +            /* Due to a bug in external snapshot creation code, the XML saved
> +             * in the save image was not a migratable XML. To ensure backwards
> +             * compatibility with the change of the saved XML type, we need
> +             * to check the ABI compatibility against the user provided XML if
> +             * the check against the migratable XML fails. */
> +            if (!virDomainDefCheckABIStability(def, def2)) {
> +                virDomainDefFree(def2);
> +                goto error;
> +            }
> +
> +            /* use the user provided XML */
> +            newdef = def2;

> +            def2 = NULL;

def2 is not used after this assignment.

>          }
> +
>          virDomainDefFree(def);
>          def = newdef;
>      }
> 

Jan