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

Re: [libvirt] [PATCHv2 variant 1] snapshot: implement new APIs for esx and vbox



2012/6/12 Eric Blake <eblake redhat com>:
> The two new APIs are rather trivial; based on bits and pieces of
> other existing APIs.  But rather than blindly return 0 or 1 for
> HasMetadata, I chose to first validate that the snapshot in
> question in fact exists.  In the process, I noticed other APIs
> that were blindly succeeding instead of checking for existence.
>
> * src/esx/esx_driver.c (esxDomainSnapshotIsCurrent)
> (esxDomainSnapshotHasMetadata): New functions.
> (esxDomainIsUpdated, esxDomainIsPersistent): Add existence checks.
> * src/vbox/vbox_tmpl.c (vboxDomainSnapshotIsCurrent)
> (vboxDomainSnapshotHasMetadata): New functions.
> (vboxDomainIsPersistent, vboxDomainIsUpdated): Add existence checks.
> ---
>
> v2: add existence checks to other functions
> I like this version better.  However, while it compiles, I'm
> completely unable to runtime test it, so I'd appreciate a
> good review.
>
>  src/esx/esx_driver.c |  116 ++++++++++++++++++++++++++++++++++++++--
>  src/vbox/vbox_tmpl.c |  145 ++++++++++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 253 insertions(+), 8 deletions(-)

> diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
> index 4b0ee2e..8b7415a 100644
> --- a/src/vbox/vbox_tmpl.c
> +++ b/src/vbox/vbox_tmpl.c

> +static int
> +vboxDomainSnapshotHasMetadata(virDomainSnapshotPtr snapshot,
> +                              unsigned int flags)
> +{
> +    virDomainPtr dom = snapshot->domain;
> +    VBOX_OBJECT_CHECK(dom->conn, int, -1);
> +    vboxIID iid = VBOX_IID_INITIALIZER;
> +    IMachine *machine = NULL;
> +    ISnapshot *snap = NULL;
> +    nsresult rc;
> +
> +    virCheckFlags(0, -1);
> +
> +    vboxIIDFromUUID(&iid, dom->uuid);
> +    rc = VBOX_OBJECT_GET_MACHINE(iid.value, &machine);
> +    if (NS_FAILED(rc)) {
> +        vboxError(VIR_ERR_NO_DOMAIN, "%s",
> +                  _("no domain with matching UUID"));
> +        goto cleanup;
> +    }
> +
> +    /* Check that snapshot exists.  If so, there is no metadata.  */
> +    if (!(snap = vboxDomainSnapshotGet(data, dom, machine, snapshot->name)))
> +        goto cleanup;
> +
> +    ret = 0;
> +
> +cleanup:

VBOX_RELEASE(snap); is missing here.

> +    VBOX_RELEASE(machine);
> +    vboxIIDUnalloc(&iid);
> +    return ret;
> +}

Tested and works. ACK.

-- 
Matthias Bolte
http://photron.blogspot.com


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