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