On Thu, Sep 12, 2013 at 6:30 AM, Martin Kletzander <mkletzan@xxxxxxxxxx> wrote:
> 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@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.
>>
>
> 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
>
If we're going to move forward with this. We potentially need to do
some other things such as allowing different certificates for a root
user using qemu:///session vs qemu:///system. Otherwise there's a bit
of inconsistency with the rest of the system.
--
Doug Goldstein
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list