[libvirt] [PATCH 0/2] qemu: Enable multihead screendumps

There's one small problem. QEMU might crash: https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg04113.html But apart from that, works like charm. Michal Privoznik (2): qemu: Introduce QEMU_CAPS_SCREENDUMP_DEVICE qemu: Implement multiple screen support for virDomainScreenshot src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_driver.c | 38 ++++++++++++++++++---- src/qemu/qemu_monitor.c | 4 ++- src/qemu/qemu_monitor.h | 2 ++ src/qemu/qemu_monitor_json.c | 4 +++ src/qemu/qemu_monitor_json.h | 2 ++ tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 1 + tests/qemumonitorjsontest.c | 2 +- 12 files changed, 50 insertions(+), 9 deletions(-) -- 2.16.1

As of v2.12.0-rc0~32^2 QEMU is capable specifying which display device and head should the screendump be taken from. Track this capability so that we can use it later in our virDomainScreenshot API. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 1 + 6 files changed, 7 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index a5cb24fec6..bface72de2 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -486,6 +486,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, /* 300 */ "sdl-gl", + "screendump_device", ); @@ -1269,6 +1270,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsQMPSchemaQueries[] = { { "blockdev-add/arg-type/+iscsi/password-secret", QEMU_CAPS_ISCSI_PASSWORD_SECRET }, { "blockdev-add/arg-type/+qcow2/encrypt/+luks/key-secret", QEMU_CAPS_QCOW2_LUKS }, { "nbd-server-start/arg-type/tls-creds", QEMU_CAPS_NBD_TLS }, + { "screendump/arg-type/device", QEMU_CAPS_SCREENDUMP_DEVICE }, }; typedef struct _virQEMUCapsObjectTypeProps virQEMUCapsObjectTypeProps; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index d23c34c24d..6f9953478a 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -470,6 +470,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ /* 300 */ QEMU_CAPS_SDL_GL, /* -sdl gl */ + QEMU_CAPS_SCREENDUMP_DEVICE, /* screendump command accepts device & head */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml index cabe4f2f07..de41d96cd0 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml @@ -162,6 +162,7 @@ <flag name='qom-list-properties'/> <flag name='memory-backend-file.discard-data'/> <flag name='sdl-gl'/> + <flag name='screendump_device'/> <version>2011090</version> <kvmVersion>0</kvmVersion> <microcodeVersion>343099</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml index bffe3b3b97..fc26f934ee 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml @@ -159,6 +159,7 @@ <flag name='qom-list-properties'/> <flag name='memory-backend-file.discard-data'/> <flag name='sdl-gl'/> + <flag name='screendump_device'/> <version>2011090</version> <kvmVersion>0</kvmVersion> <microcodeVersion>419968</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml index 2622d54ecd..bdfb81c998 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml @@ -127,6 +127,7 @@ <flag name='virtual-css-bridge.cssid-unrestricted'/> <flag name='vfio-ccw'/> <flag name='sdl-gl'/> + <flag name='screendump_device'/> <version>2012000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>371055</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml index 4247afeb31..820b3ef759 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml @@ -200,6 +200,7 @@ <flag name='qom-list-properties'/> <flag name='memory-backend-file.discard-data'/> <flag name='sdl-gl'/> + <flag name='screendump_device'/> <version>2011090</version> <kvmVersion>0</kvmVersion> <microcodeVersion>390813</microcodeVersion> -- 2.16.1

On Thu, May 17, 2018 at 17:18:28 +0200, Michal Privoznik wrote:
As of v2.12.0-rc0~32^2 QEMU is capable specifying which display device and head should the screendump be taken from. Track this capability so that we can use it later in our virDomainScreenshot API.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 1 + 6 files changed, 7 insertions(+)
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>

According to virDomainScreenshot() documentation, screens are numbered sequentially. e.g. having two graphics cards, both with four heads, screen ID 5 addresses the second head on the second card. But apart from that, there's nothing special happening here. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 38 +++++++++++++++++++++++++++++++------- src/qemu/qemu_monitor.c | 4 +++- src/qemu/qemu_monitor.h | 2 ++ src/qemu/qemu_monitor_json.c | 4 ++++ src/qemu/qemu_monitor_json.h | 2 ++ tests/qemumonitorjsontest.c | 2 +- 6 files changed, 43 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b697838070..e61af23870 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3999,6 +3999,8 @@ qemuDomainScreenshot(virDomainPtr dom, qemuDomainObjPrivatePtr priv; char *tmp = NULL; int tmp_fd = -1; + size_t i; + const char *videoAlias = NULL; char *ret = NULL; bool unlink_tmp = false; virQEMUDriverConfigPtr cfg = NULL; @@ -4020,13 +4022,35 @@ qemuDomainScreenshot(virDomainPtr dom, if (virDomainObjCheckActive(vm) < 0) goto endjob; - /* Well, even if qemu allows multiple graphic cards, heads, whatever, - * screenshot command does not */ + if (!vm->def->nvideos) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("no screens to take screenshot from")); + goto endjob; + } + if (screen) { - virReportError(VIR_ERR_INVALID_ARG, - "%s", _("currently is supported only taking " - "screenshots of screen ID 0")); - goto endjob; + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_SCREENDUMP_DEVICE)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("qemu does not allow specifying screen ID")); + goto endjob; + } + + for (i = 0; i < vm->def->nvideos; i++) { + const virDomainVideoDef *video = vm->def->videos[i]; + + if (screen < video->heads) { + videoAlias = video->info.alias; + break; + } + + screen -= video->heads; + } + + if (i == vm->def->nvideos) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("no such screen ID")); + goto endjob; + } } if (virAsprintf(&tmp, "%s/qemu.screendump.XXXXXX", cfg->cacheDir) < 0) @@ -4041,7 +4065,7 @@ qemuDomainScreenshot(virDomainPtr dom, qemuSecuritySetSavedStateLabel(driver->securityManager, vm->def, tmp); qemuDomainObjEnterMonitor(driver, vm); - if (qemuMonitorScreendump(priv->mon, tmp) < 0) { + if (qemuMonitorScreendump(priv->mon, videoAlias, screen, tmp) < 0) { ignore_value(qemuDomainObjExitMonitor(driver, vm)); goto endjob; } diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 3d7ca3ccfc..f21bf7000d 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3477,6 +3477,8 @@ qemuMonitorSendKey(qemuMonitorPtr mon, int qemuMonitorScreendump(qemuMonitorPtr mon, + const char *device, + unsigned int head, const char *file) { VIR_DEBUG("file=%s", file); @@ -3484,7 +3486,7 @@ qemuMonitorScreendump(qemuMonitorPtr mon, QEMU_CHECK_MONITOR(mon); if (mon->json) - return qemuMonitorJSONScreendump(mon, file); + return qemuMonitorJSONScreendump(mon, device, head, file); else return qemuMonitorTextScreendump(mon, file); } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 33dc521e83..6cba37c281 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -886,6 +886,8 @@ int qemuMonitorArbitraryCommand(qemuMonitorPtr mon, int qemuMonitorInjectNMI(qemuMonitorPtr mon); int qemuMonitorScreendump(qemuMonitorPtr mon, + const char *device, + unsigned int head, const char *file); int qemuMonitorSendKey(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index e2e0004e4d..6dcded9369 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4483,6 +4483,8 @@ int qemuMonitorJSONSendKey(qemuMonitorPtr mon, } int qemuMonitorJSONScreendump(qemuMonitorPtr mon, + const char *device, + unsigned int head, const char *file) { int ret = -1; @@ -4490,6 +4492,8 @@ int qemuMonitorJSONScreendump(qemuMonitorPtr mon, cmd = qemuMonitorJSONMakeCommand("screendump", "s:filename", file, + "S:device", device, + "p:head", head, NULL); if (!cmd) diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index e86b58f7ea..8461932cac 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -296,6 +296,8 @@ int qemuMonitorJSONSendKey(qemuMonitorPtr mon, unsigned int nkeycodes); int qemuMonitorJSONScreendump(qemuMonitorPtr mon, + const char *device, + unsigned int head, const char *file); int qemuMonitorJSONBlockStream(qemuMonitorPtr mon, diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index edd57067bd..add5ff0f19 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1348,7 +1348,7 @@ GEN_TEST_FUNC(qemuMonitorJSONDriveMirror, "vdb", "/foo/bar", NULL, 1024, 0, 0, VIR_DOMAIN_BLOCK_REBASE_SHALLOW | VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT) GEN_TEST_FUNC(qemuMonitorJSONBlockCommit, "vdb", "/foo/bar1", "/foo/bar2", NULL, 1024) GEN_TEST_FUNC(qemuMonitorJSONDrivePivot, "vdb") -GEN_TEST_FUNC(qemuMonitorJSONScreendump, "/foo/bar") +GEN_TEST_FUNC(qemuMonitorJSONScreendump, NULL, 0, "/foo/bar") GEN_TEST_FUNC(qemuMonitorJSONOpenGraphics, "spice", "spicefd", false) GEN_TEST_FUNC(qemuMonitorJSONNBDServerStart, "localhost", 12345, "test-alias") GEN_TEST_FUNC(qemuMonitorJSONNBDServerAdd, "vda", true) -- 2.16.1

On Thu, May 17, 2018 at 17:18:29 +0200, Michal Privoznik wrote:
According to virDomainScreenshot() documentation, screens are numbered sequentially. e.g. having two graphics cards, both with four heads, screen ID 5 addresses the second head on the second card.
But apart from that, there's nothing special happening here.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 38 +++++++++++++++++++++++++++++++------- src/qemu/qemu_monitor.c | 4 +++- src/qemu/qemu_monitor.h | 2 ++ src/qemu/qemu_monitor_json.c | 4 ++++ src/qemu/qemu_monitor_json.h | 2 ++ tests/qemumonitorjsontest.c | 2 +- 6 files changed, 43 insertions(+), 9 deletions(-)
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
participants (2)
-
Jiri Denemark
-
Michal Privoznik