Re: geom labels as aliases

https://lists.freebsd.org/pipermail/freebsd-current/2020-June/076210.html


On Wed, Sep 29, 2021 at 07:20 Warner Losh <imp@bsdimp.com> wrote:

>
>
> On Wed, Sep 29, 2021 at 7:36 AM Andriy Gapon <avg@freebsd.org> wrote:
>
>> On 29/09/2021 16:20, Warner Losh wrote:
>> >
>> >
>> > On Wed, Sep 29, 2021 at 2:37 AM Andriy Gapon <avg@freebsd.org
>> > <mailto:avg@freebsd.org>> wrote:
>> >
>> >
>> >     For an introduction, I never liked consequences of GEOM labels being
>> >     implemented
>> >     as geoms.  And I specifically do not mean explicitly created labels
>> via glabel
>> >     create / label.  I mean GEOM labels based on partition and
>> filesystem labels,
>> >     disk IDs, etc,  And I don't like things such as opening a device
>> via one label
>> >     spoiling other labels.
>> >
>> >     So, I've been reading through some recent-ish changes by Warner for
>> GEOM
>> >     aliases
>> >     and I've got this (maybe not so) bright idea, what if those labels
>> were
>> >     implemented as aliases.  Would that work?  Would that require a lot
>> of work?
>> >
>> >     I did a quick hack as a proof of concept.
>> >     The change is quite small.
>> >
>> >     diff --git a/sys/geom/label/g_label.c b/sys/geom/label/g_label.c
>> >     index 1df7e799b014..72b97d314de4 100644
>> >     --- a/sys/geom/label/g_label.c
>> >     +++ b/sys/geom/label/g_label.c
>> >     @@ -418,12 +418,15 @@ g_label_taste(struct g_class *mp, struct
>> g_provider *pp,
>> >     int flags __unused)
>> >                      if (label[0] == '\0')
>> >                              continue;
>> >                      if (g_labels[i] != &g_label_generic) {
>> >     -                       mediasize = pp->mediasize;
>> >     +                       if (g_label_is_name_ok(label)) {
>> >     +                               g_provider_add_alias(pp, "%s%s",
>> >     +                                   g_labels[i]->ld_dirprefix,
>> label);
>> >     +                       }
>> >                      } else {
>> >                              mediasize = pp->mediasize - pp->sectorsize;
>> >     +                       g_label_create(NULL, mp, pp, label,
>> >     +                           g_labels[i]->ld_dirprefix, mediasize);
>> >                      }
>> >     -               g_label_create(NULL, mp, pp, label,
>> >     -                   g_labels[i]->ld_dirprefix, mediasize);
>> >              }
>> >              g_access(cp, -1, 0, 0);
>> >        end:
>> >
>> >     This seems to work in a basic smoke test of just booting up with
>> the change.
>> [snip]
>> >
>> >     Of course, the change can break existing scripts / tools that count
>> on labels
>> >     having their own geoms.
>> >
>> >     Also, it can be argued that it would be better to create such
>> symbolic links in
>> >     userland.  It should be easy to port the label tasting code to
>> userland and
>> >     hook
>> >     it to devd events.  That should cover all use cases except for the
>> root
>> >     filesystem.
>> >
>> >     Finally, I haven't tested glabel create / label yet, so the change
>> may need
>> >     some
>> >     more work in that area.
>> >
>> >     What is your opinion?
>> >     Is this something worth pursuing?
>> >
>> >
>> > Conrad did something similar in 5b9b571cb and wound up reverting it.
>> Too many
>> > unforeseen issues, IIRC.
>>
>> Whoosh, I completely missed that commit.
>> Unfortunately, the revert message it too terse:
>>
>>      Revert r361838
>>
>>      Reported by:    delphij
>>
>> I wonder if the reported problem was indeed too hard to overcome.
>>
>> However, I already see a number of issues in my simplistic change.
>> First of all, removing obsolete aliases.
>>
>> I am thinking that maybe each "alias label" should still have a geom
>> attached to
>> the corresponding provider but without a (downstream) provider of its
>> own.  This
>> way we can still receive events for spoiling, etc without introducing an
>> alternative provider.  We then can manipulate aliases in response to the
>> upstream changes.
>>
>
> Aliases are automatically removed when the providing geom goes away,
> which as part of the tasting process automatically happens.
>
> I too wish the commit log was more complete, or had a pointer
> to the exact details.
>
> Warner
>
>
>