8:58 a.m.
Pavel Hrdina (2):
qemu_command: create prefixed alias to separate variable
qemu: always generate the same alias for tls-creds-x509 object
src/qemu/qemu_command.c | 39 ++++++++++++----------
src/qemu/qemu_hotplug.c | 9 +++--
...xml2argv-serial-tcp-tlsx509-chardev-verify.args | 4 +--
.../qemuxml2argv-serial-tcp-tlsx509-chardev.args | 4 +--
4 files changed, 33 insertions(+), 23 deletions(-)
--
2.10.1
8:58 a.m.
New subject: [libvirt] [PATCH 1/2] qemu_command: create prefixed alias to separate variable
Instead of typing the prefix every time we want to append parameters
to qemu command line use a variable that contains prefixed alias.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_command.c | 35 ++++++++++++++++++++---------------
1 file changed, 20 insertions(+), 15 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 21fd85c..74f65c0 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4861,28 +4861,32 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
bool telnet;
+ char *charAlias = NULL;
+
+ if (virAsprintf(&charAlias, "char%s", alias) < 0)
+ goto error;
switch (dev->type) {
case VIR_DOMAIN_CHR_TYPE_NULL:
- virBufferAsprintf(&buf, "null,id=char%s", alias);
+ virBufferAsprintf(&buf, "null,id=%s", charAlias);
break;
case VIR_DOMAIN_CHR_TYPE_VC:
- virBufferAsprintf(&buf, "vc,id=char%s", alias);
+ virBufferAsprintf(&buf, "vc,id=%s", charAlias);
break;
case VIR_DOMAIN_CHR_TYPE_PTY:
- virBufferAsprintf(&buf, "pty,id=char%s", alias);
+ virBufferAsprintf(&buf, "pty,id=%s", charAlias);
break;
case VIR_DOMAIN_CHR_TYPE_DEV:
- virBufferAsprintf(&buf, "%s,id=char%s,path=%s",
+ virBufferAsprintf(&buf, "%s,id=%s,path=%s",
STRPREFIX(alias, "parallel") ? "parport" :
"tty",
- alias, dev->data.file.path);
+ charAlias, dev->data.file.path);
break;
case VIR_DOMAIN_CHR_TYPE_FILE:
- virBufferAsprintf(&buf, "file,id=char%s", alias);
+ virBufferAsprintf(&buf, "file,id=%s", charAlias);
if (dev->data.file.append != VIR_TRISTATE_SWITCH_ABSENT &&
!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV_FILE_APPEND)) {
@@ -4898,12 +4902,12 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
break;
case VIR_DOMAIN_CHR_TYPE_PIPE:
- virBufferAsprintf(&buf, "pipe,id=char%s,path=%s", alias,
+ virBufferAsprintf(&buf, "pipe,id=%s,path=%s", charAlias,
dev->data.file.path);
break;
case VIR_DOMAIN_CHR_TYPE_STDIO:
- virBufferAsprintf(&buf, "stdio,id=char%s", alias);
+ virBufferAsprintf(&buf, "stdio,id=%s", charAlias);
break;
case VIR_DOMAIN_CHR_TYPE_UDP: {
@@ -4919,9 +4923,9 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
bindService = "0";
virBufferAsprintf(&buf,
- "udp,id=char%s,host=%s,port=%s,localaddr=%s,"
+ "udp,id=%s,host=%s,port=%s,localaddr=%s,"
"localport=%s",
- alias,
+ charAlias,
connectHost,
dev->data.udp.connectService,
bindHost, bindService);
@@ -4930,8 +4934,8 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
case VIR_DOMAIN_CHR_TYPE_TCP:
telnet = dev->data.tcp.protocol == VIR_DOMAIN_CHR_TCP_PROTOCOL_TELNET;
virBufferAsprintf(&buf,
- "socket,id=char%s,host=%s,port=%s%s",
- alias,
+ "socket,id=%s,host=%s,port=%s%s",
+ charAlias,
dev->data.tcp.host,
dev->data.tcp.service,
telnet ? ",telnet" : "");
@@ -4956,7 +4960,7 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
break;
case VIR_DOMAIN_CHR_TYPE_UNIX:
- virBufferAsprintf(&buf, "socket,id=char%s,path=", alias);
+ virBufferAsprintf(&buf, "socket,id=%s,path=", charAlias);
virQEMUBuildBufferEscapeComma(&buf, dev->data.nix.path);
if (dev->data.nix.listen)
virBufferAdd(&buf, nowait ? ",server,nowait" :
",server", -1);
@@ -4968,7 +4972,7 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
_("spicevmc not supported in this QEMU binary"));
goto error;
}
- virBufferAsprintf(&buf, "spicevmc,id=char%s,name=%s", alias,
+ virBufferAsprintf(&buf, "spicevmc,id=%s,name=%s", charAlias,
virDomainChrSpicevmcTypeToString(dev->data.spicevmc));
break;
@@ -4978,7 +4982,7 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
_("spiceport not supported in this QEMU binary"));
goto error;
}
- virBufferAsprintf(&buf, "spiceport,id=char%s,name=%s", alias,
+ virBufferAsprintf(&buf, "spiceport,id=%s,name=%s", charAlias,
dev->data.spiceport.channel);
break;
@@ -5007,6 +5011,7 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
return virBufferContentAndReset(&buf);
error:
+ VIR_FREE(charAlias);
virBufferFreeAndReset(&buf);
return NULL;
}
--
2.10.1
9:22 a.m.
New subject: [libvirt] [PATCH 1/2] qemu_command: create prefixed alias to separate variable
On 10/18/2016 09:58 AM, Pavel Hrdina wrote:
Instead of typing the prefix every time we want to append parameters
to qemu command line use a variable that contains prefixed alias.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_command.c | 35 ++++++++++++++++++++---------------
1 file changed, 20 insertions(+), 15 deletions(-)
Why not create a qemu_alias.c helper that then can also be used in your
followup patch?
John
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 21fd85c..74f65c0 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4861,28 +4861,32 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
bool telnet;
+ char *charAlias = NULL;
+
+ if (virAsprintf(&charAlias, "char%s", alias) < 0)
+ goto error;
switch (dev->type) {
case VIR_DOMAIN_CHR_TYPE_NULL:
- virBufferAsprintf(&buf, "null,id=char%s", alias);
+ virBufferAsprintf(&buf, "null,id=%s", charAlias);
break;
case VIR_DOMAIN_CHR_TYPE_VC:
- virBufferAsprintf(&buf, "vc,id=char%s", alias);
+ virBufferAsprintf(&buf, "vc,id=%s", charAlias);
break;
case VIR_DOMAIN_CHR_TYPE_PTY:
- virBufferAsprintf(&buf, "pty,id=char%s", alias);
+ virBufferAsprintf(&buf, "pty,id=%s", charAlias);
break;
case VIR_DOMAIN_CHR_TYPE_DEV:
- virBufferAsprintf(&buf, "%s,id=char%s,path=%s",
+ virBufferAsprintf(&buf, "%s,id=%s,path=%s",
STRPREFIX(alias, "parallel") ? "parport" :
"tty",
- alias, dev->data.file.path);
+ charAlias, dev->data.file.path);
break;
case VIR_DOMAIN_CHR_TYPE_FILE:
- virBufferAsprintf(&buf, "file,id=char%s", alias);
+ virBufferAsprintf(&buf, "file,id=%s", charAlias);
if (dev->data.file.append != VIR_TRISTATE_SWITCH_ABSENT &&
!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV_FILE_APPEND)) {
@@ -4898,12 +4902,12 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
break;
case VIR_DOMAIN_CHR_TYPE_PIPE:
- virBufferAsprintf(&buf, "pipe,id=char%s,path=%s", alias,
+ virBufferAsprintf(&buf, "pipe,id=%s,path=%s", charAlias,
dev->data.file.path);
break;
case VIR_DOMAIN_CHR_TYPE_STDIO:
- virBufferAsprintf(&buf, "stdio,id=char%s", alias);
+ virBufferAsprintf(&buf, "stdio,id=%s", charAlias);
break;
case VIR_DOMAIN_CHR_TYPE_UDP: {
@@ -4919,9 +4923,9 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
bindService = "0";
virBufferAsprintf(&buf,
- "udp,id=char%s,host=%s,port=%s,localaddr=%s,"
+ "udp,id=%s,host=%s,port=%s,localaddr=%s,"
"localport=%s",
- alias,
+ charAlias,
connectHost,
dev->data.udp.connectService,
bindHost, bindService);
@@ -4930,8 +4934,8 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
case VIR_DOMAIN_CHR_TYPE_TCP:
telnet = dev->data.tcp.protocol == VIR_DOMAIN_CHR_TCP_PROTOCOL_TELNET;
virBufferAsprintf(&buf,
- "socket,id=char%s,host=%s,port=%s%s",
- alias,
+ "socket,id=%s,host=%s,port=%s%s",
+ charAlias,
dev->data.tcp.host,
dev->data.tcp.service,
telnet ? ",telnet" : "");
@@ -4956,7 +4960,7 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
break;
case VIR_DOMAIN_CHR_TYPE_UNIX:
- virBufferAsprintf(&buf, "socket,id=char%s,path=", alias);
+ virBufferAsprintf(&buf, "socket,id=%s,path=", charAlias);
virQEMUBuildBufferEscapeComma(&buf, dev->data.nix.path);
if (dev->data.nix.listen)
virBufferAdd(&buf, nowait ? ",server,nowait" :
",server", -1);
@@ -4968,7 +4972,7 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
_("spicevmc not supported in this QEMU binary"));
goto error;
}
- virBufferAsprintf(&buf, "spicevmc,id=char%s,name=%s", alias,
+ virBufferAsprintf(&buf, "spicevmc,id=%s,name=%s", charAlias,
virDomainChrSpicevmcTypeToString(dev->data.spicevmc));
break;
@@ -4978,7 +4982,7 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
_("spiceport not supported in this QEMU binary"));
goto error;
}
- virBufferAsprintf(&buf, "spiceport,id=char%s,name=%s", alias,
+ virBufferAsprintf(&buf, "spiceport,id=%s,name=%s", charAlias,
dev->data.spiceport.channel);
break;
@@ -5007,6 +5011,7 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
return virBufferContentAndReset(&buf);
error:
+ VIR_FREE(charAlias);
virBufferFreeAndReset(&buf);
return NULL;
}
9:31 a.m.
New subject: [libvirt] [PATCH 1/2] qemu_command: create prefixed alias to separate variable
On Tue, Oct 18, 2016 at 10:22:50AM -0400, John Ferlan wrote:
On 10/18/2016 09:58 AM, Pavel Hrdina wrote:
> Instead of typing the prefix every time we want to append parameters
> to qemu command line use a variable that contains prefixed alias.
>
> Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
> ---
> src/qemu/qemu_command.c | 35 ++++++++++++++++++++---------------
> 1 file changed, 20 insertions(+), 15 deletions(-)
>
Why not create a qemu_alias.c helper that then can also be used in your
followup patch?
That's a good point, I did not realize that we have qemu_alias.c.
I'll send v2, thanks.
Pavel
8:58 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: always generate the same alias for tls-creds-x509 object
There was inconsistency between alias used to create tls-creds-x509
object and alias used to link that object to chardev while hotpluging.
In XML we have for example alias "serial0", but on qemu command line we
generate "charserial0".
The issue was that code, that creates QMP command to hotplug chardev
devices uses only the second alias "charserial0" and that alias is also
used to link the tls-creds-x509 object.
This patch unifies the aliases for tls-creds-x509 to be always generated
from "charserial0".
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_command.c | 4 ++--
src/qemu/qemu_hotplug.c | 9 +++++++--
.../qemuxml2argv-serial-tcp-tlsx509-chardev-verify.args | 4 ++--
.../qemuxml2argv-serial-tcp-tlsx509-chardev.args | 4 ++--
4 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 74f65c0..8d87e69 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4949,10 +4949,10 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
if (qemuBuildTLSx509CommandLine(cmd, cfg->chardevTLSx509certdir,
dev->data.tcp.listen,
cfg->chardevTLSx509verify,
- alias, qemuCaps) < 0)
+ charAlias, qemuCaps) < 0)
goto error;
- if (!(objalias = qemuAliasTLSObjFromChardevAlias(alias)))
+ if (!(objalias = qemuAliasTLSObjFromChardevAlias(charAlias)))
goto error;
virBufferAsprintf(&buf, ",tls-creds=%s", objalias);
VIR_FREE(objalias);
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index c2ba935..e39bd8b 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1738,7 +1738,7 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
&tlsProps) < 0)
goto cleanup;
- if (!(tlsAlias = qemuAliasTLSObjFromChardevAlias(chr->info.alias)))
+ if (!(tlsAlias = qemuAliasTLSObjFromChardevAlias(charAlias)))
goto cleanup;
dev->data.tcp.tlscreds = true;
}
@@ -4387,6 +4387,7 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
virDomainChrDefPtr tmpChr;
char *objAlias = NULL;
char *devstr = NULL;
+ char *charAlias = NULL;
if (!(tmpChr = virDomainChrFind(vmdef, chr))) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
@@ -4397,11 +4398,14 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
if (!tmpChr->info.alias && qemuAssignDeviceChrAlias(vmdef, tmpChr, -1)
< 0)
goto cleanup;
+ if (virAsprintf(&charAlias, "char%s", tmpChr->info.alias) < 0)
+ goto cleanup;
+
sa_assert(tmpChr->info.alias);
if (tmpChr->source.type == VIR_DOMAIN_CHR_TYPE_TCP &&
cfg->chardevTLS &&
- !(objAlias = qemuAliasTLSObjFromChardevAlias(tmpChr->info.alias)))
+ !(objAlias = qemuAliasTLSObjFromChardevAlias(charAlias)))
goto cleanup;
if (qemuBuildChrDeviceStr(&devstr, vmdef, chr, priv->qemuCaps) < 0)
@@ -4427,6 +4431,7 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
cleanup:
qemuDomainResetDeviceRemoval(vm);
VIR_FREE(devstr);
+ VIR_FREE(charAlias);
virObjectUnref(cfg);
return ret;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.args
b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.args
index f521e33..003d11d 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.args
@@ -25,9 +25,9 @@ server,nowait \
-chardev udp,id=charserial0,host=127.0.0.1,port=2222,localaddr=127.0.0.1,\
localport=1111 \
-device isa-serial,chardev=charserial0,id=serial0 \
--object tls-creds-x509,id=objserial1_tls0,dir=/etc/pki/libvirt-chardev,\
+-object tls-creds-x509,id=objcharserial1_tls0,dir=/etc/pki/libvirt-chardev,\
endpoint=client,verify-peer=yes \
-chardev socket,id=charserial1,host=127.0.0.1,port=5555,\
-tls-creds=objserial1_tls0 \
+tls-creds=objcharserial1_tls0 \
-device isa-serial,chardev=charserial1,id=serial1 \
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.args
b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.args
index 4c8c23e..b456cce 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.args
@@ -25,9 +25,9 @@ server,nowait \
-chardev udp,id=charserial0,host=127.0.0.1,port=2222,localaddr=127.0.0.1,\
localport=1111 \
-device isa-serial,chardev=charserial0,id=serial0 \
--object tls-creds-x509,id=objserial1_tls0,dir=/etc/pki/libvirt-chardev,\
+-object tls-creds-x509,id=objcharserial1_tls0,dir=/etc/pki/libvirt-chardev,\
endpoint=client,verify-peer=no \
-chardev socket,id=charserial1,host=127.0.0.1,port=5555,\
-tls-creds=objserial1_tls0 \
+tls-creds=objcharserial1_tls0 \
-device isa-serial,chardev=charserial1,id=serial1 \
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
--
2.10.1
9:37 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: always generate the same alias for tls-creds-x509 object
On 10/18/2016 09:58 AM, Pavel Hrdina wrote:
There was inconsistency between alias used to create tls-creds-x509
object and alias used to link that object to chardev while hotpluging.
In XML we have for example alias "serial0", but on qemu command line we
generate "charserial0".
The issue was that code, that creates QMP command to hotplug chardev
devices uses only the second alias "charserial0" and that alias is also
used to link the tls-creds-x509 object.
Which two objects used the same ID in hotplug?
tlsProps would use obj%s_tls0
and this changes it to be essentially objchar%s_tls0
Essentially I'm trying to figure out from the commit message prior to
this patch what was created incorrectly.
John
This patch unifies the aliases for tls-creds-x509 to be always generated
from "charserial0".
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_command.c | 4 ++--
src/qemu/qemu_hotplug.c | 9 +++++++--
.../qemuxml2argv-serial-tcp-tlsx509-chardev-verify.args | 4 ++--
.../qemuxml2argv-serial-tcp-tlsx509-chardev.args | 4 ++--
4 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 74f65c0..8d87e69 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4949,10 +4949,10 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
if (qemuBuildTLSx509CommandLine(cmd, cfg->chardevTLSx509certdir,
dev->data.tcp.listen,
cfg->chardevTLSx509verify,
- alias, qemuCaps) < 0)
+ charAlias, qemuCaps) < 0)
goto error;
- if (!(objalias = qemuAliasTLSObjFromChardevAlias(alias)))
+ if (!(objalias = qemuAliasTLSObjFromChardevAlias(charAlias)))
goto error;
virBufferAsprintf(&buf, ",tls-creds=%s", objalias);
VIR_FREE(objalias);
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index c2ba935..e39bd8b 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1738,7 +1738,7 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
&tlsProps) < 0)
goto cleanup;
- if (!(tlsAlias = qemuAliasTLSObjFromChardevAlias(chr->info.alias)))
+ if (!(tlsAlias = qemuAliasTLSObjFromChardevAlias(charAlias)))
goto cleanup;
dev->data.tcp.tlscreds = true;
}
@@ -4387,6 +4387,7 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
virDomainChrDefPtr tmpChr;
char *objAlias = NULL;
char *devstr = NULL;
+ char *charAlias = NULL;
if (!(tmpChr = virDomainChrFind(vmdef, chr))) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
@@ -4397,11 +4398,14 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
if (!tmpChr->info.alias && qemuAssignDeviceChrAlias(vmdef, tmpChr, -1)
< 0)
goto cleanup;
+ if (virAsprintf(&charAlias, "char%s", tmpChr->info.alias) < 0)
+ goto cleanup;
+
This would seemingly need to go after the subsequent line... Although I
think the subsequent line gets removed if use a qemu_alias.c helper.
sa_assert(tmpChr->info.alias);
if (tmpChr->source.type == VIR_DOMAIN_CHR_TYPE_TCP &&
cfg->chardevTLS &&
- !(objAlias = qemuAliasTLSObjFromChardevAlias(tmpChr->info.alias)))
+ !(objAlias = qemuAliasTLSObjFromChardevAlias(charAlias)))
goto cleanup;
if (qemuBuildChrDeviceStr(&devstr, vmdef, chr, priv->qemuCaps) < 0)
@@ -4427,6 +4431,7 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
cleanup:
qemuDomainResetDeviceRemoval(vm);
VIR_FREE(devstr);
+ VIR_FREE(charAlias);
virObjectUnref(cfg);
return ret;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.args
b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.args
index f521e33..003d11d 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.args
@@ -25,9 +25,9 @@ server,nowait \
-chardev udp,id=charserial0,host=127.0.0.1,port=2222,localaddr=127.0.0.1,\
localport=1111 \
-device isa-serial,chardev=charserial0,id=serial0 \
--object tls-creds-x509,id=objserial1_tls0,dir=/etc/pki/libvirt-chardev,\
+-object tls-creds-x509,id=objcharserial1_tls0,dir=/etc/pki/libvirt-chardev,\
endpoint=client,verify-peer=yes \
-chardev socket,id=charserial1,host=127.0.0.1,port=5555,\
-tls-creds=objserial1_tls0 \
+tls-creds=objcharserial1_tls0 \
-device isa-serial,chardev=charserial1,id=serial1 \
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.args
b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.args
index 4c8c23e..b456cce 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.args
@@ -25,9 +25,9 @@ server,nowait \
-chardev udp,id=charserial0,host=127.0.0.1,port=2222,localaddr=127.0.0.1,\
localport=1111 \
-device isa-serial,chardev=charserial0,id=serial0 \
--object tls-creds-x509,id=objserial1_tls0,dir=/etc/pki/libvirt-chardev,\
+-object tls-creds-x509,id=objcharserial1_tls0,dir=/etc/pki/libvirt-chardev,\
endpoint=client,verify-peer=no \
-chardev socket,id=charserial1,host=127.0.0.1,port=5555,\
-tls-creds=objserial1_tls0 \
+tls-creds=objcharserial1_tls0 \
-device isa-serial,chardev=charserial1,id=serial1 \
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
9:57 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: always generate the same alias for tls-creds-x509 object
On Tue, Oct 18, 2016 at 10:37:37AM -0400, John Ferlan wrote:
On 10/18/2016 09:58 AM, Pavel Hrdina wrote:
> There was inconsistency between alias used to create tls-creds-x509
> object and alias used to link that object to chardev while hotpluging.
>
> In XML we have for example alias "serial0", but on qemu command line we
> generate "charserial0".
>
> The issue was that code, that creates QMP command to hotplug chardev
> devices uses only the second alias "charserial0" and that alias is also
> used to link the tls-creds-x509 object.
Which two objects used the same ID in hotplug?
tlsProps would use obj%s_tls0
and this changes it to be essentially objchar%s_tls0
Essentially I'm trying to figure out from the commit message prior to
this patch what was created incorrectly.
The issue is that while hotpluging chardev device those QMP command are created
as I've described:
{
"execute":"object-add",
"arguments": {
"qom-type":"tls-creds-x509",
"id":"objchannel3_tls0",
^^^^^^^^^^^^^^^^
"props": {
"dir":"/etc/pki/libvirt-chardev",
"endpoint":"server",
"verify-peer":false
}
},
"id":"libvirt-29"
}
{
"execute":"chardev-add",
"arguments": {
"id":"charchannel3",
"backend": {
"type":"socket",
"data": {
"addr": {
"type":"inet",
"data": {
"host":"localhost",
"port":"3344"
}
},
"wait":false,
"telnet":false,
"server":true,
"tls-creds":"objcharchannel3_tls0"
^^^^^^^^^^^^^^^^^^^^
}
}
},
"id":"libvirt-30"
}
And as you can see the alias used to create tls-creds-x509 object is different
than the one used while attaching chardev. That's because function
qemuMonitorJSONAttachCharDevCommand() takes as first argument "charchannel3"
instead of "channel3". So the logical solution is to use
"charchannel3" as base
while creating "obj%s_tls0" alias.
> This patch unifies the aliases for tls-creds-x509 to be always
generated
> from "charserial0".
>
> Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
> ---
> src/qemu/qemu_command.c | 4 ++--
> src/qemu/qemu_hotplug.c | 9 +++++++--
> .../qemuxml2argv-serial-tcp-tlsx509-chardev-verify.args | 4 ++--
> .../qemuxml2argv-serial-tcp-tlsx509-chardev.args | 4 ++--
> 4 files changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 74f65c0..8d87e69 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -4949,10 +4949,10 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
> if (qemuBuildTLSx509CommandLine(cmd, cfg->chardevTLSx509certdir,
> dev->data.tcp.listen,
> cfg->chardevTLSx509verify,
> - alias, qemuCaps) < 0)
> + charAlias, qemuCaps) < 0)
> goto error;
>
> - if (!(objalias = qemuAliasTLSObjFromChardevAlias(alias)))
> + if (!(objalias = qemuAliasTLSObjFromChardevAlias(charAlias)))
> goto error;
> virBufferAsprintf(&buf, ",tls-creds=%s", objalias);
> VIR_FREE(objalias);
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index c2ba935..e39bd8b 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1738,7 +1738,7 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
> &tlsProps) < 0)
> goto cleanup;
>
> - if (!(tlsAlias = qemuAliasTLSObjFromChardevAlias(chr->info.alias)))
> + if (!(tlsAlias = qemuAliasTLSObjFromChardevAlias(charAlias)))
> goto cleanup;
> dev->data.tcp.tlscreds = true;
> }
> @@ -4387,6 +4387,7 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
> virDomainChrDefPtr tmpChr;
> char *objAlias = NULL;
> char *devstr = NULL;
> + char *charAlias = NULL;
>
> if (!(tmpChr = virDomainChrFind(vmdef, chr))) {
> virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> @@ -4397,11 +4398,14 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
> if (!tmpChr->info.alias && qemuAssignDeviceChrAlias(vmdef, tmpChr,
-1) < 0)
> goto cleanup;
>
> + if (virAsprintf(&charAlias, "char%s", tmpChr->info.alias) <
0)
> + goto cleanup;
> +
This would seemingly need to go after the subsequent line... Although I
think the subsequent line gets removed if use a qemu_alias.c helper.
The helper would be called here, so I'll move it after that assert.
Thanks
Pavel
> sa_assert(tmpChr->info.alias);
>
> if (tmpChr->source.type == VIR_DOMAIN_CHR_TYPE_TCP &&
> cfg->chardevTLS &&
> - !(objAlias = qemuAliasTLSObjFromChardevAlias(tmpChr->info.alias)))
> + !(objAlias = qemuAliasTLSObjFromChardevAlias(charAlias)))
> goto cleanup;
>
> if (qemuBuildChrDeviceStr(&devstr, vmdef, chr, priv->qemuCaps) < 0)
> @@ -4427,6 +4431,7 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
> cleanup:
> qemuDomainResetDeviceRemoval(vm);
> VIR_FREE(devstr);
> + VIR_FREE(charAlias);
> virObjectUnref(cfg);
> return ret;
>
> diff --git
a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.args
b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.args
> index f521e33..003d11d 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.args
> @@ -25,9 +25,9 @@ server,nowait \
> -chardev udp,id=charserial0,host=127.0.0.1,port=2222,localaddr=127.0.0.1,\
> localport=1111 \
> -device isa-serial,chardev=charserial0,id=serial0 \
> --object tls-creds-x509,id=objserial1_tls0,dir=/etc/pki/libvirt-chardev,\
> +-object tls-creds-x509,id=objcharserial1_tls0,dir=/etc/pki/libvirt-chardev,\
> endpoint=client,verify-peer=yes \
> -chardev socket,id=charserial1,host=127.0.0.1,port=5555,\
> -tls-creds=objserial1_tls0 \
> +tls-creds=objcharserial1_tls0 \
> -device isa-serial,chardev=charserial1,id=serial1 \
> -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.args
b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.args
> index 4c8c23e..b456cce 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.args
> @@ -25,9 +25,9 @@ server,nowait \
> -chardev udp,id=charserial0,host=127.0.0.1,port=2222,localaddr=127.0.0.1,\
> localport=1111 \
> -device isa-serial,chardev=charserial0,id=serial0 \
> --object tls-creds-x509,id=objserial1_tls0,dir=/etc/pki/libvirt-chardev,\
> +-object tls-creds-x509,id=objcharserial1_tls0,dir=/etc/pki/libvirt-chardev,\
> endpoint=client,verify-peer=no \
> -chardev socket,id=charserial1,host=127.0.0.1,port=5555,\
> -tls-creds=objserial1_tls0 \
> +tls-creds=objcharserial1_tls0 \
> -device isa-serial,chardev=charserial1,id=serial1 \
> -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
>
10:05 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: always generate the same alias for tls-creds-x509 object
On 10/18/2016 10:57 AM, Pavel Hrdina wrote:
On Tue, Oct 18, 2016 at 10:37:37AM -0400, John Ferlan wrote:
>
>
> On 10/18/2016 09:58 AM, Pavel Hrdina wrote:
>> There was inconsistency between alias used to create tls-creds-x509
>> object and alias used to link that object to chardev while hotpluging.
>>
>> In XML we have for example alias "serial0", but on qemu command line
we
>> generate "charserial0".
>>
>> The issue was that code, that creates QMP command to hotplug chardev
>> devices uses only the second alias "charserial0" and that alias is
also
>> used to link the tls-creds-x509 object.
>
> Which two objects used the same ID in hotplug?
>
> tlsProps would use obj%s_tls0
>
> and this changes it to be essentially objchar%s_tls0
>
> Essentially I'm trying to figure out from the commit message prior to
> this patch what was created incorrectly.
The issue is that while hotpluging chardev device those QMP command are created
as I've described:
{
"execute":"object-add",
"arguments": {
"qom-type":"tls-creds-x509",
"id":"objchannel3_tls0",
^^^^^^^^^^^^^^^^
"props": {
"dir":"/etc/pki/libvirt-chardev",
"endpoint":"server",
"verify-peer":false
}
},
"id":"libvirt-29"
}
{
"execute":"chardev-add",
"arguments": {
"id":"charchannel3",
"backend": {
"type":"socket",
"data": {
"addr": {
"type":"inet",
"data": {
"host":"localhost",
"port":"3344"
}
},
"wait":false,
"telnet":false,
"server":true,
"tls-creds":"objcharchannel3_tls0"
^^^^^^^^^^^^^^^^^^^^
}
}
},
"id":"libvirt-30"
}
And as you can see the alias used to create tls-creds-x509 object is different
than the one used while attaching chardev. That's because function
qemuMonitorJSONAttachCharDevCommand() takes as first argument "charchannel3"
instead of "channel3". So the logical solution is to use
"charchannel3" as base
while creating "obj%s_tls0" alias.
Ah... OK I see... I wasn't visualizing the chardev-add from the commit
message.
tks -
John
>> This patch unifies the aliases for tls-creds-x509 to be always generated
>> from "charserial0".
>>
>> Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
>> ---
>> src/qemu/qemu_command.c | 4 ++--
>> src/qemu/qemu_hotplug.c | 9 +++++++--
>> .../qemuxml2argv-serial-tcp-tlsx509-chardev-verify.args | 4 ++--
>> .../qemuxml2argv-serial-tcp-tlsx509-chardev.args | 4 ++--
>> 4 files changed, 13 insertions(+), 8 deletions(-)
>>
>
>
>
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index 74f65c0..8d87e69 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -4949,10 +4949,10 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
>> if (qemuBuildTLSx509CommandLine(cmd, cfg->chardevTLSx509certdir,
>> dev->data.tcp.listen,
>> cfg->chardevTLSx509verify,
>> - alias, qemuCaps) < 0)
>> + charAlias, qemuCaps) < 0)
>> goto error;
>>
>> - if (!(objalias = qemuAliasTLSObjFromChardevAlias(alias)))
>> + if (!(objalias = qemuAliasTLSObjFromChardevAlias(charAlias)))
>> goto error;
>> virBufferAsprintf(&buf, ",tls-creds=%s", objalias);
>> VIR_FREE(objalias);
>> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
>> index c2ba935..e39bd8b 100644
>> --- a/src/qemu/qemu_hotplug.c
>> +++ b/src/qemu/qemu_hotplug.c
>> @@ -1738,7 +1738,7 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
>> &tlsProps) < 0)
>> goto cleanup;
>>
>> - if (!(tlsAlias = qemuAliasTLSObjFromChardevAlias(chr->info.alias)))
>> + if (!(tlsAlias = qemuAliasTLSObjFromChardevAlias(charAlias)))
>> goto cleanup;
>> dev->data.tcp.tlscreds = true;
>> }
>> @@ -4387,6 +4387,7 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
>> virDomainChrDefPtr tmpChr;
>> char *objAlias = NULL;
>> char *devstr = NULL;
>> + char *charAlias = NULL;
>>
>> if (!(tmpChr = virDomainChrFind(vmdef, chr))) {
>> virReportError(VIR_ERR_OPERATION_INVALID, "%s",
>> @@ -4397,11 +4398,14 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
>> if (!tmpChr->info.alias && qemuAssignDeviceChrAlias(vmdef,
tmpChr, -1) < 0)
>> goto cleanup;
>>
>> + if (virAsprintf(&charAlias, "char%s", tmpChr->info.alias)
< 0)
>> + goto cleanup;
>> +
>
> This would seemingly need to go after the subsequent line... Although I
> think the subsequent line gets removed if use a qemu_alias.c helper.
The helper would be called here, so I'll move it after that assert.
Thanks
Pavel
>
>> sa_assert(tmpChr->info.alias);
>>
>> if (tmpChr->source.type == VIR_DOMAIN_CHR_TYPE_TCP &&
>> cfg->chardevTLS &&
>> - !(objAlias = qemuAliasTLSObjFromChardevAlias(tmpChr->info.alias)))
>> + !(objAlias = qemuAliasTLSObjFromChardevAlias(charAlias)))
>> goto cleanup;
>>
>> if (qemuBuildChrDeviceStr(&devstr, vmdef, chr, priv->qemuCaps) <
0)
>> @@ -4427,6 +4431,7 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
>> cleanup:
>> qemuDomainResetDeviceRemoval(vm);
>> VIR_FREE(devstr);
>> + VIR_FREE(charAlias);
>> virObjectUnref(cfg);
>> return ret;
>>
>> diff --git
a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.args
b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.args
>> index f521e33..003d11d 100644
>> --- a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.args
>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-verify.args
>> @@ -25,9 +25,9 @@ server,nowait \
>> -chardev udp,id=charserial0,host=127.0.0.1,port=2222,localaddr=127.0.0.1,\
>> localport=1111 \
>> -device isa-serial,chardev=charserial0,id=serial0 \
>> --object tls-creds-x509,id=objserial1_tls0,dir=/etc/pki/libvirt-chardev,\
>> +-object tls-creds-x509,id=objcharserial1_tls0,dir=/etc/pki/libvirt-chardev,\
>> endpoint=client,verify-peer=yes \
>> -chardev socket,id=charserial1,host=127.0.0.1,port=5555,\
>> -tls-creds=objserial1_tls0 \
>> +tls-creds=objcharserial1_tls0 \
>> -device isa-serial,chardev=charserial1,id=serial1 \
>> -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
>> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.args
b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.args
>> index 4c8c23e..b456cce 100644
>> --- a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.args
>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.args
>> @@ -25,9 +25,9 @@ server,nowait \
>> -chardev udp,id=charserial0,host=127.0.0.1,port=2222,localaddr=127.0.0.1,\
>> localport=1111 \
>> -device isa-serial,chardev=charserial0,id=serial0 \
>> --object tls-creds-x509,id=objserial1_tls0,dir=/etc/pki/libvirt-chardev,\
>> +-object tls-creds-x509,id=objcharserial1_tls0,dir=/etc/pki/libvirt-chardev,\
>> endpoint=client,verify-peer=no \
>> -chardev socket,id=charserial1,host=127.0.0.1,port=5555,\
>> -tls-creds=objserial1_tls0 \
>> +tls-creds=objcharserial1_tls0 \
>> -device isa-serial,chardev=charserial1,id=serial1 \
>> -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
>>
2992
days inactive
2992
days old
7 comments
2 participants
participants (2)
-
John Ferlan -
Pavel Hrdina