6:36 a.m.
QEMU has a validation bug which prevents vhostuser accepting the chardev
when it uses FD passing, despite it being functionally correct.
While the QEMU bug is being fixed, we need to avoid hitting this
problem with existing QEMU releases.
The easiest way todo this is to simply disable FD passing for the
chardev used with vhostuser only.
Daniel P. Berrangé (3):
qemu: remove chardevStdioLogd param from vhostuser code path
qemu: consolidate parameters of qemuBuildChrChardevStr into flags
qemu: don't use chardev FD passing for vhostuser backend
src/qemu/qemu_command.c | 117 ++++++++++++++--------
tests/qemuxml2argvdata/net-vhostuser.args | 3 +-
tests/qemuxml2argvtest.c | 2 +-
3 files changed, 75 insertions(+), 47 deletions(-)
--
2.17.1
6:36 a.m.
New subject: [libvirt] [PATCH 1/3] qemu: remove chardevStdioLogd param from vhostuser code path
The vhostuser network backend is only supported with the UNIX domain
socket chardev backend, so passing around chardevStdioLogd is not
required.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/qemu/qemu_command.c | 20 +++++++-------------
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 04c5c28438..9351b9fddb 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8195,8 +8195,7 @@ qemuBuildVhostuserCommandLine(virQEMUDriverPtr driver,
virDomainDefPtr def,
virDomainNetDefPtr net,
virQEMUCapsPtr qemuCaps,
- unsigned int bootindex,
- bool chardevStdioLogd)
+ unsigned int bootindex)
{
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
char *chardev = NULL;
@@ -8217,7 +8216,7 @@ qemuBuildVhostuserCommandLine(virQEMUDriverPtr driver,
cmd, cfg, def,
net->data.vhostuser,
net->info.alias, qemuCaps, false,
- chardevStdioLogd)))
+ false)))
goto cleanup;
break;
@@ -8291,8 +8290,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
virNetDevVPortProfileOp vmop,
bool standalone,
size_t *nnicindexes,
- int **nicindexes,
- bool chardevStdioLogd)
+ int **nicindexes)
{
int ret = -1;
char *nic = NULL, *host = NULL;
@@ -8415,8 +8413,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
ret = qemuBuildVhostuserCommandLine(driver, logManager, secManager, cmd, def,
- net, qemuCaps, bootindex,
- chardevStdioLogd);
+ net, qemuCaps, bootindex);
goto cleanup;
break;
@@ -8600,8 +8597,7 @@ qemuBuildNetCommandLine(virQEMUDriverPtr driver,
bool standalone,
size_t *nnicindexes,
int **nicindexes,
- unsigned int *bootHostdevNet,
- bool chardevStdioLogd)
+ unsigned int *bootHostdevNet)
{
size_t i;
int last_good_net = -1;
@@ -8628,8 +8624,7 @@ qemuBuildNetCommandLine(virQEMUDriverPtr driver,
if (qemuBuildInterfaceCommandLine(driver, logManager, secManager, cmd, def,
net,
qemuCaps, bootNet, vmop,
standalone, nnicindexes,
- nicindexes,
- chardevStdioLogd) < 0)
+ nicindexes) < 0)
goto error;
last_good_net = i;
@@ -10290,8 +10285,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
if (qemuBuildNetCommandLine(driver, logManager, secManager, cmd, def,
qemuCaps, vmop, standalone,
- nnicindexes, nicindexes, &bootHostdevNet,
- chardevStdioLogd) < 0)
+ nnicindexes, nicindexes, &bootHostdevNet) < 0)
goto error;
if (qemuBuildSmartcardCommandLine(logManager, secManager, cmd, cfg, def, qemuCaps,
--
2.17.1
6:36 a.m.
New subject: [libvirt] [PATCH 2/3] qemu: consolidate parameters of qemuBuildChrChardevStr into flags
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>
---
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
7:43 a.m.
New subject: [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>
---
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
10:25 a.m.
New subject: [libvirt] [PATCH 2/3] qemu: consolidate parameters of qemuBuildChrChardevStr into flags
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(a)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 :|
6:36 a.m.
New subject: [libvirt] [PATCH 3/3] qemu: don't use chardev FD passing for vhostuser backend
QEMU chardevs have a bug which makes the vhostuser backend complain
about lack of support for FD passing when validating the chardev.
While this is ultimately QEMU's responsibility to fix, libvirt needs to
avoid tickling the bug.
Simply disabling chardev FD passing just for vhostuser's chardev is
the most prudent approach, avoiding need for a QEMU version number
check.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/qemu/qemu_command.c | 31 +++++++++++++++--------
tests/qemuxml2argvdata/net-vhostuser.args | 3 +--
tests/qemuxml2argvtest.c | 2 +-
3 files changed, 23 insertions(+), 13 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 7f3eccc1bd..aaa6d0ceda 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4939,6 +4939,7 @@ qemuOpenChrChardevUNIXSocket(const virDomainChrSourceDef *dev)
enum {
QEMU_BUILD_CHARDEV_TCP_NOWAIT = (1 << 0),
QEMU_BUILD_CHARDEV_FILE_LOGD = (1 << 1),
+ QEMU_BUILD_CHARDEV_UNIX_FD_PASS = (1 << 2),
};
/* This function outputs a -chardev command line option which describes only the
@@ -5080,7 +5081,8 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
break;
case VIR_DOMAIN_CHR_TYPE_UNIX:
- if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV_FD_PASS)) {
+ if ((flags & QEMU_BUILD_CHARDEV_UNIX_FD_PASS) &&
+ virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV_FD_PASS)) {
if (qemuSecuritySetSocketLabel(secManager, (virDomainDefPtr)def) < 0)
goto cleanup;
int fd = qemuOpenChrChardevUNIXSocket(dev);
@@ -5434,7 +5436,8 @@ qemuBuildMonitorCommandLine(virLogManagerPtr logManager,
qemuDomainObjPrivatePtr priv)
{
char *chrdev;
- unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT;
+ unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT |
+ QEMU_BUILD_CHARDEV_UNIX_FD_PASS;
if (priv->chardevStdioLogd)
cdevflags |= QEMU_BUILD_CHARDEV_FILE_LOGD;
@@ -5569,7 +5572,8 @@ qemuBuildRNGBackendChrdevStr(virLogManagerPtr logManager,
char **chr,
bool chardevStdioLogd)
{
- unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT;
+ unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT |
+ QEMU_BUILD_CHARDEV_UNIX_FD_PASS;
if (chardevStdioLogd)
cdevflags |= QEMU_BUILD_CHARDEV_FILE_LOGD;
*chr = NULL;
@@ -8708,7 +8712,8 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr logManager,
virBuffer opt = VIR_BUFFER_INITIALIZER;
const char *database;
const char *contAlias = NULL;
- unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT;
+ unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT |
+ QEMU_BUILD_CHARDEV_UNIX_FD_PASS;
if (chardevStdioLogd)
cdevflags |= QEMU_BUILD_CHARDEV_FILE_LOGD;
@@ -8944,7 +8949,8 @@ qemuBuildShmemCommandLine(virLogManagerPtr logManager,
virBuffer buf = VIR_BUFFER_INITIALIZER;
char *devstr = NULL;
int rc;
- unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT;
+ unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT |
+ QEMU_BUILD_CHARDEV_UNIX_FD_PASS;
if (chardevStdioLogd)
cdevflags |= QEMU_BUILD_CHARDEV_FILE_LOGD;
@@ -9104,7 +9110,8 @@ qemuBuildSerialCommandLine(virLogManagerPtr logManager,
{
size_t i;
bool havespice = false;
- unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT;
+ unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT |
+ QEMU_BUILD_CHARDEV_UNIX_FD_PASS;
if (chardevStdioLogd)
cdevflags |= QEMU_BUILD_CHARDEV_FILE_LOGD;
@@ -9167,7 +9174,8 @@ qemuBuildParallelsCommandLine(virLogManagerPtr logManager,
bool chardevStdioLogd)
{
size_t i;
- unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT;
+ unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT |
+ QEMU_BUILD_CHARDEV_UNIX_FD_PASS;
if (chardevStdioLogd)
cdevflags |= QEMU_BUILD_CHARDEV_FILE_LOGD;
@@ -9204,7 +9212,8 @@ qemuBuildChannelsCommandLine(virLogManagerPtr logManager,
bool chardevStdioLogd)
{
size_t i;
- unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT;
+ unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT |
+ QEMU_BUILD_CHARDEV_UNIX_FD_PASS;
if (chardevStdioLogd)
cdevflags |= QEMU_BUILD_CHARDEV_FILE_LOGD;
@@ -9261,7 +9270,8 @@ qemuBuildConsoleCommandLine(virLogManagerPtr logManager,
bool chardevStdioLogd)
{
size_t i;
- unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT;
+ unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT |
+ QEMU_BUILD_CHARDEV_UNIX_FD_PASS;
if (chardevStdioLogd)
cdevflags |= QEMU_BUILD_CHARDEV_FILE_LOGD;
@@ -9441,7 +9451,8 @@ qemuBuildRedirdevCommandLine(virLogManagerPtr logManager,
bool chardevStdioLogd)
{
size_t i;
- unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT;
+ unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT |
+ QEMU_BUILD_CHARDEV_UNIX_FD_PASS;
if (chardevStdioLogd)
cdevflags |= QEMU_BUILD_CHARDEV_FILE_LOGD;
diff --git a/tests/qemuxml2argvdata/net-vhostuser.args
b/tests/qemuxml2argvdata/net-vhostuser.args
index fc4557a1f2..513fc535ab 100644
--- a/tests/qemuxml2argvdata/net-vhostuser.args
+++ b/tests/qemuxml2argvdata/net-vhostuser.args
@@ -14,8 +14,7 @@ QEMU_AUDIO_DRV=none \
-display none \
-no-user-config \
-nodefaults \
--chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
-server,nowait \
+-chardev socket,id=charmonitor,fd=1729,server,nowait \
-mon chardev=charmonitor,id=monitor,mode=control \
-rtc base=utc \
-no-shutdown \
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index d6911f9344..2d52f352b0 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1266,7 +1266,7 @@ mymain(void)
DO_TEST("misc-no-reboot", NONE);
DO_TEST("misc-uuid", NONE);
DO_TEST_PARSE_ERROR("vhost_queues-invalid", NONE);
- DO_TEST("net-vhostuser", NONE);
+ DO_TEST("net-vhostuser", QEMU_CAPS_CHARDEV_FD_PASS);
DO_TEST("net-vhostuser-multiq",
QEMU_CAPS_VHOSTUSER_MULTIQUEUE);
DO_TEST_FAILURE("net-vhostuser-multiq", NONE);
--
2.17.1
7:43 a.m.
On 07/05/2018 01:36 PM, Daniel P. Berrangé wrote:
QEMU has a validation bug which prevents vhostuser accepting the
chardev
when it uses FD passing, despite it being functionally correct.
While the QEMU bug is being fixed, we need to avoid hitting this
problem with existing QEMU releases.
The easiest way todo this is to simply disable FD passing for the
chardev used with vhostuser only.
Daniel P. Berrangé (3):
qemu: remove chardevStdioLogd param from vhostuser code path
qemu: consolidate parameters of qemuBuildChrChardevStr into flags
qemu: don't use chardev FD passing for vhostuser backend
src/qemu/qemu_command.c | 117 ++++++++++++++--------
tests/qemuxml2argvdata/net-vhostuser.args | 3 +-
tests/qemuxml2argvtest.c | 2 +-
3 files changed, 75 insertions(+), 47 deletions(-)
ACK series.
Michal
2332
days inactive
2332
days old
6 comments
2 participants
participants (2)
-
Daniel P. Berrangé -
Michal Prívozník