[Libguestfs] [nbdkit PATCH 03/10] filters: Wire up filter support for NBD_INFO_INIT_STATE

[Eric Blake]

On 2/11/20 4:40 AM, Richard W.M. Jones wrote:
> On Mon, Feb 10, 2020 at 03:43:56PM -0600, Eric Blake wrote:

> extentlist (answer based on the extents we are advertising), noextents
> (suppress the advertisement), and truncate (advertise sparse support


>> diff --git a/filters/noextents/noextents.c b/filters/noextents/noextents.c
>> index e6ac33b..091f30b 100644
>> --- a/filters/noextents/noextents.c

>>
>> +static int
>> +noextents_init_sparse (struct nbdkit_next_ops *next_ops, void *nxdata,
>> +                       void *handle)
>> +{
>> +  return 0;
>> +}
>> +
>> +static int
>> +noextents_init_zero (struct nbdkit_next_ops *next_ops, void *nxdata,
>> +                     void *handle)
>> +{
>> +  return 0;
>> +}
> 
> Is this true?  If so it could at least be worth a comment in the code
> about why because I'm not clear about it.

My thinking was that if we are suppressing all extent information (the 
entire image appears as data), then it is also worth suppressing 
information that might be learned from extents.  But in hindsight, it 
seems like this is not configurable enough.  I'm thinking that for v2, 
it might be better to:

- leave the noextents filter unchanged other than adding a doc link (it 
suppresses block status by showing the entire image as data, but does 
NOT suppress any initial state info, matching the fact that the NBD 
protocol leaves initial state and block status as orthogonal extensions, 
and keeping the filters orthogonal makes testing this orthogonality easier)

- add a new 'init' filter which allows fine-grained user control over 
what gets advertised in the init state (but does not touch extents):
  + initsparse=off, initzero=off: override the plugin to advertise 0 no 
matter what (client would have to fall back to block-status, or nothing 
at all)
  + initsparse=force, initzero=force: override the plugin to advertise 1 
no matter what (work around a plugin that does not advertise the data 
itself; use with caution as advertising incorrectly may make a client 
think zeroes are present even when they are not - then again, you could 
argue that's a useful test case to see how badly data gets corrupted 
when the answer is wrong ;)
  + initsparse=extents, initzero=extents: override the plugin to provide 
answers based on what block_status reports (may cause a slower startup, 
but is a bit more useful than force if you are worried about blind 
advertisement being too risky)
  + initsparse=check, initzero=check: do not override the plugin, but 
validate using block_status that the plugin's advertised values match 
its block status, or else cause a failure (potentially slower startup, 
or even fails to create a connection, but useful for diagnosing 
scenarios when implementing init flags in plugins)

and maybe:
  + initzero=scan: override the plugin to provide answers based on what 
pread reports (WILL cause a slower startup, and becomes prohibitively 
expensive for huge sparse disks). This mode won't help for initsparse 
(there's no way to tell where sparse areas are using just pread).

The idea of scanning for zeroes may also be useful for the extentslist 
filter (if we're going to read the entire image and look for runs of 
zeroes, or even just the data extents reported by .extents to see which 
allocated portions read as zero, then having that same scan adjust the 
results of .extents to match what was learned may be useful).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org