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@redhat.com> --- 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 :|