Re: [libvirt] [PATCH 01/11] domain: conf: Better errors on bad os <type> values

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.