On 04/20/2015 09:53 AM, Michal Privoznik wrote:
> On 18.04.2015 03:45, Cole Robinson wrote:
>> If no <os><type> was specified:
>> before: unknown OS type no OS type
>> after : xml error: an os <type> must be specified
>>
>> If an <os><type> is specified that's not in our capabiliities data:
>> before: unknown OS type: $type
>> after : unsupported configuration: no support found for os <type> '$type'
>>
>> VIR_ERR_OS_TYPE is now unused (as it should be frankly) so drop its strings
>> as well to save our translators some effort.
>> ---
>> src/conf/domain_conf.c | 9 +++++----
>> src/util/virerror.c | 5 +----
>> 2 files changed, 6 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index ab4f2bf..8458f5b 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -14638,8 +14638,8 @@ virDomainDefParseXML(xmlDocPtr xml,
>> if (VIR_STRDUP(def->os.type, "xen") < 0)
>> goto error;
>> } else {
>> - virReportError(VIR_ERR_OS_TYPE,
>> - "%s", _("no OS type"));
>> + virReportError(VIR_ERR_XML_ERROR, "%s",
>> + _("an os <type> must be specified"));
>> goto error;
>> }
>> }
>> @@ -14656,8 +14656,9 @@ virDomainDefParseXML(xmlDocPtr xml,
>> }
>>
>> if (!virCapabilitiesSupportsGuestOSType(caps, def->os.type)) {
>> - virReportError(VIR_ERR_OS_TYPE,
>> - "%s", def->os.type);
>> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> + _("no support found for os <type> '%s'"),
>> + def->os.type);
>> goto error;
>> }
>>
>> diff --git a/src/util/virerror.c b/src/util/virerror.c
>> index 73dae95..aab36ae 100644
>> --- a/src/util/virerror.c
>> +++ b/src/util/virerror.c
>> @@ -900,10 +900,7 @@ virErrorMsg(virErrorNumber error, const char *info)
>> errmsg = _("failed Xen syscall %s");
>> break;
>> case VIR_ERR_OS_TYPE:
>> - if (info == NULL)
>> - errmsg = _("unknown OS type");
>> - else
>> - errmsg = _("unknown OS type %s");
>> + errmsg = "%s";
>
> Even though there are no other calls with VIR_ERR_OS_TYPE, I'd feel more
> safe with:
>
> if (info == NULL)
> errmsg = _("invalid or missing OS type");
> else
> errmsg = "%s";
>
> I find it more future proof.
Thanks for the review. I just the dropped virerror.c diff, rather than add a
new string that needs translation yet isn't used anywhere.
Thanks,
Cole
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list