[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2] qemu: Warning prompt if invalid dump image format specified



于 2011年01月26日 01:02, Eric Blake 写道:
On 01/25/2011 01:42 AM, 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.

s/disgard to check/discard/ (another option might be "disregard")


As it reverted b6f13bb7002da76db15242e1d996e8a6222a754, and it was
intentionally to chose the behavior of falling back to raw if the
compression type is invalid, so simply use "VIR_WARN" to fix the
problem.

* src/qemu/qemu_driver.c
---
  src/qemu/qemu_driver.c |   13 ++++++-------
  1 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 34cc29f..dac71fa 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4806,16 +4806,15 @@ getCompressionType(struct qemud_driver *driver)
      if (driver->dumpImageFormat) {
          compress = qemudSaveCompressionTypeFromString(driver->dumpImageFormat);
          if (compress<  0) {
-            qemuReportError(VIR_ERR_OPERATION_FAILED, "%s",
-                            _("Invalid dump image format specified in "
-                              "configuration file, using raw"));
+            VIR_WARN("Invalid dump image format '%s' specified in "
+                     "configuration file, using raw",
+                     driver->dumpImageFormat);

Hmm, I was about to straight ACK this, then I noticed that this loses
the translation.  Which means that VIR_WARN is generally intended for
situations that the end user should never see (or we would have
translated it to the user's language).

On the other hand, I note that nwfilter/nwfilter_ebiptables_driver.c
uses VIR_WARN for an end-user-visible message, by pre-translating the
message into an snprintf() buffer, then passing that translation through
VIR_WARN0 (in order to bypass the 'make syntax-check' rule that calls us
for the majority of VIR_WARN scenarios where we don't want translation).
  That also feels awkward.

I'm not sure what the best approach is here - requiring translation for
all VIR_WARN seems awkward, but so does nwfilter's workaround just to
avoid a syntax-check rule.  Maybe we need a dedicated helper method that
requires translation but has the same effect as VIR_WARN, which both
nwfilter and this code can use.

Didn't noticed nwfilter driver translates messages for VIR_WARN, good
to known it. But all the VIR_WARN in qemu driver doesn't translate it.
so, will it be fine to push the this patch first, and make dedicated
helper method later as another patch? :-)

Thanks
Osier


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]