On Fri, Oct 07, 2011 at 12:18:51PM +0200, Guido Günther wrote:
> On Thu, Oct 06, 2011 at 04:33:49PM -0600, Eric Blake wrote:
> > On 10/06/2011 11:36 AM, Guido Günther wrote:
> > >>These three errors should probably all be changed away from
> > >>VIR_ERR_INTERNAL_ERROR into something more useful, but I'm not sure
> > >>whether that would be VIR_ERR_CONF_SYNTAX,
> > >>VIR_ERR_CONFIG_UNSUPPORTED, or something else.
> > >
> > >I was uncertain about the error codes too, that's why I kept
> > >VIR_ERR_INTERNAL_ERROR as used by other functions in this file. I'd
> > >opt for VIR_ERR_CONF_SYNTAX.
> > > -- Guido
> >
> > Yeah, for this particular case, where we are parsing a xen conf
> > file, CONF_SYNTAX is the best fit.
> >
> > >
> > >>
> > >>ACK to the concept, once we decide on the correct error code.
> >
> > I think we've decided.
> (Hopefully) final version attached. I've left the first error as
> INTERNAL_ERROR since this it's there do detect false function
> parameters. O.k. to apply?
Ping?
-- Guido
> Cheers,
> -- Guido
> >From 573d2a5a575dd551b92e603201bbae7bb5fa1733 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Guido=20G=C3=BCnther?= <agx@xxxxxxxxxxx>
> Date: Thu, 6 Oct 2011 12:42:39 +0200
> Subject: [PATCH] xen: add error handling to UUID parsing
>
> otherwise a missing UUID in a domain config just shows:
>
> error: An error occured, but the cause is unknown
>
> Now we have:
>
> error: configuration file syntax error: config value uuid was missing
> ---
> src/xenxs/xen_xm.c | 37 +++++++++++++++++++++++++++----------
> 1 files changed, 27 insertions(+), 10 deletions(-)
>
> diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
> index 70facf7..ff173d8 100644
> --- a/src/xenxs/xen_xm.c
> +++ b/src/xenxs/xen_xm.c
> @@ -174,21 +174,38 @@ static int xenXMConfigCopyStringOpt(virConfPtr conf,
> /* Convenience method to grab a string UUID from the config file object */
> static int xenXMConfigGetUUID(virConfPtr conf, const char *name, unsigned char *uuid) {
> virConfValuePtr val;
> - if (!uuid || !name || !conf)
> - return (-1);
> +
> + if (!uuid || !name || !conf) {
> + XENXS_ERROR(VIR_ERR_INVALID_ARG,
> + _("Arguments must be non null"));
> + return -1;
> + }
> +
> if (!(val = virConfGetValue(conf, name))) {
> - return (-1);
> + XENXS_ERROR(VIR_ERR_CONF_SYNTAX,
> + _("config value %s was missing"), name);
> + return -1;
> }
>
> - if (val->type != VIR_CONF_STRING)
> - return (-1);
> - if (!val->str)
> - return (-1);
> + if (val->type != VIR_CONF_STRING) {
> + XENXS_ERROR(VIR_ERR_CONF_SYNTAX,
> + _("config value %s not a string"), name);
> + return -1;
> + }
> +
> + if (!val->str) {
> + XENXS_ERROR(VIR_ERR_CONF_SYNTAX,
> + _("%s can't be empty"), name);
> + return -1;
> + }
>
> - if (virUUIDParse(val->str, uuid) < 0)
> - return (-1);
> + if (virUUIDParse(val->str, uuid) < 0) {
> + XENXS_ERROR(VIR_ERR_CONF_SYNTAX,
> + _("%s not parseable"), val->str);
> + return -1;
> + }
>
> - return (0);
> + return 0;
> }
>
> #define MAX_VFB 1024
> --
> 1.7.6.3
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list