4:35 p.m.
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