From patchwork Wed Nov 23 15:49:10 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Ferlan X-Patchwork-Id: 83706 Delivered-To: patch@linaro.org Received: by 10.182.1.168 with SMTP id 8csp2786184obn; Wed, 23 Nov 2016 07:52:35 -0800 (PST) X-Received: by 10.28.105.194 with SMTP id z63mr3851098wmh.78.1479916355307; Wed, 23 Nov 2016 07:52:35 -0800 (PST) Return-Path: Received: from mx4-phx2.redhat.com (mx4-phx2.redhat.com. [209.132.183.25]) by mx.google.com with ESMTPS id th3si31773443wjb.57.2016.11.23.07.52.34 (version=TLS1 cipher=AES128-SHA bits=128/128); Wed, 23 Nov 2016 07:52:35 -0800 (PST) Received-SPF: pass (google.com: domain of libvir-list-bounces@redhat.com designates 209.132.183.25 as permitted sender) client-ip=209.132.183.25; Authentication-Results: mx.google.com; spf=pass (google.com: domain of libvir-list-bounces@redhat.com designates 209.132.183.25 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by mx4-phx2.redhat.com (8.13.8/8.13.8) with ESMTP id uANFnC4h013177; Wed, 23 Nov 2016 10:49:12 -0500 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id uANFnBhW031557 for ; Wed, 23 Nov 2016 10:49:11 -0500 Received: from localhost.localdomain (ovpn-116-39.phx2.redhat.com [10.3.116.39]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id uANFnASN001653 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 23 Nov 2016 10:49:11 -0500 To: Chen Hanxiao , libvir-list@redhat.com References: <1478228960-4981-1-git-send-email-chen_han_xiao@126.com> <1478228960-4981-3-git-send-email-chen_han_xiao@126.com> From: John Ferlan Message-ID: Date: Wed, 23 Nov 2016 10:49:10 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: <1478228960-4981-3-git-send-email-chen_han_xiao@126.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-loop: libvir-list@redhat.com Subject: Re: [libvirt] [PATCH 2/4] lxc_driver: use virCheckControllerGoto to simplify cgroup controller check X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com $SUBJ: long line... consider: lxc: Use virCheckControllerGoto Then for all "lxc" files changed, use this patch... On 11/03/2016 11:09 PM, Chen Hanxiao wrote: > From: Chen Hanxiao > > Use macro virCheckControllerGoto > to simplify cgroup controller check codes. > > Signed-off-by: Chen Hanxiao > --- > src/lxc/lxc_driver.c | 115 +++++++++++++++------------------------------------ > 1 file changed, 34 insertions(+), 81 deletions(-) > I adjusted all the calls to put the VIR_CGROUP_CONTROLLER_ on the same line as the virCheckControllerGoto. In some cases, putting all 3 args on the same line was also possible. ACK w/ the attached squashed in to change lxc_process.c for virLXCProcessStart to use virCheckControllerGoto. John > diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c > index 4a0165a..69cdd38 100644 > --- a/src/lxc/lxc_driver.c > +++ b/src/lxc/lxc_driver.c > @@ -847,12 +847,9 @@ lxcDomainSetMemoryParameters(virDomainPtr dom, > if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0) > goto endjob; > > - if (def && > - !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_MEMORY)) { > - virReportError(VIR_ERR_OPERATION_INVALID, "%s", > - _("cgroup memory controller is not mounted")); > - goto endjob; > - } > + if (def) > + virCheckControllerGoto(priv->cgroup, > + VIR_CGROUP_CONTROLLER_MEMORY, endjob); > > #define VIR_GET_LIMIT_PARAMETER(PARAM, VALUE) \ > if ((rc = virTypedParamsGetULLong(params, nparams, PARAM, &VALUE)) < 0) \ > @@ -967,12 +964,9 @@ lxcDomainGetMemoryParameters(virDomainPtr dom, > if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0) > goto cleanup; > > - if (def && > - !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_MEMORY)) { > - virReportError(VIR_ERR_OPERATION_INVALID, > - "%s", _("cgroup memory controller is not mounted")); > - goto cleanup; > - } > + if (def) > + virCheckControllerGoto(priv->cgroup, > + VIR_CGROUP_CONTROLLER_MEMORY, cleanup); > > if ((*nparams) == 0) { > /* Current number of memory parameters supported by cgroups */ > @@ -1863,11 +1857,8 @@ static char *lxcDomainGetSchedulerType(virDomainPtr dom, > goto cleanup; > } > > - if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) { > - virReportError(VIR_ERR_OPERATION_INVALID, > - "%s", _("cgroup CPU controller is not mounted")); > - goto cleanup; > - } > + virCheckControllerGoto(priv->cgroup, > + VIR_CGROUP_CONTROLLER_CPU, cleanup); > > if (nparams) { > if (virCgroupSupportsCpuBW(priv->cgroup)) > @@ -1990,13 +1981,9 @@ lxcDomainSetSchedulerParametersFlags(virDomainPtr dom, > goto endjob; > } > > - if (def) { > - if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) { > - virReportError(VIR_ERR_OPERATION_INVALID, > - "%s", _("cgroup CPU controller is not mounted")); > - goto endjob; > - } > - } > + if (def) > + virCheckControllerGoto(priv->cgroup, > + VIR_CGROUP_CONTROLLER_CPU, endjob); > > for (i = 0; i < nparams; i++) { > virTypedParameterPtr param = ¶ms[i]; > @@ -2128,11 +2115,8 @@ lxcDomainGetSchedulerParametersFlags(virDomainPtr dom, > goto out; > } > > - if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) { > - virReportError(VIR_ERR_OPERATION_INVALID, > - "%s", _("cgroup CPU controller is not mounted")); > - goto cleanup; > - } > + virCheckControllerGoto(priv->cgroup, > + VIR_CGROUP_CONTROLLER_CPU, cleanup); > > if (virCgroupGetCpuShares(priv->cgroup, &shares) < 0) > goto cleanup; > @@ -2388,11 +2372,8 @@ lxcDomainBlockStats(virDomainPtr dom, > goto endjob; > } > > - if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_BLKIO)) { > - virReportError(VIR_ERR_OPERATION_INVALID, "%s", > - _("blkio cgroup isn't mounted")); > - goto endjob; > - } > + virCheckControllerGoto(priv->cgroup, > + VIR_CGROUP_CONTROLLER_BLKIO, endjob); > > if (!*path) { > /* empty path - return entire domain blkstats instead */ > @@ -2474,11 +2455,8 @@ lxcDomainBlockStatsFlags(virDomainPtr dom, > goto endjob; > } > > - if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_BLKIO)) { > - virReportError(VIR_ERR_OPERATION_INVALID, "%s", > - _("blkio cgroup isn't mounted")); > - goto endjob; > - } > + virCheckControllerGoto(priv->cgroup, > + VIR_CGROUP_CONTROLLER_BLKIO, endjob); > > if (!*path) { > /* empty path - return entire domain blkstats instead */ > @@ -2611,13 +2589,9 @@ lxcDomainSetBlkioParameters(virDomainPtr dom, > if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0) > goto endjob; > > - if (def) { > - if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_BLKIO)) { > - virReportError(VIR_ERR_OPERATION_INVALID, "%s", > - _("blkio cgroup isn't mounted")); > - goto endjob; > - } > - } > + if (def) > + virCheckControllerGoto(priv->cgroup, > + VIR_CGROUP_CONTROLLER_BLKIO, endjob); > > ret = 0; > if (def) { > @@ -2818,11 +2792,8 @@ lxcDomainGetBlkioParameters(virDomainPtr dom, > goto cleanup; > > if (def) { > - if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_BLKIO)) { > - virReportError(VIR_ERR_OPERATION_INVALID, "%s", > - _("blkio cgroup isn't mounted")); > - goto cleanup; > - } > + virCheckControllerGoto(priv->cgroup, > + VIR_CGROUP_CONTROLLER_BLKIO, cleanup); > > /* fill blkio weight here */ > if (virCgroupGetBlkioWeight(priv->cgroup, &val) < 0) > @@ -3856,11 +3827,8 @@ lxcDomainAttachDeviceDiskLive(virLXCDriverPtr driver, > goto cleanup; > } > > - if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) { > - virReportError(VIR_ERR_OPERATION_INVALID, "%s", > - _("devices cgroup isn't mounted")); > - goto cleanup; > - } > + virCheckControllerGoto(priv->cgroup, > + VIR_CGROUP_CONTROLLER_DEVICES, cleanup); > > src = virDomainDiskGetSource(def); > if (src == NULL) { > @@ -4408,11 +4376,8 @@ lxcDomainDetachDeviceDiskLive(virDomainObjPtr vm, > if (virAsprintf(&dst, "/dev/%s", def->dst) < 0) > goto cleanup; > > - if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) { > - virReportError(VIR_ERR_OPERATION_INVALID, "%s", > - _("devices cgroup isn't mounted")); > - goto cleanup; > - } > + virCheckControllerGoto(priv->cgroup, > + VIR_CGROUP_CONTROLLER_DEVICES, cleanup); > > if (lxcDomainAttachDeviceUnlink(vm, dst) < 0) { > virDomainAuditDisk(vm, def->src, NULL, "detach", false); > @@ -4527,11 +4492,8 @@ lxcDomainDetachDeviceHostdevUSBLive(virLXCDriverPtr driver, > usbsrc->bus, usbsrc->device) < 0) > goto cleanup; > > - if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) { > - virReportError(VIR_ERR_OPERATION_INVALID, "%s", > - _("devices cgroup isn't mounted")); > - goto cleanup; > - } > + virCheckControllerGoto(priv->cgroup, > + VIR_CGROUP_CONTROLLER_DEVICES, cleanup); > > if (!(usb = virUSBDeviceNew(usbsrc->bus, usbsrc->device, NULL))) > goto cleanup; > @@ -4587,11 +4549,8 @@ lxcDomainDetachDeviceHostdevStorageLive(virDomainObjPtr vm, > goto cleanup; > } > > - if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) { > - virReportError(VIR_ERR_OPERATION_INVALID, "%s", > - _("devices cgroup isn't mounted")); > - goto cleanup; > - } > + virCheckControllerGoto(priv->cgroup, > + VIR_CGROUP_CONTROLLER_DEVICES, cleanup); > > if (lxcDomainAttachDeviceUnlink(vm, def->source.caps.u.storage.block) < 0) { > virDomainAuditHostdev(vm, def, "detach", false); > @@ -4637,11 +4596,8 @@ lxcDomainDetachDeviceHostdevMiscLive(virDomainObjPtr vm, > goto cleanup; > } > > - if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) { > - virReportError(VIR_ERR_OPERATION_INVALID, "%s", > - _("devices cgroup isn't mounted")); > - goto cleanup; > - } > + virCheckControllerGoto(priv->cgroup, > + VIR_CGROUP_CONTROLLER_DEVICES, cleanup); > > if (lxcDomainAttachDeviceUnlink(vm, def->source.caps.u.misc.chardev) < 0) { > virDomainAuditHostdev(vm, def, "detach", false); > @@ -5438,11 +5394,8 @@ lxcDomainGetCPUStats(virDomainPtr dom, > goto cleanup; > } > > - if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUACCT)) { > - virReportError(VIR_ERR_OPERATION_INVALID, > - "%s", _("cgroup CPUACCT controller is not mounted")); > - goto cleanup; > - } > + virCheckControllerGoto(priv->cgroup, > + VIR_CGROUP_CONTROLLER_CPUACCT, cleanup); > > if (start_cpu == -1) > ret = virCgroupGetDomainTotalCpuStats(priv->cgroup, > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list >From 01ee68b76ffa2efd124cbcbe5fb2a5ba060c5095 Mon Sep 17 00:00:00 2001 From: John Ferlan Date: Wed, 23 Nov 2016 10:11:39 -0500 Subject: [PATCH] merge Signed-off-by: John Ferlan --- src/lxc/lxc_process.c | 48 +++++++++++++++++++++++------------------------- 1 file changed, 23 insertions(+), 25 deletions(-) diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index d8727c3..6cbf4c6 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -1170,6 +1170,28 @@ virLXCProcessEnsureRootFS(virDomainObjPtr vm) return -1; } + +static int +virLXCProcessCheckCgroup(void) +{ + int ret = -1; + virCgroupPtr selfcgroup; + + if (virCgroupNewSelf(&selfcgroup) < 0) + return -1; + + virCheckControllerGoto(selfcgroup, VIR_CGROUP_CONTROLLER_CPUACCT, cleanup); + virCheckControllerGoto(selfcgroup, VIR_CGROUP_CONTROLLER_DEVICES, cleanup); + virCheckControllerGoto(selfcgroup, VIR_CGROUP_CONTROLLER_MEMORY, cleanup); + + ret = 0; + + cleanup: + virCgroupFree(&selfcgroup); + return ret; +} + + /** * virLXCProcessStart: * @conn: pointer to connection @@ -1207,37 +1229,13 @@ int virLXCProcessStart(virConnectPtr conn, virCapsPtr caps = NULL; virErrorPtr err = NULL; virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver); - virCgroupPtr selfcgroup; int status; char *pidfile = NULL; bool clearSeclabel = false; bool need_stop = false; - if (virCgroupNewSelf(&selfcgroup) < 0) - return -1; - - if (!virCgroupHasController(selfcgroup, - VIR_CGROUP_CONTROLLER_CPUACCT)) { - virCgroupFree(&selfcgroup); - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Unable to find 'cpuacct' cgroups controller mount")); - return -1; - } - if (!virCgroupHasController(selfcgroup, - VIR_CGROUP_CONTROLLER_DEVICES)) { - virCgroupFree(&selfcgroup); - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Unable to find 'devices' cgroups controller mount")); + if (virLXCProcessCheckCgroup() < 0) return -1; - } - if (!virCgroupHasController(selfcgroup, - VIR_CGROUP_CONTROLLER_MEMORY)) { - virCgroupFree(&selfcgroup); - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Unable to find 'memory' cgroups controller mount")); - return -1; - } - virCgroupFree(&selfcgroup); if (vm->def->nconsoles == 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", -- 2.7.4