[libvirt] [PATCH] admin: Fix the default uri for session daemon to libvirtd:///session

Just like we decide on which URI we go with based on EUID for qemu in remote driver, do a similar thing for admin except we do not spawn a daemon in this case. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1356858 Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/libvirt-admin.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c index 4bf29b1..4552e84 100644 --- a/src/libvirt-admin.c +++ b/src/libvirt-admin.c @@ -176,10 +176,14 @@ virAdmGetDefaultURI(virConfPtr conf, char **uristr) /* Since we can't probe connecting via any hypervisor driver as libvirt * does, if no explicit URI was given and neither the environment * variable, nor the configuration parameter had previously been set, - * we set the default admin server URI to 'libvirtd://system'. + * we set the default admin server URI to 'libvirtd:///system' or + * 'libvirtd:///session' depending on the process's EUID. */ - if (VIR_STRDUP(*uristr, "libvirtd:///system") < 0) - return -1; + if (geteuid() == 0 && + VIR_STRDUP(*uristr, "libvirtd:///system") < 0) + return -1; + else if (VIR_STRDUP(*uristr, "libvirtd:///session") < 0) + return -1; } } -- 2.5.5

On Fri, Jul 29, 2016 at 02:06:00PM +0200, Erik Skultety wrote:
Just like we decide on which URI we go with based on EUID for qemu in remote driver, do a similar thing for admin except we do not spawn a daemon in this case.
IMHO we should auto-spawn the daemon too, so we have consistent behaviour virsh virsh. Regards, 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 :|

On 29.07.2016 14:08, Daniel P. Berrange wrote:
On Fri, Jul 29, 2016 at 02:06:00PM +0200, Erik Skultety wrote:
Just like we decide on which URI we go with based on EUID for qemu in remote driver, do a similar thing for admin except we do not spawn a daemon in this case.
IMHO we should auto-spawn the daemon too, so we have consistent behaviour virsh virsh.
I don't think this is a good idea. The session daemon dies after 30 seconds after last connection was closed. There's no point in setting something via admin API just to find it back on defaults after 1 minute because new daemon has been spawned. It's not like our domain APIs where libvirt refreshes the internal state of drivers upon their init. Michal

On Fri, Jul 29, 2016 at 03:40:28PM +0200, Michal Privoznik wrote:
On 29.07.2016 14:08, Daniel P. Berrange wrote:
On Fri, Jul 29, 2016 at 02:06:00PM +0200, Erik Skultety wrote:
Just like we decide on which URI we go with based on EUID for qemu in remote driver, do a similar thing for admin except we do not spawn a daemon in this case.
IMHO we should auto-spawn the daemon too, so we have consistent behaviour virsh virsh.
I don't think this is a good idea. The session daemon dies after 30 seconds after last connection was closed. There's no point in setting something via admin API just to find it back on defaults after 1 minute because new daemon has been spawned. It's not like our domain APIs where libvirt refreshes the internal state of drivers upon their init.
Yeah, there's no point in auto-starting the daemon just to set something.
Michal
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On 29.07.2016 14:06, Erik Skultety wrote:
Just like we decide on which URI we go with based on EUID for qemu in remote driver, do a similar thing for admin except we do not spawn a daemon in this case.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1356858
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/libvirt-admin.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
ACK Michal

On 08/08/16 16:27, Michal Privoznik wrote:
On 29.07.2016 14:06, Erik Skultety wrote:
Just like we decide on which URI we go with based on EUID for qemu in remote driver, do a similar thing for admin except we do not spawn a daemon in this case.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1356858
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/libvirt-admin.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
ACK
Michal
Thanks, I pushed the patch. Erik
participants (4)
-
Daniel P. Berrange
-
Erik Skultety
-
Martin Kletzander
-
Michal Privoznik