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

[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.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110125/c31c6f50/attachment-0001.sig>