Re: [Qemu-devel] [PATCH v5 2/4 for-3.0] qcow2: Options' documentation fixes
How about: "default: 4 times the cluster size; or if cache-size is specified, the part of it which is not used for the L2 cache" Even simpler, easier to understand and more accurate. I like it. Thanks! Kevin
Re: [Qemu-devel] [PATCH v5 2/4 for-3.0] qcow2: Options' documentation fixes
Am 26.07.2018 um 16:27 hat Leonid Bloch geschrieben: > On 07/26/2018 01:02 PM, Kevin Wolf wrote: > > Am 25.07.2018 um 16:27 hat Leonid Bloch geschrieben: > > > Signed-off-by: Leonid Bloch > > > --- > > > docs/qcow2-cache.txt | 3 +++ > > > qemu-options.hx | 10 ++ > > > 2 files changed, 9 insertions(+), 4 deletions(-) > > > > > > diff --git a/docs/qcow2-cache.txt b/docs/qcow2-cache.txt > > > index 8a09a5cc5f..3673f2be0e 100644 > > > --- a/docs/qcow2-cache.txt > > > +++ b/docs/qcow2-cache.txt > > > @@ -130,6 +130,9 @@ There are a few things that need to be taken into > > > account: > > > memory as possible to the L2 cache before increasing the refcount > > > cache size. > > > +- At most two of "l2-cache-size", "refcount-cache-size", and "cache-size" > > > + can be set simultaneously. > > > > The indentation is off here, the other list items have one space more. > > > > > Unlike L2 tables, refcount blocks are not used during normal I/O but > > > only during allocations and internal snapshots. In most cases they are > > > accessed sequentially (even during random guest I/O) so increasing the > > > diff --git a/qemu-options.hx b/qemu-options.hx > > > index b1bf0f485f..13ece21cb6 100644 > > > --- a/qemu-options.hx > > > +++ b/qemu-options.hx > > > @@ -752,15 +752,17 @@ image file) > > > @item cache-size > > > The maximum total size of the L2 table and refcount block caches in > > > bytes > > > -(default: 1048576 bytes or 8 clusters, whichever is larger) > > > > I think it would be good to still say something about the default. > > Maybe something like "default: the sum of l2-cache-size and > > refcount-cache-size"? > > Yes, that sounds good! > > > > > > @item l2-cache-size > > > -The maximum size of the L2 table cache in bytes > > > -(default: 4/5 of the total cache size) > > > +The maximum size of the L2 table cache. > > > > Why did you remove "in bytes" and add a period which the other options > > don't have? I prefer the old version of this line. > > I removed "in bytes" because it also accepts k, M, G, ... but on a second > thought, these are amounts of bytes as well, so changing back to the old > version. > > > > +(default: if cache-size is not defined - 1048576 bytes or 8 clusters, > > > whichever > > > +is larger; otherwise, as large as possible or needed within the > > > cache-size, > > > +while permitting the requested or the minimal refcount cache size) > > > @item refcount-cache-size > > > The maximum size of the refcount block cache in bytes > > > -(default: 1/5 of the total cache size) > > > +(default: 4 times the cluster size, or any portion of the cache-size, if > > > it is > > > +specified and large enough, left over after allocating the full L2 cache) > > > > I found the second part hard to understand. Maybe "4 times the cluster > > size; or if both cache-size and l2-cache-size are given, the part of > > the cache-size that is not used for the L2 cache yet."? > > That is not accurate. Because even if l2-cache-size is not given and > cache-size is large enough to accommodate enough L2 cache to cover the > entire image, plus the minimal amount of refcount cache with room to spare - > refcount cache will use all the rest of the cache-size. Oh, you're right! I wasn't aware that we consider the image size there. > How about: > > "default: 4 times the cluster size; or if cache-size is specified, the part > of it which is not used for the L2 cache" Even simpler, easier to understand and more accurate. I like it. Kevin
Re: [Qemu-devel] [PATCH v5 2/4 for-3.0] qcow2: Options' documentation fixes
On 07/26/2018 05:20 PM, Eric Blake wrote: On 07/26/2018 05:02 AM, Kevin Wolf wrote: Am 25.07.2018 um 16:27 hat Leonid Bloch geschrieben: Signed-off-by: Leonid Bloch --- docs/qcow2-cache.txt | 3 +++ qemu-options.hx | 10 ++ 2 files changed, 9 insertions(+), 4 deletions(-) +++ b/qemu-options.hx @@ -752,15 +752,17 @@ image file) @item cache-size The maximum total size of the L2 table and refcount block caches in bytes -(default: 1048576 bytes or 8 clusters, whichever is larger) I think it would be good to still say something about the default. Maybe something like "default: the sum of l2-cache-size and refcount-cache-size"? Except what happens if you specify only one of l2-cache-size or refcount-cache-size? Is the defaulted cache-size then just that one size you specified (and the other cache ignored), or is the total cache size still defaulted to the 1M/8-cluster size (assuming its larger than the other size specified)? I think that the meaning here is the sum of the actual l2-cache-size and refcount-cache-size. Not the specified options, but the actual caches. Leonid.
Re: [Qemu-devel] [PATCH v5 2/4 for-3.0] qcow2: Options' documentation fixes
On 07/26/2018 01:02 PM, Kevin Wolf wrote: Am 25.07.2018 um 16:27 hat Leonid Bloch geschrieben: Signed-off-by: Leonid Bloch --- docs/qcow2-cache.txt | 3 +++ qemu-options.hx | 10 ++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/docs/qcow2-cache.txt b/docs/qcow2-cache.txt index 8a09a5cc5f..3673f2be0e 100644 --- a/docs/qcow2-cache.txt +++ b/docs/qcow2-cache.txt @@ -130,6 +130,9 @@ There are a few things that need to be taken into account: memory as possible to the L2 cache before increasing the refcount cache size. +- At most two of "l2-cache-size", "refcount-cache-size", and "cache-size" + can be set simultaneously. The indentation is off here, the other list items have one space more. Unlike L2 tables, refcount blocks are not used during normal I/O but only during allocations and internal snapshots. In most cases they are accessed sequentially (even during random guest I/O) so increasing the diff --git a/qemu-options.hx b/qemu-options.hx index b1bf0f485f..13ece21cb6 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -752,15 +752,17 @@ image file) @item cache-size The maximum total size of the L2 table and refcount block caches in bytes -(default: 1048576 bytes or 8 clusters, whichever is larger) I think it would be good to still say something about the default. Maybe something like "default: the sum of l2-cache-size and refcount-cache-size"? Yes, that sounds good! @item l2-cache-size -The maximum size of the L2 table cache in bytes -(default: 4/5 of the total cache size) +The maximum size of the L2 table cache. Why did you remove "in bytes" and add a period which the other options don't have? I prefer the old version of this line. I removed "in bytes" because it also accepts k, M, G, ... but on a second thought, these are amounts of bytes as well, so changing back to the old version. +(default: if cache-size is not defined - 1048576 bytes or 8 clusters, whichever +is larger; otherwise, as large as possible or needed within the cache-size, +while permitting the requested or the minimal refcount cache size) @item refcount-cache-size The maximum size of the refcount block cache in bytes -(default: 1/5 of the total cache size) +(default: 4 times the cluster size, or any portion of the cache-size, if it is +specified and large enough, left over after allocating the full L2 cache) I found the second part hard to understand. Maybe "4 times the cluster size; or if both cache-size and l2-cache-size are given, the part of the cache-size that is not used for the L2 cache yet."? That is not accurate. Because even if l2-cache-size is not given and cache-size is large enough to accommodate enough L2 cache to cover the entire image, plus the minimal amount of refcount cache with room to spare - refcount cache will use all the rest of the cache-size. How about: "default: 4 times the cluster size; or if cache-size is specified, the part of it which is not used for the L2 cache" Leonid. Kevin
Re: [Qemu-devel] [PATCH v5 2/4 for-3.0] qcow2: Options' documentation fixes
On 07/26/2018 05:02 AM, Kevin Wolf wrote: Am 25.07.2018 um 16:27 hat Leonid Bloch geschrieben: Signed-off-by: Leonid Bloch --- docs/qcow2-cache.txt | 3 +++ qemu-options.hx | 10 ++ 2 files changed, 9 insertions(+), 4 deletions(-) +++ b/qemu-options.hx @@ -752,15 +752,17 @@ image file) @item cache-size The maximum total size of the L2 table and refcount block caches in bytes -(default: 1048576 bytes or 8 clusters, whichever is larger) I think it would be good to still say something about the default. Maybe something like "default: the sum of l2-cache-size and refcount-cache-size"? Except what happens if you specify only one of l2-cache-size or refcount-cache-size? Is the defaulted cache-size then just that one size you specified (and the other cache ignored), or is the total cache size still defaulted to the 1M/8-cluster size (assuming its larger than the other size specified)? @item l2-cache-size -The maximum size of the L2 table cache in bytes -(default: 4/5 of the total cache size) +The maximum size of the L2 table cache. Why did you remove "in bytes" and add a period which the other options don't have? I prefer the old version of this line. +(default: if cache-size is not defined - 1048576 bytes or 8 clusters, whichever +is larger; otherwise, as large as possible or needed within the cache-size, +while permitting the requested or the minimal refcount cache size) @item refcount-cache-size The maximum size of the refcount block cache in bytes -(default: 1/5 of the total cache size) +(default: 4 times the cluster size, or any portion of the cache-size, if it is +specified and large enough, left over after allocating the full L2 cache) I found the second part hard to understand. Maybe "4 times the cluster size; or if both cache-size and l2-cache-size are given, the part of the cache-size that is not used for the L2 cache yet."? Kevin -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH v5 2/4 for-3.0] qcow2: Options' documentation fixes
Am 25.07.2018 um 16:27 hat Leonid Bloch geschrieben: > Signed-off-by: Leonid Bloch > --- > docs/qcow2-cache.txt | 3 +++ > qemu-options.hx | 10 ++ > 2 files changed, 9 insertions(+), 4 deletions(-) > > diff --git a/docs/qcow2-cache.txt b/docs/qcow2-cache.txt > index 8a09a5cc5f..3673f2be0e 100644 > --- a/docs/qcow2-cache.txt > +++ b/docs/qcow2-cache.txt > @@ -130,6 +130,9 @@ There are a few things that need to be taken into account: > memory as possible to the L2 cache before increasing the refcount > cache size. > > +- At most two of "l2-cache-size", "refcount-cache-size", and "cache-size" > + can be set simultaneously. The indentation is off here, the other list items have one space more. > Unlike L2 tables, refcount blocks are not used during normal I/O but > only during allocations and internal snapshots. In most cases they are > accessed sequentially (even during random guest I/O) so increasing the > diff --git a/qemu-options.hx b/qemu-options.hx > index b1bf0f485f..13ece21cb6 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -752,15 +752,17 @@ image file) > > @item cache-size > The maximum total size of the L2 table and refcount block caches in bytes > -(default: 1048576 bytes or 8 clusters, whichever is larger) I think it would be good to still say something about the default. Maybe something like "default: the sum of l2-cache-size and refcount-cache-size"? > @item l2-cache-size > -The maximum size of the L2 table cache in bytes > -(default: 4/5 of the total cache size) > +The maximum size of the L2 table cache. Why did you remove "in bytes" and add a period which the other options don't have? I prefer the old version of this line. > +(default: if cache-size is not defined - 1048576 bytes or 8 clusters, > whichever > +is larger; otherwise, as large as possible or needed within the cache-size, > +while permitting the requested or the minimal refcount cache size) > > @item refcount-cache-size > The maximum size of the refcount block cache in bytes > -(default: 1/5 of the total cache size) > +(default: 4 times the cluster size, or any portion of the cache-size, if it > is > +specified and large enough, left over after allocating the full L2 cache) I found the second part hard to understand. Maybe "4 times the cluster size; or if both cache-size and l2-cache-size are given, the part of the cache-size that is not used for the L2 cache yet."? Kevin
[Qemu-devel] [PATCH v5 2/4 for-3.0] qcow2: Options' documentation fixes
Signed-off-by: Leonid Bloch --- docs/qcow2-cache.txt | 3 +++ qemu-options.hx | 10 ++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/docs/qcow2-cache.txt b/docs/qcow2-cache.txt index 8a09a5cc5f..3673f2be0e 100644 --- a/docs/qcow2-cache.txt +++ b/docs/qcow2-cache.txt @@ -130,6 +130,9 @@ There are a few things that need to be taken into account: memory as possible to the L2 cache before increasing the refcount cache size. +- At most two of "l2-cache-size", "refcount-cache-size", and "cache-size" + can be set simultaneously. + Unlike L2 tables, refcount blocks are not used during normal I/O but only during allocations and internal snapshots. In most cases they are accessed sequentially (even during random guest I/O) so increasing the diff --git a/qemu-options.hx b/qemu-options.hx index b1bf0f485f..13ece21cb6 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -752,15 +752,17 @@ image file) @item cache-size The maximum total size of the L2 table and refcount block caches in bytes -(default: 1048576 bytes or 8 clusters, whichever is larger) @item l2-cache-size -The maximum size of the L2 table cache in bytes -(default: 4/5 of the total cache size) +The maximum size of the L2 table cache. +(default: if cache-size is not defined - 1048576 bytes or 8 clusters, whichever +is larger; otherwise, as large as possible or needed within the cache-size, +while permitting the requested or the minimal refcount cache size) @item refcount-cache-size The maximum size of the refcount block cache in bytes -(default: 1/5 of the total cache size) +(default: 4 times the cluster size, or any portion of the cache-size, if it is +specified and large enough, left over after allocating the full L2 cache) @item cache-clean-interval Clean unused entries in the L2 and refcount caches. The interval is in seconds. -- 2.14.1
