Re: [PATCH 2/2] qemu: add a max_core setting to qemu.conf for core dump size

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

 




On 03/18/2015 08:36 AM, Daniel P. Berrange wrote:
> Currently the QEMU processes inherit their core dump rlimit
> from libvirtd, which is really suboptimal. This change allows
> their limit to be directly controller from qemu.conf instead.
> ---
>  src/libvirt_private.syms           |  2 ++
>  src/qemu/libvirtd_qemu.aug         |  1 +
>  src/qemu/qemu.conf                 | 12 ++++++++++++
>  src/qemu/qemu_conf.c               |  3 +++
>  src/qemu/qemu_conf.h               |  2 ++
>  src/qemu/qemu_process.c            |  2 ++
>  src/qemu/test_libvirtd_qemu.aug.in |  1 +
>  src/util/vircommand.c              | 14 ++++++++++++++
>  src/util/vircommand.h              |  1 +
>  src/util/virprocess.c              | 35 +++++++++++++++++++++++++++++++++++
>  src/util/virprocess.h              |  1 +
>  11 files changed, 74 insertions(+)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index ca3520d..7446357 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1240,6 +1240,7 @@ virCommandSetErrorFD;
>  virCommandSetGID;
>  virCommandSetInputBuffer;
>  virCommandSetInputFD;
> +virCommandSetMaxCoreSize;
>  virCommandSetMaxFiles;
>  virCommandSetMaxMemLock;
>  virCommandSetMaxProcesses;
> @@ -1951,6 +1952,7 @@ virProcessRunInMountNamespace;
>  virProcessSchedPolicyTypeFromString;
>  virProcessSchedPolicyTypeToString;
>  virProcessSetAffinity;
> +virProcessSetMaxCoreSize;
>  virProcessSetMaxFiles;
>  virProcessSetMaxMemLock;
>  virProcessSetMaxProcesses;
> diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
> index 62951da..029a55a 100644
> --- a/src/qemu/libvirtd_qemu.aug
> +++ b/src/qemu/libvirtd_qemu.aug
> @@ -71,6 +71,7 @@ module Libvirtd_qemu =
>                   | bool_entry "set_process_name"
>                   | int_entry "max_processes"
>                   | int_entry "max_files"
> +                 | int_entry "max_core"
>  
>     let device_entry = bool_entry "mac_filter"
>                   | bool_entry "relaxed_acs_check"
> diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
> index 1c589a2..12e4326 100644
> --- a/src/qemu/qemu.conf
> +++ b/src/qemu/qemu.conf
> @@ -390,6 +390,18 @@
>  #max_processes = 0
>  #max_files = 0
>  
> +# If max_core is set to a positive integer, then QEMU will be
> +# permitted to create core dumps when it crashes, provided its
> +# RAM size is smaller than the limit set. Be warned that the
> +# core dump will include a full copy of the guest RAM, so if
> +# the largest guest is 32 GB in size, the max_core limit will
> +# have to be at least 33/34 GB to allow enough overhead.
> +#
> +# By default it will inherit core limit from libvirtd, which
> +# is usually set to 0 by systemd/init.
> +#
> +# Size is in bytes
> +#max_core = 0

It's too bad it cannot be a "sized" value... Reading 20G in a file for
me is so much easier than the comparable byte value say nothing if we
get to 128G, 1T, etc. (some day).

>  
>  
>  # mac_filter enables MAC addressed based filtering on bridge ports.
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index bca05c9..97bb8b8 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -770,6 +770,9 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
>      GET_VALUE_BOOL("set_process_name", cfg->setProcessName);
>      GET_VALUE_ULONG("max_processes", cfg->maxProcesses);
>      GET_VALUE_ULONG("max_files", cfg->maxFiles);
> +    GET_VALUE_ULONG("max_core", cfg->maxCore);
> +    if (virConfGetValue(conf, "max_core"))
> +        cfg->setMaxCore = true;

Should we ensure cfg->maxCore >= 0 (or does that get magically done by
one the many macros). Paranoia based on shared LL/LLU algorithm (and
that my eyes/brain is tired).  Which leads me down the path of - if
'max_core' is set to zero or doesn't exist, then cfg->maxCore would be
zero - so is the 'setMaxCore' necessary.

>  
>      GET_VALUE_STR("lock_manager", cfg->lockManagerName);
>  
> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
> index cb01fb6..ac7a1bf 100644
> --- a/src/qemu/qemu_conf.h
> +++ b/src/qemu/qemu_conf.h
> @@ -143,6 +143,8 @@ struct _virQEMUDriverConfig {
>  
>      int maxProcesses;
>      int maxFiles;
> +    bool setMaxCore;
> +    unsigned long long maxCore;
>  
>      int maxQueuedJobs;
>  
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 37cdb8f..995d6ef 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -4731,6 +4731,8 @@ int qemuProcessStart(virConnectPtr conn,
>      virCommandSetPreExecHook(cmd, qemuProcessHook, &hookData);
>      virCommandSetMaxProcesses(cmd, cfg->maxProcesses);
>      virCommandSetMaxFiles(cmd, cfg->maxFiles);
> +    if (cfg->setMaxCore)
> +        virCommandSetMaxCoreSize(cmd, cfg->maxCore);
>      virCommandSetUmask(cmd, 0x002);
>  
>      VIR_DEBUG("Setting up security labelling");
> diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in
> index fc4935b..9bb3683 100644
> --- a/src/qemu/test_libvirtd_qemu.aug.in
> +++ b/src/qemu/test_libvirtd_qemu.aug.in
> @@ -61,6 +61,7 @@ module Test_libvirtd_qemu =
>  { "set_process_name" = "1" }
>  { "max_processes" = "0" }
>  { "max_files" = "0" }
> +{ "max_core" = "0" }
>  { "mac_filter" = "1" }
>  { "relaxed_acs_check" = "1" }
>  { "allow_disk_format_probing" = "1" }
> diff --git a/src/util/vircommand.c b/src/util/vircommand.c
> index 648f5ed..1982401 100644
> --- a/src/util/vircommand.c
> +++ b/src/util/vircommand.c
> @@ -123,6 +123,8 @@ struct _virCommand {
>      unsigned long long maxMemLock;
>      unsigned int maxProcesses;
>      unsigned int maxFiles;
> +    bool setMaxCore;
> +    unsigned long long maxCore;
>  
>      uid_t uid;
>      gid_t gid;
> @@ -686,6 +688,9 @@ virExec(virCommandPtr cmd)
>          goto fork_error;
>      if (virProcessSetMaxFiles(0, cmd->maxFiles) < 0)
>          goto fork_error;
> +    if (cmd->setMaxCore &&
> +        virProcessSetMaxCoreSize(0, cmd->maxCore) < 0)
> +        goto fork_error;
>  
>      if (cmd->hook) {
>          VIR_DEBUG("Run hook %p %p", cmd->hook, cmd->opaque);
> @@ -1108,6 +1113,15 @@ virCommandSetMaxFiles(virCommandPtr cmd, unsigned int files)
>      cmd->maxFiles = files;
>  }
>  
> +void virCommandSetMaxCoreSize(virCommandPtr cmd, unsigned long long bytes)
> +{
> +    if (!cmd || cmd->has_error)
> +        return;
> +
> +    cmd->maxCore = bytes;
> +    cmd->setMaxCore = true;
> +}
> +
>  void virCommandSetUmask(virCommandPtr cmd, int mask)
>  {
>      if (!cmd || cmd->has_error)
> diff --git a/src/util/vircommand.h b/src/util/vircommand.h
> index 198da2f..4f2874d 100644
> --- a/src/util/vircommand.h
> +++ b/src/util/vircommand.h
> @@ -75,6 +75,7 @@ void virCommandSetUID(virCommandPtr cmd, uid_t uid);
>  void virCommandSetMaxMemLock(virCommandPtr cmd, unsigned long long bytes);
>  void virCommandSetMaxProcesses(virCommandPtr cmd, unsigned int procs);
>  void virCommandSetMaxFiles(virCommandPtr cmd, unsigned int files);
> +void virCommandSetMaxCoreSize(virCommandPtr cmd, unsigned long long bytes);
>  void virCommandSetUmask(virCommandPtr cmd, int umask);
>  
>  void virCommandClearCaps(virCommandPtr cmd);
> diff --git a/src/util/virprocess.c b/src/util/virprocess.c
> index aa00a99..02d91d9 100644
> --- a/src/util/virprocess.c
> +++ b/src/util/virprocess.c
> @@ -825,6 +825,41 @@ virProcessSetMaxFiles(pid_t pid ATTRIBUTE_UNUSED, unsigned int files)
>  }
>  #endif /* ! (HAVE_SETRLIMIT && defined(RLIMIT_NOFILE)) */
>  
> +#if HAVE_SETRLIMIT && defined(RLIMIT_CORE)
> +int
> +virProcessSetMaxCoreSize(pid_t pid, unsigned long long bytes)
> +{
> +    struct rlimit rlim;
> +
> +    rlim.rlim_cur = rlim.rlim_max = bytes;
> +    if (pid == 0) {
> +        if (setrlimit(RLIMIT_CORE, &rlim) < 0) {
> +            virReportSystemError(errno,
> +                                 _("cannot limit core file size to %llu"),
> +                                 bytes);
> +            return -1;
> +        }
> +    } else {
> +        if (virProcessPrLimit(pid, RLIMIT_CORE, &rlim) < 0) {
> +            virReportSystemError(errno,
> +                                 _("cannot limit core file size "
> +                                   "of process %lld to %llu"),
> +                                 (long long int)pid, bytes);
> +            return -1;
> +        }
> +    }
> +    return 0;
> +}
> +#else /* ! (HAVE_SETRLIMIT && defined(RLIMIT_CORE)) */
> +int
> +virProcessSetMaxCoreSize(pid_t pid ATTRIBUTE_UNUSED, unsigned long long bytes)

Aren't they both ATTRIBUTE_UNUSED

In general an ACK with at least this change... I just keep wondering
what happens if someone provides -1 for the value...

John
> +{
> +    virReportSystemError(ENOSYS, "%s", _("Not supported on this platform"));
> +    return -1;
> +}
> +#endif /* ! (HAVE_SETRLIMIT && defined(RLIMIT_CORE)) */
> +
> +
>  #ifdef __linux__
>  /*
>   * Port of code from polkitunixprocess.c under terms
> diff --git a/src/util/virprocess.h b/src/util/virprocess.h
> index c812882..cd7b7a2 100644
> --- a/src/util/virprocess.h
> +++ b/src/util/virprocess.h
> @@ -75,6 +75,7 @@ int virProcessSetNamespaces(size_t nfdlist,
>  int virProcessSetMaxMemLock(pid_t pid, unsigned long long bytes);
>  int virProcessSetMaxProcesses(pid_t pid, unsigned int procs);
>  int virProcessSetMaxFiles(pid_t pid, unsigned int files);
> +int virProcessSetMaxCoreSize(pid_t pid, unsigned long long bytes);
>  
>  /* Callback to run code within the mount namespace tied to the given
>   * pid.  This function must use only async-signal-safe functions, as
> 

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]