On Fri, Mar 22, 2019 at 07:01:03PM +0100, Peter Krempa wrote:
>Use virDomainStorageSourceParseBase and other tricks.
>
Would be nicer to read with the virDomainStorageSourceParseBase trick
and the virDomainStorageSourceParse trick being separate.
>Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>---
> src/conf/domain_conf.c | 52 +++++++++++++-----------------------------
> 1 file changed, 16 insertions(+), 36 deletions(-)
>
>diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>index 40924b1d29..2514c60744 100644
>--- a/src/conf/domain_conf.c
>+++ b/src/conf/domain_conf.c
>@@ -9315,14 +9315,13 @@ virDomainDiskDefMirrorParse(virDomainDiskDefPtr def,
> unsigned int flags,
> virDomainXMLOptionPtr xmlopt)
> {
>- xmlNodePtr mirrorNode;
>+ VIR_XPATH_NODE_AUTORESTORE(ctxt);
> VIR_AUTOFREE(char *) mirrorFormat = NULL;
> VIR_AUTOFREE(char *) mirrorType = NULL;
> VIR_AUTOFREE(char *) ready = NULL;
> VIR_AUTOFREE(char *) blockJob = NULL;
>
>- if (!(def->mirror = virStorageSourceNew()))
>- return -1;
>+ ctxt->node = cur;
>
> if ((blockJob = virXMLPropString(cur, "job"))) {
> if ((def->mirrorJob = virDomainBlockJobTypeFromString(blockJob)) <= 0) {
>@@ -9335,35 +9334,8 @@ virDomainDiskDefMirrorParse(virDomainDiskDefPtr def,
> }
>
> if ((mirrorType = virXMLPropString(cur, "type"))) {
>- if ((def->mirror->type = virStorageTypeFromString(mirrorType)) <= 0) {
>- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>- _("unknown mirror backing store type '%s'"),
>- mirrorType);
>- return -1;
>- }
>-
>- mirrorFormat = virXPathString("string(./mirror/format/@type)", ctxt);
>-
>- if (!(mirrorNode = virXPathNode("./mirror/source", ctxt))) {
>- virReportError(VIR_ERR_XML_ERROR, "%s",
>- _("mirror requires source element"));
>- return -1;
>- }
>-
I'd move this
>- if (virDomainStorageSourceParse(mirrorNode, ctxt, def->mirror,
>- flags, xmlopt) < 0)
>- return -1;
>+ mirrorFormat = virXPathString("string(./format/@type)", ctxt);
> } else {
>- /* For back-compat reasons, we handle a file name
>- * encoded as attributes, even though we prefer
>- * modern output in the style of backingStore */
>- def->mirror->type = VIR_STORAGE_TYPE_FILE;
>- def->mirror->path = virXMLPropString(cur, "file");
>- if (!def->mirror->path) {
>- virReportError(VIR_ERR_XML_ERROR, "%s",
>- _("mirror requires file name"));
>- return -1;
>- }
> if (def->mirrorJob != VIR_DOMAIN_BLOCK_JOB_TYPE_COPY) {
> virReportError(VIR_ERR_XML_ERROR, "%s",
> _("mirror without type only supported "
>@@ -9373,11 +9345,19 @@ virDomainDiskDefMirrorParse(virDomainDiskDefPtr def,
> mirrorFormat = virXMLPropString(cur, "format");
> }
>
>- if (mirrorFormat) {
>- def->mirror->format = virStorageFileFormatTypeFromString(mirrorFormat);
>- if (def->mirror->format <= 0) {
>- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>- _("unknown mirror format value '%s'"), mirrorFormat);
>+ if (!(def->mirror = virDomainStorageSourceParseBase(mirrorType, mirrorFormat, NULL)))
>+ return -1;
>+
>+ if (mirrorType) {
here.
>+ if (virDomainStorageSourceParse(NULL, ctxt, def->mirror, flags, xmlopt) < 0)
>+ return -1;
>+ } else {
>+ /* For back-compat reasons, we handle a file name encoded as
>+ * attributes, even though we prefer modern output in the style of
>+ * backingStore */
>+ if (!(def->mirror->path = virXMLPropString(cur, "file"))) {
>+ virReportError(VIR_ERR_XML_ERROR, "%s",
>+ _("mirror requires file name"));
> return -1;
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list