On Thu, Jul 24, 2014 at 2:31 AM, Yuto KAWAMURA(kawamuray) < [email protected]> wrote:
> - Add _PrepareInstanceRootFsBdev method which will create a partition > Add the ... > mapping for block device and returns rootfs partition path. > ... for a block ... and return the ... > - Override CleanupInstance to perform cleanup on instance destruction. > - Introduce _CleanupInstance method which is the actual implementation > Introduce the ... > of the cleanup process with 'stash' argument. > ... with a 'stash' ... > - Create 'stash' data store in StartInstance, and ensure cleanup is > Create the instance stash ... > performed on exit or on error. > > Signed-off-by: Yuto KAWAMURA(kawamuray) <[email protected]> > --- > lib/hypervisor/hv_lxc.py | 100 > ++++++++++++++++++++++++++++++++++++++++------- > 1 file changed, 86 insertions(+), 14 deletions(-) > > diff --git a/lib/hypervisor/hv_lxc.py b/lib/hypervisor/hv_lxc.py > index c33ed14..dfe094b 100644 > --- a/lib/hypervisor/hv_lxc.py > +++ b/lib/hypervisor/hv_lxc.py > @@ -26,6 +26,7 @@ > import os > import os.path > import logging > +import sys > > from ganeti import constants > from ganeti import errors # pylint: disable=W0611 > @@ -153,6 +154,35 @@ class LXCHypervisor(hv_base.BaseHypervisor): > raise HypervisorError("Failed to load instance stash file %s : %s" % > (stash_file, err)) > > + def _CleanupInstance(self, instance_name, stash): > + """Actual implementation of instance cleanup procedure. > of the > + > + @type instance_name: string > + @param instance_name: instance name > + @type stash: dict(string:any) > + @param stash: dict that contains desired informations for instance > cleanup > s/informations/information/ > + > + """ > + try: > + if "loopback-device" in stash: > Make this into a class constant, please. > + utils.ReleaseBdevPartitionMapping(stash["loopback-device"]) > + except errors.CommandError, err: > + raise HypervisorError("Failed to cleanup partition mapping : %s" % > err) > + > + utils.RemoveFile(self._InstanceStashFilePath(instance_name)) > + > + def CleanupInstance(self, instance_name): > + """Cleanup after a stopped instance. > + > + """ > + try: > + stash = self._LoadInstanceStash(instance_name) > + except HypervisorError, err: > + logging.warn("%s", err) > + stash = {} > Is there a legitimate use case for this not to be handled as an error? After the upgrade and all the changes of this patch series, it is utterly inconceivable that an already existing LXC instance could survive and still run, the missing stash file aside. And you might be hiding a whole class of errors here - incompatible data format, stash file corruption, etc. > + > + self._CleanupInstance(instance_name, stash) > + > @classmethod > def _GetCgroupMountPoint(cls): > for _, mountpoint, fstype, _ in utils.GetMounts(): > @@ -329,6 +359,29 @@ class LXCHypervisor(hv_base.BaseHypervisor): > > return "\n".join(out) + "\n" > > + @classmethod > + def _PrepareInstanceRootFsBdev(cls, storage_path, stash): > + """Return mountable path for storage_path. > + > + This function creates partition mapping for storage_path and return > the > ... creates a ... and returns the ... > + first partition device path as a rootfs partition, and store allocated > s/store/stores the/ > + loopback device path to stash. > s/to/in the stash/ Alternatively: stashes the loopback device path > + If storage_path is not a multi-partition block device, just returns > s/returns/return/ > + storage_path. > + > + """ > + try: > + ret = utils.CreateBdevPartitionMapping(storage_path) > + except errors.CommandError, err: > + raise HypervisorError("Failed to create partition mapping for %s" > + ": %s" % (storage_path, err)) > A blank line here, please. > + if ret is None: > + return storage_path > + else: > + loop_dev_path, dm_dev_paths = ret > + stash["loopback-device"] = loop_dev_path > + return dm_dev_paths[0] > + > def StartInstance(self, instance, block_devices, startup_paused): > """Start an instance. > > @@ -336,6 +389,7 @@ class LXCHypervisor(hv_base.BaseHypervisor): > We use volatile containers. > > """ > + stash = {} > root_dir = self._InstanceDir(instance.name) > try: > utils.EnsureDirs([(root_dir, self._DIR_MODE)]) > @@ -351,21 +405,39 @@ class LXCHypervisor(hv_base.BaseHypervisor): > " instance %s failed: %s" % > (log_file, instance.name, err)) > > - if not block_devices: > - raise HypervisorError("LXC needs at least one disk") > - > - sda_dev_path = block_devices[0][1] > - conf_file = self._InstanceConfFile(instance.name) > - conf = self._CreateConfigFile(instance, sda_dev_path) > - utils.WriteFile(conf_file, data=conf) > + try: > + if not block_devices: > + raise HypervisorError("LXC needs at least one disk") > + > + sda_dev_path = block_devices[0][1] > + # LXC needs to use partition mapping devices to access each > partition > + # of the storage > + sda_dev_path = self._PrepareInstanceRootFsBdev(sda_dev_path, stash) > + conf_file = self._InstanceConfFile(instance.name) > + conf = self._CreateConfigFile(instance, sda_dev_path) > + utils.WriteFile(conf_file, data=conf) > + > + logging.info("Running lxc-start") > + result = utils.RunCmd(["lxc-start", > + "-n", instance.name, > + "-o", log_file, > + "-l", "DEBUG", > + "-f", conf_file, > + "-d"]) > + if result.failed: > + raise HypervisorError("Running the lxc-start failed: %s" % > + result.output) > + except: > + # Save an original error > s/an/the/ > + exc_info = sys.exc_info() > + try: > + self._CleanupInstance(instance.name, stash) > + except HypervisorError, err: > + logging.warn("Cleanup for instance %s incomplete: %s", > + instance.name, err) > + raise exc_info[0], exc_info[1], exc_info[2] > > - result = utils.RunCmd(["lxc-start", "-n", instance.name, > - "-o", log_file, > - "-l", "DEBUG", > - "-f", conf_file, "-d"]) > - if result.failed: > - raise HypervisorError("Running the lxc-start script failed: %s" % > - result.output) > + self._SaveInstanceStash(instance.name, stash) > def StopInstance(self, instance, force=False, retry=False, name=None, > timeout=None): > -- > 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
