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

[Cole Robinson]

On 09/27/2013 02:13 AM, Michal Privoznik wrote:
> 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
> 

Thanks, pushed now.

- Cole