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