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

Re: [libvirt] [PATCHv2 7/7] snapshot: implement snapshot children listing in qemu



On Fri, Sep 30, 2011 at 05:09:29PM -0600, Eric Blake wrote:
> Not too hard to wire up.  The trickiest part is realizing that
> listing children of a snapshot cannot use SNAPSHOT_LIST_ROOTS,
> and that we overloaded that bit to also mean SNAPSHOT_LIST_DESCENDANTS;
> we use that bit to decide which iteration to use, but don't want
> the existing counting/listing functions to see that bit.
> 
> * src/conf/domain_conf.h (virDomainSnapshotObjListNumFrom)
> (virDomainSnapshotObjListGetNamesFrom): New prototypes.
> * src/conf/domain_conf.c (virDomainSnapshotObjListNumFrom)
> (virDomainSnapshotObjListGetNamesFrom): New functions.
> * src/libvirt_private.syms (domain_conf.h): Export them.
> * src/qemu/qemu_driver.c (qemuDomainSnapshotNumChildren)
> (qemuDomainSnapshotListChildrenNames): New functions.
> ---
> 
> v2: no change, but now virsh changes have been tested both with
> and without this patch
> 
>  src/conf/domain_conf.c   |   51 +++++++++++++++++++++++++++
>  src/conf/domain_conf.h   |    7 ++++
>  src/libvirt_private.syms |    2 +
>  src/qemu/qemu_driver.c   |   87 ++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 147 insertions(+), 0 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index c141982..438e3b6 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -12138,6 +12138,37 @@ cleanup:
>      return -1;
>  }
> 
> +int virDomainSnapshotObjListGetNamesFrom(virDomainSnapshotObjPtr snapshot,
> +                                         virDomainSnapshotObjListPtr snapshots,
> +                                         char **const names, int maxnames,
> +                                         unsigned int flags)
> +{
> +    struct virDomainSnapshotNameData data = { 0, 0, maxnames, names, 0 };
> +    int i;
> +
> +    data.flags = flags & ~VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS;
> +
> +    if (flags & VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS)
> +        virDomainSnapshotForEachDescendant(snapshots, snapshot,
> +                                           virDomainSnapshotObjListCopyNames,
> +                                           &data);
> +    else
> +        virDomainSnapshotForEachChild(snapshots, snapshot,
> +                                      virDomainSnapshotObjListCopyNames, &data);
> +
> +    if (data.oom) {
> +        virReportOOMError();
> +        goto cleanup;
> +    }
> +
> +    return data.numnames;
> +
> +cleanup:
> +    for (i = 0; i < data.numnames; i++)
> +        VIR_FREE(data.names[i]);
> +    return -1;
> +}
> +
>  struct virDomainSnapshotNumData {
>      int count;
>      unsigned int flags;
> @@ -12165,6 +12196,26 @@ int virDomainSnapshotObjListNum(virDomainSnapshotObjListPtr snapshots,
>      return data.count;
>  }
> 
> +int
> +virDomainSnapshotObjListNumFrom(virDomainSnapshotObjPtr snapshot,
> +                                virDomainSnapshotObjListPtr snapshots,
> +                                unsigned int flags)
> +{
> +    struct virDomainSnapshotNumData data = { 0, 0 };
> +
> +    data.flags = flags & ~VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS;
> +
> +    if (flags & VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS)
> +        virDomainSnapshotForEachDescendant(snapshots, snapshot,
> +                                           virDomainSnapshotObjListCount,
> +                                           &data);
> +    else
> +        virDomainSnapshotForEachChild(snapshots, snapshot,
> +                                      virDomainSnapshotObjListCount, &data);
> +
> +    return data.count;
> +}
> +
>  virDomainSnapshotObjPtr
>  virDomainSnapshotFindByName(const virDomainSnapshotObjListPtr snapshots,
>                              const char *name)
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 0bc0042..1258740 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1488,6 +1488,13 @@ int virDomainSnapshotObjListGetNames(virDomainSnapshotObjListPtr snapshots,
>                                       unsigned int flags);
>  int virDomainSnapshotObjListNum(virDomainSnapshotObjListPtr snapshots,
>                                  unsigned int flags);
> +int virDomainSnapshotObjListGetNamesFrom(virDomainSnapshotObjPtr snapshot,
> +                                         virDomainSnapshotObjListPtr snapshots,
> +                                         char **const names, int maxnames,
> +                                         unsigned int flags);
> +int virDomainSnapshotObjListNumFrom(virDomainSnapshotObjPtr snapshot,
> +                                    virDomainSnapshotObjListPtr snapshots,
> +                                    unsigned int flags);
>  virDomainSnapshotObjPtr virDomainSnapshotFindByName(const virDomainSnapshotObjListPtr snapshots,
>                                                      const char *name);
>  void virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots,
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index c2a3fab..6b9ceaa 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -413,7 +413,9 @@ virDomainSnapshotForEachChild;
>  virDomainSnapshotForEachDescendant;
>  virDomainSnapshotHasChildren;
>  virDomainSnapshotObjListGetNames;
> +virDomainSnapshotObjListGetNamesFrom;
>  virDomainSnapshotObjListNum;
> +virDomainSnapshotObjListNumFrom;
>  virDomainSnapshotObjListRemove;
>  virDomainSnapshotStateTypeFromString;
>  virDomainSnapshotStateTypeToString;
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 1a171cf..48b0b22 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -9441,6 +9441,91 @@ cleanup:
>      return n;
>  }
> 
> +static int
> +qemuDomainSnapshotListChildrenNames(virDomainSnapshotPtr snapshot,
> +                                    char **names,
> +                                    int nameslen,
> +                                    unsigned int flags)
> +{
> +    struct qemud_driver *driver = snapshot->domain->conn->privateData;
> +    virDomainObjPtr vm = NULL;
> +    virDomainSnapshotObjPtr snap = NULL;
> +    int n = -1;
> +
> +    virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS |
> +                  VIR_DOMAIN_SNAPSHOT_LIST_METADATA, -1);
> +
> +    qemuDriverLock(driver);
> +    vm = virDomainFindByUUID(&driver->domains, snapshot->domain->uuid);
> +    if (!vm) {
> +        char uuidstr[VIR_UUID_STRING_BUFLEN];
> +        virUUIDFormat(snapshot->domain->uuid, uuidstr);
> +        qemuReportError(VIR_ERR_NO_DOMAIN,
> +                        _("no domain with matching uuid '%s'"), uuidstr);
> +        goto cleanup;
> +    }
> +
> +    snap = virDomainSnapshotFindByName(&vm->snapshots, snapshot->name);
> +    if (!snap) {
> +        qemuReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT,
> +                        _("no domain snapshot with matching name '%s'"),
> +                        snapshot->name);
> +        goto cleanup;
> +    }
> +
> +    n = virDomainSnapshotObjListGetNamesFrom(snap, &vm->snapshots,
> +                                             names, nameslen, flags);
> +
> +cleanup:
> +    if (vm)
> +        virDomainObjUnlock(vm);
> +    qemuDriverUnlock(driver);
> +    return n;
> +}
> +
> +static int
> +qemuDomainSnapshotNumChildren(virDomainSnapshotPtr snapshot,
> +                              unsigned int flags)
> +{
> +    struct qemud_driver *driver = snapshot->domain->conn->privateData;
> +    virDomainObjPtr vm = NULL;
> +    virDomainSnapshotObjPtr snap = NULL;
> +    int n = -1;
> +
> +    virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS |
> +                  VIR_DOMAIN_SNAPSHOT_LIST_METADATA, -1);
> +
> +    qemuDriverLock(driver);
> +    vm = virDomainFindByUUID(&driver->domains, snapshot->domain->uuid);
> +    if (!vm) {
> +        char uuidstr[VIR_UUID_STRING_BUFLEN];
> +        virUUIDFormat(snapshot->domain->uuid, uuidstr);
> +        qemuReportError(VIR_ERR_NO_DOMAIN,
> +                        _("no domain with matching uuid '%s'"), uuidstr);
> +        goto cleanup;
> +    }
> +
> +    snap = virDomainSnapshotFindByName(&vm->snapshots, snapshot->name);
> +    if (!snap) {
> +        qemuReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT,
> +                        _("no domain snapshot with matching name '%s'"),
> +                        snapshot->name);
> +        goto cleanup;
> +    }
> +
> +    /* All qemu snapshots have libvirt metadata, so
> +     * VIR_DOMAIN_SNAPSHOT_LIST_METADATA makes no difference to our
> +     * answer.  */
> +
> +    n = virDomainSnapshotObjListNumFrom(snap, &vm->snapshots, flags);
> +
> +cleanup:
> +    if (vm)
> +        virDomainObjUnlock(vm);
> +    qemuDriverUnlock(driver);
> +    return n;
> +}
> +
>  static virDomainSnapshotPtr qemuDomainSnapshotLookupByName(virDomainPtr domain,
>                                                             const char *name,
>                                                             unsigned int flags)
> @@ -10558,6 +10643,8 @@ static virDriver qemuDriver = {
>      .domainSnapshotGetXMLDesc = qemuDomainSnapshotGetXMLDesc, /* 0.8.0 */
>      .domainSnapshotNum = qemuDomainSnapshotNum, /* 0.8.0 */
>      .domainSnapshotListNames = qemuDomainSnapshotListNames, /* 0.8.0 */
> +    .domainSnapshotNumChildren = qemuDomainSnapshotNumChildren, /* 0.9.7 */
> +    .domainSnapshotListChildrenNames = qemuDomainSnapshotListChildrenNames, /* 0.9.7 */
>      .domainSnapshotLookupByName = qemuDomainSnapshotLookupByName, /* 0.8.0 */
>      .domainHasCurrentSnapshot = qemuDomainHasCurrentSnapshot, /* 0.8.0 */
>      .domainSnapshotGetParent = qemuDomainSnapshotGetParent, /* 0.9.7 */

  ACK,
I would tend to be a bit more defensive myself byt not dereferencing 3
layers of pointer as the first instructions in the function, but
if you're confident the structures can't be missing :-)

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


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