[libvirt] [PATCH] conf: Fill listenAddr from driver if none set

[Michal Privoznik]

On 25.06.2012 13:53, Daniel P. Berrange wrote:
> On Fri, Jun 22, 2012 at 03:52:45PM +0200, Michal Privoznik wrote:
>> Currently, if domain's graphic has no listenAddr set we provide
>> the one from qemu.conf. However, we must make this transient to
>> not overwrite future changes to the config file. Moreover, we
>> have to store this into domain state XML so we remain consistent
>> on eventual config file edits.
>> ---
>> Since users will never see new 'driverListenAddress' attribute
>> I don't think update of RNG schema is necessary.
>>
>>  src/conf/domain_conf.c  |   14 ++++++++++++++
>>  src/conf/domain_conf.h  |    6 ++++++
>>  src/qemu/qemu_command.c |   18 ++++++++++++++++--
>>  3 files changed, 36 insertions(+), 2 deletions(-)
> 
> This is a really wierd way to go about including the listen
> address. It can trivially be done without putting hacks like
> this into the XML code.

Indeed.

> 
> From 25273cfefec989d78ec867b6ff56e22f7317f82a Mon Sep 17 00:00:00 2001
> From: "Daniel P. Berrange" <berrange at redhat.com>
> Date: Mon, 25 Jun 2012 12:50:52 +0100
> Subject: [PATCH] Include the default listen address in the live guest XML
> 
> If no 'listen' attribute or <listen> element is set in the
> guest XML, the default driver configured listen address is
> used. There is no way to client applications to determine
> what this address is though. When starting the guest, we
> should update the live XML to include this default listen
> address
> ---
>  src/qemu/qemu_process.c |   21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 1df3637..c5140c3 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -3429,6 +3429,27 @@ int qemuProcessStart(virConnectPtr conn,
>                  vm->def->graphics[0]->data.spice.tlsPort = tlsPort;
>              }
>          }
> +
> +        if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC ||
> +            vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
> +            virDomainGraphicsDefPtr graphics = vm->def->graphics[0];
> +            if (graphics->nListens == 0) {
> +                if (VIR_EXPAND_N(graphics->listens, graphics->nListens, 1) < 0) {
> +                    virReportOOMError();
> +                    goto cleanup;
> +                }
> +                graphics->listens[0].type = VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS;
> +                if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC)
> +                    graphics->listens[0].address = strdup(driver->vncListen);
> +                else
> +                    graphics->listens[0].address = strdup(driver->spiceListen);
> +                if (!graphics->listens[0].address) {
> +                    VIR_SHRINK_N(graphics->listens, graphics->nListens, 1);
> +                    virReportOOMError();
> +                    goto cleanup;
> +                }
> +            }
> +        }
>      }
>  
>      if (virFileMakePath(driver->logDir) < 0) {
> 


I like this one more. ACK.

Michal