On Tue, Feb 16, 2010 at 02:37:01PM -0500, Cole Robinson wrote:

We were accessing the wrong private data structure, which would cause a segfault.

Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/network/bridge_driver.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 8cef714..4453707 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1188,7 +1188,7 @@ static int networkListDefinedNetworks(virConnectPtr conn, char **const names, in

static int networkIsActive(virNetworkPtr net) { - struct network_driver *driver = net->conn->privateData; + struct network_driver *driver = net->conn->networkPrivateData; virNetworkObjPtr obj; int ret = -1;

@@ -1209,7 +1209,7 @@ cleanup:

static int networkIsPersistent(virNetworkPtr net) { - struct network_driver *driver = net->conn->privateData; + struct network_driver *driver = net->conn->networkPrivateData; virNetworkObjPtr obj; int ret = -1;

-- ACK

Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On Tue, Feb 16, 2010 at 02:37:02PM -0500, Cole Robinson wrote:

Currently we just error with ex. 'virbr0: No such device'.

Since we are using public API calls here, we need to ensure that any raised error is properly saved and restored, since API entry points always reset messages.

Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/qemu/qemu_conf.c | 28 +++++++++++++++++++++++++--- 1 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index c9fe55b..6c64deb 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1482,17 +1482,39 @@ qemudNetworkIfaceConnect(virConnectPtr conn, int template_ifname = 0;

if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) { + int active, fail = 0; + virErrorPtr errobj; virNetworkPtr network = virNetworkLookupByName(conn, - net->data.network.name); + net->data.network.name); if (!network) return -1;

- brname = virNetworkGetBridgeName(network); + active = virNetworkIsActive(network); + if (active != 1) { + fail = 1;

+ if (active == 0) + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Network '%s' is not active."), + net->data.network.name); + } + + if (!fail) { + brname = virNetworkGetBridgeName(network); + if (brname == NULL) + fail = 1; + } + + /* Make sure any above failure is preserved */ + errobj = virSaveLastError(); virNetworkFree(network); + virSetError(errobj); + virFreeError(errobj);

- if (brname == NULL) + errobj = virSaveLastError(); + if (fail) return -1; + } else if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) { if (!(brname = strdup(net->data.bridge.brname))) { virReportOOMError(); --

ACK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On 02/18/2010 07:45 AM, Daniel P. Berrange wrote:

On Tue, Feb 16, 2010 at 02:37:00PM -0500, Cole Robinson wrote:

...

virGetLastError returns NULL if no error has been set, not on allocation error like virSetError assumed. Use virLastErrorObject instead. This fixes virSetError when no error is currently stored.

Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/util/virterror.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/util/virterror.c b/src/util/virterror.c index bbf5021..cbd0ca8 100644 --- a/src/util/virterror.c +++ b/src/util/virterror.c @@ -304,7 +304,7 @@ int virSetError(virErrorPtr newerr) { virErrorPtr err; - err = virGetLastError(); + err = virLastErrorObject(); if (!err) return -1;

--

ACK

Daniel

Thanks, I've pushed these 3 now. - Cole