Date: Thu, 27 May 2010 23:04:16 -0300 From: "Marcelo/Porks" <marcelorossi@gmail.com> To: freebsd-current@freebsd.org Subject: Re: SUJ Changes Message-ID: <AANLkTilZn6sxRZpGHXpZqggisdOq02FcRKqMFauEHgzL@mail.gmail.com> In-Reply-To: <AANLkTilMRsLGIeI6aSgUZEpHEBN3uMizenB3wPIeMeMB@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> <AANLkTilMRsLGIeI6aSgUZEpHEBN3uMizenB3wPIeMeMB@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On 5/27/10, Garrett Cooper <yanefbsd@gmail.com> wrote:
> On Thu, May 27, 2010 at 3:44 PM, Marcelo/Porks <marcelorossi@gmail.com>
> wrote:
>> 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.com>
>>>> >>
>>> 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
>>>> >> > user
>>>> >> > disable SU. Maybe this will be useful for some of you.
>>>> >> >
>>>> >> > Thanks.
>>>> >> >
>>>> >> >
>>>> >> > Index: sbin/tunefs/tunefs.c
>>>> >> > ===================================================================
>>>> >> > --- sbin/tunefs/tunefs.c (revision 208580)
>>>> >> > +++ sbin/tunefs/tunefs.c (working copy)
>>>> >> > @@ -460,6 +460,14 @@
>>>> >> > if ((~sblock.fs_flags & FS_DOSOFTDEP) ==
>>>> > FS_DOSOFTDEP)
>>>> >> > warnx("%s remains unchanged as
>>> disabled",
>>>> > name);
>>>> >> > else {
>>>> >> > + /* also disable SUJ */
>>>> >> > + if ((sblock.fs_flags & FS_SUJ) ==
>>> FS_SUJ)
>>>> > {
>>>> >> > + warnx("soft updates
>>>> >> > journaling
>>>> >> > will be disabled too");
>>>> >> > + journal_clear();
>>>> >> > + sblock.fs_flags &= ~FS_SUJ;
>>>> >> > + sblock.fs_sujfree = 0;
>>>> >> > + warnx("remove .sujournal to
>>>> >> > reclaim space");
>>>> >> > + }
>>>> >> > sblock.fs_flags &= ~FS_DOSOFTDEP;
>>>> >> > warnx("%s cleared", name);
>>>> >> > }
>>>> >>
>>>> > I think that attempting to disable SU if SUJ
>>>> > is enabled should just fail with an error message. The sysadmin can
>>>> > 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. :) For something as rarely
>>> done
>>> as tunefs I would prefer to err on the side of caution and require the
>>> admin
>>> 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.
>>
>> =========patch==========
>>
>> Index: sbin/tunefs/tunefs.c
>> ===================================================================
>> --- sbin/tunefs/tunefs.c (revision 208584)
>> +++ sbin/tunefs/tunefs.c (working copy)
>> @@ -341,16 +341,19 @@
>> if (jflag) {
>> name = "soft updates journaling";
>> if (strcmp(jvalue, "enable") == 0) {
>> - if ((sblock.fs_flags & (FS_DOSOFTDEP | FS_SUJ)) ==
>> - (FS_DOSOFTDEP | FS_SUJ)) {
>> + if ((sblock.fs_flags & FS_SUJ) == FS_SUJ) {
>> warnx("%s remains unchanged as enabled", name);
>> + } else if ((sblock.fs_flags & FS_DOSOFTDEP) !=
>> + FS_DOSOFTDEP) {
>> + warnx("%s cannot be enabled until soft updates is
>> enabled",
>> + name);
>> } else if (sblock.fs_clean == 0) {
>> warnx("%s cannot be enabled until fsck is run",
>> name);
>> } else if (journal_alloc(Svalue) != 0) {
>> warnx("%s can not be enabled", name);
>> } else {
>> - sblock.fs_flags |= FS_DOSOFTDEP | FS_SUJ;
>> + sblock.fs_flags |= FS_SUJ;
>
> I assume FS_DOSOFTDEP is being set elsewhere in the code?
>
>> warnx("%s set", name);
>> }
>> } else if (strcmp(jvalue, "disable") == 0) {
>> @@ -459,6 +462,9 @@
>> } else if (strcmp(nvalue, "disable") == 0) {
>> if ((~sblock.fs_flags & FS_DOSOFTDEP) == FS_DOSOFTDEP)
>> warnx("%s remains unchanged as disabled", name);
>> + else if ((sblock.fs_flags & FS_SUJ) == FS_SUJ)
>> + warnx("%s cannot be enabled until soft updates \
>> +journaling is disabled", name);
>
> This message doesn't make sense (enable vs disable).
Oh for god shake, I'm sorry. The correct message was supposed to be:
warnx("%s cannot be disabled until soft updates journaling is disabled", name);
Sorry...
--
Marcelo Rossi
"This e-mail is provided "AS IS" with no warranties, and confers no rights."
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTilZn6sxRZpGHXpZqggisdOq02FcRKqMFauEHgzL>