From mboxrd@z Thu Jan  1 00:00:00 1970
From: Roger Pau Monne <roger.pau@citrix.com>
Subject: Re: [PATCH v2 07/15] libxl: convert
 libxl_domain_destroy to an async op
Date: Wed, 23 May 2012 10:47:17 +0100
Message-ID: <4FBCB225.3020507@citrix.com>
References: <1337695365-5142-1-git-send-email-roger.pau@citrix.com>
	<1337695365-5142-8-git-send-email-roger.pau@citrix.com>
	<20411.50812.100404.117424@mariner.uk.xensource.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <20411.50812.100404.117424@mariner.uk.xensource.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>
List-Id: xen-devel@lists.xenproject.org

Ian Jackson wrote:
>> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
>> index d2c013c..68d076c 100644
>> --- a/tools/libxl/libxl_internal.h
>> +++ b/tools/libxl/libxl_internal.h
> ...
>> +_hidden int libxl__ao_device_check_last(libxl__gc *gc, libxl__ao_device *device,
>> +                                        libxl__ao_device *list, int num,
>> +                                        int *last);
>> +
>
> This is a good helper but it could do with a doc comment explaining
> what it does.
>
> Just a suggestion: rather than this business with *last it might be
> easier to assign a magic error code, or use +1, for "not all done
> yet".  (+1 would work since all actual libxl error codes are -ve).
> By my reading of the current interface the return value is not
> meaningful if *last==0 on output, which is the opposite of the usual
> way round.

This function currently returns two different things, the return value 
returns the rc code of any operation that has failed, and the parameter 
last marks if all device events are finished. Without the last 
parameter, we won't be able to return the failed error code if all 
devices had finished, since we had to return 0, so the error code would 
be lost.

Since I think this is not really clear, I will split this function into 
two different ones, one that checks if all devices are finished, and 
another one that checks if some devices have reported errors.