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 <phrd...@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
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to