[libvirt] [PATCH v3 0/5] qemu: Support domain snapshots with current QMP
This version 3 gets back to the second alternative for HMP passthrough implementation provided in version 1. Changes made sinve that version 1 was posted are: - separate HMP passthrough infrastructure into a standalone patch - use qemuMonitorHMPCommand* and qemuMonitorTextCommand* instead of just qemuMonitorCommand* to make it obvious they deal with HMP/text monitor - implement HMP fallback for {load,del}vm in addition to savevm - escape snapshot name Jiri Denemark (5): qemu: Setup infrastructure for HMP passthrough qemu: Rename qemuMonitorCommand{,WithFd} as qemuMonitorHMP* qemu: Rename qemuMonitorCommandWithHandler as qemuMonitorText* qemu: Fallback to HMP for snapshot commands qemu: Escape snapshot name passed to {save,load,del}vm src/qemu/qemu_monitor.c | 12 +++ src/qemu/qemu_monitor.h | 8 ++- src/qemu/qemu_monitor_json.c | 121 ++++++++++++++++++++++--------- src/qemu/qemu_monitor_json.h | 5 ++ src/qemu/qemu_monitor_text.c | 163 ++++++++++++++++++++++-------------------- src/qemu/qemu_monitor_text.h | 5 ++ 6 files changed, 200 insertions(+), 114 deletions(-) -- 1.7.4.1
JSON monitor command implementation can now just directly call text monitor implementation and it will be automatically encapsulated into QMP's human-monitor-command. --- Version 3: - new patch; separated from the patch that implements HMP fallback for snapshot commands src/qemu/qemu_monitor.c | 12 ++++++ src/qemu/qemu_monitor.h | 8 ++++- src/qemu/qemu_monitor_json.c | 81 +++++++++++++++++++++++++++++------------- src/qemu/qemu_monitor_json.h | 5 +++ src/qemu/qemu_monitor_text.c | 18 +++------ src/qemu/qemu_monitor_text.h | 5 +++ 6 files changed, 91 insertions(+), 38 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index dfb1aad..2d5124e 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -742,6 +742,18 @@ cleanup: } +int qemuMonitorCommandWithFd(qemuMonitorPtr mon, + const char *cmd, + int scm_fd, + char **reply) +{ + if (mon->json) + return qemuMonitorJSONHumanCommandWithFd(mon, cmd, scm_fd, reply); + else + return qemuMonitorTextCommandWithFd(mon, cmd, scm_fd, reply); +} + + int qemuMonitorGetDiskSecret(qemuMonitorPtr mon, virConnectPtr conn, const char *path, diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 0ea1330..7200db8 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -133,9 +133,15 @@ void qemuMonitorUnlock(qemuMonitorPtr mon); int qemuMonitorRef(qemuMonitorPtr mon); int qemuMonitorUnref(qemuMonitorPtr mon); -/* This API is for use by the internal Text/JSON monitor impl code only */ +/* These APIs are for use by the internal Text/JSON monitor impl code only */ int qemuMonitorSend(qemuMonitorPtr mon, qemuMonitorMessagePtr msg); +int qemuMonitorCommandWithFd(qemuMonitorPtr mon, + const char *cmd, + int scm_fd, + char **reply); +# define qemuMonitorCommand(mon, cmd, reply) \ + qemuMonitorCommandWithFd(mon, cmd, -1, reply) /* XXX same comment about virConnectPtr as above */ int qemuMonitorGetDiskSecret(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index e6903a1..918591e 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -676,6 +676,56 @@ static void qemuMonitorJSONHandleVNCDisconnect(qemuMonitorPtr mon, virJSONValueP int +qemuMonitorJSONHumanCommandWithFd(qemuMonitorPtr mon, + const char *cmd_str, + int scm_fd, + char **reply_str) +{ + virJSONValuePtr cmd = NULL; + virJSONValuePtr reply = NULL; + virJSONValuePtr obj; + int ret = -1; + + cmd = qemuMonitorJSONMakeCommand("human-monitor-command", + "s:command-line", cmd_str, + NULL); + + if (!cmd || qemuMonitorJSONCommandWithFd(mon, cmd, scm_fd, &reply) < 0) + goto cleanup; + + if (qemuMonitorJSONCheckError(cmd, reply)) + goto cleanup; + + if (!(obj = virJSONValueObjectGet(reply, "return"))) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("human monitor command was missing return data")); + goto cleanup; + } + + if (reply_str) { + const char *data; + + if ((data = virJSONValueGetString(obj))) + *reply_str = strdup(data); + else + *reply_str = strdup(""); + + if (!*reply_str) { + virReportOOMError(); + goto cleanup; + } + } + + ret = 0; + +cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + + +int qemuMonitorJSONSetCapabilities(qemuMonitorPtr mon) { int ret; @@ -2464,36 +2514,17 @@ int qemuMonitorJSONArbitraryCommand(qemuMonitorPtr mon, virJSONValuePtr reply = NULL; int ret = -1; - if (!hmp) { - cmd = virJSONValueFromString(cmd_str); + if (hmp) { + return qemuMonitorJSONHumanCommandWithFd(mon, cmd_str, -1, reply_str); } else { - cmd = qemuMonitorJSONMakeCommand("human-monitor-command", - "s:command-line", cmd_str, - NULL); - } - - if (!cmd) - return -1; + if (!(cmd = virJSONValueFromString(cmd_str))) + goto cleanup; - if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) - goto cleanup; + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (!hmp) { if (!(*reply_str = virJSONValueToString(reply))) goto cleanup; - } else if (qemuMonitorJSONCheckError(cmd, reply)) { - goto cleanup; - } else { - const char *data; - if (!(data = virJSONValueObjectGetString(reply, "return"))) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("human monitor command was missing return data")); - goto cleanup; - } - if (!(*reply_str = strdup(data))) { - virReportOOMError(); - goto cleanup; - } } ret = 0; diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 4ae472a..3e0624d 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -34,6 +34,11 @@ int qemuMonitorJSONIOProcess(qemuMonitorPtr mon, size_t len, qemuMonitorMessagePtr msg); +int qemuMonitorJSONHumanCommandWithFd(qemuMonitorPtr mon, + const char *cmd, + int scm_fd, + char **reply); + int qemuMonitorJSONSetCapabilities(qemuMonitorPtr mon); int qemuMonitorJSONStartCPUs(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 0aed690..6949384 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -259,21 +259,15 @@ qemuMonitorCommandWithHandler(qemuMonitorPtr mon, return ret; } -static int -qemuMonitorCommandWithFd(qemuMonitorPtr mon, - const char *cmd, - int scm_fd, - char **reply) { +int +qemuMonitorTextCommandWithFd(qemuMonitorPtr mon, + const char *cmd, + int scm_fd, + char **reply) +{ return qemuMonitorCommandWithHandler(mon, cmd, NULL, NULL, scm_fd, reply); } -static int -qemuMonitorCommand(qemuMonitorPtr mon, - const char *cmd, - char **reply) { - return qemuMonitorCommandWithFd(mon, cmd, -1, reply); -} - static int qemuMonitorSendDiskPassphrase(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index b29dbcc..e6b27ec 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -35,6 +35,11 @@ int qemuMonitorTextIOProcess(qemuMonitorPtr mon, size_t len, qemuMonitorMessagePtr msg); +int qemuMonitorTextCommandWithFd(qemuMonitorPtr mon, + const char *cmd, + int scm_fd, + char **reply); + int qemuMonitorTextStartCPUs(qemuMonitorPtr mon, virConnectPtr conn); int qemuMonitorTextStopCPUs(qemuMonitorPtr mon); -- 1.7.4.1
On Thu, Mar 10, 2011 at 10:05:54AM +0100, Jiri Denemark wrote:
JSON monitor command implementation can now just directly call text monitor implementation and it will be automatically encapsulated into QMP's human-monitor-command. --- Version 3: - new patch; separated from the patch that implements HMP fallback for snapshot commands
src/qemu/qemu_monitor.c | 12 ++++++ src/qemu/qemu_monitor.h | 8 ++++- src/qemu/qemu_monitor_json.c | 81 +++++++++++++++++++++++++++++------------- src/qemu/qemu_monitor_json.h | 5 +++ src/qemu/qemu_monitor_text.c | 18 +++------ src/qemu/qemu_monitor_text.h | 5 +++ 6 files changed, 91 insertions(+), 38 deletions(-)
ACK 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 :|
So that it's obvious that they are supposed to be used with HMP commands. --- Version 3: - new patch src/qemu/qemu_monitor.c | 8 ++-- src/qemu/qemu_monitor.h | 12 +++--- src/qemu/qemu_monitor_text.c | 86 +++++++++++++++++++++--------------------- 3 files changed, 53 insertions(+), 53 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 2d5124e..37401df 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -742,10 +742,10 @@ cleanup: } -int qemuMonitorCommandWithFd(qemuMonitorPtr mon, - const char *cmd, - int scm_fd, - char **reply) +int qemuMonitorHMPCommandWithFd(qemuMonitorPtr mon, + const char *cmd, + int scm_fd, + char **reply) { if (mon->json) return qemuMonitorJSONHumanCommandWithFd(mon, cmd, scm_fd, reply); diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 7200db8..ece15a8 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -136,12 +136,12 @@ int qemuMonitorUnref(qemuMonitorPtr mon); /* These APIs are for use by the internal Text/JSON monitor impl code only */ int qemuMonitorSend(qemuMonitorPtr mon, qemuMonitorMessagePtr msg); -int qemuMonitorCommandWithFd(qemuMonitorPtr mon, - const char *cmd, - int scm_fd, - char **reply); -# define qemuMonitorCommand(mon, cmd, reply) \ - qemuMonitorCommandWithFd(mon, cmd, -1, reply) +int qemuMonitorHMPCommandWithFd(qemuMonitorPtr mon, + const char *cmd, + int scm_fd, + char **reply); +# define qemuMonitorHMPCommand(mon, cmd, reply) \ + qemuMonitorHMPCommandWithFd(mon, cmd, -1, reply) /* XXX same comment about virConnectPtr as above */ int qemuMonitorGetDiskSecret(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 6949384..51f1226 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -360,7 +360,7 @@ int qemuMonitorTextStopCPUs(qemuMonitorPtr mon) { char *info; - if (qemuMonitorCommand(mon, "stop", &info) < 0) { + if (qemuMonitorHMPCommand(mon, "stop", &info) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("cannot stop CPU execution")); return -1; @@ -373,7 +373,7 @@ qemuMonitorTextStopCPUs(qemuMonitorPtr mon) { int qemuMonitorTextSystemPowerdown(qemuMonitorPtr mon) { char *info; - if (qemuMonitorCommand(mon, "system_powerdown", &info) < 0) { + if (qemuMonitorHMPCommand(mon, "system_powerdown", &info) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("system shutdown operation failed")); return -1; @@ -392,7 +392,7 @@ int qemuMonitorTextGetCPUInfo(qemuMonitorPtr mon, pid_t *cpupids = NULL; size_t ncpupids = 0; - if (qemuMonitorCommand(mon, "info cpus", &qemucpus) < 0) { + if (qemuMonitorHMPCommand(mon, "info cpus", &qemucpus) < 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot run monitor command to fetch CPU thread info")); return -1; @@ -544,7 +544,7 @@ int qemuMonitorTextGetBalloonInfo(qemuMonitorPtr mon, int ret = -1; char *offset; - if (qemuMonitorCommand(mon, "info balloon", &reply) < 0) { + if (qemuMonitorHMPCommand(mon, "info balloon", &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("could not query memory balloon allocation")); return -1; @@ -581,7 +581,7 @@ int qemuMonitorTextGetMemoryStats(qemuMonitorPtr mon, int ret = 0; char *offset; - if (qemuMonitorCommand(mon, "info balloon", &reply) < 0) { + if (qemuMonitorHMPCommand(mon, "info balloon", &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("could not query memory balloon statistics")); return -1; @@ -613,7 +613,7 @@ int qemuMonitorTextGetBlockStatsInfo(qemuMonitorPtr mon, const char *p, *eol; int devnamelen = strlen(devname); - if (qemuMonitorCommand (mon, "info blockstats", &info) < 0) { + if (qemuMonitorHMPCommand (mon, "info blockstats", &info) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("'info blockstats' command failed")); goto cleanup; @@ -778,7 +778,7 @@ int qemuMonitorTextSetPassword(qemuMonitorPtr mon, goto cleanup; } - if (qemuMonitorCommand(mon, cmd, &reply) < 0) { + if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("setting password failed")); goto cleanup; @@ -812,7 +812,7 @@ int qemuMonitorTextExpirePassword(qemuMonitorPtr mon, goto cleanup; } - if (qemuMonitorCommand(mon, cmd, &reply) < 0) { + if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("expiring password failed")); goto cleanup; @@ -851,7 +851,7 @@ int qemuMonitorTextSetBalloon(qemuMonitorPtr mon, return -1; } - if (qemuMonitorCommand(mon, cmd, &reply) < 0) { + if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("could not balloon memory allocation")); VIR_FREE(cmd); @@ -888,7 +888,7 @@ int qemuMonitorTextSetCPU(qemuMonitorPtr mon, int cpu, int online) return -1; } - if (qemuMonitorCommand(mon, cmd, &reply) < 0) { + if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("could not change CPU online status")); VIR_FREE(cmd); @@ -923,7 +923,7 @@ int qemuMonitorTextEjectMedia(qemuMonitorPtr mon, goto cleanup; } - if (qemuMonitorCommand(mon, cmd, &reply) < 0) { + if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("could not eject media on %s"), devname); goto cleanup; @@ -967,7 +967,7 @@ int qemuMonitorTextChangeMedia(qemuMonitorPtr mon, goto cleanup; } - if (qemuMonitorCommand(mon, cmd, &reply) < 0) { + if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("could not change media on %s"), devname); goto cleanup; @@ -1019,7 +1019,7 @@ static int qemuMonitorTextSaveMemory(qemuMonitorPtr mon, goto cleanup; } - if (qemuMonitorCommand(mon, cmd, &reply) < 0) { + if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("could not save memory region to '%s'"), path); goto cleanup; @@ -1066,7 +1066,7 @@ int qemuMonitorTextSetMigrationSpeed(qemuMonitorPtr mon, goto cleanup; } - if (qemuMonitorCommand(mon, cmd, &info) < 0) { + if (qemuMonitorHMPCommand(mon, cmd, &info) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("could not restrict migration speed")); goto cleanup; @@ -1093,7 +1093,7 @@ int qemuMonitorTextSetMigrationDowntime(qemuMonitorPtr mon, goto cleanup; } - if (qemuMonitorCommand(mon, cmd, &info) < 0) { + if (qemuMonitorHMPCommand(mon, cmd, &info) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("could not set maximum migration downtime")); goto cleanup; @@ -1128,7 +1128,7 @@ int qemuMonitorTextGetMigrationStatus(qemuMonitorPtr mon, *remaining = 0; *total = 0; - if (qemuMonitorCommand(mon, "info migrate", &reply) < 0) { + if (qemuMonitorHMPCommand(mon, "info migrate", &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("cannot query migration status")); return -1; @@ -1234,7 +1234,7 @@ static int qemuMonitorTextMigrate(qemuMonitorPtr mon, goto cleanup; } - if (qemuMonitorCommand(mon, cmd, &info) < 0) { + if (qemuMonitorHMPCommand(mon, cmd, &info) < 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, _("unable to start migration to %s"), dest); goto cleanup; @@ -1384,7 +1384,7 @@ int qemuMonitorTextMigrateCancel(qemuMonitorPtr mon) { char *info = NULL; - if (qemuMonitorCommand(mon, "migrate_cancel", &info) < 0) { + if (qemuMonitorHMPCommand(mon, "migrate_cancel", &info) < 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot run monitor command to cancel migration")); return -1; @@ -1413,7 +1413,7 @@ int qemuMonitorTextAddUSBDisk(qemuMonitorPtr mon, goto cleanup; } - if (qemuMonitorCommand(mon, cmd, &info) < 0) { + if (qemuMonitorHMPCommand(mon, cmd, &info) < 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot run monitor command to add usb disk")); goto cleanup; @@ -1449,7 +1449,7 @@ static int qemuMonitorTextAddUSBDevice(qemuMonitorPtr mon, return -1; } - if (qemuMonitorCommand(mon, cmd, &reply) < 0) { + if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("cannot attach usb device")); goto cleanup; @@ -1590,7 +1590,7 @@ int qemuMonitorTextAddPCIHostDevice(qemuMonitorPtr mon, goto cleanup; } - if (qemuMonitorCommand(mon, cmd, &reply) < 0) { + if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("cannot attach host pci device")); goto cleanup; @@ -1641,7 +1641,7 @@ try_command: goto cleanup; } - if (qemuMonitorCommand(mon, cmd, &reply) < 0) { + if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("cannot attach %s disk %s"), bus, path); goto cleanup; @@ -1683,7 +1683,7 @@ int qemuMonitorTextAddPCINetwork(qemuMonitorPtr mon, return -1; } - if (qemuMonitorCommand(mon, cmd, &reply) < 0) { + if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("failed to add NIC with '%s'"), cmd); goto cleanup; @@ -1727,7 +1727,7 @@ try_command: } } - if (qemuMonitorCommand(mon, cmd, &reply) < 0) { + if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("failed to remove PCI device")); goto cleanup; @@ -1775,7 +1775,7 @@ int qemuMonitorTextSendFileHandle(qemuMonitorPtr mon, return -1; } - if (qemuMonitorCommandWithFd(mon, cmd, fd, &reply) < 0) { + if (qemuMonitorHMPCommandWithFd(mon, cmd, fd, &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("failed to pass fd to qemu with '%s'"), cmd); goto cleanup; @@ -1818,7 +1818,7 @@ int qemuMonitorTextCloseFileHandle(qemuMonitorPtr mon, return -1; } - if (qemuMonitorCommand(mon, cmd, &reply) < 0) { + if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("failed to close fd in qemu with '%s'"), cmd); goto cleanup; @@ -1854,7 +1854,7 @@ int qemuMonitorTextAddHostNetwork(qemuMonitorPtr mon, return -1; } - if (qemuMonitorCommand(mon, cmd, &reply) < 0) { + if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("failed to add host net with '%s'"), cmd); goto cleanup; @@ -1889,7 +1889,7 @@ int qemuMonitorTextRemoveHostNetwork(qemuMonitorPtr mon, return -1; } - if (qemuMonitorCommand(mon, cmd, &reply) < 0) { + if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("failed to remove host network in qemu with '%s'"), cmd); goto cleanup; @@ -1918,7 +1918,7 @@ int qemuMonitorTextAddNetdev(qemuMonitorPtr mon, return -1; } - if (qemuMonitorCommand(mon, cmd, &reply) < 0) { + if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("failed to add netdev with '%s'"), cmd); goto cleanup; @@ -1947,7 +1947,7 @@ int qemuMonitorTextRemoveNetdev(qemuMonitorPtr mon, return -1; } - if (qemuMonitorCommand(mon, cmd, &reply) < 0) { + if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("failed to remove netdev in qemu with '%s'"), cmd); goto cleanup; @@ -1982,7 +1982,7 @@ int qemuMonitorTextGetPtyPaths(qemuMonitorPtr mon, char *reply = NULL; int ret = -1; - if (qemuMonitorCommand(mon, "info chardev", &reply) < 0) { + if (qemuMonitorHMPCommand(mon, "info chardev", &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("failed to retrieve chardev info in qemu with 'info chardev'")); goto cleanup; @@ -2067,7 +2067,7 @@ try_command: goto cleanup; } - if (qemuMonitorCommand(mon, cmd, &reply) < 0) { + if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("cannot attach %s disk controller"), bus); goto cleanup; @@ -2166,7 +2166,7 @@ try_command: goto cleanup; } - if (qemuMonitorCommand(mon, cmd, &reply) < 0) { + if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("failed to close fd in qemu with '%s'"), cmd); goto cleanup; @@ -2257,7 +2257,7 @@ int qemuMonitorTextGetAllPCIAddresses(qemuMonitorPtr mon, *retaddrs = NULL; - if (qemuMonitorCommand(mon, "info pci", &reply) < 0) { + if (qemuMonitorHMPCommand(mon, "info pci", &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("cannot query PCI addresses")); return -1; @@ -2350,7 +2350,7 @@ int qemuMonitorTextDelDevice(qemuMonitorPtr mon, } VIR_DEBUG("TextDelDevice devalias=%s", devalias); - if (qemuMonitorCommand(mon, cmd, &reply) < 0) { + if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("cannot detach %s device"), devalias); goto cleanup; @@ -2390,7 +2390,7 @@ int qemuMonitorTextAddDevice(qemuMonitorPtr mon, goto cleanup; } - if (qemuMonitorCommand(mon, cmd, &reply) < 0) { + if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("cannot attach %s device"), devicestr); goto cleanup; @@ -2435,7 +2435,7 @@ int qemuMonitorTextAddDrive(qemuMonitorPtr mon, goto cleanup; } - if (qemuMonitorCommand(mon, cmd, &reply) < 0) { + if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("failed to close fd in qemu with '%s'"), cmd); goto cleanup; @@ -2483,7 +2483,7 @@ int qemuMonitorTextDriveDel(qemuMonitorPtr mon, goto cleanup; } - if (qemuMonitorCommand(mon, cmd, &reply) < 0) { + if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("cannot delete %s drive"), drivestr); goto cleanup; @@ -2537,7 +2537,7 @@ int qemuMonitorTextSetDrivePassphrase(qemuMonitorPtr mon, goto cleanup; } - if (qemuMonitorCommand(mon, cmd, &reply) < 0) { + if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("failed to close fd in qemu with '%s'"), cmd); goto cleanup; @@ -2573,7 +2573,7 @@ int qemuMonitorTextCreateSnapshot(qemuMonitorPtr mon, const char *name) return -1; } - if (qemuMonitorCommand(mon, cmd, &reply)) { + if (qemuMonitorHMPCommand(mon, cmd, &reply)) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("failed to take snapshot using command '%s'"), cmd); goto cleanup; @@ -2618,7 +2618,7 @@ int qemuMonitorTextLoadSnapshot(qemuMonitorPtr mon, const char *name) return -1; } - if (qemuMonitorCommand(mon, cmd, &reply)) { + if (qemuMonitorHMPCommand(mon, cmd, &reply)) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("failed to restore snapshot using command '%s'"), cmd); @@ -2673,7 +2673,7 @@ int qemuMonitorTextDeleteSnapshot(qemuMonitorPtr mon, const char *name) virReportOOMError(); return -1; } - if (qemuMonitorCommand(mon, cmd, &reply)) { + if (qemuMonitorHMPCommand(mon, cmd, &reply)) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("failed to delete snapshot using command '%s'"), cmd); @@ -2714,7 +2714,7 @@ int qemuMonitorTextArbitraryCommand(qemuMonitorPtr mon, const char *cmd, return -1; } - ret = qemuMonitorCommand(mon, safecmd, reply); + ret = qemuMonitorHMPCommand(mon, safecmd, reply); if (ret != 0) qemuReportError(VIR_ERR_OPERATION_FAILED, _("failed to run cmd '%s'"), safecmd); -- 1.7.4.1
On Thu, Mar 10, 2011 at 10:05:55AM +0100, Jiri Denemark wrote:
So that it's obvious that they are supposed to be used with HMP commands. --- Version 3: - new patch
src/qemu/qemu_monitor.c | 8 ++-- src/qemu/qemu_monitor.h | 12 +++--- src/qemu/qemu_monitor_text.c | 86 +++++++++++++++++++++--------------------- 3 files changed, 53 insertions(+), 53 deletions(-)
ACK 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 :|
To make it more obvious that it is only used for text monitor. The naming also matches the style of qemuMonitorTextCommandWithFd. --- Version 3: - new patch src/qemu/qemu_monitor_text.c | 32 +++++++++++++++++--------------- 1 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 51f1226..67e27db 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -206,12 +206,13 @@ int qemuMonitorTextIOProcess(qemuMonitorPtr mon ATTRIBUTE_UNUSED, } static int -qemuMonitorCommandWithHandler(qemuMonitorPtr mon, - const char *cmd, - qemuMonitorPasswordHandler passwordHandler, - void *passwordOpaque, - int scm_fd, - char **reply) { +qemuMonitorTextCommandWithHandler(qemuMonitorPtr mon, + const char *cmd, + qemuMonitorPasswordHandler passwordHandler, + void *passwordOpaque, + int scm_fd, + char **reply) +{ int ret; qemuMonitorMessage msg; @@ -265,7 +266,8 @@ qemuMonitorTextCommandWithFd(qemuMonitorPtr mon, int scm_fd, char **reply) { - return qemuMonitorCommandWithHandler(mon, cmd, NULL, NULL, scm_fd, reply); + return qemuMonitorTextCommandWithHandler(mon, cmd, NULL, NULL, + scm_fd, reply); } @@ -345,10 +347,10 @@ qemuMonitorTextStartCPUs(qemuMonitorPtr mon, virConnectPtr conn) { char *reply; - if (qemuMonitorCommandWithHandler(mon, "cont", - qemuMonitorSendDiskPassphrase, - conn, - -1, &reply) < 0) + if (qemuMonitorTextCommandWithHandler(mon, "cont", + qemuMonitorSendDiskPassphrase, + conn, + -1, &reply) < 0) return -1; VIR_FREE(reply); @@ -750,10 +752,10 @@ int qemuMonitorTextSetVNCPassword(qemuMonitorPtr mon, { char *info = NULL; - if (qemuMonitorCommandWithHandler(mon, "change vnc password", - qemuMonitorSendVNCPassphrase, - (char *)password, - -1, &info) < 0) { + if (qemuMonitorTextCommandWithHandler(mon, "change vnc password", + qemuMonitorSendVNCPassphrase, + (char *)password, + -1, &info) < 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("setting VNC password failed")); return -1; -- 1.7.4.1
On Thu, Mar 10, 2011 at 10:05:56AM +0100, Jiri Denemark wrote:
To make it more obvious that it is only used for text monitor. The naming also matches the style of qemuMonitorTextCommandWithFd. --- Version 3: - new patch
src/qemu/qemu_monitor_text.c | 32 +++++++++++++++++--------------- 1 files changed, 17 insertions(+), 15 deletions(-)
ACK 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 :|
qemu driver in libvirt gained support for creating domain snapshots almost a year ago in libvirt 0.8.0. Since then we enabled QMP support for qemu >= 0.13.0 but QMP equivalents of {save,load,del}vm commands are not implemented in current qemu (0.14.0) so the domain snapshot support is not very useful. This patch detects when the appropriate QMP command is not implemented and tries to use human-monitor-command (aka HMP passthrough) to run it's HMP equivalent. --- Version 3: - separated from HMP passthrough infrastructure - implement HMP fallback for {load,del}vm in addition to savevm src/qemu/qemu_monitor_json.c | 40 +++++++++++++++++++++++++++++++--------- 1 files changed, 31 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 918591e..d97dc68 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -31,6 +31,7 @@ #include <string.h> #include <sys/time.h> +#include "qemu_monitor_text.h" #include "qemu_monitor_json.h" #include "qemu_command.h" #include "memory.h" @@ -2451,11 +2452,18 @@ int qemuMonitorJSONCreateSnapshot(qemuMonitorPtr mon, const char *name) if (!cmd) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0) + goto cleanup; - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { + VIR_DEBUG0("savevm command not found, trying HMP"); + ret = qemuMonitorTextCreateSnapshot(mon, name); + goto cleanup; + } + ret = qemuMonitorJSONCheckError(cmd, reply); + +cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); return ret; @@ -2473,11 +2481,18 @@ int qemuMonitorJSONLoadSnapshot(qemuMonitorPtr mon, const char *name) if (!cmd) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0) + goto cleanup; - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { + VIR_DEBUG0("loadvm command not found, trying HMP"); + ret = qemuMonitorTextLoadSnapshot(mon, name); + goto cleanup; + } + + ret = qemuMonitorJSONCheckError(cmd, reply); +cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); return ret; @@ -2495,11 +2510,18 @@ int qemuMonitorJSONDeleteSnapshot(qemuMonitorPtr mon, const char *name) if (!cmd) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0) + goto cleanup; - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { + VIR_DEBUG0("delvm command not found, trying HMP"); + ret = qemuMonitorTextDeleteSnapshot(mon, name); + goto cleanup; + } + + ret = qemuMonitorJSONCheckError(cmd, reply); +cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); return ret; -- 1.7.4.1
On Thu, Mar 10, 2011 at 10:05:57AM +0100, Jiri Denemark wrote:
qemu driver in libvirt gained support for creating domain snapshots almost a year ago in libvirt 0.8.0. Since then we enabled QMP support for qemu >= 0.13.0 but QMP equivalents of {save,load,del}vm commands are not implemented in current qemu (0.14.0) so the domain snapshot support is not very useful.
This patch detects when the appropriate QMP command is not implemented and tries to use human-monitor-command (aka HMP passthrough) to run it's HMP equivalent. --- Version 3: - separated from HMP passthrough infrastructure - implement HMP fallback for {load,del}vm in addition to savevm
src/qemu/qemu_monitor_json.c | 40 +++++++++++++++++++++++++++++++--------- 1 files changed, 31 insertions(+), 9 deletions(-)
ACK, but you need to fix the patch subject line when pushing 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, 2011 at 11:39:55 +0000, Daniel P. Berrange wrote:
On Thu, Mar 10, 2011 at 10:05:57AM +0100, Jiri Denemark wrote:
qemu driver in libvirt gained support for creating domain snapshots almost a year ago in libvirt 0.8.0. Since then we enabled QMP support for qemu >= 0.13.0 but QMP equivalents of {save,load,del}vm commands are not implemented in current qemu (0.14.0) so the domain snapshot support is not very useful.
This patch detects when the appropriate QMP command is not implemented and tries to use human-monitor-command (aka HMP passthrough) to run it's HMP equivalent. --- Version 3: - separated from HMP passthrough infrastructure - implement HMP fallback for {load,del}vm in addition to savevm
src/qemu/qemu_monitor_json.c | 40 +++++++++++++++++++++++++++++++--------- 1 files changed, 31 insertions(+), 9 deletions(-)
ACK, but you need to fix the patch subject line when pushing
Heh, no problem with that, my git contains the correct subject. I have no idea how I managed to copy 3/5 subject into this 4/5 patch :-) Jirka
--- Version 3: - new patch src/qemu/qemu_monitor_text.c | 27 ++++++++++++++++++--------- 1 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 67e27db..a423212 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -2566,13 +2566,15 @@ cleanup: int qemuMonitorTextCreateSnapshot(qemuMonitorPtr mon, const char *name) { - char *cmd; + char *cmd = NULL; char *reply = NULL; int ret = -1; + char *safename; - if (virAsprintf(&cmd, "savevm \"%s\"", name) < 0) { + if (!(safename = qemuMonitorEscapeArg(name)) || + virAsprintf(&cmd, "savevm \"%s\"", safename) < 0) { virReportOOMError(); - return -1; + goto cleanup; } if (qemuMonitorHMPCommand(mon, cmd, &reply)) { @@ -2604,6 +2606,7 @@ int qemuMonitorTextCreateSnapshot(qemuMonitorPtr mon, const char *name) ret = 0; cleanup: + VIR_FREE(safename); VIR_FREE(cmd); VIR_FREE(reply); return ret; @@ -2611,13 +2614,15 @@ cleanup: int qemuMonitorTextLoadSnapshot(qemuMonitorPtr mon, const char *name) { - char *cmd; + char *cmd = NULL; char *reply = NULL; int ret = -1; + char *safename; - if (virAsprintf(&cmd, "loadvm \"%s\"", name) < 0) { + if (!(safename = qemuMonitorEscapeArg(name)) || + virAsprintf(&cmd, "loadvm \"%s\"", safename) < 0) { virReportOOMError(); - return -1; + goto cleanup; } if (qemuMonitorHMPCommand(mon, cmd, &reply)) { @@ -2660,6 +2665,7 @@ int qemuMonitorTextLoadSnapshot(qemuMonitorPtr mon, const char *name) ret = 0; cleanup: + VIR_FREE(safename); VIR_FREE(cmd); VIR_FREE(reply); return ret; @@ -2667,13 +2673,15 @@ cleanup: int qemuMonitorTextDeleteSnapshot(qemuMonitorPtr mon, const char *name) { - char *cmd; + char *cmd = NULL; char *reply = NULL; int ret = -1; + char *safename; - if (virAsprintf(&cmd, "delvm \"%s\"", name) < 0) { + if (!(safename = qemuMonitorEscapeArg(name)) || + virAsprintf(&cmd, "delvm \"%s\"", safename) < 0) { virReportOOMError(); - return -1; + goto cleanup; } if (qemuMonitorHMPCommand(mon, cmd, &reply)) { qemuReportError(VIR_ERR_OPERATION_FAILED, @@ -2700,6 +2708,7 @@ int qemuMonitorTextDeleteSnapshot(qemuMonitorPtr mon, const char *name) ret = 0; cleanup: + VIR_FREE(safename); VIR_FREE(cmd); VIR_FREE(reply); return ret; -- 1.7.4.1
On Thu, Mar 10, 2011 at 10:05:58AM +0100, Jiri Denemark wrote:
--- Version 3: - new patch
src/qemu/qemu_monitor_text.c | 27 ++++++++++++++++++--------- 1 files changed, 18 insertions(+), 9 deletions(-)
ACK 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, 2011 at 10:05:53 +0100, Jiri Denemark wrote:
This version 3 gets back to the second alternative for HMP passthrough implementation provided in version 1. Changes made sinve that version 1 was posted are: - separate HMP passthrough infrastructure into a standalone patch - use qemuMonitorHMPCommand* and qemuMonitorTextCommand* instead of just qemuMonitorCommand* to make it obvious they deal with HMP/text monitor - implement HMP fallback for {load,del}vm in addition to savevm - escape snapshot name
Jiri Denemark (5): qemu: Setup infrastructure for HMP passthrough qemu: Rename qemuMonitorCommand{,WithFd} as qemuMonitorHMP* qemu: Rename qemuMonitorCommandWithHandler as qemuMonitorText* qemu: Fallback to HMP for snapshot commands qemu: Escape snapshot name passed to {save,load,del}vm
src/qemu/qemu_monitor.c | 12 +++ src/qemu/qemu_monitor.h | 8 ++- src/qemu/qemu_monitor_json.c | 121 ++++++++++++++++++++++--------- src/qemu/qemu_monitor_json.h | 5 ++ src/qemu/qemu_monitor_text.c | 163 ++++++++++++++++++++++-------------------- src/qemu/qemu_monitor_text.h | 5 ++ 6 files changed, 200 insertions(+), 114 deletions(-)
Thanks for the review, pushed. Jirka
participants (2)
-
Daniel P. Berrange -
Jiri Denemark