[libvirt] [PATCHv4 2/3] conf: Add new xml elements for file memorybacking support
Safka, JaroslavX
jaroslavx.safka at intel.com
Mon Jan 30 08:07:19 UTC 2017
Comments inside.
> -----Original Message-----
> From: Michal Privoznik [mailto:mprivozn at redhat.com]
> Sent: Saturday, January 28, 2017 3:03 PM
> To: Safka, JaroslavX <jaroslavx.safka at intel.com>; libvir-list at redhat.com
> Cc: Mooney, Sean K <sean.k.mooney at intel.com>; Ptacek, MichalX
> <michalx.ptacek at intel.com>
> Subject: Re: [libvirt] [PATCHv4 2/3] conf: Add new xml elements for file
> memorybacking support
>
> On 13.12.2016 13:12, Jaroslav Safka wrote:
> > This first change introduces new xml elements for file based
> > memorybacking support and their parsing.
> > It allows vhost-user to be used without hugepages.
> >
> > New xml elements:
> > <memoryBacking>
> > <source type="file|anonymous"/>
> > <access mode="shared|private"/>
> > <allocation mode="immediate|ondemand"/> </memoryBacking>
> > ---
> > docs/formatdomain.html.in | 9 ++
> > docs/schemas/domaincommon.rng | 30 +++++
> > src/conf/domain_conf.c | 133 ++++++++++++++++-----
> > src/conf/domain_conf.h | 22 ++++
> > .../qemuxml2argv-memorybacking-set.xml | 24 ++++
> > .../qemuxml2argv-memorybacking-unset.xml | 24 ++++
> > .../qemuxml2xmlout-memorybacking-set.xml | 32 +++++
> > .../qemuxml2xmlout-memorybacking-unset.xml | 32 +++++
> > tests/qemuxml2xmltest.c | 3 +
> > 9 files changed, 276 insertions(+), 33 deletions(-) create mode
> > 100644 tests/qemuxml2argvdata/qemuxml2argv-memorybacking-set.xml
> > create mode 100644
> > tests/qemuxml2argvdata/qemuxml2argv-memorybacking-unset.xml
> > create mode 100644
> > tests/qemuxml2xmloutdata/qemuxml2xmlout-memorybacking-set.xml
> > create mode 100644
> > tests/qemuxml2xmloutdata/qemuxml2xmlout-memorybacking-unset.xml
> >
> > diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> > index 9218eec..fc194bf 100644
> > --- a/docs/formatdomain.html.in
> > +++ b/docs/formatdomain.html.in
> > @@ -903,6 +903,9 @@
> > </hugepages>
> > <nosharepages/>
> > <locked/>
> > + <source type="file|anonymous"/>
> > + <access mode="shared|private"/>
> > + <allocation mode="immediate|ondemand"/>
> > </memoryBacking>
> > ...
> > </domain>
> > @@ -942,6 +945,12 @@
> > most of the host's memory). Doing so may be dangerous to both the
> > domain and the host itself since the host's kernel may run out of
> > memory. <span class="since">Since 1.0.6</span></dd>
> > + <dt><code>source</code></dt>
> > + <dd>In this attribute you can switch to file memorybacking or keep
> default anonymous.</dd>
> > + <dt><code>access</code></dt>
> > + <dd>Specify if memory is shared or private. This can be overridden per
> numa node by <code>memAccess</code></dd>
> > + <dt><code>allocation</code></dt>
> > + <dd>Specify when allocate the memory</dd>
> > </dl>
> >
> >
> > diff --git a/docs/schemas/domaincommon.rng
> > b/docs/schemas/domaincommon.rng index c004233..363ee2f 100644
> > --- a/docs/schemas/domaincommon.rng
> > +++ b/docs/schemas/domaincommon.rng
> > @@ -560,6 +560,36 @@
> > <empty/>
> > </element>
> > </optional>
> > + <optional>
> > + <element name="source">
> > + <attribute name="type">
> > + <choice>
> > + <value>file</value>
> > + <value>anonymous</value>
> > + </choice>
> > + </attribute>
> > + </element>
> > + </optional>
> > + <optional>
> > + <element name="access">
> > + <attribute name="mode">
> > + <choice>
> > + <value>shared</value>
> > + <value>private</value>
> > + </choice>
> > + </attribute>
> > + </element>
> > + </optional>
> > + <optional>
> > + <element name="allocation">
> > + <attribute name="mode">
> > + <choice>
> > + <value>immediate</value>
> > + <value>ondemand</value>
> > + </choice>
> > + </attribute>
> > + </element>
> > + </optional>
> > </interleave>
> > </element>
> > </optional>
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index
> > b0bd38d..c54fc97 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -837,6 +837,16 @@ VIR_ENUM_IMPL(virDomainDiskMirrorState,
> VIR_DOMAIN_DISK_MIRROR_STATE_LAST,
> > "abort",
> > "pivot")
> >
> > +VIR_ENUM_IMPL(virDomainMemorySource,
> VIR_DOMAIN_MEMORY_SOURCE_LAST,
> > + "none",
> > + "file",
> > + "anonymous")
> > +
> > +VIR_ENUM_IMPL(virDomainMemoryAllocation,
> VIR_DOMAIN_MEMORY_ALLOCATION_LAST,
> > + "none",
> > + "immediate",
> > + "ondemand")
> > +
> > VIR_ENUM_IMPL(virDomainLoader,
> > VIR_DOMAIN_LOADER_TYPE_LAST,
> > "rom",
> > @@ -16528,48 +16538,93 @@ virDomainDefParseXML(xmlDocPtr xml,
> > }
> > VIR_FREE(tmp);
> >
> > - if ((n = virXPathNodeSet("./memoryBacking/hugepages/page", ctxt,
> &nodes)) < 0) {
> > - virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > - _("cannot extract hugepages nodes"));
> > - goto error;
> > + tmp = virXPathString("string(./memoryBacking/source/@type)", ctxt);
> > + if (tmp) {
> > + if ((def->mem.source =
> > + virDomainMemorySourceTypeFromString(tmp)) < 0) {
>
> This will not fly. def->mem.source is typeof enum virDomainMemorySource
> which can have values 0, 1 or 2. Now, should
> virDomainMemorySourceTypeFromString() return an error (-1) (because of
> unknown @type value), this gets squeezed into the enum range, which always
> falls into 0-2 range. IOW, this condition will never be true.
> That's why we have all those:
>
> int blah; /* enum virMyAwesomeEnum */
>
> lines in the definition. Had the @source been an int, this starts working. Except,
> we might want to disallow "none" and thus the condition should be "<= 0". This
> also apply for the other two variables.
>
[Jarek] I see. I saw using enums in another structure, then I changed it but I will return it to int. Thanks for point it out
> > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > + _("unknown memoryBacking/source/type '%s'"), tmp);
> > + goto error;
> > + }
> > + VIR_FREE(tmp);
> > }
>
> ACK with that change.
>
> Michal
--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263
This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.
More information about the libvir-list
mailing list