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);
qemuBuildChrChardevReconnectStr(&buf, &dev->data.tcp.reconnect);
@@ -5092,7 +5099,8 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
virQEMUBuildBufferEscapeComma(&buf, dev->data.nix.path);
}
if (dev->data.nix.listen)
- virBufferAdd(&buf, nowait ? ",server,nowait" : ",server", -1);
+ virBufferAdd(&buf, flags & QEMU_BUILD_CHARDEV_TCP_NOWAIT ?
+ ",server,nowait" : ",server", -1);
qemuBuildChrChardevReconnectStr(&buf, &dev->data.nix.reconnect);
break;
@@ -5426,6 +5434,9 @@ qemuBuildMonitorCommandLine(virLogManagerPtr logManager,
qemuDomainObjPrivatePtr priv)
{
char *chrdev;
+ unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT;
+ if (priv->chardevStdioLogd)
+ cdevflags |= QEMU_BUILD_CHARDEV_FILE_LOGD;
if (!priv->monConfig)
return 0;
@@ -5433,8 +5444,7 @@ qemuBuildMonitorCommandLine(virLogManagerPtr logManager,
if (!(chrdev = qemuBuildChrChardevStr(logManager, secManager,
cmd, cfg, def,
priv->monConfig, "monitor",
- priv->qemuCaps, true,
- priv->chardevStdioLogd)))
+ priv->qemuCaps, cdevflags)))
return -1;
virCommandAddArg(cmd, "-chardev");
virCommandAddArg(cmd, chrdev);
@@ -5559,6 +5569,9 @@ qemuBuildRNGBackendChrdevStr(virLogManagerPtr logManager,
char **chr,
bool chardevStdioLogd)
{
+ unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT;
+ if (chardevStdioLogd)
+ cdevflags |= QEMU_BUILD_CHARDEV_FILE_LOGD;
*chr = NULL;
switch ((virDomainRNGBackend) rng->backend) {
@@ -5571,8 +5584,8 @@ qemuBuildRNGBackendChrdevStr(virLogManagerPtr logManager,
if (!(*chr = qemuBuildChrChardevStr(logManager, secManager,
cmd, cfg, def,
rng->source.chardev,
- rng->info.alias, qemuCaps, true,
- chardevStdioLogd)))
+ rng->info.alias, qemuCaps,
+ cdevflags)))
return -1;
}
@@ -8215,8 +8228,7 @@ qemuBuildVhostuserCommandLine(virQEMUDriverPtr driver,
if (!(chardev = qemuBuildChrChardevStr(logManager, secManager,
cmd, cfg, def,
net->data.vhostuser,
- net->info.alias, qemuCaps, false,
- false)))
+ net->info.alias, qemuCaps, 0)))
goto cleanup;
break;
@@ -8696,6 +8708,9 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr logManager,
virBuffer opt = VIR_BUFFER_INITIALIZER;
const char *database;
const char *contAlias = NULL;
+ unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT;
+ if (chardevStdioLogd)
+ cdevflags |= QEMU_BUILD_CHARDEV_FILE_LOGD;
if (!def->nsmartcards)
return 0;
@@ -8761,8 +8776,7 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr logManager,
cmd, cfg, def,
smartcard->data.passthru,
smartcard->info.alias,
- qemuCaps, true,
- chardevStdioLogd))) {
+ qemuCaps, cdevflags))) {
virBufferFreeAndReset(&opt);
return -1;
}
@@ -8930,6 +8944,9 @@ qemuBuildShmemCommandLine(virLogManagerPtr logManager,
virBuffer buf = VIR_BUFFER_INITIALIZER;
char *devstr = NULL;
int rc;
+ unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT;
+ if (chardevStdioLogd)
+ cdevflags |= QEMU_BUILD_CHARDEV_FILE_LOGD;
if (shmem->size) {
/*
@@ -8993,8 +9010,8 @@ qemuBuildShmemCommandLine(virLogManagerPtr logManager,
devstr = qemuBuildChrChardevStr(logManager, secManager,
cmd, cfg, def,
&shmem->server.chr,
- shmem->info.alias, qemuCaps, true,
- chardevStdioLogd);
+ shmem->info.alias, qemuCaps,
+ cdevflags);
if (!devstr)
return -1;
@@ -9087,6 +9104,9 @@ qemuBuildSerialCommandLine(virLogManagerPtr logManager,
{
size_t i;
bool havespice = false;
+ unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT;
+ if (chardevStdioLogd)
+ cdevflags |= QEMU_BUILD_CHARDEV_FILE_LOGD;
if (def->nserials) {
for (i = 0; i < def->ngraphics && !havespice; i++) {
@@ -9106,8 +9126,7 @@ qemuBuildSerialCommandLine(virLogManagerPtr logManager,
cmd, cfg, def,
serial->source,
serial->info.alias,
- qemuCaps, true,
- chardevStdioLogd)))
+ qemuCaps, cdevflags)))
return -1;
virCommandAddArg(cmd, "-chardev");
virCommandAddArg(cmd, devstr);
@@ -9148,6 +9167,9 @@ qemuBuildParallelsCommandLine(virLogManagerPtr logManager,
bool chardevStdioLogd)
{
size_t i;
+ unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT;
+ if (chardevStdioLogd)
+ cdevflags |= QEMU_BUILD_CHARDEV_FILE_LOGD;
for (i = 0; i < def->nparallels; i++) {
virDomainChrDefPtr parallel = def->parallels[i];
@@ -9157,8 +9179,7 @@ qemuBuildParallelsCommandLine(virLogManagerPtr logManager,
cmd, cfg, def,
parallel->source,
parallel->info.alias,
- qemuCaps, true,
- chardevStdioLogd)))
+ qemuCaps, cdevflags)))
return -1;
virCommandAddArg(cmd, "-chardev");
virCommandAddArg(cmd, devstr);
@@ -9183,6 +9204,9 @@ qemuBuildChannelsCommandLine(virLogManagerPtr logManager,
bool chardevStdioLogd)
{
size_t i;
+ unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT;
+ if (chardevStdioLogd)
+ cdevflags |= QEMU_BUILD_CHARDEV_FILE_LOGD;
for (i = 0; i < def->nchannels; i++) {
virDomainChrDefPtr channel = def->channels[i];
@@ -9194,8 +9218,7 @@ qemuBuildChannelsCommandLine(virLogManagerPtr logManager,
cmd, cfg, def,
channel->source,
channel->info.alias,
- qemuCaps, true,
- chardevStdioLogd)))
+ qemuCaps, cdevflags)))
return -1;
virCommandAddArg(cmd, "-chardev");
virCommandAddArg(cmd, devstr);
@@ -9212,8 +9235,7 @@ qemuBuildChannelsCommandLine(virLogManagerPtr logManager,
cmd, cfg, def,
channel->source,
channel->info.alias,
- qemuCaps, true,
- chardevStdioLogd)))
+ qemuCaps, cdevflags)))
return -1;
virCommandAddArg(cmd, "-chardev");
virCommandAddArg(cmd, devstr);
@@ -9239,6 +9261,9 @@ qemuBuildConsoleCommandLine(virLogManagerPtr logManager,
bool chardevStdioLogd)
{
size_t i;
+ unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT;
+ if (chardevStdioLogd)
+ cdevflags |= QEMU_BUILD_CHARDEV_FILE_LOGD;
/* Explicit console devices */
for (i = 0; i < def->nconsoles; i++) {
@@ -9257,8 +9282,7 @@ qemuBuildConsoleCommandLine(virLogManagerPtr logManager,
cmd, cfg, def,
console->source,
console->info.alias,
- qemuCaps, true,
- chardevStdioLogd)))
+ qemuCaps, cdevflags)))
return -1;
virCommandAddArg(cmd, "-chardev");
virCommandAddArg(cmd, devstr);
@@ -9279,8 +9303,7 @@ qemuBuildConsoleCommandLine(virLogManagerPtr logManager,
cmd, cfg, def,
console->source,
console->info.alias,
- qemuCaps, true,
- chardevStdioLogd)))
+ qemuCaps, cdevflags)))
return -1;
virCommandAddArg(cmd, "-chardev");
virCommandAddArg(cmd, devstr);
@@ -9295,8 +9318,7 @@ qemuBuildConsoleCommandLine(virLogManagerPtr logManager,
cmd, cfg, def,
console->source,
console->info.alias,
- qemuCaps, true,
- chardevStdioLogd)))
+ qemuCaps, cdevflags)))
return -1;
virCommandAddArg(cmd, "-chardev");
virCommandAddArg(cmd, devstr);
@@ -9419,6 +9441,9 @@ qemuBuildRedirdevCommandLine(virLogManagerPtr logManager,
bool chardevStdioLogd)
{
size_t i;
+ unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT;
+ if (chardevStdioLogd)
+ cdevflags |= QEMU_BUILD_CHARDEV_FILE_LOGD;
for (i = 0; i < def->nredirdevs; i++) {
virDomainRedirdevDefPtr redirdev = def->redirdevs[i];
@@ -9428,8 +9453,7 @@ qemuBuildRedirdevCommandLine(virLogManagerPtr logManager,
cmd, cfg, def,
redirdev->source,
redirdev->info.alias,
- qemuCaps, true,
- chardevStdioLogd))) {
+ qemuCaps, cdevflags))) {
return -1;
}
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-listOn 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;
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.