On Sat, Nov 01, 2025 at 12:00:40AM +0100, Pascal Hambourg wrote: > On 31/10/2025 at 22:53, Brian C. Lane wrote: > > > > > > When libparted reads an existing empty GPT partition table, it wrongly > > > creates the main and backup virtual metadata partitions with a length > > > based > > > on GPT_DEFAULT_PARTITION_ENTRIES (128) instead of the actual partition > > > table > > > length based on NumberOfPartitionEntries in the GPT header. Only after an > > > active partition is added (either read from the on-disk partition table or > > > new), the metadata partitions have the correct length. > > > > > > Looking at libparted/label/gpt.c, it seems that gpt_read() initializes the > > > partition table object with GPT_DEFAULT_PARTITION_ENTRIES and does not > > > trigger an update of metadata and free space after parsing the GPT header > > > but only when partitions are added. > > > > I've tried to figure out where this could be going wrong, but am not > > seeing it. When gpt_alloc is called it sets gpt_disk_data->entry_count > > to the default. But when gpt_read is called it calls _parse_header, and > > parse_header sets entry_count from the value it read from the header > > (gpt->NumberOfPartitionEntries). > > Here is the (trimmed) call chain as I understand it: > > ped_disk_new > ped_disk_new_fresh > type->ops->alloc (-> gpt_alloc) > _ped_disk_alloc > gpt_disk_data->entry_count = GPT_DEFAULT_PARTITION_ENTRIES > _disk_pop_update_mode <- update metadata and free space > type->ops->read (-> gpt_read) > ped_disk_delete_all > gpt_read_headers > _parse_header > gpt_disk_data->entry_count = gpt->NumberOfPartitionEntries > gpt_read_PE_array > for each active partition entry > _parse_part_entry > ped_disk_add_partition > _disk_push_update_mode > _disk_raw_add > _disk_pop_update_mode <- update metadata and free space > > Indeed _parse_header updates entry_count, but then _disk_pop_update_mode > (which updates metadata and free space partitions) is called only if the > partition table has at least one active partition. Or later when a new > partition is created, or the pmbr_boot flag is set.
Ah, ok. Now it's clear :) Thanks for that. > I quickly tested that surrounding the call to type->ops->read with > _disk_push_update_mode and _disk_pop_update_mode in ped_disk_new seems to > fix the issue: > > diff --git a/libparted/disk.c b/libparted/disk.c > index 2d6b9d49..768f4681 100644 > --- a/libparted/disk.c > +++ b/libparted/disk.c > @@ -199,8 +199,10 @@ ped_disk_new (PedDevice* dev) > if (!disk) > goto error_close_dev; > + _disk_push_update_mode (disk); > if (!type->ops->read (disk)) > goto error_destroy_disk; > + _disk_pop_update_mode (disk); > disk->needs_clobber = 0; > ped_device_close (dev); > return disk; This doesn't pass the tests -- gpt_read calls ped_disk_commit_to_dev when it needs to fix one of the headers, and that has an assert check on update_mode. But, while it seems a bit like a kludge, I think we could just do a push/pop right after doing the read instead of wrapping it. That at least passes the tests and I don't think it would have any unexpected side-effects. > But I am now working on a larger patch which aims to stay in update mode and > avoid useless transient metadata and free space partition updates until > after reading the partition table. Would it be welcome ? I'm open to anything -- but am very reluctant to make large changes without really good reasons. parted is, for the most part, really stable and I'm worried about breaking things. Brian -- Brian C. Lane (PST8PDT) - weldr.io - lorax - parted - pykickstart
