[U-Boot] [PATCH 1/2] FAT: check for partition 0 not 1 for whole-disk fs
From: Stephen Warren swarren@nvidia.com
The recent switch to use get_device_and_partition() from do_fat_ls() broke the ability to access a FAT filesystem directly on a whole device; FAT only works within a partition on a device.
This change makes e.g. "fatls mmc 0:0" work; explicitly requesting partition ID 0 is something that get_device_and_partition() fully supports. However, fat_register_device() expects partition ID 1 to be used in the full-disk case; partition ID 1 was previously implicitly specified when the user didn't actually specify a partition ID. Update fat_register_device() to expect the correct ID.
This change does imply that if a user explicitly executes "fatls mmc 0:1" then this will fail, and may be a change in behaviour.
Note that this still prevents "fatls mmc 0:auto" from working. The next patch will fix that.
Signed-off-by: Stephen Warren swarren@nvidia.com --- Tom, this series is really a bug-fix and should go in before the 9-long series I posted earlier today. I'll need to rebase that other series on top of this and repost, once any comments are addressed. --- fs/fat/fat.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/fs/fat/fat.c b/fs/fat/fat.c index 41ae15e..80156c8 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -85,7 +85,7 @@ int fat_register_device(block_dev_desc_t * dev_desc, int part_no)
/* Otherwise it might be a superfloppy (whole-disk FAT filesystem) */ if (!cur_dev) { - if (part_no != 1) { + if (part_no != 0) { printf("** Partition %d not valid on device %d **\n", part_no, dev_desc->dev); return -1;
From: Stephen Warren swarren@nvidia.com
Logically, a disk that contains a raw FAT filesystem does not in fact have a partition table. However, test_part_dos() was claiming that such disks did in fact have a DOS-style partition table. This caused get_device_and_partition() not to return a whole-disk disk_partition_t, since part_type != PART_TYPE_UNKNOWN.
part_dos.c's print_partition_extended() detected the raw FAT filesystem condition and printed a fake partition table that encompassed the whole disk.
However, part_dos.c's get_partition_info_extended() did not return any valid partitions in this case. This combination caused get_device_and_partition() not to find any valid partitions, and hence to return an error.
Fix test_part_dos() not to claim that raw FAT filesystems are DOS partition tables. In turn, this causes get_device_and_partition() to return a whole-disk disk_partition_t, and hence the following commands work:
fatls mmc 0 / fatls mmc 0:auto /
An alternative would be to modify print_partition_extended() to detect raw FAT filesystems, just like print_partition_extended() does, and to return a fake partition in this case. However, this seems logically incorrect, and also duplicates code, since get_device_and_partition() falls back to returning a whole-disk partition when there is no partition table on the device.
Signed-off-by: Stephen Warren swarren@nvidia.com --- disk/part_dos.c | 21 +++++++++------------ 1 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/disk/part_dos.c b/disk/part_dos.c index c9a3e2b..5c454e6 100644 --- a/disk/part_dos.c +++ b/disk/part_dos.c @@ -94,12 +94,13 @@ int test_part_dos (block_dev_desc_t *dev_desc) { ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, dev_desc->blksz);
- if ((dev_desc->block_read(dev_desc->dev, 0, 1, (ulong *) buffer) != 1) || - (buffer[DOS_PART_MAGIC_OFFSET + 0] != 0x55) || - (buffer[DOS_PART_MAGIC_OFFSET + 1] != 0xaa) ) { - return (-1); - } - return (0); + if (dev_desc->block_read(dev_desc->dev, 0, 1, (ulong *) buffer) != 1) + return -1; + + if (test_block_type(buffer) != DOS_MBR) + return -1; + + return 0; }
/* Print a partition that is relative to its Extended partition table @@ -117,17 +118,13 @@ static void print_partition_extended (block_dev_desc_t *dev_desc, int ext_part_s return; } i=test_block_type(buffer); - if(i==-1) { + if (i != DOS_MBR) { printf ("bad MBR sector signature 0x%02x%02x\n", buffer[DOS_PART_MAGIC_OFFSET], buffer[DOS_PART_MAGIC_OFFSET + 1]); return; } - if(i==DOS_PBR) { - printf (" 1\t\t 0\t%10ld\t%2x\n", - dev_desc->lba, buffer[DOS_PBR_MEDIA_TYPE_OFFSET]); - return; - } + /* Print all primary/logical partitions */ pt = (dos_partition_t *) (buffer + DOS_PART_TBL_OFFSET); for (i = 0; i < 4; i++, pt++) {
On Fri, Oct 05, 2012 at 01:17:40PM -0000, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
Logically, a disk that contains a raw FAT filesystem does not in fact have a partition table. However, test_part_dos() was claiming that such disks did in fact have a DOS-style partition table. This caused get_device_and_partition() not to return a whole-disk disk_partition_t, since part_type != PART_TYPE_UNKNOWN.
part_dos.c's print_partition_extended() detected the raw FAT filesystem condition and printed a fake partition table that encompassed the whole disk.
However, part_dos.c's get_partition_info_extended() did not return any valid partitions in this case. This combination caused get_device_and_partition() not to find any valid partitions, and hence to return an error.
Fix test_part_dos() not to claim that raw FAT filesystems are DOS partition tables. In turn, this causes get_device_and_partition() to return a whole-disk disk_partition_t, and hence the following commands work:
fatls mmc 0 / fatls mmc 0:auto /
An alternative would be to modify print_partition_extended() to detect raw FAT filesystems, just like print_partition_extended() does, and to return a fake partition in this case. However, this seems logically incorrect, and also duplicates code, since get_device_and_partition() falls back to returning a whole-disk partition when there is no partition table on the device.
Signed-off-by: Stephen Warren swarren@nvidia.com
Applied to u-boot/master, thanks!
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 10/05/12 16:17, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
The recent switch to use get_device_and_partition() from do_fat_ls() broke the ability to access a FAT filesystem directly on a whole device; FAT only works within a partition on a device.
This change makes e.g. "fatls mmc 0:0" work; explicitly requesting partition ID 0 is something that get_device_and_partition() fully supports. However, fat_register_device() expects partition ID 1 to be used in the full-disk case; partition ID 1 was previously implicitly specified when the user didn't actually specify a partition ID. Update fat_register_device() to expect the correct ID.
This change does imply that if a user explicitly executes "fatls mmc 0:1" then this will fail, and may be a change in behaviour.
So wait, you can't list device 0, bus 1 after this patch?
Note that this still prevents "fatls mmc 0:auto" from working. The next patch will fix that.
Signed-off-by: Stephen Warren swarren@nvidia.com --- Tom, this series is really a bug-fix and should go in before the 9-long series I posted earlier today. I'll need to rebase that other series on top of this and repost, once any comments are addressed.
Should this also go in the release?
- -- Tom
On 10/05/2012 05:27 PM, Tom Rini wrote:
On 10/05/12 16:17, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
The recent switch to use get_device_and_partition() from do_fat_ls() broke the ability to access a FAT filesystem directly on a whole device; FAT only works within a partition on a device.
This change makes e.g. "fatls mmc 0:0" work; explicitly requesting partition ID 0 is something that get_device_and_partition() fully supports. However, fat_register_device() expects partition ID 1 to be used in the full-disk case; partition ID 1 was previously implicitly specified when the user didn't actually specify a partition ID. Update fat_register_device() to expect the correct ID.
This change does imply that if a user explicitly executes "fatls mmc 0:1" then this will fail, and may be a change in behaviour.
So wait, you can't list device 0, bus 1 after this patch?
That's partition 1 not bus 1.
In the context of having a raw FAT filesystem on a device with no partition table, the partition specification "0:1" doesn't work before or after this patch; I believe (if it worked at all ever before) it was broken by the previous get_device_and_partition() rework.
If you do have a partition table, then "0:1" works just fine with/without this patch.
Note that this still prevents "fatls mmc 0:auto" from working. The next patch will fix that.
Signed-off-by: Stephen Warren swarren@nvidia.com --- Tom, this series is really a bug-fix and should go in before the 9-long series I posted earlier today. I'll need to rebase that other series on top of this and repost, once any comments are addressed.
Should this also go in the release?
I imagine so; that's why I rebased it below the previous series I sent which I assume isn't going into this release.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 10/05/12 16:36, Stephen Warren wrote:
On 10/05/2012 05:27 PM, Tom Rini wrote:
On 10/05/12 16:17, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
The recent switch to use get_device_and_partition() from do_fat_ls() broke the ability to access a FAT filesystem directly on a whole device; FAT only works within a partition on a device.
This change makes e.g. "fatls mmc 0:0" work; explicitly requesting partition ID 0 is something that get_device_and_partition() fully supports. However, fat_register_device() expects partition ID 1 to be used in the full-disk case; partition ID 1 was previously implicitly specified when the user didn't actually specify a partition ID. Update fat_register_device() to expect the correct ID.
This change does imply that if a user explicitly executes "fatls mmc 0:1" then this will fail, and may be a change in behaviour.
So wait, you can't list device 0, bus 1 after this patch?
That's partition 1 not bus 1.
Er yes, thinko there.
In the context of having a raw FAT filesystem on a device with no partition table, the partition specification "0:1" doesn't work before or after this patch; I believe (if it worked at all ever before) it was broken by the previous get_device_and_partition() rework.
If you do have a partition table, then "0:1" works just fine with/without this patch.
OK, so the behavior change here, potentially involves 2 partitions without a partition table? What is the case where fatls mmc 0:1 would fail now?
- -- Tom
On 10/05/2012 05:39 PM, Tom Rini wrote:
On 10/05/12 16:36, Stephen Warren wrote:
On 10/05/2012 05:27 PM, Tom Rini wrote:
On 10/05/12 16:17, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
The recent switch to use get_device_and_partition() from do_fat_ls() broke the ability to access a FAT filesystem directly on a whole device; FAT only works within a partition on a device.
This change makes e.g. "fatls mmc 0:0" work; explicitly requesting partition ID 0 is something that get_device_and_partition() fully supports. However, fat_register_device() expects partition ID 1 to be used in the full-disk case; partition ID 1 was previously implicitly specified when the user didn't actually specify a partition ID. Update fat_register_device() to expect the correct ID.
This change does imply that if a user explicitly executes "fatls mmc 0:1" then this will fail, and may be a change in behaviour.
So wait, you can't list device 0, bus 1 after this patch?
That's partition 1 not bus 1.
Er yes, thinko there.
In the context of having a raw FAT filesystem on a device with no partition table, the partition specification "0:1" doesn't work before or after this patch; I believe (if it worked at all ever before) it was broken by the previous get_device_and_partition() rework.
If you do have a partition table, then "0:1" works just fine with/without this patch.
OK, so the behavior change here, potentially involves 2 partitions without a partition table? What is the case where fatls mmc 0:1 would fail now?
The failure case involves zero partitions. The issue is that the following works just fine:
fdisk /dev/sda mkfs.dos /dev/sdaN for any N
but the following fails:
mkfs.dos /dev/sda (i.e. raw on the disk with no partition table)
In the latter case, I /think/ either of the following might have worked in U-Boot until recently:
(1) fatls mmc 0 / # Surely this worked (2) fatls mmc 0:1 / # I'm not sure if this works
This patch makes (1) above work.
Patch 2 in this series makes the new equivalent of (2) work, namely:
fatls mmc 0:0 /
The second 0 there is "partition 0" which means "the whole disk". IIRC, I called out that behaviour change in the get_device_and_partition() series anyway.
On Fri, Oct 05, 2012 at 01:17:39PM -0000, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
The recent switch to use get_device_and_partition() from do_fat_ls() broke the ability to access a FAT filesystem directly on a whole device; FAT only works within a partition on a device.
This change makes e.g. "fatls mmc 0:0" work; explicitly requesting partition ID 0 is something that get_device_and_partition() fully supports. However, fat_register_device() expects partition ID 1 to be used in the full-disk case; partition ID 1 was previously implicitly specified when the user didn't actually specify a partition ID. Update fat_register_device() to expect the correct ID.
This change does imply that if a user explicitly executes "fatls mmc 0:1" then this will fail, and may be a change in behaviour.
Note that this still prevents "fatls mmc 0:auto" from working. The next patch will fix that.
Signed-off-by: Stephen Warren swarren@nvidia.com
Tom, this series is really a bug-fix and should go in before the 9-long series I posted earlier today. I'll need to rebase that other series on top of this and repost, once any comments are addressed.
Applied to u-boot/master, thanks!
participants (2)
-
Stephen Warren -
Tom Rini