Re: [libvirt] [PATCH 09/15] Add libvirt-admin library

On Fri, Apr 17, 2015 at 11:28:11AM +0100, Daniel P. Berrange wrote: >On Thu, Apr 16, 2015 at 04:46:44PM +0200, Martin Kletzander wrote: >>Initial scratch of the admin library. It has its own virAdmConnectPtr >>that inherits from virAbstractConnectPtr and thus trivially supports >>error reporting. > >See my note earlier about error reporting on the connection being >a bad idea due to lack of thread safety. > >>Configuration option --with-admin is added to control whether the admin >>library should be built or not (set to 'yes' by default). > >Is there a compelling reason why we'd need/want to be able to >disable building of the admin library ? As a comparison we >unconditionally build libvirt-qemu.so & libvirt-lxc.so > No reason, I just figured someone might want to disable that. [...] >>+virAdmConnectPtr virAdmConnectOpen(unsigned int flags); > >How does this figure out which libvirtd daemon to connect to ? Presumably >you've hardcoded it based on the UID you're running as ? It doesn't. I don't think anyone is going to use this to connect to the session daemon. I also don't see anyone having session daemon running without the possibility of restarting it. >I think for future proofing we should probably define a URI syntax >for this. > >eg > > admin:///system > admin:///session > >And allow an optional parameter for the socket path, for people who >have built their daemon with an unusual --prefix arg. > I think we can keep it without the possibility of connecting to the session daemon and if that is going to change, we can add virAdmConnectSession() function. In the meantime I'd rather focus on enabling this functionality then dealing with session daemon, all the unnecessary code it adds and the problems we already have with the session daemon.