On 23.07.2013 23:20, Jim Fehlig wrote:
One comment below in addition to Konrad's...
Konrad Rzeszutek Wilk wrote:
On Mon, Jul 22, 2013 at 12:51:05PM +0200, Stefan Bader wrote:
This fixes the basic setup but there is likely more to do if things like manual CPU hirarchy (nodes, cores, threads) to be working.
Cross-posting to xen-devel to make sure I am doing things correctly.
-Stefan
From 1ec5e7ea0d3498b9f61b83e8aed87cc3cae106de Mon Sep 17 00:00:00 2001 From: Stefan Bader <stefan.bader@canonical.com> Date: Fri, 19 Jul 2013 15:20:00 +0200 Subject: [PATCH] libxl: Correctly initialize vcpu bitmap
The avai_vcpu bitmap has to be allocated before it can be used (using
avail_vcpu ?
the maximum allowed value for that). Then for each available VCPU the bit in the mask has to be set (libxl_bitmap_set takes a bit position as an argument, not the number of bits to set).
Without this, I would always only get one VCPU for guests created through libvirt/libxl.
Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
The libxl calling logic looks Ok to me. So from the libxl perspective you can tack on Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
--- src/libxl/libxl_conf.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 4a0fba9..7592dd2 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -331,7 +331,8 @@ error: }
static int -libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_domain_config *d_config) +libxlMakeDomBuildInfo(libxlDriverPrivatePtr driver, virDomainDefPtr def, + libxl_domain_config *d_config) { libxl_domain_build_info *b_info = &d_config->b_info; int hvm = STREQ(def->os.type, "hvm"); @@ -343,8 +344,15 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_domain_config *d_config) libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_HVM); else libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PV); + b_info->max_vcpus = def->maxvcpus; - libxl_bitmap_set((&b_info->avail_vcpus), def->vcpus); + if (libxl_cpu_bitmap_alloc(driver->ctx, &b_info->avail_vcpus, + def->maxvcpus))
You are using the driver-wide libxl_ctx in libxlDriverPrivate here, but should be using the per-domain libxl_ctx in libxlDomainObjPrivate structure IMO.
It looks like libxlBuildDomainConfig, which calls libxlMakeDomBuildInfo, should have just taken virDomainObjPtr instead of virDomainDefPtr. libxlBuildDomainConfig would then have access to the per-domain libxl_ctx, and no longer need the libxlDriverPrivatePtr parameter as well.
So more like attached v2. I am not sure libxlDriverPrivatePtr can really be dropped (and maybe should not as part of a fix to this issue). Maybe calling libxl_flask_context_to_sid also should use the per domain context. But at least libxlMakeVfbList->libxlMakeVfb->virPortAllocatorAcquire sounds a bit like it might need the driver context. -Stefan