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

Osier Yang jyang at redhat.com
Wed Jan 26 11:06:43 UTC 2011


于 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




More information about the libvir-list mailing list