Fix booting ZFS images. #921

davidchisnall · 2021-09-17T14:29:14Z

FreeBSD now requires ZFS bootable vdevs to live in partitions, rather
than be raw disks. This change creates a GPT partition table and
installs both MBR and UEFI boot loaders.

This change also uses the image name as a temporary name for the zpool
so that zroot can be consistently called zroot, without introducing a
conflict with the host system's zroot. If there is a use case for
having a non-default name for zroot then this can be changed.

Tested booting an image as a Hyper-V Gen1 (MBR) and Gen2 (UEFI) VM.

zrawdisk is no longer a raw disk and so probably needs renaming.

Fixes #735

davidchisnall · 2021-09-17T15:34:19Z

It sounds as if some folks were using the raw image support with mkimg. If this is useful to other people, I can factor out the common bits between ZFS raw image and ZFS + partition table and provide both.

evadot · 2021-09-21T08:01:03Z

It sounds as if some folks were using the raw image support with mkimg. If this is useful to other people, I can factor out the common bits between ZFS raw image and ZFS + partition table and provide both.

That's the goal of the zrawdisk type, if you want some partition table I suggest you put that in a new zdisk type image.

dch · 2021-10-06T08:26:57Z

@davidchisnall this sounds very handy

davidchisnall · 2021-10-06T08:53:03Z

I'll try to refactor this to pull out the common bits and add a bootable zdisk type either this week or at the weekend.

davidchisnall · 2021-10-08T14:03:10Z

I've just pushed the refactoring. I've tried to minimise the copy-and-pasted code by pulling things out into include/fs.sh. This version almost preserves the existing behaviour of zrawdisk. I'd welcome feedback if the change is annoying to anyone:

The existing zrawdisk code used the image name to specify the name of the ZFS root pool. I have changed this so that it is always zroot (as with the installer and all of the examples in the handbook). Using a non-zroot name was necessary before because the alternate name of the pool was not set and so if the host system had a pool called zroot then it would fail. I am now using the image name to generate the alternate name, so it works unless you are trying to generate two images with the same name at the same time (and since they'd end up in the same file - don't do that!).

Does anyone want / depend on the old behaviour?

davidchisnall · 2021-10-11T08:46:23Z

@evadot, can you review? Apparently I don't have permissions to explicitly request reviewers on this repo.

evadot · 2021-10-11T08:57:28Z

@evadot, can you review? Apparently I don't have permissions to explicitly request reviewers on this repo.

Yes I plan to do this this week, no firm ETA

evadot

Could you :
squash the commits (I don't like having one commit breaking zrawdisk and the one after unbreaking it and adding zdisk)
Move the stuff out from include/fs.sh, this is a file for poudriere and I'd like to have the poudriere image bits separated.
Otherwise looks good.

davidchisnall · 2021-10-13T10:25:56Z

@evadot, I've done the refactoring but I can't figure out how to install the image_helpers.sh file. Everything I worked on that used GNU autotools moved to CMake a decade ago and so I've never had to touch a build system like this before.

davidchisnall · 2021-10-13T10:30:37Z

I have tried:

Adding it to Makefile.am (doesn't seem to regenerate the things that are needed)
Rerunning ./configure (Doesn't seem to do anything)
Running ./autogen.sh (Generates a configure that contains a syntax error)

I am now giving up.

dch · 2021-10-13T17:56:30Z

src/share/poudriere/include/fs.sh

+	zfs set mountpoint=none ${zroot}/ROOT/default
+	zfs set readonly=on ${zroot}/var/empty
+	zpool set bootfs=${zroot}/ROOT/default ${zroot}
+	zpool set autoexpand=on ${zroot}


src/share/poudriere/image_rawdisk.sh

src/share/poudriere/include/fs.sh

davidchisnall · 2021-10-14T09:02:21Z

@evadot I am not joking about not being able to make the build system work. I have added the new file to Makefile.am and run autogen.sh. If I then run ./configure, I get:

$ ./configure
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... /usr/local/bin/gmkdir -p
checking for gawk... no
checking for mawk... no
checking for nawk... nawk
checking whether make sets $(MAKE)... yes
checking whether make supports nested variables... yes
checking whether make supports nested variables... (cached) yes
checking whether to enable maintainer-specific portions of Makefiles... no
./configure: 2875: Syntax error: word unexpected (expecting ")")

The offending line is:

AX_CHECK_ENABLE_DEBUG(yes)

I have no idea where that came from or why. At this point, it would take me less time to rewrite the build system in CMake than to figure out how GNU autothingies work. Please advise.

evadot · 2021-10-14T11:39:26Z

@evadot I am not joking about not being able to make the build system work. I have added the new file to Makefile.am and run autogen.sh. If I then run ./configure, I get:
$ ./configure
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... /usr/local/bin/gmkdir -p
checking for gawk... no
checking for mawk... no
checking for nawk... nawk
checking whether make sets $(MAKE)... yes
checking whether make supports nested variables... yes
checking whether make supports nested variables... (cached) yes
checking whether to enable maintainer-specific portions of Makefiles... no
./configure: 2875: Syntax error: word unexpected (expecting ")")
The offending line is:
AX_CHECK_ENABLE_DEBUG(yes)
I have no idea where that came from or why. At this point, it would take me less time to rewrite the build system in CMake than to figure out how GNU autothingies work. Please advise.

See my commit when I added the split (b768a36) I have no clue about autotool TBH.

FreeBSD now requires ZFS bootable vdevs to live in partitions, rather than be raw disks. This change adds a zdisk type that creates a GPT partition table and installs both MBR and UEFI boot loaders. The zrawdisk mode should generate a single ZFS partition as before. Common code between rawdisk, zrawdisk, and zdisk has been factored out. The zrawdisk and zdisk targets now always calls the zpool zroot, since that's what all of the docs expect and it's annoying to boot a VM image and find its ZFS root pool is called something else. The image name is used to generate a *temporary* name that means that this works even if the host already has a ZFS pool called zroot. Tested booting an image as a Hyper-V Gen1 (MBR) and Gen2 (UEFI) VM. Fixes freebsd#735

davidchisnall · 2021-10-14T15:51:15Z

I've put the helpers into image_rawdisk.sh, since they're not currently used anywhere else. I believe the squashed commit should address all of the feedback. Since I'm not adding a new file, I don't have to fight the build system.

davidchisnall · 2021-10-15T12:04:36Z

All checks passing, I believe this should be ready to land.

evadot · 2021-10-18T17:53:28Z

All checks passing, I believe this should be ready to land.

Well commit shouldn't say "Fix booting ZFS images." as this doesn't fix anything, that adds a new type.
Other than that I really don't like having the zdisk image in the same file as the rawdisks one, this really should be a different file otherwise things will start to be unreadable at one point (as it was before).

davidchisnall · 2021-10-18T19:35:36Z

Well commit shouldn't say "Fix booting ZFS images." as this doesn't fix anything, that adds a new type.

Welcome to GitHub, where you can edit commit messages when you press the squash and merge button.

Other than that I really don't like having the zdisk image in the same file as the rawdisks one, this really should be a different file otherwise things will start to be unreadable at one point (as it was before).

This patch is needed for me to be able to build Azure-bootable FreeBSD disk images. I've been trying to get it merged for over a month now. Please either:

Review it and give me a complete list of changes that you require (if this requires adding new files, please just push the change directly - I can't make this project's '80s build system do anything that it doesn't already do)
Merge it as-is and make any improvements that you want afterwards, or
Close it and the original issue as WONTFIX so I can give up on Poudriere and work on something else.

evadot · 2021-10-18T19:52:17Z

Well commit shouldn't say "Fix booting ZFS images." as this doesn't fix anything, that adds a new type.

Welcome to GitHub, where you can edit commit messages when you press the squash and merge button.

I won't do that.

Other than that I really don't like having the zdisk image in the same file as the rawdisks one, this really should be a different file otherwise things will start to be unreadable at one point (as it was before).

This patch is needed for me to be able to build Azure-bootable FreeBSD disk images. I've been trying to get it merged for over a month now. Please either:
* Review it and give me a complete list of changes that you require (if this requires adding new files, please just push the change directly - I can't make this project's '80s build system do anything that it doesn't already do)

Move the zdisk image stuff in image_zdisk.sh and no I won't do the change myself, you're a grown man.

* Merge it as-is and make any improvements that you want afterwards, or

* Close it and the original issue as WONTFIX so I can give up on Poudriere and work on something else.

davidchisnall force-pushed the zfsboot branch from a14abfb to 0d791a6 Compare September 17, 2021 15:35

grahamperrin mentioned this pull request Sep 18, 2021

Rebase onto FreeBSD 13.0-RELEASE ravynsoft/ravynos#53

Closed

evadot requested changes Oct 12, 2021

View reviewed changes

dch reviewed Oct 13, 2021

View reviewed changes

src/share/poudriere/image_rawdisk.sh Outdated Show resolved Hide resolved

dch reviewed Oct 13, 2021

View reviewed changes

src/share/poudriere/include/fs.sh Outdated Show resolved Hide resolved

davidchisnall force-pushed the zfsboot branch from eda24e0 to 6692c69 Compare October 14, 2021 15:41

davidchisnall closed this Oct 19, 2021

davidchisnall mentioned this pull request Aug 2, 2023

Poudriere image -t zrawdisk tries to copy nonexistent boot files on AArch64 #1069

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix booting ZFS images. #921

Fix booting ZFS images. #921

davidchisnall commented Sep 17, 2021

davidchisnall commented Sep 17, 2021

evadot commented Sep 21, 2021

dch commented Oct 6, 2021

davidchisnall commented Oct 6, 2021

davidchisnall commented Oct 8, 2021

davidchisnall commented Oct 11, 2021

evadot commented Oct 11, 2021

evadot left a comment

davidchisnall commented Oct 13, 2021

davidchisnall commented Oct 13, 2021

dch Oct 13, 2021

davidchisnall commented Oct 14, 2021

evadot commented Oct 14, 2021

davidchisnall commented Oct 14, 2021

davidchisnall commented Oct 15, 2021

evadot commented Oct 18, 2021

davidchisnall commented Oct 18, 2021

evadot commented Oct 18, 2021

Fix booting ZFS images. #921

Fix booting ZFS images. #921

Conversation

davidchisnall commented Sep 17, 2021

davidchisnall commented Sep 17, 2021

evadot commented Sep 21, 2021

dch commented Oct 6, 2021

davidchisnall commented Oct 6, 2021

davidchisnall commented Oct 8, 2021

davidchisnall commented Oct 11, 2021

evadot commented Oct 11, 2021

evadot left a comment

Choose a reason for hiding this comment

davidchisnall commented Oct 13, 2021

davidchisnall commented Oct 13, 2021

dch Oct 13, 2021

Choose a reason for hiding this comment

davidchisnall commented Oct 14, 2021

evadot commented Oct 14, 2021

davidchisnall commented Oct 14, 2021

davidchisnall commented Oct 15, 2021

evadot commented Oct 18, 2021

davidchisnall commented Oct 18, 2021

evadot commented Oct 18, 2021