[libvirt] [PATCH] qemu: Error prompt if invalid dump image format specified

"getCompressionType" trys to throw error when invalid dump image format is specified, or the compression program for the specified image format is not available, but at the same time, it returns enum qemud_save_formats, as a result, upper function will think it was successful, and disgard to check the error, this fix makes changes so that it returns -1 on FAILURE, And to be consistent with "save", won't use "raw" as the default dump format anymore. * src/qemu/qemu_driver.c - rename "getCompressionType" to "getDumpType" - return -1 on FAILURE --- src/qemu/qemu_driver.c | 40 +++++++++++++++++++++++++--------------- 1 files changed, 25 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 395f72f..f8c2aae 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4787,8 +4787,12 @@ cleanup: return ret; } -static enum qemud_save_formats -getCompressionType(struct qemud_driver *driver) +/* Returns enum qemud_save_formats on SUCCESS, or returns -1 if + * specified dump image format is invalid or compression program + * for the format specified is not available on FAILURE. + */ +static int +getDumpType(struct qemud_driver *driver) { int compress = QEMUD_SAVE_FORMAT_RAW; @@ -4801,15 +4805,14 @@ getCompressionType(struct qemud_driver *driver) if (compress < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("Invalid dump image format specified in " - "configuration file, using raw")); - return QEMUD_SAVE_FORMAT_RAW; + "configuration file.")); + return -1; } if (!qemudCompressProgramAvailable(compress)) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("Compression program for dump image format " - "in configuration file isn't available, " - "using raw")); - return QEMUD_SAVE_FORMAT_RAW; + "in configuration file isn't available.")); + return -1; } } return compress; @@ -4865,7 +4868,11 @@ static int qemudDomainCoreDump(virDomainPtr dom, } } - ret = doCoreDump(driver, vm, path, getCompressionType(driver)); + int compress = getDumpType(driver); + if (compress < 0) + goto endjob; + + ret = doCoreDump(driver, vm, path, compress); if (ret < 0) goto endjob; @@ -4937,13 +4944,16 @@ static void processWatchdogEvent(void *data, void *opaque) break; } - ret = doCoreDump(driver, - wdEvent->vm, - dumpfile, - getCompressionType(driver)); - if (ret < 0) - qemuReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("Dump failed")); + int compress = getDumpType(driver); + if (compress >= 0) { + ret = doCoreDump(driver, + wdEvent->vm, + dumpfile, + compress); + if (ret < 0) + qemuReportError(VIR_ERR_OPERATION_FAILED, + "%s", _("Dump failed")); + } ret = doStartCPUs(driver, wdEvent->vm, NULL); -- 1.7.3.2

On Mon, Jan 17, 2011 at 12:39:21PM +0800, Osier Yang wrote:
"getCompressionType" trys to throw error when invalid dump image format is specified, or the compression program for the specified image format is not available, but at the same time, it returns enum qemud_save_formats, as a result, upper function will think it was successful, and disgard to check the error, this fix makes changes so that it returns -1 on FAILURE, And to be consistent with "save", won't use "raw" as the default dump format anymore.
* src/qemu/qemu_driver.c - rename "getCompressionType" to "getDumpType" - return -1 on FAILURE
ACK Daniel

On 01/16/2011 09:39 PM, Osier Yang wrote:
"getCompressionType" trys to throw error when invalid dump image format is specified, or the compression program for the specified image format is not available, but at the same time, it returns enum qemud_save_formats, as a result, upper function will think it was successful, and disgard to check the error, this fix makes changes so that it returns -1 on FAILURE, And to be consistent with "save", won't use "raw" as the default dump format anymore.
* src/qemu/qemu_driver.c - rename "getCompressionType" to "getDumpType" - return -1 on FAILURE
--- src/qemu/qemu_driver.c | 40 +++++++++++++++++++++++++--------------- 1 files changed, 25 insertions(+), 15 deletions(-)
This reverts commit 1b6f13bb7002d by Hu Tao that explicitly falls back to RAW on purpose if a compression program cannot be found. We need more justification why we are reverting a prior patch, and consensus on the correct behavior, before this can be applied. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

于 2011年01月17日 22:41, Eric Blake 写道:
On 01/16/2011 09:39 PM, Osier Yang wrote:
"getCompressionType" trys to throw error when invalid dump image format is specified, or the compression program for the specified image format is not available, but at the same time, it returns enum qemud_save_formats, as a result, upper function will think it was successful, and disgard to check the error, this fix makes changes so that it returns -1 on FAILURE, And to be consistent with "save", won't use "raw" as the default dump format anymore.
* src/qemu/qemu_driver.c - rename "getCompressionType" to "getDumpType" - return -1 on FAILURE
--- src/qemu/qemu_driver.c | 40 +++++++++++++++++++++++++--------------- 1 files changed, 25 insertions(+), 15 deletions(-)
This reverts commit 1b6f13bb7002d by Hu Tao that explicitly falls back to RAW on purpose if a compression program cannot be found. We need more justification why we are reverting a prior patch, and consensus on the correct behavior, before this can be applied.
This patch was to fix: https://bugzilla.redhat.com/show_bug.cgi?id=669586 IMHO there are two choices: 1) report warning instead of error to fall back to RAW. 2) don't fall back to RAW, report error and return -1, just as "qemudDomianSave" does. I choosed 2). Regards Osier
participants (3)
-
Daniel P. Berrange
-
Eric Blake
-
Osier Yang