[libvirt] [PATCH 2/2] snapshot: simplify redefinition of disk snapshot

[Daniel Veillard]

On Thu, Oct 06, 2011 at 05:18:34PM -0600, Eric Blake wrote:
> Redefining disk-only snapshot xml should work even if the user
> did not explicitly pass VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY;
> the flag is only required for conditions where the <state>
> subelement is not already present in parsing (that is, defining
> a new snapshot).
> 
> Also, fix the error code of some user-visible errors (the remaining
> VIR_ERR_INTERNAL_ERROR should not be user-visible, since parsing
> of <active> is only done from internal code).
> 
> * src/conf/domain_conf.c (virDomainSnapshotDefParseString): Allow
> disks during redefinition of disk snapshot.
> ---
>  src/conf/domain_conf.c |   44 +++++++++++++++++++++++---------------------
>  1 files changed, 23 insertions(+), 21 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index f9007ce..34bf2d0 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -11696,25 +11696,6 @@ virDomainSnapshotDefParseString(const char *xmlStr,
> 
>      def->description = virXPathString("string(./description)", ctxt);
> 
> -    if ((i = virXPathNodeSet("./disks/*", ctxt, &nodes)) < 0)
> -        goto cleanup;
> -    if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_DISKS) {
> -        def->ndisks = i;
> -        if (def->ndisks && VIR_ALLOC_N(def->disks, def->ndisks) < 0) {
> -            virReportOOMError();
> -            goto cleanup;
> -        }
> -        for (i = 0; i < def->ndisks; i++) {
> -            if (virDomainSnapshotDiskDefParseXML(nodes[i], &def->disks[i]) < 0)
> -                goto cleanup;
> -        }
> -        VIR_FREE(nodes);
> -    } else if (i) {
> -        virDomainReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> -                             _("unable to handle disk requests in snapshot"));
> -        goto cleanup;
> -    }
> -
>      if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE) {
>          if (virXPathLongLong("string(./creationTime)", ctxt,
>                               &def->creationTime) < 0) {
> @@ -11730,13 +11711,13 @@ virDomainSnapshotDefParseString(const char *xmlStr,
>              /* there was no state in an existing snapshot; this
>               * should never happen
>               */
> -            virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +            virDomainReportError(VIR_ERR_XML_ERROR, "%s",
>                                   _("missing state from existing snapshot"));
>              goto cleanup;
>          }
>          def->state = virDomainSnapshotStateTypeFromString(state);
>          if (def->state < 0) {
> -            virDomainReportError(VIR_ERR_INTERNAL_ERROR,
> +            virDomainReportError(VIR_ERR_XML_ERROR,
>                                   _("Invalid state '%s' in domain snapshot XML"),
>                                   state);
>              goto cleanup;
> @@ -11768,6 +11749,27 @@ virDomainSnapshotDefParseString(const char *xmlStr,
>          def->creationTime = tv.tv_sec;
>      }
> 
> +    if ((i = virXPathNodeSet("./disks/*", ctxt, &nodes)) < 0)
> +        goto cleanup;
> +    if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_DISKS ||
> +        (flags & VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE &&
> +         def->state == VIR_DOMAIN_DISK_SNAPSHOT)) {
> +        def->ndisks = i;
> +        if (def->ndisks && VIR_ALLOC_N(def->disks, def->ndisks) < 0) {
> +            virReportOOMError();
> +            goto cleanup;
> +        }
> +        for (i = 0; i < def->ndisks; i++) {
> +            if (virDomainSnapshotDiskDefParseXML(nodes[i], &def->disks[i]) < 0)
> +                goto cleanup;
> +        }
> +        VIR_FREE(nodes);
> +    } else if (i) {
> +        virDomainReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> +                             _("unable to handle disk requests in snapshot"));
> +        goto cleanup;
> +    }
> +
>      if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL) {
>          if (virXPathInt("string(./active)", ctxt, &active) < 0) {
>              virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s",

  ACK,

Daniel

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