Re: [PATCH 2/3] qemu: consolidate parameters of qemuBuildChrChardevStr into flags

[Michal Prívozník <mprivozn@xxxxxxxxxx>]

On 07/05/2018 01:36 PM, Daniel P. Berrangé wrote:
> There are two boolean parameters passed to qemuBuildChrChardevStr,
> and soon there will be a third. It will be clearer to understand
> from callers' POV if we use named flags instead.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
> ---
>  src/qemu/qemu_command.c | 86 ++++++++++++++++++++++++++---------------
>  1 file changed, 55 insertions(+), 31 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 9351b9fddb..7f3eccc1bd 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -4935,6 +4935,12 @@ qemuOpenChrChardevUNIXSocket(const virDomainChrSourceDef *dev)
>      return -1;
>  }
>  
> +
> +enum {
> +    QEMU_BUILD_CHARDEV_TCP_NOWAIT = (1 << 0),
> +    QEMU_BUILD_CHARDEV_FILE_LOGD  = (1 << 1),
> +};
> +
>  /* This function outputs a -chardev command line option which describes only the
>   * host side of the character device */
>  static char *
> @@ -4946,8 +4952,7 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
>                         const virDomainChrSourceDef *dev,
>                         const char *alias,
>                         virQEMUCapsPtr qemuCaps,
> -                       bool nowait,
> -                       bool chardevStdioLogd)
> +                       unsigned int flags)
>  {
>      virBuffer buf = VIR_BUFFER_INITIALIZER;
>      bool telnet;
> @@ -4986,7 +4991,8 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
>                             _("append not supported in this QEMU binary"));
>              goto cleanup;
>          }
> -        if (qemuBuildChrChardevFileStr(chardevStdioLogd ? logManager : NULL,
> +        if (qemuBuildChrChardevFileStr(flags & QEMU_BUILD_CHARDEV_FILE_LOGD ?
> +                                       logManager : NULL,
>                                         cmd, def, &buf,
>                                         "path", dev->data.file.path,
>                                         "append", dev->data.file.append) < 0)
> @@ -5033,7 +5039,8 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
>                            telnet ? ",telnet" : "");
>  
>          if (dev->data.tcp.listen)
> -            virBufferAdd(&buf, nowait ? ",server,nowait" : ",server", -1);
> +            virBufferAdd(&buf, flags & QEMU_BUILD_CHARDEV_TCP_NOWAIT ?
> +                         ",server,nowait" : ",server", -1);

Well, since you're touching this: s/virBufferAdd/virBufferAddLit/ and 
also perhaps get rid of the ternary operator?

diff --git i/src/qemu/qemu_command.c w/src/qemu/qemu_command.c
index 7f3eccc1bd..63c7ac0f82 100644
--- i/src/qemu/qemu_command.c
+++ w/src/qemu/qemu_command.c
@@ -5038,9 +5038,11 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
                           dev->data.tcp.service,
                           telnet ? ",telnet" : "");
 
-        if (dev->data.tcp.listen)
-            virBufferAdd(&buf, flags & QEMU_BUILD_CHARDEV_TCP_NOWAIT ?
-                         ",server,nowait" : ",server", -1);
+        if (dev->data.tcp.listen) {
+            virBufferAddLit(&buf, ",server");
+            if (flags & QEMU_BUILD_CHARDEV_TCP_NOWAIT)
+                virBufferAddLit(&buf, ",nowait");
+        }
 
         qemuBuildChrChardevReconnectStr(&buf, &dev->data.tcp.reconnect);
 
@@ -5098,9 +5100,11 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
             virBufferAsprintf(&buf, "socket,id=%s,path=", charAlias);
             virQEMUBuildBufferEscapeComma(&buf, dev->data.nix.path);
         }
-        if (dev->data.nix.listen)
-            virBufferAdd(&buf, flags & QEMU_BUILD_CHARDEV_TCP_NOWAIT ?
-                         ",server,nowait" : ",server", -1);
+        if (dev->data.nix.listen) {
+            virBufferAddLit(&buf, ",server");
+            if (flags & QEMU_BUILD_CHARDEV_TCP_NOWAIT)
+                virBufferAddLit(&buf, ",nowait");
+        }
 
         qemuBuildChrChardevReconnectStr(&buf, &dev->data.nix.reconnect);
         break;


Michal

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