One general comment about the patch: Given that we are referencing the structure expected by LXC utilities, it would be good to have at least some link or explanation of what that structure is in the code.
On Mon, Sep 29, 2014 at 7:26 PM, Yuto KAWAMURA(kawamuray) < [email protected]> wrote: > Since all lxc commands supports --lxcpath option which is used to specify > s/supports/support/ the --lxcpath option > a lxc container definitions directory, we should follow the directory > structure which is expected by lxc utilities. > s/which is// > Currently we have no problem with existing directory structure, but once > s/no problem/no issues/ with the existing > we try to introduce migration feature by using lxc-checkpoint command, > s/feature// the lxc-checkpoint command > this will be a problem. > Also it would be a good idea to create a hierarchy for the instances directory instead of using the top-level of hypervisor local directory. > As this patch does this rather than proposing to do it, it should be noted in the description. This patch also creates a ... ...the hypervisor's local directory. > > Signed-off-by: Yuto KAWAMURA(kawamuray) <[email protected]> > --- > lib/hypervisor/hv_lxc.py | 32 ++++++++++++++++++-------------- > 1 file changed, 18 insertions(+), 14 deletions(-) > > diff --git a/lib/hypervisor/hv_lxc.py b/lib/hypervisor/hv_lxc.py > index 475ba18..d003748 100644 > --- a/lib/hypervisor/hv_lxc.py > +++ b/lib/hypervisor/hv_lxc.py > @@ -55,6 +55,7 @@ class LXCHypervisor(hv_base.BaseHypervisor): > """ > _ROOT_DIR = pathutils.RUN_DIR + "/lxc" > _LOG_DIR = pathutils.LOG_DIR + "/lxc" > + _INSTANCE_DIR = _ROOT_DIR + "/instance" > _CGROUP_ROOT_DIR = _ROOT_DIR + "/cgroup" > _PROC_CGROUPS_FILE = "/proc/cgroups" > _PROC_SELF_CGROUP_FILE = "/proc/self/cgroup" > @@ -69,7 +70,6 @@ class LXCHypervisor(hv_base.BaseHypervisor): > ] > > _DIR_MODE = 0755 > - _UNIQ_SUFFIX = ".conf" > _STASH_KEY_ALLOCATED_LOOP_DEV = "allocated_loopdev" > > PARAMETERS = { > @@ -101,14 +101,14 @@ class LXCHypervisor(hv_base.BaseHypervisor): > """Return the root directory for an instance. > > """ > - return utils.PathJoin(cls._ROOT_DIR, instance_name) > + return utils.PathJoin(cls._INSTANCE_DIR, instance_name) > > @classmethod > def _InstanceConfFilePath(cls, instance_name): > """Return the configuration file for an instance. > > """ > - return utils.PathJoin(cls._ROOT_DIR, instance_name + ".conf") > + return utils.PathJoin(cls._InstanceDir(instance_name), "config") > > @classmethod > def _InstanceLogFilePath(cls, instance): > @@ -121,6 +121,13 @@ class LXCHypervisor(hv_base.BaseHypervisor): > return utils.PathJoin(cls._LOG_DIR, filename) > > @classmethod > + def _InstanceConsoleLogFilePath(cls, instance_name): > + """Return the console log file path for an instance. > + > + """ > + return utils.PathJoin(cls._InstanceDir(instance_name), "console.log") > + > + @classmethod > def _InstanceStashFilePath(cls, instance_name): > """Return the stash file path for an instance. > > @@ -128,7 +135,7 @@ class LXCHypervisor(hv_base.BaseHypervisor): > destruction of the instance. > > """ > - return utils.PathJoin(cls._ROOT_DIR, instance_name + ".stash") > + return utils.PathJoin(cls._InstanceDir(instance_name), "stash") > > def _EnsureDirectoryExistence(self): > """Ensures all the directories needed for LXC use exist. > @@ -137,6 +144,7 @@ class LXCHypervisor(hv_base.BaseHypervisor): > utils.EnsureDirs([ > (self._ROOT_DIR, self._DIR_MODE), > (self._LOG_DIR, 0750), > + (self._INSTANCE_DIR, 0750), > ]) > > @classmethod > @@ -436,14 +444,10 @@ class LXCHypervisor(hv_base.BaseHypervisor): > > """ > data = [] > - for filename in os.listdir(self._ROOT_DIR): > - if not filename.endswith(self._UNIQ_SUFFIX): > - # listing all files in root directory will include instance root > - # directories, console files, etc, so use .conf as a filter of > instance > - # listings. > - continue > + filter_fn = lambda x: > os.path.isdir(utils.PathJoin(self._INSTANCE_DIR, x)) > + for dirname in filter(filter_fn, os.listdir(self._INSTANCE_DIR)): > try: > - info = self.GetInstanceInfo(filename[0:-len(self._UNIQ_SUFFIX)]) > + info = self.GetInstanceInfo(dirname) > except errors.HypervisorError: > continue > if info: > @@ -478,9 +482,9 @@ class LXCHypervisor(hv_base.BaseHypervisor): > if lxc_ttys: # if it is the number greater than 0 > out.append("lxc.tty = %s" % lxc_ttys) > # console log file > - console_log = utils.PathJoin(self._ROOT_DIR, instance.name + > ".console") > - self._CreateBlankFile(console_log, constants.SECURE_FILE_MODE) > - out.append("lxc.console = %s" % console_log) > + console_log_path = self._InstanceConsoleLogFilePath(instance.name) > + self._CreateBlankFile(console_log_path, constants.SECURE_FILE_MODE) > + out.append("lxc.console = %s" % console_log_path) > > # root FS > out.append("lxc.rootfs = %s" % sda_dev_path) > -- > 1.8.5.5 > >
