Re: [Libguestfs] [PATCH nbdkit] xz: Do not pass can_write through to the plugin.

On 1/30/19 8:36 AM, Richard W.M. Jones wrote: The fix is correct in the sense that yes, the xz filter cannot allow writes, and must therefore override write support from leaking through from the plugin. Arguing that filters.c picks the wrong defaults makes a bit of intuitive sense, but it trickier to implement correctly. We have several examples of filters that override .pwrite but not .can_write (e.g. truncate), some that override .can_write but not .pwrite (xz), some that override both (cow), some that override neither (nozero). When .can_write is overridden, it is obviously correct. But when .can_write is absent, whether or not .pwrite is also absent, I can see your idea of defaulting to "if I passed readonly=true to the plugin's open(), synthesize .can_write of false no matter what; if I passed readonly=false to the plugin's open(), then call the plugin's .can_write() which may return true or false based on other aspects up to the plugin". And we can track what the filter passes to the plugin's .open, since a filter's .open MUST call the next(nxdata) parameters it was given. Since next(nxdata) is implemented in filters.c, THAT could be the place where we stash off whether the filter is passing true/false to the underling plugin's .open (even if that value differs from the value of readonly passed to the filter's .open), such that we can then have filters.c in charge of remembering the per-connection state of whether the underlying plugin is in forced-readonly mode and therefore the filter must synthesize a .can_read that forces false. I agree that our filter API is not stable, so I'm okay with your patch going in now, regardless of whether we also change filters.c to have saner defaults. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org