image-hd: add forced-primary flag for higher MBR layout flexibility

[sairon]

The current limitation of Genimage is that it is not able to create MBR images that have primary partitions that start after a logical partition. This can be useful for images that can be later resized based on the actual device size - for this operation the partition must be at the end of the device, and if it is present in a logical partition, it must be resized first, making it a two-step process.

This commit adds the "forced-primary" flag which can be used to indicate that the partition should be put into the disk's MBR instead of creating another logical partition. Validation ensures that this syntax allows to create such partitions only after an existing logical partition, and that the maximum number of MBR entries woudn't be exceeded by doing so.

Test cases for valid and invalid configuiration has been added. Also added few more details in the debug print to make it more obvious how the MBR/EBR layout looks like.

[agners]

In a way, having to set forced-primary for each partition following the first one make sense as extended partition need to be consecutive, and there can only be one extended partition.

While in theory we could also do the opposite (add forced-extended), I don't think that is a good idea as we want a particular primary to be primary.

Having to set every partition to forced-primary after the first forced-primary is kinda over-constrained. genimage could do this implicitly. But maybe it is better for readability and validation. Either way works for me.

[sairon]

Having to set every partition to forced-primary after the first forced-primary is kinda over-constrained. genimage could do this implicitly. But maybe it is better for readability and validation. Either way works for me.

IMO "explicit is better than implicit" here - in the most extreme case you'll have to add it only to three partitions (if the first one would be the extended one) but for the resizing use case I mention above just one will be usually sufficient. And since it has this "I known what I'm doing" subtext, I'll prefer to do it this way. I think it wouldn't be harder to validate implicit forced-primary partitions though, since you can check if there's already been some forced-primary partition and know the number of MBR entries. But it's better for readability and less error-prone when moving things around.

[michaelolbrich]

I think this is getting a bit convoluted. Right now, we don't really track the actual extended partition and that makes things needlessly complex. So I think you should do the following:

• Insert an explicit extended partition at the right place in the list. It can be recognized by the partition_type 0x0f
  Then put extended_lba and extended_size as offset and size into that partition.
• rename part->extended to logical, because that's what these partitions actually are.

It probably makes sense to keep a pointer to the extended partition in the hdimage.

[sairon]

Thanks Michael, I am not insisting on taking exactly the approach I propose in this PR, so anything that does the job works for me.

I think this is getting a bit convoluted. Right now, we don't really track the actual extended partition and that makes things needlessly complex. So I think you should do the following:

• Insert an explicit extended partition at the right place in the list. It can be recognized by the partition_type 0x0f
  Then put extended_lba and extended_size as offset and size into that partition.

I am not sure how the resulting config you're suggesting should look like. Can you post an example? Currently you can specify a partition to be of whatever type but the rest of the code can't handle that you manually specified an extended partition. Also introducing this possibility shouldn't be a breaking change for old config syntax, so that might be also a limiting factor.

• rename part->extended to logical, because that's what these partitions actually are.

Absolutely, it was initially slightly confusing for me too, so if we're doing any larger changes in this area, I agree renaming this field is a good idea.

[sairon]

Meh, sorry, I'm not really bright this morning so I misinterpreted that the comment was about the actual code and not the concept, thanks @agners for pointing that out in a private discussion.

Okay, then you can disregard basically the whole above comment 🙈 Now I get it and agree with the rest as well, let's refactor it to make it less convoluted 👍

[sairon]

Okay, I adjusted the extended partition handling as suggested, hope it's a bit more readable now. PTAL 🙏

[michaelolbrich]

It's still quite complex but a good start. I'll do the rest myself.

[michaelolbrich]

Right, the check fails, because you didn't add the new files to EXTRA_DIST in Makefile.am. Can you fix that please?
It's not noticeable for the tests that are expected to fail, but those files need to be added as well. I'll improve the test-suite to detect that.

[michaelolbrich]

With #252 "hdimage syntax" will fail as well. You can check for that locally by running make distcheck

[michaelolbrich]

I've added the files to Makefile.am, squashed the commits and rebased the branch.
There was a merge conflict with #249 but I think I fixed it correctly.