Now that we've moved all the actual code into helper
functions, we can turn it into a switch statement.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
 src/conf/domain_conf.c | 77 +++++++++++++++++++++++++++++++-----------
 1 file changed, 58 insertions(+), 19 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7f66fa27ff..ffdc027983 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4966,33 +4966,72 @@ virDomainDeviceDefPostParseCommon(virDomainDeviceDefPtr dev,
                                   unsigned int parseFlags ATTRIBUTE_UNUSED,
                                   virDomainXMLOptionPtr xmlopt)
 {
-    if (dev->type == VIR_DOMAIN_DEVICE_CHR)
-        return virDomainChrDefPostParse(dev->data.chr, def);
+    int ret = -1;
 
-    if (dev->type == VIR_DOMAIN_DEVICE_RNG)
-        return virDomainRNGDefPostParse(dev->data.rng);
+    switch ((virDomainDeviceType)dev->type) {
+    case VIR_DOMAIN_DEVICE_CHR:
+        ret = virDomainChrDefPostParse(dev->data.chr, def);
+        break;
 
-    if (dev->type == VIR_DOMAIN_DEVICE_DISK)
-        return virDomainDiskDefPostParse(dev->data.disk, def, xmlopt);
+    case VIR_DOMAIN_DEVICE_RNG:
+        ret = virDomainRNGDefPostParse(dev->data.rng);
+        break;
 
-    if (dev->type == VIR_DOMAIN_DEVICE_VIDEO)
-        return virDomainVideoDefPostParse(dev->data.video, def);
+    case VIR_DOMAIN_DEVICE_DISK:
+        ret = virDomainDiskDefPostParse(dev->data.disk, def, xmlopt);
+        break;
 
-    if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV &&
-        virDomainHostdevDefPostParse(dev->data.hostdev, def, xmlopt) < 0)
-        return -1;
+    case VIR_DOMAIN_DEVICE_VIDEO:
+        ret = virDomainVideoDefPostParse(dev->data.video, def);
+        break;
+
+    case VIR_DOMAIN_DEVICE_HOSTDEV:
+        ret = virDomainHostdevDefPostParse(dev->data.hostdev, def, xmlopt);
+        break;
 
-    if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER)
-        return virDomainControllerDefPostParse(dev->data.controller);
+    case VIR_DOMAIN_DEVICE_CONTROLLER:
+        ret = virDomainControllerDefPostParse(dev->data.controller);
+        break;
 
-    if (dev->type == VIR_DOMAIN_DEVICE_NET)
-        return virDomainNetDefPostParse(dev->data.net);
+    case VIR_DOMAIN_DEVICE_NET:
+        ret = virDomainNetDefPostParse(dev->data.net);
+        break;
 
-    if (dev->type == VIR_DOMAIN_DEVICE_VSOCK &&
-        virDomainVsockDefPostParse(dev->data.vsock) < 0)
-        return -1;
+    case VIR_DOMAIN_DEVICE_VSOCK:
+        ret = virDomainVsockDefPostParse(dev->data.vsock);
+        break;
 
-    return 0;
+    case VIR_DOMAIN_DEVICE_LEASE:
+    case VIR_DOMAIN_DEVICE_FS:
+    case VIR_DOMAIN_DEVICE_INPUT:
+    case VIR_DOMAIN_DEVICE_SOUND:
+    case VIR_DOMAIN_DEVICE_WATCHDOG:
+    case VIR_DOMAIN_DEVICE_GRAPHICS:
+    case VIR_DOMAIN_DEVICE_HUB:
+    case VIR_DOMAIN_DEVICE_REDIRDEV:
+    case VIR_DOMAIN_DEVICE_SMARTCARD:
+    case VIR_DOMAIN_DEVICE_MEMBALLOON:
+    case VIR_DOMAIN_DEVICE_NVRAM:
+    case VIR_DOMAIN_DEVICE_SHMEM:
+    case VIR_DOMAIN_DEVICE_TPM:
+    case VIR_DOMAIN_DEVICE_PANIC:
+    case VIR_DOMAIN_DEVICE_MEMORY:
+    case VIR_DOMAIN_DEVICE_IOMMU:
+        ret = 0;
+        break;
+
+    case VIR_DOMAIN_DEVICE_NONE:
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("unexpected VIR_DOMAIN_DEVICE_NONE"));
+        break;
+
+    case VIR_DOMAIN_DEVICE_LAST:
+    default:
+        virReportEnumRangeError(virDomainDeviceType, dev->type);
+        break;
+    }
+
+    return ret;
 }
 
 
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Andrea Bolognani <abologna@redhat.com> [2019-02-15, 12:55PM +0100]:
> +    case VIR_DOMAIN_DEVICE_NONE:
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("unexpected VIR_DOMAIN_DEVICE_NONE"));

Can we find a better error message here?

> +        break;
> +
> +    case VIR_DOMAIN_DEVICE_LAST:
> +    default:
> +        virReportEnumRangeError(virDomainDeviceType, dev->type);
> +        break;
> +    }
> +
> +    return ret;
>  }
>  
>  
> -- 
> 2.20.1
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
> 

-- 
IBM Systems
Linux on Z & Virtualization Development
--------------------------------------------------
IBM Deutschland Research & Development GmbH
Schönaicher Str. 220, 71032 Böblingen
Phone: +49 7031 16 1819
--------------------------------------------------
Vorsitzende des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294 
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Mon, 2019-02-18 at 09:46 +0100, Bjoern Walk wrote:
> Andrea Bolognani <abologna@redhat.com> [2019-02-15, 12:55PM +0100]:
> > +    case VIR_DOMAIN_DEVICE_NONE:
> > +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > +                       _("unexpected VIR_DOMAIN_DEVICE_NONE"));
> 
> Can we find a better error message here?

Feel free to post a patch changing this (and the one in
qemuDomainDeviceDefPostParse() that I've copied it from :), but it's
one of those error messages that will only show up if we've made some
pretty serious mistake somewhere leading up to the function being
called, so I don't feel like it's particularly important for it to
be user friendly.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Fri, Feb 15, 2019 at 12:55:52PM +0100, Andrea Bolognani wrote:
>Now that we've moved all the actual code into helper
>functions, we can turn it into a switch statement.
>
>Signed-off-by: Andrea Bolognani <abologna@redhat.com>
>---
> src/conf/domain_conf.c | 77 +++++++++++++++++++++++++++++++-----------
> 1 file changed, 58 insertions(+), 19 deletions(-)
>
>diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>index 7f66fa27ff..ffdc027983 100644
>--- a/src/conf/domain_conf.c
>+++ b/src/conf/domain_conf.c
>@@ -4966,33 +4966,72 @@ virDomainDeviceDefPostParseCommon(virDomainDeviceDefPtr dev,
>                                   unsigned int parseFlags ATTRIBUTE_UNUSED,
>                                   virDomainXMLOptionPtr xmlopt)
> {
>-    if (dev->type == VIR_DOMAIN_DEVICE_CHR)
>-        return virDomainChrDefPostParse(dev->data.chr, def);
>+    int ret = -1;
>

With the preparatory patches adding an exit point in every if, I
expected the 'return's to stay.

>-    if (dev->type == VIR_DOMAIN_DEVICE_RNG)
>-        return virDomainRNGDefPostParse(dev->data.rng);
>+    switch ((virDomainDeviceType)dev->type) {

[...]

>+    case VIR_DOMAIN_DEVICE_NONE:
>+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>+                       _("unexpected VIR_DOMAIN_DEVICE_NONE"));
>+        break;
>+
>+    case VIR_DOMAIN_DEVICE_LAST:
>+    default:
>+        virReportEnumRangeError(virDomainDeviceType, dev->type);
>+        break;
>+    }
>+
>+    return ret;

But I'm not sure how some tools would handle the (lack of) presence
of this return.

Either way:
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