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

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", -- 1.7.4.4