[libvirt] [PATCH] Add qcow2 cache configuration

[Liu Qing]

On Wed, Aug 30, 2017 at 11:12:02AM +0200, Martin Kletzander wrote:
> On Wed, Aug 30, 2017 at 04:07:16PM +0800, Liu Qing wrote:
> >Random write IOPS will drop dramatically if qcow2 l2 cache could not
> >cover the whole disk. This patch give libvirt user a chance to adjust
> >the qcow2 cache configuration.
> >
> 
> Thanks for the patch, but it has no documentation, no capability
> checking, no RNG schema adjustments and no tests.  Ideally the patch
> should be a series of patches, each introducing part of the
> functionality.  For example:
> 
>  [PATCH 1/3] conf, docs: Add support for bla bla bla
> 
>    Add stuff to docs/schemas/*.rng, docs/*.html.in, src/conf/*.c, add
>    parsing tests to tests/qemuxml2xmltest.c (and possibly
>    qemuxml2argvtest.c if there is some negative test that should fail
>    parsing).  Code needs to compile cleanly and tests need to pass after
>    this patch.  Documentation should cleanly state the reasoning and
>    rules for the possible values so that users know *if* and *why* they
>    need to set this up and to *what* values.  It is also good to think
>    about why QEMU doesn't use such values as default and whether or not
>    (or why/not) libvirt should default to such values without making
>    the user do so.
> 
>  [PATCH 2/3] qemu: Add capability checking for bla bla bla
> 
>    Here you would check that we properly probe qemu for the possibility
>    of setting such tunables in src/qemu/qemu_capabilities.[hc].  Code
>    needs to compile cleanly and tests need to pass after this patch.
> 
>  [PATCH 3/3] qemu: Add support for bla bla bla
> 
>    Here you would check if the emulator has the required capabilities,
>    format them on the command line and add positive tests to
>    tests/qemuxml2argvtest.c.  Code needs to compile cleanly and tests
>    need to pass after this patch.
> 
> In rare cases where the functionality and required tests are minimal,
> patches [2/3] and [3/3] could be merged together, but they can always be
> separate, IMHO.
> 
> All of this ^^ is only about the way the patch is supposed to be sent.
> Whether or not it makes sense to expose such tunables is left as an
> exercise to all readers (and possibly a discussion on v2 of this patch).
Thanks for the guide, I will take time to complete these.
> 
> Have a nice day,
> Martin