From owner-freebsd-geom@FreeBSD.ORG Mon Aug 31 11:07:07 2009 Return-Path: Delivered-To: freebsd-geom@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 02795106568F for ; Mon, 31 Aug 2009 11:07:07 +0000 (UTC) (envelope-from owner-bugmaster@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id DBFFF8FC2E for ; Mon, 31 Aug 2009 11:07:06 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.3/8.14.3) with ESMTP id n7VB76Yo070563 for ; Mon, 31 Aug 2009 11:07:06 GMT (envelope-from owner-bugmaster@FreeBSD.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.3/8.14.3/Submit) id n7VB763W070559 for freebsd-geom@FreeBSD.org; Mon, 31 Aug 2009 11:07:06 GMT (envelope-from owner-bugmaster@FreeBSD.org) Date: Mon, 31 Aug 2009 11:07:06 GMT Message-Id: <200908311107.n7VB763W070559@freefall.freebsd.org> X-Authentication-Warning: freefall.freebsd.org: gnats set sender to owner-bugmaster@FreeBSD.org using -f From: FreeBSD bugmaster To: freebsd-geom@FreeBSD.org Cc: Subject: Current problem reports assigned to freebsd-geom@FreeBSD.org X-BeenThere: freebsd-geom@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: GEOM-specific discussions and implementations List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 31 Aug 2009 11:07:07 -0000 Note: to view an individual PR, use: http://www.freebsd.org/cgi/query-pr.cgi?pr=(number). The following is a listing of current problems submitted by FreeBSD users. These represent problem reports covering all versions including experimental development code and obsolete releases. S Tracker Resp. Description -------------------------------------------------------------------------------- o kern/135898 geom [geom] Severe filesystem corruption - large files or l o kern/134922 geom [gmirror] [panic] kernel panic when use fdisk on disk o kern/134113 geom [geli] Problem setting secondary GELI key o kern/134044 geom [geom] gmirror(8) overwrites fs with stale data from r o kern/133931 geom [geli] [request] intentionally wrong password to destr o bin/132845 geom [geom] [patch] ggated(8) does not close files opened a o kern/132273 geom glabel(8): [patch] failing on journaled partition f kern/132242 geom [gmirror] gmirror.ko fails to fully initialize o kern/131353 geom [geom] gjournal(8) kernel lock p docs/130548 geom [patch] gjournal(8) man page is missing sysctls o kern/129674 geom [geom] gjournal root did not mount on boot o kern/129645 geom gjournal(8): GEOM_JOURNAL causes system to fail to boo o kern/129245 geom [geom] gcache is more suitable for suffix based provid f kern/128276 geom [gmirror] machine lock up when gmirror module is used f kern/126902 geom [geom] geom_label: kernel panic during install boot o kern/124973 geom [gjournal] [patch] boot order affects geom_journal con o kern/124969 geom gvinum(8): gvinum raid5 plex does not detect missing s f kern/124294 geom [geom] gmirror(8) have inappropriate logic when workin o kern/123962 geom [panic] [gjournal] gjournal (455Gb data, 8Gb journal), o kern/123630 geom [patch] [gmirror] gmirror doesnt allow the original dr o kern/123122 geom [geom] GEOM / gjournal kernel lock o kern/122738 geom [geom] gmirror list "losts consumers" after gmirror de f kern/122415 geom [geom] UFS labels are being constantly created and rem o kern/122067 geom [geom] [panic] Geom crashed during boot o kern/121559 geom [patch] [geom] geom label class allows to create inacc o kern/121364 geom [gmirror] Removing all providers create a "zombie" mir o kern/120091 geom [geom] [geli] [gjournal] geli does not prompt for pass o kern/120021 geom [geom] [panic] net-p2p/qbittorrent crashes system when o kern/119743 geom [geom] geom label for cds is keeped after dismount and o kern/115856 geom [geli] ZFS thought it was degraded when it should have o kern/115547 geom [geom] [patch] [request] let GEOM Eli get password fro o kern/114532 geom [geom] GEOM_MIRROR shows up in kldstat even if compile o kern/113957 geom [gmirror] gmirror is intermittently reporting a degrad o kern/113885 geom [gmirror] [patch] improved gmirror balance algorithm o kern/113837 geom [geom] unable to access 1024 sector size storage o kern/113419 geom [geom] geom fox multipathing not failing back p bin/110705 geom gmirror(8) control utility does not exit with correct o kern/107707 geom [geom] [patch] [request] add new class geom_xbox360 to o kern/104389 geom [geom] [patch] sys/geom/geom_dump.c doesn't encode XML o kern/98034 geom [geom] dereference of NULL pointer in acd_geom_detach o kern/94632 geom [geom] Kernel output resets input while GELI asks for o kern/90582 geom [geom] [panic] Restore cause panic string (ffs_blkfree o bin/90093 geom fdisk(8) incapable of altering in-core geometry a kern/89660 geom [vinum] [patch] [panic] due to g_malloc returning null o kern/89546 geom [geom] GEOM error o kern/88601 geom [geli] geli cause kernel panic under heavy disk usage o kern/87544 geom [gbde] mmaping large files on a gbde filesystem deadlo o kern/84556 geom [geom] [panic] GBDE-encrypted swap causes panic at shu o kern/79251 geom [2TB] newfs fails on 2.6TB gbde device o kern/79035 geom [vinum] gvinum unable to create a striped set of mirro o bin/78131 geom gbde(8) "destroy" not working. s kern/73177 geom kldload geom_* causes panic due to memory exhaustion 52 problems total. From owner-freebsd-geom@FreeBSD.ORG Thu Sep 3 13:57:45 2009 Return-Path: Delivered-To: freebsd-geom@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 9C27D1065676; Thu, 3 Sep 2009 13:57:45 +0000 (UTC) (envelope-from pjd@garage.freebsd.pl) Received: from mail.garage.freebsd.pl (chello087206049004.chello.pl [87.206.49.4]) by mx1.freebsd.org (Postfix) with ESMTP id DBDA98FC29; Thu, 3 Sep 2009 13:57:44 +0000 (UTC) Received: by mail.garage.freebsd.pl (Postfix, from userid 65534) id 033CF45E48; Thu, 3 Sep 2009 15:57:42 +0200 (CEST) Received: from localhost (pdawidek.wheel.pl [10.0.1.1]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.garage.freebsd.pl (Postfix) with ESMTP id 199DB45C8A; Thu, 3 Sep 2009 15:57:38 +0200 (CEST) Date: Thu, 3 Sep 2009 15:57:41 +0200 From: Pawel Jakub Dawidek To: Mel Flynn Message-ID: <20090903135741.GK1821@garage.freebsd.pl> References: <200909030000.11961.mel.flynn+fbsd.fs@mailing.thruhere.net> <20090903124407.GJ1821@garage.freebsd.pl> <200909031548.37887.mel.flynn+fbsd.fs@mailing.thruhere.net> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="yr6OvWOSyJed8q4v" Content-Disposition: inline In-Reply-To: <200909031548.37887.mel.flynn+fbsd.fs@mailing.thruhere.net> User-Agent: Mutt/1.4.2.3i X-PGP-Key-URL: http://people.freebsd.org/~pjd/pjd.asc X-OS: FreeBSD 8.0-CURRENT i386 X-Spam-Checker-Version: SpamAssassin 3.0.4 (2005-06-05) on mail.garage.freebsd.pl X-Spam-Level: X-Spam-Status: No, score=-5.9 required=4.5 tests=ALL_TRUSTED,BAYES_00 autolearn=ham version=3.0.4 Cc: freebsd-fs@freebsd.org, freebsd-geom@freebsd.org Subject: Re: Patch to allow gmirror to set priority of a disk X-BeenThere: freebsd-geom@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: GEOM-specific discussions and implementations List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 03 Sep 2009 13:57:45 -0000 --yr6OvWOSyJed8q4v Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Sep 03, 2009 at 03:48:37PM +0200, Mel Flynn wrote: > On Thursday 03 September 2009 14:44:07 Pawel Jakub Dawidek wrote: > > I'd suggest doing this not as separate gmirror(8) subcommand, but as an > > extension to 'configure' subcommand, where one can provide priority by > > giving '-p' argument. >=20 > Except I didn't see how configure was implemented. Am I correct that this= is=20 > g_mirror_ctl_configure in sys/geom/mirror/g_mirror_ctl.c? Yes, you are correct. > On a related note, perhaps the attached can be applied so that there's no= =20 > question about the priority numbering? > --=20 > Mel > Index: sbin/geom/class/mirror/gmirror.8 > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- sbin/geom/class/mirror/gmirror.8 (revision 196776) > +++ sbin/geom/class/mirror/gmirror.8 (working copy) > @@ -115,8 +115,8 @@ > .It Cm label > Create a mirror. > The order of components is important, because a component's priority is = based on its position > -(starting from 0). > -The component with the biggest priority is used by the > +(starting from 0 to 255). > +The component with the biggest priority (the lowest number) is used by t= he > .Cm prefer > balance algorithm > and is also used as a master component when resynchronization is needed, Looks good. --=20 Pawel Jakub Dawidek http://www.wheel.pl pjd@FreeBSD.org http://www.FreeBSD.org FreeBSD committer Am I Evil? Yes, I Am! --yr6OvWOSyJed8q4v Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.4 (FreeBSD) iD8DBQFKn8tVForvXbEpPzQRApcpAJ9rfHa7mNQ7hx64QgBGc+ZCkqwcyACg9H1e 20cxVc5Q9+M+jC9EVlG5mM4= =s1QF -----END PGP SIGNATURE----- --yr6OvWOSyJed8q4v-- From owner-freebsd-geom@FreeBSD.ORG Fri Sep 4 08:32:16 2009 Return-Path: Delivered-To: freebsd-geom@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 9DCCE106566C for ; Fri, 4 Sep 2009 08:32:16 +0000 (UTC) (envelope-from mel.flynn+fbsd.fs@mailing.thruhere.net) Received: from mailhub.rachie.is-a-geek.net (rachie.is-a-geek.net [66.230.99.27]) by mx1.freebsd.org (Postfix) with ESMTP id 595A18FC15 for ; Fri, 4 Sep 2009 08:32:15 +0000 (UTC) Received: from smoochies.rachie.is-a-geek.net (mailhub.rachie.is-a-geek.net [192.168.2.11]) by mailhub.rachie.is-a-geek.net (Postfix) with ESMTP id 5E57D7E853; Fri, 4 Sep 2009 00:17:06 -0800 (AKDT) From: Mel Flynn To: freebsd-fs@freebsd.org Date: Fri, 4 Sep 2009 10:16:34 +0200 User-Agent: KMail/1.11.4 (FreeBSD/8.0-BETA3; KDE/4.2.4; i386; ; ) References: <200909030000.11961.mel.flynn+fbsd.fs@mailing.thruhere.net> <200909031548.37887.mel.flynn+fbsd.fs@mailing.thruhere.net> <20090903135741.GK1821@garage.freebsd.pl> In-Reply-To: <20090903135741.GK1821@garage.freebsd.pl> MIME-Version: 1.0 Content-Type: Multipart/Mixed; boundary="Boundary-00=_izMoKZ58GgHWEiB" Message-Id: <200909041016.34677.mel.flynn+fbsd.fs@mailing.thruhere.net> Cc: Pawel Jakub Dawidek , freebsd-geom@freebsd.org Subject: Re: Patch to allow gmirror to set priority of a disk X-BeenThere: freebsd-geom@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: GEOM-specific discussions and implementations List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 04 Sep 2009 08:32:16 -0000 --Boundary-00=_izMoKZ58GgHWEiB Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline On Thursday 03 September 2009 15:57:41 Pawel Jakub Dawidek wrote: > On Thu, Sep 03, 2009 at 03:48:37PM +0200, Mel Flynn wrote: > > On Thursday 03 September 2009 14:44:07 Pawel Jakub Dawidek wrote: > > > I'd suggest doing this not as separate gmirror(8) subcommand, but as an > > > extension to 'configure' subcommand, where one can provide priority by > > > giving '-p' argument. > > > > Except I didn't see how configure was implemented. Am I correct that this > > is g_mirror_ctl_configure in sys/geom/mirror/g_mirror_ctl.c? > > Yes, you are correct. Ok, new patch. I gathered from configure_slice, that my assumption was about flag handling is correct. Patch is only compile tested. -- Mel --Boundary-00=_izMoKZ58GgHWEiB Content-Type: text/plain; charset="UTF-8"; name="gmirror-prio.txt" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="gmirror-prio.txt" Index: sbin/geom/class/mirror/geom_mirror.c =================================================================== --- sbin/geom/class/mirror/geom_mirror.c (revision 196803) +++ sbin/geom/class/mirror/geom_mirror.c (working copy) @@ -47,7 +47,7 @@ static char label_balance[] = "split", configure_balance[] = "none"; static intmax_t label_slice = 4096, configure_slice = -1; -static intmax_t insert_priority = 0; +static intmax_t insert_priority = 0, configure_priority = -1; static void mirror_main(struct gctl_req *req, unsigned flags); static void mirror_activate(struct gctl_req *req); @@ -71,10 +71,11 @@ { 'F', "nofailsync", NULL, G_TYPE_BOOL }, { 'h', "hardcode", NULL, G_TYPE_BOOL }, { 'n', "noautosync", NULL, G_TYPE_BOOL }, + { 'p', "priority", &configure_priority, G_TYPE_NUMBER }, { 's', "slice", &configure_slice, G_TYPE_NUMBER }, G_OPT_SENTINEL }, - NULL, "[-adfFhnv] [-b balance] [-s slice] name" + NULL, "[-adfFhnv] [-b balance] [-s slice] [-p prio] name [prov]" }, { "deactivate", G_FLAG_VERBOSE, NULL, G_NULL_OPTS, NULL, "[-v] name prov ..." Index: sbin/geom/class/mirror/gmirror.8 =================================================================== --- sbin/geom/class/mirror/gmirror.8 (revision 196803) +++ sbin/geom/class/mirror/gmirror.8 (working copy) @@ -47,7 +47,9 @@ .Op Fl adfFhnv .Op Fl b Ar balance .Op Fl s Ar slice +.Op Fl p Ar priority .Ar name +.Op Ar prov .Nm .Cm rebuild .Op Fl v @@ -115,8 +117,8 @@ .It Cm label Create a mirror. The order of components is important, because a component's priority is based on its position -(starting from 0). -The component with the biggest priority is used by the +(starting from 0 to 255). +The component with the biggest priority (the lowest number) is used by the .Cm prefer balance algorithm and is also used as a master component when resynchronization is needed, @@ -175,6 +177,9 @@ Hardcode providers' names in metadata. .It Fl n Turn off autosynchronization of stale components. +.It Fl p Ar priority +Specify priority for given +.Ar prov .It Fl s Ar slice Specifies slice size for .Cm split Index: sys/geom/mirror/g_mirror_ctl.c =================================================================== --- sys/geom/mirror/g_mirror_ctl.c (revision 196803) +++ sys/geom/mirror/g_mirror_ctl.c (working copy) @@ -93,12 +93,12 @@ { struct g_mirror_softc *sc; struct g_mirror_disk *disk; - const char *name, *balancep; + const char *name, *balancep, *prov; intmax_t *slicep; uint32_t slice; uint8_t balance; int *autosync, *noautosync, *failsync, *nofailsync, *hardcode, *dynamic; - int *nargs, do_sync = 0, dirty = 1; + int *nargs, *priority, do_sync = 0, dirty = 1, do_priority = 0; nargs = gctl_get_paraml(req, "nargs", sizeof(*nargs)); if (nargs == NULL) { @@ -149,6 +149,27 @@ gctl_error(req, "No '%s' argument.", "dynamic"); return; } + priority = gctl_get_paraml(req, "priority", sizeof(*priority)); + if (priority == NULL ) { + gctl_error(req, "No '%s' argument.", "priority"); + return; + } + if( *priority < -1 || *priority > 255 ) { + gctl_error(req, "Priority range is 0 to 255, %i given", + *priority); + return; + } + /* Since we have a priority, we also need a provider now. + * Note: be WARNS safe, by always assigning prov and only throw an + * error if *priority != -1. */ + prov = gctl_get_asciiparam(req, "arg1"); + if( *priority > -1 ) { + if( prov == NULL ) { + gctl_error(req, "Priority needs a disk name"); + return; + } + do_priority = 1; + } if (*autosync && *noautosync) { gctl_error(req, "'%s' and '%s' specified.", "autosync", "noautosync"); @@ -233,6 +254,11 @@ disk->d_flags &= ~G_MIRROR_DISK_FLAG_HARDCODED; if (!dirty) disk->d_flags &= ~G_MIRROR_DISK_FLAG_DIRTY; + if (do_priority) { + if (strcmp(disk->d_name, prov) == 0) { + disk->d_priority = *priority; + } + } g_mirror_update_metadata(disk); if (do_sync) { if (disk->d_state == G_MIRROR_DISK_STATE_STALE) { --Boundary-00=_izMoKZ58GgHWEiB-- From owner-freebsd-geom@FreeBSD.ORG Sat Sep 5 09:11:41 2009 Return-Path: Delivered-To: freebsd-geom@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 6BDC8106566C; Sat, 5 Sep 2009 09:11:41 +0000 (UTC) (envelope-from pjd@garage.freebsd.pl) Received: from mail.garage.freebsd.pl (chello087206049004.chello.pl [87.206.49.4]) by mx1.freebsd.org (Postfix) with ESMTP id DB8A78FC19; Sat, 5 Sep 2009 09:11:40 +0000 (UTC) Received: by mail.garage.freebsd.pl (Postfix, from userid 65534) id 468C045EEF; Sat, 5 Sep 2009 11:11:39 +0200 (CEST) Received: from localhost (chello087206049004.chello.pl [87.206.49.4]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.garage.freebsd.pl (Postfix) with ESMTP id E922F45E11; Sat, 5 Sep 2009 11:10:28 +0200 (CEST) Date: Sat, 5 Sep 2009 11:10:30 +0200 From: Pawel Jakub Dawidek To: Mel Flynn Message-ID: <20090905091030.GC1665@garage.freebsd.pl> References: <200909030000.11961.mel.flynn+fbsd.fs@mailing.thruhere.net> <200909031548.37887.mel.flynn+fbsd.fs@mailing.thruhere.net> <20090903135741.GK1821@garage.freebsd.pl> <200909041016.34677.mel.flynn+fbsd.fs@mailing.thruhere.net> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="BwCQnh7xodEAoBMC" Content-Disposition: inline In-Reply-To: <200909041016.34677.mel.flynn+fbsd.fs@mailing.thruhere.net> User-Agent: Mutt/1.4.2.3i X-PGP-Key-URL: http://people.freebsd.org/~pjd/pjd.asc X-OS: FreeBSD 8.0-CURRENT i386 X-Spam-Checker-Version: SpamAssassin 3.0.4 (2005-06-05) on mail.garage.freebsd.pl X-Spam-Level: X-Spam-Status: No, score=-0.6 required=4.5 tests=BAYES_00,RCVD_IN_SORBS_DUL autolearn=no version=3.0.4 Cc: freebsd-fs@freebsd.org, freebsd-geom@freebsd.org Subject: Re: Patch to allow gmirror to set priority of a disk X-BeenThere: freebsd-geom@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: GEOM-specific discussions and implementations List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 05 Sep 2009 09:11:41 -0000 --BwCQnh7xodEAoBMC Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Sep 04, 2009 at 10:16:34AM +0200, Mel Flynn wrote: > On Thursday 03 September 2009 15:57:41 Pawel Jakub Dawidek wrote: > > On Thu, Sep 03, 2009 at 03:48:37PM +0200, Mel Flynn wrote: > > > On Thursday 03 September 2009 14:44:07 Pawel Jakub Dawidek wrote: > > > > I'd suggest doing this not as separate gmirror(8) subcommand, but a= s an > > > > extension to 'configure' subcommand, where one can provide priority= by > > > > giving '-p' argument. > > > > > > Except I didn't see how configure was implemented. Am I correct that = this > > > is g_mirror_ctl_configure in sys/geom/mirror/g_mirror_ctl.c? > > > > Yes, you are correct. >=20 > Ok, new patch. I gathered from configure_slice, that my assumption was ab= out=20 > flag handling is correct. Patch is only compile tested. In general the patch looks good, although I haven't tested it yet. I do have some comments though. > Index: sbin/geom/class/mirror/geom_mirror.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- sbin/geom/class/mirror/geom_mirror.c (revision 196803) > +++ sbin/geom/class/mirror/geom_mirror.c (working copy) > @@ -47,7 +47,7 @@ > =20 > static char label_balance[] =3D "split", configure_balance[] =3D "none"; > static intmax_t label_slice =3D 4096, configure_slice =3D -1; > -static intmax_t insert_priority =3D 0; > +static intmax_t insert_priority =3D 0, configure_priority =3D -1; > =20 > static void mirror_main(struct gctl_req *req, unsigned flags); > static void mirror_activate(struct gctl_req *req); > @@ -71,10 +71,11 @@ > { 'F', "nofailsync", NULL, G_TYPE_BOOL }, > { 'h', "hardcode", NULL, G_TYPE_BOOL }, > { 'n', "noautosync", NULL, G_TYPE_BOOL }, > + { 'p', "priority", &configure_priority, G_TYPE_NUMBER }, > { 's', "slice", &configure_slice, G_TYPE_NUMBER }, > G_OPT_SENTINEL > }, > - NULL, "[-adfFhnv] [-b balance] [-s slice] name" > + NULL, "[-adfFhnv] [-b balance] [-s slice] [-p prio] name [prov]" Style, extra space before '[-p prio]'. I also wonder if something like this should be possible: # gmirror configure -b round-robin -p 1 gm0 da0 In you current implementation it will change balance algorithm for all the providers and in addition priority of da0 provider. This doesn't look clear for my taste (will balance algorithm be changed on da0 only or on all providers?). I'd prefer: gmirror configure [-adfFhnv] [-b balance] [-s slice] name gmirror configure -p prio name prov Although I'm not sure if the infrastructure can actually print two usage patterns. > +.It Fl p Ar priority > +Specify priority for given > +.Ar prov s/for given/for the given/ ? > Index: sys/geom/mirror/g_mirror_ctl.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- sys/geom/mirror/g_mirror_ctl.c (revision 196803) > +++ sys/geom/mirror/g_mirror_ctl.c (working copy) > @@ -93,12 +93,12 @@ > { > struct g_mirror_softc *sc; > struct g_mirror_disk *disk; > - const char *name, *balancep; > + const char *name, *balancep, *prov; > intmax_t *slicep; > uint32_t slice; > uint8_t balance; > int *autosync, *noautosync, *failsync, *nofailsync, *hardcode, *dynamic; > - int *nargs, do_sync =3D 0, dirty =3D 1; > + int *nargs, *priority, do_sync =3D 0, dirty =3D 1, do_priority =3D 0; > =20 > nargs =3D gctl_get_paraml(req, "nargs", sizeof(*nargs)); > if (nargs =3D=3D NULL) { > @@ -149,6 +149,27 @@ > gctl_error(req, "No '%s' argument.", "dynamic"); > return; > } > + priority =3D gctl_get_paraml(req, "priority", sizeof(*priority)); > + if (priority =3D=3D NULL ) { Style, extra space after NULL. > + gctl_error(req, "No '%s' argument.", "priority"); > + return; > + } > + if( *priority < -1 || *priority > 255 ) { Style, should be: if (*priority < -1 || *priority > 255) { > + gctl_error(req, "Priority range is 0 to 255, %i given", > + *priority); > + return; > + } > + /* Since we have a priority, we also need a provider now. > + * Note: be WARNS safe, by always assigning prov and only throw an > + * error if *priority !=3D -1. */ Style, multi-line comment should look like this: /* * Since we have a priority, we also need a provider now. * Note: be WARNS safe, by always assigning prov and only throw an * error if *priority !=3D -1. */ > + prov =3D gctl_get_asciiparam(req, "arg1"); > + if( *priority > -1 ) { Style: if (*priority > -1) { > + if( prov =3D=3D NULL ) { Style: if (prov =3D=3D NULL) { > + gctl_error(req, "Priority needs a disk name"); > + return; > + } > + do_priority =3D 1; > + } > if (*autosync && *noautosync) { > gctl_error(req, "'%s' and '%s' specified.", "autosync", > "noautosync"); > @@ -233,6 +254,11 @@ > disk->d_flags &=3D ~G_MIRROR_DISK_FLAG_HARDCODED; > if (!dirty) > disk->d_flags &=3D ~G_MIRROR_DISK_FLAG_DIRTY; > + if (do_priority) { > + if (strcmp(disk->d_name, prov) =3D=3D 0) { > + disk->d_priority =3D *priority; > + } Style, you can drop { } here: if (strcmp(disk->d_name, prov) =3D=3D 0) disk->d_priority =3D *priority; > + } > g_mirror_update_metadata(disk); > if (do_sync) { > if (disk->d_state =3D=3D G_MIRROR_DISK_STATE_STALE) { Other than those small style issues good job. Now we just have to decide about mixing per-mirror options with per-provider options. --=20 Pawel Jakub Dawidek http://www.wheel.pl pjd@FreeBSD.org http://www.FreeBSD.org FreeBSD committer Am I Evil? Yes, I Am! --BwCQnh7xodEAoBMC Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.4 (FreeBSD) iD8DBQFKoisGForvXbEpPzQRAgdSAKDocMkpE/BjAhd6QIoENWG9akbmpACg0IBr F3pPqlL3ZenVeyBi0HK1jjQ= =p00c -----END PGP SIGNATURE----- --BwCQnh7xodEAoBMC-- From owner-freebsd-geom@FreeBSD.ORG Sat Sep 5 19:11:33 2009 Return-Path: Delivered-To: freebsd-geom@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 0F63B10656FA; Sat, 5 Sep 2009 19:11:33 +0000 (UTC) (envelope-from mel.flynn+fbsd.fs@mailing.thruhere.net) Received: from mailhub.rachie.is-a-geek.net (rachie.is-a-geek.net [66.230.99.27]) by mx1.freebsd.org (Postfix) with ESMTP id A69848FC14; Sat, 5 Sep 2009 19:11:31 +0000 (UTC) Received: from smoochies.rachie.is-a-geek.net (mailhub.rachie.is-a-geek.net [192.168.2.11]) by mailhub.rachie.is-a-geek.net (Postfix) with ESMTP id 785277E818; Sat, 5 Sep 2009 11:11:42 -0800 (AKDT) From: Mel Flynn To: freebsd-fs@freebsd.org Date: Sat, 5 Sep 2009 21:11:27 +0200 User-Agent: KMail/1.11.4 (FreeBSD/8.0-BETA3; KDE/4.2.4; i386; ; ) References: <200909030000.11961.mel.flynn+fbsd.fs@mailing.thruhere.net> <200909041016.34677.mel.flynn+fbsd.fs@mailing.thruhere.net> <20090905091030.GC1665@garage.freebsd.pl> In-Reply-To: <20090905091030.GC1665@garage.freebsd.pl> MIME-Version: 1.0 Content-Type: Text/Plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200909052111.27667.mel.flynn+fbsd.fs@mailing.thruhere.net> Cc: Pawel Jakub Dawidek , freebsd-geom@freebsd.org Subject: Re: Patch to allow gmirror to set priority of a disk X-BeenThere: freebsd-geom@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: GEOM-specific discussions and implementations List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 05 Sep 2009 19:11:33 -0000 Hi Pawel, I'll post-process in the future. I'm so used to if( cond ), cause it allows me to more easily spot the outer bounds of the condition, it's hard to conform to style(9) for me. :) On Saturday 05 September 2009 11:10:30 Pawel Jakub Dawidek wrote: > On Fri, Sep 04, 2009 at 10:16:34AM +0200, Mel Flynn wrote: > > On Thursday 03 September 2009 15:57:41 Pawel Jakub Dawidek wrote: > > > On Thu, Sep 03, 2009 at 03:48:37PM +0200, Mel Flynn wrote: > > > > On Thursday 03 September 2009 14:44:07 Pawel Jakub Dawidek wrote: > > > > > I'd suggest doing this not as separate gmirror(8) subcommand, but > > > > > as an extension to 'configure' subcommand, where one can provide > > > > > priority by giving '-p' argument. > > > > > > > > Except I didn't see how configure was implemented. Am I correct that > > > > this is g_mirror_ctl_configure in sys/geom/mirror/g_mirror_ctl.c? > > > > > > Yes, you are correct. > > > > Ok, new patch. I gathered from configure_slice, that my assumption was > > about flag handling is correct. Patch is only compile tested. > > In general the patch looks good, although I haven't tested it yet. I do > have some comments though. > > > Index: sbin/geom/class/mirror/geom_mirror.c > > =================================================================== > > --- sbin/geom/class/mirror/geom_mirror.c (revision 196803) > > +++ sbin/geom/class/mirror/geom_mirror.c (working copy) > > @@ -47,7 +47,7 @@ > > > > static char label_balance[] = "split", configure_balance[] = "none"; > > static intmax_t label_slice = 4096, configure_slice = -1; > > -static intmax_t insert_priority = 0; > > +static intmax_t insert_priority = 0, configure_priority = -1; > > > > static void mirror_main(struct gctl_req *req, unsigned flags); > > static void mirror_activate(struct gctl_req *req); > > @@ -71,10 +71,11 @@ > > { 'F', "nofailsync", NULL, G_TYPE_BOOL }, > > { 'h', "hardcode", NULL, G_TYPE_BOOL }, > > { 'n', "noautosync", NULL, G_TYPE_BOOL }, > > + { 'p', "priority", &configure_priority, G_TYPE_NUMBER }, > > { 's', "slice", &configure_slice, G_TYPE_NUMBER }, > > G_OPT_SENTINEL > > }, > > - NULL, "[-adfFhnv] [-b balance] [-s slice] name" > > + NULL, "[-adfFhnv] [-b balance] [-s slice] [-p prio] name [prov]" > > Style, extra space before '[-p prio]'. > > I also wonder if something like this should be possible: > > # gmirror configure -b round-robin -p 1 gm0 da0 > > In you current implementation it will change balance algorithm for all > the providers and in addition priority of da0 provider. This doesn't > look clear for my taste (will balance algorithm be changed on da0 only > or on all providers?). Right, I found it convenient, but now that I think it about it, it's confusing from user perspective. > I'd prefer: > > gmirror configure [-adfFhnv] [-b balance] [-s slice] name > gmirror configure -p prio name prov > > Although I'm not sure if the infrastructure can actually print two usage > patterns. I tried two g_command structs for the same command, like so: @@ -76,6 +76,13 @@ }, NULL, "[-adfFhnv] [-b balance] [-s slice] name" }, + { "configure", G_FLAG_VERBOSE, NULL, + { + { 'p', "priority", &configure_priority, G_TYPE_NUMBER }, + G_OPT_SENTINEL + }, + NULL, "-p prio name prov" + }, { "deactivate", G_FLAG_VERBOSE, NULL, G_NULL_OPTS, NULL, "[-v] name prov ..." }, But it errors if -p is given with illegal option, so either g_command needs to grow a second usage string, we need to do something like below, or we care less about usage and more about the man page. @@ -71,10 +71,12 @@ { 'F', "nofailsync", NULL, G_TYPE_BOOL }, { 'h', "hardcode", NULL, G_TYPE_BOOL }, { 'n', "noautosync", NULL, G_TYPE_BOOL }, + { 'p', "priority", &configure_priority, G_TYPE_NUMBER }, { 's', "slice", &configure_slice, G_TYPE_NUMBER }, G_OPT_SENTINEL }, - NULL, "[-adfFhnv] [-b balance] [-s slice] name" + NULL, "[-adfFhnv] [-b balance] [-s slice] name\n" + " gmirror configure -p priority name prov" }, { "deactivate", G_FLAG_VERBOSE, NULL, G_NULL_OPTS, NULL, "[-v] name prov ..." Fixing style issues now and reimplementing. -- Mel From owner-freebsd-geom@FreeBSD.ORG Sat Sep 5 19:23:08 2009 Return-Path: Delivered-To: freebsd-geom@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 7F452106566B; Sat, 5 Sep 2009 19:23:08 +0000 (UTC) (envelope-from pjd@garage.freebsd.pl) Received: from mail.garage.freebsd.pl (chello087206049004.chello.pl [87.206.49.4]) by mx1.freebsd.org (Postfix) with ESMTP id 11EEE8FC1C; Sat, 5 Sep 2009 19:23:06 +0000 (UTC) Received: by mail.garage.freebsd.pl (Postfix, from userid 65534) id E2FE045E6F; Sat, 5 Sep 2009 21:23:01 +0200 (CEST) Received: from localhost (chello087206049004.chello.pl [87.206.49.4]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.garage.freebsd.pl (Postfix) with ESMTP id A8FE145C89; Sat, 5 Sep 2009 21:22:53 +0200 (CEST) Date: Sat, 5 Sep 2009 21:22:51 +0200 From: Pawel Jakub Dawidek To: Mel Flynn Message-ID: <20090905192251.GJ1665@garage.freebsd.pl> References: <200909030000.11961.mel.flynn+fbsd.fs@mailing.thruhere.net> <200909041016.34677.mel.flynn+fbsd.fs@mailing.thruhere.net> <20090905091030.GC1665@garage.freebsd.pl> <200909052111.27667.mel.flynn+fbsd.fs@mailing.thruhere.net> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="hl1kWnBARzJiTscN" Content-Disposition: inline In-Reply-To: <200909052111.27667.mel.flynn+fbsd.fs@mailing.thruhere.net> User-Agent: Mutt/1.4.2.3i X-PGP-Key-URL: http://people.freebsd.org/~pjd/pjd.asc X-OS: FreeBSD 8.0-CURRENT i386 X-Spam-Checker-Version: SpamAssassin 3.0.4 (2005-06-05) on mail.garage.freebsd.pl X-Spam-Level: X-Spam-Status: No, score=-0.6 required=4.5 tests=BAYES_00,RCVD_IN_SORBS_DUL autolearn=no version=3.0.4 Cc: freebsd-fs@freebsd.org, freebsd-geom@freebsd.org Subject: Re: Patch to allow gmirror to set priority of a disk X-BeenThere: freebsd-geom@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: GEOM-specific discussions and implementations List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 05 Sep 2009 19:23:08 -0000 --hl1kWnBARzJiTscN Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, Sep 05, 2009 at 09:11:27PM +0200, Mel Flynn wrote: > Hi Pawel, >=20 > I'll post-process in the future. I'm so used to if( cond ), cause it allo= ws me=20 > to more easily spot the outer bounds of the condition, it's hard to confo= rm to=20 > style(9) for me. :) >=20 > On Saturday 05 September 2009 11:10:30 Pawel Jakub Dawidek wrote: > > On Fri, Sep 04, 2009 at 10:16:34AM +0200, Mel Flynn wrote: > > > On Thursday 03 September 2009 15:57:41 Pawel Jakub Dawidek wrote: > > > > On Thu, Sep 03, 2009 at 03:48:37PM +0200, Mel Flynn wrote: > > > > > On Thursday 03 September 2009 14:44:07 Pawel Jakub Dawidek wrote: > > > > > > I'd suggest doing this not as separate gmirror(8) subcommand, b= ut > > > > > > as an extension to 'configure' subcommand, where one can provide > > > > > > priority by giving '-p' argument. > > > > > > > > > > Except I didn't see how configure was implemented. Am I correct t= hat > > > > > this is g_mirror_ctl_configure in sys/geom/mirror/g_mirror_ctl.c? > > > > > > > > Yes, you are correct. > > > > > > Ok, new patch. I gathered from configure_slice, that my assumption was > > > about flag handling is correct. Patch is only compile tested. > > > > In general the patch looks good, although I haven't tested it yet. I do > > have some comments though. > > > > > Index: sbin/geom/class/mirror/geom_mirror.c > > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > > --- sbin/geom/class/mirror/geom_mirror.c (revision 196803) > > > +++ sbin/geom/class/mirror/geom_mirror.c (working copy) > > > @@ -47,7 +47,7 @@ > > > > > > static char label_balance[] =3D "split", configure_balance[] =3D "no= ne"; > > > static intmax_t label_slice =3D 4096, configure_slice =3D -1; > > > -static intmax_t insert_priority =3D 0; > > > +static intmax_t insert_priority =3D 0, configure_priority =3D -1; > > > > > > static void mirror_main(struct gctl_req *req, unsigned flags); > > > static void mirror_activate(struct gctl_req *req); > > > @@ -71,10 +71,11 @@ > > > { 'F', "nofailsync", NULL, G_TYPE_BOOL }, > > > { 'h', "hardcode", NULL, G_TYPE_BOOL }, > > > { 'n', "noautosync", NULL, G_TYPE_BOOL }, > > > + { 'p', "priority", &configure_priority, G_TYPE_NUMBER }, > > > { 's', "slice", &configure_slice, G_TYPE_NUMBER }, > > > G_OPT_SENTINEL > > > }, > > > - NULL, "[-adfFhnv] [-b balance] [-s slice] name" > > > + NULL, "[-adfFhnv] [-b balance] [-s slice] [-p prio] name [prov= ]" > > > > Style, extra space before '[-p prio]'. > > > > I also wonder if something like this should be possible: > > > > # gmirror configure -b round-robin -p 1 gm0 da0 > > > > In you current implementation it will change balance algorithm for all > > the providers and in addition priority of da0 provider. This doesn't > > look clear for my taste (will balance algorithm be changed on da0 only > > or on all providers?). >=20 > Right, I found it convenient, but now that I think it about it, it's conf= using=20 > from user perspective. >=20 > > I'd prefer: > > > > gmirror configure [-adfFhnv] [-b balance] [-s slice] name > > gmirror configure -p prio name prov > > > > Although I'm not sure if the infrastructure can actually print two usage > > patterns. >=20 > I tried two g_command structs for the same command, like so: > @@ -76,6 +76,13 @@ > }, > NULL, "[-adfFhnv] [-b balance] [-s slice] name" > }, > + { "configure", G_FLAG_VERBOSE, NULL, > + { > + { 'p', "priority", &configure_priority, G_TYPE_NUMBER }, > + G_OPT_SENTINEL > + }, > + NULL, "-p prio name prov" > + }, > { "deactivate", G_FLAG_VERBOSE, NULL, G_NULL_OPTS, NULL, > "[-v] name prov ..." > }, >=20 > But it errors if -p is given with illegal option, so either g_command nee= ds to=20 > grow a second usage string, we need to do something like below, or we car= e=20 > less about usage and more about the man page. >=20 > @@ -71,10 +71,12 @@ > { 'F', "nofailsync", NULL, G_TYPE_BOOL }, > { 'h', "hardcode", NULL, G_TYPE_BOOL }, > { 'n', "noautosync", NULL, G_TYPE_BOOL }, > + { 'p', "priority", &configure_priority, G_TYPE_NUMBER }, > { 's', "slice", &configure_slice, G_TYPE_NUMBER }, > G_OPT_SENTINEL > }, > - NULL, "[-adfFhnv] [-b balance] [-s slice] name" > + NULL, "[-adfFhnv] [-b balance] [-s slice] name\n" > + " gmirror configure -p priority name prov" We could also do the following: NULL, "[-adfFhnv] [-b balance] [-s slice] name\n" "-p priority name prov" And tech core geom(8) utility to split usage based on \n. usage_command() function from sbin/geom/core/geom.c would have to be modified. If you agree with this idea, would you also like to work on this? --=20 Pawel Jakub Dawidek http://www.wheel.pl pjd@FreeBSD.org http://www.FreeBSD.org FreeBSD committer Am I Evil? Yes, I Am! --hl1kWnBARzJiTscN Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.4 (FreeBSD) iD8DBQFKorqLForvXbEpPzQRAsJOAJ9x4xCw1I3h+YJ1ycuF1XdC5IsW0ACgxuma BCY5L/33FNiyzNlVRR1Nfg0= =IG5+ -----END PGP SIGNATURE----- --hl1kWnBARzJiTscN-- From owner-freebsd-geom@FreeBSD.ORG Sat Sep 5 20:25:09 2009 Return-Path: Delivered-To: freebsd-geom@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 942911065693; Sat, 5 Sep 2009 20:25:09 +0000 (UTC) (envelope-from mel.flynn+fbsd.fs@mailing.thruhere.net) Received: from mailhub.rachie.is-a-geek.net (rachie.is-a-geek.net [66.230.99.27]) by mx1.freebsd.org (Postfix) with ESMTP id 5B1368FC1A; Sat, 5 Sep 2009 20:25:09 +0000 (UTC) Received: from smoochies.rachie.is-a-geek.net (mailhub.lan.rachie.is-a-geek.net [192.168.2.11]) by mailhub.rachie.is-a-geek.net (Postfix) with ESMTP id 593867E818; Sat, 5 Sep 2009 12:25:20 -0800 (AKDT) From: Mel Flynn To: freebsd-fs@freebsd.org Date: Sat, 5 Sep 2009 22:25:05 +0200 User-Agent: KMail/1.11.4 (FreeBSD/8.0-BETA3; KDE/4.2.4; i386; ; ) References: <200909030000.11961.mel.flynn+fbsd.fs@mailing.thruhere.net> <200909052111.27667.mel.flynn+fbsd.fs@mailing.thruhere.net> <20090905192251.GJ1665@garage.freebsd.pl> In-Reply-To: <20090905192251.GJ1665@garage.freebsd.pl> MIME-Version: 1.0 Content-Type: Multipart/Mixed; boundary="Boundary-00=_iksoKYVobZGJI9m" Message-Id: <200909052225.06185.mel.flynn+fbsd.fs@mailing.thruhere.net> Cc: Pawel Jakub Dawidek , freebsd-geom@freebsd.org Subject: Re: Patch to allow gmirror to set priority of a disk X-BeenThere: freebsd-geom@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: GEOM-specific discussions and implementations List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 05 Sep 2009 20:25:09 -0000 --Boundary-00=_iksoKYVobZGJI9m Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline On Saturday 05 September 2009 21:22:51 Pawel Jakub Dawidek wrote: > And tech core geom(8) utility to split usage based on \n. > usage_command() function from sbin/geom/core/geom.c would have to be > modified. If you agree with this idea, would you also like to work on > this? Yes and see attached. Since utility exists after displaying usage, I didn't take the trouble of freeing ptr, but if this is preferred then I'll add the line. % geom mirror foo geom: Unknown command: foo. usage: geom mirror activate [-v] name prov ... geom mirror clear [-v] prov ... geom mirror configure[-adfFhnv] [-b balance] [-s slice] name geom mirror configure -p priority name prov -- Mel --Boundary-00=_iksoKYVobZGJI9m Content-Type: text/plain; charset="UTF-8"; name="geom-core.txt" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="geom-core.txt" Index: sbin/geom/core/geom.c =================================================================== --- sbin/geom/core/geom.c (revision 196868) +++ sbin/geom/core/geom.c (working copy) @@ -98,11 +98,23 @@ struct g_option *opt; unsigned i; - fprintf(stderr, "%s %s %s", prefix, comm, cmd->gc_name); if (cmd->gc_usage != NULL) { - fprintf(stderr, " %s\n", cmd->gc_usage); + char *pos, *ptr; + + ptr = strdup(cmd->gc_usage); + while ((pos = strchr(ptr, '\n')) != NULL) { + *pos = '\0'; + fprintf(stderr, "%s %s %s", prefix, comm, cmd->gc_name); + fprintf(stderr, "%s\n", ptr); + ptr = pos + 1; + } + /* Tail or no \n at all */ + fprintf(stderr, "%s %s %s", prefix, comm, cmd->gc_name); + fprintf(stderr, " %s\n", ptr); return; } + + fprintf(stderr, "%s %s %s", prefix, comm, cmd->gc_name); if ((cmd->gc_flags & G_FLAG_VERBOSE) != 0) fprintf(stderr, " [-v]"); for (i = 0; ; i++) { --Boundary-00=_iksoKYVobZGJI9m-- From owner-freebsd-geom@FreeBSD.ORG Sat Sep 5 20:36:00 2009 Return-Path: Delivered-To: freebsd-geom@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 0302B106568D; Sat, 5 Sep 2009 20:36:00 +0000 (UTC) (envelope-from pjd@garage.freebsd.pl) Received: from mail.garage.freebsd.pl (chello087206049004.chello.pl [87.206.49.4]) by mx1.freebsd.org (Postfix) with ESMTP id 47F3B8FC08; Sat, 5 Sep 2009 20:35:58 +0000 (UTC) Received: by mail.garage.freebsd.pl (Postfix, from userid 65534) id 4647B45F22; Sat, 5 Sep 2009 22:35:57 +0200 (CEST) Received: from localhost (chello087206049004.chello.pl [87.206.49.4]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.garage.freebsd.pl (Postfix) with ESMTP id 7213F456B1; Sat, 5 Sep 2009 22:35:50 +0200 (CEST) Date: Sat, 5 Sep 2009 22:35:51 +0200 From: Pawel Jakub Dawidek To: Mel Flynn Message-ID: <20090905203551.GK1665@garage.freebsd.pl> References: <200909030000.11961.mel.flynn+fbsd.fs@mailing.thruhere.net> <200909052111.27667.mel.flynn+fbsd.fs@mailing.thruhere.net> <20090905192251.GJ1665@garage.freebsd.pl> <200909052225.06185.mel.flynn+fbsd.fs@mailing.thruhere.net> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="OGLMwEELQbPC02lM" Content-Disposition: inline In-Reply-To: <200909052225.06185.mel.flynn+fbsd.fs@mailing.thruhere.net> User-Agent: Mutt/1.4.2.3i X-PGP-Key-URL: http://people.freebsd.org/~pjd/pjd.asc X-OS: FreeBSD 8.0-CURRENT i386 X-Spam-Checker-Version: SpamAssassin 3.0.4 (2005-06-05) on mail.garage.freebsd.pl X-Spam-Level: X-Spam-Status: No, score=-0.6 required=4.5 tests=BAYES_00,RCVD_IN_SORBS_DUL autolearn=no version=3.0.4 Cc: freebsd-fs@freebsd.org, freebsd-geom@freebsd.org Subject: Re: Patch to allow gmirror to set priority of a disk X-BeenThere: freebsd-geom@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: GEOM-specific discussions and implementations List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 05 Sep 2009 20:36:00 -0000 --OGLMwEELQbPC02lM Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, Sep 05, 2009 at 10:25:05PM +0200, Mel Flynn wrote: > On Saturday 05 September 2009 21:22:51 Pawel Jakub Dawidek wrote: >=20 > > And tech core geom(8) utility to split usage based on \n. > > usage_command() function from sbin/geom/core/geom.c would have to be > > modified. If you agree with this idea, would you also like to work on > > this? >=20 > Yes and see attached. Since utility exists after displaying usage, I didn= 't=20 > take the trouble of freeing ptr, but if this is preferred then I'll add t= he=20 > line. I think it would be good to free memory just to make it elegant. > % geom mirror foo > geom: Unknown command: foo. > usage: geom mirror activate [-v] name prov ... > geom mirror clear [-v] prov ... > geom mirror configure[-adfFhnv] [-b balance] [-s slice] name > geom mirror configure -p priority name prov >=20 > --=20 > Mel > Index: sbin/geom/core/geom.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- sbin/geom/core/geom.c (revision 196868) > +++ sbin/geom/core/geom.c (working copy) > @@ -98,11 +98,23 @@ > struct g_option *opt; > unsigned i; > =20 > - fprintf(stderr, "%s %s %s", prefix, comm, cmd->gc_name); > if (cmd->gc_usage !=3D NULL) { > - fprintf(stderr, " %s\n", cmd->gc_usage); > + char *pos, *ptr; > + > + ptr =3D strdup(cmd->gc_usage); > + while ((pos =3D strchr(ptr, '\n')) !=3D NULL) { Wouldn't it be easier to use strsep(3)? > + *pos =3D '\0'; > + fprintf(stderr, "%s %s %s", prefix, comm, cmd->gc_name); > + fprintf(stderr, "%s\n", ptr); Why to split this into two fprintfs? There is also space missing before %s. > + ptr =3D pos + 1; > + } > + /* Tail or no \n at all */ > + fprintf(stderr, "%s %s %s", prefix, comm, cmd->gc_name); > + fprintf(stderr, " %s\n", ptr); > return; > } > + > + fprintf(stderr, "%s %s %s", prefix, comm, cmd->gc_name); > if ((cmd->gc_flags & G_FLAG_VERBOSE) !=3D 0) > fprintf(stderr, " [-v]"); > for (i =3D 0; ; i++) { With strsep(3) it would look like this: if (cmd->gc_usage !=3D NULL) { char *cur, *ptr, *tofree; tofree =3D ptr =3D strdup(cmd->gc_usage); while ((cur =3D strsep(&ptr, "\n")) !=3D NULL) { if (*cur =3D=3D '\0') continue; fprintf(stderr, "%s %s %s %s\n", prefix, comm, cmd->gc_name, cur); } free(tofree); return; } --=20 Pawel Jakub Dawidek http://www.wheel.pl pjd@FreeBSD.org http://www.FreeBSD.org FreeBSD committer Am I Evil? Yes, I Am! --OGLMwEELQbPC02lM Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.4 (FreeBSD) iD8DBQFKosunForvXbEpPzQRAljRAJ9TdBhx47uuQOr8uppEKG51v/miVwCg76sb czv02Cs0aNf7RZhQKcjvkCA= =G0aC -----END PGP SIGNATURE----- --OGLMwEELQbPC02lM--