From owner-svn-src-head@freebsd.org Mon Aug 7 22:27:27 2017 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id E1360DCCE63 for ; Mon, 7 Aug 2017 22:27:27 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-io0-x230.google.com (mail-io0-x230.google.com [IPv6:2607:f8b0:4001:c06::230]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id A43931C7E for ; Mon, 7 Aug 2017 22:27:27 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: by mail-io0-x230.google.com with SMTP id j32so7594733iod.0 for ; Mon, 07 Aug 2017 15:27:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20150623.gappssmtp.com; s=20150623; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=Dj4uVpHicX8f2cJwwuQVH4o8zHcichwoVDCHKZZXRBE=; b=oMx32OaBhmiD9xOyFcwRz1f8ajEl90Lw/Cuq21yzmotJuQYRCLGDS2AjBvBWyaWDis HssXcQ+pHu95p3MXb0Tcc/itbh2n6FcRS8otqEbt1iCgRdKDzzmHl5fv0ayLbRWbhcOO 8my7PLdob4IlBT3Ml5vnBSL3N9z3jtbnTuQ55jhe4xZ3azc/qyhxMekILiMUR67dTHQF 9PrwdA3D3RFbo//j2RiafyXH/P8HN/kBJJy/0pnO8QmUUrvQsPurVphyWkL52fMybNQY UzaOwfpNpOrA/gqicd0JcEIMZK8CWFNvFrN0+OuPKzkwYy3LsxBRQpMOtejkj658/MKp GkMQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=Dj4uVpHicX8f2cJwwuQVH4o8zHcichwoVDCHKZZXRBE=; b=Ttt+sLw5sM2Qu8zC0YlQTsE+5K70S8GaUShtTZCbi5eCQx7Fj/n5RtExsh97wr8B6n c4OW6qupYd+jH8hz6NmveZUDeopjShuFfPucwNK7552N/qDtoC4zZVImGsi0PjVCabAB HvolQccJaSApsvWD6CHYFUw8UKXcrOLg2ppQbUTsOJS9TS1s/cV3JJnadjAW/S3uGDqM Eu9lOjQ+4vzKgk7iqMKMDbWW1w+i5P2dKAadNjmUCs3577yI4QzHz8SLvhXNzhSoRGCy okjPS48wWG9RPweeYVQdbo0+Uf1Bkg6Q6q8OcZvZBb1/KYN74trTp6NMM6oCCQRCf1ha vjIQ== X-Gm-Message-State: AHYfb5jkYYNZ3vdsuDQ2fB9Gd94ys6w4I/3ojqjcyfHpLkbZJ3Gdh5zw oiXX/qvQq1G2EWhhwlblHp+10B/pEpkD X-Received: by 10.107.36.18 with SMTP id k18mr1998886iok.147.1502144847006; Mon, 07 Aug 2017 15:27:27 -0700 (PDT) MIME-Version: 1.0 Sender: wlosh@bsdimp.com Received: by 10.79.10.71 with HTTP; Mon, 7 Aug 2017 15:27:26 -0700 (PDT) X-Originating-IP: [2603:300b:6:5100:f809:2296:13f6:2a42] In-Reply-To: References: <201708072112.v77LCcXm001489@repo.freebsd.org> From: Warner Losh Date: Mon, 7 Aug 2017 16:27:26 -0600 X-Google-Sender-Auth: v2RLNhOy1-V9VJ32s9o_BFXmqFI Message-ID: Subject: Re: svn commit: r322198 - in head: share/man/man9 sys/geom To: Nathan Whitehorn Cc: Warner Losh , src-committers , "svn-src-all@freebsd.org" , "svn-src-head@freebsd.org" Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.23 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 07 Aug 2017 22:27:28 -0000 On Mon, Aug 7, 2017 at 4:20 PM, Nathan Whitehorn wrote: > > > On 08/07/17 14:32, Warner Losh wrote: > > > > On Mon, Aug 7, 2017 at 3:19 PM, Nathan Whitehorn > 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 >>> >>> >> > >