Date: Thu, 27 May 2010 19:44:40 -0300 From: "Marcelo/Porks" <marcelorossi@gmail.com> To: John Baldwin <jhb@freebsd.org> Cc: Garrett Cooper <yanefbsd@gmail.com>, freebsd-current@freebsd.org, Jeff Roberson <jroberson@jroberson.net> Subject: Re: SUJ Changes Message-ID: <AANLkTimwxV7aEVawt3e6oSieyBpuMJ7UFSnz8wdmTARS@mail.gmail.com> In-Reply-To: <201005271147.20155.jhb@freebsd.org> References: <alpine.BSF.2.00.1005171616390.1398@desktop> <201005270933.42760.jhb@freebsd.org> <AANLkTilQcxW02tav6LS86fUIxlDuaLPdtggmOqj6CDe1@mail.gmail.com> <201005271147.20155.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--00163630f223d1052104879b224b
Content-Type: text/plain; charset=ISO-8859-1
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;
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);
else {
sblock.fs_flags &= ~FS_DOSOFTDEP;
warnx("%s cleared", name);
--
Marcelo Rossi
"This e-mail is provided "AS IS" with no warranties, and confers no rights."
--00163630f223d1052104879b224b
Content-Type: application/octet-stream; name="tunefs.diff"
Content-Disposition: attachment; filename="tunefs.diff"
Content-Transfer-Encoding: base64
X-Attachment-Id: file1
SW5kZXg6IHNiaW4vdHVuZWZzL3R1bmVmcy5jCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIHNiaW4vdHVuZWZzL3R1
bmVmcy5jCShyZXZpc2lvbiAyMDg1ODQpCisrKyBzYmluL3R1bmVmcy90dW5lZnMuYwkod29ya2lu
ZyBjb3B5KQpAQCAtMzQxLDE2ICszNDEsMTkgQEAKIAlpZiAoamZsYWcpIHsKICAJCW5hbWUgPSAi
c29mdCB1cGRhdGVzIGpvdXJuYWxpbmciOwogIAkJaWYgKHN0cmNtcChqdmFsdWUsICJlbmFibGUi
KSA9PSAwKSB7Ci0JCQlpZiAoKHNibG9jay5mc19mbGFncyAmIChGU19ET1NPRlRERVAgfCBGU19T
VUopKSA9PQotCQkJICAgIChGU19ET1NPRlRERVAgfCBGU19TVUopKSB7CisJCQlpZiAoKHNibG9j
ay5mc19mbGFncyAmIEZTX1NVSikgPT0gRlNfU1VKKSB7CiAJCQkJd2FybngoIiVzIHJlbWFpbnMg
dW5jaGFuZ2VkIGFzIGVuYWJsZWQiLCBuYW1lKTsKKwkJCX0gZWxzZSBpZiAoKHNibG9jay5mc19m
bGFncyAmIEZTX0RPU09GVERFUCkgIT0KKwkJCQlGU19ET1NPRlRERVApIHsKKwkJCQl3YXJueCgi
JXMgY2Fubm90IGJlIGVuYWJsZWQgdW50aWwgc29mdCB1cGRhdGVzIGlzIGVuYWJsZWQiLAorCQkJ
CQluYW1lKTsKIAkJCX0gZWxzZSBpZiAoc2Jsb2NrLmZzX2NsZWFuID09IDApIHsKIAkJCQl3YXJu
eCgiJXMgY2Fubm90IGJlIGVuYWJsZWQgdW50aWwgZnNjayBpcyBydW4iLAogCQkJCSAgICBuYW1l
KTsKIAkJCX0gZWxzZSBpZiAoam91cm5hbF9hbGxvYyhTdmFsdWUpICE9IDApIHsKIAkJCQl3YXJu
eCgiJXMgY2FuIG5vdCBiZSBlbmFibGVkIiwgbmFtZSk7CiAJCQl9IGVsc2UgewotIAkJCQlzYmxv
Y2suZnNfZmxhZ3MgfD0gRlNfRE9TT0ZUREVQIHwgRlNfU1VKOworIAkJCQlzYmxvY2suZnNfZmxh
Z3MgfD0gRlNfU1VKOwogIAkJCQl3YXJueCgiJXMgc2V0IiwgbmFtZSk7CiAJCQl9CiAgCQl9IGVs
c2UgaWYgKHN0cmNtcChqdmFsdWUsICJkaXNhYmxlIikgPT0gMCkgewpAQCAtNDU5LDYgKzQ2Miw5
IEBACiAgCQl9IGVsc2UgaWYgKHN0cmNtcChudmFsdWUsICJkaXNhYmxlIikgPT0gMCkgewogCQkJ
aWYgKCh+c2Jsb2NrLmZzX2ZsYWdzICYgRlNfRE9TT0ZUREVQKSA9PSBGU19ET1NPRlRERVApCiAJ
CQkJd2FybngoIiVzIHJlbWFpbnMgdW5jaGFuZ2VkIGFzIGRpc2FibGVkIiwgbmFtZSk7CisJCQll
bHNlIGlmICgoc2Jsb2NrLmZzX2ZsYWdzICYgRlNfU1VKKSA9PSBGU19TVUopCisJCQkJd2Fybngo
IiVzIGNhbm5vdCBiZSBlbmFibGVkIHVudGlsIHNvZnQgdXBkYXRlcyBcCitqb3VybmFsaW5nIGlz
IGRpc2FibGVkIiwgbmFtZSk7CiAJCQllbHNlIHsKICAJCQkJc2Jsb2NrLmZzX2ZsYWdzICY9IH5G
U19ET1NPRlRERVA7CiAgCQkJCXdhcm54KCIlcyBjbGVhcmVkIiwgbmFtZSk7Cg==
--00163630f223d1052104879b224b--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTimwxV7aEVawt3e6oSieyBpuMJ7UFSnz8wdmTARS>