[systemd-devel] Dependency loop due to network filesystem and quotacheck; suboptimal loop resolution.

[Lennart Poettering]

On Thu, 10.03.11 00:42, Andrey Borzenkov (arvidjaar at gmail.com) wrote:

> On Wed, Mar 9, 2011 at 5:12 PM, Andrey Borzenkov <arvidjaar at gmail.com> wrote:
> > https://qa.mandriva.com/show_bug.cgi?id=62746
> >
> > There are two issues in it
> >
> > 1. Dependency on quotacheck are added to network filesystem
> >
> > This is v19. It could be possible that this is fixed in v20.
> >
> 
> It is not. Suggested patch attached.

> From: Andrey Borzenkov <arvidjaar at gmail.com>
> Subject: [PATCH] mount: do not add dependency on network filesystem to quotacheck
> 
> This creates loop:
> 
> fs -> quotacheck -> basic -> network -> fs
> 
> It does not look like quota was enabled for them in /etc/init.d/netfs
> anyway.  If quota is required, it probably should be implemented as
> per mount point unit.

quota unfortunately doesn't work that way, and is really weird anyway
because it is run *after* the fs is mounted, but *before* apps may
access the fs. That means quota is more or less incompatible with being
used on hot-plugged file systems, since modern desktops watch
/proc/self/mountinfo to see what is mounted, and they access it
right-away without waiting for anything else (and right-fully so).

btrfs in the future will not need a quotacheck tool anyway, hence I
think it is fine for now to have this singleton service run once during
boot, and never again. Quota on ext3 is not hotplug-compatible, but I
guess that's something we should just accept.

Anywaym uf you rework this patch to check for the usrquota/grpquota
options I'd merge it promptly (or you can even merge it yourself, if
Tollef grants you git access by then ;-)). Even better would be if you
add a WANTS dep too, so that we can remove the service from being pulled
in by default sysinit.target.

> Fixes https://qa.mandriva.com/show_bug.cgi?id=62746
> 
> ---
>  src/mount.c |   14 ++++++++++++--
>  1 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/src/mount.c b/src/mount.c
> index 0078010..61bbf50 100644
> --- a/src/mount.c
> +++ b/src/mount.c
> @@ -412,9 +412,19 @@ static int mount_add_default_dependencies(Mount *m) {
>  
>          if (m->meta.manager->running_as == MANAGER_SYSTEM &&
>              !path_equal(m->where, "/")) {
> +                MountParameters *p;
>  
> -                if ((r = unit_add_dependency_by_name(UNIT(m), UNIT_BEFORE, SPECIAL_QUOTACHECK_SERVICE, NULL, true)) < 0)
> -                        return r;
> +                if (m->from_fragment)
> +                        p = &m->parameters_fragment;
> +                else if (m->from_etc_fstab)
> +                        p = &m->parameters_etc_fstab;
> +                else
> +                        p = NULL;
> +
> +                if (!p || (!mount_test_option(p->options, "_netdev") &&
> +                    !(p->fstype && fstype_is_network(p->fstype))))
> +                        if ((r = unit_add_dependency_by_name(UNIT(m), UNIT_BEFORE, SPECIAL_QUOTACHECK_SERVICE, NULL, true)) < 0)
> +                                return r;
>  
>                  if ((r = unit_add_two_dependencies_by_name(UNIT(m), UNIT_BEFORE, UNIT_CONFLICTS, SPECIAL_UMOUNT_TARGET, NULL, true)) < 0)
>                          return r;


Lennart

-- 
Lennart Poettering - Red Hat, Inc.