Re: [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.

> This patch unifies the aliases for tls-creds-x509 to be always generated > from "charserial0". > > Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; > --- > 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 >