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@xxxxxxxxxx>
> ---
> 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
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list