Re: [libvirt] [PATCH 2/2] qemu: Implement multiple screen support for virDomainScreenshot

2018-05-17 Thread Jiri Denemark
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 
> ---
>  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 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 2/2] qemu: Implement multiple screen support for virDomainScreenshot

2018-05-17 Thread Michal Privoznik
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 
---
 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(, "%s/qemu.screendump.XX", 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,