LGTM, thanks

On Wed, Jul 30, 2014 at 6:36 PM, Yuto KAWAMURA(kawamuray) <
[email protected]> wrote:

> The existing implementation of GetCgroupMountPoint has a problem because
> there is no guarantee that the first element of /proc/mounts which has
> fstype == cgroup contains all cgroup subsystems.
> Changes made by this commit are:
> - Introduce the _MountCgroupSubsystem method to mount the cgroup
>   subsystem fs manually.
> - Introduce the _GetOrPrepareCgroupSubsysMountPoint method to check if
>   the cgroup subsystem is already mounted, and invoke
>   _MountCgroupSubsystem if not. Return the path of the mounted
>   subsystem.
> - Replace invokation of GetCgroupMountPoint by
>   _GetOrPrepareCgroupSubsysMountPoint.
>
> Signed-off-by: Yuto KAWAMURA(kawamuray) <[email protected]>
> ---
>  lib/hypervisor/hv_lxc.py | 58
> ++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 51 insertions(+), 7 deletions(-)
>
> diff --git a/lib/hypervisor/hv_lxc.py b/lib/hypervisor/hv_lxc.py
> index 0697cb0..963659c 100644
> --- a/lib/hypervisor/hv_lxc.py
> +++ b/lib/hypervisor/hv_lxc.py
> @@ -54,6 +54,7 @@ class LXCHypervisor(hv_base.BaseHypervisor):
>
>    """
>    _ROOT_DIR = pathutils.RUN_DIR + "/lxc"
> +  _CGROUP_ROOT_DIR = _ROOT_DIR + "/cgroup"
>    _DEVS = [
>      "c 1:3",   # /dev/null
>      "c 1:5",   # /dev/zero
> @@ -157,6 +158,33 @@ class LXCHypervisor(hv_base.BaseHypervisor):
>        raise HypervisorError("Failed to load instance stash file %s : %s" %
>                              (stash_file, err))
>
> +  @classmethod
> +  def _MountCgroupSubsystem(cls, subsystem):
> +    """Mount the cgroup subsystem fs under the cgroup root dir.
> +
> +    @type subsystem: string
> +    @param subsystem: cgroup subsystem name to mount
> +    @rtype string
> +    @return path of subsystem mount point
> +
> +    """
> +    subsys_dir = utils.PathJoin(cls._GetCgroupMountPoint(), subsystem)
> +    if not os.path.isdir(subsys_dir):
> +      try:
> +        os.makedirs(subsys_dir)
> +      except EnvironmentError, err:
> +        raise HypervisorError("Failed to create directory %s: %s" %
> +                              (subsys_dir, err))
> +
> +    mount_cmd = ["mount", "-t", "cgroup", "-o", subsystem, subsystem,
> +                 subsys_dir]
> +    result = utils.RunCmd(mount_cmd)
> +    if result.failed:
> +      raise HypervisorError("Failed to mount cgroup subsystem '%s': %s" %
> +                            (subsystem, result.output))
> +
> +    return subsys_dir
> +
>    def _CleanupInstance(self, instance_name, stash):
>      """Actual implementation of the instance cleanup procedure.
>
> @@ -184,17 +212,33 @@ class LXCHypervisor(hv_base.BaseHypervisor):
>
>    @classmethod
>    def _GetCgroupMountPoint(cls):
> -    for _, mountpoint, fstype, _ in utils.GetMounts():
> -      if fstype == "cgroup":
> -        return mountpoint
> -    raise errors.HypervisorError("The cgroup filesystem is not mounted")
> +    """Return the directory that should be the base of cgroup fs.
> +
> +    """
> +    return cls._CGROUP_ROOT_DIR
> +
> +  @classmethod
> +  def _GetOrPrepareCgroupSubsysMountPoint(cls, subsystem):
> +    """Prepare cgroup subsystem mount point.
> +
> +    @type subsystem: string
> +    @param subsystem: cgroup subsystem name to mount
> +    @rtype string
> +    @return path of subsystem mount point
> +
> +    """
> +    for _, mpoint, fstype, options in utils.GetMounts():
> +      if fstype == "cgroup" and subsystem in options.split(","):
> +        return mpoint
> +
> +    return cls._MountCgroupSubsystem(subsystem)
>
>    @classmethod
>    def _GetCgroupCpuList(cls, instance_name):
>      """Return the list of CPU ids for an instance.
>
>      """
> -    cgroup = cls._GetCgroupMountPoint()
> +    cgroup = cls._GetOrPrepareCgroupSubsysMountPoint("cpuset")
>      try:
>        cpus = utils.ReadFile(utils.PathJoin(cgroup, 'lxc',
>                                             instance_name,
> @@ -210,7 +254,7 @@ class LXCHypervisor(hv_base.BaseHypervisor):
>      """Return the memory limit for an instance
>
>      """
> -    cgroup = cls._GetCgroupMountPoint()
> +    cgroup = cls._GetOrPrepareCgroupSubsysMountPoint("memory")
>      try:
>        memory = int(utils.ReadFile(utils.PathJoin(cgroup, 'lxc',
>                                                   instance_name,
> @@ -325,7 +369,7 @@ class LXCHypervisor(hv_base.BaseHypervisor):
>
>      # Memory
>      # Conditionally enable, memory resource controller might be disabled
> -    cgroup = self._GetCgroupMountPoint()
> +    cgroup = self._GetOrPrepareCgroupSubsysMountPoint("memory")
>      if os.path.exists(utils.PathJoin(cgroup, "memory.limit_in_bytes")):
>        out.append("lxc.cgroup.memory.limit_in_bytes = %dM" %
>                   instance.beparams[constants.BE_MAXMEM])
> --
> 1.8.5.5
>
>


Hrvoje Ribicic
Ganeti Engineering
Google Germany GmbH
Dienerstr. 12, 80331, München

Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Christine Elizabeth Flores
Steuernummer: 48/725/00206
Umsatzsteueridentifikationsnummer: DE813741370

Reply via email to