4:30 p.m.
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 (7):
qemu: Move getCompressionType
qemu: Adjust doCoreDump to call getCompressionType
qemu: Introduce helper qemuGetCompressionProgram
qemu: Remove getCompressionType
qemu: Use qemuGetCompressionProgram for error paths
qemu: Remove qemuCompressProgramAvailable
qemu: Get/return compressedpath program
src/qemu/qemu_driver.c | 235 +++++++++++++++++++++++--------------------------
1 file changed, 111 insertions(+), 124 deletions(-)
--
2.7.4
4:30 p.m.
New subject: [libvirt] [PATCH 1/7] 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 71eb062..9b6ac48 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
4:30 p.m.
New subject: [libvirt] [PATCH 2/7] 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 9b6ac48..6dfdaa8 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
4:30 p.m.
New subject: [libvirt] [PATCH 3/7] 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 6dfdaa8..1c41c7d 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
4:30 p.m.
New subject: [libvirt] [PATCH 4/7] 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 1c41c7d..4cd0a07 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
4:30 p.m.
New subject: [libvirt] [PATCH 5/7] 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 | 99 ++++++++++++++++++++------------------------------
1 file changed, 40 insertions(+), 59 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4cd0a07..505dd29 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3270,6 +3270,9 @@ qemuCompressProgramAvailable(virQEMUSaveFormat compress)
/* qemuGetCompressionProgram:
* @imageFormat: String representation from qemu.conf for the compression
* image format being used (dump, save, or 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
@@ -3280,7 +3283,8 @@ qemuCompressProgramAvailable(virQEMUSaveFormat compress)
* indicating none.
*/
static virQEMUSaveFormat
-qemuGetCompressionProgram(const char *imageFormat)
+qemuGetCompressionProgram(const char *imageFormat,
+ bool use_raw_on_fail)
{
virQEMUSaveFormat ret;
@@ -3296,17 +3300,31 @@ qemuGetCompressionProgram(const char *imageFormat)
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"));
+ if (ret < 0) {
+ if (use_raw_on_fail)
+ VIR_WARN("%s", _("Invalid dump image format specified in
"
+ "configuration file, using raw"));
+ else
+ virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("Invalid save image format specified "
+ "in configuration file"));
+ } else {
+ if (use_raw_on_fail)
+ VIR_WARN("%s", _("Compression program for dump image format
"
+ "in configuration file isn't available, "
+ "using raw"));
+ else
+ virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("Compression program for image format "
+ "in configuration file isn't available"));
+ }
/* 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;
}
@@ -3315,7 +3333,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;
@@ -3325,21 +3343,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,
+ false)) < 0)
+ goto cleanup;
if (!(vm = qemuDomObjFromDomain(dom)))
goto cleanup;
@@ -3388,7 +3394,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;
@@ -3415,21 +3421,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,
+ false)) < 0)
+ goto cleanup;
if (!(name = qemuDomainManagedSavePath(driver, vm)))
goto cleanup;
@@ -3590,7 +3584,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, true);
/* Create an empty file with appropriate ownership. */
if (dump_flags & VIR_DUMP_BYPASS_CACHE) {
@@ -14318,7 +14312,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.
@@ -14379,22 +14373,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,
+ false)) < 0)
+ goto cleanup;
if (!(xml = qemuDomainDefFormatLive(driver, vm->def, true, true)))
goto cleanup;
--
2.7.4
10:57 a.m.
New subject: [libvirt] [PATCH 5/7] qemu: Use qemuGetCompressionProgram for error paths
At 2016-09-22 05:30:53, "John Ferlan" <jferlan(a)redhat.com> 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 | 99 ++++++++++++++++++++------------------------------
1 file changed, 40 insertions(+), 59 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4cd0a07..505dd29 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3270,6 +3270,9 @@ qemuCompressProgramAvailable(virQEMUSaveFormat compress)
/* qemuGetCompressionProgram:
* @imageFormat: String representation from qemu.conf for the compression
* image format being used (dump, save, or 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
@@ -3280,7 +3283,8 @@ qemuCompressProgramAvailable(virQEMUSaveFormat compress)
* indicating none.
*/
static virQEMUSaveFormat
-qemuGetCompressionProgram(const char *imageFormat)
+qemuGetCompressionProgram(const char *imageFormat,
+ bool use_raw_on_fail)
{
virQEMUSaveFormat ret;
@@ -3296,17 +3300,31 @@ qemuGetCompressionProgram(const char *imageFormat)
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"));
+ if (ret < 0) {
+ if (use_raw_on_fail)
+ VIR_WARN("%s", _("Invalid dump image format specified in
"
+ "configuration file, using raw"));
+ else
+ virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("Invalid save image format specified "
+ "in configuration file"));
+ } else {
+ if (use_raw_on_fail)
+ VIR_WARN("%s", _("Compression program for dump image format
"
+ "in configuration file isn't available, "
+ "using raw"));
+ else
+ virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("Compression program for image format "
+ "in configuration file isn't available"));
+ }
Actually we had to deal with:
saveImageFormat
snapshotImageFormat
dumpImageFormat
I think a switch case with a macro should be fitted.
Patch 6, 7 looks good to me.
Regards,
- Chen
4:30 p.m.
New subject: [libvirt] [PATCH 6/7] 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 505dd29..18fd87d 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
@@ -3287,6 +3271,7 @@ qemuGetCompressionProgram(const char *imageFormat,
bool use_raw_on_fail)
{
virQEMUSaveFormat ret;
+ char *path = NULL;
if (!imageFormat)
return QEMU_SAVE_FORMAT_RAW;
@@ -3294,9 +3279,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
4:30 p.m.
New subject: [libvirt] [PATCH 7/7] 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 | 65 ++++++++++++++++++++++++++++----------------------
1 file changed, 37 insertions(+), 28 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 18fd87d..abc34fc 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,9 +3248,12 @@ 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.
* @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
@@ -3266,12 +3263,14 @@ 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,
bool use_raw_on_fail)
{
virQEMUSaveFormat ret;
- char *path = NULL;
+
+ *compresspath = NULL;
if (!imageFormat)
return QEMU_SAVE_FORMAT_RAW;
@@ -3279,11 +3278,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:
@@ -3321,6 +3318,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;
@@ -3331,6 +3329,7 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char
*dxml,
cfg = virQEMUDriverGetConfig(driver);
if ((compressed = qemuGetCompressionProgram(cfg->saveImageFormat,
+ &compressedpath,
false)) < 0)
goto cleanup;
@@ -3347,10 +3346,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;
}
@@ -3382,6 +3382,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;
@@ -3409,6 +3410,7 @@ qemuDomainManagedSave(virDomainPtr dom, unsigned int flags)
cfg = virQEMUDriverGetConfig(driver);
if ((compressed = qemuGetCompressionProgram(cfg->saveImageFormat,
+ &compressedpath,
false)) < 0)
goto cleanup;
@@ -3417,14 +3419,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;
@@ -3567,11 +3570,14 @@ 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, 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, true));
/* Create an empty file with appropriate ownership. */
if (dump_flags & VIR_DUMP_BYPASS_CACHE) {
@@ -3618,8 +3624,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);
}
@@ -3642,6 +3647,7 @@ doCoreDump(virQEMUDriverPtr driver,
if (ret != 0)
unlink(path);
virFileWrapperFdFree(wrapperFd);
+ VIR_FREE(compressedpath);
virObjectUnref(cfg);
return ret;
}
@@ -14300,6 +14306,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.
@@ -14361,15 +14368,16 @@ qemuDomainSnapshotCreateActiveExternal(virConnectPtr conn,
cfg = virQEMUDriverGetConfig(driver);
if ((compressed = qemuGetCompressionProgram(cfg->snapshotImageFormat,
+ &compressedpath,
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 */
@@ -14438,6 +14446,7 @@ qemuDomainSnapshotCreateActiveExternal(virConnectPtr conn,
}
VIR_FREE(xml);
+ VIR_FREE(compressedpath);
virObjectUnref(cfg);
if (memory_unlink && ret < 0)
unlink(snap->def->file);
--
2.7.4
10:23 a.m.
At 2016-09-22 05:30:48, "John Ferlan" <jferlan(a)redhat.com> wrote:
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 (7):
qemu: Move getCompressionType
qemu: Adjust doCoreDump to call getCompressionType
qemu: Introduce helper qemuGetCompressionProgram
qemu: Remove getCompressionType
qemu: Use qemuGetCompressionProgram for error paths
qemu: Remove qemuCompressProgramAvailable
qemu: Get/return compressedpath program
src/qemu/qemu_driver.c | 235 +++++++++++++++++++++++--------------------------
1 file changed, 111 insertions(+), 124 deletions(-)
--
Hi, John
Looks that Patch 1, 3 and 4 should be scrash into one :).
Some other inline comments, Please check.
Regards,
- Chen
6:28 p.m.
On 09/21/2016 10:23 PM, Chen Hanxiao wrote:
At 2016-09-22 05:30:48, "John Ferlan" <jferlan(a)redhat.com> wrote:
> 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 (7):
> qemu: Move getCompressionType
> qemu: Adjust doCoreDump to call getCompressionType
> qemu: Introduce helper qemuGetCompressionProgram
> qemu: Remove getCompressionType
> qemu: Use qemuGetCompressionProgram for error paths
> qemu: Remove qemuCompressProgramAvailable
> qemu: Get/return compressedpath program
>
> src/qemu/qemu_driver.c | 235 +++++++++++++++++++++++--------------------------
> 1 file changed, 111 insertions(+), 124 deletions(-)
>
> --
Hi, John
Looks that Patch 1, 3 and 4 should be scrash into one :).
I disagree - then in becomes one mish-mash of a patch which is harder to
review. The way I split things up (for me) was a logical progression of
this series.
1. Move code because we're about to use it from doCoreDump.
2. Use getCompressionType from within doCoreDump instead of passing it
3. Split out the "cfg->dumpImageFormat" into a separate helper and call
4. Remove need for getCompressionType for doCoreDump
As for the rest
5. Create new parameter to change how messaging is done. From doCoreDump
we can VIR_WARN since the code can continue; whereas, from
qemuDomainSaveFlags, qemuDomainManagedSave, and
qemuDomainSnapshotCreateActiveExternal we want to have the error and fail.
Using "ret < 0" in the error path allows us to know which of the two
options failed.
Yes, the error message for snapshot will now be different, but that's
far more easily fixed by a new parameter than the extra code created in
your patch 2 to handle that error message.
6. Remove need for qemuCompressProgramAvailable by calling
virFindFileInPath directly. No need to worry about the
QEMU_SAVE_FORMAT_RAW case since it's already handled.
7. Return that compressed program name instead of calling
qemuCompressProgramName again.
Again, since QEMU_SAVE_FORMAT_RAW was already handled we're good.
Some other inline comments, Please check.
I'll post an adjustment for patch 5 error message tomorrow - it won't be
complicated.
Tks for the comments/review -
John
Regards,
- Chen
2983
days inactive
2984
days old
10 comments
2 participants
participants (2)
-
Chen Hanxiao -
John Ferlan