[libvirt] [PATCH 0/7] qemu: Support page size tuning for pSeries guests
This applies cleanly on top of a0d6894af1b1. Patch 1/7 is too big to go through the list; it can be fetched, along with the rest of the series, from [1]. Patch 2/7 conflicts with patch 1/3 from [2], but making this one depend on it didn't feel right; whichever of the two series gets reviewed and merged first, I'll probably post a rebase of the other one immediately afterwards. Changes from [RFC]: * the QEMU interface has changed based on feedback, which means some of the code is no longer useful and has been dropped; * switched to virXMLFormatElement(), as suggested during review; * added documentation and release notes entry. [1] https://github.com/andreabolognani/libvirt/tree/pseries-hpt-maxpagesize [2] https://www.redhat.com/archives/libvir-list/2018-June/msg01374.html [RFC] https://www.redhat.com/archives/libvir-list/2018-May/msg01710.html Andrea Bolognani (7): tests: Add replies for QEMU 3.0.0 on ppc64 qemu: Add capability for the HPT maxpagesize feature conf: Reintroduce virDomainDef::hpt_resizing conf: Tweak HPT feature parsing and formatting conf: Parse and format HPT maxpagesize qemu: Format HPT maxpagesize on the command line news: Update for HPT maxpagesize feature docs/formatdomain.html.in | 11 +- docs/news.xml | 11 + docs/schemas/domaincommon.rng | 21 +- src/conf/domain_conf.c | 60 +- src/conf/domain_conf.h | 2 + src/qemu/qemu_capabilities.c | 8 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 43 +- src/qemu/qemu_domain.c | 2 +- .../caps_2.12.0.aarch64.replies | 48 +- .../caps_2.12.0.aarch64.xml | 2 +- .../caps_2.12.0.ppc64.replies | 197 +- .../caps_2.12.0.ppc64.xml | 2 +- .../caps_2.12.0.s390x.replies | 52 +- .../caps_2.12.0.s390x.xml | 2 +- .../caps_2.12.0.x86_64.replies | 64 +- .../caps_2.12.0.x86_64.xml | 2 +- ...ppc64.replies => caps_3.0.0.ppc64.replies} | 5268 ++++++++++------- ..._2.12.0.ppc64.xml => caps_3.0.0.ppc64.xml} | 19 +- tests/qemucapabilitiestest.c | 1 + tests/qemuxml2argvdata/pseries-features.args | 3 +- tests/qemuxml2argvdata/pseries-features.xml | 18 +- tests/qemuxml2argvtest.c | 1 + tests/qemuxml2xmloutdata/pseries-features.xml | 31 +- tests/qemuxml2xmltest.c | 1 + 25 files changed, 3624 insertions(+), 2246 deletions(-) copy tests/qemucapabilitiesdata/{caps_2.12.0.ppc64.replies => caps_3.0.0.ppc64.replies} (92%) copy tests/qemucapabilitiesdata/{caps_2.12.0.ppc64.xml => caps_3.0.0.ppc64.xml} (99%) mode change 120000 => 100644 tests/qemuxml2xmloutdata/pseries-features.xml -- 2.17.1
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- .../caps_3.0.0.ppc64.replies | 23673 ++++++++++++++++ .../qemucapabilitiesdata/caps_3.0.0.ppc64.xml | 1074 + tests/qemucapabilitiestest.c | 1 + 3 files changed, 24748 insertions(+) create mode 100644 tests/qemucapabilitiesdata/caps_3.0.0.ppc64.replies create mode 100644 tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.replies b/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.replies new file mode 100644 index 0000000000..6aa99733ee --- /dev/null +++ b/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.replies @@ -0,0 +1,23673 @@ +{ + "execute": "qmp_capabilities", + "id": "libvirt-1" +} + +{ + "return": { + }, + "id": "libvirt-1" +} + +{ + "execute": "query-version", + "id": "libvirt-2" +} + +{ + "return": { + "qemu": { + "micro": 50, + "minor": 12, + "major": 2 + }, + "package": "v2.12.0-1689-g518d23a" + }, + "id": "libvirt-2" +} [...] diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml new file mode 100644 index 0000000000..c98db19815 --- /dev/null +++ b/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml @@ -0,0 +1,1074 @@ +<qemuCaps> + <qemuctime>0</qemuctime> + <selfctime>0</selfctime> + <selfvers>0</selfvers> + <usedQMP/> [...] +</qemuCaps> diff --git a/tests/qemucapabilitiestest.c b/tests/qemucapabilitiestest.c index 633389f263..43f2bcfbc8 100644 --- a/tests/qemucapabilitiestest.c +++ b/tests/qemucapabilitiestest.c @@ -182,6 +182,7 @@ mymain(void) DO_TEST("ppc64", "caps_2.9.0"); DO_TEST("ppc64", "caps_2.10.0"); DO_TEST("ppc64", "caps_2.12.0"); + DO_TEST("ppc64", "caps_3.0.0"); DO_TEST("s390x", "caps_2.7.0"); DO_TEST("s390x", "caps_2.8.0"); DO_TEST("s390x", "caps_2.9.0"); -- 2.17.1
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_capabilities.c | 8 + src/qemu/qemu_capabilities.h | 1 + .../caps_2.12.0.aarch64.replies | 48 ++-- .../caps_2.12.0.aarch64.xml | 2 +- .../caps_2.12.0.ppc64.replies | 197 +++++++++++++++-- .../caps_2.12.0.ppc64.xml | 2 +- .../caps_2.12.0.s390x.replies | 52 +++-- .../caps_2.12.0.s390x.xml | 2 +- .../caps_2.12.0.x86_64.replies | 64 ++++-- .../caps_2.12.0.x86_64.xml | 2 +- .../caps_3.0.0.ppc64.replies | 207 ++++++++++++++++-- .../qemucapabilitiesdata/caps_3.0.0.ppc64.xml | 3 +- 12 files changed, 497 insertions(+), 91 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 419208ad5c..37c8fbe3d3 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -500,6 +500,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, /* 310 */ "sev-guest", + "machine.pseries.cap-hpt-max-page-size", ); @@ -1428,10 +1429,17 @@ static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsMemoryBackendFile[] = { "discard-data", QEMU_CAPS_OBJECT_MEMORY_FILE_DISCARD }, }; +static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsSPAPRMachine[] = { + { "cap-hpt-max-page-size", QEMU_CAPS_MACHINE_PSERIES_CAP_HPT_MAX_PAGE_SIZE }, +}; + static virQEMUCapsObjectTypeProps virQEMUCapsObjectProps[] = { { "memory-backend-file", virQEMUCapsObjectPropsMemoryBackendFile, ARRAY_CARDINALITY(virQEMUCapsObjectPropsMemoryBackendFile), QEMU_CAPS_OBJECT_MEMORY_FILE }, + { "spapr-machine", virQEMUCapsObjectPropsSPAPRMachine, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsSPAPRMachine), + -1 }, }; static void diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 3519a194e9..df9bf49abb 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -484,6 +484,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ /* 310 */ QEMU_CAPS_SEV_GUEST, /* -object sev-guest,... */ + QEMU_CAPS_MACHINE_PSERIES_CAP_HPT_MAX_PAGE_SIZE, /* -machine pseries.cap-hpt-max-page-size */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.replies b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.replies index e0b4f6da38..9a8e54c63d 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.replies +++ b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.replies @@ -5582,10 +5582,26 @@ } { - "execute": "query-machines", + "execute": "qom-list-properties", + "arguments": { + "typename": "spapr-machine" + }, "id": "libvirt-37" } +{ + "id": "libvirt-37", + "error": { + "class": "DeviceNotFound", + "desc": "Class 'spapr-machine' not found" + } +} + +{ + "execute": "query-machines", + "id": "libvirt-38" +} + { "return": [ { @@ -5880,12 +5896,12 @@ "cpu-max": 1 } ], - "id": "libvirt-37" + "id": "libvirt-38" } { "execute": "query-cpu-definitions", - "id": "libvirt-38" + "id": "libvirt-39" } { @@ -6061,35 +6077,35 @@ "static": false } ], - "id": "libvirt-38" + "id": "libvirt-39" } { "execute": "query-tpm-models", - "id": "libvirt-39" + "id": "libvirt-40" } { "return": [ ], - "id": "libvirt-39" + "id": "libvirt-40" } { "execute": "query-tpm-types", - "id": "libvirt-40" + "id": "libvirt-41" } { "return": [ "emulator" ], - "id": "libvirt-40" + "id": "libvirt-41" } { "execute": "query-command-line-options", - "id": "libvirt-41" + "id": "libvirt-42" } { @@ -7250,12 +7266,12 @@ "option": "drive" } ], - "id": "libvirt-41" + "id": "libvirt-42" } { "execute": "query-migrate-capabilities", - "id": "libvirt-42" + "id": "libvirt-43" } { @@ -7317,12 +7333,12 @@ "capability": "dirty-bitmaps" } ], - "id": "libvirt-42" + "id": "libvirt-43" } { "execute": "query-qmp-schema", - "id": "libvirt-43" + "id": "libvirt-44" } { @@ -18690,12 +18706,12 @@ "meta-type": "object" } ], - "id": "libvirt-43" + "id": "libvirt-44" } { "execute": "query-gic-capabilities", - "id": "libvirt-44" + "id": "libvirt-45" } { @@ -18711,7 +18727,7 @@ "kernel": false } ], - "id": "libvirt-44" + "id": "libvirt-45" } { diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml index 5ed0290397..ecc029f403 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml @@ -171,7 +171,7 @@ <flag name='tpm-emulator'/> <version>2011090</version> <kvmVersion>0</kvmVersion> - <microcodeVersion>347313</microcodeVersion> + <microcodeVersion>347550</microcodeVersion> <package>v2.12.0-rc0</package> <arch>aarch64</arch> <cpu type='kvm' name='pxa262'/> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.replies b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.replies index 1bd1baa8a8..4f819150fe 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.replies +++ b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.replies @@ -5637,10 +5637,179 @@ } { - "execute": "query-machines", + "execute": "qom-list-properties", + "arguments": { + "typename": "spapr-machine" + }, + "id": "libvirt-38" +} + +{ + "return": [ + { + "name": "graphics", + "description": "Set on/off to enable/disable graphics emulation", + "type": "bool" + }, + { + "name": "phandle-start", + "description": "The first phandle ID we may generate dynamically", + "type": "int" + }, + { + "name": "dump-guest-core", + "description": "Include guest memory in a core dump", + "type": "bool" + }, + { + "name": "kernel-irqchip", + "description": "Configure KVM in-kernel irqchip", + "type": "on|off|split" + }, + { + "name": "accel", + "description": "Accelerator list", + "type": "string" + }, + { + "name": "append", + "description": "Linux kernel command line", + "type": "string" + }, + { + "name": "dumpdtb", + "description": "Dump current dtb to a file and quit", + "type": "string" + }, + { + "name": "memory-encryption", + "description": "Set memory encyption object to use", + "type": "string" + }, + { + "name": "igd-passthru", + "description": "Set on/off to enable/disable igd passthrou", + "type": "bool" + }, + { + "name": "dt-compatible", + "description": "Overrides the \"compatible\" property of the dt root node", + "type": "string" + }, + { + "name": "kernel", + "description": "Linux kernel image file", + "type": "string" + }, + { + "name": "usb", + "description": "Set on/off to enable/disable usb", + "type": "bool" + }, + { + "name": "suppress-vmdesc", + "description": "Set on to disable self-describing migration", + "type": "bool" + }, + { + "name": "dtb", + "description": "Linux kernel device tree file", + "type": "string" + }, + { + "name": "firmware", + "description": "Firmware image", + "type": "string" + }, + { + "name": "mem-merge", + "description": "Enable/disable memory merge support", + "type": "bool" + }, + { + "name": "initrd", + "description": "Linux initial ramdisk file", + "type": "string" + }, + { + "name": "enforce-config-section", + "description": "Set on to enforce configuration section migration", + "type": "bool" + }, + { + "name": "kvm-shadow-mem", + "description": "KVM shadow MMU size", + "type": "int" + }, + { + "name": "cap-ibs", + "description": "Indirect Branch Speculation (broken, fixed-ibs, fixed-ccd)", + "type": "string" + }, + { + "name": "cap-cfpc", + "description": "Cache Flush on Privilege Change (broken, workaround, fixed)", + "type": "string" + }, + { + "name": "cap-sbbc", + "description": "Speculation Barrier Bounds Checking (broken, workaround, fixed)", + "type": "string" + }, + { + "name": "cap-dfp", + "description": "Allow Decimal Floating Point (DFP)", + "type": "bool" + }, + { + "name": "cap-htm", + "description": "Allow Hardware Transactional Memory (HTM)", + "type": "bool" + }, + { + "name": "cap-vsx", + "description": "Allow Vector Scalar Extensions (VSX)", + "type": "bool" + }, + { + "name": "cap-ibs", + "description": "Indirect Branch Speculation (broken, fixed-ibs, fixed-ccd)", + "type": "string" + }, + { + "name": "cap-cfpc", + "description": "Cache Flush on Privilege Change (broken, workaround, fixed)", + "type": "string" + }, + { + "name": "cap-sbbc", + "description": "Speculation Barrier Bounds Checking (broken, workaround, fixed)", + "type": "string" + }, + { + "name": "cap-dfp", + "description": "Allow Decimal Floating Point (DFP)", + "type": "bool" + }, + { + "name": "cap-htm", + "description": "Allow Hardware Transactional Memory (HTM)", + "type": "bool" + }, + { + "name": "cap-vsx", + "description": "Allow Vector Scalar Extensions (VSX)", + "type": "bool" + } + ], "id": "libvirt-38" } +{ + "execute": "query-machines", + "id": "libvirt-39" +} + { "return": [ { @@ -5776,12 +5945,12 @@ "cpu-max": 1 } ], - "id": "libvirt-38" + "id": "libvirt-39" } { "execute": "query-cpu-definitions", - "id": "libvirt-39" + "id": "libvirt-40" } { @@ -7977,35 +8146,35 @@ "static": false } ], - "id": "libvirt-39" + "id": "libvirt-40" } { "execute": "query-tpm-models", - "id": "libvirt-40" + "id": "libvirt-41" } { "return": [ ], - "id": "libvirt-40" + "id": "libvirt-41" } { "execute": "query-tpm-types", - "id": "libvirt-41" + "id": "libvirt-42" } { "return": [ "emulator" ], - "id": "libvirt-41" + "id": "libvirt-42" } { "execute": "query-command-line-options", - "id": "libvirt-42" + "id": "libvirt-43" } { @@ -9161,12 +9330,12 @@ "option": "drive" } ], - "id": "libvirt-42" + "id": "libvirt-43" } { "execute": "query-migrate-capabilities", - "id": "libvirt-43" + "id": "libvirt-44" } { @@ -9228,12 +9397,12 @@ "capability": "dirty-bitmaps" } ], - "id": "libvirt-43" + "id": "libvirt-44" } { "execute": "query-qmp-schema", - "id": "libvirt-44" + "id": "libvirt-45" } { @@ -20601,7 +20770,7 @@ "meta-type": "object" } ], - "id": "libvirt-44" + "id": "libvirt-45" } { diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml index c61029e479..2ee582f343 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml @@ -168,7 +168,7 @@ <flag name='tpm-emulator'/> <version>2011090</version> <kvmVersion>0</kvmVersion> - <microcodeVersion>424244</microcodeVersion> + <microcodeVersion>428334</microcodeVersion> <package>v2.12.0-rc0</package> <arch>ppc64</arch> <cpu type='kvm' name='default'/> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.replies b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.replies index fb57bcfd23..0a028a2fe6 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.replies +++ b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.replies @@ -3936,10 +3936,26 @@ } { - "execute": "query-machines", + "execute": "qom-list-properties", + "arguments": { + "typename": "spapr-machine" + }, "id": "libvirt-37" } +{ + "id": "libvirt-37", + "error": { + "class": "DeviceNotFound", + "desc": "Class 'spapr-machine' not found" + } +} + +{ + "execute": "query-machines", + "id": "libvirt-38" +} + { "return": [ { @@ -3995,12 +4011,12 @@ "alias": "s390-ccw-virtio" } ], - "id": "libvirt-37" + "id": "libvirt-38" } { "execute": "query-cpu-definitions", - "id": "libvirt-38" + "id": "libvirt-39" } { @@ -4535,35 +4551,35 @@ "migration-safe": true } ], - "id": "libvirt-38" + "id": "libvirt-39" } { "execute": "query-tpm-models", - "id": "libvirt-39" + "id": "libvirt-40" } { "return": [ ], - "id": "libvirt-39" + "id": "libvirt-40" } { "execute": "query-tpm-types", - "id": "libvirt-40" + "id": "libvirt-41" } { "return": [ "emulator" ], - "id": "libvirt-40" + "id": "libvirt-41" } { "execute": "query-command-line-options", - "id": "libvirt-41" + "id": "libvirt-42" } { @@ -5688,12 +5704,12 @@ "option": "drive" } ], - "id": "libvirt-41" + "id": "libvirt-42" } { "execute": "query-migrate-capabilities", - "id": "libvirt-42" + "id": "libvirt-43" } { @@ -5755,12 +5771,12 @@ "capability": "dirty-bitmaps" } ], - "id": "libvirt-42" + "id": "libvirt-43" } { "execute": "query-qmp-schema", - "id": "libvirt-43" + "id": "libvirt-44" } { @@ -17128,7 +17144,7 @@ "meta-type": "object" } ], - "id": "libvirt-43" + "id": "libvirt-44" } { @@ -17139,7 +17155,7 @@ "name": "host" } }, - "id": "libvirt-44" + "id": "libvirt-45" } { @@ -17177,7 +17193,7 @@ } } }, - "id": "libvirt-44" + "id": "libvirt-45" } { @@ -17191,11 +17207,11 @@ } } }, - "id": "libvirt-45" + "id": "libvirt-46" } { - "id": "libvirt-45", + "id": "libvirt-46", "error": { "class": "GenericError", "desc": "Property '.migratable' not found" diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml index 6fac6d6906..87d189e58d 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml @@ -135,7 +135,7 @@ <flag name='tpm-emulator'/> <version>2012000</version> <kvmVersion>0</kvmVersion> - <microcodeVersion>375762</microcodeVersion> + <microcodeVersion>375999</microcodeVersion> <package></package> <arch>s390x</arch> <hostCPU type='kvm' model='z14-base' migratability='no'> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.replies index 73365126f1..6f37e4301e 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.replies @@ -4995,10 +4995,26 @@ } { - "execute": "query-machines", + "execute": "qom-list-properties", + "arguments": { + "typename": "spapr-machine" + }, "id": "libvirt-42" } +{ + "id": "libvirt-42", + "error": { + "class": "DeviceNotFound", + "desc": "Class 'spapr-machine' not found" + } +} + +{ + "execute": "query-machines", + "id": "libvirt-43" +} + { "return": [ { @@ -5195,12 +5211,12 @@ "cpu-max": 255 } ], - "id": "libvirt-42" + "id": "libvirt-43" } { "execute": "query-cpu-definitions", - "id": "libvirt-43" + "id": "libvirt-44" } { @@ -5714,12 +5730,12 @@ "migration-safe": true } ], - "id": "libvirt-43" + "id": "libvirt-44" } { "execute": "query-tpm-models", - "id": "libvirt-44" + "id": "libvirt-45" } { @@ -5727,12 +5743,12 @@ "tpm-crb", "tpm-tis" ], - "id": "libvirt-44" + "id": "libvirt-45" } { "execute": "query-tpm-types", - "id": "libvirt-45" + "id": "libvirt-46" } { @@ -5740,12 +5756,12 @@ "passthrough", "emulator" ], - "id": "libvirt-45" + "id": "libvirt-46" } { "execute": "query-command-line-options", - "id": "libvirt-46" + "id": "libvirt-47" } { @@ -7032,12 +7048,12 @@ "option": "drive" } ], - "id": "libvirt-46" + "id": "libvirt-47" } { "execute": "query-migrate-capabilities", - "id": "libvirt-47" + "id": "libvirt-48" } { @@ -7099,12 +7115,12 @@ "capability": "dirty-bitmaps" } ], - "id": "libvirt-47" + "id": "libvirt-48" } { "execute": "query-qmp-schema", - "id": "libvirt-48" + "id": "libvirt-49" } { @@ -18472,7 +18488,7 @@ "meta-type": "object" } ], - "id": "libvirt-48" + "id": "libvirt-49" } { @@ -18483,7 +18499,7 @@ "name": "host" } }, - "id": "libvirt-49" + "id": "libvirt-50" } { @@ -18673,7 +18689,7 @@ } } }, - "id": "libvirt-49" + "id": "libvirt-50" } { @@ -18865,7 +18881,7 @@ } } }, - "id": "libvirt-50" + "id": "libvirt-51" } { @@ -19120,7 +19136,7 @@ } } }, - "id": "libvirt-50" + "id": "libvirt-51" } { @@ -19134,7 +19150,7 @@ } } }, - "id": "libvirt-51" + "id": "libvirt-52" } { @@ -19324,7 +19340,7 @@ } } }, - "id": "libvirt-51" + "id": "libvirt-52" } { @@ -19516,7 +19532,7 @@ } } }, - "id": "libvirt-52" + "id": "libvirt-53" } { @@ -19771,12 +19787,12 @@ } } }, - "id": "libvirt-52" + "id": "libvirt-53" } { "execute": "query-sev-capabilities", - "id": "libvirt-53" + "id": "libvirt-54" } { @@ -19786,7 +19802,7 @@ "cert-chain": "AQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAA", "pdh": "AQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAA" }, - "id": "libvirt-52" + "id": "libvirt-54" } { diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml index 2167f06976..9c1f6c327c 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml @@ -213,7 +213,7 @@ <flag name='sev-guest'/> <version>2011090</version> <kvmVersion>0</kvmVersion> - <microcodeVersion>415959</microcodeVersion> + <microcodeVersion>416196</microcodeVersion> <package>v2.12.0-rc0</package> <arch>x86_64</arch> <hostCPU type='kvm' model='base' migratability='yes'> diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.replies b/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.replies index 6aa99733ee..f759823f6c 100644 --- a/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.replies +++ b/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.replies @@ -5720,10 +5720,189 @@ } { - "execute": "query-machines", + "execute": "qom-list-properties", + "arguments": { + "typename": "spapr-machine" + }, + "id": "libvirt-38" +} + +{ + "return": [ + { + "name": "graphics", + "description": "Set on/off to enable/disable graphics emulation", + "type": "bool" + }, + { + "name": "phandle-start", + "description": "The first phandle ID we may generate dynamically", + "type": "int" + }, + { + "name": "dump-guest-core", + "description": "Include guest memory in a core dump", + "type": "bool" + }, + { + "name": "kernel-irqchip", + "description": "Configure KVM in-kernel irqchip", + "type": "on|off|split" + }, + { + "name": "accel", + "description": "Accelerator list", + "type": "string" + }, + { + "name": "append", + "description": "Linux kernel command line", + "type": "string" + }, + { + "name": "dumpdtb", + "description": "Dump current dtb to a file and quit", + "type": "string" + }, + { + "name": "memory-encryption", + "description": "Set memory encyption object to use", + "type": "string" + }, + { + "name": "igd-passthru", + "description": "Set on/off to enable/disable igd passthrou", + "type": "bool" + }, + { + "name": "dt-compatible", + "description": "Overrides the \"compatible\" property of the dt root node", + "type": "string" + }, + { + "name": "kernel", + "description": "Linux kernel image file", + "type": "string" + }, + { + "name": "usb", + "description": "Set on/off to enable/disable usb", + "type": "bool" + }, + { + "name": "suppress-vmdesc", + "description": "Set on to disable self-describing migration", + "type": "bool" + }, + { + "name": "dtb", + "description": "Linux kernel device tree file", + "type": "string" + }, + { + "name": "firmware", + "description": "Firmware image", + "type": "string" + }, + { + "name": "mem-merge", + "description": "Enable/disable memory merge support", + "type": "bool" + }, + { + "name": "initrd", + "description": "Linux initial ramdisk file", + "type": "string" + }, + { + "name": "enforce-config-section", + "description": "Set on to enforce configuration section migration", + "type": "bool" + }, + { + "name": "kvm-shadow-mem", + "description": "KVM shadow MMU size", + "type": "int" + }, + { + "name": "cap-hpt-max-page-size", + "description": "Maximum page size for Hash Page Table guests", + "type": "int" + }, + { + "name": "cap-ibs", + "description": "Indirect Branch Speculation (broken, fixed-ibs, fixed-ccd)", + "type": "string" + }, + { + "name": "cap-cfpc", + "description": "Cache Flush on Privilege Change (broken, workaround, fixed)", + "type": "string" + }, + { + "name": "cap-sbbc", + "description": "Speculation Barrier Bounds Checking (broken, workaround, fixed)", + "type": "string" + }, + { + "name": "cap-dfp", + "description": "Allow Decimal Floating Point (DFP)", + "type": "bool" + }, + { + "name": "cap-htm", + "description": "Allow Hardware Transactional Memory (HTM)", + "type": "bool" + }, + { + "name": "cap-vsx", + "description": "Allow Vector Scalar Extensions (VSX)", + "type": "bool" + }, + { + "name": "cap-hpt-max-page-size", + "description": "Maximum page size for Hash Page Table guests", + "type": "int" + }, + { + "name": "cap-ibs", + "description": "Indirect Branch Speculation (broken, fixed-ibs, fixed-ccd)", + "type": "string" + }, + { + "name": "cap-cfpc", + "description": "Cache Flush on Privilege Change (broken, workaround, fixed)", + "type": "string" + }, + { + "name": "cap-sbbc", + "description": "Speculation Barrier Bounds Checking (broken, workaround, fixed)", + "type": "string" + }, + { + "name": "cap-dfp", + "description": "Allow Decimal Floating Point (DFP)", + "type": "bool" + }, + { + "name": "cap-htm", + "description": "Allow Hardware Transactional Memory (HTM)", + "type": "bool" + }, + { + "name": "cap-vsx", + "description": "Allow Vector Scalar Extensions (VSX)", + "type": "bool" + } + ], "id": "libvirt-38" } +{ + "execute": "query-machines", + "id": "libvirt-39" +} + { "return": [ { @@ -5864,12 +6043,12 @@ "cpu-max": 1 } ], - "id": "libvirt-38" + "id": "libvirt-39" } { "execute": "query-cpu-definitions", - "id": "libvirt-39" + "id": "libvirt-40" } { @@ -8065,35 +8244,35 @@ "static": false } ], - "id": "libvirt-39" + "id": "libvirt-40" } { "execute": "query-tpm-models", - "id": "libvirt-40" + "id": "libvirt-41" } { "return": [ ], - "id": "libvirt-40" + "id": "libvirt-41" } { "execute": "query-tpm-types", - "id": "libvirt-41" + "id": "libvirt-42" } { "return": [ "emulator" ], - "id": "libvirt-41" + "id": "libvirt-42" } { "execute": "query-command-line-options", - "id": "libvirt-42" + "id": "libvirt-43" } { @@ -9223,12 +9402,12 @@ "option": "drive" } ], - "id": "libvirt-42" + "id": "libvirt-43" } { "execute": "query-migrate-capabilities", - "id": "libvirt-43" + "id": "libvirt-44" } { @@ -9298,12 +9477,12 @@ "capability": "late-block-activate" } ], - "id": "libvirt-43" + "id": "libvirt-44" } { "execute": "query-qmp-schema", - "id": "libvirt-44" + "id": "libvirt-45" } { @@ -21462,7 +21641,7 @@ "meta-type": "object" } ], - "id": "libvirt-44" + "id": "libvirt-45" } { diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml index c98db19815..7e958b2efc 100644 --- a/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml @@ -165,9 +165,10 @@ <flag name='vhost-vsock'/> <flag name='chardev-fd-pass'/> <flag name='tpm-emulator'/> + <flag name='machine.pseries.cap-hpt-max-page-size'/> <version>2012050</version> <kvmVersion>0</kvmVersion> - <microcodeVersion>442399</microcodeVersion> + <microcodeVersion>446771</microcodeVersion> <package>v2.12.0-1689-g518d23a</package> <arch>ppc64</arch> <cpu type='kvm' name='default'/> -- 2.17.1
We're going to introduce a second HPT-related setting soon, at which point using a single location to store everything is no longer going to cut it. This mostly, but not completely, reverts 3dd1eb3b2650. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/domain_conf.c | 21 ++++++++++++++------- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 7 ++++--- src/qemu/qemu_domain.c | 2 +- 4 files changed, 20 insertions(+), 11 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f0b604e125..069ff8c2f8 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -19803,9 +19803,12 @@ virDomainDefParseXML(xmlDocPtr xml, tmp); goto error; } - def->features[val] = value; + def->hpt_resizing = (virDomainHPTResizing) value; VIR_FREE(tmp); } + + if (def->hpt_resizing != VIR_DOMAIN_HPT_RESIZING_NONE) + def->features[val] = VIR_TRISTATE_SWITCH_ON; break; /* coverity[dead_error_begin] */ @@ -21983,13 +21986,16 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src, break; case VIR_DOMAIN_FEATURE_HPT: - if (src->features[i] != dst->features[i]) { + if (src->features[i] != dst->features[i] || + src->hpt_resizing != dst->hpt_resizing) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("State of feature '%s' differs: " - "source: '%s=%s', destination: '%s=%s'"), + "source: '%s,%s=%s', destination: '%s,%s=%s'"), featureName, - "resizing", virDomainHPTResizingTypeToString(src->features[i]), - "resizing", virDomainHPTResizingTypeToString(dst->features[i])); + virTristateSwitchTypeToString(src->features[i]), + "resizing", virDomainHPTResizingTypeToString(src->hpt_resizing), + virTristateSwitchTypeToString(dst->features[i]), + "resizing", virDomainHPTResizingTypeToString(dst->hpt_resizing)); return false; } break; @@ -27767,11 +27773,12 @@ virDomainDefFormatInternal(virDomainDefPtr def, break; case VIR_DOMAIN_FEATURE_HPT: - if (def->features[i] == VIR_DOMAIN_HPT_RESIZING_NONE) + if (def->features[i] != VIR_TRISTATE_SWITCH_ON || + def->hpt_resizing == VIR_DOMAIN_HPT_RESIZING_NONE) break; virBufferAsprintf(buf, "<hpt resizing='%s'/>\n", - virDomainHPTResizingTypeToString(def->features[i])); + virDomainHPTResizingTypeToString(def->hpt_resizing)); break; /* coverity[dead_error_begin] */ diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 8cefef535a..39fa2bc35a 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2438,6 +2438,7 @@ struct _virDomainDef { int kvm_features[VIR_DOMAIN_KVM_LAST]; unsigned int hyperv_spinlocks; virGICVersion gic_version; + virDomainHPTResizing hpt_resizing; char *hyperv_vendor_id; int apic_eoi; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 446df3e1d8..081314ed0a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7244,17 +7244,18 @@ qemuBuildMachineCommandLine(virCommandPtr cmd, } } - if (def->features[VIR_DOMAIN_FEATURE_HPT] != VIR_DOMAIN_HPT_RESIZING_NONE) { + if (def->features[VIR_DOMAIN_FEATURE_HPT] == VIR_TRISTATE_SWITCH_ON) { const char *str; - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT)) { + if (def->hpt_resizing != VIR_DOMAIN_HPT_RESIZING_NONE && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("HTP resizing is not supported by this " "QEMU binary")); goto cleanup; } - str = virDomainHPTResizingTypeToString(def->features[VIR_DOMAIN_FEATURE_HPT]); + str = virDomainHPTResizingTypeToString(def->hpt_resizing); if (!str) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Invalid setting for HPT resizing")); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 95c3e3a8aa..6d203e1f2e 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3812,7 +3812,7 @@ qemuDomainDefValidateFeatures(const virDomainDef *def, break; case VIR_DOMAIN_FEATURE_HPT: - if (def->features[i] != VIR_DOMAIN_HPT_RESIZING_NONE && + if (def->features[i] != VIR_TRISTATE_SWITCH_ABSENT && !qemuDomainIsPSeries(def)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("The '%s' feature is not supported for " -- 2.17.1
On 06/25/2018 07:11 PM, Andrea Bolognani wrote:
We're going to introduce a second HPT-related setting soon, at which point using a single location to store everything is no longer going to cut it.
This mostly, but not completely, reverts 3dd1eb3b2650.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/domain_conf.c | 21 ++++++++++++++------- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 7 ++++--- src/qemu/qemu_domain.c | 2 +- 4 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f0b604e125..069ff8c2f8 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -19803,9 +19803,12 @@ virDomainDefParseXML(xmlDocPtr xml, tmp); goto error; } - def->features[val] = value; + def->hpt_resizing = (virDomainHPTResizing) value;
This typecast seems useless. Also, pre-existing, the if() statement above is more verbose than it needs to be since VIR_DOMAIN_HPT_RESIZING_NONE is defined to be zero.
VIR_FREE(tmp); } + + if (def->hpt_resizing != VIR_DOMAIN_HPT_RESIZING_NONE) + def->features[val] = VIR_TRISTATE_SWITCH_ON; break;
/* coverity[dead_error_begin] */
Michal
On Tue, 2018-06-26 at 09:18 +0200, Michal Privoznik wrote:
On 06/25/2018 07:11 PM, Andrea Bolognani wrote:
@@ -19803,9 +19803,12 @@ virDomainDefParseXML(xmlDocPtr xml, tmp); goto error; } - def->features[val] = value; + def->hpt_resizing = (virDomainHPTResizing) value;
This typecast seems useless. Also, pre-existing, the if() statement above is more verbose than it needs to be since VIR_DOMAIN_HPT_RESIZING_NONE is defined to be zero.
I agree the cast is mostly unnecessary here, but it doesn't cause any issues either and as a personal preference I like being explicit about this kind of conversion :) As for the if() statement above, which looks like int value = virDomainHPTResizingTypeFromString(tmp); if (value < 0 || value == VIR_DOMAIN_HPT_RESIZING_NONE) { ... you could definitely rewrite the condition as 'value <= 0' and get the same behavior, but you would make the code more opaque as a result. Right now it doesn't take much effort to figure out what the code above does: it very obviouly tries to convert a string to an enum, and errors out if either the conversion fails entirely or the returned value is one that we don't want the user to specify. If rewritten, the two error conditions would be smushed together and the developer would need to unpack them in their head, possibly after looking up the virDomainHPTResizing enum and confirming 0 corresponds to a value that it would probably make sense to blacklist when parsing the configuration. The trade-off is very much not worth it just to save a few dozen characters, in my opinion. -- Andrea Bolognani / Red Hat / Virtualization
On 06/26/2018 10:19 AM, Andrea Bolognani wrote:
On Tue, 2018-06-26 at 09:18 +0200, Michal Privoznik wrote:
On 06/25/2018 07:11 PM, Andrea Bolognani wrote:
@@ -19803,9 +19803,12 @@ virDomainDefParseXML(xmlDocPtr xml, tmp); goto error; } - def->features[val] = value; + def->hpt_resizing = (virDomainHPTResizing) value;
This typecast seems useless. Also, pre-existing, the if() statement above is more verbose than it needs to be since VIR_DOMAIN_HPT_RESIZING_NONE is defined to be zero.
I agree the cast is mostly unnecessary here, but it doesn't cause any issues either and as a personal preference I like being explicit about this kind of conversion :)
As for the if() statement above, which looks like
int value = virDomainHPTResizingTypeFromString(tmp); if (value < 0 || value == VIR_DOMAIN_HPT_RESIZING_NONE) { ...
you could definitely rewrite the condition as 'value <= 0' and get the same behavior, but you would make the code more opaque as a result.
Right now it doesn't take much effort to figure out what the code above does: it very obviouly tries to convert a string to an enum, and errors out if either the conversion fails entirely or the returned value is one that we don't want the user to specify.
If rewritten, the two error conditions would be smushed together and the developer would need to unpack them in their head, possibly after looking up the virDomainHPTResizing enum and confirming 0 corresponds to a value that it would probably make sense to blacklist when parsing the configuration.
The trade-off is very much not worth it just to save a few dozen characters, in my opinion.
The thing is, we are not consistent. On many places we use short version and in fewer places (my feeling, haven't counted them) we are using this explicit longer version. Also, usually the enums are designed that way so the first element (if set to zero) means 'nada', unset value so that we can do checks like this later in the code: if (def->enum == 0) { /* unset */ } So by looking at the code I can usually tell if the first element is supposed to be accepted or not: if (value < 0) ... if (value <= 0) ... Either works, it's just that we ought to be consistent. But since you're not touching that line anyway, it's okay to leave it as is and say 'pre-existing :-P'. Michal
This doesn't seem very useful at the moment, but it will make sense once we introduce another HPT-related setting. The output XML is decoupled from the input XML in preparation of future changes as well; while doing so, we can shave a few lines off the latter. This commit is best viewed with 'git show -w'. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/domain_conf.c | 19 ++++++++--- src/qemu/qemu_command.c | 32 ++++++++++--------- tests/qemuxml2argvdata/pseries-features.xml | 14 ++------ tests/qemuxml2xmloutdata/pseries-features.xml | 29 ++++++++++++++++- 4 files changed, 62 insertions(+), 32 deletions(-) mode change 120000 => 100644 tests/qemuxml2xmloutdata/pseries-features.xml diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 069ff8c2f8..3f7b0d1bfe 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -27258,6 +27258,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, const char *type = NULL; int n; size_t i; + virBuffer attributeBuf = VIR_BUFFER_INITIALIZER; virBuffer childrenBuf = VIR_BUFFER_INITIALIZER; char *netprefix = NULL; @@ -27773,12 +27774,21 @@ virDomainDefFormatInternal(virDomainDefPtr def, break; case VIR_DOMAIN_FEATURE_HPT: - if (def->features[i] != VIR_TRISTATE_SWITCH_ON || - def->hpt_resizing == VIR_DOMAIN_HPT_RESIZING_NONE) + if (def->features[i] != VIR_TRISTATE_SWITCH_ON) break; - virBufferAsprintf(buf, "<hpt resizing='%s'/>\n", - virDomainHPTResizingTypeToString(def->hpt_resizing)); + virBufferFreeAndReset(&attributeBuf); + + if (def->hpt_resizing != VIR_DOMAIN_HPT_RESIZING_NONE) { + virBufferAsprintf(&attributeBuf, + " resizing='%s'", + virDomainHPTResizingTypeToString(def->hpt_resizing)); + } + + if (virXMLFormatElement(buf, "hpt", + &attributeBuf, NULL) < 0) { + goto error; + } break; /* coverity[dead_error_begin] */ @@ -28040,6 +28050,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, error: virBufferFreeAndReset(buf); virBufferFreeAndReset(&childrenBuf); + virBufferFreeAndReset(&attributeBuf); return -1; } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 081314ed0a..5cd6d44a88 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7245,24 +7245,26 @@ qemuBuildMachineCommandLine(virCommandPtr cmd, } if (def->features[VIR_DOMAIN_FEATURE_HPT] == VIR_TRISTATE_SWITCH_ON) { - const char *str; - if (def->hpt_resizing != VIR_DOMAIN_HPT_RESIZING_NONE && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("HTP resizing is not supported by this " - "QEMU binary")); - goto cleanup; - } + if (def->hpt_resizing != VIR_DOMAIN_HPT_RESIZING_NONE) { + const char *str; - str = virDomainHPTResizingTypeToString(def->hpt_resizing); - if (!str) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Invalid setting for HPT resizing")); - goto cleanup; - } + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("HTP resizing is not supported by this " + "QEMU binary")); + goto cleanup; + } + + str = virDomainHPTResizingTypeToString(def->hpt_resizing); + if (!str) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Invalid setting for HPT resizing")); + goto cleanup; + } - virBufferAsprintf(&buf, ",resize-hpt=%s", str); + virBufferAsprintf(&buf, ",resize-hpt=%s", str); + } } if (cpu && cpu->model && diff --git a/tests/qemuxml2argvdata/pseries-features.xml b/tests/qemuxml2argvdata/pseries-features.xml index 5dd0dbd0be..5ef1a744c8 100644 --- a/tests/qemuxml2argvdata/pseries-features.xml +++ b/tests/qemuxml2argvdata/pseries-features.xml @@ -2,27 +2,17 @@ <name>guest</name> <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> <memory unit='KiB'>524288</memory> - <currentMemory unit='KiB'>524288</currentMemory> <vcpu placement='static'>1</vcpu> <os> <type arch='ppc64' machine='pseries'>hvm</type> - <boot dev='hd'/> </os> <features> <hpt resizing='required'/> </features> - <clock offset='utc'/> - <on_poweroff>destroy</on_poweroff> - <on_reboot>restart</on_reboot> - <on_crash>destroy</on_crash> <devices> <emulator>/usr/bin/qemu-system-ppc64</emulator> - <controller type='usb' index='0' model='none'/> - <controller type='pci' index='0' model='pci-root'> - <model name='spapr-pci-host-bridge'/> - <target index='0'/> - </controller> + <controller type='pci' model='pci-root'/> + <controller type='usb' model='none'/> <memballoon model='none'/> - <panic model='pseries'/> </devices> </domain> diff --git a/tests/qemuxml2xmloutdata/pseries-features.xml b/tests/qemuxml2xmloutdata/pseries-features.xml deleted file mode 120000 index 1b01dbace6..0000000000 --- a/tests/qemuxml2xmloutdata/pseries-features.xml +++ /dev/null @@ -1 +0,0 @@ -../qemuxml2argvdata/pseries-features.xml \ No newline at end of file diff --git a/tests/qemuxml2xmloutdata/pseries-features.xml b/tests/qemuxml2xmloutdata/pseries-features.xml new file mode 100644 index 0000000000..e8ed842fb6 --- /dev/null +++ b/tests/qemuxml2xmloutdata/pseries-features.xml @@ -0,0 +1,28 @@ +<domain type='qemu'> + <name>guest</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>524288</memory> + <currentMemory unit='KiB'>524288</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='ppc64' machine='pseries'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <hpt resizing='required'/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <controller type='pci' index='0' model='pci-root'> + <model name='spapr-pci-host-bridge'/> + <target index='0'/> + </controller> + <controller type='usb' index='0' model='none'/> + <memballoon model='none'/> + <panic model='pseries'/> + </devices> +</domain> -- 2.17.1
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- docs/formatdomain.html.in | 11 ++++-- docs/schemas/domaincommon.rng | 21 +++++++---- src/conf/domain_conf.c | 36 ++++++++++++++++--- src/conf/domain_conf.h | 1 + tests/qemuxml2argvdata/pseries-features.xml | 4 ++- tests/qemuxml2argvtest.c | 1 + tests/qemuxml2xmloutdata/pseries-features.xml | 4 ++- tests/qemuxml2xmltest.c | 1 + 8 files changed, 63 insertions(+), 16 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 89672a0486..1cf92cd755 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1922,7 +1922,9 @@ <pvspinlock state='on'/> <gic version='2'/> <ioapic driver='qemu'/> - <hpt resizing='required'/> + <hpt resizing='required'> + <maxpagesize unit='MiB'>16</maxpagesize> + </hpt> <vmcoreinfo state='on'/> <smm state='on'> <tseg unit='MiB'>48</tseg> @@ -2149,7 +2151,12 @@ support; and <code>required</code>, which prevents the guest from starting unless both the guest and the host support HPT resizing. If the attribute is not defined, the hypervisor default will be used. - <span class="since">Since 3.10.0</span> (QEMU/KVM only) + <span class="since">Since 3.10.0</span> (QEMU/KVM only). + + <p>The optional <code>maxpagesize</code> subelement can be used to + limit the usable page size for HPT guests. Common values are 64 KiB, + 16 MiB and 16 GiB; when not specified, the hypervisor default will + be used. <span class="since">Since 4.5.0</span> (QEMU/KVM only).</p> </dd> <dt><code>vmcoreinfo</code></dt> <dd>Enable QEMU vmcoreinfo device to let the guest kernel save debug diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 4a454dddb4..0a661cf1e7 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -5121,13 +5121,20 @@ <define name="hpt"> <element name="hpt"> - <attribute name="resizing"> - <choice> - <value>enabled</value> - <value>disabled</value> - <value>required</value> - </choice> - </attribute> + <optional> + <attribute name="resizing"> + <choice> + <value>enabled</value> + <value>disabled</value> + <value>required</value> + </choice> + </attribute> + </optional> + <optional> + <element name="maxpagesize"> + <ref name='scaledInteger'/> + </element> + </optional> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3f7b0d1bfe..d8cb7f37f3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -19807,8 +19807,24 @@ virDomainDefParseXML(xmlDocPtr xml, VIR_FREE(tmp); } - if (def->hpt_resizing != VIR_DOMAIN_HPT_RESIZING_NONE) + if (virDomainParseScaledValue("./features/hpt/maxpagesize", + NULL, + ctxt, + &def->hpt_maxpagesize, + 1024, + ULLONG_MAX, + false) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", + _("Unable to parse HPT maxpagesize setting")); + goto error; + } + def->hpt_maxpagesize = VIR_DIV_UP(def->hpt_maxpagesize, 1024); + + if (def->hpt_resizing != VIR_DOMAIN_HPT_RESIZING_NONE || + def->hpt_maxpagesize > 0) { def->features[val] = VIR_TRISTATE_SWITCH_ON; + } break; /* coverity[dead_error_begin] */ @@ -21987,15 +22003,18 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src, case VIR_DOMAIN_FEATURE_HPT: if (src->features[i] != dst->features[i] || - src->hpt_resizing != dst->hpt_resizing) { + src->hpt_resizing != dst->hpt_resizing || + src->hpt_maxpagesize != dst->hpt_maxpagesize) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("State of feature '%s' differs: " - "source: '%s,%s=%s', destination: '%s,%s=%s'"), + "source: '%s,%s=%s,%s=%llu', destination: '%s,%s=%s,%s=%llu'"), featureName, virTristateSwitchTypeToString(src->features[i]), "resizing", virDomainHPTResizingTypeToString(src->hpt_resizing), + "maxpagesize", src->hpt_maxpagesize, virTristateSwitchTypeToString(dst->features[i]), - "resizing", virDomainHPTResizingTypeToString(dst->hpt_resizing)); + "resizing", virDomainHPTResizingTypeToString(dst->hpt_resizing), + "maxpagesize", dst->hpt_maxpagesize); return false; } break; @@ -27778,15 +27797,22 @@ virDomainDefFormatInternal(virDomainDefPtr def, break; virBufferFreeAndReset(&attributeBuf); + virBufferFreeAndReset(&childrenBuf); if (def->hpt_resizing != VIR_DOMAIN_HPT_RESIZING_NONE) { virBufferAsprintf(&attributeBuf, " resizing='%s'", virDomainHPTResizingTypeToString(def->hpt_resizing)); } + if (def->hpt_maxpagesize > 0) { + virBufferSetChildIndent(&childrenBuf, buf); + virBufferAsprintf(&childrenBuf, + "<maxpagesize unit='KiB'>%llu</maxpagesize>\n", + def->hpt_maxpagesize); + } if (virXMLFormatElement(buf, "hpt", - &attributeBuf, NULL) < 0) { + &attributeBuf, &childrenBuf) < 0) { goto error; } break; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 39fa2bc35a..0924fc4f3c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2439,6 +2439,7 @@ struct _virDomainDef { unsigned int hyperv_spinlocks; virGICVersion gic_version; virDomainHPTResizing hpt_resizing; + unsigned long long hpt_maxpagesize; /* Stored in KiB */ char *hyperv_vendor_id; int apic_eoi; diff --git a/tests/qemuxml2argvdata/pseries-features.xml b/tests/qemuxml2argvdata/pseries-features.xml index 5ef1a744c8..30cee5b81c 100644 --- a/tests/qemuxml2argvdata/pseries-features.xml +++ b/tests/qemuxml2argvdata/pseries-features.xml @@ -7,7 +7,9 @@ <type arch='ppc64' machine='pseries'>hvm</type> </os> <features> - <hpt resizing='required'/> + <hpt resizing='required'> + <maxpagesize unit='GiB'>1</maxpagesize> + </hpt> </features> <devices> <emulator>/usr/bin/qemu-system-ppc64</emulator> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 582a9de7bb..ac9593acbe 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1849,6 +1849,7 @@ mymain(void) DO_TEST("pseries-features", QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, + QEMU_CAPS_MACHINE_PSERIES_CAP_HPT_MAX_PAGE_SIZE, QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT); DO_TEST_FAILURE("pseries-features", QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE); diff --git a/tests/qemuxml2xmloutdata/pseries-features.xml b/tests/qemuxml2xmloutdata/pseries-features.xml index e8ed842fb6..f36705f011 100644 --- a/tests/qemuxml2xmloutdata/pseries-features.xml +++ b/tests/qemuxml2xmloutdata/pseries-features.xml @@ -9,7 +9,9 @@ <boot dev='hd'/> </os> <features> - <hpt resizing='required'/> + <hpt resizing='required'> + <maxpagesize unit='KiB'>1048576</maxpagesize> + </hpt> </features> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 4449954ad4..eac6d5b073 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -619,6 +619,7 @@ mymain(void) DO_TEST("pseries-features", QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, + QEMU_CAPS_MACHINE_PSERIES_CAP_HPT_MAX_PAGE_SIZE, QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT); DO_TEST("pseries-serial-native", -- 2.17.1
This makes the feature fully functional. https://bugzilla.redhat.com/show_bug.cgi?id=1571078 Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_command.c | 12 ++++++++++++ tests/qemuxml2argvdata/pseries-features.args | 3 ++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5cd6d44a88..748b596f63 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7265,6 +7265,18 @@ qemuBuildMachineCommandLine(virCommandPtr cmd, virBufferAsprintf(&buf, ",resize-hpt=%s", str); } + + if (def->hpt_maxpagesize > 0) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_CAP_HPT_MAX_PAGE_SIZE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Configuring the page size for HPT guests " + "is not supported by this QEMU binary")); + goto cleanup; + } + + virBufferAsprintf(&buf, ",cap-hpt-max-page-size=%lluk", + def->hpt_maxpagesize); + } } if (cpu && cpu->model && diff --git a/tests/qemuxml2argvdata/pseries-features.args b/tests/qemuxml2argvdata/pseries-features.args index f5c1959cca..12c14715c6 100644 --- a/tests/qemuxml2argvdata/pseries-features.args +++ b/tests/qemuxml2argvdata/pseries-features.args @@ -7,7 +7,8 @@ QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-ppc64 \ -name guest \ -S \ --machine pseries,accel=tcg,usb=off,dump-guest-core=off,resize-hpt=required \ +-machine pseries,accel=tcg,usb=off,dump-guest-core=off,resize-hpt=required,\ +cap-hpt-max-page-size=1048576k \ -m 512 \ -smp 1,sockets=1,cores=1,threads=1 \ -uuid 1ccfd97d-5eb4-478a-bbe6-88d254c16db7 \ -- 2.17.1
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- docs/news.xml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index f843648c75..a32d2b88a5 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -94,6 +94,17 @@ independent calls run at the same time. </description> </change> + <change> + <summary> + qemu: Allow configuring the page size for HPT pSeries guests + </summary> + <description> + For HPT pSeries guests, the size of the host pages used to back guest + memory and the usable guest page sizes are connected; the new setting + can be used to request that a certain page size is available in the + guest. + </description> + </change> </section> <section title="Bug fixes"> <change> -- 2.17.1
On 06/25/2018 07:11 PM, Andrea Bolognani wrote:
This applies cleanly on top of a0d6894af1b1.
Patch 1/7 is too big to go through the list; it can be fetched, along with the rest of the series, from [1].
Patch 2/7 conflicts with patch 1/3 from [2], but making this one depend on it didn't feel right; whichever of the two series gets reviewed and merged first, I'll probably post a rebase of the other one immediately afterwards.
Changes from [RFC]:
* the QEMU interface has changed based on feedback, which means some of the code is no longer useful and has been dropped; * switched to virXMLFormatElement(), as suggested during review; * added documentation and release notes entry.
[1] https://github.com/andreabolognani/libvirt/tree/pseries-hpt-maxpagesize
would be nice if the branch would be derived from the master you have there too ;-)
[2] https://www.redhat.com/archives/libvir-list/2018-June/msg01374.html [RFC] https://www.redhat.com/archives/libvir-list/2018-May/msg01710.html
Andrea Bolognani (7): tests: Add replies for QEMU 3.0.0 on ppc64 qemu: Add capability for the HPT maxpagesize feature conf: Reintroduce virDomainDef::hpt_resizing conf: Tweak HPT feature parsing and formatting conf: Parse and format HPT maxpagesize qemu: Format HPT maxpagesize on the command line news: Update for HPT maxpagesize feature
docs/formatdomain.html.in | 11 +- docs/news.xml | 11 + docs/schemas/domaincommon.rng | 21 +- src/conf/domain_conf.c | 60 +- src/conf/domain_conf.h | 2 + src/qemu/qemu_capabilities.c | 8 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 43 +- src/qemu/qemu_domain.c | 2 +- .../caps_2.12.0.aarch64.replies | 48 +- .../caps_2.12.0.aarch64.xml | 2 +- .../caps_2.12.0.ppc64.replies | 197 +- .../caps_2.12.0.ppc64.xml | 2 +- .../caps_2.12.0.s390x.replies | 52 +- .../caps_2.12.0.s390x.xml | 2 +- .../caps_2.12.0.x86_64.replies | 64 +- .../caps_2.12.0.x86_64.xml | 2 +- ...ppc64.replies => caps_3.0.0.ppc64.replies} | 5268 ++++++++++------- ..._2.12.0.ppc64.xml => caps_3.0.0.ppc64.xml} | 19 +- tests/qemucapabilitiestest.c | 1 + tests/qemuxml2argvdata/pseries-features.args | 3 +- tests/qemuxml2argvdata/pseries-features.xml | 18 +- tests/qemuxml2argvtest.c | 1 + tests/qemuxml2xmloutdata/pseries-features.xml | 31 +- tests/qemuxml2xmltest.c | 1 + 25 files changed, 3624 insertions(+), 2246 deletions(-) copy tests/qemucapabilitiesdata/{caps_2.12.0.ppc64.replies => caps_3.0.0.ppc64.replies} (92%) copy tests/qemucapabilitiesdata/{caps_2.12.0.ppc64.xml => caps_3.0.0.ppc64.xml} (99%) mode change 120000 => 100644 tests/qemuxml2xmloutdata/pseries-features.xml
ACK Michal
On Tue, 2018-06-26 at 09:18 +0200, Michal Privoznik wrote:
On 06/25/2018 07:11 PM, Andrea Bolognani wrote:
This applies cleanly on top of a0d6894af1b1.
Patch 1/7 is too big to go through the list; it can be fetched, along with the rest of the series, from [1]. [...] [1] https://github.com/andreabolognani/libvirt/tree/pseries-hpt-maxpagesize
would be nice if the branch would be derived from the master you have there too ;-)
Whoops, looks like I forgot to push the master branch... I even had some garbage there. Sorry about that.
ACK
Pushed, thanks :) -- Andrea Bolognani / Red Hat / Virtualization
participants (2)
-
Andrea Bolognani -
Michal Privoznik