LGTM, barring very small nitpicks

On Thu, Jul 24, 2014 at 2:31 AM, Yuto KAWAMURA(kawamuray) <
[email protected]> wrote:

> Existing implementation of GetCgroupMountPoint has a problem because
>

The existing ..


> 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 _MountCgroupSubsystem method to mount cgroup subsystem fs
>

... mount the cgroup ...


>   manually.
> - Introduce _GetOrPrepareCgroupSubsysMountPoint method to check if
>

Introduce the ... if the


>   cgroup subsystem is already mounted, and invoke _MountCgroupSubsystem
>   if not. Return the path of mounted subsystem.
>

.. the mounted ...


> - 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 9aa833a..15548a5 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
> @@ -155,6 +156,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 cgroup subsystem fs under the cgroup root dir.
>

... the cgroup ...


> +
> +    @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 instance cleanup procedure.
>
> @@ -186,17 +214,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,
> @@ -212,7 +256,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,
> @@ -328,7 +372,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