[libvirt] [PATCH] Default to qemu:///system if accessible

[Daniel P. Berrange]

On Fri, Sep 03, 2010 at 11:04:21PM +0200, Soren Hansen wrote:
> On 03-09-2010 15:59, Daniel Veillard wrote:
> >> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
> >> index a945710..17e6ead 100644
> >> --- a/src/remote/remote_driver.c
> >> +++ b/src/remote/remote_driver.c
> >> @@ -1086,7 +1086,7 @@ remoteOpen (virConnectPtr conn,
> >>      if (!conn->uri) {
> >>          DEBUG0("Auto-probe remote URI");
> >>  #ifndef __sun
> >> -        if (getuid() > 0) {
> >> +        if (access(LIBVIRTD_PRIV_UNIX_SOCKET, W_OK)) {
> >>              DEBUG0("Auto-spawn user daemon instance");
> >>              rflags |= VIR_DRV_OPEN_REMOTE_USER;
> >>              if (!autostart ||
> > 
> > Hum, that sounds like a change of semantic.
> 
> Depends. I think of the (getuid() > 0) as a check for whether you can
> access the privileged UNIX socket that simply doesn't take into account
> that users other than root may have been given this privilege by way of
> membership of the group owning the socket.
> 
> > Before as non root you would span a local daemon, now you use the system
> > one. I'm not saying it's a bad thing in most cases but it's a serious
> > change, and we try to avoid those in general.
> 
> As I suggested in another e-mail a short while ago in this thread, I'm
> making this change to maintain the status quo in Ubuntu. We already do
> this in virsh and virt-viewer and something very, very similar in
> virt-manager. Someone recently dropped the virt-viewer patch by mistake,
> and I decided to make the change directly in libvirt to not change how
> libvirt has behaved for us in the past couple of years, so I can
> certainly relate to your desire to not change the behaviour.
> 
> Personally, at least, I find this behaviour much more pleasant. On all
> my servers, I just grant whoever needs to be able to deal with the VM's
> on it access to the socket (by adding them to the libvirtd group), and
> that's it. I don't need to instruct them to set per-application
> environment variables or whatever.
> 
> Generally, I use libvirt on two classes of machine: Workstations/laptops
> (where I'm functionally the only user), or on servers whose job is to
> run virtual machines and nothing else.
> 
> In the former case, obviously I want to use the systemwide libvirtd to
> get access to all the fancy networking things. I have absolutely no
> motivation to use qemu:///session on there anymore.

Our goal is to improve qemu://session's networking such that this isn't
a reason to use qemu://system anymore. Enabing use of qemu://session
is key to solving a number of important security problems, not least
that a user should be able to just keep the disk & iso images in their
home directory and not have to worry about their ownership/permissions.
In addition by running VMs directly under the user's session things
like pulseaudio, SDL can directly access the X & GNOME sessions without
further special config.

> In the latter case, only people who are supposed to manage the virtual
> machines will have access to the server, and IMO they have no business
> running stuff under qemu:///session on that box.

This is ignoring two important use cases which are common in the
corporate world. Shared development servers where many users are
on one server, and personal workstations where the users are not
allowed to have root.

The thing about heuristics is that they're never correct for everyone.
Your patch is making it more correct for one group of people, and less
correct for a different group of people. Further making a significant
functional change to libvirt that will break things for people that are
relying on the existing behaviour.

If someone wants to save typing they need merely set LIBVIRT_CONNECT_URI
to whatever they want and thus avoid the default connection logic completely.

Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|