Re: [PATCH v2 2/2] block/null: Add read-pattern option

[Eric Blake]

On Thu, May 08, 2025 at 07:28:26AM +0200, Markus Armbruster wrote:
> Let's take a step back from the concrete interface and ponder what we're
> trying to do.  We want three cases:
> 
> * Allocated, reads return unspecified crap (security hazard)
> 
> * Allocated, reads return specified data
> 
> * Sparse, reads return zeroes
> 
> How would we do this if we started on a green field?
> 
> Here's my try:
> 
>     bool sparse
>     uint8 contents
> 
> so that
> 
> * Allocated, reads return unspecified crap (security hazard)
> 
>   @sparse is false, and @contents is absent
> 
> * Allocated, reads return specified data
> 
>   @sparse is false, and @contents is present
> 
> * Sparse, reads return zeroes
> 
>   @sparse is true, and @contents must absent or zero

That indeed is a nice view of what we hope to test with.

> 
> I'd make @sparse either mandatory or default to true, so that we don't
> default to security hazard.
> 
> Now compare this to your patch: same configuration data (bool × uint8),
> better names, cleaner semantics, better defaults.
> 
> Unless we want to break compatibility, we're stuck with the name
> @read-zeroes for the bool, and its trap-for-the-unwary default value,
> but cleaner semantics seem possible.
> 
> Thoughts?

What if we add @sparse as an optional bool, but mutually exclusive
with @read-zeroes.  That would lead to 27 combinations of absent,
present with 0 value, or present with non-zero value; but with fewer
actual cases supported.  Something like your above table of what to do
with sparse and contents, and with these additional rules:

read-zeroes omitted, sparse omitted
 - at present, defaults to sparse=false for back-compat
 - may change in the future [*]
read-zeroes present, sparse omitted
 - behaves like explicit setting of sparse, but with old spelling
 - may issue a deprecation warning [*]
read-zeroes omitted, sparse present
 - explicit spelling, no warning (use above logic for how contents acts)
read-zeroes and sparse both present
 - error, whether values were same or different

[*] Simultaneously, we start the deprecation cycle on @read-zeroes -
tagging it as deprecated now, and removing it in a couple of releases.
Once it is gone, we are left with just @sparse; at that time, we can
decide to either make it mandatory (if so, we should warn now if
neither read-zeroes nor sparse is specified), or leave it optional but
change it to default true (so that the security hazard of sparse:false
and omitted contents is now opt-in instead of default).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org