[libvirt] [PATCH] Explicitly pass uml_dir argument to user-mode-linux
uml_dir overrides user-mode-linux's default of ~/.uml. This is needed for a couple of different reasons: libvirt expects this to default to virGetUserDirectory(geteuid()) + '/.uml'. However, user-mode-linux actually uses the HOME environment variable to determine where to look for the uml sockets, but if running libvirtd under sudo (which I routinely do during development), $HOME is pointing at my user's homedir, while my euid is 0, so libvirt looks in /root. Also (and this was my actual motivation for this patch), if HOME isn't set at all, user-mode-linux utterly fails. Looking at the code, it seems it's meant to emit a warning, but alas, it doesn't for some reason. If running libvirtd from upstart, HOME is not set, so any system using upstart will need this change. Signed-off-by: Soren Hansen <soren@linux2go.dk> --- src/uml/uml_conf.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c index 65b06c5..4906192 100644 --- a/src/uml/uml_conf.c +++ b/src/uml/uml_conf.c @@ -409,7 +409,7 @@ static char *umlNextArg(char *args) * for a given virtual machine. */ int umlBuildCommandLine(virConnectPtr conn, - struct uml_driver *driver ATTRIBUTE_UNUSED, + struct uml_driver *driver, virDomainObjPtr vm, fd_set *keepfd, const char ***retargv, @@ -499,7 +499,6 @@ int umlBuildCommandLine(virConnectPtr conn, ADD_ENV_COPY("LD_PRELOAD"); ADD_ENV_COPY("LD_LIBRARY_PATH"); ADD_ENV_COPY("PATH"); - ADD_ENV_COPY("HOME"); ADD_ENV_COPY("USER"); ADD_ENV_COPY("LOGNAME"); ADD_ENV_COPY("TMPDIR"); @@ -508,6 +507,7 @@ int umlBuildCommandLine(virConnectPtr conn, //ADD_ARG_PAIR("con0", "fd:0,fd:1"); ADD_ARG_PAIR("mem", memory); ADD_ARG_PAIR("umid", vm->def->name); + ADD_ARG_PAIR("uml_dir", driver->monitorDir); if (vm->def->os.root) ADD_ARG_PAIR("root", vm->def->os.root); -- 1.7.0.4
On 08/25/2010 05:03 AM, Soren Hansen wrote:
uml_dir overrides user-mode-linux's default of ~/.uml. This is needed for a couple of different reasons:
libvirt expects this to default to virGetUserDirectory(geteuid()) + '/.uml'. However, user-mode-linux actually uses the HOME environment variable to determine where to look for the uml sockets, but if running libvirtd under sudo (which I routinely do during development), $HOME is pointing at my user's homedir, while my euid is 0, so libvirt looks in /root.
Also (and this was my actual motivation for this patch), if HOME isn't set at all, user-mode-linux utterly fails. Looking at the code, it seems it's meant to emit a warning, but alas, it doesn't for some reason. If running libvirtd from upstart, HOME is not set, so any system using upstart will need this change.
Signed-off-by: Soren Hansen <soren@linux2go.dk> --- src/uml/uml_conf.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c index 65b06c5..4906192 100644 --- a/src/uml/uml_conf.c +++ b/src/uml/uml_conf.c @@ -409,7 +409,7 @@ static char *umlNextArg(char *args) * for a given virtual machine. */ int umlBuildCommandLine(virConnectPtr conn, - struct uml_driver *driver ATTRIBUTE_UNUSED, + struct uml_driver *driver, virDomainObjPtr vm, fd_set *keepfd, const char ***retargv, @@ -499,7 +499,6 @@ int umlBuildCommandLine(virConnectPtr conn, ADD_ENV_COPY("LD_PRELOAD"); ADD_ENV_COPY("LD_LIBRARY_PATH"); ADD_ENV_COPY("PATH"); - ADD_ENV_COPY("HOME"); ADD_ENV_COPY("USER"); ADD_ENV_COPY("LOGNAME"); ADD_ENV_COPY("TMPDIR"); @@ -508,6 +507,7 @@ int umlBuildCommandLine(virConnectPtr conn, //ADD_ARG_PAIR("con0", "fd:0,fd:1"); ADD_ARG_PAIR("mem", memory); ADD_ARG_PAIR("umid", vm->def->name); + ADD_ARG_PAIR("uml_dir", driver->monitorDir);
if (vm->def->os.root) ADD_ARG_PAIR("root", vm->def->os.root);
I think this should also solve this long standing fedora/selinux bug: https://bugzilla.redhat.com/show_bug.cgi?id=499536 - Cole
On Wed, Aug 25, 2010 at 10:07:45AM -0400, Cole Robinson wrote:
On 08/25/2010 05:03 AM, Soren Hansen wrote:
uml_dir overrides user-mode-linux's default of ~/.uml. This is needed for a couple of different reasons:
libvirt expects this to default to virGetUserDirectory(geteuid()) + '/.uml'. However, user-mode-linux actually uses the HOME environment variable to determine where to look for the uml sockets, but if running libvirtd under sudo (which I routinely do during development), $HOME is pointing at my user's homedir, while my euid is 0, so libvirt looks in /root.
Also (and this was my actual motivation for this patch), if HOME isn't set at all, user-mode-linux utterly fails. Looking at the code, it seems it's meant to emit a warning, but alas, it doesn't for some reason. If running libvirtd from upstart, HOME is not set, so any system using upstart will need this change.
Signed-off-by: Soren Hansen <soren@linux2go.dk> --- src/uml/uml_conf.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c index 65b06c5..4906192 100644 --- a/src/uml/uml_conf.c +++ b/src/uml/uml_conf.c @@ -409,7 +409,7 @@ static char *umlNextArg(char *args) * for a given virtual machine. */ int umlBuildCommandLine(virConnectPtr conn, - struct uml_driver *driver ATTRIBUTE_UNUSED, + struct uml_driver *driver, virDomainObjPtr vm, fd_set *keepfd, const char ***retargv, @@ -499,7 +499,6 @@ int umlBuildCommandLine(virConnectPtr conn, ADD_ENV_COPY("LD_PRELOAD"); ADD_ENV_COPY("LD_LIBRARY_PATH"); ADD_ENV_COPY("PATH"); - ADD_ENV_COPY("HOME"); ADD_ENV_COPY("USER"); ADD_ENV_COPY("LOGNAME"); ADD_ENV_COPY("TMPDIR"); @@ -508,6 +507,7 @@ int umlBuildCommandLine(virConnectPtr conn, //ADD_ARG_PAIR("con0", "fd:0,fd:1"); ADD_ARG_PAIR("mem", memory); ADD_ARG_PAIR("umid", vm->def->name); + ADD_ARG_PAIR("uml_dir", driver->monitorDir);
if (vm->def->os.root) ADD_ARG_PAIR("root", vm->def->os.root);
I think this should also solve this long standing fedora/selinux bug:
Almost. We still need to change 'driver->monitorDir' so that it gets initialized to '$LOCAL_STATE_DIR/lib/libvirt/uml' like we do with QEMU (or $HOME/.libvirt/uml/lib for session mode) Regards, 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 :|
On 25-08-2010 16:18, Daniel P. Berrange wrote:
Almost. We still need to change 'driver->monitorDir' so that it gets initialized to '$LOCAL_STATE_DIR/lib/libvirt/uml' like we do with QEMU (or $HOME/.libvirt/uml/lib for session mode)
I think leaving it as $HOME/.uml is preferable for uml:///session. It integrates well with the existing, well-known uml-utilities (e.g. "uml_mconsole <domain name>" Just Works[tm]). -- Soren Hansen Ubuntu Developer http://www.ubuntu.com/
On Wed, Aug 25, 2010 at 05:56:22PM +0200, Soren Hansen wrote:
On 25-08-2010 16:18, Daniel P. Berrange wrote:
Almost. We still need to change 'driver->monitorDir' so that it gets initialized to '$LOCAL_STATE_DIR/lib/libvirt/uml' like we do with QEMU (or $HOME/.libvirt/uml/lib for session mode)
I think leaving it as $HOME/.uml is preferable for uml:///session. It integrates well with the existing, well-known uml-utilities (e.g. "uml_mconsole <domain name>" Just Works[tm]).
Oh, i guess so then. 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 :|
On 25-08-2010 16:07, Cole Robinson wrote:
On 08/25/2010 05:03 AM, Soren Hansen wrote:
uml_dir overrides user-mode-linux's default of ~/.uml. This is needed for a couple of different reasons: I think this should also solve this long standing fedora/selinux bug:
Well, halfway. The other half should be easy, though. Gimme a few minutes. -- Soren Hansen Ubuntu Developer http://www.ubuntu.com/
For privileged UML connections (uml:///system), we shouldn't use root's home dir, but rather somewhre in /var/run/libvirt/uml. https://bugzilla.redhat.com/show_bug.cgi?id=499536 Signed-off-by: Soren Hansen <soren@linux2go.dk> --- src/uml/uml_driver.c | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 8b129b7..c8b9997 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -373,6 +373,10 @@ umlStartup(int privileged) { if ((base = strdup (SYSCONF_DIR "/libvirt")) == NULL) goto out_of_memory; + + if (virAsprintf(¨_driver->monitorDir, + "%s/run/libvirt/uml", LOCAL_STATE_DIR) == -1) + goto out_of_memory; } else { if (virAsprintf(¨_driver->logDir, @@ -381,11 +385,11 @@ umlStartup(int privileged) { if (virAsprintf(&base, "%s/.libvirt", userdir) == -1) goto out_of_memory; - } - if (virAsprintf(¨_driver->monitorDir, - "%s/.uml", userdir) == -1) - goto out_of_memory; + if (virAsprintf(¨_driver->monitorDir, + "%s/.uml", userdir) == -1) + goto out_of_memory; + } /* Configuration paths are either ~/.libvirt/uml/... (session) or * /etc/libvirt/uml/... (system). -- 1.7.0.4
On Wed, Aug 25, 2010 at 05:52:23PM +0200, Soren Hansen wrote:
For privileged UML connections (uml:///system), we shouldn't use root's home dir, but rather somewhre in /var/run/libvirt/uml.
https://bugzilla.redhat.com/show_bug.cgi?id=499536
Signed-off-by: Soren Hansen <soren@linux2go.dk> --- src/uml/uml_driver.c | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 8b129b7..c8b9997 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -373,6 +373,10 @@ umlStartup(int privileged) {
if ((base = strdup (SYSCONF_DIR "/libvirt")) == NULL) goto out_of_memory; + + if (virAsprintf(¨_driver->monitorDir, + "%s/run/libvirt/uml", LOCAL_STATE_DIR) == -1) + goto out_of_memory;
Can we make this '%s/lib/libvirt/uml'
} else {
if (virAsprintf(¨_driver->logDir, @@ -381,11 +385,11 @@ umlStartup(int privileged) {
if (virAsprintf(&base, "%s/.libvirt", userdir) == -1) goto out_of_memory; - }
- if (virAsprintf(¨_driver->monitorDir, - "%s/.uml", userdir) == -1) - goto out_of_memory; + if (virAsprintf(¨_driver->monitorDir, + "%s/.uml", userdir) == -1) + goto out_of_memory; + }
And '%s/.libvirt/uml/lib' So that we match the QEMU driver layout. We aim to keep a strict separate for 'lib' to be the place where the guest can write/create files (control sockets), and 'run' to be the place where libvirtd can write/create files (eg pid files, xml state). 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 :|
On 25-08-2010 18:07, Daniel P. Berrange wrote:
--- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -373,6 +373,10 @@ umlStartup(int privileged) {
if ((base = strdup (SYSCONF_DIR "/libvirt")) == NULL) goto out_of_memory; + + if (virAsprintf(¨_driver->monitorDir, + "%s/run/libvirt/uml", LOCAL_STATE_DIR) == -1) + goto out_of_memory;
Can we make this '%s/lib/libvirt/uml'
You want transient stuff like UNIX sockets in /var/lib? That seems odd to me. The FHS even explicitly says: "System programs that maintain transient UNIX-domain sockets must place them in this directory." where "this directory" is /var/run.
} else {
if (virAsprintf(¨_driver->logDir, @@ -381,11 +385,11 @@ umlStartup(int privileged) {
if (virAsprintf(&base, "%s/.libvirt", userdir) == -1) goto out_of_memory; - }
- if (virAsprintf(¨_driver->monitorDir, - "%s/.uml", userdir) == -1) - goto out_of_memory; + if (virAsprintf(¨_driver->monitorDir, + "%s/.uml", userdir) == -1) + goto out_of_memory; + }
And '%s/.libvirt/uml/lib'
I understand your desire to be consistent, but I really think integrating well with existing management tools for the hypervisor (in this case uml-utilities) is more important. -- Soren Hansen Ubuntu Developer http://www.ubuntu.com/
On Wed, Aug 25, 2010 at 06:39:11PM +0200, Soren Hansen wrote:
On 25-08-2010 18:07, Daniel P. Berrange wrote:
--- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -373,6 +373,10 @@ umlStartup(int privileged) {
if ((base = strdup (SYSCONF_DIR "/libvirt")) == NULL) goto out_of_memory; + + if (virAsprintf(¨_driver->monitorDir, + "%s/run/libvirt/uml", LOCAL_STATE_DIR) == -1) + goto out_of_memory;
Can we make this '%s/lib/libvirt/uml'
You want transient stuff like UNIX sockets in /var/lib? That seems odd to me.
Not hugely, but I don't want it to overlap with the directories used by libvirtd for transient stuff. If we make it '%/run/libvirt/uml-guest' instead that would be OK
- }
- if (virAsprintf(¨_driver->monitorDir, - "%s/.uml", userdir) == -1) - goto out_of_memory; + if (virAsprintf(¨_driver->monitorDir, + "%s/.uml", userdir) == -1) + goto out_of_memory; + } And '%s/.libvirt/uml/lib'
I understand your desire to be consistent, but I really think integrating well with existing management tools for the hypervisor (in this case uml-utilities) is more important.
Agreed, I hadn't seen your reply about uml-utilities when writing this. 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 :|
On 31-08-2010 12:39, Daniel P. Berrange wrote:
Can we make this '%s/lib/libvirt/uml' You want transient stuff like UNIX sockets in /var/lib? That seems odd to me. Not hugely, but I don't want it to overlap with the directories used by libvirtd for transient stuff. If we make it '%/run/libvirt/uml-guest' instead that would be OK
Excellent.
I understand your desire to be consistent, but I really think integrating well with existing management tools for the hypervisor (in this case uml-utilities) is more important. Agreed, I hadn't seen your reply about uml-utilities when writing this.
Excellent again. I'll make these adjustments. -- Soren Hansen Ubuntu Developer http://www.ubuntu.com/
For privileged UML connections (uml:///system), we shouldn't use root's home dir, but rather somewhere in /var/run/libvirt/uml-guest. https://bugzilla.redhat.com/show_bug.cgi?id=499536 Signed-off-by: Soren Hansen <soren@linux2go.dk> --- src/uml/uml_driver.c | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 8b129b7..0a5c829 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -373,6 +373,10 @@ umlStartup(int privileged) { if ((base = strdup (SYSCONF_DIR "/libvirt")) == NULL) goto out_of_memory; + + if (virAsprintf(¨_driver->monitorDir, + "%s/run/libvirt/uml-guest", LOCAL_STATE_DIR) == -1) + goto out_of_memory; } else { if (virAsprintf(¨_driver->logDir, @@ -381,11 +385,11 @@ umlStartup(int privileged) { if (virAsprintf(&base, "%s/.libvirt", userdir) == -1) goto out_of_memory; - } - if (virAsprintf(¨_driver->monitorDir, - "%s/.uml", userdir) == -1) - goto out_of_memory; + if (virAsprintf(¨_driver->monitorDir, + "%s/.uml", userdir) == -1) + goto out_of_memory; + } /* Configuration paths are either ~/.libvirt/uml/... (session) or * /etc/libvirt/uml/... (system). -- 1.7.0.4
On 08/31/2010 05:44 AM, Soren Hansen wrote:
For privileged UML connections (uml:///system), we shouldn't use root's home dir, but rather somewhere in /var/run/libvirt/uml-guest.
https://bugzilla.redhat.com/show_bug.cgi?id=499536
Signed-off-by: Soren Hansen<soren@linux2go.dk> --- src/uml/uml_driver.c | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-)
I think this version captures all the discussion on the ideal locations for privileged vs. regular users elsewhere in the thread. ACK, and pushed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 25-08-2010 11:03, Soren Hansen wrote:
uml_dir overrides user-mode-linux's default of ~/.uml. This is needed for a couple of different reasons:
Any comments on this patch? It seems perfectly reasonable to me to get this in regardless of the outcome of this discussion: https://www.redhat.com/archives/libvir-list/2010-August/msg00672.html -- Soren Hansen Ubuntu Developer http://www.ubuntu.com/
On Wed, Aug 25, 2010 at 11:03:42AM +0200, Soren Hansen wrote:
uml_dir overrides user-mode-linux's default of ~/.uml. This is needed for a couple of different reasons:
libvirt expects this to default to virGetUserDirectory(geteuid()) + '/.uml'. However, user-mode-linux actually uses the HOME environment variable to determine where to look for the uml sockets, but if running libvirtd under sudo (which I routinely do during development), $HOME is pointing at my user's homedir, while my euid is 0, so libvirt looks in /root.
Also (and this was my actual motivation for this patch), if HOME isn't set at all, user-mode-linux utterly fails. Looking at the code, it seems it's meant to emit a warning, but alas, it doesn't for some reason. If running libvirtd from upstart, HOME is not set, so any system using upstart will need this change.
Signed-off-by: Soren Hansen <soren@linux2go.dk> --- src/uml/uml_conf.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c index 65b06c5..4906192 100644 --- a/src/uml/uml_conf.c +++ b/src/uml/uml_conf.c @@ -409,7 +409,7 @@ static char *umlNextArg(char *args) * for a given virtual machine. */ int umlBuildCommandLine(virConnectPtr conn, - struct uml_driver *driver ATTRIBUTE_UNUSED, + struct uml_driver *driver, virDomainObjPtr vm, fd_set *keepfd, const char ***retargv, @@ -499,7 +499,6 @@ int umlBuildCommandLine(virConnectPtr conn, ADD_ENV_COPY("LD_PRELOAD"); ADD_ENV_COPY("LD_LIBRARY_PATH"); ADD_ENV_COPY("PATH"); - ADD_ENV_COPY("HOME"); ADD_ENV_COPY("USER"); ADD_ENV_COPY("LOGNAME"); ADD_ENV_COPY("TMPDIR"); @@ -508,6 +507,7 @@ int umlBuildCommandLine(virConnectPtr conn, //ADD_ARG_PAIR("con0", "fd:0,fd:1"); ADD_ARG_PAIR("mem", memory); ADD_ARG_PAIR("umid", vm->def->name); + ADD_ARG_PAIR("uml_dir", driver->monitorDir);
if (vm->def->os.root) ADD_ARG_PAIR("root", vm->def->os.root);
ACK Regards, 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 :|
On 08/31/2010 04:37 AM, Daniel P. Berrange wrote:
On Wed, Aug 25, 2010 at 11:03:42AM +0200, Soren Hansen wrote:
uml_dir overrides user-mode-linux's default of ~/.uml. This is needed for a couple of different reasons:
@@ -508,6 +507,7 @@ int umlBuildCommandLine(virConnectPtr conn, //ADD_ARG_PAIR("con0", "fd:0,fd:1"); ADD_ARG_PAIR("mem", memory); ADD_ARG_PAIR("umid", vm->def->name); + ADD_ARG_PAIR("uml_dir", driver->monitorDir);
if (vm->def->os.root) ADD_ARG_PAIR("root", vm->def->os.root);
ACK
Pushed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (5)
-
Cole Robinson -
Daniel P. Berrange -
Eric Blake -
Soren Hansen -
Soren Hansen