svn commit: r322198 - in head: share/man/man9 sys/geom
Warner Losh
imp at bsdimp.com
Mon Aug 7 22:27:28 UTC 2017
On Mon, Aug 7, 2017 at 4:20 PM, Nathan Whitehorn <nwhitehorn at freebsd.org>
wrote:
>
>
> On 08/07/17 14:32, Warner Losh wrote:
>
>
>
> On Mon, Aug 7, 2017 at 3:19 PM, Nathan Whitehorn <nwhitehorn at freebsd.org>
> wrote:
>
>> It would be really nice to let gpart provide aliases correct to partition
>> labels, which would fix the existing racy and unreliable glabel code. Do
>> you see any obstacles to using this code for that?
>
>
> I'm not sure I understand well enough the issue here to have an opinion.
>
>
> We get /dev/gpt/foo (etc.) right now by parsing the GPT labels with a
> completely parallel piece of code to gpart and then create an extra geom
> provider based on the label. This falls down in four ways:
> 1. The code is racy. It is perfectly possible to get a GPT label set by
> gpart but not have it picked up by glabel for some period of time that may
> be infinite (until a retaste), since the events don't propagate.
> 2. The resulting /dev entry is unpredictable from the label, since glabel
> internally does some fixups related to spaces, etc. You need to copy and
> paste glabel's munging code.
> 3. It isn't implemented for all of the schemes gpart supports since it has
> a reimplementation of the partition table parser.
> 4. Because it uses an extra provider, mounting, say, /dev/adaXpY causes
> /dev/gpt/Z to vanish.
>
> This combination of things is why we don't currently use labels in the
> installer and never have. Having gpart internally create symlinks would fix
> all of this at a stroke. I will take a look at this some more and see how
> hard it would be to implement; at the very least, I think you would also
> need a disk_remove_alias() or the like.
>
I've experienced all but #1 and #2. I've run systems for about a decade
with root mounted on /dev/gpt/HOST-root, usr on /dev/gpt/HOST-usr, etc.
Must have gotten lucky, or not hit the use case that you've seen.
Not sure what's up with #3, but it sounds orthogonal and an incomplete
gpart/glabel integration.
#4 has always bothered me. While device aliases like I've done here would
solve that problem, I'm not sure what other issues there'd be demoting
glabel devices to mere /dev/ nodes from first class geom objects.
The main issue I see is needing to have the aliases in place when tasting
time comes for the geom being tasted. I haven't thought through geom's
sequence enough to know if that would be more robust or not.
Warner
-Nathan
>
>
> Warner
>
> On 08/07/17 14:12, Warner Losh wrote:
>>
>>> Author: imp
>>> Date: Mon Aug 7 21:12:38 2017
>>> New Revision: 322198
>>> URL: https://svnweb.freebsd.org/changeset/base/322198
>>>
>>> Log:
>>> Expose API to allow disks to ask for alias names in devfs.
>>> Implement disk_add_alias to allow aliases to be added to disks. All
>>> disk have a primary name (say "foo") can also have secondary names
>>> (say "bar") such that all instances of "foo" also have a "bar"
>>> alias. So if you have foo0, foo0p1, foo1, foo1s1 and foo1s1a nodes
>>> created by the foo driver and gpart, device nodes bar0, bar0p1, bar1,
>>> bar1s1 and bar1s1a will appear as symlinks back to the original nodes.
>>> This generalizes to multiple aliases. However, since the unit number
>>> follows the primary name, multiple device drivers can't create the
>>> same aliases unless those drives coorinate the unit number space (eg
>>> you couldn't add an alias 'disk' to both 'da' and 'ada' because it's
>>> possible to have da0 and ada0, because 'disk0' is ambiguous).
>>> Differential Revision: https://reviews.freebsd.org/D11873
>>>
>>> Modified:
>>> head/share/man/man9/disk.9
>>> head/sys/geom/geom_disk.c
>>> head/sys/geom/geom_disk.h
>>>
>>> Modified: head/share/man/man9/disk.9
>>> ============================================================
>>> ==================
>>> --- head/share/man/man9/disk.9 Mon Aug 7 21:12:33 2017 (r322197)
>>> +++ head/share/man/man9/disk.9 Mon Aug 7 21:12:38 2017 (r322198)
>>> @@ -27,7 +27,7 @@
>>> .\"
>>> .\" $FreeBSD$
>>> .\"
>>> -.Dd October 30, 2012
>>> +.Dd August 3, 2017
>>> .Dt DISK 9
>>> .Os
>>> .Sh NAME
>>> @@ -45,6 +45,8 @@
>>> .Fn disk_destroy "struct disk *disk"
>>> .Ft int
>>> .Fn disk_resize "struct disk *disk" "int flags"
>>> +.Ft void
>>> +.Fn disk_add_alias "struct disk *disk" "const char *alias"
>>> .Sh DESCRIPTION
>>> The disk storage API permits kernel device drivers providing access to
>>> disk-like storage devices to advertise the device to other kernel
>>> @@ -69,6 +71,20 @@ function,
>>> fill in the fields and call
>>> .Fn disk_create
>>> when the device is ready to service requests.
>>> +.Fn disk_add_alias
>>> +adds an alias for the disk and must be called before
>>> +.Fn disk_create ,
>>> +but may be called multiple times.
>>> +For each alias added, a device node will be created with
>>> +.Xr make_dev_alias 9
>>> +in the same way primary device nodes are created with
>>> +.Xr make_dev 9
>>> +for
>>> +.Va d_name
>>> +and
>>> +.Va d_unit .
>>> +Care should be taken to ensure that only one driver creates aliases
>>> +for any given name.
>>> .Fn disk_resize
>>> can be called by the driver after modifying
>>> .Va d_mediasize
>>> @@ -227,7 +243,13 @@ structure for this disk device.
>>> .El
>>> .Sh SEE ALSO
>>> .Xr GEOM 4 ,
>>> -.Xr devfs 5
>>> +.Xr devfs 5 ,
>>> +.Xr MAKE_DEV 9
>>> .Sh AUTHORS
>>> This manual page was written by
>>> .An Robert Watson .
>>> +.Sh BUGS
>>> +Disk aliases are not a general purpose aliasing mechanism, but are
>>> +intended only to ease the transition from one name to another.
>>> +They can be used to ensure that nvd0 and nda0 are the same thing.
>>> +They cannot be used to implement the diskX concept from macOS.
>>>
>>> Modified: head/sys/geom/geom_disk.c
>>> ============================================================
>>> ==================
>>> --- head/sys/geom/geom_disk.c Mon Aug 7 21:12:33 2017 (r322197)
>>> +++ head/sys/geom/geom_disk.c Mon Aug 7 21:12:38 2017 (r322198)
>>> @@ -676,6 +676,7 @@ g_disk_create(void *arg, int flag)
>>> struct g_provider *pp;
>>> struct disk *dp;
>>> struct g_disk_softc *sc;
>>> + struct disk_alias *dap;
>>> char tmpstr[80];
>>> if (flag == EV_CANCEL)
>>> @@ -704,6 +705,10 @@ g_disk_create(void *arg, int flag)
>>> sc->dp = dp;
>>> gp = g_new_geomf(&g_disk_class, "%s%d", dp->d_name, dp->d_unit);
>>> gp->softc = sc;
>>> + LIST_FOREACH(dap, &dp->d_aliases, da_next) {
>>> + snprintf(tmpstr, sizeof(tmpstr), "%s%d", dap->da_alias,
>>> dp->d_unit);
>>> + g_geom_add_alias(gp, tmpstr);
>>> + }
>>> pp = g_new_providerf(gp, "%s", gp->name);
>>> devstat_remove_entry(pp->stat);
>>> pp->stat = NULL;
>>> @@ -791,6 +796,7 @@ g_disk_destroy(void *ptr, int flag)
>>> struct disk *dp;
>>> struct g_geom *gp;
>>> struct g_disk_softc *sc;
>>> + struct disk_alias *dap, *daptmp;
>>> g_topology_assert();
>>> dp = ptr;
>>> @@ -802,6 +808,8 @@ g_disk_destroy(void *ptr, int flag)
>>> dp->d_geom = NULL;
>>> g_wither_geom(gp, ENXIO);
>>> }
>>> + LIST_FOREACH_SAFE(dap, &dp->d_aliases, da_next, daptmp)
>>> + g_free(dap);
>>> g_free(dp);
>>> }
>>> @@ -834,8 +842,11 @@ g_disk_ident_adjust(char *ident, size_t size)
>>> struct disk *
>>> disk_alloc(void)
>>> {
>>> + struct disk *dp;
>>> - return (g_malloc(sizeof(struct disk), M_WAITOK | M_ZERO));
>>> + dp = g_malloc(sizeof(struct disk), M_WAITOK | M_ZERO);
>>> + LIST_INIT(&dp->d_aliases);
>>> + return (dp);
>>> }
>>> void
>>> @@ -882,6 +893,18 @@ disk_destroy(struct disk *dp)
>>> if (dp->d_devstat != NULL)
>>> devstat_remove_entry(dp->d_devstat);
>>> g_post_event(g_disk_destroy, dp, M_WAITOK, NULL);
>>> +}
>>> +
>>> +void
>>> +disk_add_alias(struct disk *dp, const char *name)
>>> +{
>>> + struct disk_alias *dap;
>>> +
>>> + dap = (struct disk_alias *)g_malloc(
>>> + sizeof(struct disk_alias) + strlen(name) + 1, M_WAITOK);
>>> + strcpy((char *)(dap + 1), name);
>>> + dap->da_alias = (const char *)(dap + 1);
>>> + LIST_INSERT_HEAD(&dp->d_aliases, dap, da_next);
>>> }
>>> void
>>>
>>> Modified: head/sys/geom/geom_disk.h
>>> ============================================================
>>> ==================
>>> --- head/sys/geom/geom_disk.h Mon Aug 7 21:12:33 2017 (r322197)
>>> +++ head/sys/geom/geom_disk.h Mon Aug 7 21:12:38 2017 (r322198)
>>> @@ -66,6 +66,11 @@ typedef enum {
>>> DISK_INIT_DONE
>>> } disk_init_level;
>>> +struct disk_alias {
>>> + LIST_ENTRY(disk_alias) da_next;
>>> + const char *da_alias;
>>> +};
>>> +
>>> struct disk {
>>> /* Fields which are private to geom_disk */
>>> struct g_geom *d_geom;
>>> @@ -109,6 +114,9 @@ struct disk {
>>> /* Fields private to the driver */
>>> void *d_drv1;
>>> +
>>> + /* Fields private to geom_disk, to be moved on next version bump
>>> */
>>> + LIST_HEAD(,disk_alias) d_aliases;
>>> };
>>> #define DISKFLAG_RESERVED 0x1 /* Was NEEDSGIANT */
>>> @@ -132,6 +140,7 @@ void disk_attr_changed(struct disk *dp, const char
>>> *at
>>> void disk_media_changed(struct disk *dp, int flag);
>>> void disk_media_gone(struct disk *dp, int flag);
>>> int disk_resize(struct disk *dp, int flag);
>>> +void disk_add_alias(struct disk *disk, const char *);
>>> #define DISK_VERSION_00 0x58561059
>>> #define DISK_VERSION_01 0x5856105a
>>>
>>>
>>
>
>
More information about the svn-src-all
mailing list