On Thu, Jan 15, 2009 at 09:19:39AM -0800, john.levon@xxxxxxx wrote:
> +#ifdef __sun
> +static void
> +qemudSetupPrivs (struct qemud_server *server)
> +{
> + chown ("/var/run/libvirt", SYSTEM_UID, SYSTEM_UID);
> + chown (server->logDir, SYSTEM_UID, SYSTEM_UID);
> +
> + if (__init_daemon_priv (PU_RESETGROUPS | PU_CLEARLIMITSET,
> + SYSTEM_UID, SYSTEM_UID, PRIV_XVM_CONTROL, NULL)) {
> + fprintf (stderr, "additional privileges are required\n");
> + exit (1);
> + }
> +
> + if (priv_set (PRIV_OFF, PRIV_ALLSETS, PRIV_FILE_LINK_ANY, PRIV_PROC_INFO,
> + PRIV_PROC_SESSION, PRIV_PROC_EXEC, PRIV_PROC_FORK, NULL)) {
> + fprintf (stderr, "failed to set reduced privileges\n");
> + exit (1);
> + }
> +}
> +#else
> +#define qemudSetupPrivs(a)
> +#endif
I think this function should be made to return int 0/-1 for
failure, and let the caller propagate errors & do any cleanup
it may desire, since that's the way most other failures in
the daemon initialization are dealt with.
>
> /* Print command-line usage. */
> static void
> @@ -2417,6 +2493,20 @@ int main(int argc, char **argv) {
> sig_action.sa_handler = SIG_IGN;
> sigaction(SIGPIPE, &sig_action, NULL);
>
> + /* Change the group ownership of /var/run/libvirt to unix_sock_gid */
> + if (geteuid () == 0) {
> + const char *rundir = LOCAL_STATE_DIR "/run/libvirt";
> +
> + if (mkdir (rundir, 0755)) {
> + if (errno != EEXIST) {
> + VIR_ERROR0 (_("unable to create rundir"));
> + return (-1);
> + }
> + }
> + }
> +
> + qemudSetupPrivs(server);
This would need a "if(qemudSetupPrivs(server) < 0) return -1;"
> diff --git a/src/remote_internal.c b/src/remote_internal.c
> --- a/src/remote_internal.c
> +++ b/src/remote_internal.c
> @@ -90,7 +90,7 @@
> #define MAGIC 999 /* private_data->magic if OK */
> #define DEAD 998 /* private_data->magic if dead/closed */
>
> -static int inside_daemon = 0;
> +int inside_daemon = 0;
Eek no, the Xen code shouldn't reference this variable directly - it
should track its own. We need to avoid compile dependancies inbetween
any of the drivers, because all drivers need to be able to be turned
on/off independantly, and may be dlopen'd at runtime, so you can't
guarentee you'll have access to another driver's data.
> diff --git a/src/xen_unified.c b/src/xen_unified.c
> --- a/src/xen_unified.c
> +++ b/src/xen_unified.c
> @@ -231,6 +231,16 @@ xenUnifiedOpen (virConnectPtr conn, virC
> xenUnifiedPrivatePtr priv;
> virDomainEventCallbackListPtr cbList;
>
> +#ifdef __sun
> + extern int inside_daemon;
This flags needs to be tracked by the Xen driver explicitly &
independantly of the daemon code.
Aside from those two issues, I think this patch looks pretty good now.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list