Re: [systemd-devel] [PATCH] systemctl: systemctl --root=container/ set-default ... is totally borked.
On Thu, 17.04.14 10:40, Jan Chaloupka (jchal...@redhat.com) wrote: On 04/17/2014 04:59 AM, Zbigniew Je;drzejewski-Szmek wrote: On Thu, Apr 17, 2014 at 01:41:51AM +0100, Djalal Harouni wrote: BTW, I've a question, why there is this item in the TODO: systemctl --root=container/ set-default ... is totally borked. Can someone please shed some light on this? I added this, and I guess I should have been more specific, because I had to test this again, to see what is wrong :) systemctl --root=/var/tmp/inst1 set-default multi-user.target creates a symlink /var/tmp/inst1//usr/etc/systemd/system/default.target - /var/tmp/inst1//lib/systemd/system/multi-user.target, i.e. leaks the container name. If understood correctly, proper symlink should be /var/tmp/inst1//usr/etc/systemd/system/default.target - /lib/systemd/system/multi-user.target Not appending --root prefix in unit_file_search will fix it Hmm, we really should try to generate relative symlinks for cases like this. This is a lot nicer when one browses through an OS tree installed in some subdirectory, since the symlinks then point to the right files, instead of files on the host. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemctl: systemctl --root=container/ set-default ... is totally borked.
On Thu, Apr 17, 2014 at 01:42:23PM +0200, Zbigniew Jędrzejewski-Szmek wrote: On Thu, Apr 17, 2014 at 10:40:37AM +0200, Jan Chaloupka wrote: On 04/17/2014 04:59 AM, Zbigniew Je;drzejewski-Szmek wrote: On Thu, Apr 17, 2014 at 01:41:51AM +0100, Djalal Harouni wrote: BTW, I've a question, why there is this item in the TODO: systemctl --root=container/ set-default ... is totally borked. Can someone please shed some light on this? I added this, and I guess I should have been more specific, because I had to test this again, to see what is wrong :) systemctl --root=/var/tmp/inst1 set-default multi-user.target creates a symlink /var/tmp/inst1//usr/etc/systemd/system/default.target - /var/tmp/inst1//lib/systemd/system/multi-user.target, i.e. leaks the container name. If understood correctly, proper symlink should be /var/tmp/inst1//usr/etc/systemd/system/default.target - /lib/systemd/system/multi-user.target Hmm, I really can't follow here, when I was fixing that bug I looked at the other switches running against a container and the origin of the --root patch: http://lists.freedesktop.org/archives/systemd-devel/2011-June/002708.html And my understanding is that this should behave as we are inside a chroot IOW container...! and this is what the commit log suggests but not the man page :-/ So ? -- Djalal Harouni http://opendz.org ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemctl: systemctl --root=container/ set-default ... is totally borked.
On 04/17/2014 04:59 AM, Zbigniew Je;drzejewski-Szmek wrote:
On Thu, Apr 17, 2014 at 01:41:51AM +0100, Djalal Harouni wrote:
BTW, I've a question, why there is this item in the TODO:
systemctl --root=container/ set-default ... is totally borked.
Can someone please shed some light on this?
I added this, and I guess I should have been more specific, because I had
to test this again, to see what is wrong :)
systemctl --root=/var/tmp/inst1 set-default multi-user.target
creates a symlink /var/tmp/inst1//usr/etc/systemd/system/default.target -
/var/tmp/inst1//lib/systemd/system/multi-user.target, i.e. leaks the container
name.
If understood correctly, proper symlink should be
/var/tmp/inst1//usr/etc/systemd/system/default.target -
/lib/systemd/system/multi-user.target
Not appending --root prefix in unit_file_search will fix it
---
shared/install.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/shared/install.c b/shared/install.c
index 6334833..75d3455 100644
--- a/shared/install.c
+++ b/shared/install.c
@@ -1045,10 +1045,7 @@ static int unit_file_search(
STRV_FOREACH(p, paths-unit_path) {
char *path = NULL;
-if (isempty(root_dir))
-asprintf(path, %s/%s, *p, info-name);
-else
-asprintf(path, %s/%s/%s, root_dir, *p, info-name);
+asprintf(path, %s/%s, *p, info-name);
if (!path)
return -ENOMEM;
--
1.9.0
Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemctl: systemctl --root=container/ set-default ... is totally borked.
On Thu, Apr 17, 2014 at 10:40:37AM +0200, Jan Chaloupka wrote:
On 04/17/2014 04:59 AM, Zbigniew Je;drzejewski-Szmek wrote:
On Thu, Apr 17, 2014 at 01:41:51AM +0100, Djalal Harouni wrote:
BTW, I've a question, why there is this item in the TODO:
systemctl --root=container/ set-default ... is totally borked.
Can someone please shed some light on this?
I added this, and I guess I should have been more specific, because I had
to test this again, to see what is wrong :)
systemctl --root=/var/tmp/inst1 set-default multi-user.target
creates a symlink /var/tmp/inst1//usr/etc/systemd/system/default.target -
/var/tmp/inst1//lib/systemd/system/multi-user.target, i.e. leaks the
container name.
If understood correctly, proper symlink should be
/var/tmp/inst1//usr/etc/systemd/system/default.target -
/lib/systemd/system/multi-user.target
Not appending --root prefix in unit_file_search will fix it
---
shared/install.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/shared/install.c b/shared/install.c
index 6334833..75d3455 100644
--- a/shared/install.c
+++ b/shared/install.c
@@ -1045,10 +1045,7 @@ static int unit_file_search(
STRV_FOREACH(p, paths-unit_path) {
char *path = NULL;
-if (isempty(root_dir))
-asprintf(path, %s/%s, *p, info-name);
-else
-asprintf(path, %s/%s/%s, root_dir, *p,
info-name);
+asprintf(path, %s/%s, *p, info-name);
This path is used to load the file a few lines down... Such a simple
fix is unlikely to work.
Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemctl: systemctl --root=container/ set-default ... is totally borked.
'Twas brillig, and Jan Chaloupka at 17/04/14 12:54 did gyre and gimble:
On 04/17/2014 01:42 PM, Zbigniew Jędrzejewski-Szmek wrote:
On Thu, Apr 17, 2014 at 10:40:37AM +0200, Jan Chaloupka wrote:
On 04/17/2014 04:59 AM, Zbigniew Je;drzejewski-Szmek wrote:
On Thu, Apr 17, 2014 at 01:41:51AM +0100, Djalal Harouni wrote:
BTW, I've a question, why there is this item in the TODO:
systemctl --root=container/ set-default ... is totally borked.
Can someone please shed some light on this?
I added this, and I guess I should have been more specific, because
I had
to test this again, to see what is wrong :)
systemctl --root=/var/tmp/inst1 set-default multi-user.target
creates a symlink
/var/tmp/inst1//usr/etc/systemd/system/default.target -
/var/tmp/inst1//lib/systemd/system/multi-user.target, i.e. leaks the
container name.
If understood correctly, proper symlink should be
/var/tmp/inst1//usr/etc/systemd/system/default.target -
/lib/systemd/system/multi-user.target
Not appending --root prefix in unit_file_search will fix it
---
shared/install.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/shared/install.c b/shared/install.c
index 6334833..75d3455 100644
--- a/shared/install.c
+++ b/shared/install.c
@@ -1045,10 +1045,7 @@ static int unit_file_search(
STRV_FOREACH(p, paths-unit_path) {
char *path = NULL;
-if (isempty(root_dir))
-asprintf(path, %s/%s, *p, info-name);
-else
-asprintf(path, %s/%s/%s, root_dir, *p,
info-name);
+asprintf(path, %s/%s, *p, info-name);
This path is used to load the file a few lines down... Such a simple
fix is unlikely to work.
In a case of systemctl --root=/var/tmp/inst1 set-default
multi-user.target, it is working. I 've tested it on my machine. Symlink
is created, pointing to /lib/systemd/system/multi-user.target. Just
question, if it is not going to break something else.
Well, as Zbigniew said, the same path is used a few lines down to load a
file.
If it so happens that the target in the container also has a file in the
host then it may appear to work nicely, but is technically incorrect
(although the files may exist in both container and host, they may be
different).
If the target in question only exists inside the container, then it
would likely fail.
Col
PS I'm only going on what Zbigniew said here - didn't actually look at
the code.
--
Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/
Day Job:
Tribalogic Limited http://www.tribalogic.net/
Open Source:
Mageia Contributor http://www.mageia.org/
PulseAudio Hacker http://www.pulseaudio.org/
Trac Hacker http://trac.edgewall.org/
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
