From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefan Bader Subject: Re: [libvirt] [Xen-devel] [PATCH] libxl: Correctly initialize vcpu bitmap Date: Wed, 24 Jul 2013 13:43:43 +0200 Message-ID: <51EFBDEF.1090103@canonical.com> References: <1374490265-2019-1-git-send-email-stefan.bader@canonical.com> <20130722193959.GQ30300@phenom.dumpdata.com> <51EEF3A9.3000705@suse.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============3760876489725588321==" Return-path: In-Reply-To: <51EEF3A9.3000705@suse.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com To: Jim Fehlig Cc: libvir-list@redhat.com, xen-devel@lists.xensource.com, Ian Campbell , Konrad Rzeszutek Wilk List-Id: xen-devel@lists.xenproject.org This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --===============3760876489725588321== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="------------enigF9E8E15D4F9138C1BDEB5176" This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enigF9E8E15D4F9138C1BDEB5176 Content-Type: multipart/mixed; boundary="------------050902070800000408030905" This is a multi-part message in MIME format. --------------050902070800000408030905 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable On 23.07.2013 23:20, Jim Fehlig wrote: > One comment below in addition to Konrad's... >=20 > Konrad Rzeszutek Wilk wrote: >> On Mon, Jul 22, 2013 at 12:51:05PM +0200, Stefan Bader wrote: >> =20 >>> 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 20= 01 >>> From: Stefan Bader >>> 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= >>> =20 >> >> avail_vcpu ? >> >> =20 >>> 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 >>> =20 >> >> The libxl calling logic looks Ok to me. So from the libxl perspective >> you can tack on Reviewed-by: Konrad Rzeszutek Wilk >> >> =20 >>> --- >>> 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: >>> } >>> =20 >>> static int >>> -libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_domain_config *d_co= nfig) >>> +libxlMakeDomBuildInfo(libxlDriverPrivatePtr driver, virDomainDefPtr = def, >>> + libxl_domain_config *d_config) >>> { >>> libxl_domain_build_info *b_info =3D &d_config->b_info; >>> int hvm =3D 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 =3D def->maxvcpus; >>> - libxl_bitmap_set((&b_info->avail_vcpus), def->vcpus); >>> + if (libxl_cpu_bitmap_alloc(driver->ctx, &b_info->avail_vcpus, >>> + def->maxvcpus)) >>> =20 >=20 > You are using the driver-wide libxl_ctx in libxlDriverPrivate here, but= > should be using the per-domain libxl_ctx in libxlDomainObjPrivate > structure IMO. >=20 > It looks like libxlBuildDomainConfig, which calls libxlMakeDomBuildInfo= , > should have just taken virDomainObjPtr instead of virDomainDefPtr.=20 > libxlBuildDomainConfig would then have access to the per-domain > libxl_ctx, and no longer need the libxlDriverPrivatePtr parameter as we= ll. >=20 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 call= ing 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 --------------050902070800000408030905 Content-Type: text/x-diff; name="0001-libxl-Correctly-initialize-vcpu-bitmap.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="0001-libxl-Correctly-initialize-vcpu-bitmap.patch" =46rom 2d4b7e5ae2644e6f7a2ea930d0899ea426cb9c84 Mon Sep 17 00:00:00 2001 From: Stefan Bader 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 Signed-off-by: Stefan Bader --- 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: } =20 static int -libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_domain_config *d_config= ) +libxlMakeDomBuildInfo(virDomainObjPtr vm, libxl_domain_config *d_config)= { + virDomainDefPtr def =3D vm->def; + libxlDomainObjPrivatePtr priv =3D vm->privateData; libxl_domain_build_info *b_info =3D &d_config->b_info; int hvm =3D STREQ(def->os.type, "hvm"); size_t i; @@ -343,8 +345,15 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_dom= ain_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 =3D def->maxvcpus; - libxl_bitmap_set((&b_info->avail_vcpus), def->vcpus); + if (libxl_cpu_bitmap_alloc(priv->ctx, &b_info->avail_vcpus, + def->maxvcpus)) + goto error; + libxl_bitmap_set_none(&b_info->avail_vcpus); + for (i =3D 0; i < def->vcpus; i++) + libxl_bitmap_set((&b_info->avail_vcpus), i); + if (def->clock.ntimers > 0 && def->clock.timers[0]->name =3D=3D VIR_DOMAIN_TIMER_NAME_TSC) { switch (def->clock.timers[0]->mode) { @@ -795,14 +804,16 @@ libxlMakeCapabilities(libxl_ctx *ctx) =20 int libxlBuildDomainConfig(libxlDriverPrivatePtr driver, - virDomainDefPtr def, libxl_domain_config *d_confi= g) + virDomainObjPtr vm, libxl_domain_config *d_config= ) { + virDomainDefPtr def =3D vm->def; + libxl_domain_config_init(d_config); =20 if (libxlMakeDomCreateInfo(driver, def, &d_config->c_info) < 0) return -1; =20 - if (libxlMakeDomBuildInfo(def, d_config) < 0) { + if (libxlMakeDomBuildInfo(vm, d_config) < 0) { return -1; } =20 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, =20 int libxlBuildDomainConfig(libxlDriverPrivatePtr driver, - virDomainDefPtr def, libxl_domain_config *d_confi= g); + virDomainObjPtr vm, libxl_domain_config *d_config= ); =20 #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, virDomainO= bjPtr vm, =20 libxl_domain_config_init(&d_config); =20 - if (libxlBuildDomainConfig(driver, vm->def, &d_config) < 0) + if (libxlBuildDomainConfig(driver, vm, &d_config) < 0) goto error; =20 if (driver->autoballoon && libxlFreeMem(priv, &d_config) < 0) { --=20 1.7.9.5 --------------050902070800000408030905-- --------------enigF9E8E15D4F9138C1BDEB5176 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with undefined - http://www.enigmail.net/ iQIcBAEBCgAGBQJR773vAAoJEOhnXe7L7s6jfsoP/3RU0qJv8oFATAeCqgnK7bBV T+XvH+HCV+M0/+Ui1/+nTDNlkGzfFc8ElWKLWl1OaLOsUhJFVXjXE5cXspZ3HX8i nBYbl+3RtIGBV+WbfLws7vCMn5b4rAzg3yPu4ofdwIermx7UdWqUkn9hG9OV7iwh zCe26hA8+w48w6OMVAJAVtlw99crwlo60NfyLlOmhgUOivA3WbBC7NyJLSVVq+H7 GnoIyHg2vfQfYdrCYVsFiM3X3ZBya1C1aNgtVcAAuGt4tKqF3XYifj8aJbVv47SA fiPvBhVWa1+4LTeHzOuHJ90Ftx6D2h4XGzTcm2GwaWHkxCfaBOxCS4c4CLqXvQky m+37CVWuKbDQQv35lUwajcvCs1UpVwv044gTutA1Vs4QMXFqPLfSNoGr4d09aIak 1ibR3v8Mrt+mWnrzeIJDmM7uEwIhCBNC8/DV4gXzW4jPdRjVW3Q4sOVwhcfe5laJ L5HOdWAJ/FuxgbmsApQCoTLmwuCFEHXjARFbsZr5CAwhS0KQBklVaGWqGsER+zeU SKOONOO7GHqSIefgfg/f3H9nYqeLdKrXHaPPsNyb/ySk/dC8VD21jxAOoFBUAizV 5Hjn0dXknplIZS0LE1HW1XU4+Ntwp+rHL2pLtqoeuJpBSuSZkT/TuUCPyGMgH+T+ K23vP6TeQCubL9nbi7Zb =pYw3 -----END PGP SIGNATURE----- --------------enigF9E8E15D4F9138C1BDEB5176-- --===============3760876489725588321== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline --===============3760876489725588321==--