[haiku-development] Re: Partition block size confusion

  • From: Jessica Hamilton <jessica.l.hamilton@xxxxxxxxx>
  • To: haiku-development@xxxxxxxxxxxxx
  • Date: Tue, 27 Sep 2016 20:44:06 +0000

On Wed, 28 Sep 2016 9:28 am Adrien Destugues <pulkomandy@xxxxxxxxxxxxx>
wrote:

Hi there,

As you may have noticed, I'm trying to review some of the old patches
pending on Trac. Today I found https://dev.haiku-os.org/ticket/8990 ;,
with a lengthy and somewhat irrelevant discussion. Anyway, it leads to
this version of the patch:

https://dev.haiku-os.org/attachment/ticket/8990/0001-changes-for-block_size-value-being-sector_size.patch

The problem is the use of the block_size field in partition_data and
KPartition. We initialize it with the "physical" block size (either the
disk block size, or whatever the parent partitionning system has set).
However, scanning the partition with different filesystems, they will
all change the field to what they think is the block size.

On to the bug: it happens when you initialize a full disk with BFS. This
sets the block size to 2K. If you then try to reinitialize it with
something else, that 2K size will be used, but it doesn't match the
physical sector size. If you initialize it with another FS, the block
size will be reset. But, if you initialize it with a partitionning
system, it won't.

This does not lead to too bad consequences with our MBR add-on, because
the only place it uses the block size is to check the size limit of the
partition. It will end up allowing partitions larger than 2TB but that's
about it. It may create more problems with GPT, however.

I'm not sure the proposed solution in the patch is correct (it removes
the change of the block size from all filesystem addons). I can think of
several solutions, and I'm not sure which way to go:

1) If the FS add-ons should not change the block_size info, it should be
somehow write-protected. This may be by making partition_data a class
and making the field private with an accessor, for example.


There are already fields in the struct that should be treated as read only,
so I don't see why this needs to be different.

2) Separate block size from "sector size" (or logical/physical), so the
FS can set a size, yet we remember what the underlying layout is.


Adding an additional field seems like the sensible choice here. Isn't the
size filesystems are using more akin to inode size?

3) Partitionning systems should not use the block_size info, and query
the block size of the volume in some other way. I'm not sure there is an
easy way to reach it from the partition add-ons, however (all they get
is the partition_data and a file descriptor to access the partition
device).


This seems overly complex for no good reason, and I would not take this
route. The partition_data is also used in the bootloader.

So, basically, what does the block_size of a partition mean? Currently
it is first set to the disk sector size, but then reused to store the FS
block size of the contained filesystem. It seems it can't be both, so we
need to choose one, or add a second field to store the two values.


Another alternative is to add a field called sector_size, and leave the
addons as is and fix the code that needs to know the physical disk geometry.

Other related posts:

  1. Home
  2. haiku-development
  3. Archive
  4. 09-2016