6:30 a.m.
v1: http://www.redhat.com/archives/libvir-list/2016-September/msg00971.html
Changes in v2:
1. Create a patch 5 which alters the qemuGetCompressionProgram to take a
new const char * parameter which will be used in the warning message
as the %s image format "style" (dump, save, snapshot) for both messages.
Also removed the unnecessary translated message marker _() for the
VIR_WARN message.
2. Alter former patch 5 to pass the style ("save" or "snapshot") as
an
argument for the altered virReportError messages which now will take
a parameter that describe their style. For one message it keeps it
constant, for the other it adds the style into the message.
3. Alter former patches 6 and 7 to account for the new argument (no real
changes, just handling merge conflicts).
(from patch 1's cover)
Rather than try to describe what I was thinking about for the passing
the compress program directly series from Chen Hanxiao:
http://www.redhat.com/archives/libvir-list/2016-September/msg00677.html
and
http://www.redhat.com/archives/libvir-list/2016-August/msg01237.html
I took the liberty of creating my own set of patches which should end
in the more or less same result.
The primary benefit is not calling virFileInPath twice since we already
get it at least once for the various compressed program callers, let's
pass what we originally found.
John Ferlan (8):
qemu: Move getCompressionType
qemu: Adjust doCoreDump to call getCompressionType
qemu: Introduce helper qemuGetCompressionProgram
qemu: Remove getCompressionType
qemu: Alter qemuGetCompressionProgram warning message
qemu: Use qemuGetCompressionProgram for error paths
qemu: Remove qemuCompressProgramAvailable
qemu: Get/return compressedpath program
src/qemu/qemu_driver.c | 241 ++++++++++++++++++++++++-------------------------
1 file changed, 117 insertions(+), 124 deletions(-)
--
2.7.4
6:30 a.m.
New subject: [libvirt] [PATCH v2 1/8] qemu: Move getCompressionType
A subsequent patch will adjust the 3 callers to just call from doCoreDump.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_driver.c | 70 ++++++++++++++++++++++++++------------------------
1 file changed, 36 insertions(+), 34 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ee16cb5..af43695 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3266,6 +3266,42 @@ qemuCompressProgramAvailable(virQEMUSaveFormat compress)
return true;
}
+
+static virQEMUSaveFormat
+getCompressionType(virQEMUDriverPtr driver)
+{
+ int ret = QEMU_SAVE_FORMAT_RAW;
+ virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+
+ /*
+ * We reuse "save" flag for "dump" here. Then, we can support the
same
+ * format in "save" and "dump".
+ */
+ if (cfg->dumpImageFormat) {
+ ret = qemuSaveCompressionTypeFromString(cfg->dumpImageFormat);
+ /* Use "raw" as the format if the specified format is not valid,
+ * or the compress program is not available.
+ */
+ if (ret < 0) {
+ VIR_WARN("%s", _("Invalid dump image format specified in
"
+ "configuration file, using raw"));
+ ret = QEMU_SAVE_FORMAT_RAW;
+ goto cleanup;
+ }
+ if (!qemuCompressProgramAvailable(ret)) {
+ VIR_WARN("%s", _("Compression program for dump image format
"
+ "in configuration file isn't available, "
+ "using raw"));
+ ret = QEMU_SAVE_FORMAT_RAW;
+ goto cleanup;
+ }
+ }
+ cleanup:
+ virObjectUnref(cfg);
+ return ret;
+}
+
+
static int
qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char *dxml,
unsigned int flags)
@@ -3615,40 +3651,6 @@ doCoreDump(virQEMUDriverPtr driver,
return ret;
}
-static virQEMUSaveFormat
-getCompressionType(virQEMUDriverPtr driver)
-{
- int ret = QEMU_SAVE_FORMAT_RAW;
- virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
-
- /*
- * We reuse "save" flag for "dump" here. Then, we can support the
same
- * format in "save" and "dump".
- */
- if (cfg->dumpImageFormat) {
- ret = qemuSaveCompressionTypeFromString(cfg->dumpImageFormat);
- /* Use "raw" as the format if the specified format is not valid,
- * or the compress program is not available.
- */
- if (ret < 0) {
- VIR_WARN("%s", _("Invalid dump image format specified in
"
- "configuration file, using raw"));
- ret = QEMU_SAVE_FORMAT_RAW;
- goto cleanup;
- }
- if (!qemuCompressProgramAvailable(ret)) {
- VIR_WARN("%s", _("Compression program for dump image format
"
- "in configuration file isn't available, "
- "using raw"));
- ret = QEMU_SAVE_FORMAT_RAW;
- goto cleanup;
- }
- }
- cleanup:
- virObjectUnref(cfg);
- return ret;
-}
-
static int
qemuDomainCoreDumpWithFormat(virDomainPtr dom,
--
2.7.4
6:30 a.m.
New subject: [libvirt] [PATCH v2 2/8] qemu: Adjust doCoreDump to call getCompressionType
Rather than calling getCompressionType from each of the callers, just call
it from doCoreDump. A subsequent patch will be adjust the code even more.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_driver.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index af43695..956bddd 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3568,7 +3568,6 @@ static int
doCoreDump(virQEMUDriverPtr driver,
virDomainObjPtr vm,
const char *path,
- virQEMUSaveFormat compress,
unsigned int dump_flags,
unsigned int dumpformat)
{
@@ -3578,6 +3577,7 @@ doCoreDump(virQEMUDriverPtr driver,
int directFlag = 0;
unsigned int flags = VIR_FILE_WRAPPER_NON_BLOCKING;
const char *memory_dump_format = NULL;
+ virQEMUSaveFormat compress = getCompressionType(driver);
/* Create an empty file with appropriate ownership. */
if (dump_flags & VIR_DUMP_BYPASS_CACHE) {
@@ -3704,9 +3704,7 @@ qemuDomainCoreDumpWithFormat(virDomainPtr dom,
}
}
- ret = doCoreDump(driver, vm, path, getCompressionType(driver), flags,
- dumpformat);
- if (ret < 0)
+ if ((ret = doCoreDump(driver, vm, path, flags, dumpformat)) < 0)
goto endjob;
paused = true;
@@ -3911,10 +3909,8 @@ processWatchdogEvent(virQEMUDriverPtr driver,
}
flags |= cfg->autoDumpBypassCache ? VIR_DUMP_BYPASS_CACHE: 0;
- ret = doCoreDump(driver, vm, dumpfile,
- getCompressionType(driver), flags,
- VIR_DOMAIN_CORE_DUMP_FORMAT_RAW);
- if (ret < 0)
+ if ((ret = doCoreDump(driver, vm, dumpfile, flags,
+ VIR_DOMAIN_CORE_DUMP_FORMAT_RAW)) < 0)
virReportError(VIR_ERR_OPERATION_FAILED,
"%s", _("Dump failed"));
@@ -3951,10 +3947,8 @@ doCoreDumpToAutoDumpPath(virQEMUDriverPtr driver,
goto cleanup;
flags |= cfg->autoDumpBypassCache ? VIR_DUMP_BYPASS_CACHE: 0;
- ret = doCoreDump(driver, vm, dumpfile,
- getCompressionType(driver), flags,
- VIR_DOMAIN_CORE_DUMP_FORMAT_RAW);
- if (ret < 0)
+ if ((ret = doCoreDump(driver, vm, dumpfile, flags,
+ VIR_DOMAIN_CORE_DUMP_FORMAT_RAW)) < 0)
virReportError(VIR_ERR_OPERATION_FAILED,
"%s", _("Dump failed"));
cleanup:
--
2.7.4
6:30 a.m.
New subject: [libvirt] [PATCH v2 3/8] qemu: Introduce helper qemuGetCompressionProgram
Split out the guts of getCompressionType to perform the same functionality
in the new helper program with a subsequent patch goal to be reusable for
other callers making similar checks/calls to ensure the compression type
is valid and that the compression program cannot be found.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_driver.c | 67 ++++++++++++++++++++++++++++++++++----------------
1 file changed, 46 insertions(+), 21 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 956bddd..3f03576 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3267,36 +3267,61 @@ qemuCompressProgramAvailable(virQEMUSaveFormat compress)
}
+/* qemuGetCompressionProgram:
+ * @imageFormat: String representation from qemu.conf for the compression
+ * image format being used (dump, save, or snapshot).
+ *
+ * Returns:
+ * virQEMUSaveFormat - Integer representation of the compression
+ * program 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.
+ */
+static virQEMUSaveFormat
+qemuGetCompressionProgram(const char *imageFormat)
+{
+ virQEMUSaveFormat ret;
+
+ if (!imageFormat)
+ return QEMU_SAVE_FORMAT_RAW;
+
+ if ((ret = qemuSaveCompressionTypeFromString(imageFormat)) < 0)
+ goto error;
+
+ if (!qemuCompressProgramAvailable(ret))
+ goto error;
+
+ return ret;
+
+ error:
+ if (ret < 0)
+ VIR_WARN("%s", _("Invalid dump image format specified in "
+ "configuration file, using raw"));
+ else
+ VIR_WARN("%s", _("Compression program for dump image format
"
+ "in configuration file isn't available, "
+ "using raw"));
+
+ /* Use "raw" as the format if the specified format is not valid,
+ * or the compress program is not available. */
+ return QEMU_SAVE_FORMAT_RAW;
+}
+
+
static virQEMUSaveFormat
getCompressionType(virQEMUDriverPtr driver)
{
- int ret = QEMU_SAVE_FORMAT_RAW;
+ int ret;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
/*
* We reuse "save" flag for "dump" here. Then, we can support the
same
* format in "save" and "dump".
*/
- if (cfg->dumpImageFormat) {
- ret = qemuSaveCompressionTypeFromString(cfg->dumpImageFormat);
- /* Use "raw" as the format if the specified format is not valid,
- * or the compress program is not available.
- */
- if (ret < 0) {
- VIR_WARN("%s", _("Invalid dump image format specified in
"
- "configuration file, using raw"));
- ret = QEMU_SAVE_FORMAT_RAW;
- goto cleanup;
- }
- if (!qemuCompressProgramAvailable(ret)) {
- VIR_WARN("%s", _("Compression program for dump image format
"
- "in configuration file isn't available, "
- "using raw"));
- ret = QEMU_SAVE_FORMAT_RAW;
- goto cleanup;
- }
- }
- cleanup:
+ ret = qemuGetCompressionProgram(cfg->dumpImageFormat);
+
virObjectUnref(cfg);
return ret;
}
--
2.7.4
11:40 a.m.
New subject: [libvirt] [PATCH v2 3/8] qemu: Introduce helper qemuGetCompressionProgram
On Fri, Sep 23, 2016 at 07:30:51AM -0400, John Ferlan wrote:
Split out the guts of getCompressionType to perform the same
functionality
in the new helper program with a subsequent patch goal to be reusable for
other callers making similar checks/calls to ensure the compression type
is valid and that the compression program cannot be found.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_driver.c | 67 ++++++++++++++++++++++++++++++++++----------------
1 file changed, 46 insertions(+), 21 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 956bddd..3f03576 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3267,36 +3267,61 @@ qemuCompressProgramAvailable(virQEMUSaveFormat compress)
}
+/* qemuGetCompressionProgram:
+ * @imageFormat: String representation from qemu.conf for the compression
+ * image format being used (dump, save, or snapshot).
+ *
+ * Returns:
+ * virQEMUSaveFormat - Integer representation of the compression
+ * program 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.
+ */
+static virQEMUSaveFormat
+qemuGetCompressionProgram(const char *imageFormat)
+{
+ virQEMUSaveFormat ret;
+
+ if (!imageFormat)
+ return QEMU_SAVE_FORMAT_RAW;
+
+ if ((ret = qemuSaveCompressionTypeFromString(imageFormat)) < 0)
+ goto error;
+
The list of supported values is known at compile time.
If the value is invalid, we should reject it when parsing the config
file.
That way the 'getCompressionProgram' helper would be faithful to its
name and only try to get the compression program.
Jan
3:38 p.m.
New subject: [libvirt] [PATCH v2 3/8] qemu: Introduce helper qemuGetCompressionProgram
On 09/27/2016 12:40 PM, Ján Tomko wrote:
On Fri, Sep 23, 2016 at 07:30:51AM -0400, John Ferlan wrote:
> Split out the guts of getCompressionType to perform the same
> functionality
> in the new helper program with a subsequent patch goal to be reusable for
> other callers making similar checks/calls to ensure the compression type
> is valid and that the compression program cannot be found.
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> src/qemu/qemu_driver.c | 67
> ++++++++++++++++++++++++++++++++++----------------
> 1 file changed, 46 insertions(+), 21 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 956bddd..3f03576 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -3267,36 +3267,61 @@ qemuCompressProgramAvailable(virQEMUSaveFormat
> compress)
> }
>
>
> +/* qemuGetCompressionProgram:
> + * @imageFormat: String representation from qemu.conf for the
> compression
> + * image format being used (dump, save, or snapshot).
> + *
> + * Returns:
> + * virQEMUSaveFormat - Integer representation of the compression
> + * program 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.
> + */
> +static virQEMUSaveFormat
> +qemuGetCompressionProgram(const char *imageFormat)
> +{
> + virQEMUSaveFormat ret;
> +
> + if (!imageFormat)
> + return QEMU_SAVE_FORMAT_RAW;
> +
> + if ((ret = qemuSaveCompressionTypeFromString(imageFormat)) < 0)
> + goto error;
> +
The list of supported values is known at compile time.
If the value is invalid, we should reject it when parsing the config
file.
I'm sure we could if there was a patch for it, but for each of the 3
different types (dump, save, and snapshot) the "check" for validity was
done within this scope (even before this set of patches).
If one peeks at the virQEMUDriverConfigLoadFile where each of the
cfg->{dump|save|snapshot}ImageFormat values are read, the validation of
what's been read (for other fields) is minimal especially for strings.
It seems it's left for "other" code to handle.
I'll review a patch that validates the various fetches from qemu_conf.c
and messages that "%s_image_format" has an invalid value.
That way the 'getCompressionProgram' helper would be faithful to its
name and only try to get the compression program.
If the name is that much of an issue, then feel free to change the name.
John
6:30 a.m.
New subject: [libvirt] [PATCH v2 4/8] qemu: Remove getCompressionType
There's only one caller now anyway... Besides it's just a shell for
getting the compress type. Subsequent patches will return the path
to the compression program.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_driver.c | 25 +++++++------------------
1 file changed, 7 insertions(+), 18 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3f03576..d5acc30 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3310,23 +3310,6 @@ qemuGetCompressionProgram(const char *imageFormat)
}
-static virQEMUSaveFormat
-getCompressionType(virQEMUDriverPtr driver)
-{
- int ret;
- virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
-
- /*
- * We reuse "save" flag for "dump" here. Then, we can support the
same
- * format in "save" and "dump".
- */
- ret = qemuGetCompressionProgram(cfg->dumpImageFormat);
-
- virObjectUnref(cfg);
- return ret;
-}
-
-
static int
qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char *dxml,
unsigned int flags)
@@ -3602,7 +3585,12 @@ doCoreDump(virQEMUDriverPtr driver,
int directFlag = 0;
unsigned int flags = VIR_FILE_WRAPPER_NON_BLOCKING;
const char *memory_dump_format = NULL;
- virQEMUSaveFormat compress = getCompressionType(driver);
+ virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+ virQEMUSaveFormat compress;
+
+ /* We reuse "save" flag for "dump" here. Then, we can support the
same
+ * format in "save" and "dump". */
+ compress = qemuGetCompressionProgram(cfg->dumpImageFormat);
/* Create an empty file with appropriate ownership. */
if (dump_flags & VIR_DUMP_BYPASS_CACHE) {
@@ -3673,6 +3661,7 @@ doCoreDump(virQEMUDriverPtr driver,
if (ret != 0)
unlink(path);
virFileWrapperFdFree(wrapperFd);
+ virObjectUnref(cfg);
return ret;
}
--
2.7.4
6:30 a.m.
New subject: [libvirt] [PATCH v2 5/8] qemu: Alter qemuGetCompressionProgram warning message
Add a new parameter 'styleFormat' to be used when printing the
warning message so that it's "clearer" what style of compression
call caused the error. Add that style to both messages as a paremter.
Also a VIR_WARN error message doesn't need to be translated
(e.g. inside _()), so remove the need for the translation.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_driver.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d5acc30..7dfba50 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3270,6 +3270,7 @@ qemuCompressProgramAvailable(virQEMUSaveFormat compress)
/* qemuGetCompressionProgram:
* @imageFormat: String representation from qemu.conf for the compression
* image format being used (dump, save, or snapshot).
+ * @styleFormat: String representing the style of format (dump, save, snapshot)
*
* Returns:
* virQEMUSaveFormat - Integer representation of the compression
@@ -3280,7 +3281,8 @@ qemuCompressProgramAvailable(virQEMUSaveFormat compress)
* indicating none.
*/
static virQEMUSaveFormat
-qemuGetCompressionProgram(const char *imageFormat)
+qemuGetCompressionProgram(const char *imageFormat,
+ const char *styleFormat)
{
virQEMUSaveFormat ret;
@@ -3297,12 +3299,13 @@ qemuGetCompressionProgram(const char *imageFormat)
error:
if (ret < 0)
- VIR_WARN("%s", _("Invalid dump image format specified in "
- "configuration file, using raw"));
+ VIR_WARN("Invalid %s image format specified in "
+ "configuration file, using raw",
+ styleFormat);
else
- VIR_WARN("%s", _("Compression program for dump image format
"
- "in configuration file isn't available, "
- "using raw"));
+ VIR_WARN("Compression program for %s image format in "
+ "configuration file isn't available, using raw",
+ styleFormat);
/* Use "raw" as the format if the specified format is not valid,
* or the compress program is not available. */
@@ -3590,7 +3593,7 @@ doCoreDump(virQEMUDriverPtr driver,
/* We reuse "save" flag for "dump" here. Then, we can support the
same
* format in "save" and "dump". */
- compress = qemuGetCompressionProgram(cfg->dumpImageFormat);
+ compress = qemuGetCompressionProgram(cfg->dumpImageFormat, "dump");
/* Create an empty file with appropriate ownership. */
if (dump_flags & VIR_DUMP_BYPASS_CACHE) {
--
2.7.4
6:30 a.m.
New subject: [libvirt] [PATCH v2 6/8] qemu: Use qemuGetCompressionProgram for error paths
Let's do some more code reuse - there are 3 other callers that care to
check/get the compress program. Each of those though cares whether the
requested cfg image is valid and exists. So, add a parameter to handle
those cases.
NB: We won't need to initialize the returned value in the case where
the cfg image doesn't exist since the called program will handle that.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_driver.c | 103 +++++++++++++++++++++----------------------------
1 file changed, 43 insertions(+), 60 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7dfba50..8a47262 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3271,6 +3271,9 @@ qemuCompressProgramAvailable(virQEMUSaveFormat compress)
* @imageFormat: String representation from qemu.conf for the compression
* image format being used (dump, save, or snapshot).
* @styleFormat: String representing the style of format (dump, save, snapshot)
+ * @use_raw_on_fail: Boolean indicating how to handle the error path. For
+ * callers that are OK with invalid data or inability to
+ * find the compression program, just return a raw format
*
* Returns:
* virQEMUSaveFormat - Integer representation of the compression
@@ -3282,7 +3285,8 @@ qemuCompressProgramAvailable(virQEMUSaveFormat compress)
*/
static virQEMUSaveFormat
qemuGetCompressionProgram(const char *imageFormat,
- const char *styleFormat)
+ const char *styleFormat,
+ bool use_raw_on_fail)
{
virQEMUSaveFormat ret;
@@ -3298,18 +3302,34 @@ qemuGetCompressionProgram(const char *imageFormat,
return ret;
error:
- if (ret < 0)
- VIR_WARN("Invalid %s image format specified in "
- "configuration file, using raw",
- styleFormat);
- else
- VIR_WARN("Compression program for %s image format in "
- "configuration file isn't available, using raw",
- styleFormat);
+ if (ret < 0) {
+ if (use_raw_on_fail)
+ VIR_WARN("Invalid %s image format specified in "
+ "configuration file, using raw",
+ styleFormat);
+ else
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("Invalid %s image format specified "
+ "in configuration file"),
+ styleFormat);
+ } else {
+ if (use_raw_on_fail)
+ VIR_WARN("Compression program for %s image format in "
+ "configuration file isn't available, using raw",
+ styleFormat);
+ else
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("Compression program for %s image format "
+ "in configuration file isn't available"),
+ styleFormat);
+ }
/* Use "raw" as the format if the specified format is not valid,
* or the compress program is not available. */
- return QEMU_SAVE_FORMAT_RAW;
+ if (use_raw_on_fail)
+ return QEMU_SAVE_FORMAT_RAW;
+
+ return -1;
}
@@ -3318,7 +3338,7 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char
*dxml,
unsigned int flags)
{
virQEMUDriverPtr driver = dom->conn->privateData;
- int compressed = QEMU_SAVE_FORMAT_RAW;
+ int compressed;
int ret = -1;
virDomainObjPtr vm = NULL;
virQEMUDriverConfigPtr cfg = NULL;
@@ -3328,21 +3348,9 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char
*dxml,
VIR_DOMAIN_SAVE_PAUSED, -1);
cfg = virQEMUDriverGetConfig(driver);
- if (cfg->saveImageFormat) {
- compressed = qemuSaveCompressionTypeFromString(cfg->saveImageFormat);
- if (compressed < 0) {
- virReportError(VIR_ERR_OPERATION_FAILED, "%s",
- _("Invalid save image format specified "
- "in configuration file"));
- goto cleanup;
- }
- if (!qemuCompressProgramAvailable(compressed)) {
- virReportError(VIR_ERR_OPERATION_FAILED, "%s",
- _("Compression program for image format "
- "in configuration file isn't available"));
- goto cleanup;
- }
- }
+ if ((compressed = qemuGetCompressionProgram(cfg->saveImageFormat,
+ "save", false)) < 0)
+ goto cleanup;
if (!(vm = qemuDomObjFromDomain(dom)))
goto cleanup;
@@ -3391,7 +3399,7 @@ qemuDomainManagedSave(virDomainPtr dom, unsigned int flags)
{
virQEMUDriverPtr driver = dom->conn->privateData;
virQEMUDriverConfigPtr cfg = NULL;
- int compressed = QEMU_SAVE_FORMAT_RAW;
+ int compressed;
virDomainObjPtr vm;
char *name = NULL;
int ret = -1;
@@ -3418,21 +3426,9 @@ qemuDomainManagedSave(virDomainPtr dom, unsigned int flags)
}
cfg = virQEMUDriverGetConfig(driver);
- if (cfg->saveImageFormat) {
- compressed = qemuSaveCompressionTypeFromString(cfg->saveImageFormat);
- if (compressed < 0) {
- virReportError(VIR_ERR_OPERATION_FAILED, "%s",
- _("Invalid save image format specified "
- "in configuration file"));
- goto cleanup;
- }
- if (!qemuCompressProgramAvailable(compressed)) {
- virReportError(VIR_ERR_OPERATION_FAILED, "%s",
- _("Compression program for image format "
- "in configuration file isn't available"));
- goto cleanup;
- }
- }
+ if ((compressed = qemuGetCompressionProgram(cfg->saveImageFormat,
+ "save", false)) < 0)
+ goto cleanup;
if (!(name = qemuDomainManagedSavePath(driver, vm)))
goto cleanup;
@@ -3593,7 +3589,7 @@ doCoreDump(virQEMUDriverPtr driver,
/* We reuse "save" flag for "dump" here. Then, we can support the
same
* format in "save" and "dump". */
- compress = qemuGetCompressionProgram(cfg->dumpImageFormat, "dump");
+ compress = qemuGetCompressionProgram(cfg->dumpImageFormat, "dump",
true);
/* Create an empty file with appropriate ownership. */
if (dump_flags & VIR_DUMP_BYPASS_CACHE) {
@@ -14312,7 +14308,7 @@ qemuDomainSnapshotCreateActiveExternal(virConnectPtr conn,
int thaw = 0; /* 1 if freeze succeeded, -1 if freeze failed */
bool pmsuspended = false;
virQEMUDriverConfigPtr cfg = NULL;
- int compressed = QEMU_SAVE_FORMAT_RAW;
+ int compressed;
/* If quiesce was requested, then issue a freeze command, and a
* counterpart thaw command when it is actually sent to agent.
@@ -14373,22 +14369,9 @@ qemuDomainSnapshotCreateActiveExternal(virConnectPtr conn,
JOB_MASK(QEMU_JOB_MIGRATION_OP)));
cfg = virQEMUDriverGetConfig(driver);
- if (cfg->snapshotImageFormat) {
- compressed = qemuSaveCompressionTypeFromString(cfg->snapshotImageFormat);
- if (compressed < 0) {
- virReportError(VIR_ERR_OPERATION_FAILED, "%s",
- _("Invalid snapshot image format specified "
- "in configuration file"));
- goto cleanup;
- }
-
- if (!qemuCompressProgramAvailable(compressed)) {
- virReportError(VIR_ERR_OPERATION_FAILED, "%s",
- _("Compression program for image format "
- "in configuration file isn't
available"));
- goto cleanup;
- }
- }
+ if ((compressed = qemuGetCompressionProgram(cfg->snapshotImageFormat,
+ "snapshot", false)) <
0)
+ goto cleanup;
if (!(xml = qemuDomainDefFormatLive(driver, vm->def, true, true)))
goto cleanup;
--
2.7.4
11:53 a.m.
New subject: [libvirt] [PATCH v2 6/8] qemu: Use qemuGetCompressionProgram for error paths
On Fri, Sep 23, 2016 at 07:30:54AM -0400, John Ferlan wrote:
Let's do some more code reuse - there are 3 other callers that
care to
check/get the compress program. Each of those though cares whether the
requested cfg image is valid and exists. So, add a parameter to handle
those cases.
NB: We won't need to initialize the returned value in the case where
the cfg image doesn't exist since the called program will handle that.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_driver.c | 103 +++++++++++++++++++++----------------------------
1 file changed, 43 insertions(+), 60 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7dfba50..8a47262 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3271,6 +3271,9 @@ qemuCompressProgramAvailable(virQEMUSaveFormat compress)
* @imageFormat: String representation from qemu.conf for the compression
* image format being used (dump, save, or snapshot).
* @styleFormat: String representing the style of format (dump, save, snapshot)
+ * @use_raw_on_fail: Boolean indicating how to handle the error path. For
+ * callers that are OK with invalid data or inability to
+ * find the compression program, just return a raw format
*
* Returns:
* virQEMUSaveFormat - Integer representation of the compression
@@ -3282,7 +3285,8 @@ qemuCompressProgramAvailable(virQEMUSaveFormat compress)
*/
static virQEMUSaveFormat
qemuGetCompressionProgram(const char *imageFormat,
- const char *styleFormat)
+ const char *styleFormat,
+ bool use_raw_on_fail)
{
virQEMUSaveFormat ret;
@@ -3298,18 +3302,34 @@ qemuGetCompressionProgram(const char *imageFormat,
return ret;
error:
- if (ret < 0)
- VIR_WARN("Invalid %s image format specified in "
- "configuration file, using raw",
- styleFormat);
- else
- VIR_WARN("Compression program for %s image format in "
- "configuration file isn't available, using raw",
- styleFormat);
+ if (ret < 0) {
+ if (use_raw_on_fail)
+ VIR_WARN("Invalid %s image format specified in "
+ "configuration file, using raw",
+ styleFormat);
+ else
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("Invalid %s image format specified "
+ "in configuration file"),
+ styleFormat);
+ } else {
+ if (use_raw_on_fail)
+ VIR_WARN("Compression program for %s image format in "
+ "configuration file isn't available, using raw",
+ styleFormat);
+ else
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("Compression program for %s image format "
+ "in configuration file isn't available"),
+ styleFormat);
+ }
This might work for the English language, but constructing a translatable
message in this matter is unfriendly to translators and not worth
shaving off a few lines of code.
Also, logging the "styleFormat" (sic) only makes sense for the dump,
which might not have been a result of an API call, otherwise we're
better off putting the image format in here.
If you call the snapshot API and get an error saying "xz" was not found,
you know it's not for the core dump format.
Jan
3:42 p.m.
New subject: [libvirt] [PATCH v2 6/8] qemu: Use qemuGetCompressionProgram for error paths
On 09/27/2016 12:53 PM, Ján Tomko wrote:
On Fri, Sep 23, 2016 at 07:30:54AM -0400, John Ferlan wrote:
> Let's do some more code reuse - there are 3 other callers that care to
> check/get the compress program. Each of those though cares whether the
> requested cfg image is valid and exists. So, add a parameter to handle
> those cases.
>
> NB: We won't need to initialize the returned value in the case where
> the cfg image doesn't exist since the called program will handle that.
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> src/qemu/qemu_driver.c | 103
> +++++++++++++++++++++----------------------------
> 1 file changed, 43 insertions(+), 60 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 7dfba50..8a47262 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -3271,6 +3271,9 @@ qemuCompressProgramAvailable(virQEMUSaveFormat
> compress)
> * @imageFormat: String representation from qemu.conf for the compression
> * image format being used (dump, save, or snapshot).
> * @styleFormat: String representing the style of format (dump, save,
> snapshot)
> + * @use_raw_on_fail: Boolean indicating how to handle the error path.
> For
> + * callers that are OK with invalid data or
> inability to
> + * find the compression program, just return a raw
> format
> *
> * Returns:
> * virQEMUSaveFormat - Integer representation of the compression
> @@ -3282,7 +3285,8 @@ qemuCompressProgramAvailable(virQEMUSaveFormat
> compress)
> */
> static virQEMUSaveFormat
> qemuGetCompressionProgram(const char *imageFormat,
> - const char *styleFormat)
> + const char *styleFormat,
> + bool use_raw_on_fail)
> {
> virQEMUSaveFormat ret;
>
> @@ -3298,18 +3302,34 @@ qemuGetCompressionProgram(const char
> *imageFormat,
> return ret;
>
> error:
> - if (ret < 0)
> - VIR_WARN("Invalid %s image format specified in "
> - "configuration file, using raw",
> - styleFormat);
> - else
> - VIR_WARN("Compression program for %s image format in "
> - "configuration file isn't available, using raw",
> - styleFormat);
> + if (ret < 0) {
> + if (use_raw_on_fail)
> + VIR_WARN("Invalid %s image format specified in "
> + "configuration file, using raw",
> + styleFormat);
> + else
> + virReportError(VIR_ERR_OPERATION_FAILED,
> + _("Invalid %s image format specified "
> + "in configuration file"),
> + styleFormat);
> + } else {
> + if (use_raw_on_fail)
> + VIR_WARN("Compression program for %s image format in "
> + "configuration file isn't available, using raw",
> + styleFormat);
> + else
> + virReportError(VIR_ERR_OPERATION_FAILED,
> + _("Compression program for %s image format "
> + "in configuration file isn't
available"),
> + styleFormat);
> + }
This might work for the English language, but constructing a translatable
message in this matter is unfriendly to translators and not worth
shaving off a few lines of code.
Hmm... And qemuDomainDiskChangeSupported is any better? Similarly
qemuMonitorJSONGetBlockInfo?
Instead of 3 (or 4) message pairs that we all similar varying only by
"dump", "save", or "snapshot" there is now 1 message pair
which has a %s
parameter no different than 1000's of other libvirt messages. Since
dump, save, and snapshot are all command names - does their translation
really change?
Beyond that I'll note that qemuNodeDeviceDetachFlags uses a const char
*driverName which is just a string and can be messaged using %s. Also
qemuConnectGetDomainCapabilities uses a const char *virttype_str which
is messaged.
Also, logging the "styleFormat" (sic) only makes sense for the dump,
which might not have been a result of an API call, otherwise we're
better off putting the image format in here.
If you don't like the variable name, then change it. I'm not offended.
If you call the snapshot API and get an error saying "xz"
was not found,
you know it's not for the core dump format.
If you prefer a different mechanism, I'll review the patch when I see it.
John
6:30 a.m.
New subject: [libvirt] [PATCH v2 7/8] qemu: Remove qemuCompressProgramAvailable
There's only one caller and the code is duplicitous just converting the
recently converted cfg image name back into it's string value in order to
get/find the path to the image. A subsequent patch can return this path.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_driver.c | 21 ++++-----------------
1 file changed, 4 insertions(+), 17 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8a47262..6b755a5 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3250,22 +3250,6 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, virDomainPtr dom,
return ret;
}
-/* Returns true if a compression program is available in PATH */
-static bool
-qemuCompressProgramAvailable(virQEMUSaveFormat compress)
-{
- char *path;
-
- if (compress == QEMU_SAVE_FORMAT_RAW)
- return true;
-
- if (!(path = virFindFileInPath(qemuSaveCompressionTypeToString(compress))))
- return false;
-
- VIR_FREE(path);
- return true;
-}
-
/* qemuGetCompressionProgram:
* @imageFormat: String representation from qemu.conf for the compression
@@ -3289,6 +3273,7 @@ qemuGetCompressionProgram(const char *imageFormat,
bool use_raw_on_fail)
{
virQEMUSaveFormat ret;
+ char *path = NULL;
if (!imageFormat)
return QEMU_SAVE_FORMAT_RAW;
@@ -3296,9 +3281,11 @@ qemuGetCompressionProgram(const char *imageFormat,
if ((ret = qemuSaveCompressionTypeFromString(imageFormat)) < 0)
goto error;
- if (!qemuCompressProgramAvailable(ret))
+ if (!(path = virFindFileInPath(imageFormat)))
goto error;
+ VIR_FREE(path);
+
return ret;
error:
--
2.7.4
6:30 a.m.
New subject: [libvirt] [PATCH v2 8/8] qemu: Get/return compressedpath program
Based upon a patch from Chen Hanxiao <chenhanxiao(a)gmail.com>, rather than
need to call virFindFileInPath twice, let's just save the path and pass it
along with the compressed type. (NB: the second call would be in virExec as
called from virCommandRunAsync which is called from qemuMigrationToFile
using the argument 'compressor' which up to this point would be the string
from the cfg file that isn't the fully qualified path).
Since we now have the path, we can remove qemuCompressProgramName which
would return NULL or the string representation of the compress type.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_driver.c | 66 +++++++++++++++++++++++++++++---------------------
1 file changed, 38 insertions(+), 28 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6b755a5..fb2a8f6 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2833,14 +2833,6 @@ qemuDomainSaveHeader(int fd, const char *path, const char *xml,
return ret;
}
-/* Given a virQEMUSaveFormat compression level, return the name
- * of the program to run, or NULL if no program is needed. */
-static const char *
-qemuCompressProgramName(int compress)
-{
- return (compress == QEMU_SAVE_FORMAT_RAW ? NULL :
- qemuSaveCompressionTypeToString(compress));
-}
static virCommandPtr
qemuCompressGetCommand(virQEMUSaveFormat compression)
@@ -3039,6 +3031,7 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver,
const char *path,
const char *domXML,
int compressed,
+ const char *compressedpath,
bool was_running,
unsigned int flags,
qemuDomainAsyncJob asyncJob)
@@ -3086,8 +3079,7 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver,
goto cleanup;
/* Perform the migration */
- if (qemuMigrationToFile(driver, vm, fd, qemuCompressProgramName(compressed),
- asyncJob) < 0)
+ if (qemuMigrationToFile(driver, vm, fd, compressedpath, asyncJob) < 0)
goto cleanup;
/* Touch up file header to mark image complete. */
@@ -3139,7 +3131,8 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver,
static int
qemuDomainSaveInternal(virQEMUDriverPtr driver, virDomainPtr dom,
virDomainObjPtr vm, const char *path,
- int compressed, const char *xmlin, unsigned int flags)
+ int compressed, const char *compressedpath,
+ const char *xmlin, unsigned int flags)
{
char *xml = NULL;
bool was_running = false;
@@ -3212,7 +3205,8 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, virDomainPtr dom,
}
ret = qemuDomainSaveMemory(driver, vm, path, xml, compressed,
- was_running, flags, QEMU_ASYNC_JOB_SAVE);
+ compressedpath, was_running, flags,
+ QEMU_ASYNC_JOB_SAVE);
if (ret < 0)
goto endjob;
@@ -3254,10 +3248,13 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, virDomainPtr dom,
/* qemuGetCompressionProgram:
* @imageFormat: String representation from qemu.conf for the compression
* 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)
* @use_raw_on_fail: Boolean indicating how to handle the error path. For
* callers that are OK with invalid data or inability to
* find the compression program, just return a raw format
+ * and let the path remain as NULL.
*
* Returns:
* virQEMUSaveFormat - Integer representation of the compression
@@ -3267,13 +3264,15 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, virDomainPtr dom,
* no there was an error, then just return RAW
* indicating none.
*/
-static virQEMUSaveFormat
+static virQEMUSaveFormat ATTRIBUTE_NONNULL(2)
qemuGetCompressionProgram(const char *imageFormat,
+ char **compresspath,
const char *styleFormat,
bool use_raw_on_fail)
{
virQEMUSaveFormat ret;
- char *path = NULL;
+
+ *compresspath = NULL;
if (!imageFormat)
return QEMU_SAVE_FORMAT_RAW;
@@ -3281,11 +3280,9 @@ qemuGetCompressionProgram(const char *imageFormat,
if ((ret = qemuSaveCompressionTypeFromString(imageFormat)) < 0)
goto error;
- if (!(path = virFindFileInPath(imageFormat)))
+ if (!(*compresspath = virFindFileInPath(imageFormat)))
goto error;
- VIR_FREE(path);
-
return ret;
error:
@@ -3326,6 +3323,7 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char
*dxml,
{
virQEMUDriverPtr driver = dom->conn->privateData;
int compressed;
+ char *compressedpath = NULL;
int ret = -1;
virDomainObjPtr vm = NULL;
virQEMUDriverConfigPtr cfg = NULL;
@@ -3336,6 +3334,7 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char
*dxml,
cfg = virQEMUDriverGetConfig(driver);
if ((compressed = qemuGetCompressionProgram(cfg->saveImageFormat,
+ &compressedpath,
"save", false)) < 0)
goto cleanup;
@@ -3352,10 +3351,11 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char
*dxml,
}
ret = qemuDomainSaveInternal(driver, dom, vm, path, compressed,
- dxml, flags);
+ compressedpath, dxml, flags);
cleanup:
virDomainObjEndAPI(&vm);
+ VIR_FREE(compressedpath);
virObjectUnref(cfg);
return ret;
}
@@ -3387,6 +3387,7 @@ qemuDomainManagedSave(virDomainPtr dom, unsigned int flags)
virQEMUDriverPtr driver = dom->conn->privateData;
virQEMUDriverConfigPtr cfg = NULL;
int compressed;
+ char *compressedpath = NULL;
virDomainObjPtr vm;
char *name = NULL;
int ret = -1;
@@ -3414,6 +3415,7 @@ qemuDomainManagedSave(virDomainPtr dom, unsigned int flags)
cfg = virQEMUDriverGetConfig(driver);
if ((compressed = qemuGetCompressionProgram(cfg->saveImageFormat,
+ &compressedpath,
"save", false)) < 0)
goto cleanup;
@@ -3422,14 +3424,15 @@ qemuDomainManagedSave(virDomainPtr dom, unsigned int flags)
VIR_INFO("Saving state of domain '%s' to '%s'",
vm->def->name, name);
- ret = qemuDomainSaveInternal(driver, dom, vm, name,
- compressed, NULL, flags);
+ ret = qemuDomainSaveInternal(driver, dom, vm, name, compressed,
+ compressedpath, NULL, flags);
if (ret == 0)
vm->hasManagedSave = true;
cleanup:
virDomainObjEndAPI(&vm);
VIR_FREE(name);
+ VIR_FREE(compressedpath);
virObjectUnref(cfg);
return ret;
@@ -3572,11 +3575,15 @@ doCoreDump(virQEMUDriverPtr driver,
unsigned int flags = VIR_FILE_WRAPPER_NON_BLOCKING;
const char *memory_dump_format = NULL;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
- virQEMUSaveFormat compress;
+ char *compressedpath = NULL;
/* We reuse "save" flag for "dump" here. Then, we can support the
same
- * format in "save" and "dump". */
- compress = qemuGetCompressionProgram(cfg->dumpImageFormat, "dump",
true);
+ * format in "save" and "dump". This path doesn't need the
compression
+ * program to exist and can ignore the return value - it only cares to
+ * get the compressedpath */
+ ignore_value(qemuGetCompressionProgram(cfg->dumpImageFormat,
+ &compressedpath,
+ "dump", true));
/* Create an empty file with appropriate ownership. */
if (dump_flags & VIR_DUMP_BYPASS_CACHE) {
@@ -3623,8 +3630,7 @@ doCoreDump(virQEMUDriverPtr driver,
if (!qemuMigrationIsAllowed(driver, vm, false, 0))
goto cleanup;
- ret = qemuMigrationToFile(driver, vm, fd,
- qemuCompressProgramName(compress),
+ ret = qemuMigrationToFile(driver, vm, fd, compressedpath,
QEMU_ASYNC_JOB_DUMP);
}
@@ -3647,6 +3653,7 @@ doCoreDump(virQEMUDriverPtr driver,
if (ret != 0)
unlink(path);
virFileWrapperFdFree(wrapperFd);
+ VIR_FREE(compressedpath);
virObjectUnref(cfg);
return ret;
}
@@ -14296,6 +14303,7 @@ qemuDomainSnapshotCreateActiveExternal(virConnectPtr conn,
bool pmsuspended = false;
virQEMUDriverConfigPtr cfg = NULL;
int compressed;
+ char *compressedpath = NULL;
/* If quiesce was requested, then issue a freeze command, and a
* counterpart thaw command when it is actually sent to agent.
@@ -14357,15 +14365,16 @@ qemuDomainSnapshotCreateActiveExternal(virConnectPtr conn,
cfg = virQEMUDriverGetConfig(driver);
if ((compressed = qemuGetCompressionProgram(cfg->snapshotImageFormat,
+ &compressedpath,
"snapshot", false)) <
0)
goto cleanup;
if (!(xml = qemuDomainDefFormatLive(driver, vm->def, true, true)))
goto cleanup;
- if ((ret = qemuDomainSaveMemory(driver, vm, snap->def->file,
- xml, compressed, resume, 0,
- QEMU_ASYNC_JOB_SNAPSHOT)) < 0)
+ if ((ret = qemuDomainSaveMemory(driver, vm, snap->def->file, xml,
+ compressed, compressedpath, resume,
+ 0, QEMU_ASYNC_JOB_SNAPSHOT)) < 0)
goto cleanup;
/* the memory image was created, remove it on errors */
@@ -14434,6 +14443,7 @@ qemuDomainSnapshotCreateActiveExternal(virConnectPtr conn,
}
VIR_FREE(xml);
+ VIR_FREE(compressedpath);
virObjectUnref(cfg);
if (memory_unlink && ret < 0)
unlink(snap->def->file);
--
2.7.4
11:35 a.m.
At 2016-09-23 19:30:48, "John Ferlan" <jferlan(a)redhat.com> wrote:
v1:
http://www.redhat.com/archives/libvir-list/2016-September/msg00971.html
Changes in v2:
1. Create a patch 5 which alters the qemuGetCompressionProgram to take a
new const char * parameter which will be used in the warning message
as the %s image format "style" (dump, save, snapshot) for both messages.
Also removed the unnecessary translated message marker _() for the
VIR_WARN message.
2. Alter former patch 5 to pass the style ("save" or "snapshot") as
an
argument for the altered virReportError messages which now will take
a parameter that describe their style. For one message it keeps it
constant, for the other it adds the style into the message.
3. Alter former patches 6 and 7 to account for the new argument (no real
changes, just handling merge conflicts).
(from patch 1's cover)
Rather than try to describe what I was thinking about for the passing
the compress program directly series from Chen Hanxiao:
http://www.redhat.com/archives/libvir-list/2016-September/msg00677.html
and
http://www.redhat.com/archives/libvir-list/2016-August/msg01237.html
I took the liberty of creating my own set of patches which should end
in the more or less same result.
The primary benefit is not calling virFileInPath twice since we already
get it at least once for the various compressed program callers, let's
pass what we originally found.
John Ferlan (8):
qemu: Move getCompressionType
qemu: Adjust doCoreDump to call getCompressionType
qemu: Introduce helper qemuGetCompressionProgram
qemu: Remove getCompressionType
qemu: Alter qemuGetCompressionProgram warning message
qemu: Use qemuGetCompressionProgram for error paths
qemu: Remove qemuCompressProgramAvailable
qemu: Get/return compressedpath program
src/qemu/qemu_driver.c | 241 ++++++++++++++++++++++++-------------------------
1 file changed, 117 insertions(+), 124 deletions(-)
Looks good to me.
ACK series.
Regards,
- Chen
2978
days inactive
2982
days old
13 comments
3 participants
participants (3)
-
Chen Hanxiao -
John Ferlan -
Ján Tomko