[libvirt] [PATCH 0/5] Couple of virDomainInterfaceAddresses fixes
While testing the new API I've noticed some leaks and other errors. The very first patch changes the RPC protocol, but hey - on an unreleased procedure yet! Michal Privoznik (5): RPC: Allow HW address in remote_domain_interface struct to be NULL virsh: Adapt to new HW address scenario qemuAgentGetInterfaces: Don't error out on missing HW address cmdDomIfAddr: Free @ip_addr_str qemuGetDHCPInterfaces: Don't leak @network daemon/remote.c | 5 ++++- include/libvirt/libvirt-domain.h | 2 +- src/libvirt-domain.c | 3 ++- src/qemu/qemu_agent.c | 7 ------- src/qemu/qemu_driver.c | 6 +++++- src/remote/remote_driver.c | 3 ++- src/remote/remote_protocol.x | 2 +- src/remote_protocol-structs | 2 +- tools/virsh-domain-monitor.c | 11 +++++++---- 9 files changed, 23 insertions(+), 18 deletions(-) -- 2.0.5
Not all NICs (esp. the virtual ones like TUN) must have a hardware address. Learn our RPC that it's possible. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- daemon/remote.c | 5 ++++- include/libvirt/libvirt-domain.h | 2 +- src/libvirt-domain.c | 3 ++- src/remote/remote_driver.c | 3 ++- src/remote/remote_protocol.x | 2 +- src/remote_protocol-structs | 2 +- 6 files changed, 11 insertions(+), 6 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 1dca64a..b6ea236 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -6525,7 +6525,9 @@ remoteSerializeDomainInterface(virDomainInterfacePtr *ifaces, if ((VIR_STRDUP(iface_ret->name, iface->name)) < 0) goto cleanup; - if ((VIR_STRDUP(iface_ret->hwaddr, iface->hwaddr)) < 0) + if (iface->hwaddr && + (VIR_ALLOC(iface_ret->hwaddr) < 0 || + VIR_STRDUP(*iface_ret->hwaddr, iface->hwaddr) < 0)) goto cleanup; if (iface->naddrs > REMOTE_DOMAIN_IP_ADDR_MAX) { @@ -6561,6 +6563,7 @@ remoteSerializeDomainInterface(virDomainInterfacePtr *ifaces, for (i = 0; i < ifaces_count; i++) { remote_domain_interface *iface_ret = &(ret->ifaces.ifaces_val[i]); VIR_FREE(iface_ret->name); + VIR_FREE(*iface_ret->hwaddr); VIR_FREE(iface_ret->hwaddr); for (j = 0; j < iface_ret->addrs.addrs_len; j++) { remote_domain_ip_addr *ip_addr = diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index a026b9a..79ba3d7 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -3760,7 +3760,7 @@ typedef struct _virDomainInterface virDomainInterface; typedef virDomainInterface *virDomainInterfacePtr; struct _virDomainInterface { char *name; /* interface name */ - char *hwaddr; /* hardware address */ + char *hwaddr; /* hardware address, may be NULL */ unsigned int naddrs; /* number of items in @addrs */ virDomainIPAddressPtr addrs; /* array of IP addresses */ }; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index a3f179d..0bd9274 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -11460,7 +11460,8 @@ virDomainFSInfoFree(virDomainFSInfoPtr info) * ... do something with returned values, for example: * for (i = 0; i < ifaces_count; i++) { * printf("name: %s", ifaces[i]->name); - * printf(" hwaddr: %s", ifaces[i]->hwaddr); + * if (ifaces[i]->hwaddr) + * printf(" hwaddr: %s", ifaces[i]->hwaddr); * * for (j = 0; j < ifaces[i]->naddrs; j++) { * virDomainIPAddressPtr ip_addr = ifaces[i]->addrs + j; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index d89db47..e69f235 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -7963,7 +7963,8 @@ remoteDomainInterfaceAddresses(virDomainPtr dom, if (VIR_STRDUP(iface->name, iface_ret->name) < 0) goto cleanup; - if (VIR_STRDUP(iface->hwaddr, iface_ret->hwaddr) < 0) + if (iface_ret->hwaddr && + VIR_STRDUP(iface->hwaddr, *iface_ret->hwaddr) < 0) goto cleanup; if (iface_ret->addrs.addrs_len > REMOTE_DOMAIN_IP_ADDR_MAX) { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index fe5fcdc..eb862e1 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -3191,7 +3191,7 @@ struct remote_domain_ip_addr { struct remote_domain_interface { remote_nonnull_string name; - remote_nonnull_string hwaddr; + remote_string hwaddr; remote_domain_ip_addr addrs<REMOTE_DOMAIN_IP_ADDR_MAX>; }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 5f4ebff..b3e2e40 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2646,7 +2646,7 @@ struct remote_domain_ip_addr { }; struct remote_domain_interface { remote_nonnull_string name; - remote_nonnull_string hwaddr; + remote_string hwaddr; struct { u_int addrs_len; remote_domain_ip_addr * addrs_val; -- 2.0.5
On Tue, Mar 17, 2015 at 05:52:53PM +0100, Michal Privoznik wrote:
Not all NICs (esp. the virtual ones like TUN) must have a hardware address. Learn our RPC that it's possible.
s/Learn/Teach/
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- daemon/remote.c | 5 ++++- include/libvirt/libvirt-domain.h | 2 +- src/libvirt-domain.c | 3 ++- src/remote/remote_driver.c | 3 ++- src/remote/remote_protocol.x | 2 +- src/remote_protocol-structs | 2 +- 6 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c index 1dca64a..b6ea236 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -6525,7 +6525,9 @@ remoteSerializeDomainInterface(virDomainInterfacePtr *ifaces, if ((VIR_STRDUP(iface_ret->name, iface->name)) < 0) goto cleanup;
- if ((VIR_STRDUP(iface_ret->hwaddr, iface->hwaddr)) < 0) + if (iface->hwaddr && + (VIR_ALLOC(iface_ret->hwaddr) < 0 || + VIR_STRDUP(*iface_ret->hwaddr, iface->hwaddr) < 0)) goto cleanup;
if (iface->naddrs > REMOTE_DOMAIN_IP_ADDR_MAX) { @@ -6561,6 +6563,7 @@ remoteSerializeDomainInterface(virDomainInterfacePtr *ifaces, for (i = 0; i < ifaces_count; i++) { remote_domain_interface *iface_ret = &(ret->ifaces.ifaces_val[i]); VIR_FREE(iface_ret->name); + VIR_FREE(*iface_ret->hwaddr);
This will crash if hwaddr was NULL, and also leak if (iface_ret->hwaddr) VIR_FREE(*iface_ret->hwaddr); VIR_FREE(iface_ret->hwaddr);
VIR_FREE(iface_ret->hwaddr); for (j = 0; j < iface_ret->addrs.addrs_len; j++) { remote_domain_ip_addr *ip_addr = diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index a026b9a..79ba3d7 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -3760,7 +3760,7 @@ typedef struct _virDomainInterface virDomainInterface; typedef virDomainInterface *virDomainInterfacePtr; struct _virDomainInterface { char *name; /* interface name */ - char *hwaddr; /* hardware address */ + char *hwaddr; /* hardware address, may be NULL */ unsigned int naddrs; /* number of items in @addrs */ virDomainIPAddressPtr addrs; /* array of IP addresses */ }; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index a3f179d..0bd9274 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -11460,7 +11460,8 @@ virDomainFSInfoFree(virDomainFSInfoPtr info) * ... do something with returned values, for example: * for (i = 0; i < ifaces_count; i++) { * printf("name: %s", ifaces[i]->name); - * printf(" hwaddr: %s", ifaces[i]->hwaddr); + * if (ifaces[i]->hwaddr) + * printf(" hwaddr: %s", ifaces[i]->hwaddr); * * for (j = 0; j < ifaces[i]->naddrs; j++) { * virDomainIPAddressPtr ip_addr = ifaces[i]->addrs + j; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index d89db47..e69f235 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -7963,7 +7963,8 @@ remoteDomainInterfaceAddresses(virDomainPtr dom, if (VIR_STRDUP(iface->name, iface_ret->name) < 0) goto cleanup;
- if (VIR_STRDUP(iface->hwaddr, iface_ret->hwaddr) < 0) + if (iface_ret->hwaddr && + VIR_STRDUP(iface->hwaddr, *iface_ret->hwaddr) < 0) goto cleanup;
if (iface_ret->addrs.addrs_len > REMOTE_DOMAIN_IP_ADDR_MAX) { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index fe5fcdc..eb862e1 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -3191,7 +3191,7 @@ struct remote_domain_ip_addr {
struct remote_domain_interface { remote_nonnull_string name; - remote_nonnull_string hwaddr; + remote_string hwaddr; remote_domain_ip_addr addrs<REMOTE_DOMAIN_IP_ADDR_MAX>; };
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 5f4ebff..b3e2e40 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2646,7 +2646,7 @@ struct remote_domain_ip_addr { }; struct remote_domain_interface { remote_nonnull_string name; - remote_nonnull_string hwaddr; + remote_string hwaddr; struct { u_int addrs_len; remote_domain_ip_addr * addrs_val;
ACK with the inline fix applied Regards, 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 :|
Make sure we don't print (null) (which in fact is printf()'s cleverness anyway, not ours). If no HW address is present, print "N/A" string just like we do for other fields. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-domain-monitor.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index c9bf156..0717076 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -2278,7 +2278,8 @@ cmdDomIfAddr(vshControl *ctl, const vshCmd *cmd) /* When the interface has no IP address */ if (!iface->naddrs) { vshPrintExtra(ctl, " %-10s %-17s %-12s %s\n", - iface->name, iface->hwaddr, "N/A", "N/A"); + iface->name, + iface->hwaddr ? iface->hwaddr : "N/A", "N/A", "N/A"); continue; } @@ -2312,7 +2313,8 @@ cmdDomIfAddr(vshControl *ctl, const vshCmd *cmd) /* Don't repeat interface name */ if (full || !j) vshPrintExtra(ctl, " %-10s %-17s %s\n", - iface->name, iface->hwaddr, ip_addr_str); + iface->name, + iface->hwaddr ? iface->hwaddr : "", ip_addr_str); else vshPrintExtra(ctl, " %-10s %-17s %s\n", "-", "-", ip_addr_str); -- 2.0.5
On Tue, Mar 17, 2015 at 05:52:54PM +0100, Michal Privoznik wrote:
Make sure we don't print (null) (which in fact is printf()'s cleverness anyway, not ours). If no HW address is present, print "N/A" string just like we do for other fields.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-domain-monitor.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index c9bf156..0717076 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -2278,7 +2278,8 @@ cmdDomIfAddr(vshControl *ctl, const vshCmd *cmd) /* When the interface has no IP address */ if (!iface->naddrs) { vshPrintExtra(ctl, " %-10s %-17s %-12s %s\n", - iface->name, iface->hwaddr, "N/A", "N/A"); + iface->name, + iface->hwaddr ? iface->hwaddr : "N/A", "N/A", "N/A"); continue; }
@@ -2312,7 +2313,8 @@ cmdDomIfAddr(vshControl *ctl, const vshCmd *cmd) /* Don't repeat interface name */ if (full || !j) vshPrintExtra(ctl, " %-10s %-17s %s\n", - iface->name, iface->hwaddr, ip_addr_str); + iface->name, + iface->hwaddr ? iface->hwaddr : "", ip_addr_str); else vshPrintExtra(ctl, " %-10s %-17s %s\n", "-", "-", ip_addr_str);
ACK Regards, 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 :|
Now that we allow HW address to be not present on our RPC layer, don't error out if qemu-ga hasn't provided any. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_agent.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 5f90b15..a7b3279 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -2054,13 +2054,6 @@ qemuAgentGetInterfaces(qemuAgentPtr mon, goto error; hwaddr = virJSONValueObjectGetString(tmp_iface, "hardware-address"); - if (!hwaddr) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("qemu agent didn't provide" - " 'hardware-address' field")); - goto error; - } - if (VIR_STRDUP(iface->hwaddr, hwaddr) < 0) goto error; } -- 2.0.5
On Tue, Mar 17, 2015 at 05:52:55PM +0100, Michal Privoznik wrote:
Now that we allow HW address to be not present on our RPC layer, don't error out if qemu-ga hasn't provided any.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_agent.c | 7 ------- 1 file changed, 7 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 5f90b15..a7b3279 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -2054,13 +2054,6 @@ qemuAgentGetInterfaces(qemuAgentPtr mon, goto error;
hwaddr = virJSONValueObjectGetString(tmp_iface, "hardware-address"); - if (!hwaddr) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("qemu agent didn't provide" - " 'hardware-address' field")); - goto error; - }
Why would QEMU guest agent not provide the MAC address ? That sounds like it would be a bug if that ever happened surely. Regards, 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 :|
On 17.03.2015 17:58, Daniel P. Berrange wrote:
On Tue, Mar 17, 2015 at 05:52:55PM +0100, Michal Privoznik wrote:
Now that we allow HW address to be not present on our RPC layer, don't error out if qemu-ga hasn't provided any.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_agent.c | 7 ------- 1 file changed, 7 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 5f90b15..a7b3279 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -2054,13 +2054,6 @@ qemuAgentGetInterfaces(qemuAgentPtr mon, goto error;
hwaddr = virJSONValueObjectGetString(tmp_iface, "hardware-address"); - if (!hwaddr) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("qemu agent didn't provide" - " 'hardware-address' field")); - goto error; - }
Why would QEMU guest agent not provide the MAC address ? That sounds like it would be a bug if that ever happened surely.
Not every network interface has a HW address, e.g. PTP devices don't have one. Therefore, qemu-ga doesn't return any if that's the case. Michal
On 17.03.2015 18:00, Michal Privoznik wrote:
On 17.03.2015 17:58, Daniel P. Berrange wrote:
On Tue, Mar 17, 2015 at 05:52:55PM +0100, Michal Privoznik wrote:
Now that we allow HW address to be not present on our RPC layer, don't error out if qemu-ga hasn't provided any.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_agent.c | 7 ------- 1 file changed, 7 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 5f90b15..a7b3279 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -2054,13 +2054,6 @@ qemuAgentGetInterfaces(qemuAgentPtr mon, goto error;
hwaddr = virJSONValueObjectGetString(tmp_iface, "hardware-address"); - if (!hwaddr) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("qemu agent didn't provide" - " 'hardware-address' field")); - goto error; - }
Why would QEMU guest agent not provide the MAC address ? That sounds like it would be a bug if that ever happened surely.
Not every network interface has a HW address, e.g. PTP devices don't have one. Therefore, qemu-ga doesn't return any if that's the case.
This is from qga/qapi-schema.json: ## # @GuestNetworkInterface: # # @name: The name of interface for which info are being delivered # # @hardware-address: Hardware address of @name # # @ip-addresses: List of addresses assigned to @name # # Since: 1.1 ## { 'type': 'GuestNetworkInterface', 'data': {'name': 'str', '*hardware-address': 'str', '*ip-addresses': ['GuestIpAddress'] } } So the hardware address field is optional. Michal
On Tue, Mar 17, 2015 at 06:09:17PM +0100, Michal Privoznik wrote:
On 17.03.2015 18:00, Michal Privoznik wrote:
On 17.03.2015 17:58, Daniel P. Berrange wrote:
On Tue, Mar 17, 2015 at 05:52:55PM +0100, Michal Privoznik wrote:
Now that we allow HW address to be not present on our RPC layer, don't error out if qemu-ga hasn't provided any.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_agent.c | 7 ------- 1 file changed, 7 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 5f90b15..a7b3279 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -2054,13 +2054,6 @@ qemuAgentGetInterfaces(qemuAgentPtr mon, goto error;
hwaddr = virJSONValueObjectGetString(tmp_iface, "hardware-address"); - if (!hwaddr) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("qemu agent didn't provide" - " 'hardware-address' field")); - goto error; - }
Why would QEMU guest agent not provide the MAC address ? That sounds like it would be a bug if that ever happened surely.
Not every network interface has a HW address, e.g. PTP devices don't have one. Therefore, qemu-ga doesn't return any if that's the case.
This is from qga/qapi-schema.json: ## # @GuestNetworkInterface: # # @name: The name of interface for which info are being delivered # # @hardware-address: Hardware address of @name # # @ip-addresses: List of addresses assigned to @name # # Since: 1.1 ## { 'type': 'GuestNetworkInterface', 'data': {'name': 'str', '*hardware-address': 'str', '*ip-addresses': ['GuestIpAddress'] } }
So the hardware address field is optional.
Ah, ok then. Regards, 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 :|
On Tue, Mar 17, 2015 at 05:52:55PM +0100, Michal Privoznik wrote:
Now that we allow HW address to be not present on our RPC layer, don't error out if qemu-ga hasn't provided any.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_agent.c | 7 ------- 1 file changed, 7 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 5f90b15..a7b3279 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -2054,13 +2054,6 @@ qemuAgentGetInterfaces(qemuAgentPtr mon, goto error;
hwaddr = virJSONValueObjectGetString(tmp_iface, "hardware-address"); - if (!hwaddr) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("qemu agent didn't provide" - " 'hardware-address' field")); - goto error; - } -
ACK Regards, 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 :|
The variable holds formatted suffix to each line printed out (address type, address and prefix). However, the variable is never freed. At the same time, honour fact, that data held in the variable is not constant. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-domain-monitor.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index 0717076..0eb1b62 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -2269,7 +2269,7 @@ cmdDomIfAddr(vshControl *ctl, const vshCmd *cmd) for (i = 0; i < ifaces_count; i++) { virDomainInterfacePtr iface = ifaces[i]; - const char *ip_addr_str = NULL; + char *ip_addr_str = NULL; const char *type = NULL; if (interface && STRNEQ(interface, iface->name)) @@ -2308,7 +2308,7 @@ cmdDomIfAddr(vshControl *ctl, const vshCmd *cmd) ip_addr_str = virBufferContentAndReset(&buf); if (!ip_addr_str) - ip_addr_str = ""; + ip_addr_str = vshStrdup(ctl, ""); /* Don't repeat interface name */ if (full || !j) @@ -2320,6 +2320,7 @@ cmdDomIfAddr(vshControl *ctl, const vshCmd *cmd) "-", "-", ip_addr_str); virBufferFreeAndReset(&buf); + VIR_FREE(ip_addr_str); } } -- 2.0.5
On Tue, Mar 17, 2015 at 05:52:56PM +0100, Michal Privoznik wrote:
The variable holds formatted suffix to each line printed out (address type, address and prefix). However, the variable is never freed. At the same time, honour fact, that data held in the variable is not constant.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-domain-monitor.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
ACK Regards, 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 :|
The function needs a pointer to the network to get list of DHCP leases. The pointer is obtained via virNetworkLookupByName() which requires callers to free the returned network once no longer needed. Otherwise it's leaked. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 77f51d9..555c140 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19652,7 +19652,7 @@ qemuGetDHCPInterfaces(virDomainPtr dom, int n_leases = 0; size_t i, j; size_t ifaces_count = 0; - virNetworkPtr network; + virNetworkPtr network = NULL; char macaddr[VIR_MAC_STRING_BUFLEN]; virDomainInterfacePtr iface = NULL; virNetworkDHCPLeasePtr *leases = NULL; @@ -19670,6 +19670,8 @@ qemuGetDHCPInterfaces(virDomainPtr dom, continue; virMacAddrFormat(&(vm->def->nets[i]->mac), macaddr); + if (network) + virNetworkFree(network); network = virNetworkLookupByName(dom->conn, vm->def->nets[i]->data.network.name); @@ -19720,6 +19722,8 @@ qemuGetDHCPInterfaces(virDomainPtr dom, rv = ifaces_count; cleanup: + if (network) + virNetworkFree(network); if (leases) { for (i = 0; i < n_leases; i++) virNetworkDHCPLeaseFree(leases[i]); -- 2.0.5
On Tue, Mar 17, 2015 at 05:52:57PM +0100, Michal Privoznik wrote:
The function needs a pointer to the network to get list of DHCP leases. The pointer is obtained via virNetworkLookupByName() which requires callers to free the returned network once no longer needed. Otherwise it's leaked.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
ACK Regards, 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 :|
participants (2)
-
Daniel P. Berrange -
Michal Privoznik