[libvirt] [PATCH v2 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 enable debug-threads if supported. Related to: https://bugzilla.redhat.com/show_bug.cgi?id=1140121 cheers v1->v2: - drop the configuration, enable debug-thread automatically Marc-André Lureau (2): qemu: check for debug-threads capability qemu: enable debug threads src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 13 +++++++++---- 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 + 7 files changed, 16 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

On Thu, Mar 10, 2016 at 17:12:23 +0100, Marc-André Lureau wrote:
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(+)
ACK, although this patch will require resolving a trivial merge conflict because of another capability added in the meantime. Jirka

Hi ----- Original Message -----
On Thu, Mar 10, 2016 at 17:12:23 +0100, Marc-André Lureau wrote:
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(+)
ACK, although this patch will require resolving a trivial merge conflict because of another capability added in the meantime.
I'll fix it and push. thanks

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/qemu_command.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 000c29d..c730a01 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,17 @@ 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 (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 */ -- 2.5.0

On Thu, Mar 10, 2016 at 17:12:24 +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/qemu_command.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 000c29d..c730a01 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,17 @@ 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 (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NAME_DEBUG_THREADS)) { + virBufferAddLit(&name_opts, ",debug-threads=on"); }
Drop {} from the if statement above; make syntax-check should have told you so.
+ if (virBufferCheckError(&name_opts) < 0) + goto error; + virCommandAddArg(cmd, virBufferContentAndReset(&name_opts));
Anyway, I think it would be nice to move all the -name ... construction code into a dedicated qemuBuildNameCommandLine function. Jirka

Hi ----- Original Message -----
On Thu, Mar 10, 2016 at 17:12:24 +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/qemu_command.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 000c29d..c730a01 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,17 @@ 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 (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NAME_DEBUG_THREADS)) { + virBufferAddLit(&name_opts, ",debug-threads=on"); }
Drop {} from the if statement above; make syntax-check should have told you so.
+ if (virBufferCheckError(&name_opts) < 0) + goto error; + virCommandAddArg(cmd, virBufferContentAndReset(&name_opts));
Anyway, I think it would be nice to move all the -name ... construction code into a dedicated qemuBuildNameCommandLine function.
ok, let's do a v3 then, thanks

On Thu, Mar 10, 2016 at 17:12:22 +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.
Yeah, the output of "info threads" gdb command is very nice with debug-threads turned on. Jirka
participants (4)
-
Jiri Denemark
-
Marc-André Lureau
-
Marc-André Lureau
-
Marc-André Lureau