[libvirt] [PATCH 1/2] qemu: Clean up qemuBuildControllerDevCommandLine()

Andrea Bolognani posted 2 patches 8 years, 3 months ago
[libvirt] [PATCH 1/2] qemu: Clean up qemuBuildControllerDevCommandLine()
Posted by Andrea Bolognani 8 years, 3 months ago
Add a cleanup: label, which will be used later, and improve
the readability of one of the checks by making it conform to
our formatting standard and moving a comment.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
 src/qemu/qemu_command.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 9a27987d4..83430b33f 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3122,6 +3122,7 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd,
         VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL,
         VIR_DOMAIN_CONTROLLER_TYPE_CCID,
     };
+    int ret = -1;
 
     for (j = 0; j < ARRAY_CARDINALITY(contOrder); j++) {
         for (i = 0; i < def->ncontrollers; i++) {
@@ -3183,7 +3184,7 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd,
                     virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                                    _("Multiple legacy USB controllers are "
                                      "not supported"));
-                    return -1;
+                    goto cleanup;
                 }
                 usblegacy = true;
                 continue;
@@ -3191,7 +3192,7 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd,
 
             if (qemuBuildControllerDevStr(def, cont, qemuCaps,
                                           &devstr, &usbcontroller) < 0)
-                return -1;
+                goto cleanup;
 
             if (devstr) {
                 virCommandAddArg(cmd, "-device");
@@ -3201,16 +3202,20 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd,
         }
     }
 
-    /* We haven't added any USB controller yet, but we haven't been asked
-     * not to add one either. Add a legacy USB controller, unless we're
-     * creating a kind of guest we want to keep legacy-free */
     if (usbcontroller == 0 &&
         !qemuDomainIsQ35(def) &&
         !qemuDomainIsVirt(def) &&
-        !ARCH_IS_S390(def->os.arch))
+        !ARCH_IS_S390(def->os.arch)) {
+        /* We haven't added any USB controller yet, but we haven't been asked
+         * not to add one either. Add a legacy USB controller, unless we're
+         * creating a kind of guest we want to keep legacy-free */
         virCommandAddArg(cmd, "-usb");
+    }
 
-    return 0;
+    ret = 0;
+
+ cleanup:
+    return ret;
 }
 
 
-- 
2.13.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] qemu: Clean up qemuBuildControllerDevCommandLine()
Posted by Laine Stump 8 years, 3 months ago
On 09/05/2017 09:25 AM, Andrea Bolognani wrote:
> Add a cleanup: label, which will be used later, and improve
> the readability of one of the checks by making it conform to
> our formatting standard and moving a comment.
>
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---

Reviewed-by: Laine Stump <laine@laine.org>

(or "ACK" if you prefer. I'm still unclear who is on which side of this
debate, and why.)

>  src/qemu/qemu_command.c | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 9a27987d4..83430b33f 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -3122,6 +3122,7 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd,
>          VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL,
>          VIR_DOMAIN_CONTROLLER_TYPE_CCID,
>      };
> +    int ret = -1;
>  
>      for (j = 0; j < ARRAY_CARDINALITY(contOrder); j++) {
>          for (i = 0; i < def->ncontrollers; i++) {
> @@ -3183,7 +3184,7 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd,
>                      virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                                     _("Multiple legacy USB controllers are "
>                                       "not supported"));
> -                    return -1;
> +                    goto cleanup;
>                  }
>                  usblegacy = true;
>                  continue;
> @@ -3191,7 +3192,7 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd,
>  
>              if (qemuBuildControllerDevStr(def, cont, qemuCaps,
>                                            &devstr, &usbcontroller) < 0)
> -                return -1;
> +                goto cleanup;
>  
>              if (devstr) {
>                  virCommandAddArg(cmd, "-device");
> @@ -3201,16 +3202,20 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd,
>          }
>      }
>  
> -    /* We haven't added any USB controller yet, but we haven't been asked
> -     * not to add one either. Add a legacy USB controller, unless we're
> -     * creating a kind of guest we want to keep legacy-free */
>      if (usbcontroller == 0 &&
>          !qemuDomainIsQ35(def) &&
>          !qemuDomainIsVirt(def) &&
> -        !ARCH_IS_S390(def->os.arch))
> +        !ARCH_IS_S390(def->os.arch)) {
> +        /* We haven't added any USB controller yet, but we haven't been asked
> +         * not to add one either. Add a legacy USB controller, unless we're
> +         * creating a kind of guest we want to keep legacy-free */
>          virCommandAddArg(cmd, "-usb");
> +    }
>  
> -    return 0;
> +    ret = 0;
> +
> + cleanup:
> +    return ret;
>  }
>  
>  


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