Re: [PATCH 3/3] qemu: Change return value of SaveImageGetCompressionProgram

qemuSaveImageGetCompressionProgram is a bit overloaded. Along with getting a compression program, it checks the validity of the image format and returns the integer representation of the format. Change the function to only handle retrieving the specified compression program, returning success or failure. Checking the validity of the image format can be left to the calling functions. Signed-off-by: Jim Fehlig <jfehlig(a)suse.com&gt; --- src/qemu/qemu_driver.c | 33 +++++++++++++---------- src/qemu/qemu_saveimage.c | 55 ++++++++++++--------------------------- src/qemu/qemu_saveimage.h | 2 +- src/qemu/qemu_snapshot.c | 11 +++++--- 4 files changed, 43 insertions(+), 58 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0a1bcc0ed5..577e799c2b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2752,7 +2752,7 @@ qemuDomainManagedSaveHelper(virQEMUDriver *driver, g_autoptr(virQEMUDriverConfig) cfg = NULL; g_autoptr(virCommand) compressor = NULL; g_autofree char *path = NULL; - int format; + int format = QEMU_SAVE_FORMAT_RAW; if (virDomainObjCheckActive(vm) < 0) return -1; @@ -2764,9 +2764,11 @@ qemuDomainManagedSaveHelper(virQEMUDriver *driver, } cfg = virQEMUDriverGetConfig(driver); - if ((format = qemuSaveImageGetCompressionProgram(cfg->saveImageFormat, - &compressor, - "save")) < 0) + if (cfg->saveImageFormat && + (format = qemuSaveFormatTypeFromString(cfg->saveImageFormat)) < 0) + return -1;

+ if (qemuSaveImageGetCompressionProgram(format, &compressor, "save") < 0) return -1;

path = qemuDomainManagedSavePath(driver, vm); @@ -2787,7 +2789,7 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char *dxml, unsigned int flags) { virQEMUDriver *driver = dom->conn->privateData; - int format; + int format = QEMU_SAVE_FORMAT_RAW; g_autoptr(virCommand) compressor = NULL; int ret = -1; virDomainObj *vm = NULL; @@ -2798,9 +2800,11 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char *dxml, VIR_DOMAIN_SAVE_PAUSED, -1); cfg = virQEMUDriverGetConfig(driver); - if ((format = qemuSaveImageGetCompressionProgram(cfg->saveImageFormat, - &compressor, - "save")) < 0) + if (cfg->saveImageFormat && + (format = qemuSaveFormatTypeFromString(cfg->saveImageFormat)) < 0) + goto cleanup; + + if (qemuSaveImageGetCompressionProgram(format, &compressor, "save") < 0) goto cleanup; if (!(vm = qemuDomainObjFromDomain(dom))) @@ -2838,7 +2842,7 @@ qemuDomainSaveParams(virDomainPtr dom, g_autoptr(virCommand) compressor = NULL; const char *to = NULL; const char *dxml = NULL; - int format; + int format = QEMU_SAVE_FORMAT_RAW; int ret = -1; virCheckFlags(VIR_DOMAIN_SAVE_BYPASS_CACHE | @@ -2872,9 +2876,11 @@ qemuDomainSaveParams(virDomainPtr dom, } cfg = virQEMUDriverGetConfig(driver); - if ((format = qemuSaveImageGetCompressionProgram(cfg->saveImageFormat, - &compressor, - "save")) < 0) + if (cfg->saveImageFormat && + (format = qemuSaveFormatTypeFromString(cfg->saveImageFormat)) < 0) + goto cleanup; + + if (qemuSaveImageGetCompressionProgram(format, &compressor, "save") < 0) goto cleanup; if (virDomainObjCheckActive(vm) < 0) @@ -3097,8 +3103,7 @@ doCoreDump(virQEMUDriver *driver, * format in "save" and "dump". If the compression program doesn't exist, * reset any errors and continue on using the raw format. */ - if (qemuSaveImageGetCompressionProgram(cfg->dumpImageFormat, - &compressor, "dump") < 0) { + if (qemuSaveImageGetCompressionProgram(format, &compressor, "dump") < 0) { virResetLastError(); VIR_WARN("Compression program for dump image format in " "configuration file isn't available, using raw"); diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c index eea35df175..d940bfb5c3 100644 --- a/src/qemu/qemu_saveimage.c +++ b/src/qemu/qemu_saveimage.c @@ -508,63 +508,40 @@ qemuSaveImageCreate(virQEMUDriver *driver, /* qemuSaveImageGetCompressionProgram: - * @imageFormat: String representation from qemu.conf of the image format - * being used (dump, save, or snapshot). + * @format: Integer representation of the image format being used + * (dump, save, or snapshot). * @compresspath: Pointer to a character string to store the fully qualified * path from virFindFileInPath. * @styleFormat: String representing the style of format (dump, save, snapshot) * - * Returns: - * virQEMUSaveFormat - Integer representation of the save image - * format to be used for particular style - * (e.g. dump, save, or snapshot). - * QEMU_SAVE_FORMAT_RAW - If there is no qemu.conf imageFormat value or - * no there was an error, then just return RAW - * indicating none. + * Returns -1 on failure, 0 on success. */ int -qemuSaveImageGetCompressionProgram(const char *imageFormat, +qemuSaveImageGetCompressionProgram(int format, virCommand **compressor, const char *styleFormat) { - int ret; + const char *imageFormat = qemuSaveFormatTypeToString(format); const char *prog; *compressor = NULL; - if (!imageFormat) - return QEMU_SAVE_FORMAT_RAW; - - if ((ret = qemuSaveFormatTypeFromString(imageFormat)) < 0) - goto error; - - if (ret == QEMU_SAVE_FORMAT_RAW) - return QEMU_SAVE_FORMAT_RAW; - - if (!(prog = virFindFileInPath(imageFormat))) - goto error; - + if (format == QEMU_SAVE_FORMAT_RAW) + return 0; + + if (!(prog = virFindFileInPath(imageFormat))) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("Compression program for %1$s image format in configuration file isn't available"), + styleFormat); + return -1; + } *compressor = virCommandNew(prog); virCommandAddArg(*compressor, "-c"); - if (ret == QEMU_SAVE_FORMAT_XZ) + if (format == QEMU_SAVE_FORMAT_XZ) virCommandAddArg(*compressor, "-3"); - return ret; - - error: - if (ret < 0) { - ret = QEMU_SAVE_FORMAT_RAW; - virReportError(VIR_ERR_OPERATION_FAILED, - _("Invalid %1$s image format specified in configuration file"), - styleFormat); - } else { - virReportError(VIR_ERR_OPERATION_FAILED, - _("Compression program for %1$s image format in configuration file isn't available"), - styleFormat); - } - - return ret; + return 0; } diff --git a/src/qemu/qemu_saveimage.h b/src/qemu/qemu_saveimage.h index aa905768de..0d8ee542af 100644 --- a/src/qemu/qemu_saveimage.h +++ b/src/qemu/qemu_saveimage.h @@ -110,7 +110,7 @@ qemuSaveImageOpen(virQEMUDriver *driver, ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4); int -qemuSaveImageGetCompressionProgram(const char *imageFormat, +qemuSaveImageGetCompressionProgram(int format, virCommand **compressor, const char *styleFormat) ATTRIBUTE_NONNULL(2); diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 4ff7e09bd4..c672d4f24f 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -1579,7 +1579,7 @@ qemuSnapshotCreateActiveExternal(virQEMUDriver *driver, bool memory_existing = false; bool thaw = false; bool pmsuspended = false; - int format; + int format = QEMU_SAVE_FORMAT_RAW; g_autoptr(virCommand) compressor = NULL; virQEMUSaveData *data = NULL; g_autoptr(GHashTable) blockNamedNodeData = NULL; @@ -1656,9 +1656,12 @@ qemuSnapshotCreateActiveExternal(virQEMUDriver *driver, JOB_MASK(VIR_JOB_SUSPEND) | JOB_MASK(VIR_JOB_MIGRATION_OP))); - if ((format = qemuSaveImageGetCompressionProgram(cfg->snapshotImageFormat, - &compressor, - "snapshot")) < 0) + if (cfg->snapshotImageFormat && + (format = qemuSaveFormatTypeFromString(cfg->snapshotImageFormat)) < 0) + goto cleanup; + + if (qemuSaveImageGetCompressionProgram(format, &compressor, + "snapshot") < 0) goto cleanup; if (!(xml = qemuDomainDefFormatLive(driver, priv->qemuCaps, -- 2.43.0