[libvirt] [PATCH] libxl: add default VNC listen address

Jim Fehlig posted 1 patch 6 years, 11 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20170517213848.32089-1-jfehlig@suse.com
src/libxl/libxl_conf.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
[libvirt] [PATCH] libxl: add default VNC listen address
Posted by Jim Fehlig 6 years, 11 months ago
If a VNC listen address is not specified in domXML, libxl will
default to 127.0.0.1, but this is never reflected in the domXML.
If not specified, set the listen address in virDomainGraphicsDef
struct to the libxl default when creating the frame buffer device.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1334562
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
 src/libxl/libxl_conf.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 56bc09719..0c5d7a700 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -1227,12 +1227,16 @@ libxlMakeVfb(virPortAllocatorPtr graphicsports,
             }
             x_vfb->vnc.display = l_vfb->data.vnc.port - LIBXL_VNC_PORT_MIN;
 
-            if ((glisten = virDomainGraphicsGetListen(l_vfb, 0)) &&
-                glisten->address) {
-                /* libxl_device_vfb_init() does VIR_STRDUP("127.0.0.1") */
-                VIR_FREE(x_vfb->vnc.listen);
-                if (VIR_STRDUP(x_vfb->vnc.listen, glisten->address) < 0)
-                    return -1;
+            if ((glisten = virDomainGraphicsGetListen(l_vfb, 0))) {
+                if (glisten->address) {
+                    /* libxl_device_vfb_init() does VIR_STRDUP("127.0.0.1") */
+                    VIR_FREE(x_vfb->vnc.listen);
+                    if (VIR_STRDUP(x_vfb->vnc.listen, glisten->address) < 0)
+                        return -1;
+                } else {
+                    if (VIR_STRDUP(glisten->address, "127.0.0.1") < 0)
+                        return -1;
+                }
             }
             if (VIR_STRDUP(x_vfb->vnc.passwd, l_vfb->data.vnc.auth.passwd) < 0)
                 return -1;
-- 
2.11.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] libxl: add default VNC listen address
Posted by Joao Martins 6 years, 11 months ago

On 05/17/2017 10:38 PM, Jim Fehlig wrote:
> If a VNC listen address is not specified in domXML, libxl will
> default to 127.0.0.1, but this is never reflected in the domXML.
> If not specified, set the listen address in virDomainGraphicsDef
> struct to the libxl default when creating the frame buffer device.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1334562
> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
>
Just a nit (more of a suggestion in the future), "127.0.0.1" seems to be
hardcoded all over libvirt AFAIK (e.g. qemu driver, socketaddr.c) , maybe
defining a macro in a well defined place would be better.

Other than that,

Reviewed-by: Joao Martins <joao.m.martins@oracle.com>

This fix should silence that message virt-viewer warnings. nice :)

Joao

> ---
>  src/libxl/libxl_conf.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index 56bc09719..0c5d7a700 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -1227,12 +1227,16 @@ libxlMakeVfb(virPortAllocatorPtr graphicsports,
>              }
>              x_vfb->vnc.display = l_vfb->data.vnc.port - LIBXL_VNC_PORT_MIN;
>  
> -            if ((glisten = virDomainGraphicsGetListen(l_vfb, 0)) &&
> -                glisten->address) {
> -                /* libxl_device_vfb_init() does VIR_STRDUP("127.0.0.1") */
> -                VIR_FREE(x_vfb->vnc.listen);
> -                if (VIR_STRDUP(x_vfb->vnc.listen, glisten->address) < 0)
> -                    return -1;
> +            if ((glisten = virDomainGraphicsGetListen(l_vfb, 0))) {
> +                if (glisten->address) {
> +                    /* libxl_device_vfb_init() does VIR_STRDUP("127.0.0.1") */
> +                    VIR_FREE(x_vfb->vnc.listen);
> +                    if (VIR_STRDUP(x_vfb->vnc.listen, glisten->address) < 0)
> +                        return -1;
> +                } else {
> +                    if (VIR_STRDUP(glisten->address, "127.0.0.1") < 0)
> +                        return -1;
> +                }
>              }
>              if (VIR_STRDUP(x_vfb->vnc.passwd, l_vfb->data.vnc.auth.passwd) < 0)
>                  return -1;
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] libxl: add default VNC listen address
Posted by Jim Fehlig 6 years, 11 months ago
On 05/19/2017 11:56 AM, Joao Martins wrote:
>
>
> On 05/17/2017 10:38 PM, Jim Fehlig wrote:
>> If a VNC listen address is not specified in domXML, libxl will
>> default to 127.0.0.1, but this is never reflected in the domXML.
>> If not specified, set the listen address in virDomainGraphicsDef
>> struct to the libxl default when creating the frame buffer device.
>>
>> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1334562
>> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
>>
> Just a nit (more of a suggestion in the future), "127.0.0.1" seems to be
> hardcoded all over libvirt AFAIK (e.g. qemu driver, socketaddr.c) , maybe
> defining a macro in a well defined place would be better.

Thanks! I've sent a V2 series where patch1 does just that (excluding tests/ 
directory). In V2, this patch becomes patch2 and is also enhanced to cover spice.

https://www.redhat.com/archives/libvir-list/2017-May/msg00760.html

Regards,
Jim

>
> Other than that,
>
> Reviewed-by: Joao Martins <joao.m.martins@oracle.com>
>
> This fix should silence that message virt-viewer warnings. nice :)
>
> Joao
>
>> ---
>>  src/libxl/libxl_conf.c | 16 ++++++++++------
>>  1 file changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
>> index 56bc09719..0c5d7a700 100644
>> --- a/src/libxl/libxl_conf.c
>> +++ b/src/libxl/libxl_conf.c
>> @@ -1227,12 +1227,16 @@ libxlMakeVfb(virPortAllocatorPtr graphicsports,
>>              }
>>              x_vfb->vnc.display = l_vfb->data.vnc.port - LIBXL_VNC_PORT_MIN;
>>
>> -            if ((glisten = virDomainGraphicsGetListen(l_vfb, 0)) &&
>> -                glisten->address) {
>> -                /* libxl_device_vfb_init() does VIR_STRDUP("127.0.0.1") */
>> -                VIR_FREE(x_vfb->vnc.listen);
>> -                if (VIR_STRDUP(x_vfb->vnc.listen, glisten->address) < 0)
>> -                    return -1;
>> +            if ((glisten = virDomainGraphicsGetListen(l_vfb, 0))) {
>> +                if (glisten->address) {
>> +                    /* libxl_device_vfb_init() does VIR_STRDUP("127.0.0.1") */
>> +                    VIR_FREE(x_vfb->vnc.listen);
>> +                    if (VIR_STRDUP(x_vfb->vnc.listen, glisten->address) < 0)
>> +                        return -1;
>> +                } else {
>> +                    if (VIR_STRDUP(glisten->address, "127.0.0.1") < 0)
>> +                        return -1;
>> +                }
>>              }
>>              if (VIR_STRDUP(x_vfb->vnc.passwd, l_vfb->data.vnc.auth.passwd) < 0)
>>                  return -1;
>>
>

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