|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3 3/7] libxl: In libxl_set_vcpuonline check for maximum number of VCPUs against the cpumap.
On Tue, Mar 24, 2015 at 03:22:55PM +0000, Ian Campbell wrote:
> On Mon, 2015-03-23 at 14:20 -0400, Konrad Rzeszutek Wilk wrote:
> > There is no sense in trying to online (or offline) CPUs when the size of
> > cpumap is greater than the maximum number of VCPUs the guest can go to.
> >
> > As such fail the operation if the count of CPUs to online is greater
> > than what the guest started with. For the offline case we do not
> > check (as the bits are unset in the cpumap) and let it go through.
> >
> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> > ---
> > tools/libxl/libxl.c | 27 ++++++++++++++++++++++++---
> > 1 file changed, 24 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> > index 4152ee4..d2b5ff3 100644
> > --- a/tools/libxl/libxl.c
> > +++ b/tools/libxl/libxl.c
> > @@ -5449,6 +5449,20 @@ int libxl_domain_get_nodeaffinity(libxl_ctx *ctx,
> > uint32_t domid,
> > return 0;
> > }
> >
> > +static int libxl__check_max(libxl__gc *gc, libxl_dominfo *info,
> > + libxl_bitmap *cpumap)
> > +{
> > + int maxcpus = libxl_bitmap_count_set(cpumap);
> > +
> > + if (maxcpus > info->vcpu_max_id + 1)
> > + {
> > + LOGE(ERROR, "You have a max of %d vCPUs and you want %d vCPUs!",
> > + info->vcpu_max_id + 1, maxcpus);
>
> Please avoid personal pronouns in libxl logs (I don't have a max of any
> number of vcpus, the domain does). Here "setting %d vcpus, max vcpus is
> %d" perhaps.
>
> Both libxl__set_vcpuonline_qmp and libxl__set_vcpuonline_xenstore start
> with the same code to get info. Please could you move that (and the
> associated dispose) into libxl_set_vcpuonline and check the limit once
> up front (no need for helper then) and pass the info as a pointer to the
> specific functions.
Would this be OK ?
From a160e64842f36f96f5da610c2498cf1770665dcb Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Date: Fri, 20 Mar 2015 16:29:16 -0400
Subject: [PATCH v3 3/8] libxl: In libxl_set_vcpuonline check for maximum
number of VCPUs against the cpumap.
There is no sense in trying to online (or offline) CPUs when the size of
cpumap is greater than the maximum number of VCPUs the guest can go to.
As such fail the operation if the count of CPUs to online is greater
than what the guest started with. For the offline case we do not
check (as the bits are unset in the cpumap) and let it go through.
We coalesce some of the underlaying libxl_set_vcpuonline code
together to take advantage for the QMP and XenStore ways.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
---
tools/libxl/libxl.c | 75 ++++++++++++++++++++++++++++++++---------------------
1 file changed, 45 insertions(+), 30 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index f957713..c37d057 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -5447,28 +5447,34 @@ int libxl_domain_get_nodeaffinity(libxl_ctx *ctx,
uint32_t domid,
return 0;
}
+static int libxl__check_max(libxl__gc *gc, libxl_dominfo *info,
+ libxl_bitmap *cpumap)
+{
+ int maxcpus = libxl_bitmap_count_set(cpumap);
+
+ if (maxcpus > info->vcpu_max_id + 1)
+ {
+ LOGE(ERROR, "Requested %d VCPUs, however maxcpus is %d!",
+ maxcpus, info->vcpu_max_id + 1);
+ return ERROR_FAIL;
+ }
+ return 0;
+}
+
static int libxl__set_vcpuonline_xenstore(libxl__gc *gc, uint32_t domid,
- libxl_bitmap *cpumap)
+ libxl_bitmap *cpumap,
+ libxl_dominfo *info)
{
- libxl_dominfo info;
char *dompath;
xs_transaction_t t;
- int i, rc;
-
- libxl_dominfo_init(&info);
+ int i, rc = ERROR_FAIL;
- rc = libxl_domain_info(CTX, &info, domid);
- if (rc < 0) {
- LOGE(ERROR, "getting domain info list");
- goto out;
- }
- rc = ERROR_FAIL;
if (!(dompath = libxl__xs_get_dompath(gc, domid)))
goto out;
retry_transaction:
t = xs_transaction_start(CTX->xsh);
- for (i = 0; i <= info.vcpu_max_id; i++)
+ for (i = 0; i <= info->vcpu_max_id; i++)
libxl__xs_write(gc, t,
libxl__sprintf(gc, "%s/cpu/%u/availability", dompath,
i),
"%s", libxl_bitmap_test(cpumap, i) ? "online" :
"offline");
@@ -5478,25 +5484,15 @@ retry_transaction:
} else
rc = 0;
out:
- libxl_dominfo_dispose(&info);
return rc;
}
static int libxl__set_vcpuonline_qmp(libxl__gc *gc, uint32_t domid,
- libxl_bitmap *cpumap)
+ libxl_bitmap *cpumap, libxl_dominfo *info)
{
- libxl_dominfo info;
- int i, rc;
-
- libxl_dominfo_init(&info);
+ int i;
- rc = libxl_domain_info(CTX, &info, domid);
- if (rc < 0) {
- LOGE(ERROR, "getting domain info list");
- libxl_dominfo_dispose(&info);
- return rc;
- }
- for (i = 0; i <= info.vcpu_max_id; i++) {
+ for (i = 0; i <= info->vcpu_max_id; i++) {
if (libxl_bitmap_test(cpumap, i)) {
/* Return value is ignore because it does not tell anything useful
* on the completion of the command.
@@ -5506,7 +5502,6 @@ static int libxl__set_vcpuonline_qmp(libxl__gc *gc,
uint32_t domid,
libxl__qmp_cpu_add(gc, domid, i);
}
}
- libxl_dominfo_dispose(&info);
return 0;
}
@@ -5514,21 +5509,39 @@ int libxl_set_vcpuonline(libxl_ctx *ctx, uint32_t
domid, libxl_bitmap *cpumap)
{
GC_INIT(ctx);
int rc;
- switch (libxl__domain_type(gc, domid)) {
+ libxl_domain_type type;
+ libxl_dominfo info;
+
+ libxl_dominfo_init(&info);
+ type = libxl__domain_type(gc, domid);
+
+ if (type == LIBXL_DOMAIN_TYPE_HVM || type == LIBXL_DOMAIN_TYPE_PV) {
+
+ rc = libxl_domain_info(CTX, &info, domid);
+ if (rc < 0) {
+ LOGE(ERROR, "getting domain info list");
+ goto out;
+ }
+ rc = libxl__check_max(gc, &info, cpumap);
+ if (rc)
+ goto out;
+ }
+
+ switch (type) {
case LIBXL_DOMAIN_TYPE_HVM:
switch (libxl__device_model_version_running(gc, domid)) {
case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
- rc = libxl__set_vcpuonline_xenstore(gc, domid, cpumap);
+ rc = libxl__set_vcpuonline_xenstore(gc, domid, cpumap, &info);
break;
case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
- rc = libxl__set_vcpuonline_qmp(gc, domid, cpumap);
+ rc = libxl__set_vcpuonline_qmp(gc, domid, cpumap, &info);
break;
default:
rc = ERROR_INVAL;
}
break;
case LIBXL_DOMAIN_TYPE_PV:
- rc = libxl__set_vcpuonline_xenstore(gc, domid, cpumap);
+ rc = libxl__set_vcpuonline_xenstore(gc, domid, cpumap, &info);
break;
case LIBXL_DOMAIN_TYPE_NOTFOUND:
rc = ERROR_DOMAIN_NOTFOUND;
@@ -5536,6 +5549,8 @@ int libxl_set_vcpuonline(libxl_ctx *ctx, uint32_t domid,
libxl_bitmap *cpumap)
default:
rc = ERROR_INVAL;
}
+out:
+ libxl_dominfo_dispose(&info);
GC_FREE;
return rc;
}
--
2.1.0
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |