[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