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
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
