[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2 1/3] Add support for limiting guest coredump



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

> +            <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'

> +            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.

> +            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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]