[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v3 4/8] test: Implement snapshot create/delete/revert APIs



On 25.09.2013 21:15, Cole Robinson wrote:
> Again stolen from qemu_driver.c, but dropping all the unneeded bits.
> This aims to copy all the current qemu validation checks since that's
> the most commonly used real driver, but some of the checks are
> completely artificial in the test driver.
> 
> This only supports creation of internal snapshots for initial
> simplicity.
> ---
> 
> v3:
>     Use STRNEQ_NULLABLE for domain_conf.c change
> 
>  src/conf/domain_conf.c |   2 +-
>  src/test/test_driver.c | 504 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 504 insertions(+), 2 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 70fdafc..dbf239e 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -13515,7 +13515,7 @@ virDomainDefCheckABIStability(virDomainDefPtr src,
>                         virArchToString(src->os.arch));
>          return false;
>      }
> -    if (STRNEQ(src->os.machine, dst->os.machine)) {
> +    if (STRNEQ_NULLABLE(src->os.machine, dst->os.machine)) {
>          virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                         _("Target domain OS type %s does not match source %s"),
>                         dst->os.machine, src->os.machine);
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index 9a39087..1b79b70 100644
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -2860,9 +2860,11 @@ static int testDomainUndefineFlags(virDomainPtr domain,
>      testConnPtr privconn = domain->conn->privateData;
>      virDomainObjPtr privdom;
>      virDomainEventPtr event = NULL;
> +    int nsnapshots;
>      int ret = -1;
>  
> -    virCheckFlags(VIR_DOMAIN_UNDEFINE_MANAGED_SAVE, -1);
> +    virCheckFlags(VIR_DOMAIN_UNDEFINE_MANAGED_SAVE |
> +                  VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA, -1);
>  
>      testDriverLock(privconn);
>      privdom = virDomainObjListFindByName(privconn->domains,
> @@ -2882,6 +2884,24 @@ static int testDomainUndefineFlags(virDomainPtr domain,
>      }
>      privdom->hasManagedSave = false;

This is exactly the problem I'm describing in 2/8. If something below
fails, we've undefined the managed save image.

>  
> +    /* Requiring an inactive VM is part of the documented API for
> +     * UNDEFINE_SNAPSHOTS_METADATA
> +     */
> +    if (!virDomainObjIsActive(privdom) &&
> +        (nsnapshots = virDomainSnapshotObjListNum(privdom->snapshots,
> +                                                  NULL, 0))) {
> +        if (!(flags & VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA)) {
> +            virReportError(VIR_ERR_OPERATION_INVALID,
> +                           _("cannot delete inactive domain with %d "
> +                             "snapshots"),
> +                           nsnapshots);
> +            goto cleanup;
> +        }
> +
> +        /* There isn't actually anything to do, we are just emulating qemu
> +         * behavior here. */
> +    }
> +
>      event = virDomainEventNewFromObj(privdom,
>                                       VIR_DOMAIN_EVENT_UNDEFINED,
>                                       VIR_DOMAIN_EVENT_UNDEFINED_REMOVED);
> @@ -6475,6 +6495,485 @@ cleanup:
>      return ret;
>  }
>  
> +static int
> +testDomainSnapshotAlignDisks(virDomainObjPtr vm,
> +                             virDomainSnapshotDefPtr def,
> +                             unsigned int flags)
> +{
> +    int align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL;
> +    int align_match = true;
> +
> +    if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) {
> +        align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
> +        align_match = false;
> +        if (virDomainObjIsActive(vm))
> +            def->state = VIR_DOMAIN_DISK_SNAPSHOT;
> +        else
> +            def->state = VIR_DOMAIN_SHUTOFF;
> +        def->memory = VIR_DOMAIN_SNAPSHOT_LOCATION_NONE;
> +    } else if (def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
> +        def->state = virDomainObjGetState(vm, NULL);
> +        align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
> +        align_match = false;
> +    } else {
> +        def->state = virDomainObjGetState(vm, NULL);
> +        def->memory = (def->state == VIR_DOMAIN_SHUTOFF ?
> +                       VIR_DOMAIN_SNAPSHOT_LOCATION_NONE :
> +                       VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL);

Needless parentheses.

> +    }
> +
> +    return virDomainSnapshotAlignDisks(def, align_location, align_match);
> +}
> +
> +static virDomainSnapshotPtr
> +testDomainSnapshotCreateXML(virDomainPtr domain,
> +                            const char *xmlDesc,
> +                            unsigned int flags)
> +{
> +    testConnPtr privconn = domain->conn->privateData;
> +    virDomainObjPtr vm = NULL;
> +    virDomainSnapshotDefPtr def = NULL;
> +    virDomainSnapshotObjPtr snap = NULL;
> +    virDomainSnapshotPtr snapshot = NULL;
> +    virDomainEventPtr event = NULL;
> +    char *xml = NULL;
> +    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
> +     *
> +     * HALT: Implemented
> +     * QUIESCE: Nothing to do
> +     * ATOMIC: Nothing to do
> +     * LIVE: Nothing to do
> +     */
> +    virCheckFlags(
> +        VIR_DOMAIN_SNAPSHOT_CREATE_HALT |
> +        VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE |
> +        VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC |
> +        VIR_DOMAIN_SNAPSHOT_CREATE_LIVE, NULL);
> +
> +    if (!(vm = testDomObjFromDomain(domain)))
> +        goto cleanup;
> +
> +    testDriverLock(privconn);

No need to lock the driver this soon.

> +
> +    if (!vm->persistent && (flags & VIR_DOMAIN_SNAPSHOT_CREATE_HALT)) {
> +        virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                       _("cannot halt after transient domain snapshot"));
> +        goto cleanup;
> +    }

IE vm is locked here and this has no impact on the driver.

> +
> +    if (!(def = virDomainSnapshotDefParseString(xmlDesc,
> +                                                privconn->caps,
> +                                                privconn->xmlopt,
> +                                                1 << VIR_DOMAIN_VIRT_TEST,
> +                                                parse_flags)))
> +        goto cleanup;

Neither has this ...

> +
> +    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;

.. or this ..
BTW we have virDomainDefCopy().


> +
> +    if (testDomainSnapshotAlignDisks(vm, def, flags) < 0)
> +        goto cleanup;
> +

If you intended the driver lock as mutex for vm->snapshots I don't think
it's necessary since vm is locked throughout the whole API. So there is
no chance for other API to hop in and change the vm->snapshots.

> +    if (!(snap = virDomainSnapshotAssignDef(vm->snapshots, def)))
> +        goto cleanup;
> +    def = NULL;

I don't think this is gonna work. If flags has _REDEFINE flag set, the
virDomainSnapshotAssignDef() will fail and the code below won't get any
chance to reverse the upcoming error.

> +
> +    if (vm->current_snapshot) {
> +        if (!(flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) &&
> +            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:
> +    VIR_FREE(xml);
> +    if (vm) {
> +        if (snapshot) {
> +            virDomainSnapshotObjPtr other;
> +            vm->current_snapshot = snap;
> +            other = virDomainSnapshotFindByName(vm->snapshots,
> +                                                snap->def->parent);
> +            snap->parent = other;
> +            other->nchildren++;
> +            snap->sibling = other->first_child;
> +            other->first_child = snap;
> +        }
> +        virObjectUnlock(vm);
> +    }
> +    if (event) {

In fact this is the only place that require test driver to be locked.

> +        testDomainEventQueue(privconn, event);
> +    }
> +    testDriverUnlock(privconn);
> +    virDomainSnapshotDefFree(def);
> +    return snapshot;
> +}
> +
> +
> +typedef struct _testSnapRemoveData testSnapRemoveData;
> +typedef testSnapRemoveData *testSnapRemoveDataPtr;
> +struct _testSnapRemoveData {
> +    virDomainObjPtr vm;
> +    bool current;
> +};
> +
> +static void
> +testDomainSnapshotDiscard(void *payload,
> +                          const void *name ATTRIBUTE_UNUSED,
> +                          void *data)

Please keep the 'All' suffix just like the qemu counterpart has. It made
me wondering why is the function header different to
qemuDomainSnapshotDiscard, while I needed to look at
qemuDomainSnaphostDiscardAll with the very same header.

> +{
> +    virDomainSnapshotObjPtr snap = payload;
> +    testSnapRemoveDataPtr curr = data;
> +
> +    if (snap->def->current)
> +        curr->current = true;
> +    virDomainSnapshotObjListRemove(curr->vm->snapshots, snap);
> +}
> +
> +typedef struct _testSnapReparentData testSnapReparentData;
> +typedef testSnapReparentData *testSnapReparentDataPtr;
> +struct _testSnapReparentData {
> +    virDomainSnapshotObjPtr parent;
> +    virDomainObjPtr vm;
> +    int err;
> +    virDomainSnapshotObjPtr last;
> +};
> +
> +static void
> +testDomainSnapshotReparentChildren(void *payload,
> +                                   const void *name ATTRIBUTE_UNUSED,
> +                                   void *data)
> +{
> +    virDomainSnapshotObjPtr snap = payload;
> +    testSnapReparentDataPtr rep = data;
> +
> +    if (rep->err < 0) {
> +        return;
> +    }
> +
> +    VIR_FREE(snap->def->parent);
> +    snap->parent = rep->parent;
> +
> +    if (rep->parent->def &&
> +        VIR_STRDUP(snap->def->parent, rep->parent->def->name) < 0) {
> +        rep->err = -1;
> +        return;
> +    }
> +
> +    if (!snap->sibling)
> +        rep->last = snap;
> +}
> +
> +static int
> +testDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
> +                         unsigned int flags)
> +{
> +    virDomainObjPtr vm = NULL;
> +    virDomainSnapshotObjPtr snap = NULL;
> +    virDomainSnapshotObjPtr parentsnap = NULL;
> +    int ret = -1;
> +
> +    virCheckFlags(VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN |
> +                  VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY, -1);
> +
> +    if (!(vm = testDomObjFromSnapshot(snapshot)))
> +        return -1;
> +
> +    if (!(snap = testSnapObjFromSnapshot(vm, snapshot)))
> +        goto cleanup;
> +
> +    if (flags & (VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN |
> +                 VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY)) {
> +        testSnapRemoveData rem;
> +        rem.vm = vm;
> +        rem.current = false;
> +        virDomainSnapshotForEachDescendant(snap,
> +                                           testDomainSnapshotDiscard,

s/Discard/DiscardAll/

> +                                           &rem);
> +        if (rem.current) {
> +            if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) {
> +                snap->def->current = true;
> +            }
> +            vm->current_snapshot = snap;
> +        }
> +    } else if (snap->nchildren) {
> +        testSnapReparentData rep;
> +        rep.parent = snap->parent;
> +        rep.vm = vm;
> +        rep.err = 0;
> +        rep.last = NULL;
> +        virDomainSnapshotForEachChild(snap,
> +                                      testDomainSnapshotReparentChildren,
> +                                      &rep);
> +        if (rep.err < 0)
> +            goto cleanup;
> +
> +        /* Can't modify siblings during ForEachChild, so do it now.  */
> +        snap->parent->nchildren += snap->nchildren;
> +        rep.last->sibling = snap->parent->first_child;
> +        snap->parent->first_child = snap->first_child;
> +    }
> +
> +    if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) {
> +        snap->nchildren = 0;
> +        snap->first_child = NULL;
> +    } else {
> +        virDomainSnapshotDropParent(snap);
> +        if (snap == vm->current_snapshot) {
> +            if (snap->def->parent) {
> +                parentsnap = virDomainSnapshotFindByName(vm->snapshots,
> +                                                         snap->def->parent);
> +                if (!parentsnap) {
> +                    VIR_WARN("missing parent snapshot matching name '%s'",
> +                             snap->def->parent);
> +                } else {
> +                    parentsnap->def->current = true;
> +                }
> +            }
> +            vm->current_snapshot = parentsnap;
> +        }
> +        virDomainSnapshotObjListRemove(vm->snapshots, snap);
> +    }
> +
> +    ret = 0;
> +cleanup:
> +    if (vm)
> +        virObjectUnlock(vm);
> +    return ret;
> +}
> +
> +static int
> +testDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
> +                           unsigned int flags)
> +{
> +    testConnPtr privconn = snapshot->domain->conn->privateData;
> +    virDomainObjPtr vm = NULL;
> +    virDomainSnapshotObjPtr snap = NULL;
> +    virDomainEventPtr event = NULL;
> +    virDomainEventPtr event2 = NULL;
> +    virDomainDefPtr config = NULL;
> +    int ret = -1;
> +
> +    virCheckFlags(VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING |
> +                  VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED |
> +                  VIR_DOMAIN_SNAPSHOT_REVERT_FORCE, -1);
> +
> +    /* We have the following transitions, which create the following events:
> +     * 1. inactive -> inactive: none
> +     * 2. inactive -> running:  EVENT_STARTED
> +     * 3. inactive -> paused:   EVENT_STARTED, EVENT_PAUSED
> +     * 4. running  -> inactive: EVENT_STOPPED
> +     * 5. running  -> running:  none
> +     * 6. running  -> paused:   EVENT_PAUSED
> +     * 7. paused   -> inactive: EVENT_STOPPED
> +     * 8. paused   -> running:  EVENT_RESUMED
> +     * 9. paused   -> paused:   none
> +     * Also, several transitions occur even if we fail partway through,
> +     * and use of FORCE can cause multiple transitions.
> +     */
> +
> +    if (!(vm = testDomObjFromSnapshot(snapshot)))
> +        return -1;
> +
> +    if (!(snap = testSnapObjFromSnapshot(vm, snapshot)))
> +        goto cleanup;
> +
> +    testDriverLock(privconn);

This looks suspicious again.

> +
> +    if (!vm->persistent &&
> +        snap->def->state != VIR_DOMAIN_RUNNING &&
> +        snap->def->state != VIR_DOMAIN_PAUSED &&
> +        (flags & (VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING |
> +                  VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)) == 0) {
> +        virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                       _("transient domain needs to request run or pause "
> +                         "to revert to inactive snapshot"));
> +        goto cleanup;
> +    }
> +
> +    if (!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_FORCE)) {
> +        if (!snap->def->dom) {
> +            virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY,
> +                           _("snapshot '%s' lacks domain '%s' rollback info"),
> +                           snap->def->name, vm->def->name);
> +            goto cleanup;
> +        }
> +        if (virDomainObjIsActive(vm) &&
> +            !(snap->def->state == VIR_DOMAIN_RUNNING
> +              || snap->def->state == VIR_DOMAIN_PAUSED) &&
> +            (flags & (VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING |
> +                      VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED))) {
> +            virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY, "%s",
> +                           _("must respawn guest to start inactive snapshot"));
> +            goto cleanup;
> +        }
> +    }
> +
> +
> +    if (vm->current_snapshot) {
> +        vm->current_snapshot->def->current = false;
> +        vm->current_snapshot = NULL;
> +    }
> +
> +    snap->def->current = true;
> +    config = virDomainDefCopy(snap->def->dom,
> +                              privconn->caps, privconn->xmlopt, true);
> +    if (!config)
> +        goto cleanup;
> +
> +    if (snap->def->state == VIR_DOMAIN_RUNNING ||
> +        snap->def->state == VIR_DOMAIN_PAUSED) {
> +        /* Transitions 2, 3, 5, 6, 8, 9 */
> +        bool was_running = false;
> +        bool was_stopped = false;
> +
> +        if (virDomainObjIsActive(vm)) {
> +            /* Transitions 5, 6, 8, 9 */
> +            /* Check for ABI compatibility.  */
> +            if (!virDomainDefCheckABIStability(vm->def, config)) {
> +                virErrorPtr err = virGetLastError();
> +
> +                if (!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_FORCE)) {
> +                    /* Re-spawn error using correct category. */
> +                    if (err->code == VIR_ERR_CONFIG_UNSUPPORTED)
> +                        virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY, "%s",
> +                                       err->str2);
> +                    goto cleanup;
> +                }
> +
> +                virResetError(err);
> +                testDomainShutdownState(snapshot->domain, vm,
> +                                        VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT);
> +                event = virDomainEventNewFromObj(vm,
> +                            VIR_DOMAIN_EVENT_STOPPED,
> +                            VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT);
> +                if (event)
> +                    testDomainEventQueue(privconn, event);
> +                goto load;
> +            }
> +
> +            if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) {
> +                /* Transitions 5, 6 */
> +                was_running = true;
> +                virDomainObjSetState(vm, VIR_DOMAIN_PAUSED,
> +                                     VIR_DOMAIN_PAUSED_FROM_SNAPSHOT);

Shouldn't this be testDomainShutdownState() instead of
virDomainObjSetState()?

> +                /* Create an event now in case the restore fails, so
> +                 * that user will be alerted that they are now paused.
> +                 * If restore later succeeds, we might replace this. */
> +                event = virDomainEventNewFromObj(vm,
> +                                VIR_DOMAIN_EVENT_SUSPENDED,
> +                                VIR_DOMAIN_EVENT_SUSPENDED_FROM_SNAPSHOT);
> +            }
> +            virDomainObjAssignDef(vm, config, false, NULL);
> +
> +        } else {
> +            /* Transitions 2, 3 */
> +        load:
> +            was_stopped = true;
> +            virDomainObjAssignDef(vm, config, false, NULL);
> +            if (testDomainStartState(privconn, vm,
> +                                VIR_DOMAIN_RUNNING_FROM_SNAPSHOT) < 0)
> +                goto cleanup;
> +            event = virDomainEventNewFromObj(vm,
> +                                VIR_DOMAIN_EVENT_STARTED,
> +                                VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT);
> +        }
> +
> +        /* Touch up domain state.  */
> +        if (!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING) &&
> +            (snap->def->state == VIR_DOMAIN_PAUSED ||
> +             (flags & VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED))) {
> +            /* Transitions 3, 6, 9 */
> +            virDomainObjSetState(vm, VIR_DOMAIN_PAUSED,
> +                                 VIR_DOMAIN_PAUSED_FROM_SNAPSHOT);
> +            if (was_stopped) {
> +                /* Transition 3, use event as-is and add event2 */
> +                event2 = virDomainEventNewFromObj(vm,
> +                                VIR_DOMAIN_EVENT_SUSPENDED,
> +                                VIR_DOMAIN_EVENT_SUSPENDED_FROM_SNAPSHOT);
> +            } /* else transition 6 and 9 use event as-is */
> +        } else {
> +            /* Transitions 2, 5, 8 */
> +            virDomainEventFree(event);
> +            event = NULL;
> +
> +            if (was_stopped) {
> +                /* Transition 2 */
> +                event = virDomainEventNewFromObj(vm,
> +                                VIR_DOMAIN_EVENT_STARTED,
> +                                VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT);
> +            } else if (was_running) {
> +                /* Transition 8 */
> +                event = virDomainEventNewFromObj(vm,
> +                                VIR_DOMAIN_EVENT_RESUMED,
> +                                VIR_DOMAIN_EVENT_RESUMED);
> +            }
> +        }
> +    } else {
> +        /* Transitions 1, 4, 7 */
> +        virDomainObjAssignDef(vm, config, false, NULL);
> +
> +        if (virDomainObjIsActive(vm)) {
> +            /* Transitions 4, 7 */
> +            testDomainShutdownState(snapshot->domain, vm,
> +                                    VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT);
> +            event = virDomainEventNewFromObj(vm,
> +                                    VIR_DOMAIN_EVENT_STOPPED,
> +                                    VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT);
> +        }
> +
> +        if (flags & (VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING |
> +                     VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)) {
> +            /* Flush first event, now do transition 2 or 3 */
> +            bool paused = (flags & VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED) != 0;
> +
> +            if (event)
> +                testDomainEventQueue(privconn, event);
> +            event = virDomainEventNewFromObj(vm,
> +                            VIR_DOMAIN_EVENT_STARTED,
> +                            VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT);
> +            if (paused) {
> +                event2 = virDomainEventNewFromObj(vm,
> +                                VIR_DOMAIN_EVENT_SUSPENDED,
> +                                VIR_DOMAIN_EVENT_SUSPENDED_FROM_SNAPSHOT);
> +            }
> +        }
> +    }
> +
> +    vm->current_snapshot = snap;
> +    ret = 0;
> +cleanup:
> +    if (event) {
> +        testDomainEventQueue(privconn, event);
> +        if (event2)
> +            testDomainEventQueue(privconn, event2);
> +    }
> +    virObjectUnlock(vm);
> +    testDriverUnlock(privconn);
> +
> +    return ret;
> +}
> +
> +
>  
>  static virDriver testDriver = {
>      .no = VIR_DRV_TEST,
> @@ -6566,6 +7065,9 @@ static virDriver testDriver = {
>      .domainSnapshotCurrent = testDomainSnapshotCurrent, /* 1.1.3 */
>      .domainSnapshotIsCurrent = testDomainSnapshotIsCurrent, /* 1.1.3 */
>      .domainSnapshotHasMetadata = testDomainSnapshotHasMetadata, /* 1.1.3 */
> +    .domainSnapshotCreateXML = testDomainSnapshotCreateXML, /* 1.1.3 */
> +    .domainRevertToSnapshot = testDomainRevertToSnapshot, /* 1.1.3 */
> +    .domainSnapshotDelete = testDomainSnapshotDelete, /* 1.1.3 */
>  };
>  
>  static virNetworkDriver testNetworkDriver = {
> 

ACK if you address the nits I've pointed out.

Michal


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]