On Thu, Sep 12, 2013 at 12:07:54PM +0200, Martin Kletzander wrote: > On 09/12/2013 12:00 PM, Daniel P. Berrange wrote: > > On Thu, Sep 12, 2013 at 11:53:32AM +0200, Martin Kletzander wrote: > >> Currently, we have two configuration file paths, one global (where > >> "global" means root-only and we're probably not changing this in near > >> future) and one per-user. Unfortunately root user cannot use the > >> second option because until now we were choosing the file path > >> depending only on whether the user is root or not. > >> > >> This patch modifies the mentioned behavior for root only, allowing him > >> to set his own configuration files without changing anything in > >> system-wide configuration folders. > >> > >> This also makes the virsh-uriprecedence test pass its first test case > >> when ran as root. > >> > >> Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx> > >> --- > >> > >> Notes: > >> I'm playing along previously mentioned "proper behavior" in this > >> patch. However, IMNSHO, our "global" or "system-wide" configuration > >> file (defaulting to '/etc/libvirt/libvirt.conf') should be accessible > >> for all users since this has no security impact (security information > >> may be in files 'libvirtd.conf' or 'qemu.conf'). This file should be > >> also read and used for all users. After that, settings in user > >> configuration file (defaulting to '~/.config/libvirt/libvirt.conf') > >> may override some of these settings for that user. > >> > >> This is how all sensible configurations are loaded and that's also > >> what I'd prefer. Unfortunately some developers feels this should be > >> done in completely different way. > >> > >> src/libvirt.c | 56 ++++++++++++++++++++++++++++++++++++-------------------- > >> 1 file changed, 36 insertions(+), 20 deletions(-) > > > > NACK to this. The root user already has their own dedicated configuration > > file, /etc/libvirt/libvirt.conf. The /etc/libvirt directory permissions > > prevent *any* file there being read by non-root, so the /etc/libvirt/libvirt.conf > > file could not be used by non-root. > > > > As mentioned in the commit message, this patch doesn't change the > behavior for non-root users, that's only a nag in the notes. > > The only thing that changes after applying this patch is that *iff* ran > as root, we'll *also* check "${XDG_CONFIG_HOME}/libvirt/libvirt.conf". I don't see any point in doing that at all. Root already has a config file exclusively for their own use. They don't need 2. 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list