Re: [PATCH 2/2] qemu: allow turning off QEMU guest RAM dump globally

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

 




On 08/03/2016 11:31 AM, Daniel P. Berrange wrote:
> We already have the ability to turn off dumping of guest
> RAM via the domain XML. This is not particularly useful
> though, as it is under control of the management application.
> What is needed is a way for the sysadmin to turn off guest
> RAM defaults globally, regardless of whether the mgmt app
> provides its own way to set this in the domain XML.
> 
> So this adds a 'dump_guest_core' option in /etc/libvirt/qemu.conf
> which defaults to false. ie guest RAM will never be included in
> the QEMU core dumps by default. This default is different from
> historical practice, but is considered to be more suitable as
> a default because
> 
>  a) guest RAM can be huge and so inflicts a DOS on the host
>     I/O subsystem when dumping core for QEMU crashes
> 
>  b) guest RAM can contain alot of sensitive data belonging
>     to the VM owner. This should not generally be copied
>     around inside QEMU core dumps submitted to vendors for
>     debugging
> 
>  c) guest RAM contents are rarely useful in diagnosing
>     QEMU crashes
> 
> Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> ---
>  src/qemu/libvirtd_qemu.aug         |  1 +
>  src/qemu/qemu.conf                 | 16 +++++++++++++---
>  src/qemu/qemu_command.c            | 18 ++++++++++++------
>  src/qemu/qemu_conf.c               |  3 +++
>  src/qemu/qemu_conf.h               |  1 +
>  src/qemu/test_libvirtd_qemu.aug.in |  1 +
>  tests/qemuxml2argvtest.c           |  4 ++++
>  7 files changed, 35 insertions(+), 9 deletions(-)
> 

Couldn't git am -3 this one, so I'm sure you have some merges to do in
qemu_command.c (at least 90b42f0f and others afterwards from Michal) as
well as qemuxml2argvtest.c...

> diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
> index 9ec8250..c4ca77e 100644
> --- a/src/qemu/libvirtd_qemu.aug
> +++ b/src/qemu/libvirtd_qemu.aug
> @@ -76,6 +76,7 @@ module Libvirtd_qemu =
>                   | int_entry "max_processes"
>                   | int_entry "max_files"
>                   | limits_entry "max_core"
> +		 | bool_entry "dump_guest_core"
>                   | str_entry "stdio_handler"
>  

Strange alignment...

>     let device_entry = bool_entry "mac_filter"
> diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
> index abf9938..67ab215 100644
> --- a/src/qemu/qemu.conf
> +++ b/src/qemu/qemu.conf
> @@ -406,10 +406,10 @@
>  # RAM size is smaller than the limit set.
>  #
>  # Be warned that the core dump will include a full copy of the
> -# guest RAM, unless it has been disabled via the guest XML by
> -# setting:
> +# guest RAM, if the 'dump_guest_core' setting has been enabled,
> +# or if the guest XML contains
>  #
> -#   <memory dumpcore="off">...guest ram...</memory>
> +#   <memory dumpcore="on">...guest ram...</memory>
>  #
>  # If guest RAM is to be included, ensure the max_core limit
>  # is set to at least the size of the largest expected guest
> @@ -424,6 +424,16 @@
>  #
>  #max_core = "unlimited"
>  
> +# Determine if guest RAM is included in QEMU core dumps. By
> +# default guest RAM will be excluded if a new enough QEMU is
> +# present. Setting this to '1' will force guest RAM to always
> +# be included in QEMU core dumps.
> +#
> +# This setting will be ignored if the guest XML has set the
> +# dumpcore attribute on the <memory> element.
> +#
> +#dump_guest_core = 1
> +
>  # mac_filter enables MAC addressed based filtering on bridge ports.
>  # This currently requires ebtables to be installed.
>  #
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 197537f..944e0b1 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -6927,6 +6927,7 @@ qemuBuildNameCommandLine(virCommandPtr cmd,
>  
>  static int
>  qemuBuildMachineCommandLine(virCommandPtr cmd,
> +                            virQEMUDriverConfigPtr cfg,
>                              const virDomainDef *def,
>                              virQEMUCapsPtr qemuCaps)
>  {
> @@ -6999,17 +7000,22 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
>                                virTristateSwitchTypeToString(vmport));
>          }
>  
> -        if (def->mem.dump_core) {
> -            if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DUMP_GUEST_CORE)) {
> +        if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DUMP_GUEST_CORE)) {
> +            if (def->mem.dump_core) {
> +                virBufferAsprintf(&buf, ",dump-guest-core=%s",
> +                                  virTristateSwitchTypeToString(def->mem.dump_core));
> +            } else if (cfg->dumpGuestCore != true) {
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Will this even matter? Since there's a ternary below?  IOW: There's no
way to set this to "off" and I assume the by not providing it, it's off.
Maybe the ternary doesn't matter...

ACK with what appear to be obvious adjustments.

John

> +                virBufferAsprintf(&buf, ",dump-guest-core=%s",
> +                                  cfg->dumpGuestCore ? "on" : "off");
> +            }
> +        } else {
> +            if (def->mem.dump_core) {
>                  virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                                 _("dump-guest-core is not available "
>                                   "with this QEMU binary"));
>                  virBufferFreeAndReset(&buf);
>                  return -1;
>              }
> -
> -            virBufferAsprintf(&buf, ",dump-guest-core=%s",
> -                              virTristateSwitchTypeToString(def->mem.dump_core));
>          }
>  
>          if (def->mem.nosharepages) {
> @@ -9365,7 +9371,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
>      if (enableFips)
>          virCommandAddArg(cmd, "-enable-fips");
>  
> -    if (qemuBuildMachineCommandLine(cmd, def, qemuCaps) < 0)
> +    if (qemuBuildMachineCommandLine(cmd, cfg, def, qemuCaps) < 0)
>          goto error;
>  
>      if (qemuBuildCpuCommandLine(cmd, driver, def, qemuCaps, !!migrateURI) < 0)
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index 45d039c..2cf879b 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -649,6 +649,9 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
>          goto cleanup;
>      }
>  
> +    if (virConfGetValueBool(conf, "dump_guest_core", &cfg->dumpGuestCore) < 0)
> +        goto cleanup;
> +
>      if (virConfGetValueString(conf, "lock_manager", &cfg->lockManagerName) < 0)
>          goto cleanup;
>      if (virConfGetValueString(conf, "stdio_handler", &stdioHandler) < 0)
> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
> index b730202..c73d812 100644
> --- a/src/qemu/qemu_conf.h
> +++ b/src/qemu/qemu_conf.h
> @@ -149,6 +149,7 @@ struct _virQEMUDriverConfig {
>      unsigned int maxProcesses;
>      unsigned int maxFiles;
>      unsigned long long maxCore;
> +    bool dumpGuestCore;
>  
>      unsigned int maxQueuedJobs;
>  
> diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in
> index a4797af..b4cc8d0 100644
> --- a/src/qemu/test_libvirtd_qemu.aug.in
> +++ b/src/qemu/test_libvirtd_qemu.aug.in
> @@ -63,6 +63,7 @@ module Test_libvirtd_qemu =
>  { "max_processes" = "0" }
>  { "max_files" = "0" }
>  { "max_core" = "unlimited" }
> +{ "dump_guest_core" = "1" }
>  { "mac_filter" = "1" }
>  { "relaxed_acs_check" = "1" }
>  { "allow_disk_format_probing" = "1" }
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index a5d51a8..0697a15 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -614,8 +614,12 @@ mymain(void)
>      DO_TEST("machine-aliases2", QEMU_CAPS_KVM);
>      DO_TEST("machine-core-on", QEMU_CAPS_MACHINE_OPT,
>              QEMU_CAPS_DUMP_GUEST_CORE);
> +    driver.config->dumpGuestCore = true;
>      DO_TEST("machine-core-off", QEMU_CAPS_MACHINE_OPT,
>              QEMU_CAPS_DUMP_GUEST_CORE);
> +    driver.config->dumpGuestCore = false;
> +    DO_TEST("machine-core-cfg-off", QEMU_CAPS_MACHINE_OPT,
> +            QEMU_CAPS_DUMP_GUEST_CORE);
>      DO_TEST_FAILURE("machine-core-on", NONE);
>      DO_TEST_FAILURE("machine-core-on", QEMU_CAPS_MACHINE_OPT);
>      DO_TEST("machine-usb-opt", QEMU_CAPS_MACHINE_OPT,
> 

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