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

[Michal Privoznik]

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.

Michal