[systemd-devel] [PATCH] systemctl: "systemctl --root=container/ set-default ... is totally borked."

[Colin Guthrie]

'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/