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

On 04/17/2015 07:06 AM, 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: s/capabiliities/capabilities/ > 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. NACK to that part - even if a newer libvirtd never sends the error, newer clients can still connect to older libvirtd and the new client must still be prepared to receive the error from the older server. We are stuck carrying the translation, even if we no longer generate it.

> --- > 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 4d7e3c9..a145e11 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; > } ACK to these two hunks.