2:18 p.m.
These area a followup to Luyao Huang's patch to fix incorrect
reporting from virsh domdisplay:
https://www.redhat.com/archives/libvir-list/2015-February/msg00298.html
While his patch works when the remote libvirtd is 0.9.4 or greater, it
won't work when talking to older libvirt. Also the code as written
isn't what was intended. Patch 1 takes care of that. Patch 2 fixes two
theoretical memory leaks (which never happen in real life, but could
some day if the functions are used differently).
Laine Stump (2):
domain: backfill listen address to parent <graphics> listen attribute
domain: avoid potential memory leak in virDomainGraphicsListenSet*()
src/conf/domain_conf.c | 30 ++++++++++++++++++------------
1 file changed, 18 insertions(+), 12 deletions(-)
--
2.1.0
2:18 p.m.
New subject: [libvirt] [PATCH 1/2] domain: backfill listen address to parent <graphics> listen attribute
Prior to 0.9.4, libvirt only supported a single listen, and it had to
be an IP address:
<graphics listen='1.2.3.4' ..../>
Starting with 0.9.4, a graphics element could have a <listen>
subelement (actually the grammar supports multiples, but all of the
drivers only support a single <listen> per <graphics>), and that
listen element can be of type='address' or type='network'. For
type='address', <listen> also has an attribute called 'address'
which
contains the IP address for listening:
<graphics ....>
<listen type='address' address='1.2.3.4' .../>
</graphics>
type can also be "network", and in that case listen will have a
"network" attribute which will contain the name of a libvirt
network:
<graphics ....>
<listen type='network' network='testnet' .../>
</graphics>
At domain start (or migrate) time, libvirt will attempt to
find an IP address associated with that network (e.g. the IP address
of the bridge device used by the network, or the physical device
listed in <forward dev='physdev'/>) and fill in that address in the
status XML:
<graphics ....>
<listen type='network' network='testnet'
address='1.2.3.4' .../>
</graphics>
In the case that a <graphics> element has a <listen> subelement of
type='address', that listen subelement's "address" attribute is
backfilled into the parent graphics element's "listen" *attribute* for
backward compatibility (so that a management application unaware of
the separate <listen> element can still learn the listen
address). This backfill should be done with the IP learned from
type='network' as well, and that's what this patch does:
<graphics listen='1.2.3.4' ....>
<listen type='network' network='testnet'
address='1.2.3.4' .../>
</graphics>
This is a continuation of the fix for:
https://bugzilla.redhat.com/show_bug.cgi?id=1191016
---
src/conf/domain_conf.c | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6a57d80..6614fb1 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -18906,17 +18906,23 @@ virDomainGraphicsDefFormat(virBufferPtr buf,
return -1;
}
- /* find the first listen element of type='address' and duplicate
- * its address attribute as the listen attribute of
- * <graphics>. This is done to improve backward compatibility. */
+ /* find the first listen subelement with a valid address and
+ * duplicate its address attribute as the listen attribute of
+ * <graphics>. This is done to improve backward compatibility.
+ */
for (i = 0; i < def->nListens; i++) {
- if (virDomainGraphicsListenGetType(def, i)
- == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS) {
- if (flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE &&
- def->listens[i].fromConfig)
- continue;
- listenAddr = virDomainGraphicsListenGetAddress(def, i);
- break;
+ virDomainGraphicsListenType listenType;
+
+ if (flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE &&
+ def->listens[i].fromConfig)
+ continue;
+ listenType = virDomainGraphicsListenGetType(def, i);
+
+ if (listenType == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS ||
+ (listenType == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK &&
+ !(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE))) {
+ if ((listenAddr = virDomainGraphicsListenGetAddress(def, i)))
+ break;
}
}
--
2.1.0
2:18 p.m.
New subject: [libvirt] [PATCH 2/2] domain: avoid potential memory leak in virDomainGraphicsListenSet*()
virDomainGraphicsListenSetAddress() and
virDomainGraphicsListenSetNetwork() both set their respective char* to
NULL directly when asked to set it to NULL, which is okay as long as
it's already set to NULL. If these functions are ever called to clear
a listen object that has a valid string in address or network, it will
end up leaking the old value. Currently that doesn't happen, so this
is just a preemptive strike.
---
src/conf/domain_conf.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6614fb1..8600eff 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -21440,7 +21440,7 @@ virDomainGraphicsListenSetAddress(virDomainGraphicsDefPtr def,
listenInfo->type = VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS;
if (!address) {
- listenInfo->address = NULL;
+ VIR_FREE(listenInfo->address);
return 0;
}
@@ -21478,7 +21478,7 @@ virDomainGraphicsListenSetNetwork(virDomainGraphicsDefPtr def,
listenInfo->type = VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK;
if (!network) {
- listenInfo->network = NULL;
+ VIR_FREE(listenInfo->network);
return 0;
}
--
2.1.0
3:47 a.m.
On Tue, Feb 10, 2015 at 03:18:22PM -0500, Laine Stump wrote:
These area a followup to Luyao Huang's patch to fix incorrect
reporting from virsh domdisplay:
https://www.redhat.com/archives/libvir-list/2015-February/msg00298.html
While his patch works when the remote libvirtd is 0.9.4 or greater, it
won't work when talking to older libvirt. Also the code as written
isn't what was intended. Patch 1 takes care of that. Patch 2 fixes two
theoretical memory leaks (which never happen in real life, but could
some day if the functions are used differently).
Laine Stump (2):
domain: backfill listen address to parent <graphics> listen attribute
domain: avoid potential memory leak in virDomainGraphicsListenSet*()
src/conf/domain_conf.c | 30 ++++++++++++++++++------------
1 file changed, 18 insertions(+), 12 deletions(-)
ACK series
Jan
3605
days inactive
3606
days old
3 comments
2 participants
participants (2)
-
Ján Tomko -
Laine Stump