On 12/18/2015 08:20 AM, Dmitry Andreev wrote:
>
>
> On 17.12.2015 23:25, John Ferlan wrote:
>>
>>
>> On 12/14/2015 06:35 AM, Dmitry Andreev wrote:
>>> Excessive memory balloon inflation can cause invocation of OOM-killer,
>>> when Linux is under severe memory pressure. QEMU memballoon device
>>> has a feature to release some memory at the last moment before some
>>> process will be get killed by OOM-killer.
>>>
>>> Introduced attribute allows to enable or disable this feature.
>>> ---
>>> docs/formatdomain.html.in | 10 ++++++++++
>>> docs/schemas/domaincommon.rng | 5 +++++
>>> src/conf/domain_conf.c | 23 +++++++++++++++++++++++
>>> src/conf/domain_conf.h | 1 +
>>> 4 files changed, 39 insertions(+)
>>>
>>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>>> index a8bd48e..fa68e7b 100644
>>> --- a/docs/formatdomain.html.in
>>> +++ b/docs/formatdomain.html.in
>>> @@ -5954,6 +5954,16 @@ qemu-kvm -net nic,model=? /dev/null
>>> <li>'xen' — default with Xen</li>
>>> </ul>
>>> </dd>
>>> + <dt><code>deflate-on-oom</code></dt>
>>> + <dd>
>>> + <p>
>>> + The optional <code>deflate-on-oom</code> attribute allows to
>>> + enable/disable (values "on"/"off", respectively) the
>>> ability of the
>>> + QEMU virtio memory balloon to release some memory at the
>>> last moment
>>> + before a guest's process get killed by OOM-killer.
>>> + <span class="since">Since 1.3.1, QEMU and KVM only</span>
>>> + </p>
>>> + </dd>
>>> <dt><code>period</code></dt>
>>> <dd>
>>> <p>
>>> diff --git a/docs/schemas/domaincommon.rng
>>> b/docs/schemas/domaincommon.rng
>>> index 4804c69..9587849 100644
>>> --- a/docs/schemas/domaincommon.rng
>>> +++ b/docs/schemas/domaincommon.rng
>>> @@ -3415,6 +3415,11 @@
>>> <value>none</value>
>>> </choice>
>>> </attribute>
>>> + <optional>
>>> + <attribute name="deflate-on-oom">
>>
>>
>> Typically don't want those "-", either use some sort of single word or
>> other attributes to have "_"
> Is it OK to use 'deflation' or 'deflate' word? Can't choose the name
> by myself.
>
How about 'autodeflate' ? I think that describes the feature adequately.
John
>>
>> This will of course have ramifications throughout the code. I'll only
>> mention it here though.
>>
>>> + <ref name="virOnOff"/>
>>> + </attribute>
>>> + </optional>
>>> <interleave>
>>> <optional>
>>> <ref name="alias"/>
>>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>>> index 5200c27..b332097 100644
>>> --- a/src/conf/domain_conf.c
>>> +++ b/src/conf/domain_conf.c
>>> @@ -11312,6 +11312,7 @@ virDomainMemballoonDefParseXML(xmlNodePtr node,
>>> unsigned int flags)
>>> {
>>> char *model;
>>> + char *deflate;
>>> virDomainMemballoonDefPtr def;
>>> xmlNodePtr save = ctxt->node;
>>> unsigned int period = 0;
>>> @@ -11332,6 +11333,13 @@ virDomainMemballoonDefParseXML(xmlNodePtr node,
>>> goto error;
>>> }
>>>
>>> + if ((deflate = virXMLPropString(node, "deflate-on-oom")) &&
>>> + (def->deflate = virTristateSwitchTypeFromString(deflate)) <
>>> 0) {
>>
>> Since you're not using/allowing "default", then the comparison should be
>> <= 0
>>
>> yes, there are some others that are not correct it seems.
>>
>>> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>>> + _("invalid deflate-on-oom attribute value
>>> '%s'"), deflate);
>>> + goto error;
>>> + }
>>> +
>>> ctxt->node = node;
>>> if (virXPathUInt("string(./stats/@period)", ctxt, &period) < -1) {
>>> virReportError(VIR_ERR_XML_ERROR, "%s",
>>> @@ -11350,6 +11358,7 @@ virDomainMemballoonDefParseXML(xmlNodePtr node,
>>>
>>> cleanup:
>>> VIR_FREE(model);
>>> + VIR_FREE(deflate);
>>>
>>> ctxt->node = save;
>>> return def;
>>> @@ -17223,6 +17232,15 @@
>>> virDomainMemballoonDefCheckABIStability(virDomainMemballoonDefPtr src,
>>> return false;
>>> }
>>>
>>> + if (src->deflate != dst->deflate) {
>>> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>>> + _("Target balloon deflate-on-oom attribute
>>> value "
>>> + "%s does not match source %s"),
>>> + virTristateSwitchTypeToString(dst->deflate),
>>> + virTristateSwitchTypeToString(src->deflate));
>>> + return false;
>>> + }
>>> +
>>> if (!virDomainDeviceInfoCheckABIStability(&src->info, &dst->info))
>>> return false;
>>>
>>> @@ -20390,6 +20408,7 @@ virDomainMemballoonDefFormat(virBufferPtr buf,
>>> unsigned int flags)
>>> {
>>> const char *model =
>>> virDomainMemballoonModelTypeToString(def->model);
>>> + const char *deflate = virTristateSwitchTypeToString(def->deflate);
>>> virBuffer childrenBuf = VIR_BUFFER_INITIALIZER;
>>> int indent = virBufferGetIndent(buf, false);
>>>
>>> @@ -20400,6 +20419,10 @@ virDomainMemballoonDefFormat(virBufferPtr buf,
>>> }
>>>
>>> virBufferAsprintf(buf, "<memballoon model='%s'", model);
>>> +
>>> + if (def->deflate > 0 && deflate)
>>
>> This would be use of def->deflate != VIR_TRISTATE_SWITCH_ABSENT
>>
>>> + virBufferAsprintf(buf, " deflate-on-oom='%s'", deflate);
>>
>> The 'deflate' isn't required - see other examples.
>>
>> Looks OK otherwise.
>>
>> John
>>> +
>>> virBufferAdjustIndent(&childrenBuf, indent + 2);
>>>
>>> if (def->period)
>>> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>>> index cec681a..707ffb2 100644
>>> --- a/src/conf/domain_conf.h
>>> +++ b/src/conf/domain_conf.h
>>> @@ -1634,6 +1634,7 @@ struct _virDomainMemballoonDef {
>>> int model;
>>> virDomainDeviceInfo info;
>>> int period; /* seconds between collections */
>>> + int deflate; /* enum virTristateSwitch */
>>> };
>>>
>>> struct _virDomainNVRAMDef {
>>>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list