On 09/20/2012 02:54 PM, Michal Privoznik wrote:
> On 20.09.2012 10:58, Martin Kletzander wrote:
>> Sometimes when guest machine crashes, coredump can get huge due to the
>> guest memory. This can be limited using madvise(2) system call and is
>> being used in QEMU hypervisor. This patch adds an option for configuring
>> that in the domain XML and related documentation.
>> ---
>> docs/formatdomain.html.in | 12 +++++++++---
>> docs/schemas/domaincommon.rng | 8 ++++++++
>> src/conf/domain_conf.c | 25 ++++++++++++++++++++++++-
>> src/conf/domain_conf.h | 10 ++++++++++
>> src/libvirt_private.syms | 2 ++
>> 5 files changed, 53 insertions(+), 4 deletions(-)
>>
>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>> index d021837..210ebe0 100644
>> --- a/docs/formatdomain.html.in
>> +++ b/docs/formatdomain.html.in
>> @@ -515,9 +515,15 @@
>> However, the value will be rounded up to the nearest kibibyte
>> by libvirt, and may be further rounded to the granularity
>> supported by the hypervisor. Some hypervisors also enforce a
>> - minimum, such as
>> - 4000KiB. <span class='since'><code>unit</code> since
>> - 0.9.11</span></dd>
>> + minimum, such as 4000KiB.
>> +
>> + In the case of crash, optional attribute <code>dump-core</code>
>> + can be used to control whether the guest memory should be
>> + included in the generated coredump or not (values "on", "off").
>> +
>> + <span class='since'><code>unit</code> since 0.9.11</span>,
>> + <span class='since'><code>dump-core</code> since 0.10.2
>> + (QEMU only)</span></dd>
>> <dt><code>currentMemory</code></dt>
>> <dd>The actual allocation of memory for the guest. This value can
>> be less than the maximum allocation, to allow for ballooning
>> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
>> index ed25f58..bf6afbb 100644
>> --- a/docs/schemas/domaincommon.rng
>> +++ b/docs/schemas/domaincommon.rng
>> @@ -470,6 +470,14 @@
>> <interleave>
>> <element name="memory">
>> <ref name='scaledInteger'/>
>> + <optional>
>> + <attribute name="dump-core">
>
> s/-c/C/ as Dan pointed out
>
Done and double checked with tests.
>> + <choice>
>> + <value>on</value>
>> + <value>off</value>
>> + </choice>
>> + </attribute>
>> + </optional>
>> </element>
>> <optional>
>> <element name="currentMemory">
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 4714312..c0cff7b 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -381,6 +381,11 @@ VIR_ENUM_IMPL(virDomainSoundModel, VIR_DOMAIN_SOUND_MODEL_LAST,
>> "ac97",
>> "ich6")
>>
>> +VIR_ENUM_IMPL(virDomainMemDump, VIR_DOMAIN_MEM_DUMP_LAST,
>> + "default",
>> + "on",
>> + "off")
>> +
>> VIR_ENUM_IMPL(virDomainMemballoonModel, VIR_DOMAIN_MEMBALLOON_MODEL_LAST,
>> "virtio",
>> "xen",
>> @@ -8525,6 +8530,19 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
>> &def->mem.cur_balloon, false) < 0)
>> goto error;
>>
>> + /* and info about it */
>> + tmp = virXPathString("string(./memory[1]/@dump-core)", ctxt);
>> + if (tmp) {
>> + def->mem.dump_core = virDomainMemDumpTypeFromString(tmp);
>> + VIR_FREE(tmp);
>> +
>> + if (def->mem.dump_core < 0) {
>
> s/</<=/ because we don't want to let users type in 'default'
>
Fixed.
>> + virReportError(VIR_ERR_XML_ERROR, "%s",
>> + _("Invalid value for <memory> 'dump-core' attribute"));
>
> we tend to write the value passed by user:
>
> virReportError(VIR_ERR_XML_ERROR,
> _("Bad value '%s'"),
> tmp);
>
> but that requires you won't free 'tmp' just a few lines above.
>
I did that except the function fitted on one line.
>> + goto error;
>> + }
>> + }
>> +
>> if (def->mem.cur_balloon > def->mem.max_balloon) {
>> /* Older libvirt could get into this situation due to
>> * rounding; if the discrepancy is less than 1MiB, we silently
>> @@ -13267,8 +13285,13 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>> xmlIndentTreeOutput = oldIndentTreeOutput;
>> }
>>
>> - virBufferAsprintf(buf, " <memory unit='KiB'>%llu</memory>\n",
>> + virBufferAddLit(buf, " <memory");
>> + if (def->mem.dump_core)
>> + virBufferAsprintf(buf, " dump-core='%s'",
>> + virDomainMemDumpTypeToString(def->mem.dump_core));
>> + virBufferAsprintf(buf, " unit='KiB'>%llu</memory>\n",
>> def->mem.max_balloon);
>> +
>> virBufferAsprintf(buf, " <currentMemory unit='KiB'>%llu</currentMemory>\n",
>> def->mem.cur_balloon);
>>
>> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>> index d719d57..fae699f 100644
>> --- a/src/conf/domain_conf.h
>> +++ b/src/conf/domain_conf.h
>> @@ -1320,6 +1320,14 @@ struct _virDomainRedirFilterDef {
>> virDomainRedirFilterUsbDevDefPtr *usbdevs;
>> };
>>
>> +enum virDomainMemDump {
>> + VIR_DOMAIN_MEM_DUMP_DEFAULT = 0,
>> + VIR_DOMAIN_MEM_DUMP_ON,
>> + VIR_DOMAIN_MEM_DUMP_OFF,
>> +
>> + VIR_DOMAIN_MEM_DUMP_LAST,
>> +};
>> +
>> enum {
>> VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO,
>> VIR_DOMAIN_MEMBALLOON_MODEL_XEN,
>> @@ -1641,6 +1649,7 @@ struct _virDomainDef {
>> unsigned long long max_balloon; /* in kibibytes */
>> unsigned long long cur_balloon; /* in kibibytes */
>> bool hugepage_backed;
>> + int dump_core; /* enum virDomainMemDump */
>> unsigned long long hard_limit; /* in kibibytes */
>> unsigned long long soft_limit; /* in kibibytes */
>> unsigned long long min_guarantee; /* in kibibytes */
>> @@ -2177,6 +2186,7 @@ VIR_ENUM_DECL(virDomainChrTcpProtocol)
>> VIR_ENUM_DECL(virDomainChrSpicevmc)
>> VIR_ENUM_DECL(virDomainSoundCodec)
>> VIR_ENUM_DECL(virDomainSoundModel)
>> +VIR_ENUM_DECL(virDomainMemDump)
>> VIR_ENUM_DECL(virDomainMemballoonModel)
>> VIR_ENUM_DECL(virDomainSmbiosMode)
>> VIR_ENUM_DECL(virDomainWatchdogModel)
>> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
>> index 0b53895..0b6068d 100644
>> --- a/src/libvirt_private.syms
>> +++ b/src/libvirt_private.syms
>> @@ -426,6 +426,8 @@ virDomainLiveConfigHelperMethod;
>> virDomainLoadAllConfigs;
>> virDomainMemballoonModelTypeFromString;
>> virDomainMemballoonModelTypeToString;
>> +virDomainMemDumpTypeFromString;
>> +virDomainMemDumpTypeToString;
>> virDomainNetDefFree;
>> virDomainNetFind;
>> virDomainNetGetActualBandwidth;
>>
>
> ACK if you fix those nits.
>
> Michal
>
Thanks for the review. Nits fixed, syntax-check and check ran OK, it's
pushed now.
Martin
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list