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