Re: [Xen-devel] [PATCH RFC 1/9] libxl idl: add comments to error enum

[Rob Hoes <Rob.Hoes@xxxxxxxxxx>]

> On 24 Jun 2015, at 16:06, Ian Jackson <ian.jackson@xxxxxxxxxxxxx> wrote:
...
>> +    # Out of memory (malloc or similar failed)
>>     (-5, "NOMEM"),
> 
> Should say whether this includes "host does not have enough memory to
> create the specified domain" as well as "malloc failed in toolstack".
> 
> NB that we are trying to phase out the handling of the latter as
> anything except a fatal error.

Indeed, we should probably split this into something like 
ERROR_NOMEM_FOR_NEW_DOMAIN and ERROR_NOMEM_IN_TOOLSTACK.

> 
>> +    # General failure; code should be avoided
>>     (-6, "INVAL"),
> 
> ERROR_INVAL means "there is something wrong with the parameters to the
> operation (but we don't say exactly what).

Yes, this is an easy target for splitting out into many ERROR_INVAL_<some 
parameter> (since we cannot parametrise the error itself).

> 
>> +    # General failure; code should be avoided (used only in "xl")
>>     (-7, "BADFAIL"),
> 
> Maybe it should be renamed.

Perhaps ERROR_INTERNAL_XL?

> 
>> +    # Domain responded to suspend request
>>     (-8, "GUEST_TIMEDOUT"),
> 
> Failed to respond, you mean.

Uhm, yes :)

>> +    # A xenstore watch has timed out
>>     (-9, "TIMEDOUT"),
> 
> Not true, I'm afraid.  xenstore watches cannot time out, and this is
> used for other things besides `we were waiting for something in
> xenstore and decided it was taking too long' (which anyway does not
> really say what it was that we were waiting for - lots of things
> communicate via xenstore).
> 
> Also this error doesn't say what timed out, so this error code should
> be deprecated.

Ok, so weâll probably need to replace it by ERROR_<something>_TIMEDOUT, e.g. 
ERROR_HOTPLUG_TIMEDOUT.

>> +    # Event has not happened (libxl_event_check)
>>     (-11, "NOT_READY"),
> 
> I would say "Requested event has not (yet) happened".
> 
>> +    # Could not acquire lock
>>     (-15, "LOCK_FAIL"),
> 
> This seems to be used entirely for the userdata lock.  I would be
> tempted to argue that it's not particularly be useful to report this
> distinctly to the application, because what it really means is "either
> the system is totally hosed, or the permissions or existence of the
> userdata storage area are wrong".  The latter ought to be a separate
> error code.

For the âtotally hosedâ condition, we could use ERROR_INTERNAL (as in some of 
my other patches), implying âplease kill me or just reboot the hostâ.

> 
>> +    # Unable to find JSON domain config
>>     (-16, "JSON_CONFIG_EMPTY"),
> 
> There is clearly something wrong here.  This is supposed to be handled
> by most callers I think.  The comment at the one generation site says:
> 
>        /* No logging, not necessary an error from caller's PoV. */
>        rc = ERROR_JSON_CONFIG_EMPTY;
>        goto out;
> 
> But I can find no special handling of it by callers.

I did see this error when testing Xen 4.5 under xenopsd+libxl. It can happen 
when executing a libxl function on a domain that was not started by libxl, for 
example dom0. Specifically, we saw it because we didnât run xen-init-dom0 yet, 
and we tried to attach a block device to dom0 using libxl.

Cheers,
Rob

> 
>> +    # Remus ops do not match device
>>     (-18, "REMUS_DEVOPS_DOES_NOT_MATCH"),
> 
> I think this is supposed to be internal-only.
> 
>> +    # Remus device not supported
>>     (-19, "REMUS_DEVICE_NOT_SUPPORTED"),
> 
> Starting remus was attempted but the domain has a device which is not
> supported by remus.
> 
>> +    # Requested domain was not found
>>     (-21, "DOMAIN_NOTFOUND"),
> 
> AFAICT might also sometimes happen if the domain disappears during an
> operation ?
> 
> Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel