[libvirt] [PATCHv3 5/7] qemu: blockPull: Refactor the rest of qemuDomainBlockJobImpl

Since it now handles only block pull code paths we can refactor it and remove tons of cruft. --- Notes: Version 3: - no change, ACKed and explained src/qemu/qemu_driver.c | 86 ++++++++++++++++++++------------------------ src/qemu/qemu_monitor.c | 30 ++++++++-------- src/qemu/qemu_monitor.h | 17 ++++----- src/qemu/qemu_monitor_json.c | 59 +++++++----------------------- src/qemu/qemu_monitor_json.h | 13 ++++--- 5 files changed, 78 insertions(+), 127 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1c0e966..2a2c26a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16156,17 +16156,16 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver, /* bandwidth in MiB/s per public API. Caller must lock vm beforehand, * and not access it afterwards. */ static int -qemuDomainBlockJobImpl(virDomainObjPtr vm, - virConnectPtr conn, - const char *path, const char *base, - unsigned long bandwidth, - int mode, unsigned int flags) +qemuDomainBlockPullCommon(virQEMUDriverPtr driver, + virDomainObjPtr vm, + const char *path, + const char *base, + unsigned long bandwidth, + unsigned int flags) { - virQEMUDriverPtr driver = conn->privateData; - qemuDomainObjPrivatePtr priv; + qemuDomainObjPrivatePtr priv = vm->privateData; char *device = NULL; - int ret = -1; - bool async = false; + bool modern; int idx; virDomainDiskDefPtr disk; virStorageSourcePtr baseSource = NULL; @@ -16174,12 +16173,7 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, char *basePath = NULL; char *backingPath = NULL; unsigned long long speed = bandwidth; - - if (!virDomainObjIsActive(vm)) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("domain is not running")); - goto cleanup; - } + int ret = -1; if (flags & VIR_DOMAIN_BLOCK_REBASE_RELATIVE && !base) { virReportError(VIR_ERR_INVALID_ARG, "%s", @@ -16188,23 +16182,23 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, goto cleanup; } - priv = vm->privateData; - if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKJOB_ASYNC)) { - async = true; - } else if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKJOB_SYNC)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("block jobs not supported with this QEMU binary")); - goto cleanup; - } else if (base) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("partial block pull not supported with this " - "QEMU binary")); - goto cleanup; - } else if (mode == BLOCK_JOB_PULL && bandwidth) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("setting bandwidth at start of block pull not " - "supported with this QEMU binary")); + if (qemuDomainSupportsBlockJobs(vm, &modern) < 0) goto cleanup; + + if (!modern) { + if (base) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("partial block pull not supported with this " + "QEMU binary")); + goto cleanup; + } + + if (bandwidth) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("setting bandwidth at start of block pull not " + "supported with this QEMU binary")); + goto cleanup; + } } if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) @@ -16216,12 +16210,11 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, goto endjob; } - device = qemuDiskPathToAlias(vm, path, &idx); - if (!device) + if (!(device = qemuDiskPathToAlias(vm, path, &idx))) goto endjob; disk = vm->def->disks[idx]; - if (mode == BLOCK_JOB_PULL && qemuDomainDiskBlockJobIsActive(disk)) + if (qemuDomainDiskBlockJobIsActive(disk)) goto endjob; if (base && @@ -16244,7 +16237,6 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, &backingPath) < 0) goto endjob; - if (!backingPath) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("can't keep relative backing relationship")); @@ -16254,8 +16246,7 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, } /* Convert bandwidth MiB to bytes, if needed */ - if (mode == BLOCK_JOB_PULL && - !(flags & VIR_DOMAIN_BLOCK_PULL_BANDWIDTH_BYTES)) { + if (!(flags & VIR_DOMAIN_BLOCK_PULL_BANDWIDTH_BYTES)) { if (speed > LLONG_MAX >> 20) { virReportError(VIR_ERR_OVERFLOW, _("bandwidth must be less than %llu"), @@ -16270,15 +16261,15 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, basePath = qemuMonitorDiskNameLookup(priv->mon, device, disk->src, baseSource); if (!baseSource || basePath) - ret = qemuMonitorBlockJob(priv->mon, device, basePath, backingPath, - speed, mode, async); + ret = qemuMonitorBlockStream(priv->mon, device, basePath, backingPath, + speed, modern); if (qemuDomainObjExitMonitor(driver, vm) < 0) ret = -1; - if (ret < 0) { + + if (ret < 0) goto endjob; - } else if (mode == BLOCK_JOB_PULL) { - disk->blockjob = true; - } + + disk->blockjob = true; endjob: qemuDomainObjEndJob(driver, vm); @@ -16291,6 +16282,7 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, return ret; } + static int qemuDomainBlockJobAbort(virDomainPtr dom, const char *path, @@ -16773,6 +16765,7 @@ static int qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base, unsigned long bandwidth, unsigned int flags) { + virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; int ret = -1; unsigned long long speed = bandwidth; @@ -16795,8 +16788,7 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base, /* For normal rebase (enhanced blockpull), the common code handles * everything, including vm cleanup. */ if (!(flags & VIR_DOMAIN_BLOCK_REBASE_COPY)) - return qemuDomainBlockJobImpl(vm, dom->conn, path, base, bandwidth, - BLOCK_JOB_PULL, flags); + return qemuDomainBlockPullCommon(driver, vm, path, base, bandwidth, flags); /* If we got here, we are doing a block copy rebase. */ if (VIR_ALLOC(dest) < 0) @@ -16934,8 +16926,8 @@ qemuDomainBlockPull(virDomainPtr dom, const char *path, unsigned long bandwidth, return -1; } - return qemuDomainBlockJobImpl(vm, dom->conn, path, NULL, bandwidth, - BLOCK_JOB_PULL, flags); + return qemuDomainBlockPullCommon(dom->conn->privateData, + vm, path, NULL, bandwidth, flags); } diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 882d2e3..2846605 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3577,28 +3577,26 @@ int qemuMonitorScreendump(qemuMonitorPtr mon, /* bandwidth is in bytes/sec */ int -qemuMonitorBlockJob(qemuMonitorPtr mon, - const char *device, - const char *base, - const char *backingName, - unsigned long long bandwidth, - qemuMonitorBlockJobCmd mode, - bool modern) +qemuMonitorBlockStream(qemuMonitorPtr mon, + const char *device, + const char *base, + const char *backingName, + unsigned long long bandwidth, + bool modern) { - int ret = -1; - VIR_DEBUG("mon=%p, device=%s, base=%s, backingName=%s, bandwidth=%lluB, " - "mode=%o, modern=%d", + "modern=%d", mon, device, NULLSTR(base), NULLSTR(backingName), - bandwidth, mode, modern); + bandwidth, modern); - if (mon->json) - ret = qemuMonitorJSONBlockJob(mon, device, base, backingName, - bandwidth, mode, modern); - else + if (!mon->json) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("block jobs require JSON monitor")); - return ret; + return -1; + } + + return qemuMonitorJSONBlockStream(mon, device, base, backingName, + bandwidth, modern); } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 5cc811a..d673154 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -755,17 +755,12 @@ int qemuMonitorSendKey(qemuMonitorPtr mon, unsigned int *keycodes, unsigned int nkeycodes); -typedef enum { - BLOCK_JOB_PULL, -} qemuMonitorBlockJobCmd; - -int qemuMonitorBlockJob(qemuMonitorPtr mon, - const char *device, - const char *base, - const char *backingName, - unsigned long long bandwidth, - qemuMonitorBlockJobCmd mode, - bool modern) +int qemuMonitorBlockStream(qemuMonitorPtr mon, + const char *device, + const char *base, + const char *backingName, + unsigned long long bandwidth, + bool modern) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int qemuMonitorBlockJobCancel(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 97a6d9f..3af319c 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4282,57 +4282,24 @@ qemuMonitorJSONBlockJobError(virJSONValuePtr reply, /* speed is in bytes/sec */ int -qemuMonitorJSONBlockJob(qemuMonitorPtr mon, - const char *device, - const char *base, - const char *backingName, - unsigned long long speed, - qemuMonitorBlockJobCmd mode, - bool modern) +qemuMonitorJSONBlockStream(qemuMonitorPtr mon, + const char *device, + const char *base, + const char *backingName, + unsigned long long speed, + bool modern) { int ret = -1; virJSONValuePtr cmd = NULL; virJSONValuePtr reply = NULL; - const char *cmd_name = NULL; - - if (base && (mode != BLOCK_JOB_PULL || !modern)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("only modern block pull supports base: %s"), base); - return -1; - } - - if (backingName && mode != BLOCK_JOB_PULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("backing name is supported only for block pull")); - return -1; - } - - if (backingName && !base) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("backing name requires a base image")); - return -1; - } + const char *cmd_name = modern ? "block-stream" : "block_stream"; - if (speed && mode == BLOCK_JOB_PULL && !modern) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("only modern block pull supports speed: %llu"), - speed); - return -1; - } - - switch (mode) { - case BLOCK_JOB_PULL: - cmd_name = modern ? "block-stream" : "block_stream"; - cmd = qemuMonitorJSONMakeCommand(cmd_name, - "s:device", device, - "Y:speed", speed, - "S:base", base, - "S:backing-file", backingName, - NULL); - break; - } - - if (!cmd) + if (!(cmd = qemuMonitorJSONMakeCommand(cmd_name, + "s:device", device, + "Y:speed", speed, + "S:base", base, + "S:backing-file", backingName, + NULL))) return -1; if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index c88972c..43adc90 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -297,13 +297,12 @@ int qemuMonitorJSONSendKey(qemuMonitorPtr mon, int qemuMonitorJSONScreendump(qemuMonitorPtr mon, const char *file); -int qemuMonitorJSONBlockJob(qemuMonitorPtr mon, - const char *device, - const char *base, - const char *backingName, - unsigned long long speed, - qemuMonitorBlockJobCmd mode, - bool modern) +int qemuMonitorJSONBlockStream(qemuMonitorPtr mon, + const char *device, + const char *base, + const char *backingName, + unsigned long long speed, + bool modern) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int qemuMonitorJSONBlockJobCancel(qemuMonitorPtr mon, -- 2.2.2