[PATCH 1/1] docs: fix mistake in dirty bitmap feature description

[PATCH 1/1] docs: fix mistake in dirty bitmap feature description

[Denis V. Lunev]

Original specification says that l1 table size if 64 * l1_size, which
is obviously wrong. The size of the l1 entry is 64 _bits_, not bytes.
Thus 64 is to be replaces with 8 as specification says about bytes.

There is also minor tweak, field name is renamed from l1 to l1_table,
which matches with the later text.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 docs/interop/parallels.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/interop/parallels.txt b/docs/interop/parallels.txt
index e9271eba5d..f15bf35bd1 100644
--- a/docs/interop/parallels.txt
+++ b/docs/interop/parallels.txt
@@ -208,7 +208,7 @@ of its data area are:
   28 - 31:    l1_size
               The number of entries in the L1 table of the bitmap.
 
-  variable:   l1 (64 * l1_size bytes)
+  variable:   l1_table (8 * l1_size bytes)
               L1 offset table (in bytes)
 
 A dirty bitmap is stored using a one-level structure for the mapping to host
-- 
2.27.0

Re: [PATCH 1/1] docs: fix mistake in dirty bitmap feature description

[Vladimir Sementsov-Ogievskiy]

28.01.2021 20:13, Denis V. Lunev wrote:
> Original specification says that l1 table size if 64 * l1_size, which
> is obviously wrong. The size of the l1 entry is 64 _bits_, not bytes.
> Thus 64 is to be replaces with 8 as specification says about bytes.
> 
> There is also minor tweak, field name is renamed from l1 to l1_table,
> which matches with the later text.
> 
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Stefan Hajnoczi <stefanha@redhat.com>
> CC: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

> ---
>   docs/interop/parallels.txt | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/docs/interop/parallels.txt b/docs/interop/parallels.txt
> index e9271eba5d..f15bf35bd1 100644
> --- a/docs/interop/parallels.txt
> +++ b/docs/interop/parallels.txt
> @@ -208,7 +208,7 @@ of its data area are:
>     28 - 31:    l1_size
>                 The number of entries in the L1 table of the bitmap.
>   
> -  variable:   l1 (64 * l1_size bytes)
> +  variable:   l1_table (8 * l1_size bytes)
>                 L1 offset table (in bytes)

I don't remember why this "(in bytes)" is here.. What in bytes? L1 table size? But the described field is not L1 table size, but L1 table itself.. It's not in bytes, it's just L1 table :)

So, I'd also drop "(in bytes)" while being here. Or the whole line "L1 offset table (in bytes)" altogether.

>   
>   A dirty bitmap is stored using a one-level structure for the mapping to host
> 


-- 
Best regards,
Vladimir

Re: [PATCH 1/1] docs: fix mistake in dirty bitmap feature description

[Eric Blake]

On 1/28/21 11:21 AM, Vladimir Sementsov-Ogievskiy wrote:
> 28.01.2021 20:13, Denis V. Lunev wrote:
>> Original specification says that l1 table size if 64 * l1_size, which
>> is obviously wrong. The size of the l1 entry is 64 _bits_, not bytes.
>> Thus 64 is to be replaces with 8 as specification says about bytes.
>>
>> There is also minor tweak, field name is renamed from l1 to l1_table,
>> which matches with the later text.
>>
>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>> CC: Stefan Hajnoczi <stefanha@redhat.com>
>> CC: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> 
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> 

I saw the subject "dirty bitmap", and assumed it would go through my
dirty bitmap tree.  In reality, it's unrelated to the dirty bitmap code.
 Would an improved subject line help?

>> ---
>>   docs/interop/parallels.txt | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/docs/interop/parallels.txt b/docs/interop/parallels.txt
>> index e9271eba5d..f15bf35bd1 100644
>> --- a/docs/interop/parallels.txt
>> +++ b/docs/interop/parallels.txt
>> @@ -208,7 +208,7 @@ of its data area are:
>>     28 - 31:    l1_size
>>                 The number of entries in the L1 table of the bitmap.
>>   -  variable:   l1 (64 * l1_size bytes)
>> +  variable:   l1_table (8 * l1_size bytes)
>>                 L1 offset table (in bytes)
> 
> I don't remember why this "(in bytes)" is here.. What in bytes? L1 table
> size? But the described field is not L1 table size, but L1 table
> itself.. It's not in bytes, it's just L1 table :)
> 
> So, I'd also drop "(in bytes)" while being here. Or the whole line "L1
> offset table (in bytes)" altogether.
> 
>>     A dirty bitmap is stored using a one-level structure for the
>> mapping to host
>>
> 
> 

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

Re: [PATCH 1/1] docs: fix mistake in dirty bitmap feature description

[Denis V. Lunev]

On 2/3/21 1:15 AM, Eric Blake wrote:
> On 1/28/21 11:21 AM, Vladimir Sementsov-Ogievskiy wrote:
>> 28.01.2021 20:13, Denis V. Lunev wrote:
>>> Original specification says that l1 table size if 64 * l1_size, which
>>> is obviously wrong. The size of the l1 entry is 64 _bits_, not bytes.
>>> Thus 64 is to be replaces with 8 as specification says about bytes.
>>>
>>> There is also minor tweak, field name is renamed from l1 to l1_table,
>>> which matches with the later text.
>>>
>>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>>> CC: Stefan Hajnoczi <stefanha@redhat.com>
>>> CC: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>
> I saw the subject "dirty bitmap", and assumed it would go through my
> dirty bitmap tree.  In reality, it's unrelated to the dirty bitmap code.
>  Would an improved subject line help?
hmm. Actually this is about "how the dirty bitmaps are stored in the
Parallels Image format". The section is called "dirty bitmap feature".

What I can propose? :)

"docs: fix mistake in Parallels Image "dirty bitmap" feature description"

Will this work for you?

Den

>>> ---
>>>   docs/interop/parallels.txt | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/docs/interop/parallels.txt b/docs/interop/parallels.txt
>>> index e9271eba5d..f15bf35bd1 100644
>>> --- a/docs/interop/parallels.txt
>>> +++ b/docs/interop/parallels.txt
>>> @@ -208,7 +208,7 @@ of its data area are:
>>>     28 - 31:    l1_size
>>>                 The number of entries in the L1 table of the bitmap.
>>>   -  variable:   l1 (64 * l1_size bytes)
>>> +  variable:   l1_table (8 * l1_size bytes)
>>>                 L1 offset table (in bytes)
>> I don't remember why this "(in bytes)" is here.. What in bytes? L1 table
>> size? But the described field is not L1 table size, but L1 table
>> itself.. It's not in bytes, it's just L1 table :)
>>
>> So, I'd also drop "(in bytes)" while being here. Or the whole line "L1
>> offset table (in bytes)" altogether.
>>
>>>     A dirty bitmap is stored using a one-level structure for the
>>> mapping to host
>>>
>>

Re: [PATCH 1/1] docs: fix mistake in dirty bitmap feature description

[Eric Blake]

On 2/2/21 4:50 PM, Denis V. Lunev wrote:
> On 2/3/21 1:15 AM, Eric Blake wrote:
>> On 1/28/21 11:21 AM, Vladimir Sementsov-Ogievskiy wrote:
>>> 28.01.2021 20:13, Denis V. Lunev wrote:
>>>> Original specification says that l1 table size if 64 * l1_size, which
>>>> is obviously wrong. The size of the l1 entry is 64 _bits_, not bytes.
>>>> Thus 64 is to be replaces with 8 as specification says about bytes.
>>>>
>>>> There is also minor tweak, field name is renamed from l1 to l1_table,
>>>> which matches with the later text.
>>>>
>>>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>>>> CC: Stefan Hajnoczi <stefanha@redhat.com>
>>>> CC: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>>
>> I saw the subject "dirty bitmap", and assumed it would go through my
>> dirty bitmap tree.  In reality, it's unrelated to the dirty bitmap code.
>>  Would an improved subject line help?
> hmm. Actually this is about "how the dirty bitmaps are stored in the
> Parallels Image format". The section is called "dirty bitmap feature".
> 
> What I can propose? :)
> 
> "docs: fix mistake in Parallels Image "dirty bitmap" feature description"
> 
> Will this work for you?

That feels a bit long; maybe just:

docs: fix Parallels Image "dirty bitmap" section


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

Re: [PATCH 1/1] docs: fix mistake in dirty bitmap feature description

[Denis V. Lunev]

On 2/3/21 2:08 AM, Eric Blake wrote:
> On 2/2/21 4:50 PM, Denis V. Lunev wrote:
>> On 2/3/21 1:15 AM, Eric Blake wrote:
>>> On 1/28/21 11:21 AM, Vladimir Sementsov-Ogievskiy wrote:
>>>> 28.01.2021 20:13, Denis V. Lunev wrote:
>>>>> Original specification says that l1 table size if 64 * l1_size, which
>>>>> is obviously wrong. The size of the l1 entry is 64 _bits_, not bytes.
>>>>> Thus 64 is to be replaces with 8 as specification says about bytes.
>>>>>
>>>>> There is also minor tweak, field name is renamed from l1 to l1_table,
>>>>> which matches with the later text.
>>>>>
>>>>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>>>>> CC: Stefan Hajnoczi <stefanha@redhat.com>
>>>>> CC: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>>> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>>>
>>> I saw the subject "dirty bitmap", and assumed it would go through my
>>> dirty bitmap tree.  In reality, it's unrelated to the dirty bitmap code.
>>>  Would an improved subject line help?
>> hmm. Actually this is about "how the dirty bitmaps are stored in the
>> Parallels Image format". The section is called "dirty bitmap feature".
>>
>> What I can propose? :)
>>
>> "docs: fix mistake in Parallels Image "dirty bitmap" feature description"
>>
>> Will this work for you?
> That feels a bit long; maybe just:
>
> docs: fix Parallels Image "dirty bitmap" section
>
>

looks great to me. Should I resend?

Den

Re: [PATCH 1/1] docs: fix mistake in dirty bitmap feature description

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1905 bytes --]

On Wed, Feb 03, 2021 at 01:29:03PM +0300, Denis V. Lunev wrote:
> On 2/3/21 2:08 AM, Eric Blake wrote:
> > On 2/2/21 4:50 PM, Denis V. Lunev wrote:
> >> On 2/3/21 1:15 AM, Eric Blake wrote:
> >>> On 1/28/21 11:21 AM, Vladimir Sementsov-Ogievskiy wrote:
> >>>> 28.01.2021 20:13, Denis V. Lunev wrote:
> >>>>> Original specification says that l1 table size if 64 * l1_size, which
> >>>>> is obviously wrong. The size of the l1 entry is 64 _bits_, not bytes.
> >>>>> Thus 64 is to be replaces with 8 as specification says about bytes.
> >>>>>
> >>>>> There is also minor tweak, field name is renamed from l1 to l1_table,
> >>>>> which matches with the later text.
> >>>>>
> >>>>> Signed-off-by: Denis V. Lunev <den@openvz.org>
> >>>>> CC: Stefan Hajnoczi <stefanha@redhat.com>
> >>>>> CC: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> >>>> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> >>>>
> >>> I saw the subject "dirty bitmap", and assumed it would go through my
> >>> dirty bitmap tree.  In reality, it's unrelated to the dirty bitmap code.
> >>>  Would an improved subject line help?
> >> hmm. Actually this is about "how the dirty bitmaps are stored in the
> >> Parallels Image format". The section is called "dirty bitmap feature".
> >>
> >> What I can propose? :)
> >>
> >> "docs: fix mistake in Parallels Image "dirty bitmap" feature description"
> >>
> >> Will this work for you?
> > That feels a bit long; maybe just:
> >
> > docs: fix Parallels Image "dirty bitmap" section
> >
> >
> 
> looks great to me. Should I resend?

It's okay. I have merged it, updated the commit message, and added a
note about the original commit message. The final commit has a
 Message-id: header so it's easy to find the original email thread.

Thanks, applied to my block tree:
https://gitlab.com/stefanha/qemu/commits/block

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]