[libvirt] [PATCH 3/5] qemu_conf: add new configuration key bridge_helper

[Daniel P. Berrange]

On Thu, Apr 18, 2013 at 01:35:51PM -0400, Laine Stump wrote:
> On 03/25/2013 10:25 AM, Paolo Bonzini wrote:
> > Signed-off-by: Paolo Bonzini <pbonzini at redhat.com>
> > ---
> >  src/qemu/libvirtd_qemu.aug         | 1 +
> >  src/qemu/qemu.conf                 | 8 ++++++++
> >  src/qemu/qemu_conf.c               | 3 +++
> >  src/qemu/qemu_conf.h               | 1 +
> >  src/qemu/test_libvirtd_qemu.aug.in | 1 +
> >  5 files changed, 14 insertions(+)
> >
> > diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
> > index 91f5f77..61740a9 100644
> > --- a/src/qemu/libvirtd_qemu.aug
> > +++ b/src/qemu/libvirtd_qemu.aug
> > @@ -60,6 +60,7 @@ module Libvirtd_qemu =
> >  
> >     let process_entry = str_entry "hugetlbfs_mount"
> >                   | bool_entry "clear_emulator_capabilities"
> > +                 | str_entry "bridge_helper"
> >                   | bool_entry "set_process_name"
> >                   | int_entry "max_processes"
> >                   | int_entry "max_files"
> > diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
> > index dd853c8..87bdf70 100644
> > --- a/src/qemu/qemu.conf
> > +++ b/src/qemu/qemu.conf
> > @@ -301,6 +301,14 @@
> >  #hugetlbfs_mount = "/dev/hugepages"
> >  
> >  
> > +# Path to the setuid helper for creating tap devices.  This executable
> > +# is used to create <source type='bridge'> interfaces when libvirtd is
> > +# running unprivileged.  libvirt invokes the helper directly, instead
> > +# of using "-netdev bridge", for security reasons.
> > +#bridge_helper = "/usr/libexec/qemu-bridge-helper"
> > +
> > +
> 
> Are we sure we want to allow this to be configured? That could lead to
> some "interesting" troubleshooting incidents :-)
> 
> On the other hand, I guess the path to qemu itself is right there in the
> domain config file, so how much worse could this be...
> 
> 
> > +
> >  # If clear_emulator_capabilities is enabled, libvirt will drop all
> >  # privileged capabilities of the QEmu/KVM emulator. This is enabled by
> >  # default.
> > diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> > index c2e2e10..f648fc3 100644
> > --- a/src/qemu/qemu_conf.c
> > +++ b/src/qemu/qemu_conf.c
> > @@ -248,6 +248,7 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)
> >          }
> >      }
> >  #endif
> > +    cfg->bridgeHelperName = strdup("/usr/libexec/qemu-bridge-helper");
> >  
> >      cfg->clearEmulatorCapabilities = true;
> >  
> > @@ -297,6 +298,7 @@ static void virQEMUDriverConfigDispose(void *obj)
> >  
> >      VIR_FREE(cfg->hugetlbfsMount);
> >      VIR_FREE(cfg->hugepagePath);
> > +    VIR_FREE(cfg->bridgeHelperName);
> >  
> >      VIR_FREE(cfg->saveImageFormat);
> >      VIR_FREE(cfg->dumpImageFormat);
> > @@ -509,6 +511,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
> >      GET_VALUE_BOOL("auto_start_bypass_cache", cfg->autoStartBypassCache);
> >  
> >      GET_VALUE_STR("hugetlbfs_mount", cfg->hugetlbfsMount);
> > +    GET_VALUE_STR("bridge_helper", cfg->bridgeHelperName);
> >  
> >      GET_VALUE_BOOL("mac_filter", cfg->macFilter);
> >  
> > diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
> > index c5ddaad..666ac33 100644
> > --- a/src/qemu/qemu_conf.h
> > +++ b/src/qemu/qemu_conf.h
> > @@ -117,6 +117,7 @@ struct _virQEMUDriverConfig {
> >  
> >      char *hugetlbfsMount;
> >      char *hugepagePath;
> > +    char *bridgeHelperName;
> >  
> >      bool macFilter;
> >  
> > diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in
> > index 2892204..0aec997 100644
> > --- a/src/qemu/test_libvirtd_qemu.aug.in
> > +++ b/src/qemu/test_libvirtd_qemu.aug.in
> > @@ -49,6 +49,7 @@ module Test_libvirtd_qemu =
> >  { "auto_dump_bypass_cache" = "0" }
> >  { "auto_start_bypass_cache" = "0" }
> >  { "hugetlbfs_mount" = "/dev/hugepages" }
> > +{ "bridge_helper" = "/usr/libexec/qemu-bridge-helper" }
> >  { "clear_emulator_capabilities" = "1" }
> >  { "set_process_name" = "1" }
> >  { "max_processes" = "0" }
> 
> ACK. (But I'd like at least one other ACK from someone else due to the
> fact that this is polluting the config namespace with something we would
> like to eventually eliminate.)

We have to support this really, since we need to cope with people
installing into non-/usr locations

ACK

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 :|