[libvirt] [PATCH] Fix virsh {net,pool}-edit
Jim Meyering
jim at meyering.net
Fri Feb 13 15:58:17 UTC 2009
Cole Robinson <crobinso at redhat.com> wrote:
> The dumpxml commands for networks and pools don't support any flag
> arguments, and in fact explictly fail if flags != 0. This is not the
> case for vm dumpxml though, and flags were added to the base 'edit'
> implementation in virsh recently.
>
> The net and pool derivatives were not addressing this difference, so any
> net-edit or pool-edit attempt currently gives an error like:
>
> Network error : invalid argument in virNetworkGetXMLDesc
>
> The attached patch is one way to fix this. Thanks to Charles Duffy for
> the report.
Hi Cole,
That looks good to me.
I confirmed it fixes the bug when using the qemu-based driver:
cat <<EOF > net.xml
<network>
<name>N</name>
<ip address="192.168.199.1" netmask="255.255.255.0"></ip>
</network>
EOF
printf '#!/bin/sh\nperl -pi -e "s/199/19/" "$@"\n' > ed; chmod +x ed
qemud/libvirtd &
EDITOR=$PWD/ed src/virsh 'net-define net.xml; net-edit N'
However, using the test driver, my little test exposed a bug:
EDITOR=$PWD/ed src/virsh -c test:///default 'net-define net.xml; net-edit N'
zsh: segmentation fault
That was because the "def = NULL" assignment (to make it so
the upcoming "free" is a no-op) is too early, since there are
uses of def right after that.
And the same bug appears in 3 other nearby functions.
Here's the fix I'll commit soon, along with tests based on the above.
>From 04bbdb86008744abd5e32cffb0573b873c9fc448 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Fri, 13 Feb 2009 16:49:50 +0100
Subject: [PATCH] test:///default driver: don't dereference NULL "def"
* src/test.c (testNetworkCreate, testNetworkDefine): Don't set
"def" to NULL until after the final use.
(testStoragePoolCreate, testStoragePoolDefine): Likewise.
---
src/test.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/test.c b/src/test.c
index 0e0ec7c..7e4e64d 100644
--- a/src/test.c
+++ b/src/test.c
@@ -2054,9 +2054,9 @@ static virNetworkPtr testNetworkCreate(virConnectPtr conn, const char *xml) {
goto cleanup;
}
net->active = 1;
- def = NULL;
ret = virGetNetwork(conn, def->name, def->uuid);
+ def = NULL;
cleanup:
virNetworkDefFree(def);
@@ -2081,9 +2081,9 @@ static virNetworkPtr testNetworkDefine(virConnectPtr conn, const char *xml) {
goto cleanup;
}
net->persistent = 1;
- def = NULL;
ret = virGetNetwork(conn, def->name, def->uuid);
+ def = NULL;
cleanup:
virNetworkDefFree(def);
@@ -2532,7 +2532,6 @@ testStoragePoolCreate(virConnectPtr conn,
if (!(pool = virStoragePoolObjAssignDef(conn, &privconn->pools, def))) {
goto cleanup;
}
- def = NULL;
if (testStoragePoolObjSetDefaults(conn, pool) == -1) {
virStoragePoolObjRemove(&privconn->pools, pool);
@@ -2542,6 +2541,7 @@ testStoragePoolCreate(virConnectPtr conn,
pool->active = 1;
ret = virGetStoragePool(conn, pool->def->name, pool->def->uuid);
+ def = NULL;
cleanup:
virStoragePoolDefFree(def);
@@ -2571,7 +2571,6 @@ testStoragePoolDefine(virConnectPtr conn,
if (!(pool = virStoragePoolObjAssignDef(conn, &privconn->pools, def))) {
goto cleanup;
}
- def = NULL;
if (testStoragePoolObjSetDefaults(conn, pool) == -1) {
virStoragePoolObjRemove(&privconn->pools, pool);
@@ -2580,6 +2579,7 @@ testStoragePoolDefine(virConnectPtr conn,
}
ret = virGetStoragePool(conn, pool->def->name, pool->def->uuid);
+ def = NULL;
cleanup:
virStoragePoolDefFree(def);
--
1.6.2.rc0.234.g2cc0b3
More information about the libvir-list
mailing list