Re: [PATCH 1/1] docs: fix mistake in dirty bitmap feature description
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 > > CC: Stefan Hajnoczi > > CC: Vladimir Sementsov-Ogievskiy > Reviewed-by: Vladimir Sementsov-Ogievskiy > > >>> 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 signature.asc Description: PGP signature
Re: [PATCH 1/1] docs: fix mistake in dirty bitmap feature description
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 > CC: Stefan Hajnoczi > CC: Vladimir Sementsov-Ogievskiy Reviewed-by: Vladimir Sementsov-Ogievskiy >>> 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
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 CC: Stefan Hajnoczi CC: Vladimir Sementsov-Ogievskiy >>> Reviewed-by: Vladimir Sementsov-Ogievskiy >>> >> 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
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 >>> CC: Stefan Hajnoczi >>> CC: Vladimir Sementsov-Ogievskiy >> Reviewed-by: Vladimir Sementsov-Ogievskiy >> > 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
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 >> CC: Stefan Hajnoczi >> CC: Vladimir Sementsov-Ogievskiy > > Reviewed-by: Vladimir Sementsov-Ogievskiy > 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
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 CC: Stefan Hajnoczi CC: Vladimir Sementsov-Ogievskiy Reviewed-by: Vladimir Sementsov-Ogievskiy --- 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
