Re: [libvirt] [PATCH 4/4] libxl: implement virDomainGetNumaParameters
On mar, 2013-07-02 at 14:22 -0600, Jim Fehlig wrote:
> On 06/28/2013 08:33 AM, Dario Faggioli wrote:
> >
> > diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> > index 53af609..9bd6d99 100644
> >
> > +/* NUMA node affinity information is available through libxl
> > + * starting from Xen 4.3. */
> > +#ifdef LIBXL_HAVE_DOMAIN_NODEAFFINITY
> > +
> > +/* Number of Xen NUMA parameters */
> > +#define LIBXL_NUMA_NPARAM 2
>
> Is there a similar definition in Xen? E.g. would future changes to libxl
> adding
> more parameters, but neglecting to update here, cause problems?
>
There is nothing like this in Xen. I went for this #define based
approach because that is right what the QEMU driver does for NUMA
parameters and, also, what libxl driver does for scheduler parameters.
I therefore think that this could be considered safe, from that point of
view. What could happen is that more NUMA (and, perhaps, even more
scheduler) parameters can pop up in the future. However, given libxl
commitment for API stability, this will be advertised with something
like LIBXL_HAVE_THIS_NEW_NUMA_PARAM symbol.
That means we, as soon as we wan to support it, will end up with
something like:
#ifdef LIBXL_HAVE_THIS_NEW_NUMA_PARAM
# define LIBXL_NUMA_NPARAM 3
#else
# define LIBXL_NUMA_NPARAM 2
#endif
Is that reasonable?
> > +
> > +static int
> > +libxlDomainGetNumaParameters(virDomainPtr dom,
> > + virTypedParameterPtr params,
> > + int *nparams,
> > + unsigned int flags)
> > +{
> > +libxlDriverPrivatePtr driver = dom->conn->privateData;
> > +libxlDomainObjPrivatePtr priv;
> > +virDomainObjPtr vm;
> > +
> > +libxl_bitmap nodemap;
> > +virBitmapPtr nodes = NULL;
> > +char *nodeset = NULL;
> > +
> > +int rc, ret = -1;
> > +int i, j;
>
> No need for the extra whitespace between these local variables.
>
Ok, done.
> > +case 1:
> > +/* Node affinity */
> > +
> > +/* Let's allocate both libxl and libvirt bitmaps */
> > +if (libxl_node_bitmap_alloc(priv->ctx, &nodemap, 0) ||
> > +!(nodes = virBitmapNew(libxl_get_max_nodes(priv->ctx {
> > +virReportOOMError();
> > +goto cleanup;
> > +}
> > +
> > +rc = libxl_domain_get_nodeaffinity(priv->ctx, vm->def->id,
> > + &nodemap);
>
> Fits on one line, albeit with nothing to spare :).
>
Does it? According to my editor here, if I put it in just one line that
ends at 82. :-(
I'm therefore changing it in
rc = libxl_domain_get_nodeaffinity(priv->ctx,
vm->def->id,
&nodemap);
As you mentioned in another e-mail.
> > @@ -4741,6 +4876,9 @@ static virDriver libxlDriver = {
> > .domainGetSchedulerParametersFlags =
> > libxlDomainGetSchedulerParametersFlags, /* 0.9.2 */
> > .domainSetSchedulerParameters = libxlDomainSetSchedulerParameters, /*
> > 0.9.0 */
> > .domainSetSchedulerParametersFlags =
> > libxlDomainSetSchedulerParametersFlags, /* 0.9.2 */
> > +#ifdef LIBXL_HAVE_DOMAIN_NODEAFFINITY
> > +.domainGetNumaParameters = libxlDomainGetNumaParameters, /* 1.1.1 */
> > +#endif
> > .nodeGetFreeMemory = libxlNodeGetFreeMemory, /* 0.9.0 */
> > .nodeGetCellsFreeMemory = libxlNodeGetCellsFreeMemory, /* 1.1.1 */
> > .connectDomainEventRegister = libxlConnectDomainEventRegister, /*
> > 0.9.0 */
>
> Looks good with the exception of these minor nits, but needs to go along with
> an
> updated 3/4.
>
Sure.
Thanks and Regards,
Dario
--
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
signature.asc
Description: This is a digitally signed message part
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] libxl: implement virDomainGetNumaParameters
On mar, 2013-07-02 at 14:35 -0600, Jim Fehlig wrote: > BTW, with cppi installed this fails 'make syntax-check' > > preprocessor_indentation > cppi: src/libxl/libxl_driver.c: line 4523: not properly indented > maint.mk: incorrect preprocessor indentation > make: *** [sc_preprocessor_indentation] Error 1 > > Preprocessor nesting is indented as follows > > #if ... > # define ... > # if ... > # define ... > # endif > #endif > Oh, I see... I indeed tried to install cppi, but `yum search' found nothing, probably because of some typo, since I'm finding (and I have installed it) now! :-) Will fix this. Thanks and Regards, Dario -- <> (Raistlin Majere) - Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) signature.asc Description: This is a digitally signed message part -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] libxl: implement virDomainGetNumaParameters
On 07/02/2013 02:22 PM, Jim Fehlig wrote:
On 06/28/2013 08:33 AM, Dario Faggioli wrote:
Although, having it depending on Xen >= 4.3 (by using the proper
libxl feature flag).
Xen currently implements a NUMA placement policy which is basically
the same as the 'interleaved' policy of `numactl', although it can
be applied on a subset of the available nodes. We therefore hardcode
"interleave" as 'numa_mode', and we use the newly introduced libxl
interface to figure out what nodes a domain spans ('numa_nodeset').
With this change, it is now possible to query the NUMA node
affinity of a running domain:
[raistlin@Zhaman ~]$ sudo virsh --connect xen:/// list
IdName State
23F18_x64running
[raistlin@Zhaman ~]$ sudo virsh --connect xen:/// numatune 23
numa_mode : interleave
numa_nodeset : 1
Signed-off-by: Dario Faggioli
---
src/libxl/libxl_driver.c | 138 ++
1 file changed, 138 insertions(+)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 53af609..9bd6d99 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -28,6 +28,7 @@
#include
#include
+#include
#include
#include
@@ -4514,6 +4515,140 @@ libxlDomainSetSchedulerParameters(virDomainPtr dom,
virTypedParameterPtr params,
return libxlDomainSetSchedulerParametersFlags(dom, params, nparams, 0);
}
+/* NUMA node affinity information is available through libxl
+ * starting from Xen 4.3. */
+#ifdef LIBXL_HAVE_DOMAIN_NODEAFFINITY
+
+/* Number of Xen NUMA parameters */
+#define LIBXL_NUMA_NPARAM 2
Is there a similar definition in Xen? E.g. would future changes to libxl
adding more parameters, but neglecting to update here, cause problems?
BTW, with cppi installed this fails 'make syntax-check'
preprocessor_indentation
cppi: src/libxl/libxl_driver.c: line 4523: not properly indented
maint.mk: incorrect preprocessor indentation
make: *** [sc_preprocessor_indentation] Error 1
Preprocessor nesting is indented as follows
#if ...
# define ...
# if ...
# define ...
# endif
#endif
Regards,
Jim
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] libxl: implement virDomainGetNumaParameters
On 06/28/2013 08:33 AM, Dario Faggioli wrote:
Although, having it depending on Xen >= 4.3 (by using the proper
libxl feature flag).
Xen currently implements a NUMA placement policy which is basically
the same as the 'interleaved' policy of `numactl', although it can
be applied on a subset of the available nodes. We therefore hardcode
"interleave" as 'numa_mode', and we use the newly introduced libxl
interface to figure out what nodes a domain spans ('numa_nodeset').
With this change, it is now possible to query the NUMA node
affinity of a running domain:
[raistlin@Zhaman ~]$ sudo virsh --connect xen:/// list
IdName State
23F18_x64running
[raistlin@Zhaman ~]$ sudo virsh --connect xen:/// numatune 23
numa_mode : interleave
numa_nodeset : 1
Signed-off-by: Dario Faggioli
---
src/libxl/libxl_driver.c | 138 ++
1 file changed, 138 insertions(+)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 53af609..9bd6d99 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -28,6 +28,7 @@
#include
#include
+#include
#include
#include
@@ -4514,6 +4515,140 @@ libxlDomainSetSchedulerParameters(virDomainPtr dom, virTypedParameterPtr params,
return libxlDomainSetSchedulerParametersFlags(dom, params, nparams, 0);
}
+/* NUMA node affinity information is available through libxl
+ * starting from Xen 4.3. */
+#ifdef LIBXL_HAVE_DOMAIN_NODEAFFINITY
+
+/* Number of Xen NUMA parameters */
+#define LIBXL_NUMA_NPARAM 2
Is there a similar definition in Xen? E.g. would future changes to libxl adding
more parameters, but neglecting to update here, cause problems?
+
+static int
+libxlDomainGetNumaParameters(virDomainPtr dom,
+ virTypedParameterPtr params,
+ int *nparams,
+ unsigned int flags)
+{
+libxlDriverPrivatePtr driver = dom->conn->privateData;
+libxlDomainObjPrivatePtr priv;
+virDomainObjPtr vm;
+
+libxl_bitmap nodemap;
+virBitmapPtr nodes = NULL;
+char *nodeset = NULL;
+
+int rc, ret = -1;
+int i, j;
No need for the extra whitespace between these local variables.
+
+/* In Xen 4.3, it is possible to query the NUMA node affinity of a domain
+ * via libxl, but not to change it. We therefore only allow AFFECT_LIVE. */
+virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+ VIR_TYPED_PARAM_STRING_OKAY, -1);
+
+libxlDriverLock(driver);
+vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
+libxlDriverUnlock(driver);
+
+if (!vm) {
+virReportError(VIR_ERR_NO_DOMAIN, "%s",
+ _("no domain with matching uuid"));
+goto cleanup;
+}
+
+if (virDomainGetNumaParametersEnsureACL(dom->conn, vm->def) < 0)
+goto cleanup;
+
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("Domain is not running"));
+goto cleanup;
+}
+
+priv = vm->privateData;
+
+libxl_bitmap_init(&nodemap);
+
+if ((*nparams) == 0) {
+*nparams = LIBXL_NUMA_NPARAM;
+ret = 0;
+goto cleanup;
+}
+
+for (i = 0; i < LIBXL_NUMA_NPARAM && i < *nparams; i++) {
+virMemoryParameterPtr param = ¶ms[i];
+
+switch (i) {
+case 0:
+/* NUMA mode */
+
+/* Xen implements something that is is really close to numactl's
+ * 'interleave' policy (see `man 8 numactl' for details). */
+if (virTypedParameterAssign(param, VIR_DOMAIN_NUMA_MODE,
+VIR_TYPED_PARAM_INT,
+VIR_DOMAIN_NUMATUNE_MEM_INTERLEAVE) <
0)
+goto cleanup;
+
+break;
+
+case 1:
+/* Node affinity */
+
+/* Let's allocate both libxl and libvirt bitmaps */
+if (libxl_node_bitmap_alloc(priv->ctx, &nodemap, 0) ||
+!(nodes = virBitmapNew(libxl_get_max_nodes(priv->ctx {
+virReportOOMError();
+goto cleanup;
+}
+
+rc = libxl_domain_get_nodeaffinity(priv->ctx, vm->def->id,
+ &nodemap);
Fits on one line, albeit with nothing to spare :).
+if (rc != 0) {
+virReportSystemError(-rc, "%s",
+ _("unable to get numa affinity"));
+goto cleanup;
+}
+
+/* First, we convert libxl_bitmap into virBitmap. After that,
+ * we format virBitmap as a string that can be returned. */
+virBitmapClearAll(nodes);
+libxl_for_each_set_bit(j, nodemap) {
+if (virBitmapSetBit(nodes, j)) {
+v
