9:46 a.m.
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