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

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(a)redhat.com&gt; > --- > 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;