Libguestfs - Jul 2022 - nbdkit test-checkwrite.sh failing occasionally on ppc64le

On 07/19/22 16:17, Richard W.M. Jones wrote:
> Note I'm not claiming the assertion will not fail!  Just that the
> assertion is correct, and if it fails we've got a bug.
Fair enough, but how do we explain the second paragraph of the leading comment
on the function then?

 * Note we don't check that the extents exactly match since a valid
 * copying operation is to either add sparseness (qemu-img convert -S)
 * or create a fully allocated target (nbdcopy --allocated).

I think it directly contradicts the assertion. That, or I don't understand
the comment at all.

(It's from commit 2d0954dd6431, "New filter: checkwrite",
2020-12-22.)

Laszlo

On Tue, Jul 19, 2022 at 04:41:02PM +0200, Laszlo Ersek
wrote:
> On 07/19/22 16:17, Richard W.M. Jones wrote:
> 
> > Note I'm not claiming the assertion will not fail!  Just that the
> > assertion is correct, and if it fails we've got a bug.
> 
> Fair enough, but how do we explain the second paragraph of the leading
comment on the function then?
> 
>  * Note we don't check that the extents exactly match since a valid
>  * copying operation is to either add sparseness (qemu-img convert -S)
>  * or create a fully allocated target (nbdcopy --allocated).
This is slightly different from the issue of the range covered by
extents, but it does require explanation.

What I meant here was that we treat an extent marked with the zero
flag, and an extent marked with the data flag[0] that returns zeroes
when read as being the same thing.  However extents returned by
nbdkit_extents_full are still assumed to cover the entire range of
[offset..offset+count-1].

The following disks would be logically equivalent[1]:

  $ truncate -s 1M empty
  $ nbdkit file empty --run 'nbdinfo --map $uri'
           0     1048576    3  hole,zero

  $ dd if=/dev/zero of=empty2 count=1 bs=1M
  $ nbdkit file empty2 --run 'nbdinfo --map $uri'
           0     1048576    0  data

nbdkit-checkwrite-filter should permit you to either write zeroes
(using pwrite), trim or zero (or any combination) both of those disks.
It should reject with EIO attempts to write non-zeroes anywhere.

The reason for this is because this filter is designed for testing
copying tools like qemu-img convert & nbdcopy.  You could back it with
a local file like one of the ones above, but more fun is to use a
randomly created virtual disk, eg:

  $ nbdkit linuxdisk ~/d/nbdkit --filter=checkwrite --run 'nbdcopy
"$uri" "$uri"'
  $ nbdkit pattern 1G --filter=checkwrite --run 'nbdcopy --allocated
"$uri" "$uri"'

Because those tools commonly sparsify (-S) or allocate (--allocated)
when they copy, what is written can contain trims or zeroes where the
source disk had data reading as zero, or vice versa.

Some fun facts ...

Some plugins already have the equivalent of checkwrite functionality,
eg this just works without the filter:

  $ nbdkit sparse-random 1T --run 'qemu-img convert -p "$uri"
"$uri"'

qemu-img has new(?) behaviour that breaks some things:

  $ nbdkit linuxdisk ~/d/nbdkit --filter=checkwrite --run 'qemu-img convert
"$uri" "$uri"'
  nbdkit: linuxdisk.0: error: data written does not match expected
  qemu-img: nbd://localhost:10809: error while converting raw: Failed to clear
the new image's first sector: Input/output error

I understand why this happens, but also it's kind of a bug ...  We
should probably modify checkwrite to handle this, but it's rather
complicated.
> I think it directly contradicts the assertion. That, or I don't
understand the comment at all.
> 
> (It's from commit 2d0954dd6431, "New filter: checkwrite",
2020-12-22.)
> 
> Laszlo
Rich.

[0] More precisely: an extent with type == 0.

[1] This is actually the same way that qemu-img compare works.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW