Re: [libvirt] [PATCH] Allow root users to have their own configuration file

On Thu 12 Sep 2013 12:12:22 PM CEST, Daniel P. Berrange wrote: > 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(a)redhat.com&gt; >>>> --- >>>> >>>> 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. > I wasn't discussing that, but it might be possible to differentiate that configs and made it possible to for them to share /etc configs between hosts and make his own Don't get me wrong, I'm not arguing nor I need this to go in, it just (still) makes way more sense to me. Take a look at git for example (or virtually anything) and his /etc/gitconfig => ~/.gitconfig; $XDG_CONFIG_HOME/git/config => $repository/.git/config and so on... I've sent a v2 [1] fixing the test. Martin [1] http://www.redhat.com/archives/libvir-list/2013-September/msg00701.html