8:52 a.m.
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(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 4651765..29fa215 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -909,6 +909,7 @@ void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def)
for (ii = 0; ii < def->nListens; ii++)
virDomainGraphicsListenDefClear(&def->listens[ii]);
VIR_FREE(def->listens);
+ VIR_FREE(def->driverListenAddress);
VIR_FREE(def);
}
@@ -5976,6 +5977,7 @@ virDomainGraphicsDefParseXML(xmlNodePtr node,
int nListens;
xmlNodePtr *listenNodes = NULL;
char *listenAddr = NULL;
+ char *driverListenAddress = NULL;
xmlNodePtr save = ctxt->node;
if (VIR_ALLOC(def) < 0) {
@@ -5999,6 +6001,12 @@ virDomainGraphicsDefParseXML(xmlNodePtr node,
goto error;
}
+ if (flags & VIR_DOMAIN_XML_INTERNAL_STATUS) {
+ driverListenAddress = virXMLPropString(node, "driverListenAddress");
+ if (driverListenAddress)
+ def->driverListenAddress = driverListenAddress;
+ }
+
if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC ||
def->type == VIR_DOMAIN_GRAPHICS_TYPE_RDP ||
def->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
@@ -12323,9 +12331,15 @@ virDomainGraphicsDefFormat(virBufferPtr buf,
break;
}
}
+ if (!listenAddr && !(flags & VIR_DOMAIN_XML_INACTIVE))
+ listenAddr = def->driverListenAddress;
virBufferAsprintf(buf, " <graphics type='%s'", type);
+ if (flags & VIR_DOMAIN_XML_INTERNAL_STATUS &&
def->driverListenAddress)
+ virBufferAsprintf(buf, " driverListenAddress='%s'",
+ def->driverListenAddress);
+
switch (def->type) {
case VIR_DOMAIN_GRAPHICS_TYPE_VNC:
if (def->data.vnc.socket) {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 3712785..6ad48fd 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1273,6 +1273,12 @@ struct _virDomainGraphicsDef {
* simplify parsing code.*/
size_t nListens;
virDomainGraphicsListenDefPtr listens;
+
+ /* In qemu driver if none of 'listens' contain usable listen address,
+ * the one from qemu.conf file is taken. However, we cannot store it
+ * into inactive XML as it would discard all future changes made to
+ * the config file. */
+ char * driverListenAddress;
};
enum virDomainRedirdevBus {
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 6549f57..9878a66 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5496,8 +5496,15 @@ qemuBuildCommandLine(virConnectPtr conn,
break;
}
- if (!listenAddr)
+ if (!listenAddr) {
listenAddr = driver->vncListen;
+ if (listenAddr &&
+ !(def->graphics[0]->driverListenAddress =
+ strdup(listenAddr))) {
+ virReportOOMError();
+ goto error;
+ }
+ }
escapeAddr = strchr(listenAddr, ':') != NULL;
if (escapeAddr)
@@ -5647,8 +5654,15 @@ qemuBuildCommandLine(virConnectPtr conn,
break;
}
- if (!listenAddr)
+ if (!listenAddr) {
listenAddr = driver->spiceListen;
+ if (listenAddr &&
+ !(def->graphics[0]->driverListenAddress =
+ strdup(listenAddr))) {
+ virReportOOMError();
+ goto error;
+ }
+ }
if (listenAddr)
virBufferAsprintf(&opt, ",addr=%s", listenAddr);
--
1.7.8.5
6:53 a.m.
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.
From 25273cfefec989d78ec867b6ff56e22f7317f82a Mon Sep 17 00:00:00 2001
From: "Daniel P. Berrange" <berrange(a)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) {
--
1.7.10.2
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
7:03 a.m.
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(a)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
7:06 a.m.
On Mon, Jun 25, 2012 at 02:03:04PM +0200, Michal Privoznik wrote:
On 25.06.2012 13:53, Daniel P. Berrange wrote:
> From 25273cfefec989d78ec867b6ff56e22f7317f82a Mon Sep 17 00:00:00 2001
> From: "Daniel P. Berrange" <berrange(a)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.
Ok, pushed this
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
7:08 a.m.
On 25.06.2012 14:03, Michal Privoznik wrote:
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(a)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
Ugrh, I gave premature ACK. Problem with this approach is, once the
domain without any listenAddress set is started and hence we go through
this code snippet current settings from qemu.conf (that is
{vnc|spice}Listen) gets written into domain XML which is not desired.
Michal
7:14 a.m.
On Mon, Jun 25, 2012 at 02:08:23PM +0200, Michal Privoznik wrote:
On 25.06.2012 14:03, Michal Privoznik wrote:
> 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(a)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
>
Ugrh, I gave premature ACK. Problem with this approach is, once the
domain without any listenAddress set is started and hence we go through
this code snippet current settings from qemu.conf (that is
{vnc|spice}Listen) gets written into domain XML which is not desired.
This only affects the /live/ XML. The inactive XML still reports the
original user config, without this attribute, so updates of the inactive
XML will be preserved. This is just the same as any other attribute we
auto-add to the live XML, eg all the <address> elements, or VNC port,
or domain ID, or SELinux label, etc, etc
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
4534
days inactive
4537
days old
5 comments
2 participants
participants (2)
-
Daniel P. Berrange -
Michal Privoznik