Skip to content

Conversation

@fintelia
Copy link
Contributor

This PR adds new fields to COSI so that we can avoid losing information when converting VHD -> COSI. Specifically, it adds information from the disk partition table that may be important, like what the part UUIDs were and what order the partitions appeared on disk.

The plan is for trident stream-image to honor these additional fields while trident install would continue its current approach of generating new random UUIDs and create partitions in the order specified in the Host Configuration.

@fintelia fintelia requested a review from a team as a code owner December 11, 2025 00:05
Copilot AI review requested due to automatic review settings December 11, 2025 00:05
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces COSI specification version 1.2 to support baremetal image workflows by preserving partition table metadata during VHD to COSI conversion. The changes add four new required fields to the ImageFile object that capture original partition characteristics.

Key changes:

  • Add COSI version 1.2 to the revision summary with spec date 2025-12-10
  • Extend ImageFile object with partition metadata fields: originalSize, partUuid, partLabel, and partNumber
  • Update documentation to clarify that ImageFile objects represent disk partitions with both content and partition table information

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot AI review requested due to automatic review settings December 11, 2025 18:59
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings December 11, 2025 19:04
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

| `uncompressedSize` | number | 1.0 | Yes (since 1.0) | Size of the raw uncompressed image in bytes. |
| `sha384` | string | 1.0 | Yes (since 1.1) | SHA-384 hash of the compressed hash image. |

##### `Partition` Object
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of interest, why did you split out this information into a separate partition object?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discovered in #394 that Trident's current parsing of COSI metadata rejects unknown fields in places it shouldn't

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙁

Copilot AI review requested due to automatic review settings December 11, 2025 22:20
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

`Partition` objects hold the data necessary to recreate partition tables. They provide a mapping
between each image file and its original partition information. Starting in 1.2, each `ImageFile` in
the COSI MUST have a corresponding `Partition` object. The correspondence between an `ImageFile` and a `Partition` object is established by matching their `path` fields: a `Partition` object corresponds to the `ImageFile` whose `path` field is identical.

Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The description states that each ImageFile in the COSI MUST have a corresponding Partition object starting in version 1.2. However, this requirement creates a potential issue: what about verity hash partition images? The Filesystem object can contain a verity.image field which is also an ImageFile. The current specification doesn't clarify whether hash partition images also require a corresponding Partition object, or if this requirement only applies to the main filesystem image. Consider clarifying this relationship, especially for verity hash partitions and their expected partition metadata.

Suggested change
**Note:** The `Filesystem` object may contain a `verity.image` field, which is also an `ImageFile`. If the verity hash image is stored as a separate partition on disk, it MUST have a corresponding `Partition` object. If the verity hash image is not mapped to a real partition (for example, if it is embedded or used in a way that does not correspond to a disk partition), then a `Partition` object is NOT required for that `ImageFile`. Implementations SHOULD document how verity hash images are handled in their COSI files.

Copilot uses AI. Check for mistakes.
| `originalSize` | number | 1.2 | Yes (since 1.2) | Size of the partition before any filesystem shrinking. SHOULD be at least as large as the `uncompressedSize` field of the corresponding `ImageFile` object (matched by `path`). |
| `partUuid` | string | 1.2 | Yes (since 1.2) | The partition UUID. Not to be confused with the partition type UUID. |
| `label` | string | 1.2 | Yes (since 1.2) | Partition label (GPT partition name, may be an empty string). |
| `number` | number | 1.2 | Yes (since 1.2) | The index where the partition originally appeared on disk (1-indexed). |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not blocking mostly curious, is there a specific reason to preserve this if we already have a list of partitions?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could be persuaded either way. Currently imagecustomizer doesn't include partitions that are unmounted or have no filesystem type (other than verity hash partitions)

| `path` | string | 1.2 | Yes (since 1.2) | Absolute path of the compressed image file inside the tarball. MUST start with `images/`. |
| `originalSize` | number | 1.2 | Yes (since 1.2) | Size of the partition before any filesystem shrinking. SHOULD be at least as large as the `uncompressedSize` field of the corresponding `ImageFile` object (matched by `path`). |
| `partUuid` | string | 1.2 | Yes (since 1.2) | The partition UUID. Not to be confused with the partition type UUID. |
| `label` | string | 1.2 | Yes (since 1.2) | Partition label (GPT partition name, may be an empty string). |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this can be null, seems reasonable to explicitly allow that (or even allow the field to be missing)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't feel strongly about whether to allow null or missing. They'd both just mean the same thing as an empty string (GPT doesn't distinguish)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants