[libvirt] [PATCH 0/3] add support for hugepages mapping as shared

- BLURB Martin Kletzander (3): schemas: finish virTristate{Bool,Switch} transition docs, conf, schema: add support for shared memory mapping qemu: add support for shared memory mapping docs/formatdomain.html.in | 7 +- docs/schemas/basictypes.rng | 19 ++- docs/schemas/capability.rng | 10 +- docs/schemas/domaincaps.rng | 5 +- docs/schemas/domaincommon.rng | 160 +++++---------------- docs/schemas/interface.rng | 19 +-- docs/schemas/network.rng | 29 +--- docs/schemas/nwfilter.rng | 5 +- docs/schemas/secret.rng | 10 +- src/conf/cpu_conf.c | 25 +++- src/conf/cpu_conf.h | 7 +- src/qemu/qemu_command.c | 19 +++ .../qemuxml2argv-cpu-numa-memshared.xml | 28 ++++ .../qemuxml2argv-hugepages-shared.args | 16 +++ .../qemuxml2argv-hugepages-shared.xml | 45 ++++++ tests/qemuxml2argvtest.c | 5 + tests/qemuxml2xmltest.c | 2 + 17 files changed, 214 insertions(+), 197 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.xml -- 2.1.0

Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- docs/schemas/basictypes.rng | 19 ++++-- docs/schemas/capability.rng | 10 +-- docs/schemas/domaincaps.rng | 5 +- docs/schemas/domaincommon.rng | 155 +++++++++--------------------------------- docs/schemas/interface.rng | 19 +----- docs/schemas/network.rng | 29 ++------ docs/schemas/nwfilter.rng | 5 +- docs/schemas/secret.rng | 10 +-- 8 files changed, 61 insertions(+), 191 deletions(-) diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index 75d5238..d26da57 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -77,10 +77,7 @@ </attribute> <optional> <attribute name="multifunction"> - <choice> - <value>on</value> - <value>off</value> - </choice> + <ref name="virSwitch"/> </attribute> </optional> </define> @@ -446,4 +443,18 @@ </optional> </define> + <define name="virBool"> + <choice> + <value>yes</value> + <value>no</value> + </choice> + </define> + + <define name="virSwitch"> + <choice> + <value>on</value> + <value>off</value> + </choice> + </define> + </grammar> diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng index f954599..65a8a0d 100644 --- a/docs/schemas/capability.rng +++ b/docs/schemas/capability.rng @@ -405,16 +405,10 @@ <define name='featuretoggle'> <attribute name='toggle'> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> <attribute name='default'> - <choice> - <value>on</value> - <value>off</value> - </choice> + <ref name="virSwitch"/> </attribute> </define> diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng index 627b699..bc36a28 100644 --- a/docs/schemas/domaincaps.rng +++ b/docs/schemas/domaincaps.rng @@ -66,10 +66,7 @@ <define name='supported'> <attribute name='supported'> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </define> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index cedceae..25ff386 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -118,10 +118,7 @@ </attribute> <optional> <attribute name='relabel'> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <interleave> @@ -254,10 +251,7 @@ <optional> <element name="bootmenu"> <attribute name="enable"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> <optional> <attribute name="timeout"> @@ -556,10 +550,7 @@ <ref name='scaledInteger'/> <optional> <attribute name="dumpCore"> - <choice> - <value>on</value> - <value>off</value> - </choice> + <ref name="virSwitch"/> </attribute> </optional> </element> @@ -972,10 +963,7 @@ </choice> <optional> <attribute name="present"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <empty/> @@ -1225,10 +1213,7 @@ </attribute> <optional> <attribute name="rawio"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <optional> @@ -1496,10 +1481,7 @@ </optional> <optional> <attribute name="removable"> - <choice> - <value>on</value> - <value>off</value> - </choice> + <ref name="virSwitch"/> </attribute> </optional> </element> @@ -1632,26 +1614,17 @@ </define> <define name="ioeventfd"> <attribute name="ioeventfd"> - <choice> - <value>on</value> - <value>off</value> - </choice> + <ref name="virSwitch"/> </attribute> </define> <define name="event_idx"> <attribute name="event_idx"> - <choice> - <value>on</value> - <value>off</value> - </choice> + <ref name="virSwitch"/> </attribute> </define> <define name="copy_on_read"> <attribute name='copy_on_read'> - <choice> - <value>on</value> - <value>off</value> - </choice> + <ref name="virSwitch"/> </attribute> </define> <define name="discard"> @@ -2182,20 +2155,14 @@ </attribute> <optional> <attribute name="managed"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <interleave> <element name="source"> <optional> <attribute name="missing"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <choice> @@ -2418,10 +2385,7 @@ </optional> <optional> <attribute name="fullscreen"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> </group> @@ -2438,10 +2402,7 @@ </optional> <optional> <attribute name="autoport"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <optional> @@ -2512,10 +2473,7 @@ </optional> <optional> <attribute name="autoport"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <optional> @@ -2624,10 +2582,7 @@ <optional> <element name="playback"> <attribute name="compression"> - <choice> - <value>on</value> - <value>off</value> - </choice> + <ref name="virSwitch"/> </attribute> <empty/> </element> @@ -2647,10 +2602,7 @@ <optional> <element name="clipboard"> <attribute name="copypaste"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> <empty/> </element> @@ -2669,10 +2621,7 @@ <optional> <element name="filetransfer"> <attribute name="enable"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> <empty/> </element> @@ -2690,26 +2639,17 @@ </optional> <optional> <attribute name="autoport"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <optional> <attribute name="replaceUser"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <optional> <attribute name="multiUser"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <optional> @@ -2730,10 +2670,7 @@ </optional> <optional> <attribute name="fullscreen"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> </group> @@ -2812,28 +2749,19 @@ </optional> <optional> <attribute name="primary"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <optional> <element name="acceleration"> <optional> <attribute name="accel3d"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <optional> <attribute name="accel2d"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> </element> @@ -2959,10 +2887,7 @@ <define name="suspendChoices"> <optional> <attribute name="enabled"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> </define> @@ -3054,10 +2979,7 @@ <define name="usbdevfilter"> <element name="usbdev"> <attribute name="allow"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> <optional> <attribute name="class"> @@ -3517,10 +3439,7 @@ </optional> <optional> <attribute name="managed"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <choice> @@ -3860,10 +3779,7 @@ <element name="apic"> <optional> <attribute name="eoi"> - <choice> - <value>on</value> - <value>off</value> - </choice> + <ref name="virSwitch"/> </attribute> </optional> </element> @@ -4157,10 +4073,7 @@ <element name="bios"> <optional> <attribute name="useserial"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <optional> @@ -4230,10 +4143,7 @@ <element name="rom"> <optional> <attribute name="bar"> - <choice> - <value>on</value> - <value>off</value> - </choice> + <ref name="virSwitch"/> </attribute> </optional> <optional> @@ -4703,10 +4613,7 @@ <define name="featurestate"> <attribute name="state"> - <choice> - <value>on</value> - <value>off</value> - </choice> + <ref name="virSwitch"/> </attribute> </define> diff --git a/docs/schemas/interface.rng b/docs/schemas/interface.rng index 0f577d6..bf1c982 100644 --- a/docs/schemas/interface.rng +++ b/docs/schemas/interface.rng @@ -127,7 +127,7 @@ <element name="bridge"> <optional> <attribute name="stp"> - <ref name="on-or-off"/> + <ref name="virSwitch"/> </attribute> </optional> <!-- Bridge forward delay (see 'brctl setfd') --> @@ -376,7 +376,7 @@ <element name="dhcp"> <optional> <attribute name="peerdns"> - <ref name="yes-or-no"/> + <ref name="virBool"/> </attribute> </optional> </element> @@ -415,21 +415,6 @@ instead of destination and nexthop instead of gateway. --> - <!-- Auxiliary definitions --> - <define name="on-or-off"> - <choice> - <value>on</value> - <value>off</value> - </choice> - </define> - - <define name="yes-or-no"> - <choice> - <value>yes</value> - <value>no</value> - </choice> - </define> - <!-- Type library --> <define name="timeval"> diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index 0e7da89..9f967a1 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -21,12 +21,9 @@ with no gateways addresses specified --> <optional> <attribute name="ipv6"> - <choice> - <value>yes</value> - <value>no</value> - </choice> - </attribute> - </optional> + <ref name="virBool"/> + </attribute> + </optional> <interleave> <!-- The name of the network, used to refer to it through the API @@ -53,10 +50,7 @@ <optional> <attribute name="stp"> - <choice> - <value>on</value> - <value>off</value> - </choice> + <ref name="virSwitch"/> </attribute> </optional> @@ -104,10 +98,7 @@ <optional> <attribute name="managed"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <interleave> @@ -203,10 +194,7 @@ </attribute> <optional> <attribute name="default"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <interleave> @@ -236,10 +224,7 @@ <element name="dns"> <optional> <attribute name="forwardPlainNames"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <interleave> diff --git a/docs/schemas/nwfilter.rng b/docs/schemas/nwfilter.rng index f1aa699..e057947 100644 --- a/docs/schemas/nwfilter.rng +++ b/docs/schemas/nwfilter.rng @@ -377,10 +377,7 @@ <interleave> <optional> <attribute name="match"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> </interleave> diff --git a/docs/schemas/secret.rng b/docs/schemas/secret.rng index d7b8f83..c9035b6 100644 --- a/docs/schemas/secret.rng +++ b/docs/schemas/secret.rng @@ -11,18 +11,12 @@ <element name='secret'> <optional> <attribute name='ephemeral'> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <optional> <attribute name='private'> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <interleave> -- 2.1.0

On 09/08/2014 07:40 AM, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- docs/schemas/basictypes.rng | 19 ++++-- docs/schemas/capability.rng | 10 +-- docs/schemas/domaincaps.rng | 5 +- docs/schemas/domaincommon.rng | 155 +++++++++--------------------------------- docs/schemas/interface.rng | 19 +----- docs/schemas/network.rng | 29 ++------ docs/schemas/nwfilter.rng | 5 +- docs/schemas/secret.rng | 10 +-- 8 files changed, 61 insertions(+), 191 deletions(-)
diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index 75d5238..d26da57 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -77,10 +77,7 @@ </attribute> <optional> <attribute name="multifunction"> - <choice> - <value>on</value> - <value>off</value> - </choice> + <ref name="virSwitch"/>
Purely cosmetic, but how about calling them "virYesNo" and "virOnOff" to avoid confusion? When I see "virBool" I think "true/false", and when I see "virSwitch" I think "Does this have something to do with a network device?" :-)
</attribute> </optional> </define> @@ -446,4 +443,18 @@ </optional> </define>
+ <define name="virBool"> + <choice> + <value>yes</value> + <value>no</value> + </choice> + </define> + + <define name="virSwitch"> + <choice> + <value>on</value> + <value>off</value> + </choice> + </define> + </grammar> diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng index f954599..65a8a0d 100644 --- a/docs/schemas/capability.rng +++ b/docs/schemas/capability.rng @@ -405,16 +405,10 @@
<define name='featuretoggle'> <attribute name='toggle'> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> <attribute name='default'> - <choice> - <value>on</value> - <value>off</value> - </choice> + <ref name="virSwitch"/> </attribute> </define>
diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng index 627b699..bc36a28 100644 --- a/docs/schemas/domaincaps.rng +++ b/docs/schemas/domaincaps.rng @@ -66,10 +66,7 @@
<define name='supported'> <attribute name='supported'> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </define>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index cedceae..25ff386 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -118,10 +118,7 @@ </attribute> <optional> <attribute name='relabel'> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <interleave> @@ -254,10 +251,7 @@ <optional> <element name="bootmenu"> <attribute name="enable"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> <optional> <attribute name="timeout"> @@ -556,10 +550,7 @@ <ref name='scaledInteger'/> <optional> <attribute name="dumpCore"> - <choice> - <value>on</value> - <value>off</value> - </choice> + <ref name="virSwitch"/> </attribute> </optional> </element> @@ -972,10 +963,7 @@ </choice> <optional> <attribute name="present"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <empty/> @@ -1225,10 +1213,7 @@ </attribute> <optional> <attribute name="rawio"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <optional> @@ -1496,10 +1481,7 @@ </optional> <optional> <attribute name="removable"> - <choice> - <value>on</value> - <value>off</value> - </choice> + <ref name="virSwitch"/> </attribute> </optional> </element> @@ -1632,26 +1614,17 @@ </define> <define name="ioeventfd"> <attribute name="ioeventfd"> - <choice> - <value>on</value> - <value>off</value> - </choice> + <ref name="virSwitch"/> </attribute> </define> <define name="event_idx"> <attribute name="event_idx"> - <choice> - <value>on</value> - <value>off</value> - </choice> + <ref name="virSwitch"/> </attribute> </define> <define name="copy_on_read"> <attribute name='copy_on_read'> - <choice> - <value>on</value> - <value>off</value> - </choice> + <ref name="virSwitch"/> </attribute> </define> <define name="discard"> @@ -2182,20 +2155,14 @@ </attribute> <optional> <attribute name="managed"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <interleave> <element name="source"> <optional> <attribute name="missing"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <choice> @@ -2418,10 +2385,7 @@ </optional> <optional> <attribute name="fullscreen"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> </group> @@ -2438,10 +2402,7 @@ </optional> <optional> <attribute name="autoport"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <optional> @@ -2512,10 +2473,7 @@ </optional> <optional> <attribute name="autoport"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <optional> @@ -2624,10 +2582,7 @@ <optional> <element name="playback"> <attribute name="compression"> - <choice> - <value>on</value> - <value>off</value> - </choice> + <ref name="virSwitch"/> </attribute> <empty/> </element> @@ -2647,10 +2602,7 @@ <optional> <element name="clipboard"> <attribute name="copypaste"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> <empty/> </element> @@ -2669,10 +2621,7 @@ <optional> <element name="filetransfer"> <attribute name="enable"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> <empty/> </element> @@ -2690,26 +2639,17 @@ </optional> <optional> <attribute name="autoport"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <optional> <attribute name="replaceUser"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <optional> <attribute name="multiUser"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <optional> @@ -2730,10 +2670,7 @@ </optional> <optional> <attribute name="fullscreen"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> </group> @@ -2812,28 +2749,19 @@ </optional> <optional> <attribute name="primary"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <optional> <element name="acceleration"> <optional> <attribute name="accel3d"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <optional> <attribute name="accel2d"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> </element> @@ -2959,10 +2887,7 @@ <define name="suspendChoices"> <optional> <attribute name="enabled"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> </define> @@ -3054,10 +2979,7 @@ <define name="usbdevfilter"> <element name="usbdev"> <attribute name="allow"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> <optional> <attribute name="class"> @@ -3517,10 +3439,7 @@ </optional> <optional> <attribute name="managed"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <choice> @@ -3860,10 +3779,7 @@ <element name="apic"> <optional> <attribute name="eoi"> - <choice> - <value>on</value> - <value>off</value> - </choice> + <ref name="virSwitch"/> </attribute> </optional> </element> @@ -4157,10 +4073,7 @@ <element name="bios"> <optional> <attribute name="useserial"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <optional> @@ -4230,10 +4143,7 @@ <element name="rom"> <optional> <attribute name="bar"> - <choice> - <value>on</value> - <value>off</value> - </choice> + <ref name="virSwitch"/> </attribute> </optional> <optional> @@ -4703,10 +4613,7 @@
<define name="featurestate"> <attribute name="state"> - <choice> - <value>on</value> - <value>off</value> - </choice> + <ref name="virSwitch"/> </attribute> </define>
diff --git a/docs/schemas/interface.rng b/docs/schemas/interface.rng index 0f577d6..bf1c982 100644 --- a/docs/schemas/interface.rng +++ b/docs/schemas/interface.rng @@ -127,7 +127,7 @@ <element name="bridge"> <optional> <attribute name="stp"> - <ref name="on-or-off"/> + <ref name="virSwitch"/> </attribute> </optional> <!-- Bridge forward delay (see 'brctl setfd') --> @@ -376,7 +376,7 @@ <element name="dhcp"> <optional> <attribute name="peerdns"> - <ref name="yes-or-no"/> + <ref name="virBool"/> </attribute> </optional> </element> @@ -415,21 +415,6 @@ instead of destination and nexthop instead of gateway. -->
- <!-- Auxiliary definitions --> - <define name="on-or-off"> - <choice> - <value>on</value> - <value>off</value> - </choice> - </define> - - <define name="yes-or-no"> - <choice> - <value>yes</value> - <value>no</value> - </choice> - </define> - <!-- Type library -->
<define name="timeval"> diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index 0e7da89..9f967a1 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -21,12 +21,9 @@ with no gateways addresses specified --> <optional> <attribute name="ipv6"> - <choice> - <value>yes</value> - <value>no</value> - </choice> - </attribute> - </optional> + <ref name="virBool"/> + </attribute> + </optional> <interleave>
<!-- The name of the network, used to refer to it through the API @@ -53,10 +50,7 @@
<optional> <attribute name="stp"> - <choice> - <value>on</value> - <value>off</value> - </choice> + <ref name="virSwitch"/> </attribute> </optional>
@@ -104,10 +98,7 @@
<optional> <attribute name="managed"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <interleave> @@ -203,10 +194,7 @@ </attribute> <optional> <attribute name="default"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <interleave> @@ -236,10 +224,7 @@ <element name="dns"> <optional> <attribute name="forwardPlainNames"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <interleave> diff --git a/docs/schemas/nwfilter.rng b/docs/schemas/nwfilter.rng index f1aa699..e057947 100644 --- a/docs/schemas/nwfilter.rng +++ b/docs/schemas/nwfilter.rng @@ -377,10 +377,7 @@ <interleave> <optional> <attribute name="match"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> </interleave> diff --git a/docs/schemas/secret.rng b/docs/schemas/secret.rng index d7b8f83..c9035b6 100644 --- a/docs/schemas/secret.rng +++ b/docs/schemas/secret.rng @@ -11,18 +11,12 @@ <element name='secret'> <optional> <attribute name='ephemeral'> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <optional> <attribute name='private'> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virBool"/> </attribute> </optional> <interleave>

On Thu, Sep 11, 2014 at 09:54:51AM -0400, Laine Stump wrote:
On 09/08/2014 07:40 AM, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- docs/schemas/basictypes.rng | 19 ++++-- docs/schemas/capability.rng | 10 +-- docs/schemas/domaincaps.rng | 5 +- docs/schemas/domaincommon.rng | 155 +++++++++--------------------------------- docs/schemas/interface.rng | 19 +----- docs/schemas/network.rng | 29 ++------ docs/schemas/nwfilter.rng | 5 +- docs/schemas/secret.rng | 10 +-- 8 files changed, 61 insertions(+), 191 deletions(-)
diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index 75d5238..d26da57 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -77,10 +77,7 @@ </attribute> <optional> <attribute name="multifunction"> - <choice> - <value>on</value> - <value>off</value> - </choice> + <ref name="virSwitch"/>
Purely cosmetic, but how about calling them "virYesNo" and "virOnOff" to avoid confusion? When I see "virBool" I think "true/false", and when I see "virSwitch" I think "Does this have something to do with a network device?" :-)
That's another way (and makes more sense). I just wanted to shorten the "Tristate" since it's not a tristate anyway. Should I count it as an ACK for s/virSwitch/virOnOff/ and s/virBool/virYesNo/ ? Martin

On 09/15/2014 01:46 AM, Martin Kletzander wrote:
On Thu, Sep 11, 2014 at 09:54:51AM -0400, Laine Stump wrote:
On 09/08/2014 07:40 AM, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> ---
+ <ref name="virSwitch"/>
Purely cosmetic, but how about calling them "virYesNo" and "virOnOff" to avoid confusion? When I see "virBool" I think "true/false", and when I see "virSwitch" I think "Does this have something to do with a network device?" :-)
That's another way (and makes more sense). I just wanted to shorten the "Tristate" since it's not a tristate anyway. Should I count it as an ACK for s/virSwitch/virOnOff/ and s/virBool/virYesNo/ ?
Yes, I'll agree to that rename. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Mon, Sep 15, 2014 at 09:29:05AM -0600, Eric Blake wrote:
On 09/15/2014 01:46 AM, Martin Kletzander wrote:
On Thu, Sep 11, 2014 at 09:54:51AM -0400, Laine Stump wrote:
On 09/08/2014 07:40 AM, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> ---
+ <ref name="virSwitch"/>
Purely cosmetic, but how about calling them "virYesNo" and "virOnOff" to avoid confusion? When I see "virBool" I think "true/false", and when I see "virSwitch" I think "Does this have something to do with a network device?" :-)
That's another way (and makes more sense). I just wanted to shorten the "Tristate" since it's not a tristate anyway. Should I count it as an ACK for s/virSwitch/virOnOff/ and s/virBool/virYesNo/ ?
Yes, I'll agree to that rename.
There were more things to be changed, so I sent a completely new version with this changed: https://www.redhat.com/archives/libvir-list/2014-September/msg00894.html Martin

Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- docs/formatdomain.html.in | 7 +++- docs/schemas/domaincommon.rng | 5 +++ src/conf/cpu_conf.c | 25 +++++++++++- src/conf/cpu_conf.h | 7 ++-- .../qemuxml2argv-cpu-numa-memshared.xml | 28 ++++++++++++++ .../qemuxml2argv-hugepages-shared.xml | 45 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 2 + 7 files changed, 113 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 94236dd..b284d6e 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1105,7 +1105,7 @@ ... <numa> <cell id='0' cpus='0-3' memory='512000'/> - <cell id='1' cpus='4-7' memory='512000'/> + <cell id='1' cpus='4-7' memory='512000' memShared='on'/> </numa> ... </cpu> @@ -1122,6 +1122,11 @@ assigned <code>id</code>s in the increasing order starting from 0. Mixing cells with and without the <code>id</code> attribute is not recommended as it may result in unwanted behaviour. + + <span class='since'>Since 1.2.9</span> the optional attribute + <code>memShared</code> can control whether the memory is to be + mapped as shared or not (values "on"/"off"). This is valid only + for hugepages-backed memory. </p> <p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 25ff386..89eb953 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3980,6 +3980,11 @@ <attribute name="memory"> <ref name="memoryKB"/> </attribute> + <optional> + <attribute name="memShared"> + <ref name="virSwitch"/> + </attribute> + </optional> </element> </define> diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 5003cf1..ab31f8d 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -435,7 +435,7 @@ virCPUDefParseXML(xmlNodePtr node, def->ncells = n; for (i = 0; i < n; i++) { - char *cpus, *memory; + char *cpus, *memory, *memSharedStr; int ret, ncpus = 0; unsigned int cur_cell; char *tmp = NULL; @@ -491,7 +491,7 @@ virCPUDefParseXML(xmlNodePtr node, goto error; } - ret = virStrToLong_ui(memory, NULL, 10, &def->cells[cur_cell].mem); + ret = virStrToLong_ui(memory, NULL, 10, &def->cells[cur_cell].mem); if (ret == -1) { virReportError(VIR_ERR_XML_ERROR, "%s", _("Invalid 'memory' attribute in NUMA cell")); @@ -499,6 +499,22 @@ virCPUDefParseXML(xmlNodePtr node, goto error; } VIR_FREE(memory); + + memSharedStr = virXMLPropString(nodes[i], "memShared"); + if (memSharedStr) { + def->cells[cur_cell].memShared = + virTristateSwitchTypeFromString(memSharedStr); + + if (def->cells[cur_cell].memShared <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Invalid 'memShared' attribute " + "value '%s'"), + memSharedStr); + VIR_FREE(memSharedStr); + goto cleanup; + } + VIR_FREE(memSharedStr); + } } } @@ -674,10 +690,15 @@ virCPUDefFormatBuf(virBufferPtr buf, virBufferAddLit(buf, "<numa>\n"); virBufferAdjustIndent(buf, 2); for (i = 0; i < def->ncells; i++) { + virTristateSwitch memShared = def->cells[i].memShared; + virBufferAddLit(buf, "<cell"); virBufferAsprintf(buf, " id='%zu'", i); virBufferAsprintf(buf, " cpus='%s'", def->cells[i].cpustr); virBufferAsprintf(buf, " memory='%d'", def->cells[i].mem); + if (memShared) + virBufferAsprintf(buf, " memShared='%s'", + virTristateSwitchTypeToString(memShared)); virBufferAddLit(buf, "/>\n"); } virBufferAdjustIndent(buf, -2); diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h index 2d538db..3083f45 100644 --- a/src/conf/cpu_conf.h +++ b/src/conf/cpu_conf.h @@ -93,9 +93,10 @@ struct _virCPUFeatureDef { typedef struct _virCellDef virCellDef; typedef virCellDef *virCellDefPtr; struct _virCellDef { - virBitmapPtr cpumask; /* CPUs that are part of this node */ - char *cpustr; /* CPUs stored in string form for dumpxml */ - unsigned int mem; /* Node memory in kB */ + virBitmapPtr cpumask; /* CPUs that are part of this node */ + char *cpustr; /* CPUs stored in string form for dumpxml */ + unsigned int mem; /* Node memory in kB */ + virTristateSwitch memShared; }; typedef struct _virCPUDef virCPUDef; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.xml new file mode 100644 index 0000000..63d94fa --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.xml @@ -0,0 +1,28 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>16</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='network'/> + </os> + <cpu> + <topology sockets='2' cores='4' threads='2'/> + <numa> + <cell id='0' cpus='0-7' memory='109550' memShared='on'/> + <cell id='1' cpus='8-15' memory='109550' memShared='off'/> + </numa> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.xml b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.xml new file mode 100644 index 0000000..f5226f7 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.xml @@ -0,0 +1,45 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>4194304</memory> + <currentMemory unit='KiB'>4194304</currentMemory> + <memoryBacking> + <hugepages> + <page size='2048' unit='KiB' nodeset='1'/> + <page size='1048576' unit='KiB' nodeset='0,2-3'/> + </hugepages> + </memoryBacking> + <vcpu placement='static'>4</vcpu> + <numatune> + <memory mode='strict' nodeset='0-3'/> + <memnode cellid='3' mode='strict' nodeset='3'/> + </numatune> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <cpu> + <numa> + <cell id='0' cpus='0' memory='1048576'/> + <cell id='1' cpus='1' memory='1048576' memShared='on'/> + <cell id='2' cpus='2' memory='1048576' memShared='off'/> + <cell id='3' cpus='3' memory='1048576'/> + </numa> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 03c05da..ea0015a 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -205,6 +205,7 @@ mymain(void) DO_TEST("hugepages-pages"); DO_TEST("hugepages-pages2"); DO_TEST("hugepages-pages3"); + DO_TEST("hugepages-shared"); DO_TEST("nosharepages"); DO_TEST("disk-aio"); DO_TEST("disk-cdrom"); @@ -391,6 +392,7 @@ mymain(void) DO_TEST_DIFFERENT("cpu-numa1"); DO_TEST_DIFFERENT("cpu-numa2"); DO_TEST("cpu-numa-disjoint"); + DO_TEST("cpu-numa-memshared"); DO_TEST_DIFFERENT("numatune-auto-prefer"); DO_TEST_DIFFERENT("numatune-memnode"); -- 2.1.0

On 09/08/2014 01:40 PM, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- docs/formatdomain.html.in | 7 +++- docs/schemas/domaincommon.rng | 5 +++ src/conf/cpu_conf.c | 25 +++++++++++- src/conf/cpu_conf.h | 7 ++-- .../qemuxml2argv-cpu-numa-memshared.xml | 28 ++++++++++++++ .../qemuxml2argv-hugepages-shared.xml | 45 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 2 + 7 files changed, 113 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 94236dd..b284d6e 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1105,7 +1105,7 @@ ... <numa> <cell id='0' cpus='0-3' memory='512000'/> - <cell id='1' cpus='4-7' memory='512000'/> + <cell id='1' cpus='4-7' memory='512000' memShared='on'/>
I wonder if "shared='on'" would be enough, avoiding the need for a multi-word attribute.
</numa> ... </cpu> @@ -1122,6 +1122,11 @@ assigned <code>id</code>s in the increasing order starting from 0. Mixing cells with and without the <code>id</code> attribute is not recommended as it may result in unwanted behaviour. + + <span class='since'>Since 1.2.9</span> the optional attribute + <code>memShared</code> can control whether the memory is to be + mapped as shared or not (values "on"/"off"). This is valid only + for hugepages-backed memory. </p>
<p>
@@ -491,7 +491,7 @@ virCPUDefParseXML(xmlNodePtr node, goto error; }
- ret = virStrToLong_ui(memory, NULL, 10, &def->cells[cur_cell].mem); + ret = virStrToLong_ui(memory, NULL, 10, &def->cells[cur_cell].mem); if (ret == -1) { virReportError(VIR_ERR_XML_ERROR, "%s", _("Invalid 'memory' attribute in NUMA cell")); @@ -93,9 +93,10 @@ struct _virCPUFeatureDef { typedef struct _virCellDef virCellDef; typedef virCellDef *virCellDefPtr; struct _virCellDef { - virBitmapPtr cpumask; /* CPUs that are part of this node */ - char *cpustr; /* CPUs stored in string form for dumpxml */ - unsigned int mem; /* Node memory in kB */ + virBitmapPtr cpumask; /* CPUs that are part of this node */ + char *cpustr; /* CPUs stored in string form for dumpxml */
The comments would look nicer aligned.
+ unsigned int mem; /* Node memory in kB */ + virTristateSwitch memShared; };
typedef struct _virCPUDef virCPUDef;
Please push these whitespace cleanups separately. Jan

On Wed, Sep 10, 2014 at 05:36:58PM +0200, Ján Tomko wrote:
On 09/08/2014 01:40 PM, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- docs/formatdomain.html.in | 7 +++- docs/schemas/domaincommon.rng | 5 +++ src/conf/cpu_conf.c | 25 +++++++++++- src/conf/cpu_conf.h | 7 ++-- .../qemuxml2argv-cpu-numa-memshared.xml | 28 ++++++++++++++ .../qemuxml2argv-hugepages-shared.xml | 45 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 2 + 7 files changed, 113 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 94236dd..b284d6e 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1105,7 +1105,7 @@ ... <numa> <cell id='0' cpus='0-3' memory='512000'/> - <cell id='1' cpus='4-7' memory='512000'/> + <cell id='1' cpus='4-7' memory='512000' memShared='on'/>
I wonder if "shared='on'" would be enough, avoiding the need for a multi-word attribute.
Or how about access="shared|private" ? Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Wed, Sep 10, 2014 at 04:46:27PM +0100, Daniel P. Berrange wrote:
On Wed, Sep 10, 2014 at 05:36:58PM +0200, Ján Tomko wrote:
On 09/08/2014 01:40 PM, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- docs/formatdomain.html.in | 7 +++- docs/schemas/domaincommon.rng | 5 +++ src/conf/cpu_conf.c | 25 +++++++++++- src/conf/cpu_conf.h | 7 ++-- .../qemuxml2argv-cpu-numa-memshared.xml | 28 ++++++++++++++ .../qemuxml2argv-hugepages-shared.xml | 45 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 2 + 7 files changed, 113 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 94236dd..b284d6e 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1105,7 +1105,7 @@ ... <numa> <cell id='0' cpus='0-3' memory='512000'/> - <cell id='1' cpus='4-7' memory='512000'/> + <cell id='1' cpus='4-7' memory='512000' memShared='on'/>
I wonder if "shared='on'" would be enough, avoiding the need for a multi-word attribute.
Or how about access="shared|private" ?
I prepended the "mem" so that it is visible that it has something to do with the memory, not the whole node. But I'm OK with pushing shared= as well. Using access= seems too ambiguously worded to me, although if most of you agree... Martin

On Mon, Sep 15, 2014 at 09:47:45AM +0200, Martin Kletzander wrote:
On Wed, Sep 10, 2014 at 04:46:27PM +0100, Daniel P. Berrange wrote:
On Wed, Sep 10, 2014 at 05:36:58PM +0200, Ján Tomko wrote:
On 09/08/2014 01:40 PM, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- docs/formatdomain.html.in | 7 +++- docs/schemas/domaincommon.rng | 5 +++ src/conf/cpu_conf.c | 25 +++++++++++- src/conf/cpu_conf.h | 7 ++-- .../qemuxml2argv-cpu-numa-memshared.xml | 28 ++++++++++++++ .../qemuxml2argv-hugepages-shared.xml | 45 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 2 + 7 files changed, 113 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 94236dd..b284d6e 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1105,7 +1105,7 @@ ... <numa> <cell id='0' cpus='0-3' memory='512000'/> - <cell id='1' cpus='4-7' memory='512000'/> + <cell id='1' cpus='4-7' memory='512000' memShared='on'/>
I wonder if "shared='on'" would be enough, avoiding the need for a multi-word attribute.
Or how about access="shared|private" ?
I prepended the "mem" so that it is visible that it has something to do with the memory, not the whole node. But I'm OK with pushing shared= as well. Using access= seems too ambiguously worded to me, although if most of you agree...
Sure, memAccess is fine with me. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Mon, Sep 15, 2014 at 10:20:01AM +0100, Daniel P. Berrange wrote:
On Mon, Sep 15, 2014 at 09:47:45AM +0200, Martin Kletzander wrote:
On Wed, Sep 10, 2014 at 04:46:27PM +0100, Daniel P. Berrange wrote:
On Wed, Sep 10, 2014 at 05:36:58PM +0200, Ján Tomko wrote:
On 09/08/2014 01:40 PM, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- docs/formatdomain.html.in | 7 +++- docs/schemas/domaincommon.rng | 5 +++ src/conf/cpu_conf.c | 25 +++++++++++- src/conf/cpu_conf.h | 7 ++-- .../qemuxml2argv-cpu-numa-memshared.xml | 28 ++++++++++++++ .../qemuxml2argv-hugepages-shared.xml | 45 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 2 + 7 files changed, 113 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 94236dd..b284d6e 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1105,7 +1105,7 @@ ... <numa> <cell id='0' cpus='0-3' memory='512000'/> - <cell id='1' cpus='4-7' memory='512000'/> + <cell id='1' cpus='4-7' memory='512000' memShared='on'/>
I wonder if "shared='on'" would be enough, avoiding the need for a multi-word attribute.
Or how about access="shared|private" ?
I prepended the "mem" so that it is visible that it has something to do with the memory, not the whole node. But I'm OK with pushing shared= as well. Using access= seems too ambiguously worded to me, although if most of you agree...
Sure, memAccess is fine with me.
Is there any possibility of that option having another value (in the future)? Otherwise shared= seems more appropriate to me. Let's see what others think, so I can finally get rid of this problem :) Martin

On Mon, Sep 15, 2014 at 01:19:25PM +0200, Martin Kletzander wrote:
On Mon, Sep 15, 2014 at 10:20:01AM +0100, Daniel P. Berrange wrote:
On Mon, Sep 15, 2014 at 09:47:45AM +0200, Martin Kletzander wrote:
On Wed, Sep 10, 2014 at 04:46:27PM +0100, Daniel P. Berrange wrote:
On Wed, Sep 10, 2014 at 05:36:58PM +0200, Ján Tomko wrote:
On 09/08/2014 01:40 PM, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- docs/formatdomain.html.in | 7 +++- docs/schemas/domaincommon.rng | 5 +++ src/conf/cpu_conf.c | 25 +++++++++++- src/conf/cpu_conf.h | 7 ++-- .../qemuxml2argv-cpu-numa-memshared.xml | 28 ++++++++++++++ .../qemuxml2argv-hugepages-shared.xml | 45 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 2 + 7 files changed, 113 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 94236dd..b284d6e 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1105,7 +1105,7 @@ ... <numa> <cell id='0' cpus='0-3' memory='512000'/> - <cell id='1' cpus='4-7' memory='512000'/> + <cell id='1' cpus='4-7' memory='512000' memShared='on'/>
I wonder if "shared='on'" would be enough, avoiding the need for a multi-word attribute.
Or how about access="shared|private" ?
I prepended the "mem" so that it is visible that it has something to do with the memory, not the whole node. But I'm OK with pushing shared= as well. Using access= seems too ambiguously worded to me, although if most of you agree...
Sure, memAccess is fine with me.
Is there any possibility of that option having another value (in the future)? Otherwise shared= seems more appropriate to me. Let's see what others think, so I can finally get rid of this problem :)
I prefer the approach of having values reflect the usage, as 'shared' vs 'private' for the value is clearer than 'on' vs 'off' IMHO. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Mon, Sep 15, 2014 at 12:40:48PM +0100, Daniel P. Berrange wrote:
On Mon, Sep 15, 2014 at 01:19:25PM +0200, Martin Kletzander wrote:
On Mon, Sep 15, 2014 at 10:20:01AM +0100, Daniel P. Berrange wrote:
On Mon, Sep 15, 2014 at 09:47:45AM +0200, Martin Kletzander wrote:
On Wed, Sep 10, 2014 at 04:46:27PM +0100, Daniel P. Berrange wrote:
On Wed, Sep 10, 2014 at 05:36:58PM +0200, Ján Tomko wrote:
On 09/08/2014 01:40 PM, Martin Kletzander wrote: > Signed-off-by: Martin Kletzander <mkletzan@redhat.com> > --- > docs/formatdomain.html.in | 7 +++- > docs/schemas/domaincommon.rng | 5 +++ > src/conf/cpu_conf.c | 25 +++++++++++- > src/conf/cpu_conf.h | 7 ++-- > .../qemuxml2argv-cpu-numa-memshared.xml | 28 ++++++++++++++ > .../qemuxml2argv-hugepages-shared.xml | 45 ++++++++++++++++++++++ > tests/qemuxml2xmltest.c | 2 + > 7 files changed, 113 insertions(+), 6 deletions(-) > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.xml > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.xml > > diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in > index 94236dd..b284d6e 100644 > --- a/docs/formatdomain.html.in > +++ b/docs/formatdomain.html.in > @@ -1105,7 +1105,7 @@ > ... > <numa> > <cell id='0' cpus='0-3' memory='512000'/> > - <cell id='1' cpus='4-7' memory='512000'/> > + <cell id='1' cpus='4-7' memory='512000' memShared='on'/>
I wonder if "shared='on'" would be enough, avoiding the need for a multi-word attribute.
Or how about access="shared|private" ?
I prepended the "mem" so that it is visible that it has something to do with the memory, not the whole node. But I'm OK with pushing shared= as well. Using access= seems too ambiguously worded to me, although if most of you agree...
Sure, memAccess is fine with me.
Is there any possibility of that option having another value (in the future)? Otherwise shared= seems more appropriate to me. Let's see what others think, so I can finally get rid of this problem :)
I prefer the approach of having values reflect the usage, as 'shared' vs 'private' for the value is clearer than 'on' vs 'off' IMHO.
OK, it makes more sense when you put it like that. I'll send a v2 to make sure everyone agrees on all three patches. Martin

Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_command.c | 19 +++++++++++++++++++ .../qemuxml2argv-hugepages-shared.args | 16 ++++++++++++++++ tests/qemuxml2argvtest.c | 5 +++++ 3 files changed, 40 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.args diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1ca98fb..6e4b0e4 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6545,6 +6545,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, for (i = 0; i < def->cpu->ncells; i++) { int cellmem = VIR_DIV_UP(def->cpu->cells[i].mem, 1024); def->cpu->cells[i].mem = cellmem * 1024; + virTristateSwitch memShared = def->cpu->cells[i].memShared; VIR_FREE(cpumask); VIR_FREE(nodemask); @@ -6623,7 +6624,18 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, virBufferAsprintf(&buf, "memory-backend-file,prealloc=yes,mem-path=%s", mem_path); + + if (memShared) { + virBufferAsprintf(&buf, ",share=%s", + virTristateSwitchTypeToString(memShared)); + } } else { + if (memShared) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Shared memory mapping is supported " + "only with hugepages")); + goto cleanup; + } virBufferAddLit(&buf, "memory-backend-ram"); } @@ -6654,6 +6666,13 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, virCommandAddArg(cmd, "-object"); virCommandAddArgBuffer(cmd, &buf); + } else { + if (memShared) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Shared memory mapping is not supported " + "with this QEMU")); + goto cleanup; + } } virCommandAddArg(cmd, "-numa"); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.args b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.args new file mode 100644 index 0000000..a7c7d92 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.args @@ -0,0 +1,16 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu -S -M pc -m 4096 -smp 4 \ +-object memory-backend-file,prealloc=yes,mem-path=/dev/hugepages1G/libvirt/qemu,\ +size=1024M,id=ram-node0,host-nodes=0-3,policy=bind \ +-numa node,nodeid=0,cpus=0,memdev=ram-node0 \ +-object memory-backend-file,prealloc=yes,mem-path=/dev/hugepages2M/libvirt/qemu,\ +share=on,size=1024M,id=ram-node1,host-nodes=0-3,policy=bind \ +-numa node,nodeid=1,cpus=1,memdev=ram-node1 \ +-object memory-backend-file,prealloc=yes,mem-path=/dev/hugepages1G/libvirt/qemu,\ +share=off,size=1024M,id=ram-node2,host-nodes=0-3,policy=bind \ +-numa node,nodeid=2,cpus=2,memdev=ram-node2 \ +-object memory-backend-file,prealloc=yes,mem-path=/dev/hugepages1G/libvirt/qemu,\ +size=1024M,id=ram-node3,host-nodes=3,policy=bind \ +-numa node,nodeid=3,cpus=3,memdev=ram-node3 \ +-nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb \ +-hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 3feb2fe..41dcd60 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -682,6 +682,8 @@ mymain(void) QEMU_CAPS_OBJECT_MEMORY_FILE); DO_TEST("hugepages-pages3", QEMU_CAPS_MEM_PATH, QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE); + DO_TEST("hugepages-shared", QEMU_CAPS_MEM_PATH, QEMU_CAPS_OBJECT_MEMORY_RAM, + QEMU_CAPS_OBJECT_MEMORY_FILE); DO_TEST("nosharepages", QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_MEM_MERGE); DO_TEST("disk-cdrom", NONE); DO_TEST("disk-cdrom-network-http", QEMU_CAPS_KVM, QEMU_CAPS_DEVICE, @@ -1205,6 +1207,9 @@ mymain(void) DO_TEST_PARSE_ERROR("cpu-numa3", NONE); DO_TEST_FAILURE("cpu-numa-disjoint", NONE); DO_TEST("cpu-numa-disjoint", QEMU_CAPS_NUMA); + DO_TEST_FAILURE("cpu-numa-memshared", QEMU_CAPS_SMP_TOPOLOGY, + QEMU_CAPS_OBJECT_MEMORY_RAM); + DO_TEST_FAILURE("cpu-numa-memshared", QEMU_CAPS_SMP_TOPOLOGY); DO_TEST("cpu-host-model", NONE); skipLegacyCPUs = true; DO_TEST("cpu-host-model-fallback", NONE); -- 2.1.0

On 09/08/2014 01:40 PM, Martin Kletzander wrote:
- BLURB
Martin Kletzander (3): schemas: finish virTristate{Bool,Switch} transition docs, conf, schema: add support for shared memory mapping qemu: add support for shared memory mapping
docs/formatdomain.html.in | 7 +- docs/schemas/basictypes.rng | 19 ++- docs/schemas/capability.rng | 10 +- docs/schemas/domaincaps.rng | 5 +- docs/schemas/domaincommon.rng | 160 +++++---------------- docs/schemas/interface.rng | 19 +-- docs/schemas/network.rng | 29 +--- docs/schemas/nwfilter.rng | 5 +- docs/schemas/secret.rng | 10 +- src/conf/cpu_conf.c | 25 +++- src/conf/cpu_conf.h | 7 +- src/qemu/qemu_command.c | 19 +++ .../qemuxml2argv-cpu-numa-memshared.xml | 28 ++++ .../qemuxml2argv-hugepages-shared.args | 16 +++ .../qemuxml2argv-hugepages-shared.xml | 45 ++++++ tests/qemuxml2argvtest.c | 5 + tests/qemuxml2xmltest.c | 2 + 17 files changed, 214 insertions(+), 197 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.xml
ACK series, see 2/3 for a bit of bikeshedding. Jan
participants (5)
-
Daniel P. Berrange
-
Eric Blake
-
Ján Tomko
-
Laine Stump
-
Martin Kletzander