RHBZ: 800338 Adds a new capability to qemu, QEMU_CAPS_SCREENDUMP_ASYNC, available if the qmp command "screendump-async" exists. If that cap exists qemuDomainScreenshot uses it. The implementation consists of a hash from filename to struct holding the stream and temporary fd. The fd is closed and the stream is written to (in reverse order) by the completion callback, qemuProcessScreenshotComplete. Note: in qemuDomainScreenshot I don't check for an existing entry in the screenshots hash table because we the key is a temporary filename, produced by mkstemp, and it's only unlinked at qemuProcessScreenshotComplete. For testing you need to apply the following patches (they are still pending review on qemu-devel): http://patchwork.ozlabs.org/patch/144706/ http://patchwork.ozlabs.org/patch/144705/ http://patchwork.ozlabs.org/patch/144704/ Signed-off-by: Alon Levy <alevy@redhat.com> v2 changes: * v1 missed unlink of temp file on completion callback. * review issues from Eric Blake addressed: * virHashFree * screenshot free * s/VIR_ERR_INTERNAL_ERROR/VIR_ERR_OPERATION_INVALID/ * virReportOOMError * check for NULL filename in qemuProcessScreenshotComplete. --- src/qemu/qemu_capabilities.c | 1 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_domain.c | 7 ++++++ src/qemu/qemu_domain.h | 12 +++++++++++ src/qemu/qemu_driver.c | 43 +++++++++++++++++++++++++++++++++++------ src/qemu/qemu_monitor.c | 26 +++++++++++++++++++++++++ src/qemu/qemu_monitor.h | 8 +++++++ src/qemu/qemu_monitor_json.c | 39 ++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 ++ src/qemu/qemu_process.c | 30 +++++++++++++++++++++++++++++ 10 files changed, 163 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 64a4546..57771ff 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -154,6 +154,7 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST, "drive-iotune", /* 85 */ "system_wakeup", "scsi-disk.channel", + "screendump-async", ); struct qemu_feature_flags { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index db584ce..24d620d 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -122,6 +122,7 @@ enum qemuCapsFlags { QEMU_CAPS_DRIVE_IOTUNE = 85, /* -drive bps= and friends */ QEMU_CAPS_WAKEUP = 86, /* system_wakeup monitor command */ QEMU_CAPS_SCSI_DISK_CHANNEL = 87, /* Is scsi-disk.channel available? */ + QEMU_CAPS_SCREENDUMP_ASYNC = 88, /* screendump-async qmp command */ QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2fed91e..0c18d12 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -181,6 +181,10 @@ qemuDomainObjFreeJob(qemuDomainObjPrivatePtr priv) ignore_value(virCondDestroy(&priv->job.asyncCond)); } +static void +freeScreenshot(void *payload, const void *name ATTRIBUTE_UNUSED) { + VIR_FREE(payload); +} static void *qemuDomainObjPrivateAlloc(void) { @@ -196,6 +200,8 @@ static void *qemuDomainObjPrivateAlloc(void) goto error; priv->migMaxBandwidth = QEMU_DOMAIN_DEFAULT_MIG_BANDWIDTH_MAX; + priv->screenshots = virHashCreate(QEMU_DOMAIN_SCREENSHOTS_CONCURRENT_MAX, + freeScreenshot); return priv; @@ -218,6 +224,7 @@ static void qemuDomainObjPrivateFree(void *data) VIR_FREE(priv->origname); virConsoleFree(priv->cons); + virHashFree(priv->screenshots); /* This should never be non-NULL if we get here, but just in case... */ if (priv->mon) { diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 1333d8c..15721ec 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -40,6 +40,8 @@ # define QEMU_DOMAIN_DEFAULT_MIG_BANDWIDTH_MAX 32 +# define QEMU_DOMAIN_SCREENSHOTS_CONCURRENT_MAX 16 + # define JOB_MASK(job) (1 << (job - 1)) # define DEFAULT_JOB_MASK \ (JOB_MASK(QEMU_JOB_QUERY) | \ @@ -91,6 +93,14 @@ struct qemuDomainJobObj { virDomainJobInfo info; /* Async job progress data */ }; +struct _qemuScreenshotAsync { + virStreamPtr stream; /* stream to write results to */ + const char *filename; /* temporary file to read results from */ + int fd; /* handle to open temporary file */ +}; +typedef struct _qemuScreenshotAsync qemuScreenshotAsync; +typedef qemuScreenshotAsync *qemuScreenshotAsyncPtr; + typedef struct _qemuDomainPCIAddressSet qemuDomainPCIAddressSet; typedef qemuDomainPCIAddressSet *qemuDomainPCIAddressSetPtr; @@ -130,6 +140,8 @@ struct _qemuDomainObjPrivate { char *origname; virConsolesPtr cons; + + virHashTablePtr screenshots; }; struct qemuDomainWatchdogEvent diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 733df0a..eaf9284 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3135,6 +3135,7 @@ qemuDomainScreenshot(virDomainPtr dom, int tmp_fd = -1; char *ret = NULL; bool unlink_tmp = false; + qemuScreenshotAsync *screenshot = NULL; virCheckFlags(0, NULL); @@ -3184,9 +3185,35 @@ qemuDomainScreenshot(virDomainPtr dom, virSecurityManagerSetSavedStateLabel(qemu_driver->securityManager, vm->def, tmp); qemuDomainObjEnterMonitor(driver, vm); - if (qemuMonitorScreendump(priv->mon, tmp) < 0) { - qemuDomainObjExitMonitor(driver, vm); - goto endjob; + if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_SCREENDUMP_ASYNC)) { + if (virHashSize(priv->screenshots) >= + QEMU_DOMAIN_SCREENSHOTS_CONCURRENT_MAX) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("too many ongoing screenshots")); + goto endjob; + } + if (VIR_ALLOC(screenshot) < 0) { + virReportOOMError(); + goto endjob; + } + screenshot->fd = tmp_fd; + screenshot->filename = tmp; + screenshot->stream = st; + virHashAddEntry(priv->screenshots, tmp, screenshot); + if (qemuMonitorScreendumpAsync(priv->mon, tmp) < 0) { + qemuDomainObjExitMonitor(driver, vm); + VIR_FREE(screenshot); + goto endjob; + } + /* string and fd are freed by qmp event callback */ + tmp = NULL; + tmp_fd = -1; + unlink_tmp = false; + } else { + if (qemuMonitorScreendump(priv->mon, tmp) < 0) { + qemuDomainObjExitMonitor(driver, vm); + goto endjob; + } } qemuDomainObjExitMonitor(driver, vm); @@ -3195,10 +3222,12 @@ qemuDomainScreenshot(virDomainPtr dom, goto endjob; } - if (virFDStreamOpenFile(st, tmp, 0, 0, O_RDONLY) < 0) { - qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", - _("unable to open stream")); - goto endjob; + if (!qemuCapsGet(priv->qemuCaps, QEMU_CAPS_SCREENDUMP_ASYNC)) { + if (virFDStreamOpenFile(st, tmp, 0, 0, O_RDONLY) < 0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("unable to open stream")); + goto endjob; + } } ret = strdup("image/x-portable-pixmap"); diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 1da73f6..0df63e7 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1054,6 +1054,18 @@ int qemuMonitorEmitBlockJob(qemuMonitorPtr mon, } +int qemuMonitorEmitScreenDumpComplete(qemuMonitorPtr mon, + const char *filename) +{ + int ret = -1; + VIR_DEBUG("mon=%p", mon); + + QEMU_MONITOR_CALLBACK(mon, ret, domainScreenshotComplete, mon->vm, + filename); + return ret; +} + + int qemuMonitorSetCapabilities(qemuMonitorPtr mon, virBitmapPtr qemuCaps) @@ -2710,6 +2722,20 @@ int qemuMonitorScreendump(qemuMonitorPtr mon, return ret; } +int qemuMonitorScreendumpAsync(qemuMonitorPtr mon, + const char *file) +{ + VIR_DEBUG("mon=%p, file=%s", mon, file); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG,"%s", + _("monitor must not be NULL")); + return -1; + } + + return qemuMonitorJSONScreendumpAsync(mon, file); +} + int qemuMonitorBlockJob(qemuMonitorPtr mon, const char *device, const char *base, diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index b1c956c..abe977c 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -124,6 +124,9 @@ struct _qemuMonitorCallbacks { const char *diskAlias, int type, int status); + int (*domainScreenshotComplete)(qemuMonitorPtr mon, + virDomainObjPtr vm, + const char *filename); }; @@ -195,6 +198,8 @@ int qemuMonitorEmitBlockJob(qemuMonitorPtr mon, const char *diskAlias, int type, int status); +int qemuMonitorEmitScreenDumpComplete(qemuMonitorPtr mon, + const char *filename); @@ -505,6 +510,9 @@ int qemuMonitorInjectNMI(qemuMonitorPtr mon); int qemuMonitorScreendump(qemuMonitorPtr mon, const char *file); +int qemuMonitorScreendumpAsync(qemuMonitorPtr mon, + const char *file); + int qemuMonitorSendKey(qemuMonitorPtr mon, unsigned int holdtime, unsigned int *keycodes, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index dc67b4b..79ec6ba 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -59,6 +59,7 @@ static void qemuMonitorJSONHandleVNCConnect(qemuMonitorPtr mon, virJSONValuePtr static void qemuMonitorJSONHandleVNCInitialize(qemuMonitorPtr mon, virJSONValuePtr data); static void qemuMonitorJSONHandleVNCDisconnect(qemuMonitorPtr mon, virJSONValuePtr data); static void qemuMonitorJSONHandleBlockJob(qemuMonitorPtr mon, virJSONValuePtr data); +static void qemuMonitorJSONHandleScreenDumpComplete(qemuMonitorPtr mon, virJSONValuePtr data); static struct { const char *type; @@ -75,6 +76,7 @@ static struct { { "VNC_INITIALIZED", qemuMonitorJSONHandleVNCInitialize, }, { "VNC_DISCONNECTED", qemuMonitorJSONHandleVNCDisconnect, }, { "BLOCK_JOB_COMPLETED", qemuMonitorJSONHandleBlockJob, }, + { "SCREEN_DUMP_COMPLETE", qemuMonitorJSONHandleScreenDumpComplete, }, }; @@ -725,6 +727,16 @@ out: qemuMonitorEmitBlockJob(mon, device, type, status); } +static void qemuMonitorJSONHandleScreenDumpComplete(qemuMonitorPtr mon, + virJSONValuePtr data) +{ + const char *filename; + + if ((filename = virJSONValueObjectGetString(data, "filename")) == NULL) { + VIR_WARN("missing filename in screen dump complete event"); + } + qemuMonitorEmitScreenDumpComplete(mon, filename); +} int qemuMonitorJSONHumanCommandWithFd(qemuMonitorPtr mon, @@ -836,6 +848,9 @@ qemuMonitorJSONCheckCommands(qemuMonitorPtr mon, if (STREQ(name, "system_wakeup")) qemuCapsSet(qemuCaps, QEMU_CAPS_WAKEUP); + + if (STREQ(name, "screendump-async")) + qemuCapsSet(qemuCaps, QEMU_CAPS_SCREENDUMP_ASYNC); } ret = 0; @@ -3135,6 +3150,30 @@ int qemuMonitorJSONScreendump(qemuMonitorPtr mon, return ret; } +int qemuMonitorJSONScreendumpAsync(qemuMonitorPtr mon, + const char *file) +{ + int ret; + virJSONValuePtr cmd, reply = NULL; + + cmd = qemuMonitorJSONMakeCommand("screendump-async", + "s:filename", file, + NULL); + + if (!cmd) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + + static int qemuMonitorJSONGetBlockJobInfoOne(virJSONValuePtr entry, const char *device, virDomainBlockJobInfoPtr info) diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 0932a2c..74acce1 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -244,6 +244,9 @@ int qemuMonitorJSONSendKey(qemuMonitorPtr mon, int qemuMonitorJSONScreendump(qemuMonitorPtr mon, const char *file); +int qemuMonitorJSONScreendumpAsync(qemuMonitorPtr mon, + const char *file); + int qemuMonitorJSONBlockJob(qemuMonitorPtr mon, const char *device, const char *base, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 7b99814..3b547e9 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -47,6 +47,7 @@ #include "datatypes.h" #include "logging.h" +#include "fdstream.h" #include "virterror_internal.h" #include "memory.h" #include "hooks.h" @@ -918,6 +919,34 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED, } static int +qemuProcessScreenshotComplete(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + virDomainObjPtr vm, + const char *filename) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + qemuScreenshotAsyncPtr screenshot; + int ret = 0; + + if (!filename || (screenshot = virHashLookup(priv->screenshots, filename)) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("got screendump completion event for wrong filename")); + ret = -1; + goto end; + } + if (virFDStreamOpenFile(screenshot->stream, filename, 0, 0, O_RDONLY) < 0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("unable to open stream")); + ret = -1; + } +end: + VIR_FORCE_CLOSE(screenshot->fd); + unlink(screenshot->filename); + VIR_FREE(screenshot->filename); + virHashRemoveEntry(priv->screenshots, filename); + return ret; +} + +static int qemuProcessHandleGraphics(qemuMonitorPtr mon ATTRIBUTE_UNUSED, virDomainObjPtr vm, int phase, @@ -1034,6 +1063,7 @@ static qemuMonitorCallbacks monitorCallbacks = { .domainIOError = qemuProcessHandleIOError, .domainGraphics = qemuProcessHandleGraphics, .domainBlockJob = qemuProcessHandleBlockJob, + .domainScreenshotComplete = qemuProcessScreenshotComplete, }; static int -- 1.7.9.1