Bug 274312 - gpart restore fails with "gpart: entries '4': Invalid argument" with mkimg(1) images
Summary: gpart restore fails with "gpart: entries '4': Invalid argument" with mkimg(1)...
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 15.0-CURRENT
Hardware: Any Any
: --- Affects Many People
Assignee: Warner Losh
URL:
Keywords:
Depends on:
Blocks: 14.0r
  Show dependency treegraph
 
Reported: 2023-10-06 17:58 UTC by Michael Dexter
Modified: 2023-11-06 20:26 UTC (History)
8 users (show)

See Also:


Attachments
truss(1) output for the gpart backup/restore command (8.73 KB, text/plain)
2023-10-06 17:58 UTC, Michael Dexter
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Dexter 2023-10-06 17:58:44 UTC
Created attachment 245469 [details]
truss(1) output for the gpart backup/restore command

Hello, I see a few PRs related to gpart restore but not with this error. I have only seen mentions of the error in mailing lists.

FreeBSD freebsd 15.0-CURRENT FreeBSD 15.0-CURRENT #0 main-n265728-8818f0f1124e:
Thu Oct  5 03:15:28 UTC 2023     root@releng3.nyi.freebsd.org:/usr/obj/usr/src/a
md64.amd64/sys/GENERIC amd64

Steps:

root@freebsd:~ # sysctl kern.disks
kern.disks: vtbd1 vtbd0
root@freebsd:~ # gpart show
=>       3  12649842  vtbd0  GPT  (6.0G)
         3       346      1  freebsd-boot  (173K)
       349     66584      2  efi  (33M)
     66933   2097152      3  freebsd-swap  (1.0G)
   2164085  10485760      4  freebsd-zfs  (5.0G)

root@freebsd:~ # diskinfo vtbd0
vtbd0   512     6476721664      12649847        131072  0
root@freebsd:~ # diskinfo vtbd1
vtbd1   512     6476721664      12649847        131072  0

root@freebsd:~ # man gpart | grep restore | tail -1
           /sbin/gpart backup ada0 | /sbin/gpart restore -F ada1 ada2

root@freebsd:~ # /sbin/gpart backup vtbd0 | /sbin/gpart restore -F vtbd1
gpart: entries '4': Invalid argument

The vtbd0 is a root-on-ZFS weekly snapshot image and the second image is created with truncate, using the size from the unmodified RE image.

Backing up the partition table to a file and restoring from file produces the same error.

I have attached the truss output of the command listed in the manual page.
Comment 1 Michael Dexter 2023-10-06 20:29:54 UTC
The "backup" table reads:

GPT 4
1 freebsd-boot        3      346 bootfs 
2          efi      349    66584 efiesp 
3 freebsd-swap    66933  2097152 swapfs 
4  freebsd-zfs  2164085 10485760 rootfs

This 2014 discussion provides additional details about mkimg not working with gpt backup/restore:

https://groups.google.com/g/mpc.lists.freebsd.current/c/RHbHOO5UfNQ/m/Gs3e4kPgk9QJ

In case it is not clear, the release engineering image are created using makefs(8).
Comment 2 Andrew "RhodiumToad" Gierth 2023-10-06 21:41:25 UTC
This is simply that it is not legal to create a GPT partition table with only 4 slots - the minimum allowed value in the spec is 128 (16 kbytes of table, 128 bytes per entry).

gpart restore correctly refuses to create an out-of-spec table.

So if anything this is a bug in makefs.
Comment 3 Michael Dexter 2023-10-06 21:53:43 UTC
(In reply to Andrew "RhodiumToad" Gierth from comment #2)
It may also be possible to adjust the input to makefs.

Either way, perhaps "gpart: entries '4': Invalid argument" could be a little more verbose as to what went wrong.
Comment 4 Andrew "RhodiumToad" Gierth 2023-10-06 23:00:30 UTC
Incidentally the 2014 discussion seems to be claiming that that the spec doesn't give a hard lower bound on the table size. This is incorrect: the spec (UEFI version 2.9, section 5.3.1 "GPT Overview") says:

A minimum of 16,384 bytes of space must be reserved for the GPT Partition Entry Array.

If the block size is 512, the First Usable LBA must be greater than or equal to 34 (allowing 1 block for the Protective MBR, 1 block for the Partition Table Header, and 32 blocks for the GPT Partition Entry Array); if the logical block size is 4096, the First Useable LBA must be greater than or equal to 6 (allowing 1 block for the Protective MBR, 1 block for the GPT Header, and 4 blocks for the GPT Partition Entry Array).
Comment 5 Michael Dexter 2023-10-07 04:46:56 UTC
In running this by cperciva@, I confirmed that the issue exists:

With both the UFS and ZFS images from release engineering.

With stock and truncated to 10G, after 'service growfs onestart' which grew the partitioned device.

FWIW
Comment 6 Michael Dexter 2023-10-07 04:54:06 UTC
(In reply to Andrew "RhodiumToad" Gierth from comment #4)
Is that the "40" that most systems start with?

Installer-produced partition tables:

=>       40  976773088  nda0  GPT  (466G)
=>       40  7814037088  ada1  GPT  (3.6T)
=>       40  500118112  ada0  GPT  (238G)

The makefs(8) one:

=>       3  20971509  vtbd0  GPT  (10G)
Comment 7 Michael Dexter 2023-10-07 05:00:05 UTC
The invocations that produced this images in /usr/src/release/tools/vmimage.subr:

        ufs)
                makefs ${MAKEFSARGS} -o label=rootfs -o version=2 -o softupdates
=1 \
                        ${VMBASE} ${DESTDIR}
                ;;
        zfs)
                makefs -t zfs ${MAKEFSARGS} \
                        -o poolname=zroot -o bootfs=zroot/ROOT/default -o rootpa
th=/ \
                        -o fs=zroot\;mountpoint=none \
...
Comment 8 Michael Dexter 2023-10-07 05:08:28 UTC
(In reply to Michael Dexter from comment #7)
I realize that's late in the process. Looking at how the individual partition images are assembled...
Comment 9 Colin Percival freebsd_committer freebsd_triage 2023-10-07 05:46:31 UTC
This seems like a bug in mkimg, since that's where the partition table gets put together.  Assigning to marcel.
Comment 10 Michael Dexter 2023-10-07 06:04:09 UTC
In re-reading the thread and looking through the mkimg code, my naive observation is that hdr_lba_start in /usr/src/usr.bin/mkimg/gpt.c might be to blame, but I could easily be wrong.
Comment 11 Andrew "RhodiumToad" Gierth 2023-10-07 12:22:25 UTC
(In reply to Michael Dexter from comment #10)

A bunch of stuff in mkimg/gpt.c is wrong - gpt_tblsz is computing a minimal size table (i.e. just enough sectors to fit the requested number of partitions) without respecting the spec's minimum; hdr_lba_start and hdr_lba_end are calculated from this without doing the same rounding to 4k that gpart does; etc.
Comment 12 Andrew "RhodiumToad" Gierth 2023-10-07 12:31:36 UTC
(In reply to Michael Dexter from comment #6)

The "40" starting offset is computed by rounding up the minimum size to a 4k boundary - on a device with 512-byte logical blocks, block 0 is the PMBR, block 1 is the GPT header, blocks 2-33 are the partition table (16kB), so the first usable block could be block 34, but that would lead to misalignment when the physical block size is 4k, so rounding up to the next 4k boundary means we make block 40 the first usable block.
Comment 13 yu shan wei 2023-10-10 23:30:34 UTC
gpart默认至少128个分区项目,mkimg创建的硬盘映像,可以只有三两个分区项目
Comment 14 Michael Dexter 2023-10-15 18:34:31 UTC
This bug impacts the images produced by release engineering.
Comment 15 Warner Losh freebsd_committer freebsd_triage 2023-10-16 15:51:43 UTC
This error message comes from:
        if ((gpp->gpp_parms & G_PART_PARM_ENTRIES) &&
            (gpp->gpp_entries < scheme->gps_minent ||
             gpp->gpp_entries > scheme->gps_maxent)) {
                gctl_error(req, "%d entries '%d'", EINVAL, gpp->gpp_entries);
		return (EINVAL);
        }

which is failing because:

g_part_gpt.c:157:       .gps_minent = 128,

which is likely too strict a requirement.

Reading section 5.3.2, we see the following fields determine if the GPT is valid:
"The following test must be performed to determine if a GPT is valid:
• Check the Signature
• Check the Header CRC
• Check that the MyLBA entry points to the LBA that contains the GUID Partition Table
• Check the CRC of the GUID Partition Entry Array"

so the check for the minent/maxent to reject the GPT is more strict than is required by the standard.

The standard does not explicitly state that this array must be 128 or larger. In fact, it explicitly states that it can be shorter, and that the CRC of the partition table only covers "The CRC32 of the GUID Partition Entry array. Starts at PartitionEntryLBA and is computed over a byte length of NumberOfPartitionEntries * SizeOfPartitionEntry." While there are restrictions on SizeOfPartitionEntry, there's no such restriction on NumberOfPartitionEntries. There's another note that says: "There is a 32-bit CRC of the GPT Partition Entry
Array that is stored in the GPT Header in Partition Entry Array CRC32 field. The size of the GPT Partition Entry Array is SizeOfPartitionEntry multiplied by Number OfPartitionEntries. If the size of the GUID Partition Entry Array is not an even multiple of the logical block size, then any space left over in the last logical block is Reserved and not covered by the Partition Entry Array CRC32 field."

Therefore, I think the right fix for this is:
diff --git a/sys/geom/part/g_part_gpt.c b/sys/geom/part/g_part_gpt.c
index 990ace4a25d4..ab0ab07f67c8 100644
--- a/sys/geom/part/g_part_gpt.c
+++ b/sys/geom/part/g_part_gpt.c
@@ -154,7 +154,7 @@ static struct g_part_scheme g_part_gpt_scheme = {
        g_part_gpt_methods,
        sizeof(struct g_part_gpt_table),
        .gps_entrysz = sizeof(struct g_part_gpt_entry),
-   .gps_minent = 128,
+ .gps_minent = 4,
        .gps_maxent = 4096,
        .gps_bootcodesz = MBRSIZE,
 };

because the entry size is 128 and you can fit 4 of those into a 512 byte block. Even this may be too strict, thinking about it. The additions LBAs that are there may be reserved ("If the block size is 512, the First Usable LBA must be greater than or equal to 34 (allowing 1 block for the Protective MBR, 1 block for the Partition Table Header, and 32 blocks for the GPT Partition Entry Array); if the logical block size is 4096, the First Useable LBA must be greater than or equal to 6 (allowing 1 block for the Protective MBR, 1 block for the GPT Header, and 4 blocks for the GPT Partition Entry Array)."), but there's no requirement that the GUID Partition Entry Array cover all of those LBAs. I claim the above language, to the contrary, explicitly allows it to be smaller.

So while a minimum space must be reserved for it, there's no requirement that all of it be used for the entries, and there's explicit language for what to do when that's not the case. I understand where the 128 came from, but I claim it is an improper interpretation of the standard. In other parts of the code, we handle it just fine, only here do we whine, which makes us also inconsistent.

I 100% agree this error message is crap.

One could also make a 'be liberal in what you accept, and conservative in what you generate' argument to also fix mkimg.c, which doesn't enforce a minimum:

diff --git a/usr.bin/mkimg/gpt.c b/usr.bin/mkimg/gpt.c
index 59c51a6a177b..11c210f8d582 100644
--- a/usr.bin/mkimg/gpt.c
+++ b/usr.bin/mkimg/gpt.c
@@ -130,7 +130,7 @@ gpt_tblsz(void)
        u_int ents;

        ents = secsz / sizeof(struct gpt_ent);
-   return ((nparts + ents - 1) / ents);
+ return (min(16384 / secsz, (nparts + ents - 1) / ents));
 }

 static lba_t

But I've not had time to test either completely, nor to see if there's a suitable #define for 16384, but nothing jumped out at me.

I think we should fix both, tbh, since that will make us more conservative about what we generate, and more liberal about what we accept and allow maximum compatibility with old releases.
Comment 16 Warner Losh freebsd_committer freebsd_triage 2023-10-16 15:54:39 UTC
(In reply to Andrew "RhodiumToad" Gierth from comment #4)

I maintain that the 2014 discussion is correct in one aspect: The number of entries has no lower bound. The size of the reserved space, however, does have a lower bound. So we should accept the smaller number of entries, since the standard explicitly says that's ok and prescribes behavior for any number of entries. We should also fix mkimg so that it respects the minimum reserved space, which my quick read of the code shows that it's not doing.

So both geom and mkimg are wrong, and both need fixes.
Comment 17 Warner Losh freebsd_committer freebsd_triage 2023-10-16 15:57:54 UTC
(In reply to Warner Losh from comment #15)

Replying to myself: I said both would be nice in comment #15. In further reading mkimg code after I posted it, I'd suggest that both are required: geom is bogus for rejecting < 128 and mkimg is bogus for not generating the required minimum reserved space (again, assuming I reading the code correctly).
Comment 18 Warner Losh freebsd_committer freebsd_triage 2023-10-16 16:03:15 UTC
(In reply to Warner Losh from comment #15)

For posterity: All quotes are from UEFI version 2.10 section 5.3.
Comment 19 Andrew "RhodiumToad" Gierth 2023-10-16 16:32:18 UTC
(In reply to Warner Losh from comment #17)

I would say that mkimg should, in addition to respecting the reserved space requirement, also set the number of entries to the conventional value (128) and not 4, simply because it looks like gpart can't increase the value of that field when adding a partition, and you don't want to create images that can't have new partitions added if need be. (If you respect the reserved space requirement, then there is no space saving from making the number of entries smaller, so why do it?)
Comment 20 Ed Maste freebsd_committer freebsd_triage 2023-10-16 16:37:22 UTC
> I would say that mkimg should, in addition to respecting the reserved space requirement,
> also set the number of entries to the conventional value (128) and not 4,

Yes, absolutely mkimg should be creating a GPT with 128 entries - that's the most important bug to fix.
Comment 21 Warner Losh freebsd_committer freebsd_triage 2023-10-16 17:06:53 UTC
(In reply to Ed Maste from comment #20)

The most important bug to fix is the reserved size. The 4 vs 128 is a nice to have because out geom part has a limitation on changing the number of entries. That insuffient reserved space is a spec violation. The number of entries in the header is not. Thankfully, fixing the minimum reserved size will also increase the number of entries reported in the header. We must also fix geom to accept fewer entries in the header because that also is a spec violation.
Comment 22 Ed Maste freebsd_committer freebsd_triage 2023-10-16 17:19:22 UTC
(In reply to Warner Losh from comment #21)
I probably trimmed too much context - I meant to say that between gpart and makefs, makefs is the most important to fix. In particular, because images produced by the current, buggy makefs will persist beyond a bug fix.

Increasing the reserved space in mkimg but leaving it at 4 entries would be more work than just using 128, and still leave us with a bug (i.e., inability to add more partitions later on / difference between mkimg and gpart use).

I agree with you that having the first LBA comply with the spec is the hard requirement.
Comment 23 Warner Losh freebsd_committer freebsd_triage 2023-10-17 02:26:34 UTC
Use my preferred email address.
Comment 24 Warner Losh freebsd_committer freebsd_triage 2023-10-17 02:30:59 UTC
https://reviews.freebsd.org/D42245 to fix mkimg
https://reviews.freebsd.org/D42246 to fix gpart
https://reviews.freebsd.org/D42247 to add warning that the reserved area is too small to gpart (but it's only a warning because the GPT is considered valid since the nonconformity is not listed in the criteria to check to reject the GPT).

If people could test these, that would be great. They pass my stupid simple tests, I think, but I've not had time to delve into this problem.
Comment 25 Warner Losh freebsd_committer freebsd_triage 2023-10-17 02:47:11 UTC
A buggy hexdump:
00000200  45 46 49 20 50 41 52 54  00 00 01 00 5c 00 00 00  |EFI PART....\...|
00000210  ea c7 2f 95 00 00 00 00  01 00 00 00 00 00 00 00  |../.............|
00000220  04 60 09 00 00 00 00 00  03 00 00 00 00 00 00 00  |.`..............|
00000230  02 60 09 00 00 00 00 00  12 7a a9 63 4a f6 ed 11  |.`.......z.cJ...|
00000240  a7 d3 e0 d5 5e 1e 73 bd  02 00 00 00 00 00 00 00  |....^.s.........|
00000250  04 00 00 00 80 00 00 00  58 9c b0 81 00 00 00 00  |........X.......|

So Signature is 'EFI PART'
Revision is 0x00010000 (1.0)
HeaderSize is 0x5c / 92 bytes
HeaderCRC s 0x952fc7ea (presumably correct)
Reserved is 0x0000000 (correctly zero)
MyLBA is 0x0000000000000001 (correctly 1)
AltLBA is 0x00000000096004 (likely correct)
FirstUseableLBA is 0x000000000000003 (3, which is wrong: it must be >= 32 to have the required space)
LastUseableLBA is 0x0000000000096002 (2 less than the AltLBA, which seems legit)
The UUID looks good (I'm not copying it down)
PartitionEntryLBA is 0x2, which is correct
NumberOfPartitionEntries is 0x4, which is fine (though a lot of software wants this to be >= 128)
SizeOfParitionEntry is 0x00000080, 128, which is right.
And then we have a CRC of the partition table. (and the rest of the sector is 0's).

So this confirms the lack of reserved space.
Comment 26 Warner Losh freebsd_committer freebsd_triage 2023-10-17 03:07:35 UTC
With the latest version of D42245, we now get:

00000200  45 46 49 20 50 41 52 54  00 00 01 00 5c 00 00 00  |EFI PART....\...|
00000210  d8 2e 74 5a 00 00 00 00  01 00 00 00 00 00 00 00  |..tZ............|
00000220  42 60 09 00 00 00 00 00  22 00 00 00 00 00 00 00  |B`......".......|
                                   ^^^^^^^^^^^^^^^^^^^^^^^ FirstUsableLBA = -x22
00000230  21 60 09 00 00 00 00 00  84 7b f1 cc 98 6c ee 11  |!`.......{...l..|
00000240  a8 54 a0 36 9f 09 4f 1e  02 00 00 00 00 00 00 00  |.T.6..O.........|
00000250  80 00 00 00 80 00 00 00  71 5c 49 7b 00 00 00 00  |........q\I{....|
          ^^^^^^^^^^^ NumberOfPartitionEntries = 0x80


which has FirstUsableLBA of 0x22 (34 -- correct)
NumberOfPartitionEntries of 0x80 (128 -- desirable)

And the rest the same (except AltLBA is now more offset from end of disk to reflect the larger GPT Entry Array allocation, the CRCs are different (Good) and the UUID is different (also good).
Comment 27 commit-hook freebsd_committer freebsd_triage 2023-10-17 17:17:41 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=9b42d3e12ffc6896fcb4e60c1b239ddf60705831

commit 9b42d3e12ffc6896fcb4e60c1b239ddf60705831
Author:     Warner Losh <imp@FreeBSD.org>
AuthorDate: 2023-10-17 17:14:14 +0000
Commit:     Warner Losh <imp@FreeBSD.org>
CommitDate: 2023-10-17 17:14:23 +0000

    mkimg: Ensure GPT Entry Array is at least 16k

    UEFI v2.10 Section 5.3 documentes that the minimum reserved space after
    the GPT header be at least 16kB. Enforce this minimum. Before, we'd only
    set the number of entries to be the unpadded size. gpart's selective
    enforcement of aspects of the GPT standard meant that these images would
    work, but couldn't be changed (to add a partition or grow the size of a
    partition). This ensures that gpart's overly picky standards don't cause
    problems for people wishing to, for example, resize release images.

    MFC after:              1 day (we want this in 14.0)
    PR:                     274312
    Sponsored by:           Netflix
    Reviewed by:            emaste
    Differential Revision:  https://reviews.freebsd.org/D42245

 sys/sys/disk/gpt.h  |  7 +++++++
 usr.bin/mkimg/gpt.c | 16 ++++++++++++----
 2 files changed, 19 insertions(+), 4 deletions(-)
Comment 28 Andrew "RhodiumToad" Gierth 2023-10-17 17:32:50 UTC
(In reply to Warner Losh from comment #26)

> which has FirstUsableLBA of 0x22 (34 -- correct)

gpart makes this 40 so that it's aligned to physical sectors on devices that have 512 byte emulated logical sectors on 4kbyte physical sectors. It's probably a good idea for mkimg to do the same, so that the misalignment doesn't bugger performance if the image is copied to a device of this type.
Comment 29 Ed Maste freebsd_committer freebsd_triage 2023-10-17 17:44:42 UTC
(In reply to Andrew "RhodiumToad" Gierth from comment #28)
> gpart makes this 40 so that it's aligned to physical sectors on devices that have 512
> byte emulated logical sectors on 4kbyte physical sectors.

Oh, yeah. It would be good for mkimg to behave the same as gpart.
Comment 30 Warner Losh freebsd_committer freebsd_triage 2023-10-17 18:14:16 UTC
(In reply to Andrew "RhodiumToad" Gierth from comment #28)
> gpart makes it 40 to align with 4k devices.

4k devices already need a separate GPT (since a GPT on a 4k device is different than one on a 512 device, and you can't just splat one on the other). So it isn't really going to help devices that much... Especially since physical block sizes are really more like 16k or 64k for flash devices (where most things wind up), so even from that perspective, it's inadequate.

Since there's a lot of hair around what 'the right thing to do here' would be, I'm not going to do anything. Most likely it should be 64k or 1MB into the drive and all partitions are aligned to 64k or 1MB (since we have no way to know the underlying device). But for boot media, that's not needed (since we don't write to it once it's dd'd to the device). For the ARM images, some alignment makes sense, but I'd rather that be an explicit thing on the command line, rather than an automatic thing mkimg always does. There's currently no alignment flag for mkimg, and adding it may require researching acceptable alignment for other partitioning schemes.... Also various SoCs have different 'hidden' areas they put some of the boot chain, which makes it tricky to steer clear of things... The current arm creation scripts know all this, and I already a little concerned this change may break them in subtle, hard to find ways.

So I'm going to say all of this is beyond the scope of this bug, but will happily review changes from others...
Comment 31 Andrew "RhodiumToad" Gierth 2023-10-17 18:28:46 UTC
(In reply to Warner Losh from comment #30)

Devices with 4k _logical_ sectors need a different format, but those are rare; almost all disks with 4k physical sectors are configured to emulate 512-byte sectors, just with reduced performance for writes that aren't 4k-aligned (since those require read-modify-write).
Comment 32 Andrew "RhodiumToad" Gierth 2023-10-17 19:03:16 UTC
The bottom line, IMO, is that making the results different from those of gpart is likely just laying down landmines for the future, so why not fix it now?
Comment 33 Warner Losh freebsd_committer freebsd_triage 2023-10-17 20:35:23 UTC
(In reply to Andrew "RhodiumToad" Gierth from comment #32)

I'm saying that compatibility with gpart is not a goal. It's a trivial point that honestly doesn't matter given the other issues that we have.

But the problem is more that 'underlying' storage for these isn't 4k disks. It's FLASH devices that have 16k or 64k physical blocks. It's different backing stores for the cloud images. Etc. Those are the primary use cases where it matters. And in those cases, 4k alignment doesn't help at all since we're still misaligning things. 512e/4kn disks aren't the target market for these images.

I do think there's value in adding -a functionality to mkimg (though I think -a is already taken for setting the active partition). But we don't have it today, and I don't want to try to hack mkimg to kinda sorta have it to match details of what gpart produces that at the end of the day aren't the most helpful. It would be way better to align all the partitions to 1MB boundaries, since that will work best in the most places, even in the cloud, and would waste trivial amounts of space. But doing that also needs to be tested, test cases need to be written (like I need to write one for this bug), and performance measurements done. For the arm/arm64 SD card images, I think this will have a bigger impact, especially when people growfs things.

I totally understand the desire to be gpart compatible, but mkimg wasn't written with that as a goal, and retrofitting it is going to be tricky. In large part because gpart does a number of things sloppily that need to be corrected. I'm hoping that I've not broken tests as it is, and the more we change, the more we run that risk.
Comment 34 Warner Losh freebsd_committer freebsd_triage 2023-10-17 20:52:02 UTC
From the CI email:

29 tests failed.
FAILED:  usr.bin.mkimg.mkimg_test.gpt_1x1_4096_qcow

Error Message:
atf-check failed; see the output of the test for details

FAILED:  usr.bin.mkimg.mkimg_test.gpt_1x1_4096_qcow2

Error Message:
atf-check failed; see the output of the test for details

FAILED:  usr.bin.mkimg.mkimg_test.gpt_1x1_4096_raw

Error Message:
atf-check failed; see the output of the test for details

FAILED:  usr.bin.mkimg.mkimg_test.gpt_1x1_4096_vhd

Error Message:
atf-check failed; see the output of the test for details

FAILED:  usr.bin.mkimg.mkimg_test.gpt_1x1_4096_vhdf

Error Message:
atf-check failed; see the output of the test for details

FAILED:  usr.bin.mkimg.mkimg_test.gpt_1x1_4096_vhdx

Error Message:
atf-check failed; see the output of the test for details

FAILED:  usr.bin.mkimg.mkimg_test.gpt_1x1_4096_vmdk

Error Message:
atf-check failed; see the output of the test for details

FAILED:  usr.bin.mkimg.mkimg_test.gpt_1x1_512_qcow

Error Message:
atf-check failed; see the output of the test for details

FAILED:  usr.bin.mkimg.mkimg_test.gpt_1x1_512_qcow2

Error Message:
atf-check failed; see the output of the test for details

FAILED:  usr.bin.mkimg.mkimg_test.gpt_1x1_512_raw

Error Message:
atf-check failed; see the output of the test for details

FAILED:  usr.bin.mkimg.mkimg_test.gpt_1x1_512_vhd

Error Message:
atf-check failed; see the output of the test for details

FAILED:  usr.bin.mkimg.mkimg_test.gpt_1x1_512_vhdf

Error Message:
atf-check failed; see the output of the test for details

FAILED:  usr.bin.mkimg.mkimg_test.gpt_1x1_512_vhdx

Error Message:
atf-check failed; see the output of the test for details

FAILED:  usr.bin.mkimg.mkimg_test.gpt_1x1_512_vmdk

Error Message:
atf-check failed; see the output of the test for details

FAILED:  usr.bin.mkimg.mkimg_test.gpt_63x255_4096_qcow

Error Message:
atf-check failed; see the output of the test for details

FAILED:  usr.bin.mkimg.mkimg_test.gpt_63x255_4096_qcow2

Error Message:
atf-check failed; see the output of the test for details

FAILED:  usr.bin.mkimg.mkimg_test.gpt_63x255_4096_raw

Error Message:
atf-check failed; see the output of the test for details

FAILED:  usr.bin.mkimg.mkimg_test.gpt_63x255_4096_vhd

Error Message:
atf-check failed; see the output of the test for details

FAILED:  usr.bin.mkimg.mkimg_test.gpt_63x255_4096_vhdf

Error Message:
atf-check failed; see the output of the test for details

FAILED:  usr.bin.mkimg.mkimg_test.gpt_63x255_4096_vhdx

Error Message:
atf-check failed; see the output of the test for details

FAILED:  usr.bin.mkimg.mkimg_test.gpt_63x255_4096_vmdk

Error Message:
atf-check failed; see the output of the test for details

FAILED:  usr.bin.mkimg.mkimg_test.gpt_63x255_512_qcow

Error Message:
atf-check failed; see the output of the test for details

FAILED:  usr.bin.mkimg.mkimg_test.gpt_63x255_512_qcow2

Error Message:
atf-check failed; see the output of the test for details

FAILED:  usr.bin.mkimg.mkimg_test.gpt_63x255_512_raw

Error Message:
atf-check failed; see the output of the test for details

FAILED:  usr.bin.mkimg.mkimg_test.gpt_63x255_512_vhd

Error Message:
atf-check failed; see the output of the test for details

FAILED:  usr.bin.mkimg.mkimg_test.gpt_63x255_512_vhdf

Error Message:
atf-check failed; see the output of the test for details

FAILED:  usr.bin.mkimg.mkimg_test.gpt_63x255_512_vhdx

Error Message:
atf-check failed; see the output of the test for details

FAILED:  usr.bin.mkimg.mkimg_test.gpt_63x255_512_vmdk

Error Message:
atf-check failed; see the output of the test for details
Comment 35 Warner Losh freebsd_committer freebsd_triage 2023-10-18 15:16:34 UTC
So the tests were easy to fix: just re-run the generation script.

But the number of unanticipated issues from minor changes worries me.
Comment 36 commit-hook freebsd_committer freebsd_triage 2023-10-18 15:29:36 UTC
A commit in branch stable/14 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=5401ebd33cf5073e8c0f095527504d38f74a926b

commit 5401ebd33cf5073e8c0f095527504d38f74a926b
Author:     Warner Losh <imp@FreeBSD.org>
AuthorDate: 2023-10-18 15:23:40 +0000
Commit:     Warner Losh <imp@FreeBSD.org>
CommitDate: 2023-10-18 15:23:40 +0000

    mkimg: Ensure GPT Entry Array is at least 16k

    UEFI v2.10 Section 5.3 documentes that the minimum reserved space after
    the GPT header be at least 16kB. Enforce this minimum. Before, we'd only
    set the number of entries to be the unpadded size. gpart's selective
    enforcement of aspects of the GPT standard meant that these images would
    work, but couldn't be changed (to add a partition or grow the size of a
    partition). This ensures that gpart's overly picky standards don't cause
    problems for people wishing to, for example, resize release images.

    MFC after:              1 day (we want this in 14.0)
    PR:                     274312
    Sponsored by:           Netflix
    Reviewed by:            emaste
    Differential Revision:  https://reviews.freebsd.org/D42245

    (cherry picked from commit 9b42d3e12ffc6896fcb4e60c1b239ddf60705831)

 sys/sys/disk/gpt.h  |  7 +++++++
 usr.bin/mkimg/gpt.c | 16 ++++++++++++----
 2 files changed, 19 insertions(+), 4 deletions(-)
Comment 37 commit-hook freebsd_committer freebsd_triage 2023-10-19 19:23:47 UTC
A commit in branch releng/14.0 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=d99f6566d53d86e4ec1854e0c602ea7ed7ded48d

commit d99f6566d53d86e4ec1854e0c602ea7ed7ded48d
Author:     Warner Losh <imp@FreeBSD.org>
AuthorDate: 2023-10-18 15:23:40 +0000
Commit:     Warner Losh <imp@FreeBSD.org>
CommitDate: 2023-10-19 18:37:44 +0000

    mkimg: Ensure GPT Entry Array is at least 16k

    UEFI v2.10 Section 5.3 documentes that the minimum reserved space after
    the GPT header be at least 16kB. Enforce this minimum. Before, we'd only
    set the number of entries to be the unpadded size. gpart's selective
    enforcement of aspects of the GPT standard meant that these images would
    work, but couldn't be changed (to add a partition or grow the size of a
    partition). This ensures that gpart's overly picky standards don't cause
    problems for people wishing to, for example, resize release images.

    MFC after:              1 day (we want this in 14.0)
    PR:                     274312
    Sponsored by:           Netflix
    Reviewed by:            emaste
    Differential Revision:  https://reviews.freebsd.org/D42245

    (cherry picked from commit 9b42d3e12ffc6896fcb4e60c1b239ddf60705831)
    (cherry picked from commit 5401ebd33cf5073e8c0f095527504d38f74a926b)
    Approved-by: re (cperciva)

 sys/sys/disk/gpt.h  |  7 +++++++
 usr.bin/mkimg/gpt.c | 16 ++++++++++++----
 2 files changed, 19 insertions(+), 4 deletions(-)
Comment 38 Michael Dexter 2023-11-06 20:26:12 UTC
I have confirmed that this is working exactly as expected on 14.0-RC4!

Synopsis: mdconfig -af the VM-IMAGE, truncate one of the same size, mdconfig -af that, and then 'gpart backup md0 | gpart restore md1' and the partition table is transferred without error.

Thank you Warner, Colin, Andrew, Ed, and everyone else who helped track this down and resolve it!

I will mark this FIXED.