Stefan Bader wrote:
> On 23.07.2013 23:20, Jim Fehlig wrote:
>
[...]
>> 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.
Yeah, I think so, but as a separate patch.
> But at least
> libxlMakeVfbList->libxlMakeVfb->virPortAllocatorAcquire sounds a bit like it
> might need the driver context.
>
Ah, right, neglected that call chain when suggesting to remove
libxlDriverPrivatePtr.
> From 2d4b7e5ae2644e6f7a2ea930d0899ea426cb9c84 Mon Sep 17 00:00:00 2001
> From: Stefan Bader <stefan.bader@xxxxxxxxxxxxx>
> Date: Fri, 19 Jul 2013 15:20:00 +0200
> Subject: [PATCH] libxl: Correctly initialize vcpu bitmap
>
> The avail_vcpu bitmap has to be allocated before it can be used (using
> 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.
>
> [v2] Use private ctx from virDomainObjPtr->privateData
>
No need for history of patch revisions in the commit message. They can
be added with 'git send-email --annotate ...'.
> Signed-off-by: Stefan Bader <stefan.bader@xxxxxxxxxxxxx>
> ---
> src/libxl/libxl_conf.c | 19 +++++++++++++++----
> src/libxl/libxl_conf.h | 2 +-
> src/libxl/libxl_driver.c | 2 +-
> 3 files changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index 4a0fba9..f732135 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -331,8 +331,10 @@ error:
> }
>
> static int
> -libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_domain_config *d_config)
> +libxlMakeDomBuildInfo(virDomainObjPtr vm, libxl_domain_config *d_config)
> {
> + virDomainDefPtr def = vm->def;
> + libxlDomainObjPrivatePtr priv = vm->privateData;
> libxl_domain_build_info *b_info = &d_config->b_info;
> int hvm = STREQ(def->os.type, "hvm");
> size_t i;
> @@ -343,8 +345,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(priv->ctx, &b_info->avail_vcpus,
> + def->maxvcpus))
>
Fits on one line.
ACK to the patch though. I fixed these minor nits and pushed.
Regards,
Jim
> + goto error;
> + libxl_bitmap_set_none(&b_info->avail_vcpus);
> + for (i = 0; i < def->vcpus; i++)
> + libxl_bitmap_set((&b_info->avail_vcpus), i);
> +
> if (def->clock.ntimers > 0 &&
> def->clock.timers[0]->name == VIR_DOMAIN_TIMER_NAME_TSC) {
> switch (def->clock.timers[0]->mode) {
> @@ -795,14 +804,16 @@ libxlMakeCapabilities(libxl_ctx *ctx)
>
> int
> libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
> - virDomainDefPtr def, libxl_domain_config *d_config)
> + virDomainObjPtr vm, libxl_domain_config *d_config)
> {
> + virDomainDefPtr def = vm->def;
> +
> libxl_domain_config_init(d_config);
>
> if (libxlMakeDomCreateInfo(driver, def, &d_config->c_info) < 0)
> return -1;
>
> - if (libxlMakeDomBuildInfo(def, d_config) < 0) {
> + if (libxlMakeDomBuildInfo(vm, d_config) < 0) {
> return -1;
> }
>
> diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
> index 2b4a281..942cdd5 100644
> --- a/src/libxl/libxl_conf.h
> +++ b/src/libxl/libxl_conf.h
> @@ -126,6 +126,6 @@ libxlMakeVfb(libxlDriverPrivatePtr driver,
>
> int
> libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
> - virDomainDefPtr def, libxl_domain_config *d_config);
> + virDomainObjPtr vm, libxl_domain_config *d_config);
>
> #endif /* LIBXL_CONF_H */
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 358d387..98b1985 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -968,7 +968,7 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
>
> libxl_domain_config_init(&d_config);
>
> - if (libxlBuildDomainConfig(driver, vm->def, &d_config) < 0)
> + if (libxlBuildDomainConfig(driver, vm, &d_config) < 0)
> goto error;
>
> if (driver->autoballoon && libxlFreeMem(priv, &d_config) < 0) {
> -- 1.7.9.5
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list