[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/2] xen: add error handling to UUID parsing



Hi Eric,
On Thu, Oct 06, 2011 at 10:50:46AM -0600, Eric Blake wrote:
> On 10/06/2011 03:15 AM, Guido Günther wrote:
> >otherwise a missing UUID in a domain config just shows:
> >
> >libxlDomainXMLFromNative:2600 : Internal Error parsing xm config failed
> >
> >Now we have:
> >
> >xenXMConfigGetUUID:186 : Internal Error config value uuid was missing
> 
> Still doesn't sound quite right - it's not an internal error (where
> libvirt has gotten internal logic wrong) so much as a user-supplied
> data validation error.
> 
> >+    if (val->type != VIR_CONF_STRING) {
> >+        XENXS_ERROR(VIR_ERR_INTERNAL_ERROR,
> >+                   _("config value %s not a string"), name);
> >+        return -1;
> >+    }
> >+
> >+    if (!val->str) {
> >+        XENXS_ERROR(VIR_ERR_INTERNAL_ERROR,
> >+                   _("%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_INTERNAL_ERROR,
> >+                   _("%s not parseable"), val->str);
> 
> 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

> 
> ACK to the concept, once we decide on the correct error code.
> 
> -- 
> Eric Blake   eblake redhat com    +1-801-349-2682
> Libvirt virtualization library http://libvirt.org
> 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]