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

Re: [Xen-devel] [PATCH v3 10/10] libxl: fix caller of libxl_cpupool functions

On 07/16/2015 10:57 AM, Dario Faggioli wrote:
[Adding Juergen]

Near miss. You took my old mail address. :-)

On Wed, 2015-07-15 at 18:16 +0100, Wei Liu wrote:
On Tue, Jul 14, 2015 at 06:33:22PM +0100, Ian Jackson wrote:
Wei Liu writes ("[PATCH v3 10/10] libxl: fix caller of libxl_cpupool 

--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -771,8 +771,11 @@ libxl_cpupoolinfo * libxl_list_cpupool(libxl_ctx *ctx, int 

      poolid = 0;
      for (i = 0;; i++) {
-        if (cpupool_info(gc, &info, poolid, false))
+        libxl_cpupoolinfo_init(&info);
+        if (cpupool_info(gc, &info, poolid, false)) {
+            libxl_cpupoolinfo_dispose(&info);
+        }

I'm not convinced by this change.

I think that this function has broken error handling: if cpupool_info
fails, it simply ignores the error.

I think the semantics of this function is to get back as many cpupool
info as it can.

Yes, I also think this is the case.


Unfortunately the failing of cpupool_info can either mean the cpupool id
does not exist or other internal errors.  So not cleaning up is the
right thing to do (speaking from semantics point of view).


I think I need to overhaul cpupool_info a bit if we want to make this
API better.

Well, perhaps having cpupool_info() treating specially the situation
where the pool ID is not there may help... However, what would you do
once you have this additional piece of information available?

Maybe, depending on the error, we can cleanup the whole array? Is it
this that we are after?

I think the best would be:

Modify cpupool_info() to return either success, internal error, or no
cpupool found.

In case of internal error libxl_list_cpupool() should clean up and
return NULL.

I'm not sure if it is possible to interleave pool ids. If so, we need a
way to get back the exact number of cpupools. Dario?

libxl_list_cpupool() does that (2nd parameter).

Sorry, what you mean by 'interleave pool ids'?

Not sure if this is an answer to interleaving of pool ids, but it is
possible to specify the pool id when creating a new cpupool at the
libxc interface. Even in case this is not used, pool ids can easily
be sparse after having deleted a cpupool.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.