Re: [Gluster-devel] quota
Vijay Bellur wrote: > You would need to set features.soft-timeout and features.hard-timeout > values to 0 when testing with lower values of directory quota. It works more like expected this way, but there are still oddities: for instance once quota is reached, I can still append smal chunk to a file, and do it a lot of times. A few debug printf tell me this is because of write-behind: the writing process gets success, then glusterfs attemps to write - and fail. This means we silently discard data, which does not looks nice. Is it the expected behavior or is it a NetBSD bug? I assume it is expected but undesirable behavior: couldn't we check for quota space left, and avoid write behind if we aregoing to hit the barrier? -- Emmanuel Dreyfus http://hcpnet.free.fr/pubz m...@netbsd.org ___ Gluster-devel mailing list Gluster-devel@gluster.org http://supercolony.gluster.org/mailman/listinfo/gluster-devel
Re: [Gluster-devel] if/else coding style :-)
> Without taking sides: the last grep is including else without either { or }.
That's true. I stand corrected.
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://supercolony.gluster.org/mailman/listinfo/gluster-devel
Re: [Gluster-devel] if/else coding style :-)
On Mon, Oct 13, 2014 at 2:00 PM, Shyam wrote:
> (apologies, last one on the metrics from me :), as I believe it is more
> about style than actual numbers at a point)
>
> _maybe_ this is better, and it is pretty close to call now ;)
>
> find -name '*.c' | xargs grep else | wc -l
> 3719
> find -name '*.c' | xargs grep else | grep '}' | wc -l
> 1986
> find -name '*.c' | xargs grep else | grep -v '}' | wc -l
> 1733
Without taking sides: the last grep is including else without either { or }.
[~/work/glusterfs]
sh$ git grep '} else {' | wc -l
1331
[~/work/glusterfs]
sh$ git grep 'else {' | grep -v '}' | wc -l
142
So going by just numbers, "} else {" is 10x more common than "}\n else {".
I also find that believable based on familiarity of seeing this pattern in
the code. Either way, good idea to stick to one and not allow both in
future code.
Thanks
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://supercolony.gluster.org/mailman/listinfo/gluster-devel
Re: [Gluster-devel] if/else coding style :-)
(apologies, last one on the metrics from me :), as I believe it is more
about style than actual numbers at a point)
_maybe_ this is better, and it is pretty close to call now ;)
find -name '*.c' | xargs grep else | wc -l
3719
find -name '*.c' | xargs grep else | grep '}' | wc -l
1986
find -name '*.c' | xargs grep else | grep -v '}' | wc -l
1733
Shyam
On 10/13/2014 04:52 PM, Joe Julian wrote:
Not taking sides, though if I were I would support the kernel style
because I, personally, find it easier to read. Just to clarify the point:
$ find -name '*.c' | xargs grep '} else {' | wc -l
1284
$ find -name '*.c' | xargs grep else | grep -v '}' | wc -l
1646
On 10/13/2014 01:46 PM, Shyam wrote:
On 10/13/2014 04:34 PM, Jeff Darcy wrote:
- Original Message -
+1 to existing Linux kernel style. Moreover, its a style which is used
heavily in existing code base. I don't see any advantage in changing
the
style now.
It's not a change. It's already common in our code, if not actually
the *most* common style.
% find . -name '*.c' | xargs grep '} else {' | wc -l
1284
jeff@odroid3 ~/glusterfs (nsr-design)
% find . -name '*.c' | xargs grep 'else {' | wc -l
1431
Jeff, the above would include the references for "} else {" as well,
so ideally it would be something like, 1431 - 1284 = 147 references to
the other (non-kernel?) style.
Jut tried removing the 'wc -l' pipe and saw the same. So currently
based on this method the metric for "} else {" should be the preferred
one (just stating), or I am doing something wrong at my end :)
Shyam
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://supercolony.gluster.org/mailman/listinfo/gluster-devel
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://supercolony.gluster.org/mailman/listinfo/gluster-devel
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://supercolony.gluster.org/mailman/listinfo/gluster-devel
Re: [Gluster-devel] if/else coding style :-)
Not taking sides, though if I were I would support the kernel style
because I, personally, find it easier to read. Just to clarify the point:
$ find -name '*.c' | xargs grep '} else {' | wc -l
1284
$ find -name '*.c' | xargs grep else | grep -v '}' | wc -l
1646
On 10/13/2014 01:46 PM, Shyam wrote:
On 10/13/2014 04:34 PM, Jeff Darcy wrote:
- Original Message -
+1 to existing Linux kernel style. Moreover, its a style which is used
heavily in existing code base. I don't see any advantage in changing
the
style now.
It's not a change. It's already common in our code, if not actually
the *most* common style.
% find . -name '*.c' | xargs grep '} else {' | wc -l
1284
jeff@odroid3 ~/glusterfs (nsr-design)
% find . -name '*.c' | xargs grep 'else {' | wc -l
1431
Jeff, the above would include the references for "} else {" as well,
so ideally it would be something like, 1431 - 1284 = 147 references to
the other (non-kernel?) style.
Jut tried removing the 'wc -l' pipe and saw the same. So currently
based on this method the metric for "} else {" should be the preferred
one (just stating), or I am doing something wrong at my end :)
Shyam
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://supercolony.gluster.org/mailman/listinfo/gluster-devel
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://supercolony.gluster.org/mailman/listinfo/gluster-devel
Re: [Gluster-devel] if/else coding style :-)
On 10/13/2014 04:34 PM, Jeff Darcy wrote:
- Original Message -
+1 to existing Linux kernel style. Moreover, its a style which is used
heavily in existing code base. I don't see any advantage in changing the
style now.
It's not a change. It's already common in our code, if not actually the *most*
common style.
% find . -name '*.c' | xargs grep '} else {' | wc -l
1284
jeff@odroid3 ~/glusterfs (nsr-design)
% find . -name '*.c' | xargs grep 'else {' | wc -l
1431
Jeff, the above would include the references for "} else {" as well, so
ideally it would be something like, 1431 - 1284 = 147 references to the
other (non-kernel?) style.
Jut tried removing the 'wc -l' pipe and saw the same. So currently based
on this method the metric for "} else {" should be the preferred one
(just stating), or I am doing something wrong at my end :)
Shyam
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://supercolony.gluster.org/mailman/listinfo/gluster-devel
Re: [Gluster-devel] jdarcy status (October 2014)
> > More recently, a *completely separate* approach to > > multi-threading - "multi-threaded epoll" - has been getting some > > attention. Here's what I see as the pros and cons of this new approach. > > You forgot: > CON: epoll is Linux specific and code using it is not easily portable. Excellent point. Portability to other platforms is becoming a strong differentiator for GlusterFS, largely thanks to your help, and we should try to keep it that way. ___ Gluster-devel mailing list Gluster-devel@gluster.org http://supercolony.gluster.org/mailman/listinfo/gluster-devel
Re: [Gluster-devel] if/else coding style :-)
- Original Message -
> +1 to existing Linux kernel style. Moreover, its a style which is used
> heavily in existing code base. I don't see any advantage in changing the
> style now.
It's not a change. It's already common in our code, if not actually the *most*
common style.
% find . -name '*.c' | xargs grep '} else {' | wc -l
1284
jeff@odroid3 ~/glusterfs (nsr-design)
% find . -name '*.c' | xargs grep 'else {' | wc -l
1431
I haven't checked whether *all* instances of the latter follow a line with a
closing brace, but I would bet that the vast majority do because "braces around
all if/else blocks" has also been part of our coding style since forever.
Again: checkpatch.pl was *already* rejecting a valid and common form when it
was first introduced. If people want to fix places where they feel that's
still happening, I strongly suggest submitting patches against checkpatch.pl
and we can discuss them there.
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://supercolony.gluster.org/mailman/listinfo/gluster-devel
Re: [Gluster-devel] if/else coding style :-)
>I urge you guys to notify others before making basic style > changes like this. Yes, all style changes - including the one being enforced by the original version of checkpatch.pl - should be submitted for review. ___ Gluster-devel mailing list Gluster-devel@gluster.org http://supercolony.gluster.org/mailman/listinfo/gluster-devel
Re: [Gluster-devel] if/else coding style :-)
On Mon, Oct 13, 2014 at 10:05:27PM +0530, Raghavendra G wrote:
> +1 to existing Linux kernel style. Moreover, its a style which is used
> heavily in existing code base. I don't see any advantage in changing the
> style now.
I fully agree with this. Also, if changes are done, please update the
doc/coding-standard.* files in the sources.
Thanks,
Niels
>
> On Mon, Oct 13, 2014 at 9:12 PM, Kaleb S. KEITHLEY
> wrote:
>
> >
> >
> > ISTR we agreed to use Linux kernel style!
> >
> > Which is
> >
> >if (foo) {
> >/* ... */
> >} else {
> >/* ... */
> >}
> >
> > I don't recall any discussion on -devel about changing this.
> >
> >
> >
> >
> > On 10/13/2014 11:05 AM, Pranith Kumar Karampuri wrote:
> >
> >>
> >> On 10/13/2014 07:43 PM, Shyam wrote:
> >>
> >>> On 10/13/2014 10:08 AM, Pranith Kumar Karampuri wrote:
> >>>
>
> On 10/13/2014 07:27 PM, Shyam wrote:
>
> > On 10/13/2014 08:01 AM, Pranith Kumar Karampuri wrote:
> >
> >> hi,
> >> Why are we moving away from this coding style?:
> >> if (x) {
> >> /*code*/
> >> } else {
> >> /* code */
> >> }
> >>
> >
> > This patch (in master) introduces the same and explains why,
> >
> > commit 0a8371bdfdd88e662d09def717cc0b822feb64e8
> > Author: Jeff Darcy
> > Date: Mon Sep 29 17:27:14 2014 -0400
> >
> > extras: reverse test for '}' vs. following 'else' placement
> >
> > The two-line form "}\nelse {" has been more common than the
> > one-line
> > form "} else {" in our code for years, and IMO for good reason (see
> > the comment in the diff).
> >
> Will there be any objections to allow the previous way of writing this
> if/else block? I just don't want to get any errors in 'check-formatting'
> when I write the old way for this.
> May be we can change it to warning?
>
> >>>
> >>> I am going to state my experience/expectation :)
> >>>
> >>> I actually got this _error_ when submitting a patch, and thought to
> >>> myself "isn't the one-line form the right one?" then went to see why
> >>> this check was in place and read the above. Going by the reason in the
> >>> patch, I just adapted myself.
> >>>
> >>> Now, coming to _allowing_ both forms with a warning, my personal call
> >>> is _no_, we should allow one form so that the code is readable and
> >>> there is little to no confusion for others on which form to use. So I
> >>> would say no to your proposal.
> >>>
> >> Hmm... okay (It is still not an emphatic yes). But it is a waste of time
> >> to talk more about this.
> >>
> >> Jeff/Vijay,
> >>I urge you guys to notify others before making basic style
> >> changes like this.
> >>
> >> Pranith
> >>
> >>>
> >>> Shyam
> >>>
> >>
> >> ___
> >> Gluster-devel mailing list
> >> Gluster-devel@gluster.org
> >> http://supercolony.gluster.org/mailman/listinfo/gluster-devel
> >>
> >
> > --
> >
> > Kaleb
> >
> > ___
> > Gluster-devel mailing list
> > Gluster-devel@gluster.org
> > http://supercolony.gluster.org/mailman/listinfo/gluster-devel
> >
>
>
>
> --
> Raghavendra G
> ___
> Gluster-devel mailing list
> Gluster-devel@gluster.org
> http://supercolony.gluster.org/mailman/listinfo/gluster-devel
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://supercolony.gluster.org/mailman/listinfo/gluster-devel
[Gluster-devel] Failed tests/basic/uss.t
Hello everyone, I have cloned and built today's GlusterFS. The test tests/basic/uss.t fails. The console log and /var/log/glusterfs/mnt-glusterfs-0 attached. Any ideas? Thanks in advance, Edward.= TEST 75 (line 159): fd_close 5 ok 75 RESULT 75: 0 = TEST 76 (line 162): gluster --mode=script --wignore volume set patchy features.snapshot-directory .history ok 76 RESULT 76: 0 = TEST 77 (line 164): ls /mnt/glusterfs/0/.history ls: cannot access /mnt/glusterfs/0/.history: No such file or directory not ok 77 RESULT 77: 2 ls: cannot access /mnt/glusterfs/0/.history: No such file or directory = TEST 78 (line 168): [ 0 == 4 ] not ok 78 RESULT 78: 1 = TEST 79 (line 170): ls /mnt/glusterfs/0/.history/snap1 ls: cannot access /mnt/glusterfs/0/.history/snap1: No such file or directory not ok 79 RESULT 79: 2 = TEST 80 (line 171): ls /mnt/glusterfs/0/.history/snap2 ls: cannot access /mnt/glusterfs/0/.history/snap2: No such file or directory not ok 80 RESULT 80: 2 = TEST 81 (line 172): ls /mnt/glusterfs/0/.history/snap3 ls: cannot access /mnt/glusterfs/0/.history/snap3: No such file or directory not ok 81 RESULT 81: 2 = TEST 82 (line 173): ls /mnt/glusterfs/0/.history/snap4 ls: cannot access /mnt/glusterfs/0/.history/snap4: No such file or directory not ok 82 RESULT 82: 2 = TEST 83 (line 175): ls /mnt/glusterfs/0/.history/snap3/dir1 ls: cannot access /mnt/glusterfs/0/.history/snap3/dir1: No such file or directory not ok 83 RESULT 83: 2 = TEST 84 (line 176): ls /mnt/glusterfs/0/.history/snap3/dir2 ls: cannot access /mnt/glusterfs/0/.history/snap3/dir2: No such file or directory not ok 84 RESULT 84: 2 = TEST 85 (line 178): ls /mnt/glusterfs/0/.history/snap4/dir1 ls: cannot access /mnt/glusterfs/0/.history/snap4/dir1: No such file or directory not ok 85 RESULT 85: 2 = TEST 86 (line 179): ls /mnt/glusterfs/0/.history/snap4/dir2 ls: cannot access /mnt/glusterfs/0/.history/snap4/dir2: No such file or directory not ok 86 RESULT 86: 2 = TEST 87 (line 181): ls /mnt/glusterfs/0/dir1/.history/ ls: cannot access /mnt/glusterfs/0/dir1/.history/: No such file or directory not ok 87 RESULT 87: 2 = TEST 88 (line 182): ! ls /mnt/glusterfs/0/dir1/.history/snap1 ok 88 RESULT 88: 0 = TEST 89 (line 183): ! ls /mnt/glusterfs/0/dir2/.history/snap2 ok 89 RESULT 89: 0 = TEST 90 (line 184): ls /mnt/glusterfs/0/dir1/.history/snap3 ls: cannot access /mnt/glusterfs/0/dir1/.history/snap3: No such file or directory not ok 90 RESULT 90: 2 = TEST 91 (line 185): ls /mnt/glusterfs/0/dir2/.history/snap4 ls: cannot access /mnt/glusterfs/0/dir2/.history/snap4: No such file or directory not ok 91 RESULT 91: 2 = TEST 92 (line 187): fd1=4 ok 92 RESULT 92: 0 = TEST 93 (line 188): fd_open 4 r /mnt/glusterfs/0/.history/snap1/file1 ./tests/basic/../fileio.rc: line 21: /mnt/glusterfs/0/.history/snap1/file1: No such file or directory not ok 93 RESULT 93: 1 = TEST 94 (line 189): fd_cat 4 ./tests/basic/../fileio.rc: line 35: 4: Bad file descriptor not ok 94 RESULT 94: 1 = TEST 95 (line 192): fd2=4 ok 95 RESULT 95: 0 = TEST 96 (line 193): ! fd_open 4 w /mnt/glusterfs/0/.history/snap1/file2 ok 96 RESULT 96: 0 = TEST 97 (line 196): ! stat /mnt/glusterfs/0/.history/snap1/.history ok 97 RESULT 97: 0 = TEST 98 (line 199): ! mkdir /mnt/glusterfs/0/.history/new ok 98 RESULT 98: 0 = TEST 99 (line 200): ! touch /mnt/glusterfs/0/.history/snap2/other ok 99 RESULT 99: 0 = TEST 100 (line 202): fd3=4 ok 100 RESULT 100: 0 = TEST 101 (line 203): fd_open 4 r /mnt/glusterfs/0/dir1/.history/snap3/foo1 ok 101 RESULT 101: 0 = TEST 102 (line 205): fd_cat 4 ok 102 RESULT 102: 0 = TEST 103 (line 207): fd_close 4 ok 103 RESULT 103: 0 = TEST 104 (line 208): fd_close 4 ok 104 RESULT 104: 0 = TEST 105 (line 209): fd_close 4 ok 105 RESULT 105: 0 = TEST 106 (line 214): ls /mnt/nfs/0/.history ok 106 RESULT 106: 0 = TEST 107 (line 218): [ 4 == 4 ] ok 107 RESULT 107: 0 = TEST 108 (line 220): ls -l /mnt/nfs/0/.history/snap1 ok 108 RESULT 108: 0 = TEST 109 (line 221): ls -l /mnt/nfs/0/.history/snap2 ok 109 RESULT 109: 0 = TEST 110 (line 222): ls -l /mnt/nfs/0/.history/snap3 ok 110 RESULT 110: 0 = TEST 111 (line 223): ls
[Gluster-devel] Upcoming Bug Prioritization meeting Tuesday, 14-Oct
Good day, all Just a reminder that we will be having a GlusterFS bug prioritization meeting Tuesday 14-Oct-2014, 12:00 UTC. More information available at : http://blog.gluster.org/2014/10/whats-that-bug-to-you-glusterfs-bug-priority-meeting/ davemc ___ Gluster-devel mailing list Gluster-devel@gluster.org http://supercolony.gluster.org/mailman/listinfo/gluster-devel
Re: [Gluster-devel] if/else coding style :-)
+1 to existing Linux kernel style. Moreover, its a style which is used
heavily in existing code base. I don't see any advantage in changing the
style now.
On Mon, Oct 13, 2014 at 9:12 PM, Kaleb S. KEITHLEY
wrote:
>
>
> ISTR we agreed to use Linux kernel style!
>
> Which is
>
>if (foo) {
>/* ... */
>} else {
>/* ... */
>}
>
> I don't recall any discussion on -devel about changing this.
>
>
>
>
> On 10/13/2014 11:05 AM, Pranith Kumar Karampuri wrote:
>
>>
>> On 10/13/2014 07:43 PM, Shyam wrote:
>>
>>> On 10/13/2014 10:08 AM, Pranith Kumar Karampuri wrote:
>>>
On 10/13/2014 07:27 PM, Shyam wrote:
> On 10/13/2014 08:01 AM, Pranith Kumar Karampuri wrote:
>
>> hi,
>> Why are we moving away from this coding style?:
>> if (x) {
>> /*code*/
>> } else {
>> /* code */
>> }
>>
>
> This patch (in master) introduces the same and explains why,
>
> commit 0a8371bdfdd88e662d09def717cc0b822feb64e8
> Author: Jeff Darcy
> Date: Mon Sep 29 17:27:14 2014 -0400
>
> extras: reverse test for '}' vs. following 'else' placement
>
> The two-line form "}\nelse {" has been more common than the
> one-line
> form "} else {" in our code for years, and IMO for good reason (see
> the comment in the diff).
>
Will there be any objections to allow the previous way of writing this
if/else block? I just don't want to get any errors in 'check-formatting'
when I write the old way for this.
May be we can change it to warning?
>>>
>>> I am going to state my experience/expectation :)
>>>
>>> I actually got this _error_ when submitting a patch, and thought to
>>> myself "isn't the one-line form the right one?" then went to see why
>>> this check was in place and read the above. Going by the reason in the
>>> patch, I just adapted myself.
>>>
>>> Now, coming to _allowing_ both forms with a warning, my personal call
>>> is _no_, we should allow one form so that the code is readable and
>>> there is little to no confusion for others on which form to use. So I
>>> would say no to your proposal.
>>>
>> Hmm... okay (It is still not an emphatic yes). But it is a waste of time
>> to talk more about this.
>>
>> Jeff/Vijay,
>>I urge you guys to notify others before making basic style
>> changes like this.
>>
>> Pranith
>>
>>>
>>> Shyam
>>>
>>
>> ___
>> Gluster-devel mailing list
>> Gluster-devel@gluster.org
>> http://supercolony.gluster.org/mailman/listinfo/gluster-devel
>>
>
> --
>
> Kaleb
>
> ___
> Gluster-devel mailing list
> Gluster-devel@gluster.org
> http://supercolony.gluster.org/mailman/listinfo/gluster-devel
>
--
Raghavendra G
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://supercolony.gluster.org/mailman/listinfo/gluster-devel
Re: [Gluster-devel] if/else coding style :-)
ISTR we agreed to use Linux kernel style!
Which is
if (foo) {
/* ... */
} else {
/* ... */
}
I don't recall any discussion on -devel about changing this.
On 10/13/2014 11:05 AM, Pranith Kumar Karampuri wrote:
On 10/13/2014 07:43 PM, Shyam wrote:
On 10/13/2014 10:08 AM, Pranith Kumar Karampuri wrote:
On 10/13/2014 07:27 PM, Shyam wrote:
On 10/13/2014 08:01 AM, Pranith Kumar Karampuri wrote:
hi,
Why are we moving away from this coding style?:
if (x) {
/*code*/
} else {
/* code */
}
This patch (in master) introduces the same and explains why,
commit 0a8371bdfdd88e662d09def717cc0b822feb64e8
Author: Jeff Darcy
Date: Mon Sep 29 17:27:14 2014 -0400
extras: reverse test for '}' vs. following 'else' placement
The two-line form "}\nelse {" has been more common than the
one-line
form "} else {" in our code for years, and IMO for good reason (see
the comment in the diff).
Will there be any objections to allow the previous way of writing this
if/else block? I just don't want to get any errors in 'check-formatting'
when I write the old way for this.
May be we can change it to warning?
I am going to state my experience/expectation :)
I actually got this _error_ when submitting a patch, and thought to
myself "isn't the one-line form the right one?" then went to see why
this check was in place and read the above. Going by the reason in the
patch, I just adapted myself.
Now, coming to _allowing_ both forms with a warning, my personal call
is _no_, we should allow one form so that the code is readable and
there is little to no confusion for others on which form to use. So I
would say no to your proposal.
Hmm... okay (It is still not an emphatic yes). But it is a waste of time
to talk more about this.
Jeff/Vijay,
I urge you guys to notify others before making basic style
changes like this.
Pranith
Shyam
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://supercolony.gluster.org/mailman/listinfo/gluster-devel
--
Kaleb
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://supercolony.gluster.org/mailman/listinfo/gluster-devel
Re: [Gluster-devel] if/else coding style :-)
On 10/13/2014 07:43 PM, Shyam wrote:
On 10/13/2014 10:08 AM, Pranith Kumar Karampuri wrote:
On 10/13/2014 07:27 PM, Shyam wrote:
On 10/13/2014 08:01 AM, Pranith Kumar Karampuri wrote:
hi,
Why are we moving away from this coding style?:
if (x) {
/*code*/
} else {
/* code */
}
This patch (in master) introduces the same and explains why,
commit 0a8371bdfdd88e662d09def717cc0b822feb64e8
Author: Jeff Darcy
Date: Mon Sep 29 17:27:14 2014 -0400
extras: reverse test for '}' vs. following 'else' placement
The two-line form "}\nelse {" has been more common than the
one-line
form "} else {" in our code for years, and IMO for good reason (see
the comment in the diff).
Will there be any objections to allow the previous way of writing this
if/else block? I just don't want to get any errors in 'check-formatting'
when I write the old way for this.
May be we can change it to warning?
I am going to state my experience/expectation :)
I actually got this _error_ when submitting a patch, and thought to
myself "isn't the one-line form the right one?" then went to see why
this check was in place and read the above. Going by the reason in the
patch, I just adapted myself.
Now, coming to _allowing_ both forms with a warning, my personal call
is _no_, we should allow one form so that the code is readable and
there is little to no confusion for others on which form to use. So I
would say no to your proposal.
Hmm... okay (It is still not an emphatic yes). But it is a waste of time
to talk more about this.
Jeff/Vijay,
I urge you guys to notify others before making basic style
changes like this.
Pranith
Shyam
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://supercolony.gluster.org/mailman/listinfo/gluster-devel
Re: [Gluster-devel] if/else coding style :-)
+1 on choosing a single brace style [1] for the entire codebase.
[1]
http://en.wikipedia.org/wiki/Indent_style
- Original Message -
> From: "Shyam"
> To: "Pranith Kumar Karampuri" , gluster-devel@gluster.org
> Sent: Monday, October 13, 2014 10:13:38 AM
> Subject: Re: [Gluster-devel] if/else coding style :-)
>
> On 10/13/2014 10:08 AM, Pranith Kumar Karampuri wrote:
> >
> > On 10/13/2014 07:27 PM, Shyam wrote:
> >> On 10/13/2014 08:01 AM, Pranith Kumar Karampuri wrote:
> >>> hi,
> >>> Why are we moving away from this coding style?:
> >>> if (x) {
> >>> /*code*/
> >>> } else {
> >>> /* code */
> >>> }
> >>
> >> This patch (in master) introduces the same and explains why,
> >>
> >> commit 0a8371bdfdd88e662d09def717cc0b822feb64e8
> >> Author: Jeff Darcy
> >> Date: Mon Sep 29 17:27:14 2014 -0400
> >>
> >> extras: reverse test for '}' vs. following 'else' placement
> >>
> >> The two-line form "}\nelse {" has been more common than the one-line
> >> form "} else {" in our code for years, and IMO for good reason (see
> >> the comment in the diff).
> > Will there be any objections to allow the previous way of writing this
> > if/else block? I just don't want to get any errors in 'check-formatting'
> > when I write the old way for this.
> > May be we can change it to warning?
>
> I am going to state my experience/expectation :)
>
> I actually got this _error_ when submitting a patch, and thought to
> myself "isn't the one-line form the right one?" then went to see why
> this check was in place and read the above. Going by the reason in the
> patch, I just adapted myself.
>
> Now, coming to _allowing_ both forms with a warning, my personal call is
> _no_, we should allow one form so that the code is readable and there is
> little to no confusion for others on which form to use. So I would say
> no to your proposal.
>
> Shyam
> ___
> Gluster-devel mailing list
> Gluster-devel@gluster.org
> http://supercolony.gluster.org/mailman/listinfo/gluster-devel
>
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://supercolony.gluster.org/mailman/listinfo/gluster-devel
Re: [Gluster-devel] quota
On 10/13/2014 06:19 PM, Emmanuel Dreyfus wrote: Hello Testing quota feature on NetBSD, I get strange results, I wonder if this is what should happen. If I set a directory quota to 4 MB, the start writing to it uwing dd bs=1024k, I will get EDQUOTA after 15 blocks (that is 15 MB). Is there a minimal granularity, or do I have a bug to fix? You would need to set features.soft-timeout and features.hard-timeout values to 0 when testing with lower values of directory quota. I have a WIP patch describing these options at [1]. -Vijay [1] http://review.gluster.org/#/c/8423/1/doc/admin-guide/en-US/markdown/admin_quota.md ___ Gluster-devel mailing list Gluster-devel@gluster.org http://supercolony.gluster.org/mailman/listinfo/gluster-devel
Re: [Gluster-devel] if/else coding style :-)
On 10/13/2014 10:08 AM, Pranith Kumar Karampuri wrote:
On 10/13/2014 07:27 PM, Shyam wrote:
On 10/13/2014 08:01 AM, Pranith Kumar Karampuri wrote:
hi,
Why are we moving away from this coding style?:
if (x) {
/*code*/
} else {
/* code */
}
This patch (in master) introduces the same and explains why,
commit 0a8371bdfdd88e662d09def717cc0b822feb64e8
Author: Jeff Darcy
Date: Mon Sep 29 17:27:14 2014 -0400
extras: reverse test for '}' vs. following 'else' placement
The two-line form "}\nelse {" has been more common than the one-line
form "} else {" in our code for years, and IMO for good reason (see
the comment in the diff).
Will there be any objections to allow the previous way of writing this
if/else block? I just don't want to get any errors in 'check-formatting'
when I write the old way for this.
May be we can change it to warning?
I am going to state my experience/expectation :)
I actually got this _error_ when submitting a patch, and thought to
myself "isn't the one-line form the right one?" then went to see why
this check was in place and read the above. Going by the reason in the
patch, I just adapted myself.
Now, coming to _allowing_ both forms with a warning, my personal call is
_no_, we should allow one form so that the code is readable and there is
little to no confusion for others on which form to use. So I would say
no to your proposal.
Shyam
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://supercolony.gluster.org/mailman/listinfo/gluster-devel
Re: [Gluster-devel] if/else coding style :-)
On 10/13/2014 07:27 PM, Shyam wrote:
On 10/13/2014 08:01 AM, Pranith Kumar Karampuri wrote:
hi,
Why are we moving away from this coding style?:
if (x) {
/*code*/
} else {
/* code */
}
This patch (in master) introduces the same and explains why,
commit 0a8371bdfdd88e662d09def717cc0b822feb64e8
Author: Jeff Darcy
Date: Mon Sep 29 17:27:14 2014 -0400
extras: reverse test for '}' vs. following 'else' placement
The two-line form "}\nelse {" has been more common than the one-line
form "} else {" in our code for years, and IMO for good reason (see
the comment in the diff).
Will there be any objections to allow the previous way of writing this
if/else block? I just don't want to get any errors in 'check-formatting'
when I write the old way for this.
May be we can change it to warning?
Pranith
Change-Id: Ic22c76fe76f0d91300daff36e755a18a8db58852
Signed-off-by: Jeff Darcy
Reviewed-on: http://review.gluster.org/
Tested-by: Gluster Build System
Reviewed-by: Vijay Bellur
Shyam
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://supercolony.gluster.org/mailman/listinfo/gluster-devel
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://supercolony.gluster.org/mailman/listinfo/gluster-devel
Re: [Gluster-devel] if/else coding style :-)
On 10/13/2014 08:01 AM, Pranith Kumar Karampuri wrote:
hi,
Why are we moving away from this coding style?:
if (x) {
/*code*/
} else {
/* code */
}
This patch (in master) introduces the same and explains why,
commit 0a8371bdfdd88e662d09def717cc0b822feb64e8
Author: Jeff Darcy
Date: Mon Sep 29 17:27:14 2014 -0400
extras: reverse test for '}' vs. following 'else' placement
The two-line form "}\nelse {" has been more common than the one-line
form "} else {" in our code for years, and IMO for good reason (see
the comment in the diff).
Change-Id: Ic22c76fe76f0d91300daff36e755a18a8db58852
Signed-off-by: Jeff Darcy
Reviewed-on: http://review.gluster.org/
Tested-by: Gluster Build System
Reviewed-by: Vijay Bellur
Shyam
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://supercolony.gluster.org/mailman/listinfo/gluster-devel
Re: [Gluster-devel] if/else coding style :-)
On Monday 13 October 2014 05:31 PM, Pranith Kumar Karampuri wrote:
hi,
Why are we moving away from this coding style?:
if (x) {
/*code*/
} else {
/* code */
}
Pranith
For me the script that checks the coding style (checkpatch.pl - which is
present in the extras directory within the glusterfs repo) gave the
above style as an error when I executed rfc.sh for submitting the patch.
And the patch will not be submitted to gerrit if errors are reported by
the script.
Regards,
Raghavendra Bhat
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://supercolony.gluster.org/mailman/listinfo/gluster-devel
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://supercolony.gluster.org/mailman/listinfo/gluster-devel
[Gluster-devel] quota
Hello Testing quota feature on NetBSD, I get strange results, I wonder if this is what should happen. If I set a directory quota to 4 MB, the start writing to it uwing dd bs=1024k, I will get EDQUOTA after 15 blocks (that is 15 MB). Is there a minimal granularity, or do I have a bug to fix? -- Emmanuel Dreyfus m...@netbsd.org ___ Gluster-devel mailing list Gluster-devel@gluster.org http://supercolony.gluster.org/mailman/listinfo/gluster-devel
[Gluster-devel] if/else coding style :-)
hi,
Why are we moving away from this coding style?:
if (x) {
/*code*/
} else {
/* code */
}
Pranith
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://supercolony.gluster.org/mailman/listinfo/gluster-devel
Re: [Gluster-devel] Invalid DIR * usage in quota xlator
On Mon, Oct 13, 2014 at 02:58:12PM +0530, Pranith Kumar Karampuri wrote: > I do not know :-(. Send the patch out for review, may be someone with this > knowledge can do the review... Here it is: http://review.gluster.org/8926 -- Emmanuel Dreyfus m...@netbsd.org ___ Gluster-devel mailing list Gluster-devel@gluster.org http://supercolony.gluster.org/mailman/listinfo/gluster-devel
Re: [Gluster-devel] Invalid DIR * usage in quota xlator
On 10/13/2014 02:45 PM, Emmanuel Dreyfus wrote: On Mon, Oct 13, 2014 at 02:37:12PM +0530, Pranith Kumar Karampuri wrote: I am not aware of backend filesystems that much, may be someone with that knowledge can comment here, what happens when new entries are created in the directory after this readdir is responded with '-1'? In the meantime, I changed -1 to sizeof (void *) to address alignment issues. I am not sure we have cases where we loop awaiting for new entries to be added at the end of the directory. That is doomed to fail anyway because getdents(2) cn return deleted entries, and a new entry may be added before the end of the buffer. I do not know :-(. Send the patch out for review, may be someone with this knowledge can do the review... Pranith ___ Gluster-devel mailing list Gluster-devel@gluster.org http://supercolony.gluster.org/mailman/listinfo/gluster-devel
Re: [Gluster-devel] Invalid DIR * usage in quota xlator
On Mon, Oct 13, 2014 at 02:37:12PM +0530, Pranith Kumar Karampuri wrote: > I am not aware of backend filesystems that much, may be someone with that > knowledge can comment here, what happens when new entries are created in the > directory after this readdir is responded with '-1'? In the meantime, I changed -1 to sizeof (void *) to address alignment issues. I am not sure we have cases where we loop awaiting for new entries to be added at the end of the directory. That is doomed to fail anyway because getdents(2) cn return deleted entries, and a new entry may be added before the end of the buffer. -- Emmanuel Dreyfus m...@netbsd.org ___ Gluster-devel mailing list Gluster-devel@gluster.org http://supercolony.gluster.org/mailman/listinfo/gluster-devel
Re: [Gluster-devel] Invalid DIR * usage in quota xlator
On 10/13/2014 02:37 PM, Pranith Kumar Karampuri wrote:
On 10/13/2014 02:27 PM, Emmanuel Dreyfus wrote:
On Mon, Oct 13, 2014 at 01:42:38PM +0530, Pranith Kumar Karampuri wrote:
No bug here, just suboptimal behavior, both in glusterfs and NetBSD
FUSE.
oh!, but shouldn't it get op_ret = 0 instead of op_ret -1, op_errno
EINVAL?
It happens because of thei change I introduced:
seekdir (dir, (long)off);
#ifndef GF_LINUX_HOST_OS
if (telldir(dir) != off) {
gf_log (THIS->name, GF_LOG_ERROR,
"seekdir(%ld) failed on dir=%p: "
"Invalid argument (offset reused from "
"another DIR * structure?)",
(long)off, dir);
errno = EINVAL;
count = -1;
goto out;
}
#endif /* GF_LINUX_HOST_OS */
The idea is to catch wrong opendir/closedir usage, and the errno =
EINVAL
is critical in such a situation to avoid an infinite loop. On the other
hand it also fires for last entry. I did not notice it before because
this EINVAL will not reach calling process, but it may cause trouble
for internal glusterfs usage.
I see an easy way yo fix it (patch below)
- when posix xlator detects EOF, return an offset of -1 in last entry.
- When called with an offset of -1, immediatly return op_ret = 0 with
no entry
What do you think?
I am not aware of backend filesystems that much, may be someone with
that knowledge can comment here, what happens when new entries are
created in the directory after this readdir is responded with '-1'?
diff --git a/xlators/storage/posix/src/posix.c
b/xlators/storage/posix/src/posix
index 6b12d39..3e539bb 100644
--- a/xlators/storage/posix/src/posix.c
+++ b/xlators/storage/posix/src/posix.c
@@ -79,6 +79,10 @@ extern char *marker_xattrs[];
#define SET_TO_OLD_FS_ID()
#endif
+
+/* offset < sizeof(struct dirent) cannot be valid */
+#define INVALID_DIRENT_OFFSET sizeof (void *)
+
int
posix_forget (xlator_t *this, inode_t *inode)
{
@@ -4851,6 +4855,10 @@ posix_fill_readdir (fd_t *fd, DIR *dir, off_t
off, size_t
int len = 0;
int ret = 0;
+/* EOF was previously detected */
+if (off == INVALID_DIRENT_OFFSET)
+ goto out;
+
if (skip_dirs) {
len = posix_handle_path (this, fd->inode->gfid,
NULL, NULL, 0);
hpath = alloca (len + 256); /* NAME_MAX */
@@ -4969,9 +4977,12 @@ posix_fill_readdir (fd_t *fd, DIR *dir, off_t
off, size_t
count ++;
}
-if ((!readdir (dir) && (errno == 0)))
+if ((!readdir (dir) && (errno == 0))) {
/* Indicate EOF */
errno = ENOENT;
Now I understand why you kept saying ENOENT ;-). This may not be a
good idea IMO. Code all through gluster doesn't care what errno it
unwinds upwards.
In case op_ret is zero I mean :-)
Pranith
+/* Set invalid offset for later detection */
+this_entry->d_off = INVALID_DIRENT_OFFSET;
+}
out:
return count;
}
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://supercolony.gluster.org/mailman/listinfo/gluster-devel
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://supercolony.gluster.org/mailman/listinfo/gluster-devel
Re: [Gluster-devel] Invalid DIR * usage in quota xlator
On 10/13/2014 02:27 PM, Emmanuel Dreyfus wrote:
On Mon, Oct 13, 2014 at 01:42:38PM +0530, Pranith Kumar Karampuri wrote:
No bug here, just suboptimal behavior, both in glusterfs and NetBSD FUSE.
oh!, but shouldn't it get op_ret = 0 instead of op_ret -1, op_errno EINVAL?
It happens because of thei change I introduced:
seekdir (dir, (long)off);
#ifndef GF_LINUX_HOST_OS
if (telldir(dir) != off) {
gf_log (THIS->name, GF_LOG_ERROR,
"seekdir(%ld) failed on dir=%p: "
"Invalid argument (offset reused from "
"another DIR * structure?)", (long)off, dir);
errno = EINVAL;
count = -1;
goto out;
}
#endif /* GF_LINUX_HOST_OS */
The idea is to catch wrong opendir/closedir usage, and the errno = EINVAL
is critical in such a situation to avoid an infinite loop. On the other
hand it also fires for last entry. I did not notice it before because
this EINVAL will not reach calling process, but it may cause trouble
for internal glusterfs usage.
I see an easy way yo fix it (patch below)
- when posix xlator detects EOF, return an offset of -1 in last entry.
- When called with an offset of -1, immediatly return op_ret = 0 with no entry
What do you think?
I am not aware of backend filesystems that much, may be someone with
that knowledge can comment here, what happens when new entries are
created in the directory after this readdir is responded with '-1'?
diff --git a/xlators/storage/posix/src/posix.c b/xlators/storage/posix/src/posix
index 6b12d39..3e539bb 100644
--- a/xlators/storage/posix/src/posix.c
+++ b/xlators/storage/posix/src/posix.c
@@ -79,6 +79,10 @@ extern char *marker_xattrs[];
#define SET_TO_OLD_FS_ID()
#endif
+
+/* offset < sizeof(struct dirent) cannot be valid */
+#define INVALID_DIRENT_OFFSET sizeof (void *)
+
int
posix_forget (xlator_t *this, inode_t *inode)
{
@@ -4851,6 +4855,10 @@ posix_fill_readdir (fd_t *fd, DIR *dir, off_t off, size_t
int len = 0;
int ret = 0;
+/* EOF was previously detected */
+if (off == INVALID_DIRENT_OFFSET)
+ goto out;
+
if (skip_dirs) {
len = posix_handle_path (this, fd->inode->gfid, NULL, NULL,
0);
hpath = alloca (len + 256); /* NAME_MAX */
@@ -4969,9 +4977,12 @@ posix_fill_readdir (fd_t *fd, DIR *dir, off_t off, size_t
count ++;
}
-if ((!readdir (dir) && (errno == 0)))
+if ((!readdir (dir) && (errno == 0))) {
/* Indicate EOF */
errno = ENOENT;
Now I understand why you kept saying ENOENT ;-). This may not be a good
idea IMO. Code all through gluster doesn't care what errno it unwinds
upwards.
+/* Set invalid offset for later detection */
+this_entry->d_off = INVALID_DIRENT_OFFSET;
+}
out:
return count;
}
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://supercolony.gluster.org/mailman/listinfo/gluster-devel
Re: [Gluster-devel] Invalid DIR * usage in quota xlator
On Mon, Oct 13, 2014 at 01:42:38PM +0530, Pranith Kumar Karampuri wrote:
> >No bug here, just suboptimal behavior, both in glusterfs and NetBSD FUSE.
> oh!, but shouldn't it get op_ret = 0 instead of op_ret -1, op_errno EINVAL?
It happens because of thei change I introduced:
seekdir (dir, (long)off);
#ifndef GF_LINUX_HOST_OS
if (telldir(dir) != off) {
gf_log (THIS->name, GF_LOG_ERROR,
"seekdir(%ld) failed on dir=%p: "
"Invalid argument (offset reused from "
"another DIR * structure?)", (long)off, dir);
errno = EINVAL;
count = -1;
goto out;
}
#endif /* GF_LINUX_HOST_OS */
The idea is to catch wrong opendir/closedir usage, and the errno = EINVAL
is critical in such a situation to avoid an infinite loop. On the other
hand it also fires for last entry. I did not notice it before because
this EINVAL will not reach calling process, but it may cause trouble
for internal glusterfs usage.
I see an easy way yo fix it (patch below)
- when posix xlator detects EOF, return an offset of -1 in last entry.
- When called with an offset of -1, immediatly return op_ret = 0 with no entry
What do you think?
diff --git a/xlators/storage/posix/src/posix.c b/xlators/storage/posix/src/posix
index 6b12d39..3e539bb 100644
--- a/xlators/storage/posix/src/posix.c
+++ b/xlators/storage/posix/src/posix.c
@@ -79,6 +79,10 @@ extern char *marker_xattrs[];
#define SET_TO_OLD_FS_ID()
#endif
+
+/* offset < sizeof(struct dirent) cannot be valid */
+#define INVALID_DIRENT_OFFSET sizeof (void *)
+
int
posix_forget (xlator_t *this, inode_t *inode)
{
@@ -4851,6 +4855,10 @@ posix_fill_readdir (fd_t *fd, DIR *dir, off_t off, size_t
int len = 0;
int ret = 0;
+/* EOF was previously detected */
+if (off == INVALID_DIRENT_OFFSET)
+ goto out;
+
if (skip_dirs) {
len = posix_handle_path (this, fd->inode->gfid, NULL, NULL, 0);
hpath = alloca (len + 256); /* NAME_MAX */
@@ -4969,9 +4977,12 @@ posix_fill_readdir (fd_t *fd, DIR *dir, off_t off, size_t
count ++;
}
-if ((!readdir (dir) && (errno == 0)))
+if ((!readdir (dir) && (errno == 0))) {
/* Indicate EOF */
errno = ENOENT;
+/* Set invalid offset for later detection */
+this_entry->d_off = INVALID_DIRENT_OFFSET;
+}
out:
return count;
}
--
Emmanuel Dreyfus
m...@netbsd.org
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://supercolony.gluster.org/mailman/listinfo/gluster-devel
Re: [Gluster-devel] Invalid DIR * usage in quota xlator
On 10/13/2014 01:14 PM, Emmanuel Dreyfus wrote: On Mon, Oct 13, 2014 at 12:39:33PM +0530, Pranith Kumar Karampuri wrote: Op_errno is valid only if 'op_ret < 0'. so that doesn't say much. After the last readdir call with op_ret > 0, there will be one more readdir call for which op_ret will come as '0'. That means reading of the directory is complete. Right, I have 3 calls for end of directory: op_ret > 0, op_errno = ENOENT, but glusterfs does an extra call and gets: op_ret = -1, op_errno = EINVAL, but NetBSD FUSE does an extra call ang gets: op_ret = -1, op_errno = EINVAL No bug here, just suboptimal behavior, both in glusterfs and NetBSD FUSE. oh!, but shouldn't it get op_ret = 0 instead of op_ret -1, op_errno EINVAL? Pranith ___ Gluster-devel mailing list Gluster-devel@gluster.org http://supercolony.gluster.org/mailman/listinfo/gluster-devel
Re: [Gluster-devel] Invalid DIR * usage in quota xlator
On Mon, Oct 13, 2014 at 12:39:33PM +0530, Pranith Kumar Karampuri wrote: > Op_errno is valid only if 'op_ret < 0'. so that doesn't say much. After the > last readdir call with op_ret > 0, there will be one more readdir call for > which op_ret will come as '0'. That means reading of the directory is > complete. Right, I have 3 calls for end of directory: op_ret > 0, op_errno = ENOENT, but glusterfs does an extra call and gets: op_ret = -1, op_errno = EINVAL, but NetBSD FUSE does an extra call ang gets: op_ret = -1, op_errno = EINVAL No bug here, just suboptimal behavior, both in glusterfs and NetBSD FUSE. -- Emmanuel Dreyfus m...@netbsd.org ___ Gluster-devel mailing list Gluster-devel@gluster.org http://supercolony.gluster.org/mailman/listinfo/gluster-devel
Re: [Gluster-devel] Invalid DIR * usage in quota xlator
On 10/13/2014 12:19 PM, Emmanuel Dreyfus wrote: On Mon, Oct 13, 2014 at 10:32:12AM +0530, Pranith Kumar Karampuri wrote: End of directory is determined by 'op_ret == 0'. I get op_ret > 0, op_errno == ENOENT at quota xlator level. On my system with xfs as backend, things are working fine. Which backend filesystem are you using? FFS, but I am not sure that is the problem since ENOENT on last entry is produced by glusterfs itself. Op_errno is valid only if 'op_ret < 0'. so that doesn't say much. After the last readdir call with op_ret > 0, there will be one more readdir call for which op_ret will come as '0'. That means reading of the directory is complete. Pranith I wil investigate further. ___ Gluster-devel mailing list Gluster-devel@gluster.org http://supercolony.gluster.org/mailman/listinfo/gluster-devel
