Re: [PATCH 3/5] virDomainCheckpointRedefineCommit: Don't check ABI of definition in checkpoint

[Daniel Henrique Barboza <danielhb413@xxxxxxxxx>]

On 12/2/20 11:13 AM, Peter Krempa wrote:
> Checking the definition ABI when redefining checkpoints doesn't make
> much sense for the following reasons:
>
> * the domain definition in the checkpoint is mostly unused (a relic
>    adopted from the snapshot code)
>
> * can be very easily overriden by deleting the checkpoint metadata


s/overriden/overridden

>    before redefinition
>
> Rather than complicating the logic when we'll be taking into account
> that the domain definition may be missing, let's just remove the check.
>
> Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx>
> ---


Reviewed-by: Daniel Henrique Barboza <danielhb413@xxxxxxxxx>


>   src/conf/checkpoint_conf.c | 7 +------
>   src/conf/checkpoint_conf.h | 3 +--
>   src/qemu/qemu_checkpoint.c | 7 +++----
>   src/test/test_driver.c     | 2 +-
>   4 files changed, 6 insertions(+), 13 deletions(-)
>
> diff --git a/src/conf/checkpoint_conf.c b/src/conf/checkpoint_conf.c
> index 8744ac83e1..73fdb78e7a 100644
> --- a/src/conf/checkpoint_conf.c
> +++ b/src/conf/checkpoint_conf.c
> @@ -545,8 +545,7 @@ virDomainCheckpointRedefinePrep(virDomainObjPtr vm,
>
>   virDomainMomentObjPtr
>   virDomainCheckpointRedefineCommit(virDomainObjPtr vm,
> -                                  virDomainCheckpointDefPtr *defptr,
> -                                  virDomainXMLOptionPtr xmlopt)
> +                                  virDomainCheckpointDefPtr *defptr)
>   {
>       virDomainCheckpointDefPtr def = *defptr;
>       virDomainMomentObjPtr other = NULL;
> @@ -556,10 +555,6 @@ virDomainCheckpointRedefineCommit(virDomainObjPtr vm,
>       other = virDomainCheckpointFindByName(vm->checkpoints, def->parent.name);
>       if (other) {
>           otherdef = virDomainCheckpointObjGetDef(other);
> -        if (!virDomainDefCheckABIStability(otherdef->parent.dom,
> -                                           def->parent.dom, xmlopt))
> -            return NULL;
> -
>           /* Drop and rebuild the parent relationship, but keep all
>            * child relations by reusing chk.  */
>           virDomainMomentDropParent(other);
> diff --git a/src/conf/checkpoint_conf.h b/src/conf/checkpoint_conf.h
> index 631f863151..4508f61b29 100644
> --- a/src/conf/checkpoint_conf.h
> +++ b/src/conf/checkpoint_conf.h
> @@ -97,7 +97,6 @@ virDomainCheckpointRedefinePrep(virDomainObjPtr vm,
>
>   virDomainMomentObjPtr
>   virDomainCheckpointRedefineCommit(virDomainObjPtr vm,
> -                                  virDomainCheckpointDefPtr *defptr,
> -                                  virDomainXMLOptionPtr xmlopt);
> +                                  virDomainCheckpointDefPtr *defptr);
>
>   VIR_ENUM_DECL(virDomainCheckpoint);
> diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
> index e8d18b2e02..1740cadabf 100644
> --- a/src/qemu/qemu_checkpoint.c
> +++ b/src/qemu/qemu_checkpoint.c
> @@ -439,8 +439,7 @@ qemuCheckpointRedefineValidateBitmaps(virDomainObjPtr vm,
>
>
>   static virDomainMomentObjPtr
> -qemuCheckpointRedefine(virQEMUDriverPtr driver,
> -                       virDomainObjPtr vm,
> +qemuCheckpointRedefine(virDomainObjPtr vm,
>                          virDomainCheckpointDefPtr *def,
>                          bool *update_current,
>                          bool validate_bitmaps)
> @@ -452,7 +451,7 @@ qemuCheckpointRedefine(virQEMUDriverPtr driver,
>           qemuCheckpointRedefineValidateBitmaps(vm, *def) < 0)
>           return NULL;
>
> -    return virDomainCheckpointRedefineCommit(vm, def, driver->xmlopt);
> +    return virDomainCheckpointRedefineCommit(vm, def);
>   }
>
>
> @@ -605,7 +604,7 @@ qemuCheckpointCreateXML(virDomainPtr domain,
>           return NULL;
>
>       if (redefine) {
> -        chk = qemuCheckpointRedefine(driver, vm, &def, &update_current, validate_bitmaps);
> +        chk = qemuCheckpointRedefine(vm, &def, &update_current, validate_bitmaps);
>       } else {
>           chk = qemuCheckpointCreate(driver, vm, &def);
>       }
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index 136269de3d..29c4c86b1d 100644
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -8991,7 +8991,7 @@ testDomainCheckpointCreateXML(virDomainPtr domain,
>           if (virDomainCheckpointRedefinePrep(vm, def, &update_current) < 0)
>               goto cleanup;
>
> -        if (!(chk = virDomainCheckpointRedefineCommit(vm, &def, privconn->xmlopt)))
> +        if (!(chk = virDomainCheckpointRedefineCommit(vm, &def)))
>               goto cleanup;
>       } else {
>           if (!(def->parent.dom = virDomainDefCopy(vm->def,