diff mbox series

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

Message ID 20210128171313.2210947-1-den@openvz.org (mailing list archive)
State New, archived
Headers show
Series [1/1] docs: fix mistake in dirty bitmap feature description | expand

Commit Message

Denis V. Lunev Jan. 28, 2021, 5:13 p.m. UTC
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(-)

Comments

Vladimir Sementsov-Ogievskiy Jan. 28, 2021, 5:21 p.m. UTC | #1
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
>
Eric Blake Feb. 2, 2021, 10:15 p.m. UTC | #2
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
>>
> 
>
Denis V. Lunev Feb. 2, 2021, 10:50 p.m. UTC | #3
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
>>>
>>
Eric Blake Feb. 2, 2021, 11:08 p.m. UTC | #4
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
Denis V. Lunev Feb. 3, 2021, 10:29 a.m. UTC | #5
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
Stefan Hajnoczi Feb. 3, 2021, 4:50 p.m. UTC | #6
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
diff mbox series

Patch

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