Re: [PATCH v2 2/6] parallels.txt: fix bitmap L1 table description

From: Kevin Wolf <kwolf@redhat.com>
To: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Cc: qemu-block@nongnu.org, qemu-devel@nongnu.org, mreitz@redhat.com,
	stefanha@redhat.com, den@openvz.org, jsnow@redhat.com
Subject: Re: [PATCH v2 2/6] parallels.txt: fix bitmap L1 table description
Date: Thu, 4 Mar 2021 15:21:45 +0100	[thread overview]
Message-ID: <20210304142145.GC9607@merkur.fritz.box> (raw)
In-Reply-To: <20210224104707.88430-3-vsementsov@virtuozzo.com>

Am 24.02.2021 um 11:47 hat Vladimir Sementsov-Ogievskiy geschrieben:
> Actually L1 table entry offset is in 512 bytes sectors. Fix the spec.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  docs/interop/parallels.txt | 27 +++++++++++++++------------
>  1 file changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/docs/interop/parallels.txt b/docs/interop/parallels.txt
> index f15bf35bd1..73af9a2c4b 100644
> --- a/docs/interop/parallels.txt
> +++ b/docs/interop/parallels.txt
> @@ -208,21 +208,24 @@ of its data area are:
>    28 - 31:    l1_size
>                The number of entries in the L1 table of the bitmap.
>  
> -  variable:   l1_table (8 * l1_size bytes)
> -              L1 offset table (in bytes)
> +  variable:   L1 offset table (l1_table), size: 8 * l1_size bytes
>  
> -A dirty bitmap is stored using a one-level structure for the mapping to host
> -clusters - an L1 table.
> +Dirty bitmap is stored in the array of clusters inside Parallels Image file.
> +Offsets of these clusters are saved in L1 offset table here. Each L1 table
> +entry is a 64bit integer described below:

I think the English grammar needs some fixes here (missing articles).

If I understand correctly, it's also not really an array, which I would
understand as consecutive clusters.

Maybe something like this:

  The dirty bitmap described by this feature extension is stored in a set
  of clusters inside the Parallels image file. The offsets of these
  clusters are saved in the L1 offset table specified by the feature
  extension. Each L1 table entry is a 64 bit integer as described below:

> -Given an offset in bytes into the bitmap data, the offset in bytes into the
> -image file can be obtained as follows:
> +Given an offset in bytes into the bitmap data, corresponding L1 entry is
>  
> -    offset = l1_table[offset / cluster_size] + (offset % cluster_size)
> +    l1_table[offset / cluster_size]
>  
> -If an L1 table entry is 0, the corresponding cluster of the bitmap is assumed
> -to be zero.
> +If L1 table entry is 0, all bits in the corresponding cluster of the bitmap
> +are assumed to be 0.

"an L1 table", like before.

> -If an L1 table entry is 1, the corresponding cluster of the bitmap is assumed
> -to have all bits set.
> +If L1 table entry is 1, all bits in the corresponding cluster of the bitmap
> +are assumed to be 1.

Same here.

> -If an L1 table entry is not 0 or 1, it allocates a cluster from the data area.
> +If an L1 table entry is not 0 or 1, it contains corresponding cluster offset

"the corresponding cluster offset"

> +(in 512b sectors). Given an offset in bytes into the bitmap data the offset in
> +bytes into the image file can be obtained as follows:
> +
> +    offset = l1_table[offset / cluster_size] * 512 + (offset % cluster_size)

These changes can be made while applying the patch.

Kevin