commit c474897ffc2170d351abcf26cad6ba22c6de76ab
author Frederick Mayle <fmayle@google.com> Thu Sep 05 19:20:39 2024 -0700
committer Frederick Mayle <fmayle@google.com> Thu Sep 12 13:00:19 2024 -0700
tree cdff084ee98f6ae6f4c565093b032e750da34c67
parent eae6d2d3eafe04f9ad849a0abdf58f216d8d2dd2

virtmgr: drop support for qcow2 and composite as partition images

We'll now throw an error if qcow2 or composite disks are passed as
partition images. The code for detecting the image type and calculating
the partition size has been forked from crosvm and significantly
simplified.

We still depend on crosvm's library for generating composite disk files
and GUID Partition Tables.

=== Why? ===

The real motivation for this change is that the `disk` crate APIs have
changed in crosvm and it doesn't seem possible to use the new API
correctly from virtmgr. The main issue is that the `disk` crate now
wants to open the file itself from a path (so that it can ensure
consistent file locking and use of features like O_DIRECT and overlapped
IO). virtmgr only has an FD for the partition images, so it would need
to give the crosvm library a "/proc/self/fd/..." path. However, crosvm
code only allows those paths if they've come from a parent process and
it attempts to validate that by checking for the absense of the CLOEXEC
flag. We can't safely clear the CLOEXEC flag on a file in virtmgr
without introducing a race condition with exec calls on other threads.

You might think "the real problem is the crosvm API change, it should be
reverted", but these are crosvm internal APIs and we aren't supposed to
be using them to begin with. I'm the author the API changes and I can't
think of a good alternative that doesn't meaningfully make the crosvm
code worse and possibly reintroduce some of the bugs that are being
fixed.

=== Are we losing something useful? ===

I think these two image formats aren't actually useful as partition
images in the current AVF design.

If a VM is requested with a DiskImage containing a list of partition
images, virtmgr generates a composite disk image from those partitions.
The individual partition images are passed as FDs, at least in part
because virtmgr and crosvm processes are unlikely to have permissions to
open them from a filesystem path.

When one of the partition images themselves is a composite image things
get tricky. The crosvm process would need to open the files specified
within that image using their paths (which it doesn't have permissions
to do, as mentioned above). One might try working around that using a
"/proc/self/fd/..." path, however, there is no way to pass those FDs the
virtmgr process and virtmgr doesn't have any logic to pass them along to
the crosvm process.

QCOW2 has similar issues for its "backing file" feature. Given the
subset of QCOW2 features supported by crosvm right now, I believe the
only good reason to use it is for the "backing file" functionality.

Note that these two formats will continue to work (to the limited degree
they did before) when passed directly as the root of a DiskImage.

=== Is there a better long term solution? ===

The virtmgr code for creating a composite file looks like it has been
forked from the `crosvm create_composite` CLI command. One idea is to
de-fork by having virtmgr invoke that command. At least one hurdle in
that direction is that we'll probably want that invocation of crosvm to
use a different SELinux domain so that we don't have to give the main
VMM instance permissions to create files and such.

Another idea is to push the complexity onto the caller. Make them pass
the partition size explicitly instead of inferring it from the image
file.

Bug: 364645148, 330911976, 269356487
Test: launch microdroid
Change-Id: Ibb686b51b646c52993c3c02f5738bcb2356f7432