[libvirt] [PATCH] Prevent initializing ebtables if disabled in qemu.conf
* src/qemu/qemu_conf.c: don't initialize ebtables if disabled --- src/qemu/qemu_conf.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 28567b2..5f492dd 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -322,7 +322,7 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, p = virConfGetValue (conf, "mac_filter"); CHECK_TYPE ("mac_filter", VIR_CONF_LONG); - if (p) { + if (p && p->l) { driver->macFilter = p->l; if (!(driver->ebtables = ebtablesContextNew("qemu"))) { driver->macFilter = 0; -- 1.6.2.5
On 11/10/2009 06:32 PM, Ryota Ozaki wrote:
* src/qemu/qemu_conf.c: don't initialize ebtables if disabled --- src/qemu/qemu_conf.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 28567b2..5f492dd 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -322,7 +322,7 @@ int qemudLoadDriverConfig(struct qemud_driver *driver,
p = virConfGetValue (conf, "mac_filter"); CHECK_TYPE ("mac_filter", VIR_CONF_LONG); - if (p) { + if (p && p->l) { driver->macFilter = p->l; if (!(driver->ebtables = ebtablesContextNew("qemu"))) { driver->macFilter = 0;
ACK Also, hijacking this patch a bit to point out a few small issues I noticed with the ebtables code (cc-ing Gerhard) The mac_filter value in qemu.conf isn't documented and isn't commented out by default, unlike the other values in the conf file. I think it would be good to match existing convention. Even though we check for the ebtables binary in configure, we don't disable the driver if the binary doesn't exist (like it didn't on my rawhide box). This causes it to print lots of error messages on libvirtd startup. It would be nice to find a way to just VIR_DEBUG that the binary wasn't found and skip initializing the driver. Thanks, Cole
Cole Robinson <crobinso@redhat.com> wrote on 11/11/2009 02:34:17 AM:
Also, hijacking this patch a bit to point out a few small issues I noticed with the ebtables code (cc-ing Gerhard)
Apologies for any inconviences...
The mac_filter value in qemu.conf isn't documented and isn't commented
out by
default, unlike the other values in the conf file. I think it would be good to match existing convention.
ok. I will fix this with a separate patch.
Even though we check for the ebtables binary in configure, we don't
disable
the driver if the binary doesn't exist (like it didn't on my rawhide box). This causes it to print lots of error messages on libvirtd startup. It would be nice to find a way to just VIR_DEBUG that the binary wasn't found and skip initializing the driver.
ok. I will look into this.
Thanks, Cole
Best regards, Gerhard Stenzel, Linux on Cell/Hybrid Technologies, LTC ----------------------------------------------------------------------------------------------------------------------------------- IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martin Jetter | Geschäftsführung: Erich Baier Sitz der Gesellschaft: Böblingen | Registergericht: Amtsgericht Stuttgart, HRB 243294
This patch disables the mac_filter config switch by default to match existing convention. Signed-off-by: Gerhard Stenzel <gerhard.stenzel@de.ibm.com> Index: libvirt/src/qemu/qemu.conf =================================================================== --- libvirt.orig/src/qemu/qemu.conf +++ libvirt/src/qemu/qemu.conf @@ -153,4 +153,7 @@ # hugetlbfs_mount = "/dev/hugepages" -mac_filter = 1 +# mac_filter enables MAC addressed based filtering on bridge ports. +# This currently requires ebtables to be installed. +# +# mac_filter = 1 -- Best regards, Gerhard Stenzel, ----------------------------------------------------------------------------------------------------------------------------------- IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martin Jetter Geschäftsführung: Erich Baier Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
On Wed, Nov 11, 2009 at 10:43:22AM +0100, Gerhard Stenzel wrote:
This patch disables the mac_filter config switch by default to match existing convention.
Signed-off-by: Gerhard Stenzel <gerhard.stenzel@de.ibm.com>
Index: libvirt/src/qemu/qemu.conf =================================================================== --- libvirt.orig/src/qemu/qemu.conf +++ libvirt/src/qemu/qemu.conf @@ -153,4 +153,7 @@
# hugetlbfs_mount = "/dev/hugepages"
-mac_filter = 1 +# mac_filter enables MAC addressed based filtering on bridge ports. +# This currently requires ebtables to be installed. +# +# mac_filter = 1
Okay, makes sense and documenting the option is important, applied, I just removed the extra end of line space, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Tue, Nov 10, 2009 at 08:34:17PM -0500, Cole Robinson wrote:
On 11/10/2009 06:32 PM, Ryota Ozaki wrote:
* src/qemu/qemu_conf.c: don't initialize ebtables if disabled --- src/qemu/qemu_conf.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 28567b2..5f492dd 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -322,7 +322,7 @@ int qemudLoadDriverConfig(struct qemud_driver *driver,
p = virConfGetValue (conf, "mac_filter"); CHECK_TYPE ("mac_filter", VIR_CONF_LONG); - if (p) { + if (p && p->l) { driver->macFilter = p->l; if (!(driver->ebtables = ebtablesContextNew("qemu"))) { driver->macFilter = 0;
ACK
I concur, applied :-), thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (5)
-
Cole Robinson -
Daniel Veillard -
Gerhard Stenzel -
Gerhard Stenzel -
Ryota Ozaki