[libvirt] [PATCH] qemu: Remove duplicated code in qemuBuildSerialChrDeviceStr()

Andrea Bolognani posted 1 patch 6 years, 9 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/1498112443-8885-1-git-send-email-abologna@redhat.com
There is a newer version of this series
src/qemu/qemu_command.c | 20 +++++++-------------
1 file changed, 7 insertions(+), 13 deletions(-)
[libvirt] [PATCH] qemu: Remove duplicated code in qemuBuildSerialChrDeviceStr()
Posted by Andrea Bolognani 6 years, 9 months ago
The call to qemuBuildDeviceAddressStr() happens no matter
what, so we can move it outside of the switch. We can also
move the call to virBufferAsprintf() closer to it to avoid
having formatting - error checking - more formatting.

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

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c53ab97..9bb0163 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -10296,10 +10296,6 @@ qemuBuildSerialChrDeviceStr(char **deviceStr,
                 goto error;
         }
     } else {
-        virBufferAsprintf(&cmd, "%s,chardev=char%s,id=%s",
-                          virDomainChrSerialTargetTypeToString(serial->targetType),
-                          serial->info.alias, serial->info.alias);
-
         switch (serial->targetType) {
         case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB:
             if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_SERIAL)) {
@@ -10314,9 +10310,6 @@ qemuBuildSerialChrDeviceStr(char **deviceStr,
                                _("usb-serial requires address of usb type"));
                 goto error;
             }
-
-            if (qemuBuildDeviceAddressStr(&cmd, def, &serial->info, qemuCaps) < 0)
-                goto error;
             break;
 
         case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA:
@@ -10326,9 +10319,6 @@ qemuBuildSerialChrDeviceStr(char **deviceStr,
                                _("isa-serial requires address of isa type"));
                 goto error;
             }
-
-            if (qemuBuildDeviceAddressStr(&cmd, def, &serial->info, qemuCaps) < 0)
-                goto error;
             break;
 
         case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI:
@@ -10344,11 +10334,15 @@ qemuBuildSerialChrDeviceStr(char **deviceStr,
                                _("pci-serial requires address of pci type"));
                 goto error;
             }
-
-            if (qemuBuildDeviceAddressStr(&cmd, def, &serial->info, qemuCaps) < 0)
-                goto error;
             break;
         }
+
+        virBufferAsprintf(&cmd, "%s,chardev=char%s,id=%s",
+                          virDomainChrSerialTargetTypeToString(serial->targetType),
+                          serial->info.alias, serial->info.alias);
+
+        if (qemuBuildDeviceAddressStr(&cmd, def, &serial->info, qemuCaps) < 0)
+            goto error;
     }
 
     if (virBufferCheckError(&cmd) < 0)
-- 
2.7.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Remove duplicated code in qemuBuildSerialChrDeviceStr()
Posted by Shivaprasad bhat 6 years, 9 months ago
Hi Andrea,


On Thu, Jun 22, 2017 at 11:50 AM, Andrea Bolognani <abologna@redhat.com>
wrote:

> The call to qemuBuildDeviceAddressStr() happens no matter
> what, so we can move it outside of the switch. We can also
> move the call to virBufferAsprintf() closer to it to avoid
> having formatting - error checking - more formatting.
>
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
>  src/qemu/qemu_command.c | 20 +++++++-------------
>  1 file changed, 7 insertions(+), 13 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index c53ab97..9bb0163 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -10296,10 +10296,6 @@ qemuBuildSerialChrDeviceStr(char **deviceStr,
>

We can remove the qemuBuildDeviceAddressStr() from this line too. Like,

https://paste.fedoraproject.org/paste/D2P1sQT~ElzQ6Ywe5DlIxA

Thanks,
Shivaprasad

                 goto error;

         }
>      } else {
> -        virBufferAsprintf(&cmd, "%s,chardev=char%s,id=%s",
> -                          virDomainChrSerialTargetTypeTo
> String(serial->targetType),
> -                          serial->info.alias, serial->info.alias);
> -
>          switch (serial->targetType) {
>          case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB:
>              if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_SERIAL)) {
> @@ -10314,9 +10310,6 @@ qemuBuildSerialChrDeviceStr(char **deviceStr,
>                                 _("usb-serial requires address of usb
> type"));
>                  goto error;
>              }
> -
> -            if (qemuBuildDeviceAddressStr(&cmd, def, &serial->info,
> qemuCaps) < 0)
> -                goto error;
>              break;
>
>          case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA:
> @@ -10326,9 +10319,6 @@ qemuBuildSerialChrDeviceStr(char **deviceStr,
>                                 _("isa-serial requires address of isa
> type"));
>                  goto error;
>              }
> -
> -            if (qemuBuildDeviceAddressStr(&cmd, def, &serial->info,
> qemuCaps) < 0)
> -                goto error;
>              break;
>
>          case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI:
> @@ -10344,11 +10334,15 @@ qemuBuildSerialChrDeviceStr(char **deviceStr,
>                                 _("pci-serial requires address of pci
> type"));
>                  goto error;
>              }
> -
> -            if (qemuBuildDeviceAddressStr(&cmd, def, &serial->info,
> qemuCaps) < 0)
> -                goto error;
>              break;
>          }
> +
> +        virBufferAsprintf(&cmd, "%s,chardev=char%s,id=%s",
> +                          virDomainChrSerialTargetTypeTo
> String(serial->targetType),
> +                          serial->info.alias, serial->info.alias);
> +
> +        if (qemuBuildDeviceAddressStr(&cmd, def, &serial->info,
> qemuCaps) < 0)
> +            goto error;
>      }
>
>      if (virBufferCheckError(&cmd) < 0)
> --
> 2.7.5
>
> --
> 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
Re: [libvirt] [PATCH] qemu: Remove duplicated code in qemuBuildSerialChrDeviceStr()
Posted by Andrea Bolognani 6 years, 9 months ago
On Thu, 2017-06-22 at 13:55 +0530, Shivaprasad bhat wrote:
> > @@ -10296,10 +10296,6 @@ qemuBuildSerialChrDeviceStr(char **deviceStr,
>  
> We can remove the qemuBuildDeviceAddressStr() from this line too. Like,
> 
> https://paste.fedoraproject.org/paste/D2P1sQT~ElzQ6Ywe5DlIxA

Good point. v2 coming up.

-- 
Andrea Bolognani / Red Hat / Virtualization

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