On 06/22/2012 19:02 +0200, David Sterba wrote:
On Fri, Jun 22, 2012 at 06:26:44PM +0200, Stefan Behrens wrote:
I still do not understand your reason and the benefit of your change.
The reset command and the read command are two completely different
operations. Therefore I assigned two different ioctl commands. Then you
can use strace to see which command is sent, and you can also grep the
sources without additional effort.
I'll try better.
I see the primary purpose to read the device stats. A standalone stats
reset ioctl does not make much sense, so it should go along with reading
the last state and then reset. So far ok.
.From implementation and interface POV, the reset is a hint how I want
to read the stats, not directly the command.
The switch whether to do the reset or not, as currently implemented, has
to set the ioctl number, while I'd expect to set a flag.
Other concerns are about future adding more flags or reading them back
from kernel. I don't have examples, this is what I base on my experience
that such things happen and then I can only scratch my head 'why didn't
I add just one more byte there'. Spare bytes avoid some of backward
compatibility problems.
As for strace, it could be taught to be verbose and descriptive about
ioctls arguments (the 3rd parameter). Quick example is
$ strace stty sane
...
ioctl(0, SNDCTL_TMR_TIMEBASE or SNDRV_TIMER_IOCTL_NEXT_DEVICE or TCGETS, {B38400 opost isig icanon echo ...}) = 0
...
Currently, strace does not know about btrfs-specific ioctls, eg snapshot:
ioctl(3, 0x50009417, 0x7ffff4ae8850) = 0
david
OK. Then let's do it your way. I will prepare the patch to adapt the
btrfs-progs side, unless you have already created one.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
