From owner-freebsd-current@FreeBSD.ORG Fri May 28 01:31:08 2010 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id A83E6106564A; Fri, 28 May 2010 01:31:08 +0000 (UTC) (envelope-from yanefbsd@gmail.com) Received: from mail-qy0-f181.google.com (mail-qy0-f181.google.com [209.85.221.181]) by mx1.freebsd.org (Postfix) with ESMTP id 4ADE88FC12; Fri, 28 May 2010 01:31:07 +0000 (UTC) Received: by qyk11 with SMTP id 11so999447qyk.13 for ; Thu, 27 May 2010 18:31:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:received:in-reply-to :references:date:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=xRDhLJ0XL4ljDTsZrDgyFhtuIZD21CyfXZNmtbh524A=; b=tZDNJwcq3xgYZejBZNtqBseRmH5GwDoesXLc7lHvSnDxb1Imxi0mDzfaYugETGLfeT jQvYr3RsQ0iVKRy+QHSjx36IXyWAWpMYBCthvEZYpJZ0Mz/0EYRV1i1rXOydV7RfV1mx 0zfpvUNAZmeGV1nA1chqZK3L7KybwE/DS/J/Q= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; b=Dcy5VZ5CcuCHxOO/7YcPtU8q9gaDX15tNxny6lulej6rc1eU3oyjhToULZo+H7rSTj KkHcqzcrd2vJuW4Db1NKSctDjdrtJ6Ch6US6OraDNdumyy0DdCyyCiMMi4jzsAYul/mD XhYfHb9EpZuvjYb+xVHzYDDVw4wKIydOJh4/A= MIME-Version: 1.0 Received: by 10.224.29.208 with SMTP id r16mr5602560qac.212.1275010266745; Thu, 27 May 2010 18:31:06 -0700 (PDT) Received: by 10.229.190.83 with HTTP; Thu, 27 May 2010 18:31:06 -0700 (PDT) In-Reply-To: References: <201005270933.42760.jhb@freebsd.org> <201005271147.20155.jhb@freebsd.org> Date: Thu, 27 May 2010 18:31:06 -0700 Message-ID: From: Garrett Cooper To: "Marcelo/Porks" Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: freebsd-current@freebsd.org, Jeff Roberson Subject: Re: SUJ Changes X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 28 May 2010 01:31:08 -0000 On Thu, May 27, 2010 at 3:44 PM, Marcelo/Porks wro= te: > On 5/27/10, John Baldwin wrote: >> On Thursday 27 May 2010 10:13:38 am Marcelo/Porks wrote: >>> On Thu, May 27, 2010 at 10:33 AM, John Baldwin wrote: >>> > On Wednesday 26 May 2010 7:56:24 pm Garrett Cooper wrote: >>> >> On Wed, May 26, 2010 at 3:52 PM, Marcelo/Porks >>> >> >> wrote: >>> >> > >>> >> > Hi guys. I'm not sure if I could call this a problem but I can >>> >> > disable >>> >> > SU when SUJ is enabled, so SUJ will remain enabled and SU will be >>> >> > disabled. >>> >> > >>> >> > #tunefs -j enable /dev/device >>> >> > #tunefs -n disable /dev/device >>> >> > >>> >> > I did a patch for sbin/tunefs/tunefs.c that disable SUJ when the u= ser >>> >> > disable SU. Maybe this will be useful for some of you. >>> >> > >>> >> > Thanks. >>> >> > >>> >> > >>> >> > Index: sbin/tunefs/tunefs.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/tunefs/tunefs.c =A0 =A0 =A0 =A0(revision 208580) >>> >> > +++ sbin/tunefs/tunefs.c =A0 =A0 =A0 =A0(working copy) >>> >> > @@ -460,6 +460,14 @@ >>> >> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if ((~sblock.fs_fla= gs & FS_DOSOFTDEP) =3D=3D >>> > FS_DOSOFTDEP) >>> >> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0war= nx("%s remains unchanged as >> disabled", >>> > name); >>> >> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0else { >>> >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* a= lso disable SUJ */ >>> >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (= (sblock.fs_flags & FS_SUJ) =3D=3D >> FS_SUJ) >>> > { >>> >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 warnx("soft updates >>> >> > journaling >>> >> > will be disabled too"); >>> >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 journal_clear(); >>> >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 sblock.fs_flags &=3D ~FS_SUJ; >>> >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 sblock.fs_sujfree =3D 0; >>> >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 warnx("remove .sujournal to >>> >> > reclaim space"); >>> >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 } >>> >> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0sbl= ock.fs_flags &=3D ~FS_DOSOFTDEP; >>> >> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0war= nx("%s cleared", name); >>> >> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0} >>> >> >>> > I think that attempting to disable SU if SUJ >>> > is enabled should just fail with an error message. =A0The sysadmin ca= n >>> > then >>> > choose to disable both SUJ and SU if desired. >>> >>> If SU is disabled and One tries to enable SUJ then SU will be >>> automatically enabled. >>> So Why not automatically disable SUJ when One tries to disable SU? >> >> I'm probably not a big fan of either really. :) =A0For something as rare= ly >> done >> as tunefs I would prefer to err on the side of caution and require the a= dmin >> to explicitly specify everything. > > Hi John. I agree with you, I do prefer a fail of tunefs in this > situation. Before of I have posted the first patch I have done another > one that fails to enable SUJ when SU is disabled and fails to disable > SU when SUJ is enabled. Now this patch is bellow and attached. > > *All the stuff that I will say bellow is based on the attached patch* > > The attached patch fails to disable SU when SUJ is enabled. > And it fails to enable SUJ when SU is disabled. > > Earlier I decided to not post the patch because it will cause > confusion when SU and SUJ is disabled and you try to do something such > as: > # tunefs -j enable -n enable /dev/ad2s1d > > The flags's processing order is alphabetically, so It will try to > enable SUJ first (and it will fail) and after it will enable SU. We > could resolve this by processing '-n' (SU) first and after '-j' (SUJ) > but this schema will make to fail the following command: > # tunefs -j disable -n disable /dev/ad2s1d > > So I think we could create a function for each flag to be processed > and the main() calls this functions as necessary (and main() disabled > the relative flag so the function will not be called again). > > But I'm just a curious doing trivial patches, You can talk about this > with more experience and knowlege. > > =3D=3D=3D=3D=3D=3D=3D=3D=3Dpatch=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > Index: sbin/tunefs/tunefs.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/tunefs/tunefs.c =A0 =A0(revision 208584) > +++ sbin/tunefs/tunefs.c =A0 =A0(working copy) > @@ -341,16 +341,19 @@ > =A0 =A0if (jflag) { > =A0 =A0 =A0 =A0name =3D "soft updates journaling"; > =A0 =A0 =A0 =A0if (strcmp(jvalue, "enable") =3D=3D 0) { > - =A0 =A0 =A0 =A0 =A0 if ((sblock.fs_flags & (FS_DOSOFTDEP | FS_SUJ)) =3D= =3D > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 (FS_DOSOFTDEP | FS_SUJ)) { > + =A0 =A0 =A0 =A0 =A0 if ((sblock.fs_flags & FS_SUJ) =3D=3D FS_SUJ) { > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0warnx("%s remains unchanged as enabled", n= ame); > + =A0 =A0 =A0 =A0 =A0 } else if ((sblock.fs_flags & FS_DOSOFTDEP) !=3D > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 FS_DOSOFTDEP) { > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 warnx("%s cannot be enabled until soft upda= tes is enabled", > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 name); > =A0 =A0 =A0 =A0 =A0 =A0} else if (sblock.fs_clean =3D=3D 0) { > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0warnx("%s cannot be enabled until fsck is = run", > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0name); > =A0 =A0 =A0 =A0 =A0 =A0} else if (journal_alloc(Svalue) !=3D 0) { > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0warnx("%s can not be enabled", name); > =A0 =A0 =A0 =A0 =A0 =A0} else { > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 sblock.fs_flags |=3D FS_DOSOFTDEP | FS_SUJ; > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 sblock.fs_flags |=3D FS_SUJ; I assume FS_DOSOFTDEP is being set elsewhere in the code? > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0warnx("%s set", name); > =A0 =A0 =A0 =A0 =A0 =A0} > =A0 =A0 =A0 =A0} else if (strcmp(jvalue, "disable") =3D=3D 0) { > @@ -459,6 +462,9 @@ > =A0 =A0 =A0 =A0} else if (strcmp(nvalue, "disable") =3D=3D 0) { > =A0 =A0 =A0 =A0 =A0 =A0if ((~sblock.fs_flags & FS_DOSOFTDEP) =3D=3D FS_DO= SOFTDEP) > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0warnx("%s remains unchanged as disabled", = name); > + =A0 =A0 =A0 =A0 =A0 else if ((sblock.fs_flags & FS_SUJ) =3D=3D FS_SUJ) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 warnx("%s cannot be enabled until soft upda= tes \ > +journaling is disabled", name); This message doesn't make sense (enable vs disable). > =A0 =A0 =A0 =A0 =A0 =A0else { > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0sblock.fs_flags &=3D ~FS_DOSOFTDEP; > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0warnx("%s cleared", name); Thanks, -Garrett