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
