[libvirt] [PATCH 0/6] qemu: Add support for 'write-cache' disk frontend argument (blockdev-add saga)
3:13 a.m.
'write-cache' is one of the booleans the 'cache' attribute of -drive
translates to. Since we are getting rid of -drive this needs to be
implemented in a different way.
Peter Krempa (6):
tests: utils: Allow parsing test capability file without virCaps
tests: qemuxm2argv: Add infrastructure for testing with real qemuCaps
qemu: domain: Add helper for translating disk cachemode to qemu flags
qemu: caps: Add capability for 'write-cache' parameter of disk
frontends
qemu: Format 'write-cache' parameter for disk frontends
tests: qemuxml2argv: Test formatting of 'write-cache' parameter
src/qemu/qemu_capabilities.c | 5 ++
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_command.c | 27 ++++++++
src/qemu/qemu_domain.c | 75 ++++++++++++++++++++++
src/qemu/qemu_domain.h | 6 ++
tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 +
tests/qemuxml2argvdata/disk-drive-write-cache.args | 45 +++++++++++++
tests/qemuxml2argvdata/disk-drive-write-cache.xml | 45 +++++++++++++
tests/qemuxml2argvtest.c | 32 +++++++++
tests/testutilsqemu.c | 21 ++++--
tests/testutilsqemu.h | 3 +
26 files changed, 270 insertions(+), 6 deletions(-)
create mode 100644 tests/qemuxml2argvdata/disk-drive-write-cache.args
create mode 100644 tests/qemuxml2argvdata/disk-drive-write-cache.xml
--
2.16.2
3:13 a.m.
New subject: [libvirt] [PATCH 1/6] tests: utils: Allow parsing test capability file without virCaps
virCaps was used only to propagate the host architecture, so the
function can be extracted in a way which does not require it.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
tests/testutilsqemu.c | 21 +++++++++++++++------
tests/testutilsqemu.h | 3 +++
2 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index f8182033fc..9671a46f12 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -486,16 +486,13 @@ qemuTestSetHostCPU(virCapsPtr caps,
virQEMUCapsPtr
-qemuTestParseCapabilities(virCapsPtr caps,
- const char *capsFile)
+qemuTestParseCapabilitiesArch(virArch arch,
+ const char *capsFile)
{
virQEMUCapsPtr qemuCaps = NULL;
- if (!caps)
- return NULL;
-
if (!(qemuCaps = virQEMUCapsNew()) ||
- virQEMUCapsLoadCache(caps->host.arch, qemuCaps, capsFile) < 0)
+ virQEMUCapsLoadCache(arch, qemuCaps, capsFile) < 0)
goto error;
return qemuCaps;
@@ -505,6 +502,18 @@ qemuTestParseCapabilities(virCapsPtr caps,
return NULL;
}
+
+virQEMUCapsPtr
+qemuTestParseCapabilities(virCapsPtr caps,
+ const char *capsFile)
+{
+ if (!caps)
+ return NULL;
+
+ return qemuTestParseCapabilitiesArch(caps->host.arch, capsFile);
+}
+
+
void qemuTestDriverFree(virQEMUDriver *driver)
{
virMutexDestroy(&driver->lock);
diff --git a/tests/testutilsqemu.h b/tests/testutilsqemu.h
index f29c6e5d62..7ae8324933 100644
--- a/tests/testutilsqemu.h
+++ b/tests/testutilsqemu.h
@@ -16,6 +16,9 @@ enum {
virCapsPtr testQemuCapsInit(void);
virDomainXMLOptionPtr testQemuXMLConfInit(void);
+
+virQEMUCapsPtr qemuTestParseCapabilitiesArch(virArch arch,
+ const char *capsFile);
virQEMUCapsPtr qemuTestParseCapabilities(virCapsPtr caps,
const char *capsFile);
--
2.16.2
10:14 a.m.
New subject: [libvirt] [PATCH 1/6] tests: utils: Allow parsing test capability file without virCaps
On 04/04/2018 04:13 AM, Peter Krempa wrote:
virCaps was used only to propagate the host architecture, so the
function can be extracted in a way which does not require it.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
tests/testutilsqemu.c | 21 +++++++++++++++------
tests/testutilsqemu.h | 3 +++
2 files changed, 18 insertions(+), 6 deletions(-)
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
10:52 a.m.
New subject: [libvirt] [PATCH 1/6] tests: utils: Allow parsing test capability file without virCaps
On Wed, Apr 04, 2018 at 10:13:54AM +0200, Peter Krempa wrote:
virCaps was used only to propagate the host architecture, so the
function can be extracted in a way which does not require it.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
tests/testutilsqemu.c | 21 +++++++++++++++------
tests/testutilsqemu.h | 3 +++
2 files changed, 18 insertions(+), 6 deletions(-)
ACK
Jano
3:13 a.m.
New subject: [libvirt] [PATCH 2/6] tests: qemuxm2argv: Add infrastructure for testing with real qemuCaps
Allow testing of XML->argv conversion with using a real capability map
as used in the qemucapabilitiestest. This allows specifying the required
qemu version with the test rather than having to enumerate all the
required capabilities.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
tests/qemuxml2argvtest.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index d79913dd0a..c540ce2f50 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -654,6 +654,37 @@ mymain(void)
if (VIR_STRDUP_QUIET(driver.config->memoryBackingDir,
"/var/lib/libvirt/qemu/ram") < 0)
return EXIT_FAILURE;
+
+# define DO_TEST_CAPS_ARCH(name, migrateFrom, flags, \
+ parseFlags, gic, arch, ver) \
+ do { \
+ static struct testInfo info = { \
+ name, NULL, migrateFrom, migrateFrom ? 7 : -1, (flags), parseFlags, \
+ false, NULL \
+ }; \
+ info.skipLegacyCPUs = skipLegacyCPUs; \
+ if (testInitQEMUCaps(&info, gic) < 0) \
+ return EXIT_FAILURE; \
+ if (!(info.qemuCaps = qemuTestParseCapabilitiesArch(virArchFromString(arch), \
+ abs_srcdir
"/qemucapabilitiesdata/caps_" \
+ ver "." arch ".xml"))) \
+ return EXIT_FAILURE; \
+ if (virTestRun("QEMU XML-2-ARGV " name, \
+ testCompareXMLToArgv, &info) < 0) \
+ ret = -1; \
+ if (info.vm && virTestRun("QEMU XML-2-startup-XML " name, \
+ testCompareXMLToStartupXML, &info) < 0) \
+ ret = -1; \
+ virObjectUnref(info.qemuCaps); \
+ virObjectUnref(info.vm); \
+ } while (0)
+
+# define DO_TEST_CAPS_FULL(name, flags, parseFlags, ver) \
+ DO_TEST_CAPS_ARCH(name, NULL, flags, parseFlags, GIC_NONE, "x86_64", ver)
+
+# define DO_TEST_CAPS(name, ver) \
+ DO_TEST_CAPS_FULL(name, 0, 0, ver)
+
# define DO_TEST_FULL(name, migrateFrom, migrateFd, flags, \
parseFlags, gic, ...) \
do { \
--
2.16.2
10:16 a.m.
New subject: [libvirt] [PATCH 2/6] tests: qemuxm2argv: Add infrastructure for testing with real qemuCaps
On 04/04/2018 04:13 AM, Peter Krempa wrote:
Allow testing of XML->argv conversion with using a real capability
map
as used in the qemucapabilitiestest. This allows specifying the required
qemu version with the test rather than having to enumerate all the
required capabilities.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
tests/qemuxml2argvtest.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index d79913dd0a..c540ce2f50 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -654,6 +654,37 @@ mymain(void)
if (VIR_STRDUP_QUIET(driver.config->memoryBackingDir,
"/var/lib/libvirt/qemu/ram") < 0)
return EXIT_FAILURE;
+
+# define DO_TEST_CAPS_ARCH(name, migrateFrom, flags, \
+ parseFlags, gic, arch, ver) \
+ do { \
+ static struct testInfo info = { \
+ name, NULL, migrateFrom, migrateFrom ? 7 : -1, (flags), parseFlags, \
+ false, NULL \
+ }; \
+ info.skipLegacyCPUs = skipLegacyCPUs; \
+ if (testInitQEMUCaps(&info, gic) < 0) \
+ return EXIT_FAILURE; \
+ if (!(info.qemuCaps = qemuTestParseCapabilitiesArch(virArchFromString(arch), \
+ abs_srcdir
"/qemucapabilitiesdata/caps_" \
+ ver "." arch ".xml"))) \
+ return EXIT_FAILURE; \
+ if (virTestRun("QEMU XML-2-ARGV " name, \
+ testCompareXMLToArgv, &info) < 0) \
+ ret = -1; \
+ if (info.vm && virTestRun("QEMU XML-2-startup-XML " name, \
+ testCompareXMLToStartupXML, &info) < 0) \
+ ret = -1; \
+ virObjectUnref(info.qemuCaps); \
+ virObjectUnref(info.vm); \
+ } while (0)
+
+# define DO_TEST_CAPS_FULL(name, flags, parseFlags, ver) \
+ DO_TEST_CAPS_ARCH(name, NULL, flags, parseFlags, GIC_NONE, "x86_64", ver)
Assumes x64_64...
+
+# define DO_TEST_CAPS(name, ver) \
+ DO_TEST_CAPS_FULL(name, 0, 0, ver)
+
# define DO_TEST_FULL(name, migrateFrom, migrateFd, flags, \
parseFlags, gic, ...) \
do { \
Do you expect to see wide spread and future use of this model as opposed
to the existing DO_TEST model? Shouldn't at least some of the existing
tests be converted, then?
Perhaps the macro(s) should be DO_TEST_VERS[ION] or DO_TEST_<ARCH>_VERS
instead since DO_TEST runs the test with specific capabilities and this
new model runs the test with a specific version for a specific platform.
How do or should we enforce that when adding a new test from this point
forward that the DO_TEST_CAPS model is chosen over DO_TEST? I suppose
there is still value in DO_TEST since one can pick and choose which CAP
is needed - although I'll freely admit that's usually a cut-n-paste from
some other test and change the name type activity.
I think I have too many questions for an R-b at this point.
John
8:29 a.m.
New subject: [libvirt] [PATCH 2/6] tests: qemuxm2argv: Add infrastructure for testing with real qemuCaps
On Wed, Apr 11, 2018 at 11:16:27AM -0400, John Ferlan wrote:
On 04/04/2018 04:13 AM, Peter Krempa wrote:
> Allow testing of XML->argv conversion with using a real capability map
> as used in the qemucapabilitiestest. This allows specifying the required
> qemu version with the test rather than having to enumerate all the
> required capabilities.
>
> Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
> ---
> tests/qemuxml2argvtest.c | 31 +++++++++++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index d79913dd0a..c540ce2f50 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -654,6 +654,37 @@ mymain(void)
> if (VIR_STRDUP_QUIET(driver.config->memoryBackingDir,
"/var/lib/libvirt/qemu/ram") < 0)
> return EXIT_FAILURE;
>
> +
> +# define DO_TEST_CAPS_ARCH(name, migrateFrom, flags, \
> + parseFlags, gic, arch, ver) \
> + do { \
> + static struct testInfo info = { \
> + name, NULL, migrateFrom, migrateFrom ? 7 : -1, (flags), parseFlags, \
> + false, NULL \
> + }; \
> + info.skipLegacyCPUs = skipLegacyCPUs; \
> + if (testInitQEMUCaps(&info, gic) < 0) \
> + return EXIT_FAILURE; \
> + if (!(info.qemuCaps = qemuTestParseCapabilitiesArch(virArchFromString(arch),
\
> + abs_srcdir
"/qemucapabilitiesdata/caps_" \
> + ver "." arch ".xml")))
\
> + return EXIT_FAILURE; \
> + if (virTestRun("QEMU XML-2-ARGV " name, \
> + testCompareXMLToArgv, &info) < 0) \
> + ret = -1; \
> + if (info.vm && virTestRun("QEMU XML-2-startup-XML " name,
\
> + testCompareXMLToStartupXML, &info) < 0) \
> + ret = -1; \
> + virObjectUnref(info.qemuCaps); \
> + virObjectUnref(info.vm); \
> + } while (0)
> +
> +# define DO_TEST_CAPS_FULL(name, flags, parseFlags, ver) \
> + DO_TEST_CAPS_ARCH(name, NULL, flags, parseFlags, GIC_NONE, "x86_64",
ver)
Assumes x64_64...
> +
> +# define DO_TEST_CAPS(name, ver) \
> + DO_TEST_CAPS_FULL(name, 0, 0, ver)
> +
> # define DO_TEST_FULL(name, migrateFrom, migrateFd, flags, \
> parseFlags, gic, ...) \
> do { \
>
Do you expect to see wide spread and future use of this model as opposed
to the existing DO_TEST model? Shouldn't at least some of the existing
tests be converted, then?
Yes to both.
Perhaps the macro(s) should be DO_TEST_VERS[ION] or DO_TEST_<ARCH>_VERS
instead since DO_TEST runs the test with specific capabilities and this
new model runs the test with a specific version for a specific platform.
How do or should we enforce that when adding a new test from this point
forward that the DO_TEST_CAPS model is chosen over DO_TEST? I suppose
there is still value in DO_TEST since one can pick and choose which CAP
is needed - although I'll freely admit that's usually a cut-n-paste from
some other test and change the name type activity.
The problem with that approach is that we're testing against
capability combinations that might never occur in the wild.
I actually remember fixing some bug that was supposed to be covered by
a unit test, but it was missing the important capability.
Enforcement by review ought to be enough for everybody.
Though there still might be features that you can compile out even from
QEMU versions that otherwise have the capability, but we don't add
negative tests for that often.
Jano
I think I have too many questions for an R-b at this point.
John
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
8:27 a.m.
New subject: [libvirt] [PATCH 2/6] tests: qemuxm2argv: Add infrastructure for testing with real qemuCaps
On Sun, Apr 15, 2018 at 15:29:23 +0200, Ján Tomko wrote:
On Wed, Apr 11, 2018 at 11:16:27AM -0400, John Ferlan wrote:
>
>
> On 04/04/2018 04:13 AM, Peter Krempa wrote:
> > Allow testing of XML->argv conversion with using a real capability map
> > as used in the qemucapabilitiestest. This allows specifying the required
> > qemu version with the test rather than having to enumerate all the
> > required capabilities.
> >
> > Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
> > ---
> > tests/qemuxml2argvtest.c | 31 +++++++++++++++++++++++++++++++
> > 1 file changed, 31 insertions(+)
> Perhaps the macro(s) should be DO_TEST_VERS[ION] or
DO_TEST_<ARCH>_VERS
> instead since DO_TEST runs the test with specific capabilities and this
> new model runs the test with a specific version for a specific platform.
>
> How do or should we enforce that when adding a new test from this point
> forward that the DO_TEST_CAPS model is chosen over DO_TEST? I suppose
> there is still value in DO_TEST since one can pick and choose which CAP
> is needed - although I'll freely admit that's usually a cut-n-paste from
> some other test and change the name type activity.
The problem with that approach is that we're testing against
capability combinations that might never occur in the wild.
I actually remember fixing some bug that was supposed to be covered by
a unit test, but it was missing the important capability.
Actually a very good example is the recent series of adding the
'discard-data' attribute to memory devices. Since the new feature is
decided with capability bits, all existing tests were still passing, but
the attribute would be added to NVDIMM memory devices which would delete
the contents.
I think we also need to add a way to take the latest caps present in the
repo to be used with the bulk of tests rather than binding it to a
specific version.
That way we can see whether the command line is stable when we add a
capability based test.
If the command line will change, the test case then can be split for the
specific version of qemu capabilities, so that the contemporary test
case still captures the new case, but the old behaviour is kept too.
7:48 a.m.
New subject: [libvirt] [PATCH 2/6] tests: qemuxm2argv: Add infrastructure for testing with real qemuCaps
On Wed, Apr 11, 2018 at 11:16:27 -0400, John Ferlan wrote:
On 04/04/2018 04:13 AM, Peter Krempa wrote:
> Allow testing of XML->argv conversion with using a real capability map
> as used in the qemucapabilitiestest. This allows specifying the required
> qemu version with the test rather than having to enumerate all the
> required capabilities.
>
> Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
> ---
> tests/qemuxml2argvtest.c | 31 +++++++++++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index d79913dd0a..c540ce2f50 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -654,6 +654,37 @@ mymain(void)
> if (VIR_STRDUP_QUIET(driver.config->memoryBackingDir,
"/var/lib/libvirt/qemu/ram") < 0)
> return EXIT_FAILURE;
>
> +
> +# define DO_TEST_CAPS_ARCH(name, migrateFrom, flags, \
> + parseFlags, gic, arch, ver) \
> + do { \
> + static struct testInfo info = { \
> + name, NULL, migrateFrom, migrateFrom ? 7 : -1, (flags), parseFlags, \
> + false, NULL \
> + }; \
> + info.skipLegacyCPUs = skipLegacyCPUs; \
> + if (testInitQEMUCaps(&info, gic) < 0) \
> + return EXIT_FAILURE; \
> + if (!(info.qemuCaps =
qemuTestParseCapabilitiesArch(virArchFromString(arch), \
> + abs_srcdir
"/qemucapabilitiesdata/caps_" \
> + ver "." arch
".xml"))) \
> + return EXIT_FAILURE; \
> + if (virTestRun("QEMU XML-2-ARGV " name, \
> + testCompareXMLToArgv, &info) < 0) \
> + ret = -1; \
> + if (info.vm && virTestRun("QEMU XML-2-startup-XML " name,
\
> + testCompareXMLToStartupXML, &info) < 0) \
> + ret = -1; \
> + virObjectUnref(info.qemuCaps); \
> + virObjectUnref(info.vm); \
> + } while (0)
> +
> +# define DO_TEST_CAPS_FULL(name, flags, parseFlags, ver) \
> + DO_TEST_CAPS_ARCH(name, NULL, flags, parseFlags, GIC_NONE, "x86_64",
ver)
Assumes x64_64...
Yes. Intentionally. Hipster-arches can introduce their own helpers.
> +
> +# define DO_TEST_CAPS(name, ver) \
> + DO_TEST_CAPS_FULL(name, 0, 0, ver)
> +
> # define DO_TEST_FULL(name, migrateFrom, migrateFd, flags, \
> parseFlags, gic, ...) \
> do { \
>
Do you expect to see wide spread and future use of this model as opposed
to the existing DO_TEST model? Shouldn't at least some of the existing
tests be converted, then?
I hope so, actually for both cases.
Perhaps the macro(s) should be DO_TEST_VERS[ION] or
DO_TEST_<ARCH>_VERS
instead since DO_TEST runs the test with specific capabilities and this
new model runs the test with a specific version for a specific platform.
Hmm, yes. I can rename it to DO_TEST_VERSION. The default will be for
x86. As said, other arches can add their own macros.
How do or should we enforce that when adding a new test from this
point
forward that the DO_TEST_CAPS model is chosen over DO_TEST? I suppose
I think reviewers should ask for using the new test approach for any
positive cases, so that we don't test something that is not possible in
the wild.
In cases when it's required to do so (negative tests, or weird config
tests) use of DO_TEST is justified.
there is still value in DO_TEST since one can pick and choose which
CAP
is needed - although I'll freely admit that's usually a cut-n-paste from
some other test and change the name type activity.
I think I have too many questions for an R-b at this point.
John
10:56 a.m.
New subject: [libvirt] [PATCH 2/6] tests: qemuxm2argv: Add infrastructure for testing with real qemuCaps
s/xm/xml/ in the summary
On Wed, Apr 04, 2018 at 10:13:55AM +0200, Peter Krempa wrote:
Allow testing of XML->argv conversion with using a real capability
map
as used in the qemucapabilitiestest. This allows specifying the required
qemu version with the test rather than having to enumerate all the
required capabilities.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
tests/qemuxml2argvtest.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
ACK
Jano
3:13 a.m.
New subject: [libvirt] [PATCH 3/6] qemu: domain: Add helper for translating disk cachemode to qemu flags
Add helper which will map values of disk cache mode to the flags which
are accepted by various parts of the qemu block layer.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_domain.h | 6 ++++
2 files changed, 81 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 9d1c33b54a..20333c9568 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -11920,6 +11920,81 @@ qemuDomainPrepareDiskSource(virDomainDiskDefPtr disk,
}
+/**
+ * qemuDomainDiskCachemodeFlags:
+ *
+ * Converts disk cachemode to the cache mode options for qemu. Returns -1 for
+ * invalid @cachemode values and fills the flags and returns 0 on success.
+ * Flags may be NULL.
+ */
+int
+qemuDomainDiskCachemodeFlags(int cachemode,
+ bool *writeback,
+ bool *direct,
+ bool *noflush)
+{
+ bool dummy;
+
+ if (!writeback)
+ writeback = &dummy;
+
+ if (!direct)
+ direct = &dummy;
+
+ if (!noflush)
+ noflush = &dummy;
+
+ /* Mapping of cache modes to the attributes according to qemu-options.hx
+ * │ cache.writeback cache.direct cache.no-flush
+ * ─────────────┼─────────────────────────────────────────────────
+ * writeback │ true false false
+ * none │ true true false
+ * writethrough │ false false false
+ * directsync │ false true false
+ * unsafe │ true false true
+ */
+ switch ((virDomainDiskCache) cachemode) {
+ case VIR_DOMAIN_DISK_CACHE_DISABLE: /* 'none' */
+ *writeback = true;
+ *direct = true;
+ *noflush = false;
+ break;
+
+ case VIR_DOMAIN_DISK_CACHE_WRITETHRU:
+ *writeback = false;
+ *direct = false;
+ *noflush = false;
+ break;
+
+ case VIR_DOMAIN_DISK_CACHE_WRITEBACK:
+ *writeback = true;
+ *direct = false;
+ *noflush = false;
+ break;
+
+ case VIR_DOMAIN_DISK_CACHE_DIRECTSYNC:
+ *writeback = false;
+ *direct = true;
+ *noflush = false;
+ break;
+
+ case VIR_DOMAIN_DISK_CACHE_UNSAFE:
+ *writeback = true;
+ *direct = false;
+ *noflush = true;
+ break;
+
+ case VIR_DOMAIN_DISK_CACHE_DEFAULT:
+ case VIR_DOMAIN_DISK_CACHE_LAST:
+ default:
+ virReportEnumRangeError(virDomainDiskCache, cachemode);
+ return -1;
+ }
+
+ return 0;
+}
+
+
void
qemuProcessEventFree(struct qemuProcessEvent *event)
{
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 21e12f6594..bbc2e83760 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -1003,4 +1003,10 @@ qemuDomainPrepareDiskSource(virDomainDiskDefPtr disk,
qemuDomainObjPrivatePtr priv,
virQEMUDriverConfigPtr cfg);
+int
+qemuDomainDiskCachemodeFlags(int cachemode,
+ bool *writeback,
+ bool *direct,
+ bool *noflush);
+
#endif /* __QEMU_DOMAIN_H__ */
--
2.16.2
10:17 a.m.
New subject: [libvirt] [PATCH 3/6] qemu: domain: Add helper for translating disk cachemode to qemu flags
On 04/04/2018 04:13 AM, Peter Krempa wrote:
Add helper which will map values of disk cache mode to the flags
which
are accepted by various parts of the qemu block layer.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_domain.h | 6 ++++
2 files changed, 81 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 9d1c33b54a..20333c9568 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -11920,6 +11920,81 @@ qemuDomainPrepareDiskSource(virDomainDiskDefPtr disk,
}
+/**
+ * qemuDomainDiskCachemodeFlags:
+ *
+ * Converts disk cachemode to the cache mode options for qemu. Returns -1 for
+ * invalid @cachemode values and fills the flags and returns 0 on success.
+ * Flags may be NULL.
+ */
+int
+qemuDomainDiskCachemodeFlags(int cachemode,
+ bool *writeback,
+ bool *direct,
+ bool *noflush)
Yuck, multiple boolean returns.... and when a new mode is added, we
possibly get another bool argument leading to another useless argument.
This could conceivably do nothing if each of the return args was NULL.
It seems what you're doing is translating or converting the
VIR_DOMAIN_DISK_CACHE_* values into some mask that you'll use elsewhere
perhaps similar to virDomainDefFormatConvertXMLFlags.
I think this just returns an unsigned int mask that's particular to some
to be defined enum that would set the right bits so that when building
the command line we can add features based on those bits.
That way it really doesn't matter that direct and noflush aren't used
(yet) in any patch that's been posted...
+{
+ bool dummy;
+
+ if (!writeback)
+ writeback = &dummy;
+
+ if (!direct)
+ direct = &dummy;
+
+ if (!noflush)
+ noflush = &dummy;
+
+ /* Mapping of cache modes to the attributes according to qemu-options.hx
+ * │ cache.writeback cache.direct cache.no-flush
+ * ─────────────┼─────────────────────────────────────────────────
+ * writeback │ true false false
+ * none │ true true false
+ * writethrough │ false false false
+ * directsync │ false true false
+ * unsafe │ true false true
+ */
+ switch ((virDomainDiskCache) cachemode) {
+ case VIR_DOMAIN_DISK_CACHE_DISABLE: /* 'none' */
+ *writeback = true;
+ *direct = true;
+ *noflush = false;
+ break;
+
+ case VIR_DOMAIN_DISK_CACHE_WRITETHRU:
+ *writeback = false;
+ *direct = false;
+ *noflush = false;
+ break;
+
+ case VIR_DOMAIN_DISK_CACHE_WRITEBACK:
+ *writeback = true;
+ *direct = false;
+ *noflush = false;
+ break;
+
+ case VIR_DOMAIN_DISK_CACHE_DIRECTSYNC:
+ *writeback = false;
+ *direct = true;
+ *noflush = false;
+ break;
+
+ case VIR_DOMAIN_DISK_CACHE_UNSAFE:
+ *writeback = true;
+ *direct = false;
+ *noflush = true;
+ break;
+
+ case VIR_DOMAIN_DISK_CACHE_DEFAULT:
Based on the patch 5 consumer qemuBuildDriveDevCacheStr - this would
DEFAULT would never happen.
+ case VIR_DOMAIN_DISK_CACHE_LAST:
+ default:
Considering how/when this helper is being used, one would hope by the
time this is used that cachemode has already been verified to be in
range. It would seem that's true because virDomainDiskDefDriverParseXML
would have failed the virDomainDiskCacheTypeFromString.
John
+ virReportEnumRangeError(virDomainDiskCache, cachemode);
+ return -1;
+ }
+
+ return 0;
+}
+
+
void
qemuProcessEventFree(struct qemuProcessEvent *event)
{
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 21e12f6594..bbc2e83760 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -1003,4 +1003,10 @@ qemuDomainPrepareDiskSource(virDomainDiskDefPtr disk,
qemuDomainObjPrivatePtr priv,
virQEMUDriverConfigPtr cfg);
+int
+qemuDomainDiskCachemodeFlags(int cachemode,
+ bool *writeback,
+ bool *direct,
+ bool *noflush);
+
#endif /* __QEMU_DOMAIN_H__ */
7:43 a.m.
New subject: [libvirt] [PATCH 3/6] qemu: domain: Add helper for translating disk cachemode to qemu flags
On Wed, Apr 11, 2018 at 11:17:46 -0400, John Ferlan wrote:
On 04/04/2018 04:13 AM, Peter Krempa wrote:
> Add helper which will map values of disk cache mode to the flags which
> are accepted by various parts of the qemu block layer.
>
> Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
> ---
> src/qemu/qemu_domain.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++
> src/qemu/qemu_domain.h | 6 ++++
> 2 files changed, 81 insertions(+)
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 9d1c33b54a..20333c9568 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -11920,6 +11920,81 @@ qemuDomainPrepareDiskSource(virDomainDiskDefPtr disk,
> }
>
>
> +/**
> + * qemuDomainDiskCachemodeFlags:
> + *
> + * Converts disk cachemode to the cache mode options for qemu. Returns -1 for
> + * invalid @cachemode values and fills the flags and returns 0 on success.
> + * Flags may be NULL.
> + */
> +int
> +qemuDomainDiskCachemodeFlags(int cachemode,
> + bool *writeback,
> + bool *direct,
> + bool *noflush)
Yuck, multiple boolean returns.... and when a new mode is added, we
possibly get another bool argument leading to another useless argument.
This could conceivably do nothing if each of the return args was NULL.
It seems what you're doing is translating or converting the
VIR_DOMAIN_DISK_CACHE_* values into some mask that you'll use elsewhere
perhaps similar to virDomainDefFormatConvertXMLFlags.
Exactly. Even the function comment says that.
I think this just returns an unsigned int mask that's particular
to some
to be defined enum that would set the right bits so that when building
the command line we can add features based on those bits.
It can't return unsigned since due to the new switch/case handling for
enums we need to be able return failure.
Also re-compressing this to a bitmap is obviously possible, but rather
pointless, since any user will need to re-extract the presence of
individual bits.
That way it really doesn't matter that direct and noflush
aren't used
(yet) in any patch that's been posted...
I fail to see how that's different from re-extracting them from a
bitmap.
> +{
> + bool dummy;
> +
> + if (!writeback)
> + writeback = &dummy;
> +
> + if (!direct)
> + direct = &dummy;
> +
> + if (!noflush)
> + noflush = &dummy;
> +
> + /* Mapping of cache modes to the attributes according to qemu-options.hx
> + * │ cache.writeback cache.direct cache.no-flush
> + * ─────────────┼─────────────────────────────────────────────────
> + * writeback │ true false false
> + * none │ true true false
> + * writethrough │ false false false
> + * directsync │ false true false
> + * unsafe │ true false true
> + */
> + switch ((virDomainDiskCache) cachemode) {
> + case VIR_DOMAIN_DISK_CACHE_DISABLE: /* 'none' */
> + *writeback = true;
> + *direct = true;
> + *noflush = false;
> + break;
> +
> + case VIR_DOMAIN_DISK_CACHE_WRITETHRU:
> + *writeback = false;
> + *direct = false;
> + *noflush = false;
> + break;
> +
> + case VIR_DOMAIN_DISK_CACHE_WRITEBACK:
> + *writeback = true;
> + *direct = false;
> + *noflush = false;
> + break;
> +
> + case VIR_DOMAIN_DISK_CACHE_DIRECTSYNC:
> + *writeback = false;
> + *direct = true;
> + *noflush = false;
> + break;
> +
> + case VIR_DOMAIN_DISK_CACHE_UNSAFE:
> + *writeback = true;
> + *direct = false;
> + *noflush = true;
> + break;
> +
> + case VIR_DOMAIN_DISK_CACHE_DEFAULT:
Based on the patch 5 consumer qemuBuildDriveDevCacheStr - this would
DEFAULT would never happen.
> + case VIR_DOMAIN_DISK_CACHE_LAST:
> + default:
Considering how/when this helper is being used, one would hope by the
time this is used that cachemode has already been verified to be in
range. It would seem that's true because virDomainDiskDefDriverParseXML
would have failed the virDomainDiskCacheTypeFromString.
No, callers only verify one end of the range. The other end of the range
is not verified. Also we recently decided to always include the default
case even when we validate the callers or fully enumerate all values,
since it does not prevent assigning any invalid value.
11:04 a.m.
New subject: [libvirt] [PATCH 3/6] qemu: domain: Add helper for translating disk cachemode to qemu flags
On Wed, Apr 04, 2018 at 10:13:56AM +0200, Peter Krempa wrote:
Add helper which will map values of disk cache mode to the flags
which
are accepted by various parts of the qemu block layer.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_domain.h | 6 ++++
2 files changed, 81 insertions(+)
ACK
Jano
3:13 a.m.
New subject: [libvirt] [PATCH 4/6] qemu: caps: Add capability for 'write-cache' parameter of disk frontends
QEMU translates the cache mode of a disk internally into 3 flags.
'write-cache' is a flag of the frontend while others are flag of the
backing storage. Add capability which will allow expressing it via the
frontend attribute.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 5 +++++
src/qemu/qemu_capabilities.h | 1 +
tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 +
18 files changed, 22 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 959c27f3bf..dea2191e03 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -467,6 +467,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
"virtio-mouse-ccw",
"virtio-tablet-ccw",
"qcow2-luks",
+ "disk-write-cache",
);
@@ -1727,6 +1728,7 @@ static struct virQEMUCapsStringFlags
virQEMUCapsObjectPropsVirtioBlk[] = {
{ "disable-legacy", QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY },
{ "iommu_platform", QEMU_CAPS_VIRTIO_PCI_IOMMU_PLATFORM },
{ "ats", QEMU_CAPS_VIRTIO_PCI_ATS },
+ { "write-cache", QEMU_CAPS_DISK_WRITE_CACHE },
};
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioNet[] = {
@@ -1764,11 +1766,13 @@ static struct virQEMUCapsStringFlags
virQEMUCapsObjectPropsSCSIDisk[] = {
{ "channel", QEMU_CAPS_SCSI_DISK_CHANNEL },
{ "wwn", QEMU_CAPS_SCSI_DISK_WWN },
{ "share-rw", QEMU_CAPS_DISK_SHARE_RW },
+ { "write-cache", QEMU_CAPS_DISK_WRITE_CACHE },
};
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsIDEDrive[] = {
{ "wwn", QEMU_CAPS_IDE_DRIVE_WWN },
{ "share-rw", QEMU_CAPS_DISK_SHARE_RW },
+ { "write-cache", QEMU_CAPS_DISK_WRITE_CACHE },
};
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsPiix4PM[] = {
@@ -1800,6 +1804,7 @@ static struct virQEMUCapsStringFlags
virQEMUCapsObjectPropsQ35PCIHost[] = {
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsUSBStorage[] = {
{ "removable", QEMU_CAPS_USB_STORAGE_REMOVABLE },
{ "share-rw", QEMU_CAPS_DISK_SHARE_RW },
+ { "write-cache", QEMU_CAPS_DISK_WRITE_CACHE },
};
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsKVMPit[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 2203c28aa0..b2a48304f4 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -451,6 +451,7 @@ typedef enum {
QEMU_CAPS_DEVICE_VIRTIO_MOUSE_CCW, /* -device virtio-mouse-ccw */
QEMU_CAPS_DEVICE_VIRTIO_TABLET_CCW, /* -device virtio-tablet-ccw */
QEMU_CAPS_QCOW2_LUKS, /* qcow2 format support LUKS encryption */
+ QEMU_CAPS_DISK_WRITE_CACHE, /* qemu block frontends support write-cache param */
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml
b/tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml
index 7585e02da2..2cdb246563 100644
--- a/tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml
@@ -187,6 +187,7 @@
<flag name='pl011'/>
<flag name='dump-completed'/>
<flag name='qcow2-luks'/>
+ <flag name='disk-write-cache'/>
<version>2010000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>303541</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml
b/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml
index 153e199c41..26c39051f7 100644
--- a/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml
@@ -186,6 +186,7 @@
<flag name='machine.pseries.max-cpu-compat'/>
<flag name='dump-completed'/>
<flag name='qcow2-luks'/>
+ <flag name='disk-write-cache'/>
<version>2010000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>382824</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml
index c4be3fca51..ce7c6eff09 100644
--- a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml
@@ -147,6 +147,7 @@
<flag name='iscsi.password-secret'/>
<flag name='dump-completed'/>
<flag name='qcow2-luks'/>
+ <flag name='disk-write-cache'/>
<version>2010000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>303326</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml
index 4dd5602014..4e6dac34fb 100644
--- a/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml
@@ -230,6 +230,7 @@
<flag name='isa-serial'/>
<flag name='dump-completed'/>
<flag name='qcow2-luks'/>
+ <flag name='disk-write-cache'/>
<version>2010000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>344938</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
index cbd645ae93..3cdea03a71 100644
--- a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
@@ -151,6 +151,7 @@
<flag name='virtio-mouse-ccw'/>
<flag name='virtio-tablet-ccw'/>
<flag name='qcow2-luks'/>
+ <flag name='disk-write-cache'/>
<version>2011000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>342058</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml
b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml
index ec2eec17f4..81d67388f4 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml
@@ -188,6 +188,7 @@
<flag name='pl011'/>
<flag name='dump-completed'/>
<flag name='qcow2-luks'/>
+ <flag name='disk-write-cache'/>
<version>2011090</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>342346</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
index 1122d6408b..b8b62c85ff 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
@@ -186,6 +186,7 @@
<flag name='machine.pseries.max-cpu-compat'/>
<flag name='dump-completed'/>
<flag name='qcow2-luks'/>
+ <flag name='disk-write-cache'/>
<version>2011090</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>419215</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
index 191b1e0e37..c634800340 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
@@ -151,6 +151,7 @@
<flag name='virtio-mouse-ccw'/>
<flag name='virtio-tablet-ccw'/>
<flag name='qcow2-luks'/>
+ <flag name='disk-write-cache'/>
<version>2011090</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>0</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml
index aa5de811e1..97d8355451 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml
@@ -226,6 +226,7 @@
<flag name='isa-serial'/>
<flag name='dump-completed'/>
<flag name='qcow2-luks'/>
+ <flag name='disk-write-cache'/>
<version>2011090</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>390060</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml
index b58c3a1bda..7547d66479 100644
--- a/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml
@@ -137,6 +137,7 @@
<flag name='virtio-blk.num-queues'/>
<flag name='sclplmconsole'/>
<flag name='dump-completed'/>
+ <flag name='disk-write-cache'/>
<version>2007000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>216732</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml
index 736e61aeb2..e4d53f3dd1 100644
--- a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml
@@ -210,6 +210,7 @@
<flag name='virtio-blk.num-queues'/>
<flag name='isa-serial'/>
<flag name='dump-completed'/>
+ <flag name='disk-write-cache'/>
<version>2007000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>239029</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
index 2b91337fde..ee7d4d6b81 100644
--- a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
@@ -139,6 +139,7 @@
<flag name='virtio-blk.num-queues'/>
<flag name='sclplmconsole'/>
<flag name='dump-completed'/>
+ <flag name='disk-write-cache'/>
<version>2007093</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>241633</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml
index df6f7be340..f815c5c324 100644
--- a/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml
@@ -212,6 +212,7 @@
<flag name='virtio-blk.num-queues'/>
<flag name='isa-serial'/>
<flag name='dump-completed'/>
+ <flag name='disk-write-cache'/>
<version>2008000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>255684</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml
b/tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml
index c83b5211f3..1c4082ac52 100644
--- a/tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml
@@ -178,6 +178,7 @@
<flag name='iscsi.password-secret'/>
<flag name='isa-serial'/>
<flag name='dump-completed'/>
+ <flag name='disk-write-cache'/>
<version>2009000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>346538</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml
index ac8d145d19..67ee72155b 100644
--- a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml
@@ -142,6 +142,7 @@
<flag name='disk-share-rw'/>
<flag name='iscsi.password-secret'/>
<flag name='dump-completed'/>
+ <flag name='disk-write-cache'/>
<version>2009000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>265051</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml
index 13fc8c143f..97908bf5a0 100644
--- a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml
@@ -225,6 +225,7 @@
<flag name='iscsi.password-secret'/>
<flag name='isa-serial'/>
<flag name='dump-completed'/>
+ <flag name='disk-write-cache'/>
<version>2009000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>320947</microcodeVersion>
--
2.16.2
10:18 a.m.
New subject: [libvirt] [PATCH 4/6] qemu: caps: Add capability for 'write-cache' parameter of disk frontends
On 04/04/2018 04:13 AM, Peter Krempa wrote:
QEMU translates the cache mode of a disk internally into 3 flags.
'write-cache' is a flag of the frontend while others are flag of the
backing storage. Add capability which will allow expressing it via the
frontend attribute.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 5 +++++
src/qemu/qemu_capabilities.h | 1 +
tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 +
18 files changed, 22 insertions(+)
Will require merges with top of tree, but that's a trivial exercise.
Curious though - the testing patch 6 of this series would seem to imply
that this feature is only available for at least QEMU 2.12 on x64_64;
however, the bit shows up starting in 2.7 on other arches, so since
patch 5 only uses QEMU_CAPS_DISK_WRITE_CACHE to decide whether to use
this or not, doesn't that imply earlier versions could use this? or
that the testing should be different?
John
7:32 a.m.
New subject: [libvirt] [PATCH 4/6] qemu: caps: Add capability for 'write-cache' parameter of disk frontends
On Wed, Apr 11, 2018 at 11:18:32 -0400, John Ferlan wrote:
On 04/04/2018 04:13 AM, Peter Krempa wrote:
> QEMU translates the cache mode of a disk internally into 3 flags.
> 'write-cache' is a flag of the frontend while others are flag of the
> backing storage. Add capability which will allow expressing it via the
> frontend attribute.
>
> Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
> ---
> src/qemu/qemu_capabilities.c | 5 +++++
> src/qemu/qemu_capabilities.h | 1 +
> tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml | 1 +
> tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml | 1 +
> tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 1 +
> tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml | 1 +
> tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml | 1 +
> tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 1 +
> tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml | 1 +
> tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml | 1 +
> tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 1 +
> tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml | 1 +
> tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 +
> tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 +
> tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml | 1 +
> tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml | 1 +
> tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 +
> tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 +
> 18 files changed, 22 insertions(+)
>
Will require merges with top of tree, but that's a trivial exercise.
Curious though - the testing patch 6 of this series would seem to imply
that this feature is only available for at least QEMU 2.12 on x64_64;
I don't think that the DO_TEST_CAPS should be taken as an implication of
when the feature was introduced.
DO_TEST_CAPS is a better option for positive test cases than having to
explicitly name capabilities as with DO_TEST.
however, the bit shows up starting in 2.7 on other arches, so since
patch 5 only uses QEMU_CAPS_DISK_WRITE_CACHE to decide whether to use
this or not, doesn't that imply earlier versions could use this? or
that the testing should be different?
No. DO_TEST_CAPS is not implying anything.
11:04 a.m.
New subject: [libvirt] [PATCH 4/6] qemu: caps: Add capability for 'write-cache' parameter of disk frontends
On Wed, Apr 04, 2018 at 10:13:57AM +0200, Peter Krempa wrote:
QEMU translates the cache mode of a disk internally into 3 flags.
'write-cache' is a flag of the frontend while others are flag of the
backing storage. Add capability which will allow expressing it via the
frontend attribute.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 5 +++++
src/qemu/qemu_capabilities.h | 1 +
tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 +
18 files changed, 22 insertions(+)
ACK
Jano
3:13 a.m.
New subject: [libvirt] [PATCH 5/6] qemu: Format 'write-cache' parameter for disk frontends
The disk cache mode translates to various frontend and backend
attributes for the qemu block layer. For the frontend device the
'writeback' parameter is used and provided as 'write-cache'. Implement
this so that we can later switch to using -blockdev where we will not
pass the cachemode directly any more.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_command.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index bbd3cd0a7d..83e263e9f9 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1882,6 +1882,30 @@ qemuCheckIOThreads(const virDomainDef *def,
}
+static int
+qemuBuildDriveDevCacheStr(virDomainDiskDefPtr disk,
+ virBufferPtr buf,
+ virQEMUCapsPtr qemuCaps)
+{
+ bool wb;
+
+ if (disk->cachemode == VIR_DOMAIN_DISK_CACHE_DEFAULT)
+ return 0;
+
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DISK_WRITE_CACHE))
+ return 0;
+
+ if (qemuDomainDiskCachemodeFlags(disk->cachemode, &wb, NULL, NULL) < 0)
+ return -1;
+
+ virBufferStrcat(buf, ",write-cache=",
+ virTristateSwitchTypeToString(virTristateSwitchFromBool(wb)),
+ NULL);
+
+ return 0;
+}
+
+
char *
qemuBuildDriveDevStr(const virDomainDef *def,
virDomainDiskDefPtr disk,
@@ -2194,6 +2218,9 @@ qemuBuildDriveDevStr(const virDomainDef *def,
}
}
+ if (qemuBuildDriveDevCacheStr(disk, &opt, qemuCaps) < 0)
+ goto error;
+
if (virBufferCheckError(&opt) < 0)
goto error;
--
2.16.2
10:19 a.m.
New subject: [libvirt] [PATCH 5/6] qemu: Format 'write-cache' parameter for disk frontends
On 04/04/2018 04:13 AM, Peter Krempa wrote:
The disk cache mode translates to various frontend and backend
attributes for the qemu block layer. For the frontend device the
'writeback' parameter is used and provided as 'write-cache'. Implement
this so that we can later switch to using -blockdev where we will not
pass the cachemode directly any more.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_command.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index bbd3cd0a7d..83e263e9f9 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1882,6 +1882,30 @@ qemuCheckIOThreads(const virDomainDef *def,
}
+static int
+qemuBuildDriveDevCacheStr(virDomainDiskDefPtr disk,
+ virBufferPtr buf,
+ virQEMUCapsPtr qemuCaps)
+{
+ bool wb;
+
+ if (disk->cachemode == VIR_DOMAIN_DISK_CACHE_DEFAULT)
+ return 0;
+
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DISK_WRITE_CACHE))
+ return 0;
As I noted in patch 4 - this is checking a bit that's defined in QEMU
2.7 on some arches, but the test in patch 6 (probably should be in this
patch) would seem to indicate QEMU 2.12 is required.
+
+ if (qemuDomainDiskCachemodeFlags(disk->cachemode, &wb, NULL, NULL) < 0)
+ return -1;
Wouldn't bother with -1 since we already know that cachemode will be
valid (e.g. from my comments in patch 4).
John
+
+ virBufferStrcat(buf, ",write-cache=",
+ virTristateSwitchTypeToString(virTristateSwitchFromBool(wb)),
+ NULL);
+
+ return 0;
+}
+
+
char *
qemuBuildDriveDevStr(const virDomainDef *def,
virDomainDiskDefPtr disk,
@@ -2194,6 +2218,9 @@ qemuBuildDriveDevStr(const virDomainDef *def,
}
}
+ if (qemuBuildDriveDevCacheStr(disk, &opt, qemuCaps) < 0)
+ goto error;
+
if (virBufferCheckError(&opt) < 0)
goto error;
11:04 a.m.
New subject: [libvirt] [PATCH 5/6] qemu: Format 'write-cache' parameter for disk frontends
On Wed, Apr 04, 2018 at 10:13:58AM +0200, Peter Krempa wrote:
The disk cache mode translates to various frontend and backend
attributes for the qemu block layer. For the frontend device the
'writeback' parameter is used and provided as 'write-cache'. Implement
this so that we can later switch to using -blockdev where we will not
pass the cachemode directly any more.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_command.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
ACK
Jano
3:13 a.m.
New subject: [libvirt] [PATCH 6/6] tests: qemuxml2argv: Test formatting of 'write-cache' parameter
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
tests/qemuxml2argvdata/disk-drive-write-cache.args | 45 ++++++++++++++++++++++
tests/qemuxml2argvdata/disk-drive-write-cache.xml | 45 ++++++++++++++++++++++
tests/qemuxml2argvtest.c | 1 +
3 files changed, 91 insertions(+)
create mode 100644 tests/qemuxml2argvdata/disk-drive-write-cache.args
create mode 100644 tests/qemuxml2argvdata/disk-drive-write-cache.xml
diff --git a/tests/qemuxml2argvdata/disk-drive-write-cache.args
b/tests/qemuxml2argvdata/disk-drive-write-cache.args
new file mode 100644
index 0000000000..3973249d18
--- /dev/null
+++ b/tests/qemuxml2argvdata/disk-drive-write-cache.args
@@ -0,0 +1,45 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-i686 \
+-name guest=QEMUGuest1,debug-threads=on \
+-S \
+-object secret,id=masterKey0,format=raw,\
+file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
+-machine pc-i440fx-2.12,accel=tcg,usb=off,dump-guest-core=off \
+-m 214 \
+-realtime mlock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-no-acpi \
+-boot strict=on \
+-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \
+-device lsi,id=scsi0,bus=pci.0,addr=0x2 \
+-drive file=/dev/HostVG/QEMUGuest1,format=qcow2,if=none,id=drive-ide0-0-0,\
+cache=writeback \
+-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1,\
+write-cache=on \
+-drive file=/dev/HostVG/QEMUGuest1,format=qcow2,if=none,id=drive-scsi0-0-0,\
+cache=none \
+-device scsi-hd,bus=scsi0.0,scsi-id=0,drive=drive-scsi0-0-0,id=scsi0-0-0,\
+write-cache=on \
+-drive file=/dev/HostVG/QEMUGuest1,format=qcow2,if=none,id=drive-virtio-disk0,\
+cache=writethrough \
+-device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x3,drive=drive-virtio-disk0,\
+id=virtio-disk0,write-cache=off \
+-drive file=/dev/HostVG/QEMUGuest1,format=qcow2,if=none,id=drive-usb-disk1,\
+cache=directsync \
+-device usb-storage,bus=usb.0,port=1,drive=drive-usb-disk1,id=usb-disk1,\
+removable=off,write-cache=off \
+-msg timestamp=on
diff --git a/tests/qemuxml2argvdata/disk-drive-write-cache.xml
b/tests/qemuxml2argvdata/disk-drive-write-cache.xml
new file mode 100644
index 0000000000..dc7bdd6050
--- /dev/null
+++ b/tests/qemuxml2argvdata/disk-drive-write-cache.xml
@@ -0,0 +1,45 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219136</memory>
+ <currentMemory unit='KiB'>219136</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-system-i686</emulator>
+ <disk type='block' device='disk'>
+ <driver name='qemu' type='qcow2' cache='writeback'/>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='hda' bus='ide'/>
+ <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
+ </disk>
+ <disk type='block' device='disk'>
+ <driver name='qemu' type='qcow2' cache='none'/>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='sda' bus='scsi'/>
+ </disk>
+ <disk type='block' device='disk'>
+ <driver name='qemu' type='qcow2'
cache='writethrough'/>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='vda' bus='virtio'/>
+ </disk>
+ <disk type='block' device='disk'>
+ <driver name='qemu' type='qcow2'
cache='directsync'/>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='sdb' bus='usb'/>
+ </disk>
+ <controller type='usb' index='0'/>
+ <controller type='ide' index='0'/>
+ <controller type='pci' index='0' model='pci-root'/>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index c540ce2f50..b70780bf4e 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -998,6 +998,7 @@ mymain(void)
DO_TEST("disk-drive-cache-v2-wt", NONE);
DO_TEST("disk-drive-cache-v2-wb", NONE);
DO_TEST("disk-drive-cache-v2-none", NONE);
+ DO_TEST_CAPS("disk-drive-write-cache", "2.12.0");
DO_TEST("disk-drive-cache-directsync",
QEMU_CAPS_DRIVE_CACHE_DIRECTSYNC);
DO_TEST("disk-drive-cache-unsafe",
--
2.16.2
10:20 a.m.
New subject: [libvirt] [PATCH 6/6] tests: qemuxml2argv: Test formatting of 'write-cache' parameter
On 04/04/2018 04:13 AM, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
tests/qemuxml2argvdata/disk-drive-write-cache.args | 45 ++++++++++++++++++++++
tests/qemuxml2argvdata/disk-drive-write-cache.xml | 45 ++++++++++++++++++++++
tests/qemuxml2argvtest.c | 1 +
3 files changed, 91 insertions(+)
create mode 100644 tests/qemuxml2argvdata/disk-drive-write-cache.args
create mode 100644 tests/qemuxml2argvdata/disk-drive-write-cache.xml
This should be merged with patch 5 - there's also no xml2xml test
Also considering patch 2, all we're testing is x86_64 on 2.12 even
though we have the feature available on other arches for other versions.
I don't believe we have any existing test that sets cache= to anything
other than 'none' (e.g. qemuDiskCacheV2 isn't really used or checked
currently).
Why wouldn't any of the existing cache=none would change too... OK
other than the fact that you're only testing for 2.12 and beyond...
Perhaps it would have been interesting to see the results of this test
before the new capability and how it changes afterwards.
John
diff --git a/tests/qemuxml2argvdata/disk-drive-write-cache.args
b/tests/qemuxml2argvdata/disk-drive-write-cache.args
new file mode 100644
index 0000000000..3973249d18
--- /dev/null
+++ b/tests/qemuxml2argvdata/disk-drive-write-cache.args
@@ -0,0 +1,45 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-i686 \
+-name guest=QEMUGuest1,debug-threads=on \
+-S \
+-object secret,id=masterKey0,format=raw,\
+file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
+-machine pc-i440fx-2.12,accel=tcg,usb=off,dump-guest-core=off \
+-m 214 \
+-realtime mlock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-no-acpi \
+-boot strict=on \
+-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \
+-device lsi,id=scsi0,bus=pci.0,addr=0x2 \
+-drive file=/dev/HostVG/QEMUGuest1,format=qcow2,if=none,id=drive-ide0-0-0,\
+cache=writeback \
+-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1,\
+write-cache=on \
+-drive file=/dev/HostVG/QEMUGuest1,format=qcow2,if=none,id=drive-scsi0-0-0,\
+cache=none \
+-device scsi-hd,bus=scsi0.0,scsi-id=0,drive=drive-scsi0-0-0,id=scsi0-0-0,\
+write-cache=on \
+-drive file=/dev/HostVG/QEMUGuest1,format=qcow2,if=none,id=drive-virtio-disk0,\
+cache=writethrough \
+-device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x3,drive=drive-virtio-disk0,\
+id=virtio-disk0,write-cache=off \
+-drive file=/dev/HostVG/QEMUGuest1,format=qcow2,if=none,id=drive-usb-disk1,\
+cache=directsync \
+-device usb-storage,bus=usb.0,port=1,drive=drive-usb-disk1,id=usb-disk1,\
+removable=off,write-cache=off \
+-msg timestamp=on
diff --git a/tests/qemuxml2argvdata/disk-drive-write-cache.xml
b/tests/qemuxml2argvdata/disk-drive-write-cache.xml
new file mode 100644
index 0000000000..dc7bdd6050
--- /dev/null
+++ b/tests/qemuxml2argvdata/disk-drive-write-cache.xml
@@ -0,0 +1,45 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219136</memory>
+ <currentMemory unit='KiB'>219136</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-system-i686</emulator>
+ <disk type='block' device='disk'>
+ <driver name='qemu' type='qcow2'
cache='writeback'/>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='hda' bus='ide'/>
+ <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
+ </disk>
+ <disk type='block' device='disk'>
+ <driver name='qemu' type='qcow2' cache='none'/>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='sda' bus='scsi'/>
+ </disk>
+ <disk type='block' device='disk'>
+ <driver name='qemu' type='qcow2'
cache='writethrough'/>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='vda' bus='virtio'/>
+ </disk>
+ <disk type='block' device='disk'>
+ <driver name='qemu' type='qcow2'
cache='directsync'/>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='sdb' bus='usb'/>
+ </disk>
+ <controller type='usb' index='0'/>
+ <controller type='ide' index='0'/>
+ <controller type='pci' index='0' model='pci-root'/>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index c540ce2f50..b70780bf4e 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -998,6 +998,7 @@ mymain(void)
DO_TEST("disk-drive-cache-v2-wt", NONE);
DO_TEST("disk-drive-cache-v2-wb", NONE);
DO_TEST("disk-drive-cache-v2-none", NONE);
+ DO_TEST_CAPS("disk-drive-write-cache", "2.12.0");
DO_TEST("disk-drive-cache-directsync",
QEMU_CAPS_DRIVE_CACHE_DIRECTSYNC);
DO_TEST("disk-drive-cache-unsafe",
7:23 a.m.
New subject: [libvirt] [PATCH 6/6] tests: qemuxml2argv: Test formatting of 'write-cache' parameter
On Wed, Apr 11, 2018 at 11:20:43 -0400, John Ferlan wrote:
On 04/04/2018 04:13 AM, Peter Krempa wrote:
> Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
> ---
> tests/qemuxml2argvdata/disk-drive-write-cache.args | 45 ++++++++++++++++++++++
> tests/qemuxml2argvdata/disk-drive-write-cache.xml | 45 ++++++++++++++++++++++
> tests/qemuxml2argvtest.c | 1 +
> 3 files changed, 91 insertions(+)
> create mode 100644 tests/qemuxml2argvdata/disk-drive-write-cache.args
> create mode 100644 tests/qemuxml2argvdata/disk-drive-write-cache.xml
>
This should be merged with patch 5 - there's also no xml2xml test
I can squash this in to the previous patch, but I usually prefer having
them separate when we are adding a lot of new lines.
I don't think a XML2XML test is necessary here. This feature is not
using any new XML configuration, but rather uses existing values to
express the new configuration.
Also considering patch 2, all we're testing is x86_64 on 2.12
even
though we have the feature available on other arches for other versions.
Well, given that prior to the ability to test it with real capabilities,
we'd test it with a configuration which would not exist by listing the
required capability bits in the surce file, the same would apply.
I don't believe we have any existing test that sets cache= to
anything
other than 'none' (e.g. qemuDiskCacheV2 isn't really used or checked
currently).
Why wouldn't any of the existing cache=none would change too... OK
other than the fact that you're only testing for 2.12 and beyond...
As said above. That's originating from the fact that our previous
testing sucked and did not really compare to anything real-life.
If I'd were to add a DO_TEST case using the capability explicitly the
tests would not change in the same way as they did not change here, so
the version used for capabilities does not really matter in any way
here.
Perhaps it would have been interesting to see the results of this test
before the new capability and how it changes afterwards.
I can shuffle them around if you want. But it then can't be merged with
previous patch. Also until the feature is implemented, the test will not
test what it is declaring to test.
11:05 a.m.
New subject: [libvirt] [PATCH 6/6] tests: qemuxml2argv: Test formatting of 'write-cache' parameter
On Wed, Apr 04, 2018 at 10:13:59AM +0200, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
tests/qemuxml2argvdata/disk-drive-write-cache.args | 45 ++++++++++++++++++++++
tests/qemuxml2argvdata/disk-drive-write-cache.xml | 45 ++++++++++++++++++++++
tests/qemuxml2argvtest.c | 1 +
3 files changed, 91 insertions(+)
create mode 100644 tests/qemuxml2argvdata/disk-drive-write-cache.args
create mode 100644 tests/qemuxml2argvdata/disk-drive-write-cache.xml
ACK
Jano
2411
days inactive
2424
days old
24 comments
3 participants
participants (3)
-
John Ferlan -
Ján Tomko -
Peter Krempa