Le lundi 25 mars 2013 à 12:04 +0000, Colin Guthrie a écrit :
> 'Twas brillig, and Frederic Crozat at 25/03/13 10:14 did gyre and gimble:
> > Le vendredi 22 mars 2013 à 23:53 +0100, Lennart Poettering a écrit :
> >> On Thu, 21.03.13 17:04, Frederic Crozat ([email protected]) wrote:
> >>
> >>> Hi all,
> >>>
> >>> in https://bugzilla.novell.com/show_bug.cgi?id=809646 we noticed LSB
> >>> Provides can sometime be incorrectly handled (resulting in "Failed to
> >>> add LSB Provides name XXXX.service, ignoring: File exists" errors),
> >>> depending on initscript parsing order (if a provides is required by
> >>> another initscript and this initscript is parsed before the one with the
> >>> provides).
> >>
> >> Can you explain the problem in more detail? Not following here.
> > 
> > Yes, sorry, I didn't give our test example :
> > 
> > Let's say you have two initscripts, A and B:
> > 
> > A contains in its LSB header:
> > Required-Start: C
> > 
> > and B contains in its LSB header:
> > Provides: C
> > 
> > When systemd is parsing /etc/rc.d/, depending on the file order, you can
> > end up with:
> > - B is parsed first. An unit "C.service" will be "created" and will be
> > added as additional name to B.service, with unit_add_name. No bug.
> > - A is parsed first. An unit "C.service" is created for the
> > "Required-Start" dependency (it will have no file attached, since
> > nothing provides this dependency yet). Then B is parsed and when trying
> > to handle "Provides: C", unit_add_name is called but will fail, because
> > "C.service" already exists in manager->units. Therefore, a merge should
> > occur for that case.
> 
> Is this explanation perhaps worthy of going in the commit message? It
> was all very clear when I read this extended explanation but I didn't
> 100% grok it before :)
> 
> >>
> >> Also, whitespace/coding style issues. 
> > 
> > Will fix.
> 
> Sorry to be a pain, but technically the new, much leaner patch still
> contains a whitespace/coding style issue :p
> 
> unit_merge_by_name should not have a space after it before it's parenthesis.
> 
> 
> 
> Of course it's trivial for someone to do both those things above when
> merging, but..... :)

New patch version, this time with the improved commit message and space
error fixed.

-- 
Frederic Crozat <[email protected]>
SUSE
>From a99435109b83e7146a30ccf5387037b51c1fe907 Mon Sep 17 00:00:00 2001
From: Frederic Crozat <[email protected]>
Date: Thu, 21 Mar 2013 15:40:45 +0100
Subject: [PATCH] core: ensure LSB Provides are handled correctly

Let's say you have two initscripts, A and B:

A contains in its LSB header:
Required-Start: C

and B contains in its LSB header:
Provides: C

When systemd is parsing /etc/rc.d/, depending on the file order, you
can end up with either:
- B is parsed first. An unit "C.service" will be "created" and will be
added as additional name to B.service, with unit_add_name. No bug.
- A is parsed first. An unit "C.service" is created for the
"Required-Start" dependency (it will have no file attached, since
nothing provides this dependency yet). Then B is parsed and when trying
to handle "Provides: C", unit_add_name is called but will fail, because
"C.service" already exists in manager->units. Therefore, a merge should
occur for that case.
---
 src/core/service.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/core/service.c b/src/core/service.c
index 4451d38..fa8a1cb 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -762,7 +762,7 @@ static int service_load_sysv_path(Service *s, const char *path) {
                                                 continue;
 
                                         if (unit_name_to_type(m) == UNIT_SERVICE)
-                                                r = unit_add_name(u, m);
+                                                r = unit_merge_by_name(u, m);
                                         else
                                                 /* NB: SysV targets
                                                  * which are provided
-- 
1.8.1.4

_______________________________________________
systemd-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Reply via email to