Re: [Qemu-block] [Qemu-devel] Options for 4.0

[Markus Armbruster]

Peter Krempa <address@hidden> writes:

> On Sun, Mar 31, 2019 at 14:56:19 +0200, Markus Armbruster wrote:
>> Let's review our options for 4.0.
>> 
>> Please note my analysis is handicapped by incomplete information, in
>> particular on libvirt's needs.
>
> Libvirt's needs are "simple" we need to do block-commit. It has a few
> caveats though. The selinux labels for backing images don't allow write
> operation when starting qemu. When doing block-commit libvirt relabels
> the backing chain prior to using block-commit.

Paraphrasing, to make sure I got you.

Consider the common read/write COW image with a backing image.

Libvirt wants to grant QEMU read/write access to the COW and read-only
access to the backing image.  It does that with SELinux.  QEMU has to
open the backing image read-only (read/write would fail).

But libvirt also wants to do block-commit.  Libvirt first relabels to
grant QEMU read/write access to the backing image, then $something makes
QEMU reopen the backing image read/write, block-commit does its job,
$something makes QEMU reopen the backing image read-only, libvirt
relabels to revoke read/write access.

Correct?

With dynamic read-only, $something is the block commit job (a writer)
attaching and detaching to the backing image node.

> That works with -drive as qemu reopens the files internally, but does
> not work when using -blockdev.

Happy to take your word for it.

> Since we need to keep the images RO until doing  the blockjob there
> needs to be a way when either qemu automagically does what libvirt needs
> even if the image did not have the write permission originally.
>
> auto-read-only was a naive impl when qemu attempted to keep the storage
> access nodes RW. At the point when I was testing it I didn't enable
> s-virt as it's a hassle when you want to run git versions of libvirt and
> qemu and thus didn't see the problem with missing permissions.

Yes, fallback read-only is useless for the scenario above.

Kevin wrote "libvirt requires dynamic read-only for file-posix. It
prefers dynamic read-only for other drivers, but can live with fallback
read-only there."  I'm not sure how it can live with fallback read-only.
Is it because only *files* can labelled with SELinux?

If yes, then my next question is how libvirt makes use of fallback
read-only for non-files.  Can you give me an example?

> The dynamic version attempts to fix that. That is still the automagic
> approach. I don't really mind also doing a hands-on approach where we'd
> need to tell qemu to reopen given nodes.

Aha!  You just corrected my overly narrow idea of the solution space.

> I'm not sure what ends up being less work.

"Less work" is an important consideration.

We may be able to accept some extra work to get a saner interface.
Depends on how much saner and how much extra exactly, and on who needs
to do the work.

>> Terminology:
>> 
>> * "Hard read-write" semantics: open read/write.
>> 
>> * "Hard read-only" semantics: open read-only.
>> 
>> * "Dynamic read-only" semantics: open read-only, reopen read/write when
>>   a writer attaches, reopen read-only when the last writer detaches.
>> 
>> * "Fallback read-only" semantics:. try to open read/write, fall back to
>>   read-only.
>> 
>> We have a use case for dynamic read-only: libvirt.  I'm not aware of a
>> use case for fallback read-only.
>> 
>> Behavior before 3.1:
>> 
>> * read-only=on: hard read-only
>> 
>> * read-only=off: hard read-write
>> 
>> Behavior in 3.1: new parameter auto-read-only, default off.
>> 
>> * read-only=on: hard read-only (no change)
>> 
>> * read-only=on,auto-read-only=on: hard read-only (auto-read-only=on
>>   silently ignored)
>> 
>> * read-only=off: hard read-write
>> 
>> * read-only=off,auto-read-only=on: depends on the driver
>> 
>>   - file-posix.c's drivers: fallback read-only
>>   - some other drivers: fallback read-only
>>   - remaining drivers: hard read/write
>> 
>> Behavior in 4.0 so far:
>> 
>> * read-only=on: hard read-only (no change)
>> 
>> * read-only=on,auto-read-only=on: hard read-only (no change)
>> 
>> * read-only=off: hard read-write (no change)
>> 
>> * read-only=off,auto-read-only=on: depends on the driver
>> 
>>   - file-posix.c's drivers: dynamic read-only
>>   - some other drivers: fallback read-only (no change)
>>   - remaining drivers: hard read/write (no change)
>> 
>> 
>> Option 1: Rename @auto-read-only.
>> 
>> Rationale: we released auto-read-only in v3.1 with unproven fallback
>> read-only semantics.  It turned out not to be useful.  Admit the
>> mistake, retract it.  Release our next attempt in 4.0 under a suitable
>> new name with fallback read-only semantics.
>
> I concur. Nobody probably uses this. Setting up selinux and blockdev is
> not a trivial excercise.

Right.

>> CON:
>> 
>> * Compatibility break.  No-go if there are users.  Users seem quite
>>   unlikely, though.
>> 
>> * Still unproven, albeit less so: this time we have some unreleased
>>   (unfinished?) libvirt code.
>
> unfinished and thus also unreleased.

Noted.

>> 
>> * Semantics are still largely left to drivers, and the schema can't tell
>>   which one does what.  Awkward.
>> 
>> * Unless we're fairly confident we won't upgrade drivers from "hard" to
>>   "fallback" to "dynamic", this merely kicks the can down the road:
>>   we'll face the exact same "how can libvirt find out" problem on every
>>   upgrade.
>> 
>> PRO:
>> 
>> * When libvirt sees the new name, it can rely on file-posix.c's drivers
>>   providing dynamic read-only semantics.
>> 
>> 
>> Option 2: Add query-qemu-features command, return
>> file-dynamic-auto-read-only #ifdef CONFIG_POSIX.
>> 
>> Rationale: we released auto-read-only in v3.1 with unproven fallback
>> read-only semantics.  It turned out not to be useful.  Admit the
>> mistake, and patch it up in 4.0.  Libirt needs to know whether it's
>> patche up, and this is a simple way to tell it.
>> 
>> CON:
>> 
>> * All of option 1's, except for the compatibility break
>> 
>> * Uses query-qemu-features to expose a property of the build.  I
>>   consider that a mistake.
>> 
>> PRO
>> 
>> * Libvirt can use either query-qmp-schema or query-qemu-features to find
>>   out whether it can can rely on file-posix.c's drivers providing
>>   dynamic read-only semantics.  To make query-qemu-features usable, we
>>   need to promise query-qemu-features never returns false for it.  To
>>   make query-qemu-features usable, we need to promise the value will
>>   remain valid for the remainder of the QEMU run (defeats caching) or
>>   for any future run of the same QEMU binary (enables caching).
>> 
>> 
>> Option 3: Add @dynamic-read-only to the drivers that provide dynamic
>> read-only, default to value of auto-read-only, promise we'll never add
>> it to drivers that don't.
>> 
>> Rationale: give users explicit control over dynamic vs. fallback for all
>> drivers that can provide dynamic.  This makes some sense as an
>> interface, as long as you ignore the fact that no driver implements both
>> dynamic and fallback.  I can't see why a driver couldn't implement both.
>> It also makes dynamic support visible in the schema.
>> 
>> Behavior (three bools -> eight cases):
>> 
>> * read-only=on: hard read-only (no change)
>> 
>>   Shorthand for read-only=on,auto-read-only=off,dynamic-read-only=off
>> 
>> * read-only=on,auto-read-only=on: hard read-only (no change)
>> 
>>   Shorthand for read-only=on,auto-read-only=on,dynamic-read-only=on
>> 
>> * read-only=off: hard read-write (no change)
>> 
>>   Shorthand for read-only=off,auto-read-only=off,dynamic-read-only=off
>> 
>> * read-only=off,auto-read-only=on:
>> 
>>   Shorthand for read-only=off,auto-read-only=on,dynamic-read-only=on
>> 
>>   - file-posix.c's drivers: dynamic read-only (no change,
>>     dynamic-read-only=on is the default)
>>   - some other drivers: fallback read-only (no change)
>>   - remaining drivers: hard read/write (no change)
>> 
>> * read-only=off,auto-read-only=on,dynamic-read-only=off
>> 
>>   - file-posix.c's drivers: error
>>   - all other drivers: N/A
>> 
>> * read-only=off,auto-read-only=off,dynamic-read-only=on
>> 
>>   - file-posix.c's drivers: error
>>   - all other drivers: N/A
>> 
>> * read-only=on,dynamic-read-only=on
>> 
>>   Shorthand for read-only=on,auto-read-only=off,dynamic-read-only=on
>> 
>>   - file-posix.c's drivers: error
>>   - all other drivers: N/A
>> 
>> * read-only=on,auto-read-only=on,dynamic-read-only=off
>> 
>>   Shorthand for read-only=on,auto-read-only=on,dynamic-read-only=off
>> 
>>   - file-posix.c's drivers: error
>>   - all other drivers: N/A
>> 
>> CON:
>> 
>> * Two bools (read-only and auto-read-only) to select from three choices
>>   was already ugly.  Three bools (the same plus dynamic-read-only) to
>>   select from four choices is even uglier.
>> 
>> * The explicit control is just a facade so far: since only the default
>>   setting is implemented, it doesn't actually control anything.
>> 
>> PRO:
>> 
>> * When libvirt sees a driver providing a dynamic-read-only parameter, it
>>   knows it can rely on the driver providing dynamic read-only semantics.
>> 
>> * Adding dynamic read-only capability to drivers creates no
>>   introspection problems: we simply add dynamic-read-only to their
>>   parameters.
>
> In the end libvirt does not care that much if the storage is open read
> only or read write. We need the format node or guest frontend to deny
> writes if the disk is declared as read-only. We also need to be able to
> do blockjobs. The requirement is that images may not be accessible in
> R/W mode when qemu is starting but later may become. Then it's expected
> that block-commit will succeed.

Fallback read-only can't help there.

Dynamic read-only should work.  Explicit reopen triggers could also
work.