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

Re: [Xen-devel] [PATCH v3 11/14] libxl: get and set soft affinity

On 20/11/13 11:27, Ian Campbell wrote:
On Tue, 2013-11-19 at 18:51 +0100, Dario Faggioli wrote:
+    libxl_cputopology *topology;
+    libxl_bitmap ecpumap;
+    int nr_cpus = 0, rc;
+    topology = libxl_get_cpu_topology(ctx, &nr_cpus);
+    if (!topology) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "failed to retrieve CPU topology");
It's not consistent within the file but I think for new functions we
should use the LOG macro variants.

Right, but don't I need a gc to use it? Should I "make up" one just for
the purpose of using LOG/LOGE?
I think a call to GC_INIT/GC_FREE should be cheap enough.

+        return ERROR_FAIL;
+    }
+    libxl_cputopology_list_free(topology, nr_cpus);
Why are you retrieving this only to immediately throw it away?

Because I need nr_cpus. :-)
Surely this is not the recommended way to get nr_cpus!

libxl_get_cpu_topology() itself calls libxl_get_max_cpus() which seems
like the obvious candidate.

diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index c7dceda..504c57b 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -82,6 +82,20 @@
+ * LIBXL_HAVE_VCPUINFO_SOFTAFFINITY indicates that a 'cpumap_soft'
+ * field (of libxl_bitmap type) is present in libxl_vcpuinfo,
+ * containing the soft affinity for the vcpu.
+ */
+ * LIBXL_HAVE_BUILDINFO_SOFTAFFINITY indicates that a 'cpumap_soft'
+ * field (of libxl_bitmap type) is present in libxl_domain_build_info,
+ * containing the soft affinity for the vcpu.
+ */
Given that they arrive can we just use HAVE_SOFTRAFFINITY?

You mean just introducing one #define? Sure... For some reason I assumed
that every new field should come with it's own symbol. But if it's fine
to have one, I'm all for it. :-)
I think it's ok.

+/* Flags, consistent with domctl.h */
Can these be an enum in the idl?

I think yes.

I did actually check and, of all the enum-s in the IDL, none are used as
flags, they're rather used as "single values". OTOH, the only actual
flags I found (I think it was LIBXL_SUSPEND_DEBUG, LIBXL_SUSPEND_LIVE)
were defined like I did myself above... That's why I went for it.
I have a feeling they predate the IDL, or at least the Enumeration
support. It's true that we don't have any other bit fields in enums
though. I can't see the harm, it's probably not worth introducing a new
IDL type for them.

Since these are bits, not numbers, I don't think an enum is the right construct. Or, the enum values should be the *bit numbers*, and the flags should be (1<<[bit_humber]).


Xen-devel mailing list



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