master - systemd: depend on systemd-udev-settle unit in activation unit

master - systemd: depend on systemd-udev-settle unit in activation unit

[Peter Rajnoha]

Gitweb:        http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=6d75ff138cb1614ee5e31bbd0c3c25721ff9514c
Commit:        6d75ff138cb1614ee5e31bbd0c3c25721ff9514c
Parent:        31271606265ec8f7c14734aedbb0a314dfea2cd1
Author:        Peter Rajnoha <prajnoha@redhat.com>
AuthorDate:    Wed Sep 12 11:30:13 2012 +0200
Committer:     Peter Rajnoha <prajnoha@redhat.com>
CommitterDate: Wed Sep 12 11:30:13 2012 +0200

systemd: depend on systemd-udev-settle unit in activation unit

The "fedora-wait-storage.service" that the "lvm2-activation.service"
had as a dependency (which was fedora-specific solution anyway)
is obsolete now as this unit called "modprobe scsi_wait_scan"
which is not used anymore.

The "fedora-wait-storage.service" had "systemd-udev-settle" as
its dependency, so let's depend on this one directly now,
bypassing the out-dated "fedora-wait-storage.service".
---
 WHATS_NEW                                          |    1 +
 .../lvm2_activation_generator_systemd_red_hat.c    |    4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/WHATS_NEW b/WHATS_NEW
index 4884626..2b897e8 100644
--- a/WHATS_NEW
+++ b/WHATS_NEW
@@ -1,5 +1,6 @@
 Version 2.02.98 -
 =================================
+  Depend on systemd-udev-settle in units generated by activation generator.
   Fix vgchange -aay to activate proper logical volumes.
   Properly handle 'resync' of RAID LVs.
   Disallow addition of RAID images until the array is in-sync.
diff --git a/scripts/lvm2_activation_generator_systemd_red_hat.c b/scripts/lvm2_activation_generator_systemd_red_hat.c
index 9fcee20..786d795 100644
--- a/scripts/lvm2_activation_generator_systemd_red_hat.c
+++ b/scripts/lvm2_activation_generator_systemd_red_hat.c
@@ -116,12 +116,12 @@ static int generate_unit(const char *dir, int early)
 	      "DefaultDependencies=no\n", f);
 
 	if (early)
-		fputs("After=fedora-wait-storage.service\n", f);
+		fputs("After=systemd-udev-settle.service\n", f);
 	else
 		fputs("After=lvm2-activation-early.service cryptsetup.target\n", f);
 
 	fputs("Before=local-fs.target shutdown.target\n"
-	      "Wants=fedora-wait-storage.service\n\n"
+	      "Wants=systemd-udev-settle.service\n\n"
 	      "[Service]\n"
 	      "ExecStart=/usr/sbin/lvm vgchange -aay --sysinit\n"
 	      "Type=oneshot\n", f);

master - systemd: depend on systemd-udev-settle unit in activation unit

[Zdenek Kabelac]

Dne 12.9.2012 11:37, Peter Rajnoha napsal(a):
> Gitweb:        http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=6d75ff138cb1614ee5e31bbd0c3c25721ff9514c
> Commit:        6d75ff138cb1614ee5e31bbd0c3c25721ff9514c
> Parent:        31271606265ec8f7c14734aedbb0a314dfea2cd1
> Author:        Peter Rajnoha <prajnoha@redhat.com>
> AuthorDate:    Wed Sep 12 11:30:13 2012 +0200
> Committer:     Peter Rajnoha <prajnoha@redhat.com>
> CommitterDate: Wed Sep 12 11:30:13 2012 +0200
>
> systemd: depend on systemd-udev-settle unit in activation unit
>
> The "fedora-wait-storage.service" that the "lvm2-activation.service"
> had as a dependency (which was fedora-specific solution anyway)
> is obsolete now as this unit called "modprobe scsi_wait_scan"
> which is not used anymore.
>
> The "fedora-wait-storage.service" had "systemd-udev-settle" as
> its dependency, so let's depend on this one directly now,
> bypassing the out-dated "fedora-wait-storage.service".
> ---
>   WHATS_NEW                                          |    1 +
>   .../lvm2_activation_generator_systemd_red_hat.c    |    4 ++--
>   2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/WHATS_NEW b/WHATS_NEW
> index 4884626..2b897e8 100644
> --- a/WHATS_NEW
> +++ b/WHATS_NEW
> @@ -1,5 +1,6 @@
>   Version 2.02.98 -
>   =================================
> +  Depend on systemd-udev-settle in units generated by activation generator.
>     Fix vgchange -aay to activate proper logical volumes.
>     Properly handle 'resync' of RAID LVs.
>     Disallow addition of RAID images until the array is in-sync.
> diff --git a/scripts/lvm2_activation_generator_systemd_red_hat.c b/scripts/lvm2_activation_generator_systemd_red_hat.c
> index 9fcee20..786d795 100644
> --- a/scripts/lvm2_activation_generator_systemd_red_hat.c
> +++ b/scripts/lvm2_activation_generator_systemd_red_hat.c
> @@ -116,12 +116,12 @@ static int generate_unit(const char *dir, int early)
>   	      "DefaultDependencies=no\n", f);
>
>   	if (early)
> -		fputs("After=fedora-wait-storage.service\n", f);
> +		fputs("After=systemd-udev-settle.service\n", f);
>   	else
>   		fputs("After=lvm2-activation-early.service cryptsetup.target\n", f);
>
>   	fputs("Before=local-fs.target shutdown.target\n"
> -	      "Wants=fedora-wait-storage.service\n\n"
> +	      "Wants=systemd-udev-settle.service\n\n"
>   	      "[Service]\n"
>   	      "ExecStart=/usr/sbin/lvm vgchange -aay --sysinit\n"
>   	      "Type=oneshot\n", f);
>


I think we need here some backward compatible cross-distro solution.

Zdenek

master - systemd: depend on systemd-udev-settle unit in activation unit

[Peter Rajnoha]

On 09/12/2012 01:11 PM, Zdenek Kabelac wrote:
>> diff --git a/scripts/lvm2_activation_generator_systemd_red_hat.c b/scripts/lvm2_activation_generator_systemd_red_hat.c
>> index 9fcee20..786d795 100644
>> --- a/scripts/lvm2_activation_generator_systemd_red_hat.c
>> +++ b/scripts/lvm2_activation_generator_systemd_red_hat.c
>> @@ -116,12 +116,12 @@ static int generate_unit(const char *dir, int early)
>>             "DefaultDependencies=no\n", f);
>>
>>       if (early)
>> -        fputs("After=fedora-wait-storage.service\n", f);
>> +        fputs("After=systemd-udev-settle.service\n", f);
>>       else
>>           fputs("After=lvm2-activation-early.service cryptsetup.target\n", f);
>>
>>       fputs("Before=local-fs.target shutdown.target\n"
>> -          "Wants=fedora-wait-storage.service\n\n"
>> +          "Wants=systemd-udev-settle.service\n\n"
>>             "[Service]\n"
>>             "ExecStart=/usr/sbin/lvm vgchange -aay --sysinit\n"
>>             "Type=oneshot\n", f);
>>
> 
> 
> I think we need here some backward compatible cross-distro solution.
> 

The fedora-wait-storage.service itself was not quite perfect as
it was only Fedora specific (therefore it's named
lvm2_activation_generator_systemd_red_hat.c, having "red hat" in
its name).

The fedora-wait-storage just called "modprobe scsi_wait_scan" which
is not functional anymore as the module is gone now in recent kernels.

The dependency on systemd-udev-settle is far better and more
cross-distro solution as we're actually waiting on udev to be settled.

The thing with scsi_wait_scan was only added in the way as
an addendum to udev-settle functionality (as the next unit
that is executed just after udev-settle).

If scsi_wait_scan stayed, then we'd better ask systemd
upstream to include the unit with scsi_wait_scan there
as an official unit distributed with systemd (but I think
they counted with its removal so they did not bother to
include it anymore). I haven't inspected where other
distros called this scsi_wait_scan in their bootup process,
if it was called at all there.

As for Fedora, F17 still uses fedora-wait-storage unit
(having the modprobe scsi_wait_scan call), F18 and higher
still has this unit, but it's failing as the module is not
available anymore (the unit will be be removed soon I guess).

I think that for upstream solution, it's far better to depend
on systemd-udev-settle.service. If there's anything else to
wait for besides udev, it should be included in systemd upstream
as an official and extra unit (so other distros would take that
from systemd upstream when packaging).

Peter

master - systemd: depend on systemd-udev-settle unit in activation unit

[Bill Nottingham]

Peter Rajnoha (prajnoha at redhat.com) said: 
> On 09/12/2012 01:11 PM, Zdenek Kabelac wrote:
> >> diff --git a/scripts/lvm2_activation_generator_systemd_red_hat.c b/scripts/lvm2_activation_generator_systemd_red_hat.c
> >> index 9fcee20..786d795 100644
> >> --- a/scripts/lvm2_activation_generator_systemd_red_hat.c
> >> +++ b/scripts/lvm2_activation_generator_systemd_red_hat.c
> >> @@ -116,12 +116,12 @@ static int generate_unit(const char *dir, int early)
> >>             "DefaultDependencies=no\n", f);
> >>
> >>       if (early)
> >> -        fputs("After=fedora-wait-storage.service\n", f);
> >> +        fputs("After=systemd-udev-settle.service\n", f);
> >>       else
> >>           fputs("After=lvm2-activation-early.service cryptsetup.target\n", f);
> >>
> >>       fputs("Before=local-fs.target shutdown.target\n"
> >> -          "Wants=fedora-wait-storage.service\n\n"
> >> +          "Wants=systemd-udev-settle.service\n\n"
> >>             "[Service]\n"
> >>             "ExecStart=/usr/sbin/lvm vgchange -aay --sysinit\n"
> >>             "Type=oneshot\n", f);
> >>
> > 
> > 
> > I think we need here some backward compatible cross-distro solution.
> > 
> 
> The fedora-wait-storage.service itself was not quite perfect as
> it was only Fedora specific (therefore it's named
> lvm2_activation_generator_systemd_red_hat.c, having "red hat" in
> its name).
> 
> The fedora-wait-storage just called "modprobe scsi_wait_scan" which
> is not functional anymore as the module is gone now in recent kernels.
> 
> The dependency on systemd-udev-settle is far better and more
> cross-distro solution as we're actually waiting on udev to be settled.
> 
> The thing with scsi_wait_scan was only added in the way as
> an addendum to udev-settle functionality (as the next unit
> that is executed just after udev-settle).
> 
> If scsi_wait_scan stayed, then we'd better ask systemd
> upstream to include the unit with scsi_wait_scan there
> as an official unit distributed with systemd (but I think
> they counted with its removal so they did not bother to
> include it anymore). I haven't inspected where other
> distros called this scsi_wait_scan in their bootup process,
> if it was called at all there.
> 
> As for Fedora, F17 still uses fedora-wait-storage unit
> (having the modprobe scsi_wait_scan call), F18 and higher
> still has this unit, but it's failing as the module is not
> available anymore (the unit will be be removed soon I guess).
> 
> I think that for upstream solution, it's far better to depend
> on systemd-udev-settle.service. If there's anything else to
> wait for besides udev, it should be included in systemd upstream
> as an official and extra unit (so other distros would take that
> from systemd upstream when packaging).

Without scsi-wait-scan, I'm not sure that depending on udev-settle
will actually gain you much here - udev will have settled while
background scsi scans are still happening.

Bill

master - systemd: depend on systemd-udev-settle unit in activation unit

[Peter Rajnoha]

On 09/12/2012 05:19 PM, Bill Nottingham wrote:
>> The thing with scsi_wait_scan was only added in the way as
>> an addendum to udev-settle functionality (as the next unit
>> that is executed just after udev-settle).
>>
>> If scsi_wait_scan stayed, then we'd better ask systemd
>> upstream to include the unit with scsi_wait_scan there
>> as an official unit distributed with systemd (but I think
>> they counted with its removal so they did not bother to
>> include it anymore). I haven't inspected where other
>> distros called this scsi_wait_scan in their bootup process,
>> if it was called at all there.
>>
>> As for Fedora, F17 still uses fedora-wait-storage unit
>> (having the modprobe scsi_wait_scan call), F18 and higher
>> still has this unit, but it's failing as the module is not
>> available anymore (the unit will be be removed soon I guess).
>>
>> I think that for upstream solution, it's far better to depend
>> on systemd-udev-settle.service. If there's anything else to
>> wait for besides udev, it should be included in systemd upstream
>> as an official and extra unit (so other distros would take that
>> from systemd upstream when packaging).
> 
> Without scsi-wait-scan, I'm not sure that depending on udev-settle
> will actually gain you much here - udev will have settled while
> background scsi scans are still happening.
> 

Yes, you're right... for older systems where this module is still
present, we should still use the scsi-wait-scan.

However, this is now (at least in Fedora) maintained under
our own initscripts which is not present in all distros I guess.
I'm just trying to point out that it would be better to have
this "fedora-storage-wait" unit in systemd/udev upstream directly
and give it a common name like "storage-wait" which other
storage-related upstream projects could use for reference in
their systemd units if we want to keep it for backward compatibility.

But since the module is now gone, I'm inclined to using systemd-udev-settle
instead and if an update/rebase is needed in packages in older
distro releases, just providing a simple one line patch attached
directly to the build (making use of the original fedora-storage-wait.service).

BTW, just curious, shouldn't we call udev-settle even after
modprobe scsi_wait_scan as well?

Peter

master - systemd: depend on systemd-udev-settle unit in activation unit

[Bill Nottingham]

Peter Rajnoha (prajnoha at redhat.com) said: 
> Yes, you're right... for older systems where this module is still
> present, we should still use the scsi-wait-scan.
> 
> However, this is now (at least in Fedora) maintained under
> our own initscripts which is not present in all distros I guess.
> I'm just trying to point out that it would be better to have
> this "fedora-storage-wait" unit in systemd/udev upstream directly
> and give it a common name like "storage-wait" which other
> storage-related upstream projects could use for reference in
> their systemd units if we want to keep it for backward compatibility.
> 
> But since the module is now gone, I'm inclined to using systemd-udev-settle
> instead and if an update/rebase is needed in packages in older
> distro releases, just providing a simple one line patch attached
> directly to the build (making use of the original fedora-storage-wait.service).

The problem with the module being gone (as I understand it) is
that systemd-udev-settle doesn't replace it. Before, the sequence was:

- udev starts
- udev loads stuff
- udev-settle, waits until all the loading of stuff is processed
- scsi_wait_scan loads
- ... waits for background scans to finish ...
- can continue with LVM or whatever activation

The removal of scsi_wait_scan doesn't actually mean any of the bus scans are
done when you wait on systemd-udev-settle, so making that change won't
really solve the issue you need. If I'm understanding it right, I think
in a world without scsi-wait-scan, you're likely going to need to enable
lvmetad and do incremental activation to have thigns work properly.

Bill

master - systemd: depend on systemd-udev-settle unit in activation unit

[Peter Rajnoha]

On 09/13/2012 08:12 PM, Bill Nottingham wrote:
> The problem with the module being gone (as I understand it) is
> that systemd-udev-settle doesn't replace it. Before, the sequence was:
> 
> - udev starts
> - udev loads stuff
> - udev-settle, waits until all the loading of stuff is processed
> - scsi_wait_scan loads
> - ... waits for background scans to finish ...

I assume there are still udev events generated as these devices are being
discovered. So even with scsi_wait_scan, we could end with scsi_wait_scan
done, but events still being processed by udev in userspace(?).
Shouldn't there be this sequence:
    - udev starts
    - udev loads stuff
    - udev-settle
    - scsi_wait_scan
 ---> udev-settle <---

> - can continue with LVM or whatever activation
> 
> The removal of scsi_wait_scan doesn't actually mean any of the bus scans are
> done when you wait on systemd-udev-settle, so making that change won't
> really solve the issue you need. If I'm understanding it right, I think

...sure, udev-settle *does not* solve the background scsi scan.

> in a world without scsi-wait-scan, you're likely going to need to enable
> lvmetad and do incremental activation to have thigns work properly.
> 

Yes, we're hardening lvmetad at the moment to make it enabled by
default and use it as a primary source of metadata information,
where the initial scan done on the devices is based on udev events
that update the lvmetad itself. So LVM is finally going to be fully
event-based.

That's exactly where we're heading actually...

Peter