[haiku-development] Re: Review needed for Sector Size not being reported correctly bug

  • From: Kushal Singh <kushal.spiderman.singh@xxxxxxxxx>
  • To: "haiku-development@xxxxxxxxxxxxx" <haiku-development@xxxxxxxxxxxxx>
  • Date: Sun, 22 Feb 2015 19:51:32 +0000

On Sun Feb 22 2015 at 8:55:14 PM Ingo Weinhold <ingo_weinhold@xxxxxx> wrote:

Long story short, the file systems setting partition_data::block_size to
> their content block size is correct, or IOW your patch 0001 is incorrect
> I have only glanced over the comments in the ticket. From that and the
> not very helpful ticket description unfortunately I cannot say what the
> actual problem is that your patch 0001 is meant to fix. Your comment 17
> [1] seems to imply that when uninitializing/re-initializing a partition
> the block_size field isn't reset correctly.
>
> That is one part of the problem that the value isn't reset correctly after
initialization (this can be solved if I initialize the value in intel
partition).
But the main problem is that suppose I have a bfs partition , In order to
make sure that an intel partition map can be initialized for it , the
CanInitialze() function (this was actually the part that needed correction
initially ) at
http://grok.bikemonkey.org/source/xref/haiku/src/add-ons/disk_systems/intel/PartitionMapAddOn.cpp#90

checks if it is possible or not (the function checks without actual
initialization). It uses BlockSize() function which should return sector
size  as commit  5f1b31debdf97da331be6e97a6c2bc5899e2423a
suggests. However that is not the case as the value that is returned via
BlockSize() is the file block size.

Now the BlockSize() returns the value of fData->block_size. But the value
it gets is from the partition data at
http://grok.bikemonkey.org/source/xref/haiku/src/kits/storage/disk_device/MutablePartition.cpp#561
(which undoubtedly should be file block size now )
and so it does return value of file block size (but this is not what we
want , we want the return value = sector size) .

Now in order to fix the bug ( and the patch) I have the following
suggestions
1) Changing the return value of BlockSize() by using ioctl ( using
bytes_per_sector value, since anyway the value that we get now is surely
not what is expected there) for the return value.
2) Use the current patch 0001 and thus changing the interpretation of
partition->block_size value , note that the changes that I suggested in my
patch does change the interpretation but as you can see from the search at
http://grok.bikemonkey.org/source/search?q=partition%5C-%5C%3Eblock_size&defs=&refs=&path=&hist=&project=haiku

the value of partition->block_size if viewed in a different perspective
does not affect the existing code in any harmful way ( in fact I don't
think does affect any major thing ).

3)Something else , tell me ?

I think I have stated the problem clearly. So what would you suggest  ?

Thanks for helping
Kushal :)

Other related posts: