fdisk: Add support for fixing MBR partitions CHS values

[pali]

Add a new extended option 'F' to fdisk which recalculates and fixes CHS
values for each partition in MBR table according to current fdisk settings
for number of heads and sectors per track.

This allows in interactive mode to repair existing partitions to be
compatible with CHS addressing after changing extended option 'h' (number
of heads) or 's' (sectors per track) as these options do not modify content
of existing partitions.

[karelzak]

It seems like a good idea and it's a completely independent change with a new API function.

Maybe split it into two patches, 1st for libfdisk and 2nd for fdisk.

[karelzak]

BTW, libfdisk/src/dos.c uses on many places dos_partition_set_start() and dos_partition_set_size() but it does not update CHS. The nice example is fdisk_dos_move_begin() (fdisk expert menu and 'b' command), it does not update CHS at all.

What about to add to include/pt-mbr.h a new function dos_partition_sync_chs() to update CHS according to the current start and size? So, in the library we will use

  dos_partition_set_start(p, start);
  dos_partition_set_size(p, size);
  dos_partition_sync_chs(p, cxt->geom.sectors, cxt->geom.head);

and int will internally do what we now do in libfdisk/src/dos.c:set_partition(), it means

        if (start/(cxt->geom.sectors*cxt->geom.heads) > 1023)
                start = cxt->geom.heads*cxt->geom.sectors*1024 - 1;
        set_hsc(p->bh, p->bs, p->bc, start);
        if (stop/(cxt->geom.sectors*cxt->geom.heads) > 1023)
                stop = cxt->geom.heads*cxt->geom.sectors*1024 - 1;
        set_hsc(p->eh, p->es, p->ec, stop);

but start and stop will be current LBAs from struct dos_partition p. It will hide all details and keep all the low-level MBR calculations in include/pt-mbr.h, in the library code we will need to call dos_partition_sync_chs() on all places where we modify
partition start and size.

Your previous changes fixed add-partition use-case where we update on-disk stuff by set_partition(), but many other use-cases ignore CHS at all.

[pali]

Ok, seems that dos_partition_sync_chs() helper function can cleanup this part of code.

[pali]

Well, now I see one problem with this dos_partition_sync_chs() function. In current form it is not possible to implement it because of EBR:

• starting LBA sector of logical partition is relative to the start of EBR, but beginning CHS value is absolute (from start of the disk)
• starting LBA sector for next EBR is again relative to the current EBR, but in CHS addressing again absolute

Therefore in p is stored relative LBA value which is not enough to calculate absolute LBA (needed for CHS calculation).

[karelzak]

Therefore in p is stored relative LBA value which is not enough to calculate absolute LBA (needed for CHS calculation).

What about to add offset argument to the dos_partition_sync_chs()? For example in libfdisk/src/dos.c:set_partition() we have it already calculated this offset and I guess on other places it's usually pe->offset or so.

[pali]

Updated. But dos_partition_sync_chs() cannot be used in fdisk_dos_fix_chs() as it needs to compare and operate on C/H/S values instead of LBA.

[lgtm-com]

This pull request introduces 1 alert when merging 8c1c35f into 37c6c57 - view on LGTM.com

new alerts:

• 1 for Comparison result is always the same

[karelzak]

Thanks!

[pali]

Anyway, I see that in debug log output are partitions sometimes indexed from zero and sometimes from one. Not sure how big it is issue, but sometimes it can be misleading when debugging if in debug log is written that modifying partition 1 and then modifying partition 2 (and it still modifies second MBR partition)...

[karelzak]

It would be better to index it from zero for DBG(), for normal user output it uses i + 1 everywhere (I hope).

I did a small change to two debug messages.