While qemu supports the 'transaction' command since v1.1.0 (52e7c241ac766406f05fa) and the 'blockdev-snapshot-sync' command since v0.14.0-rc0 we need to keep the capability bits present since some qemu downstreams (RHEL/CentOS 7 for example) chose to cripple qemu by arbitrarily compile out some stuff which was already present at that time. To simplify the crazy code just require both commands to be present at the beginning of a external snapshot so that we can remove the case when 'transaction' would not be supported. This also allows to drop any logic connected to the VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC flag since snapshots are atomic with the 'transaction' command. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 62 +++++++++++--------------------------------------- 1 file changed, 13 insertions(+), 49 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 825b2b27e6..4f577e50d2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14614,18 +14614,9 @@ qemuDomainSnapshotPrepare(virDomainObjPtr vm, size_t i; bool active = virDomainObjIsActive(vm); bool reuse = (*flags & VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT) != 0; - bool atomic = (*flags & VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC) != 0; bool found_internal = false; bool forbid_internal = false; int external = 0; - qemuDomainObjPrivatePtr priv = vm->privateData; - - if (def->state == VIR_DOMAIN_DISK_SNAPSHOT && - reuse && !virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_TRANSACTION)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("reuse is not supported with this QEMU binary")); - goto cleanup; - } for (i = 0; i < def->ndisks; i++) { virDomainSnapshotDiskDefPtr disk = &def->disks[i]; @@ -14756,18 +14747,6 @@ qemuDomainSnapshotPrepare(virDomainObjPtr vm, if (external && !active) *flags |= VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY; - if (def->state != VIR_DOMAIN_DISK_SNAPSHOT && active) { - if (external == 1 || - virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_TRANSACTION)) { - *flags |= VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC; - } else if (atomic && external > 1) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("atomic live snapshot of multiple disks " - "is unsupported")); - goto cleanup; - } - } - ret = 0; cleanup: @@ -15033,15 +15012,8 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, if (virDomainObjCheckActive(vm) < 0) return -1; - if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_TRANSACTION)) { - if (!(actions = virJSONValueNewArray())) - return -1; - } else if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DISK_SNAPSHOT)) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("live disk snapshot not supported with this " - "QEMU binary")); + if (!(actions = virJSONValueNewArray())) return -1; - } /* prepare a list of objects to use in the vm definition so that we don't * have to roll back later */ @@ -15154,8 +15126,6 @@ qemuDomainSnapshotCreateActiveExternal(virQEMUDriverPtr driver, char *xml = NULL; bool memory = snap->def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL; bool memory_unlink = false; - bool atomic = !!(flags & VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC); - bool transaction = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_TRANSACTION); int thaw = 0; /* 1 if freeze succeeded, -1 if freeze failed */ bool pmsuspended = false; virQEMUDriverConfigPtr cfg = NULL; @@ -15163,6 +15133,14 @@ qemuDomainSnapshotCreateActiveExternal(virQEMUDriverPtr driver, char *compressedpath = NULL; virQEMUSaveDataPtr data = NULL; + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DISK_SNAPSHOT) || + !virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_TRANSACTION)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("live disk snapshot not supported with this " + "QEMU binary")); + return -1; + } + /* If quiesce was requested, then issue a freeze command, and a * counterpart thaw command when it is actually sent to agent. * The command will fail if the guest is paused or the guest agent @@ -15197,20 +15175,11 @@ qemuDomainSnapshotCreateActiveExternal(virQEMUDriverPtr driver, } else if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { /* For external checkpoints (those with memory), the guest * must pause (either by libvirt up front, or by qemu after - * _LIVE converges). For disk-only snapshots with multiple - * disks, libvirt must pause externally to get all snapshots - * to be at the same point in time, unless qemu supports - * transactions. For a single disk, snapshot is atomic - * without requiring a pause. Thanks to - * qemuDomainSnapshotPrepare, if we got to this point, the - * atomic flag now says whether we need to pause, and a - * capability bit says whether to use transaction. - */ + * _LIVE converges). */ if (memory) resume = true; - if ((memory && !(flags & VIR_DOMAIN_SNAPSHOT_CREATE_LIVE)) || - (!memory && atomic && !transaction)) { + if (memory && !(flags & VIR_DOMAIN_SNAPSHOT_CREATE_LIVE)) { if (qemuProcessStopCPUs(driver, vm, VIR_DOMAIN_PAUSED_SNAPSHOT, QEMU_ASYNC_JOB_SNAPSHOT) < 0) goto cleanup; @@ -15265,13 +15234,8 @@ qemuDomainSnapshotCreateActiveExternal(virQEMUDriverPtr driver, qemuDomainObjSetAsyncJobMask(vm, QEMU_JOB_DEFAULT_MASK); } - /* now the domain is now paused if: - * - if a memory snapshot was requested - * - an atomic snapshot was requested AND - * qemu does not support transactions - * - * Next we snapshot the disks. - */ + /* now the domain is now paused if a memory snapshot was requested */ + if ((ret = qemuDomainSnapshotCreateDiskActive(driver, vm, snap, flags, QEMU_ASYNC_JOB_SNAPSHOT)) < 0) goto cleanup; -- 2.16.2

Since we now always do the snapshot via the 'transaction' command we can drop the code which would enter monitor for individual disk snapshots. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 25 +++---------------------- 1 file changed, 3 insertions(+), 22 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4f577e50d2..39b745b1db 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14925,14 +14925,12 @@ qemuDomainSnapshotUpdateDiskSources(qemuDomainSnapshotDiskDataPtr dd, } -/* The domain is expected to hold monitor lock. */ static int qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuDomainSnapshotDiskDataPtr dd, virJSONValuePtr actions, - bool reuse, - qemuDomainAsyncJob asyncJob) + bool reuse) { qemuDomainObjPrivatePtr priv = vm->privateData; char *device = NULL; @@ -14964,23 +14962,10 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, dd->prepared = true; - /* create the actual snapshot */ formatStr = virStorageFileFormatTypeToString(dd->src->format); - /* The monitor is only accessed if qemu doesn't support transactions. - * Otherwise the following monitor command only constructs the command. - */ - if (!actions && - qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) - goto cleanup; - ret = rc = qemuMonitorDiskSnapshot(priv->mon, actions, device, source, formatStr, reuse); - if (!actions) { - if (qemuDomainObjExitMonitor(driver, vm) < 0) - ret = -1; - } - virDomainAuditDisk(vm, dd->disk->src, dd->src, "snapshot", rc >= 0); cleanup: @@ -15032,11 +15017,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, ret = qemuDomainSnapshotCreateSingleDiskActive(driver, vm, &diskdata[i], - actions, reuse, asyncJob); - - /* without transaction support the change can't be rolled back */ - if (!actions) - qemuDomainSnapshotUpdateDiskSources(&diskdata[i], &persist); + actions, reuse); if (ret < 0) goto error; @@ -15044,7 +15025,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, do_transaction = true; } - if (actions && do_transaction) { + if (do_transaction) { if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) goto cleanup; -- 2.16.2

In the cleanup path we already checked whether a snapshot needed to be taken by looking into the collected data. Use the same approach when creating the snapshot command data and when commiting the changes to the domain definition. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 39b745b1db..e5005fd829 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15012,7 +15012,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, * VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL with a valid file name and * qcow2 format. */ for (i = 0; i < snap->def->ndisks; i++) { - if (snap->def->disks[i].snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE) + if (!diskdata[i].src) continue; ret = qemuDomainSnapshotCreateSingleDiskActive(driver, vm, @@ -15036,8 +15036,14 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, goto error; } - for (i = 0; i < snap->def->ndisks; i++) + for (i = 0; i < snap->def->ndisks; i++) { + qemuDomainSnapshotDiskDataPtr dd = &diskdata[i]; + + if (!dd->src) + continue; + qemuDomainSnapshotUpdateDiskSources(&diskdata[i], &persist); + } } error: -- 2.16.2

Currently we'd audit that we managed to format the data for the 'transaction' command rather than the (un)successful attempt to create the snapshot. Move the auditing code so that it can actually audit the result of the 'transaction' command. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e5005fd829..ea06e23ff1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14936,7 +14936,7 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, char *device = NULL; char *source = NULL; const char *formatStr = NULL; - int ret = -1, rc; + int ret = -1; if (!(device = qemuAliasDiskDriveFromDisk(dd->disk))) goto cleanup; @@ -14964,9 +14964,8 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, formatStr = virStorageFileFormatTypeToString(dd->src->format); - ret = rc = qemuMonitorDiskSnapshot(priv->mon, actions, device, source, - formatStr, reuse); - virDomainAuditDisk(vm, dd->disk->src, dd->src, "snapshot", rc >= 0); + ret = qemuMonitorDiskSnapshot(priv->mon, actions, device, source, + formatStr, reuse); cleanup: VIR_FREE(device); @@ -15031,10 +15030,8 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, ret = qemuMonitorTransaction(priv->mon, &actions); - if (qemuDomainObjExitMonitor(driver, vm) < 0 || ret < 0) { + if (qemuDomainObjExitMonitor(driver, vm) < 0) ret = -1; - goto error; - } for (i = 0; i < snap->def->ndisks; i++) { qemuDomainSnapshotDiskDataPtr dd = &diskdata[i]; @@ -15042,8 +15039,14 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, if (!dd->src) continue; - qemuDomainSnapshotUpdateDiskSources(&diskdata[i], &persist); + virDomainAuditDisk(vm, dd->disk->src, dd->src, "snapshot", ret >= 0); + + if (ret == 0) + qemuDomainSnapshotUpdateDiskSources(dd, &persist); } + + if (ret < 0) + goto error; } error: -- 2.16.2

Add a new helper that will be solely used to create arguments for the transaction command. Later on this will make it possible to remove the overloading which was caused by the fact that snapshots were created without transaction and also will help in blockdevizing of snapshots. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor_json.c | 46 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 4 ++++ 2 files changed, 50 insertions(+) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 3e90279b71..54fefcb612 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -464,6 +464,52 @@ qemuMonitorJSONHasError(virJSONValuePtr reply, } +/** + * qemuMonitorJSONTransactionAdd: + * @actions: array of actions for the 'transaction' command + * @cmdname: command to add to @actions + * @...: arguments for @cmdname (see virJSONValueObjectAddVArgs for formatting) + * + * Add a new command with arguments to the existing ones. The resulting array + * is used as argument for the 'transaction' command. + * + * Returns 0 on success and -1 on error. + */ +int +qemuMonitorJSONTransactionAdd(virJSONValuePtr actions, + const char *cmdname, + ...) +{ + virJSONValuePtr entry = NULL; + virJSONValuePtr data = NULL; + va_list args; + int ret = -1; + + va_start(args, cmdname); + + if (virJSONValueObjectCreateVArgs(&data, args) < 0) + goto cleanup; + + if (virJSONValueObjectCreate(&entry, + "s:type", cmdname, + "A:data", &data, NULL) < 0) + goto cleanup; + + if (virJSONValueArrayAppend(actions, entry) < 0) + goto cleanup; + + entry = NULL; + ret = 0; + + cleanup: + virJSONValueFree(entry); + virJSONValueFree(data); + va_end(args); + + return ret; +} + + /** * qemuMonitorJSONMakeCommandInternal: * @cmdname: QMP command name diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 6bc0dd3ad2..da6c121d72 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -32,6 +32,10 @@ # include "cpu/cpu.h" # include "util/virgic.h" +int qemuMonitorJSONTransactionAdd(virJSONValuePtr actions, + const char *cmdname, + ...); + int qemuMonitorJSONIOProcessLine(qemuMonitorPtr mon, const char *line, qemuMonitorMessagePtr msg); -- 2.16.2

With 'transaction' support we don't need to keep around the multipurpose code which would create the snapshot if 'transaction' is not supported. To simplify this add a new helper that just wraps the arguments for 'blockdev-snapshot-sync' operation in 'transaction' and use it instead of qemuBlockSnapshotAddLegacy. Additionally this allows to format the arguments prior to creating the file for simpler cleanup. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 37 +++++++++++++++++++++++++++++++++++++ src/qemu/qemu_block.h | 6 ++++++ src/qemu/qemu_driver.c | 18 +++--------------- 3 files changed, 46 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 0ebf2d2aff..db1579ca20 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -19,8 +19,10 @@ #include <config.h> #include "qemu_block.h" +#include "qemu_command.h" #include "qemu_domain.h" #include "qemu_alias.h" +#include "qemu_monitor_json.h" #include "viralloc.h" #include "virstring.h" @@ -1683,3 +1685,38 @@ qemuBlockStorageSourceDetachOneBlockdev(virQEMUDriverPtr driver, return ret; } + + +int +qemuBlockSnapshotAddLegacy(virJSONValuePtr actions, + virDomainDiskDefPtr disk, + virStorageSourcePtr newsrc, + bool reuse) +{ + const char *format = virStorageFileFormatTypeToString(newsrc->format); + char *device = NULL; + char *source = NULL; + int ret = -1; + + if (!(device = qemuAliasDiskDriveFromDisk(disk))) + goto cleanup; + + if (qemuGetDriveSourceString(newsrc, NULL, &source) < 0) + goto cleanup; + + if (qemuMonitorJSONTransactionAdd(actions, "blockdev-snapshot-sync", + "s:device", device, + "s:snapshot-file", source, + "s:format", format, + "S:mode", reuse ? "existing" : NULL, + NULL) < 0) + goto cleanup; + + ret = 0; + + cleanup: + VIR_FREE(device); + VIR_FREE(source); + + return ret; +} diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h index 418b5064b5..fd8984e60b 100644 --- a/src/qemu/qemu_block.h +++ b/src/qemu/qemu_block.h @@ -117,4 +117,10 @@ qemuBlockStorageSourceDetachOneBlockdev(virQEMUDriverPtr driver, qemuDomainAsyncJob asyncJob, virStorageSourcePtr src); +int +qemuBlockSnapshotAddLegacy(virJSONValuePtr actions, + virDomainDiskDefPtr disk, + virStorageSourcePtr newsrc, + bool reuse); + #endif /* __QEMU_BLOCK_H__ */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ea06e23ff1..2d8af5daaa 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14932,23 +14932,16 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, virJSONValuePtr actions, bool reuse) { - qemuDomainObjPrivatePtr priv = vm->privateData; - char *device = NULL; - char *source = NULL; - const char *formatStr = NULL; int ret = -1; - if (!(device = qemuAliasDiskDriveFromDisk(dd->disk))) - goto cleanup; - - if (qemuGetDriveSourceString(dd->src, NULL, &source) < 0) + if (qemuBlockSnapshotAddLegacy(actions, dd->disk, dd->src, reuse) < 0) goto cleanup; /* pre-create the image file so that we can label it before handing it to qemu */ if (!reuse && dd->src->type != VIR_STORAGE_TYPE_BLOCK) { if (virStorageFileCreate(dd->src) < 0) { virReportSystemError(errno, _("failed to create image file '%s'"), - source); + NULLSTR(dd->src->path)); goto cleanup; } dd->created = true; @@ -14962,14 +14955,9 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, dd->prepared = true; - formatStr = virStorageFileFormatTypeToString(dd->src->format); - - ret = qemuMonitorDiskSnapshot(priv->mon, actions, device, source, - formatStr, reuse); + ret = 0; cleanup: - VIR_FREE(device); - VIR_FREE(source); return ret; } -- 2.16.2

Remove the dual mode code which allowed to create snapshots without support for 'transaction'. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor.c | 17 ----------------- src/qemu/qemu_monitor.h | 6 ------ src/qemu/qemu_monitor_json.c | 37 ------------------------------------- src/qemu/qemu_monitor_json.h | 8 -------- 4 files changed, 68 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 6ed475ede0..5d7f6905ae 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3177,23 +3177,6 @@ qemuMonitorDeleteSnapshot(qemuMonitorPtr mon, const char *name) } -/* Use the snapshot_blkdev command to convert the existing file for - * device into a read-only backing file of a new qcow2 image located - * at file. */ -int -qemuMonitorDiskSnapshot(qemuMonitorPtr mon, virJSONValuePtr actions, - const char *device, const char *file, - const char *format, bool reuse) -{ - VIR_DEBUG("actions=%p, device=%s, file=%s, format=%s, reuse=%d", - actions, device, file, format, reuse); - - QEMU_CHECK_MONITOR(mon); - - return qemuMonitorJSONDiskSnapshot(mon, actions, device, file, format, reuse); -} - - /* Start a drive-mirror block job. bandwidth is in bytes/sec. */ int qemuMonitorDriveMirror(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index b3d62324b4..e09ca14bfa 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -827,12 +827,6 @@ int qemuMonitorCreateSnapshot(qemuMonitorPtr mon, const char *name); int qemuMonitorLoadSnapshot(qemuMonitorPtr mon, const char *name); int qemuMonitorDeleteSnapshot(qemuMonitorPtr mon, const char *name); -int qemuMonitorDiskSnapshot(qemuMonitorPtr mon, - virJSONValuePtr actions, - const char *device, - const char *file, - const char *format, - bool reuse); int qemuMonitorTransaction(qemuMonitorPtr mon, virJSONValuePtr *actions) ATTRIBUTE_NONNULL(2); int qemuMonitorDriveMirror(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 54fefcb612..cf1636d858 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4101,43 +4101,6 @@ int qemuMonitorJSONDelObject(qemuMonitorPtr mon, } -int -qemuMonitorJSONDiskSnapshot(qemuMonitorPtr mon, virJSONValuePtr actions, - const char *device, const char *file, - const char *format, bool reuse) -{ - int ret = -1; - virJSONValuePtr cmd; - virJSONValuePtr reply = NULL; - - cmd = qemuMonitorJSONMakeCommandRaw(actions != NULL, - "blockdev-snapshot-sync", - "s:device", device, - "s:snapshot-file", file, - "s:format", format, - "S:mode", reuse ? "existing" : NULL, - NULL); - if (!cmd) - return -1; - - if (actions) { - if (virJSONValueArrayAppend(actions, cmd) == 0) { - ret = 0; - cmd = NULL; - } - } else { - if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0) - goto cleanup; - - ret = qemuMonitorJSONCheckError(cmd, reply); - } - - cleanup: - virJSONValueFree(cmd); - virJSONValueFree(reply); - return ret; -} - /* speed is in bytes/sec */ int qemuMonitorJSONDriveMirror(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index da6c121d72..b61046379c 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -242,14 +242,6 @@ int qemuMonitorJSONAddObject(qemuMonitorPtr mon, int qemuMonitorJSONDelObject(qemuMonitorPtr mon, const char *objalias); -int qemuMonitorJSONDiskSnapshot(qemuMonitorPtr mon, - virJSONValuePtr actions, - const char *device, - const char *file, - const char *format, - bool reuse) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3) - ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5); int qemuMonitorJSONTransaction(qemuMonitorPtr mon, virJSONValuePtr *actions) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int qemuMonitorJSONDriveMirror(qemuMonitorPtr mon, -- 2.16.2

Now that we use only the separate function for creating data for the 'transaction' command we can remove all the boilerplate which was necessary before. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor_json.c | 42 ++++++++++-------------------------------- 1 file changed, 10 insertions(+), 32 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index cf1636d858..87ec496d0d 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -514,48 +514,30 @@ qemuMonitorJSONTransactionAdd(virJSONValuePtr actions, * qemuMonitorJSONMakeCommandInternal: * @cmdname: QMP command name * @arguments: a JSON object containing command arguments or NULL - * @transaction: format the command as arguments for the 'transaction' command * - * Create a JSON object used on the QMP monitor to call a command. If - * @transaction is true, the returned object is formatted to be used as a member - * of the 'transaction' command. + * Create a JSON object used on the QMP monitor to call a command. * * Note that @arguments is always consumed and should not be referenced after * the call to this function. */ static virJSONValuePtr qemuMonitorJSONMakeCommandInternal(const char *cmdname, - virJSONValuePtr arguments, - bool transaction) + virJSONValuePtr arguments) { - virJSONValuePtr cmd = NULL; virJSONValuePtr ret = NULL; - if (!transaction) { - if (virJSONValueObjectCreate(&cmd, - "s:execute", cmdname, - "A:arguments", &arguments, NULL) < 0) - goto cleanup; - } else { - if (virJSONValueObjectCreate(&cmd, - "s:type", cmdname, - "A:data", &arguments, NULL) < 0) - goto cleanup; - } - - VIR_STEAL_PTR(ret, cmd); + ignore_value(virJSONValueObjectCreate(&ret, + "s:execute", cmdname, + "A:arguments", &arguments, NULL)); - cleanup: virJSONValueFree(arguments); - virJSONValueFree(cmd); return ret; } static virJSONValuePtr ATTRIBUTE_SENTINEL -qemuMonitorJSONMakeCommandRaw(bool transaction, - const char *cmdname, - ...) +qemuMonitorJSONMakeCommand(const char *cmdname, + ...) { virJSONValuePtr obj = NULL; virJSONValuePtr jargs = NULL; @@ -566,7 +548,7 @@ qemuMonitorJSONMakeCommandRaw(bool transaction, if (virJSONValueObjectCreateVArgs(&jargs, args) < 0) goto cleanup; - obj = qemuMonitorJSONMakeCommandInternal(cmdname, jargs, transaction); + obj = qemuMonitorJSONMakeCommandInternal(cmdname, jargs); cleanup: va_end(args); @@ -574,9 +556,6 @@ qemuMonitorJSONMakeCommandRaw(bool transaction, return obj; } -#define qemuMonitorJSONMakeCommand(cmdname, ...) \ - qemuMonitorJSONMakeCommandRaw(false, cmdname, __VA_ARGS__) - static virJSONValuePtr qemuMonitorJSONKeywordStringToJSON(const char *str, const char *firstkeyword) @@ -4057,7 +4036,7 @@ qemuMonitorJSONAddObject(qemuMonitorPtr mon, virJSONValuePtr cmd; virJSONValuePtr reply = NULL; - if (!(cmd = qemuMonitorJSONMakeCommandInternal("object-add", props, false))) + if (!(cmd = qemuMonitorJSONMakeCommandInternal("object-add", props))) goto cleanup; if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) @@ -7964,8 +7943,7 @@ qemuMonitorJSONBlockdevAdd(qemuMonitorPtr mon, virJSONValuePtr reply = NULL; int ret = -1; - if (!(cmd = qemuMonitorJSONMakeCommandInternal("blockdev-add", - props, false))) + if (!(cmd = qemuMonitorJSONMakeCommandInternal("blockdev-add", props))) return -1; if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) -- 2.16.2