[libvirt] [PATCH v2 23/29] qemu: Use virDomainStorageSourceParseBase in qemuDomainObjPrivateXMLParseJobNBDSource

Peter Krempa posted 29 patches 6 years, 10 months ago
[libvirt] [PATCH v2 23/29] qemu: Use virDomainStorageSourceParseBase in qemuDomainObjPrivateXMLParseJobNBDSource
Posted by Peter Krempa 6 years, 10 months ago
The slight possible regression in error message descriptivness doesn't
matter much as the function parses private data which should not be
touched by users in the first place.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/qemu/qemu_domain.c | 25 ++++---------------------
 1 file changed, 4 insertions(+), 21 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index c7454ce821..0c06e3b23a 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2725,32 +2725,15 @@ qemuDomainObjPrivateXMLParseJobNBDSource(xmlNodePtr node,
     if (!(ctxt->node = virXPathNode("./migrationSource", ctxt)))
         return 0;

-    if (!(migrSource = virStorageSourceNew()))
-        return -1;
-
-    if (!(type = virXMLPropString(ctxt->node, "type"))) {
-        virReportError(VIR_ERR_XML_ERROR, "%s",
-                       _("missing storage source type"));
-        return -1;
-    }
-
-    if (!(format = virXMLPropString(ctxt->node, "format"))) {
+    if (!(type = virXMLPropString(ctxt->node, "type")) ||
+        !(format = virXMLPropString(ctxt->node, "format"))) {
         virReportError(VIR_ERR_XML_ERROR, "%s",
-                       _("missing storage source format"));
+                       _("missing NBD migration storage source type or format"));
         return -1;
     }

-    if ((migrSource->type = virStorageTypeFromString(type)) <= 0) {
-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                       _("unknown storage source type '%s'"), type);
-        return -1;
-    }
-
-    if ((migrSource->format = virStorageFileFormatTypeFromString(format)) <= 0) {
-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                       _("unknown storage source format '%s'"), format);
+    if (!(migrSource = virDomainStorageSourceParseBase(type, format, NULL)))
         return -1;
-    }

     if ((sourceNode = virXPathNode("./source", ctxt)))
         ctxt->node = sourceNode;
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 23/29] qemu: Use virDomainStorageSourceParseBase in qemuDomainObjPrivateXMLParseJobNBDSource
Posted by Ján Tomko 6 years, 10 months ago
On Fri, Mar 22, 2019 at 07:00:59PM +0100, Peter Krempa wrote:
>The slight possible regression in error message descriptivness doesn't
>matter much as the function parses private data which should not be
>touched by users in the first place.
>

Joining the error messages here is not necessary to use virDomainStorageSourceParseBase
and therefore does not belong in this commit.

Moreover, hereunder, if the error messages do not matter in practice I
suggest leaving them untouched to save the translators some work.

>Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>---
> src/qemu/qemu_domain.c | 25 ++++---------------------
> 1 file changed, 4 insertions(+), 21 deletions(-)
>
>diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>index c7454ce821..0c06e3b23a 100644
>--- a/src/qemu/qemu_domain.c
>+++ b/src/qemu/qemu_domain.c
>@@ -2725,32 +2725,15 @@ qemuDomainObjPrivateXMLParseJobNBDSource(xmlNodePtr node,
>     if (!(ctxt->node = virXPathNode("./migrationSource", ctxt)))
>         return 0;
>
>-    if (!(migrSource = virStorageSourceNew()))
>-        return -1;
>-



>-    if (!(type = virXMLPropString(ctxt->node, "type"))) {
>-        virReportError(VIR_ERR_XML_ERROR, "%s",
>-                       _("missing storage source type"));
>-        return -1;
>-    }
>-
>-    if (!(format = virXMLPropString(ctxt->node, "format"))) {
>+    if (!(type = virXMLPropString(ctxt->node, "type")) ||
>+        !(format = virXMLPropString(ctxt->node, "format"))) {
>         virReportError(VIR_ERR_XML_ERROR, "%s",
>-                       _("missing storage source format"));
>+                       _("missing NBD migration storage source type or format"));
>         return -1;
>     }

Wit this hunk left alone:
Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano

>
>-    if ((migrSource->type = virStorageTypeFromString(type)) <= 0) {
>-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>-                       _("unknown storage source type '%s'"), type);
>-        return -1;
>-    }
>-
>-    if ((migrSource->format = virStorageFileFormatTypeFromString(format)) <= 0) {
>-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>-                       _("unknown storage source format '%s'"), format);
>+    if (!(migrSource = virDomainStorageSourceParseBase(type, format, NULL)))
>         return -1;
>-    }
>
>     if ((sourceNode = virXPathNode("./source", ctxt)))
>         ctxt->node = sourceNode;
>-- 
>2.20.1
>
>--
>libvir-list mailing list
>libvir-list@redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list