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

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.