[libvirt] [PATCH 0/2] qemu: add debug-threads support

QEMU (somewhere around 2.0) added a new sub-option to the -name flag -name debug-threads=on This causes the naming of individual QEMU threads to be helpful; e.g. 'CPU/KVM 0' or 'migration' these show up in top once the H key is pressed, and also show up in a core dump, making it easy to figure out which thread is which. The following 2 patches add a capability check and a qemu-conf key to enable debug-threads. Related to: https://bugzilla.redhat.com/show_bug.cgi?id=1140121 cheers Marc-André Lureau (2): qemu: check for debug-threads capability qemu_conf: add set_debug_threads_name configuration key src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 6 ++++++ src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 14 ++++++++++---- src/qemu/qemu_conf.c | 1 + src/qemu/qemu_conf.h | 1 + src/qemu/test_libvirtd_qemu.aug.in | 1 + tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 1 + tests/qemucapabilitiesdata/caps_2.4.0-1.caps | 1 + tests/qemucapabilitiesdata/caps_2.5.0-1.caps | 1 + tests/qemucapabilitiesdata/caps_2.6.0-1.caps | 1 + 12 files changed, 27 insertions(+), 4 deletions(-) -- 2.5.0

From: Marc-André Lureau <marcandre.lureau@gmail.com> QEMU (somewhere around 2.0) added a new sub-option to the -name flag -name debug-threads=on. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 1 + tests/qemucapabilitiesdata/caps_2.4.0-1.caps | 1 + tests/qemucapabilitiesdata/caps_2.5.0-1.caps | 1 + tests/qemucapabilitiesdata/caps_2.6.0-1.caps | 1 + 6 files changed, 7 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 355b76e..a189e72 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -319,6 +319,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "qxl.vram64_size_mb", "qxl-vga.vram64_size_mb", /* 215 */ + "debug-threads", ); @@ -2634,6 +2635,7 @@ static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = { { "machine", "dea-key-wrap", QEMU_CAPS_DEA_KEY_WRAP }, { "chardev", "append", QEMU_CAPS_CHARDEV_FILE_APPEND }, { "spice", "gl", QEMU_CAPS_SPICE_GL }, + { "name", "debug-threads", QEMU_CAPS_NAME_DEBUG_THREADS }, }; static int diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 9aa79aa..61cc9b4 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -349,6 +349,7 @@ typedef enum { /* 215 */ QEMU_CAPS_QXL_VGA_VRAM64, /* -device qxl-vga.vram64_size_mb */ + QEMU_CAPS_NAME_DEBUG_THREADS, /* Is -name debug-threads= available */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_2.1.1-1.caps b/tests/qemucapabilitiesdata/caps_2.1.1-1.caps index fd2639d..59d0323 100644 --- a/tests/qemucapabilitiesdata/caps_2.1.1-1.caps +++ b/tests/qemucapabilitiesdata/caps_2.1.1-1.caps @@ -162,4 +162,5 @@ <flag name='vserport-change-event'/> <flag name='qxl.vram64_size_mb'/> <flag name='qxl-vga.vram64_size_mb'/> + <flag name='debug-threads'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_2.4.0-1.caps b/tests/qemucapabilitiesdata/caps_2.4.0-1.caps index 4b61fc1..efbf9af 100644 --- a/tests/qemucapabilitiesdata/caps_2.4.0-1.caps +++ b/tests/qemucapabilitiesdata/caps_2.4.0-1.caps @@ -173,4 +173,5 @@ <flag name='virtio-balloon-pci.deflate-on-oom'/> <flag name='qxl.vram64_size_mb'/> <flag name='qxl-vga.vram64_size_mb'/> + <flag name='debug-threads'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_2.5.0-1.caps b/tests/qemucapabilitiesdata/caps_2.5.0-1.caps index 2061d0e..5fd3bce 100644 --- a/tests/qemucapabilitiesdata/caps_2.5.0-1.caps +++ b/tests/qemucapabilitiesdata/caps_2.5.0-1.caps @@ -174,4 +174,5 @@ <flag name='virtio-balloon-pci.deflate-on-oom'/> <flag name='qxl.vram64_size_mb'/> <flag name='qxl-vga.vram64_size_mb'/> + <flag name='debug-threads'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-1.caps b/tests/qemucapabilitiesdata/caps_2.6.0-1.caps index 8374cda..549759c 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0-1.caps +++ b/tests/qemucapabilitiesdata/caps_2.6.0-1.caps @@ -177,4 +177,5 @@ <flag name='spice-gl'/> <flag name='qxl.vram64_size_mb'/> <flag name='qxl-vga.vram64_size_mb'/> + <flag name='debug-threads'/> </qemuCaps> -- 2.5.0

When debug-threads is enabled, individual threads are given a separate name (on Linux) Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1140121 Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 6 ++++++ src/qemu/qemu_command.c | 14 ++++++++++---- src/qemu/qemu_conf.c | 1 + src/qemu/qemu_conf.h | 1 + src/qemu/test_libvirtd_qemu.aug.in | 1 + 6 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index b6f6dc4..8a5e656 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -69,6 +69,7 @@ module Libvirtd_qemu = | bool_entry "clear_emulator_capabilities" | str_entry "bridge_helper" | bool_entry "set_process_name" + | bool_entry "set_debug_threads_name" | int_entry "max_processes" | int_entry "max_files" | str_entry "stdio_handler" diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 4fa5e8a..12800e9 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -380,6 +380,12 @@ # #set_process_name = 1 +# If enabled, QEMU will name the individual QEMU threads to be +# helpful, e.g. 'CPU/KVM 0' or 'migration'. these show up in top once +# the H key is pressed, and also show up in a core dump, making it +# easy to figure out which thread is which. +# +#set_debug_threads_name = 0 # If max_processes is set to a positive integer, libvirt will use # it to set the maximum number of processes that can be run by qemu diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 000c29d..8c95181 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7081,6 +7081,7 @@ qemuBuildCommandLine(virConnectPtr conn, virBuffer boot_buf = VIR_BUFFER_INITIALIZER; char *boot_order_str = NULL, *boot_opts_str = NULL; virBuffer fdc_opts = VIR_BUFFER_INITIALIZER; + virBuffer name_opts = VIR_BUFFER_INITIALIZER; char *fdc_opts_str = NULL; int bootCD = 0, bootFloppy = 0, bootDisk = 0, bootHostdevNet = 0; @@ -7106,13 +7107,18 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddEnvPassCommon(cmd); virCommandAddArg(cmd, "-name"); + virBufferAsprintf(&name_opts, "%s", def->name); if (cfg->setProcessName && virQEMUCapsGet(qemuCaps, QEMU_CAPS_NAME_PROCESS)) { - virCommandAddArgFormat(cmd, "%s,process=qemu:%s", - def->name, def->name); - } else { - virCommandAddArg(cmd, def->name); + virBufferAsprintf(&name_opts, ",process=qemu:%s", def->name); + } + if (cfg->setDebugThreadsName && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_NAME_DEBUG_THREADS)) { + virBufferAddLit(&name_opts, ",debug-threads=on"); } + if (virBufferCheckError(&name_opts) < 0) + goto error; + virCommandAddArg(cmd, virBufferContentAndReset(&name_opts)); if (!standalone) virCommandAddArg(cmd, "-S"); /* freeze CPU */ diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 77ef4fe..c827d71 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -794,6 +794,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, GET_VALUE_BOOL("clear_emulator_capabilities", cfg->clearEmulatorCapabilities); GET_VALUE_BOOL("allow_disk_format_probing", cfg->allowDiskFormatProbing); GET_VALUE_BOOL("set_process_name", cfg->setProcessName); + GET_VALUE_BOOL("set_debug_threads_name", cfg->setDebugThreadsName); GET_VALUE_ULONG("max_processes", cfg->maxProcesses); GET_VALUE_ULONG("max_files", cfg->maxFiles); diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index a714b84..0869bfd 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -143,6 +143,7 @@ struct _virQEMUDriverConfig { bool clearEmulatorCapabilities; bool allowDiskFormatProbing; bool setProcessName; + bool setDebugThreadsName; int maxProcesses; int maxFiles; diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index 8bec743..bf11ef9 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -59,6 +59,7 @@ module Test_libvirtd_qemu = { "bridge_helper" = "/usr/libexec/qemu-bridge-helper" } { "clear_emulator_capabilities" = "1" } { "set_process_name" = "1" } +{ "set_debug_threads_name" = "0" } { "max_processes" = "0" } { "max_files" = "0" } { "mac_filter" = "1" } -- 2.5.0

On Thu, Mar 10, 2016 at 01:54:59PM +0100, Marc-André Lureau wrote:
When debug-threads is enabled, individual threads are given a separate name (on Linux)
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1140121
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 6 ++++++ src/qemu/qemu_command.c | 14 ++++++++++---- src/qemu/qemu_conf.c | 1 + src/qemu/qemu_conf.h | 1 + src/qemu/test_libvirtd_qemu.aug.in | 1 + 6 files changed, 20 insertions(+), 4 deletions(-)
IMHO, we should just turn this on unconditionally if available, no need for a config file option. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Thu, Mar 10, 2016 at 13:54:57 +0100, Marc-André Lureau wrote:
QEMU (somewhere around 2.0) added a new sub-option to the -name flag -name debug-threads=on
This causes the naming of individual QEMU threads to be helpful; e.g. 'CPU/KVM 0' or 'migration' these show up in top once the H key is pressed, and also show up in a core dump, making it easy to figure out which thread is which.
The following 2 patches add a capability check and a qemu-conf key to enable debug-threads.
Is there any reason against enabling this unconditionally? It sounds like a nice thing to have if possible so I'd just always enable it if QEMU supports that... Jirka

Hi On Thu, Mar 10, 2016 at 2:00 PM, Jiri Denemark <jdenemar@redhat.com> wrote:
On Thu, Mar 10, 2016 at 13:54:57 +0100, Marc-André Lureau wrote:
QEMU (somewhere around 2.0) added a new sub-option to the -name flag -name debug-threads=on
This causes the naming of individual QEMU threads to be helpful; e.g. 'CPU/KVM 0' or 'migration' these show up in top once the H key is pressed, and also show up in a core dump, making it easy to figure out which thread is which.
The following 2 patches add a capability check and a qemu-conf key to enable debug-threads.
Is there any reason against enabling this unconditionally? It sounds like a nice thing to have if possible so I'd just always enable it if QEMU supports that...
That would be fine for me, I just wanted to be as careful as qemu is. They were probably thinking some monitoring tool could be confused. I suppose in libvirt case, there should not be furthere such monitoring, and libvirt is not confused. So why not? Keep the first patch, simplify the second? thanks -- Marc-André Lureau

On Thu, Mar 10, 2016 at 02:56:04PM +0100, Marc-André Lureau wrote:
Hi
On Thu, Mar 10, 2016 at 2:00 PM, Jiri Denemark <jdenemar@redhat.com> wrote:
On Thu, Mar 10, 2016 at 13:54:57 +0100, Marc-André Lureau wrote:
QEMU (somewhere around 2.0) added a new sub-option to the -name flag -name debug-threads=on
This causes the naming of individual QEMU threads to be helpful; e.g. 'CPU/KVM 0' or 'migration' these show up in top once the H key is pressed, and also show up in a core dump, making it easy to figure out which thread is which.
The following 2 patches add a capability check and a qemu-conf key to enable debug-threads.
Is there any reason against enabling this unconditionally? It sounds like a nice thing to have if possible so I'd just always enable it if QEMU supports that...
That would be fine for me, I just wanted to be as careful as qemu is. They were probably thinking some monitoring tool could be confused. I suppose in libvirt case, there should not be furthere such monitoring, and libvirt is not confused. So why not? Keep the first patch, simplify the second?
As far as libvirt is concerned, any monitoring tool should be using the libvirt APIs. Any tool that is talking straight to QEMU for a libvirt managed VM is in unsupported territory and gets to keep both pieces if it breaks. So while I understand QEMU's caution, I don't think it is needed in libvirt. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (4)
-
Daniel P. Berrange
-
Jiri Denemark
-
Marc-André Lureau
-
Marc-André Lureau