[Libguestfs] [libnbd PATCH] interop: Tolerate qemu-nbd 6.0 change in behavior

[Nir Soffer]

On Tue, Apr 13, 2021 at 1:07 AM Eric Blake <eblake at redhat.com> wrote:
>
> Qemu 6.0 commit 0da9856851 (nbd: server: Report holes for faw images)

raw

> intentionally changed how holes are reported, not only for raw images,
> but for known-zero portions of qcow2 files.

Do you have an example for known-zero?

>  As the point of our
> dirty-bitmap test is more about reading multiple contexts from a
> single qemu-nbd process, it is okay to accept both behaviors for
> base:allocation reporting on a block of zeros.
> ---
>  interop/dirty-bitmap.c | 40 +++++++++++++++++++++++++++++-----------
>  1 file changed, 29 insertions(+), 11 deletions(-)
>
> diff --git a/interop/dirty-bitmap.c b/interop/dirty-bitmap.c
> index 5f9fa12f..ed445712 100644
> --- a/interop/dirty-bitmap.c
> +++ b/interop/dirty-bitmap.c
> @@ -1,5 +1,5 @@
>  /* NBD client library in userspace
> - * Copyright (C) 2013-2019 Red Hat Inc.
> + * Copyright (C) 2013-2021 Red Hat Inc.
>   *
>   * This library is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU Lesser General Public
> @@ -48,7 +48,10 @@ cb (void *opaque, const char *metacontext, uint64_t offset,
>
>    /* libnbd does not actually verify that a server is fully compliant
>     * to the spec; the asserts marked [qemu-nbd] are thus dependent on
> -   * the fact that qemu-nbd is compliant.
> +   * the fact that qemu-nbd is compliant.  Furthermore, qemu 5.2 and
> +   * 6.0 disagree on whether base:allocation includes the hole bit for
> +   * the zeroes at 512k (both answers are compliant); but we care more
> +   * that the zeroes show up in the dirty bitmap

So 6.0 show reports a hole for zeroed area, and 5.2 reports data?

When we copy the image with qemu-img convert, do we get
a hole in the destination?

>     */
>    assert (offset == 0);
>    assert (!*error || (data->fail && data->count == 1 && *error == EPROTO));
> @@ -57,19 +60,34 @@ cb (void *opaque, const char *metacontext, uint64_t offset,
>    if (strcmp (metacontext, LIBNBD_CONTEXT_BASE_ALLOCATION) == 0) {
>      assert (!data->seen_base); /* [qemu-nbd] */
>      data->seen_base = true;
> -    assert (len == (data->req_one ? 2 : 8)); /* [qemu-nbd] */
> +    if (data->req_one)
> +      assert (len == 2); /* [qemu-nbd] */
> +    else
> +      assert ((len & 1) == 0 && len > 2); /* [qemu-nbd] */
>
>      /* Data block offset 0 size 128k */
>      assert (entries[0] == 131072); assert (entries[1] == 0);
>      if (!data->req_one) {
> -      /* hole|zero offset 128k size 384k */
> -      assert (entries[2] == 393216); assert (entries[3] == (LIBNBD_STATE_HOLE|
> -                                                            LIBNBD_STATE_ZERO));
> -      /* allocated zero offset 512k size 64k */
> -      assert (entries[4] ==  65536); assert (entries[5] == LIBNBD_STATE_ZERO);
> -      /* hole|zero offset 576k size 448k */
> -      assert (entries[6] == 458752); assert (entries[7] == (LIBNBD_STATE_HOLE|
> -                                                            LIBNBD_STATE_ZERO));
> +      if (len == 4) {
> +        /* hole|zero offset 128k size 896k */

Is this the qemu-nbd 5.2 behavior?

> +        assert (entries[2] == 917504);
> +        assert (entries[3] == (LIBNBD_STATE_HOLE|
> +                               LIBNBD_STATE_ZERO));
> +      }
> +      else {
> +        assert (len == 8);
> +        /* hole|zero offset 128k size 384k */

Moving the comment under the else will be more clear.

> +        assert (entries[2] == 393216);
> +        assert (entries[3] == (LIBNBD_STATE_HOLE|
> +                               LIBNBD_STATE_ZERO));
> +        /* allocated zero offset 512k size 64k */
> +        assert (entries[4] ==  65536);
> +        assert (entries[5] == LIBNBD_STATE_ZERO);
> +        /* hole|zero offset 576k size 448k */
> +        assert (entries[6] == 458752);
> +        assert (entries[7] == (LIBNBD_STATE_HOLE|
> +                               LIBNBD_STATE_ZERO));
> +      }
>      }
>    }
>    else if (strcmp (metacontext, bitmap) == 0) {
> --
> 2.31.1
>