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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jul 05, 2018 at 02:43:18PM +0200, Michal Prívozník wrote:
> 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;

Sure, this is fine.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux