search

RPi-Distro / pi-gen

Tool used to create the official Raspberry Pi OS images
BSD 3-Clause "New" or "Revised" License
2.58k stars 1.61k forks source link

Remove QCOW2 build mechanism #648

Closed mmmspatz closed 6 months ago

mmmspatz commented 1 year ago

IMO bf8c9f5 added an undue amount of complexity, and this comment suggests that maintainers are open to getting rid of it. So I went ahead and did that.

Carefully revert bf8c9f5, making a few exceptions:

  1. Keep some inconsequential formatting changes & line reordering. No reason to change it back.
  2. Keep the change that mounts host /dev in the container during the docker build. This happens to have been introduced in bf8c9f5, but is necessary for loop devices to show up after losetup runs. See: https://github.com/moby/moby/issues/27886

I suggest viewing this commit with "hide whitespace changes" enabled.

XECDesign commented 1 year ago

Many thanks for doing this.

I'll leave it for a bit to go over it and give people a chance to raise objections.

In the meantime, I'll implement a simpler alternative approach.

mmmspatz commented 1 year ago

I took a stab at doing a similar thing in a much simpler way with btrfs subvolumes. It is just an optimization of copy_previous(); the final export still rsyncs files into filesystems backed by a loop device. It happens automatically if the host filesystem happens to be btrfs.

Here is the main commit: https://github.com/mmmspatz/pi-gen/commit/862219c8066c4fc51a93898ec06af7be5db074de And here that is on a branch.

If this is of interest or appeases anyone, I can make a new PR for it. The speedup is pretty much nil for me (copying a handful of GB a handful of times does not take that long relative to everything else), but the disk usage savings are there. And maybe someone has a situation where the speedup would be noticeable.

An even simpler change than what I've done here would be to replace rsync with cp in copy_previous(), then we could pass --reflink=auto and get almost exactly the same benefit. rsync does not seem to have a similar feature.

mmmspatz commented 1 year ago

Crossed my mind to bump this, so here I am doing so. Just rebased onto current master.

jakicoll commented 11 months ago

Now that the qcow2 mechanism is broken on bookworm (#710), I guess this should really be merged in.

mmmspatz commented 9 months ago

Just rebased onto current master again. Merge conflicts were straightforward to solve.

mmmspatz commented 8 months ago

Another straightforward rebase onto master.

XECDesign commented 6 months ago

Thanks and sorry about the delay. I just wanted to make sure there has been enough warning that this is happening.