From mboxrd@z Thu Jan 1 00:00:00 1970 From: Koushik Chakravarty Subject: Re: tools/libxl - Async Task Cancellation Query Date: Wed, 8 Apr 2015 12:13:41 +0000 Message-ID: References: <21797.3921.740361.402596@mariner.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <21797.3921.740361.402596@mariner.uk.xensource.com> Content-Language: en-US List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Jackson Cc: "'xen-devel@lists.xensource.com'" , Euan Harris List-Id: xen-devel@lists.xenproject.org Thanks Ian for the answers. I have a follow - up on the below: "Can I suggest adding a unique private 'id' field to the libxl_asyncop_how structure, that will be populated by AO_CREATE? This will help finding the matching corresponding libxl_ao from the ctx->aos_inprogress in libxl_ao_cancel() quicker by looking for search->id == libxl_asyncop_how->id. That would require the caller to preserve the ao_how which seems awkward to me. Also, allocating these ids presents some difficulties. I think it is better to allow the caller to identify aos." When you say - " That would require the caller to preserve the ao_how which seems awkward to me " - whom do you refer as the caller? >>From what I picked up, the libxl_ao_cancel() API is passed with the ao_how. The libxl_ao_cancel then looks into the ctx->aos_inprogress list to find the ao that matches the ao_how. So what I was suggesting was that if the ao_how was allotted an 'id' from the original libxl api call (done using a counter increment from the global libxl ctx with a lock held), and the same id was saved in the ao entry in ctx->aos_inprogress, then searching/matching them in libxl_ao_cancel() would have been a little faster. Let me know if I am not being very clear. Regards, Koushik Chakravarty Mobile - +91-9663396424 -----Original Message----- From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com] Sent: Wednesday, April 8, 2015 4:52 PM To: Koushik Chakravarty Cc: Euan Harris; 'xen-devel@lists.xensource.com' Subject: Re: tools/libxl - Async Task Cancellation Query Koushik Chakravarty writes ("tools/libxl - Async Task Cancellation Query"): > I am currently looking into the asynchronous task cancellation in libxl and have a few very specific queries, if you could answer. Sure. Thanks for what has evidently been a careful review of the code. > 1. In libxl_domain_resume(),why is libxl_ao_complete called before AO_INPROGRESS? I was going to refer you to the internal API documentation, but I find that it doesn't describe this situation. I have drafted a patch to the documentation which I hope will answer this question. I'll send it as a followup to this mail. > 2. In libxl_ao_cancel() - the function goes through the ctx->aos_inprogress and tries to find a suitable libxl_ao that matches the input libxl_asyncop_how. It does so, by a few 'if' checks. Regarding this - > > a. Where does the libxl__ao get inserted to the ctx->aos_inprogress? I could not find that somehow - sorry if I overlooked. Thank you again for your review. I think you have spotted a bug. I will send a followup patch, again as a followup to this mail. > b. Can I suggest adding a unique private 'id' field to the libxl_asyncop_how structure, that will be populated by AO_CREATE? This will help finding the matching corresponding libxl_ao from the ctx->aos_inprogress in libxl_ao_cancel() quicker by looking for search->id == libxl_asyncop_how->id. That would require the caller to preserve the ao_how which seems awkward to me. Also, allocating these ids presents some difficulties. I think it is better to allow the caller to identify aos. > 3. In libxl_device_vkb_add(), shouldn't the function invoke libxl__ao_abort in the error path? I think this will be answered by my documentation patch. But, to summarise, the codepaths: assert(rc); libxl__ao_complete(egc, ao, rc); return AO_INPROGRESS; and assert(rc); return AO_ABORT(rc); are equivalent. Ian.