Date: Thu, 27 May 2010 18:31:06 -0700 From: Garrett Cooper <yanefbsd@gmail.com> To: "Marcelo/Porks" <marcelorossi@gmail.com> Cc: freebsd-current@freebsd.org, Jeff Roberson <jroberson@jroberson.net> Subject: Re: SUJ Changes Message-ID: <AANLkTilMRsLGIeI6aSgUZEpHEBN3uMizenB3wPIeMeMB@mail.gmail.com> In-Reply-To: <AANLkTimwxV7aEVawt3e6oSieyBpuMJ7UFSnz8wdmTARS@mail.gmail.com> References: <alpine.BSF.2.00.1005171616390.1398@desktop> <201005270933.42760.jhb@freebsd.org> <AANLkTilQcxW02tav6LS86fUIxlDuaLPdtggmOqj6CDe1@mail.gmail.com> <201005271147.20155.jhb@freebsd.org> <AANLkTimwxV7aEVawt3e6oSieyBpuMJ7UFSnz8wdmTARS@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, May 27, 2010 at 3:44 PM, Marcelo/Porks <marcelorossi@gmail.com> wro=
te:
> On 5/27/10, John Baldwin <jhb@freebsd.org> wrote:
>> On Thursday 27 May 2010 10:13:38 am Marcelo/Porks wrote:
>>> On Thu, May 27, 2010 at 10:33 AM, John Baldwin <jhb@freebsd.org> wrote:
>>> > On Wednesday 26 May 2010 7:56:24 pm Garrett Cooper wrote:
>>> >> On Wed, May 26, 2010 at 3:52 PM, Marcelo/Porks <marcelorossi@gmail.c=
om>
>>> >>
>> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTilMRsLGIeI6aSgUZEpHEBN3uMizenB3wPIeMeMB>