Libguestfs - Jun 2022 - [nbdkit PATCH 3/3] tests: Add regression test for NBDKIT_EMULATE_CACHE fix

Series looks good:

Acked-by: Richard W.M. Jones <rjones at redhat.com>

I think nbdkit-ext2-filter is still wrong (although at least it should
no longer corrupt disk images) because it unnecessarily calls
next->can_zero, so it might be worth dropping those two lines.

Regression test for the LUKS case might be useful.  But note that the
test should not only check that zeroes work, but also that the result
is still a LUKS image.  The reason is that a naive fix for the earlier
problem can (as I discovered) corrupt the disk image, because it will
forward zeroes (eg to offset 0) straight through to the plugin.  If
this is too much, then I can write this test case tomorrow.

Thanks!

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

On Fri, Jun 10, 2022 at 05:21:19PM +0100, Richard W.M. Jones
wrote:
> 
> Series looks good:
> 
> Acked-by: Richard W.M. Jones <rjones at redhat.com>
> 
> I think nbdkit-ext2-filter is still wrong (although at least it should
> no longer corrupt disk images) because it unnecessarily calls
> next->can_zero, so it might be worth dropping those two lines.
ext2 does not have an interface (yet?) for emulating fallocate()
operations (or other bulk-zeroing operation) against a file; if it
_did_ have one, then that's what ext2's .zero should do.  But we _do_
want to allow the compressed transmission effects of
NBD_CMD_WRITE_ZEROES over the network, so having .can_zero return
NBDKIT_ZERO_EMULATE is right.  On the other hand, ext2 DOES have a way
for the file system itself to request bulk-zeroing of a portion of the
underlying disk, so we _do_ call into the plugin's next->zero(), which
means we _do_ need to check next->can_zero() up front (see the io.c
callback io_zeroout).  I don't see any bugs in this area, once this
series is in.
> 
> Regression test for the LUKS case might be useful.  But note that the
> test should not only check that zeroes work, but also that the result
> is still a LUKS image.  The reason is that a naive fix for the earlier
> problem can (as I discovered) corrupt the disk image, because it will
> forward zeroes (eg to offset 0) straight through to the plugin.  If
> this is too much, then I can write this test case tomorrow.
I'll let you handle that one, then.  These three patches pushed as:

09a0e4dc..6ced998b

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