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