On Mon, 27.04.15 18:28, Ivan Shapovalov (intelfx...@gmail.com) wrote: > On 2015-04-27 at 17:14 +0200, Lennart Poettering wrote: > > On Sat, 25.04.15 05:48, Ivan Shapovalov (intelfx...@gmail.com) wrote: > > > > > On 2015-04-25 at 04:00 +0300, Ivan Shapovalov wrote: > > > > On 2015-04-24 at 16:04 +0200, Lennart Poettering wrote: > > > > > [...] > > > > > > > > > > Actually, it really is about the UNIT_TRIGGERS dependencies > > > > > only, > > > > > since we don't do the retroactive deps stuff at all when we are > > > > > coldplugging, it's conditionalized in m->n_reloading <= 0. > > > > > > > > So, I think I understand the problem. We should do this not only > > > > for > > > > UNIT_TRIGGERS, but also for any dependencies which may matter > > > > when activating that unit. That is, anything which is referenced > > > > by > > > > transaction_add_job_and_dependencies()... recursively. > > > > > > Here is what I have in mind. Don't know whether this is correct, > > > but > > > it fixes the problem for me. > > > > > > From 515d878e526e52fc154874e93a4c97555ebd8cff Mon Sep 17 00:00:00 > > > 2001 > > > From: Ivan Shapovalov <intelfx...@gmail.com> > > > Date: Sat, 25 Apr 2015 04:57:59 +0300 > > > Subject: [PATCH] core: coldplug all units which participate in jobs > > > > > > This is yet another attempt to fix coldplugging order (more > > > especially, > > > the problem which happens when one creates a job during > > > coldplugging, and > > > it references a not-yet-coldplugged unit). > > > > > > Now we forcibly coldplug all units which participate in jobs. This > > > is a superset of previously implemented handling of the > > > UNIT_TRIGGERS > > > dependencies, so that handling is removed. > > > --- > > > src/core/transaction.c | 6 ++++++ > > > src/core/unit.c | 8 -------- > > > 2 files changed, 6 insertions(+), 8 deletions(-) > > > > > > diff --git a/src/core/transaction.c b/src/core/transaction.c > > > index 5974b1e..a02c02c 100644 > > > --- a/src/core/transaction.c > > > +++ b/src/core/transaction.c > > > @@ -848,6 +848,12 @@ int transaction_add_job_and_dependencies( > > > assert(type < _JOB_TYPE_MAX_IN_TRANSACTION); > > > assert(unit); > > > > > > + /* Before adding jobs for this unit, let's ensure that > > > its state has been loaded. > > > + * This matters when jobs are spawned as part of > > > coldplugging itself (see. e. g. path_coldplug(). > > > + * This way, we "recursively" coldplug units, ensuring > > > that we do not look at state of > > > + * not-yet-coldplugged units. */ > > > + unit_coldplug(unit); > > > > I like the simplicity of this patch actually, but it's unfortunately > > too simple: coldplugging is to be applied only for services that are > > around at the time we come back from a reload. If you start a service > > during runtime, without any reloading anywhere around, we should not > > coldplug at all. > > > > I figure we need a "coldplugging" bool or so in Manager, which we set > > while coldplugging and can then check here. > > Yeah, right, I've fixed it locally but forgot to send a follow-up mail. > Actually, isn't it "unit->manager->n_reloading > 0"?
Yes, indeed, that should suffice. Lennart -- Lennart Poettering, Red Hat _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
