PR#7: Add support for setting the GUID of the partition to set its correct type. - fedora-qa/createhdds

fedora-qa / createhdds

#7 Add support for setting the GUID of the partition to set its correct type.

Merged 4 years ago by adamwill. Opened 4 years ago by lruzicka.

windows_ntfs into master

Add support for setting the GUID of the partition to set

Lukáš Růžička • 4 years ago

e63e831

createhdds.py

file modified

+9 -3

		`@@ -147,9 +147,15 @@`
		`# start and end sector numbers - more details in`
		`# guestfs docs`
		`gfs.part_add(disk, part['type'], int(part['start']), int(part['end']))`
		`- # identify the partition and format it`
		`- partn = gfs.list_partitions()[-1]`
		`- gfs.mkfs(part['filesystem'], partn, label=part.get('label'))`
		`+ # identify the partition`
		`+ partname = gfs.list_partitions()[-1]`
		`+ partnum = gfs.part_list(disk)[-1]["part_num"]`
		`+ # sometimes, we want to set the gpt type of the partition`
		`+ gpt_type = part.get("gpt_type", None)`
		`+ if gpt_type:`
		`+ gfs.part_set_gpt_type(disk, partnum, gpt_type)`
		`+ # format the partition`
		`+ gfs.mkfs(part['filesystem'], partname, label=part.get('label'))`
		`# do file 'writes' (create a file with a given string as`
		`# its content)`
		`for write in self.writes:`

lruzicka commented 4 years ago

Previously, it was not possible to set the type of the partition via
the specific GUID. This commit adds support for adding the GUID into
the gtp_type of the partition description in hdds.json and this field
will be utilized in to code.

Metadata Update from @lruzicka:
- Request assigned

4 years ago

Metadata Update from @lruzicka:
- Request assignee reset

4 years ago

Metadata Update from @lruzicka:
- Request assigned

4 years ago

rebased onto bdec3e4

4 years ago

adamwill commented 4 years ago

The code here looks fine, but can you separate the addition of the feature from the addition of the disk to hdds.json? They feel like separate commits to me. And depending on exactly how we decide to implement the test side of this, the disk image may have to be called something other than 'windows'...

rebased onto a89530c

4 years ago

2 new commits added

Add a new hdd that matches the Windows partition layout to be used for dualboot tests.
Add support for setting the GUID of the partition to set

4 years ago

lruzicka commented 4 years ago

I renamed the image to "dualboot" and I also added some partitions to mock the Windows partition to use it in "dualboot" tests where Fedora is installed onto HDD1 and this one should stay intact. This partition layout is too small to actually resize it and install Fedora onto it. But maybe I can make it bigger, like 20G to enable partition resizing. What do you think, @kparal and @adamwill ?

2 new commits added

Add a new hdd that matches the Windows partition layout to be used for dualboot tests.
Add support for setting the GUID of the partition to set

4 years ago

adamwill commented on line 14 of createhdds.py 4 years ago

why not just use gpt_type here?

adamwill commented 4 years ago

And yeah, making it bigger sounds sensible.

1 new commit added

Change the source of parameter according to review.

4 years ago

lruzicka commented 4 years ago

And yeah, making it bigger sounds sensible.

Ok, I will make it bigger tomorrow. Will 10GB be enough?

1 new commit added

Make the image big enough for a system to be installed onto it.

4 years ago

rebased onto b633593

4 years ago

rebased onto 8de704b

4 years ago

rebased onto f77d272

4 years ago

kparal commented 4 years ago

If it has Microsoft-defined GUIDs and partition labels, it's probably silly not to call the image "Windows" :-) About default size, I don't know. If this is used just for "install to a second drive" dual-boot testing, then this can be as minimal as possible (the only partition adjusted might be the ESP). And then for "dual-boot on a single drive" another image can be defined. If you want to share the same image for both use cases, then the overall size should be something like "default Fedora disk size + 1GB" (that's about 500MB for ESP and extra MS partitions, and 500BM for the main Windows partition to get squeezed into).

rebased onto de104a8

4 years ago

lruzicka commented 4 years ago

We might not need this fake Windows disk in the end, but can we merge this? I think that the gpt option is useful for the future?

adamwill commented 4 years ago

what I meant about the naming is we may need the name to be such that it can be 'selected' in the templates based on an openQA variable, like we do for various other disks and tests already, using the %VARNAME% syntax. I was thinking we may have a test that's run on both BIOS and UEFI and needs to use a different disk image for each case - in this scenario we would need the names for the disk images to include the MACHINE variable name but otherwise be identical, or something like that. But I guess we can fix that later.

Edited 4 years ago by adamwill

adamwill commented 4 years ago

can you drop the commit that actually adds the new image from the PR for now, and I'll just merge the feature addition? we can do the new image as a separate PR whenever we actually need it, at the same time as an os-autoinst-distri-fedora PR that uses it.

rebased onto b1287e6

4 years ago

lruzicka commented 4 years ago

Sure, I deleted it :)

adamwill commented 4 years ago

ok, can you rebase this now...we should also update the docs...

rebased onto e63e831

4 years ago

lruzicka commented 4 years ago

Rebased. I created an issue to update the docs and will do so in a different PR.