[libvirt] [PATCH v3 7/8] test: snapshot: Add REDEFINE support

[Michal Privoznik]

On 25.09.2013 21:15, Cole Robinson wrote:
> ---
>  src/test/test_driver.c | 67 ++++++++++++++++++++++++++++++++------------------
>  1 file changed, 43 insertions(+), 24 deletions(-)
> 
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index 1b79b70..8a690fd 100644
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -6537,26 +6537,35 @@ testDomainSnapshotCreateXML(virDomainPtr domain,
>      virDomainSnapshotPtr snapshot = NULL;
>      virDomainEventPtr event = NULL;
>      char *xml = NULL;
> +    bool update_current = true;
> +    bool redefine = flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE;

Oh, so you'd done what I suggested in 5/8. Nice.

>      unsigned int parse_flags = VIR_DOMAIN_SNAPSHOT_PARSE_DISKS;
>  
>      /*
> -     * REDEFINE + CURRENT: Not implemented yet
>       * DISK_ONLY: Not implemented yet
>       * REUSE_EXT: Not implemented yet
>       *
>       * NO_METADATA: Explicitly not implemented
>       *
> +     * REDEFINE + CURRENT: Implemented
>       * HALT: Implemented
>       * QUIESCE: Nothing to do
>       * ATOMIC: Nothing to do
>       * LIVE: Nothing to do
>       */
>      virCheckFlags(
> +        VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE |
> +        VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT |
>          VIR_DOMAIN_SNAPSHOT_CREATE_HALT |
>          VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE |
>          VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC |
>          VIR_DOMAIN_SNAPSHOT_CREATE_LIVE, NULL);
>  
> +    if ((redefine && !(flags & VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT)))
> +        update_current = false;
> +    if (redefine)
> +        parse_flags |= VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE;
> +
>      if (!(vm = testDomObjFromDomain(domain)))
>          goto cleanup;
>  
> @@ -6575,34 +6584,43 @@ testDomainSnapshotCreateXML(virDomainPtr domain,
>                                                  parse_flags)))
>          goto cleanup;
>  
> -    if (!(xml = virDomainDefFormat(vm->def, VIR_DOMAIN_XML_INACTIVE)) ||
> -        !(def->dom = virDomainDefParseString(xml,
> -                                             privconn->caps,
> -                                             privconn->xmlopt,
> -                                             1 << VIR_DOMAIN_VIRT_TEST,
> -                                             VIR_DOMAIN_XML_INACTIVE)))
> -        goto cleanup;
> -
> -    if (testDomainSnapshotAlignDisks(vm, def, flags) < 0)
> -        goto cleanup;
> -
> -    if (!(snap = virDomainSnapshotAssignDef(vm->snapshots, def)))
> -        goto cleanup;
> -    def = NULL;
> +    if (redefine) {
> +        if (!virDomainSnapshotRedefinePrep(domain, vm, &def, &snap,
> +                                           &update_current, flags) < 0)
> +            goto cleanup;
> +    } else {
> +        if (!(xml = virDomainDefFormat(vm->def, VIR_DOMAIN_XML_INACTIVE)) ||
> +            !(def->dom = virDomainDefParseString(xml,
> +                                                 privconn->caps,
> +                                                 privconn->xmlopt,
> +                                                 1 << VIR_DOMAIN_VIRT_TEST,
> +                                                 VIR_DOMAIN_XML_INACTIVE)))
> +            goto cleanup;
>  
> -    if (vm->current_snapshot) {
> -        if (!(flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) &&
> -            VIR_STRDUP(snap->def->parent, vm->current_snapshot->def->name) < 0)
> +        if (testDomainSnapshotAlignDisks(vm, def, flags) < 0)
>              goto cleanup;
>      }
>  
> -    if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_HALT) &&
> -        virDomainObjIsActive(vm)) {
> -        testDomainShutdownState(domain, vm, VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT);
> -        event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED,
> -                                        VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT);
> +    if (!snap) {
> +        if (!(snap = virDomainSnapshotAssignDef(vm->snapshots, def)))
> +            goto cleanup;
> +        def = NULL;
>      }

This fixes the bug I'm mentioning in 4/8.

>  
> +    if (!redefine) {
> +        if (vm->current_snapshot &&
> +            (VIR_STRDUP(snap->def->parent,
> +                        vm->current_snapshot->def->name) < 0))
> +            goto cleanup;
> +
> +        if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_HALT) &&
> +            virDomainObjIsActive(vm)) {
> +            testDomainShutdownState(domain, vm,
> +                                    VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT);
> +            event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED,
> +                                    VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT);
> +        }
> +    }
>  
>      snapshot = virGetDomainSnapshot(domain, snap->def->name);
>  cleanup:
> @@ -6610,7 +6628,8 @@ cleanup:
>      if (vm) {
>          if (snapshot) {
>              virDomainSnapshotObjPtr other;
> -            vm->current_snapshot = snap;
> +            if (update_current)
> +                vm->current_snapshot = snap;
>              other = virDomainSnapshotFindByName(vm->snapshots,
>                                                  snap->def->parent);
>              snap->parent = other;
> 

ACK

Michal