diff mbox

[2/4] lxc_driver: use virCheckControllerGoto to simplify cgroup controller check

Message ID b87ddbf8-b8fc-eee5-d1bc-f7b7492a6bea@redhat.com
State New
Headers show

Commit Message

John Ferlan Nov. 23, 2016, 3:49 p.m. UTC 
$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 <chenhanxiao@gmail.com>

> 

> Use macro virCheckControllerGoto

> to simplify cgroup controller check codes.

> 

> Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>

> ---

>  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 = &params[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
diff mbox

Patch

>From 01ee68b76ffa2efd124cbcbe5fb2a5ba060c5095 Mon Sep 17 00:00:00 2001
From: John Ferlan <jferlan@redhat.com>
Date: Wed, 23 Nov 2016 10:11:39 -0500
Subject: [PATCH] merge

Signed-off-by: John Ferlan <jferlan@redhat.com>
---
 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