On Mon, 2019-07-29 at 16:49 +0100, Daniel P. Berrangé wrote:
On Mon, Jul 29, 2019 at 02:32:31PM +0200, Andrea Bolognani wrote:
On Tue, 2019-07-23 at 17:03 +0100, Daniel P. Berrangé wrote:
- if (virAsprintf(&sockname, - "%s/" LIBVIRTD_USER_UNIX_SOCKET, userdir) < 0) + if (virAsprintf(&sockname, "%s/%s-sock", + userdir, sock_prefix) < 0)
I kinda just noticed, but don't we support R/O connections in session mode?
The client app is required to be the same user ID as the daemon. As such there's no meaningful security separation between the two from a DAC pov, so R/O socket was deemed to be a waste of time.
If you had SELinux strictly locking things down it could be considered slightly more secure, but no one has ever cared enough to enable it.
Alright.
+ if (!direct_sock_name) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Cannot use direct socket mode if no URI is set")); + return NULL; + }
Is the error message accurate? We should be way past making sure we have a URI to work with by now.
We'll only hit direct_sock_name == NULL, if driver == NULL.
We'll only hit driver == NULL, if the original URI was NULL.
Okay.
+#ifndef WIN32 +static const char * +remoteGetDaemonPathEnv(void) +{ + /* We prefer a VIRTD_PATH env var to use for all daemons, + * but if it is not set we will fallback to LIBVIRTD_PATH + * for previous behaviour + */ + if (virGetEnvBlockSUID("VIRTD_PATH") != NULL) { + return "VIRTD_PATH"; + } else { + return "LIBVIRTD_PATH"; + } +} +#endif /* WIN32 */
I don't think this function needs to be guarded by 'ifndef WIN32': we already do so at the call site, and AFAICT there's nothing in the helper itself that warrants compiling it out on Windows.
It is a static function, so will trigger an unused function warning.
You've convinced me, so Reviewed-by: Andrea Bolognani <abologna@redhat.com> if you address the style issues and most importantly switch the default mode to legacy. -- Andrea Bolognani / Red Hat / Virtualization