do not spoil a geom label if it can not be modified via its underlying provider
AbandonedPublic

Authored by avg on Sep 14 2017, 12:01 PM.

Details

Reviewers
ae
imp
marcel
trasz
Summary

Examples:

  • disk id can not be modified at all
  • GPT partition ID or label can no be modified within the partition itself, they can be modified only via the table where the partition is defined
  • ephemeral labels (glabel create) can not be modified by modifying disk content

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 11562
Build 11915: arc lint + arc unit
avg created this revision.Sep 14 2017, 12:01 PM
ae added a comment.Sep 14 2017, 1:38 PM

I'm not sure about GPT volume labels. What will be if you do something like this?

MD=`mdconfig -s 100m`
gpart create -s GPT $MD
gpart add -t freebsd-ufs $MD
for i in `seq 0 100`; do
    gpart modify -i 0 -l LABEL$i $MD
done
avg added a comment.Sep 14 2017, 4:14 PM

@ae initially I was a little bit confused about your example, but now I see what you mean.
It seems that "GEOM::media" attribute change is posted for consumers that do not get spoiled.
Maybe the g_label code can make use of that fact.
Here is a very rough change that seems to do the job:

static void
g_label_attrchanged(struct g_consumer *cp, const char *attr)
{
        struct g_class *mp;
        struct g_provider *pp;

        if (strcmp(attr, "GEOM::media") != 0)
                return;
        pp = cp->provider;
        mp = cp->geom->class;
        g_detach(cp);
        g_slice_spoiled(cp);
        mp->taste(mp, pp, 0);
}

But I wonder if there is something smarter and "gentler" that the code could do to handle that kind of change.

I feel that using GEOM::media as a way to signal a label change is missing the more fundamental problem: the code change is based on the assumption that labels can't change, which ae@ showed is false.

It's best not to declare the GPT partition label as a persistent label. The GPT partition UUID can be assumed to be persistent.

avg added a comment.Sep 14 2017, 5:22 PM

@marcel I never intended for "persistent" to mean "can never change". I only meant it to mean that, using Andrey's example, opening md0p1 for writing can not result in its label changing.
However, I did miss the fact that gpart control interface operates by explicitly spoiling consumers attached to modified partitions. I thought that the partitions themselves were spoiled and the consumers became orphaned.
So, this was a hole in my original design that Andrey pointed out.
But I think that it does not invalidate the general idea.
It's unfortunate that both the opening of md0p1 for writing and the re-labeling of md0p1 use the same mechanism, the spoiling, to convey the actions to consumers.

But in any case it seems that I got more than one thing very wrong and the current patch does not do what I intended for it to do.
So, it is not ready for a review yet...

avg added a comment.Sep 22 2017, 1:57 PM

I see two main problems to implementing what I wanted:

  • spoil can be called for different reasons (e.g., a provider is opened for writing via another consumer vs. some properties of the provider are modified) and there is no way of telling them apart
  • g_label can attach several consumers, each with its own geom, to the same provider and GEOM does not call taste method of a class if a geom of that class is already attached to a provider
    • this makes it impossible to taste for new labels if a label is already attached

I do not see any solution or easy way around these issues, so I am abandoning the whole idea for now.

avg abandoned this revision.Sep 22 2017, 1:57 PM