On 24 Jun 2015, at 16:28, Ian Jackson <ian.jack...@eu.citrix.com> wrote:
>
> Rob Hoes writes ("[PATCH RFC 7/9] libxl: introduce specific error codes in
> libxl_device_disk_add"):
>> Signed-off-by: Rob Hoes <rob.h...@citrix.com>
> ...
>> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
>> index f622981..2f56c6e 100644
>> --- a/tools/libxl/libxl.c
>> +++ b/tools/libxl/libxl.c
>> @@ -2008,9 +2008,16 @@ static int libxl__device_nextid(libxl__gc *gc,
>> uint32_t domid, char *device)
>> static int libxl__resolve_domid(libxl__gc *gc, const char *name,
>> uint32_t *domid)
>> {
>> + int rc;
>> if (!name)
>> return 0;
>> - return libxl_domain_qualifier_to_domid(CTX, name, domid);
>> +
>> + rc = libxl_domain_qualifier_to_domid(CTX, name, domid);
>> +
>> + if (rc < 0)
>> + return ERROR_DOMAIN_NOTFOUND;
>> + else
>> + return rc;
>
> This doesn't seem right. You're smashing all the errors to domain not
> found. Surely libxl_domain_qualifier_to_domid should return the right
> error to start with.
The idea was to translate a lower-level error into one that makes sense in the
context of the higher-level function. When talking about this offline, I think
that we concluded that this may be the right thing to do in certain cases, but
then we should explicitly translate specific error codes rather than doing it
by “if (rc < 0)”.
However, in this specific case, I should have probably changed
libxl_name_to_domid instead, to return ERROR_DOMAIN_NOTFOUND rather than
ERROR_INVAL.
>
> The rest looks plausible (modulo my quibbles about names in my earlier
> mails).
>
> Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
