Re: [PATCH] Add tmpfs support (#918621)

[Vratislav Podzimek]

On Thu, 2013-09-26 at 18:56 +0200, Martin Kolman wrote:
> Add support for creating tmpfs mounts, mounting them
> and adding them to fstab.
>
> Tmpfs is a filesystem that lives in the Kernel page cache
> and stores data primarily in RAM and in swap once RAM runs out.
>
> Data stored on tmpfs mounts will not survive a system
> reboot/crash/shutdown.
>
> About tmpfs mount size:
> if no size is specified, the size will be 50% by default
> if size is specified, the mount will have this size
>
> -> there is no limit on the size on tmpfs mounts
> -> just note that once it fills RAM (and any swap),
> the system will grind to a halt
> grow and maxsize are supported
>
> -> just using grow without maxsize will set the size to 100% RAM
> -> if maxsize is > RAM, the size will be 100% RAM
> -> if maxize is < RAM, the size will be a corresponding percentage of RAM
> Signed-off-by: Martin Kolman
>
> blivet/__init__.py     |   4 ++
>  blivet/deviceaction.py |   7 ++-
>  blivet/devices.py      |  24 ++++++++++
>  blivet/formats/fs.py   | 125 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  4 files changed, 157 insertions(+), 3 deletions(-)
>
> diff --git a/blivet/__init__.py b/blivet/__init__.py
> index d15a0d0..afd5a49 100644
> --- a/blivet/__init__.py
> +++ b/blivet/__init__.py
> @@ -1192,6 +1192,10 @@ class Blivet(object):
>          kwargs["subvol"] = True
>          return self.newBTRFS(*args, **kwargs)
> def newTmpFS(self, *args, **kwargs):
>    """ Return a new TmpFSDevice. """
>    return TmpFSDevice(*args, **kwargs)
> def createDevice(self, device):
>        """ Schedule creation of a device.
>
> diff --git a/blivet/deviceaction.py b/blivet/deviceaction.py
> index 202a66c..7160cce 100644
> --- a/blivet/deviceaction.py
> +++ b/blivet/deviceaction.py
> @@ -473,8 +473,11 @@ class ActionCreateFormat(DeviceAction):
>              udev_settle()
>              self.device.updateSysfsPath()
>              info = udev_get_block_device(self.device.sysfsPath)
>        self.device.format.uuid = udev_device_get_uuid(info)
>        self.device.deviceLinks = udev_device_get_symlinks(info)
>        # only do this if the format has a device known to udev
>        # (the format might not have a normal device at all)
>        if info:
>            self.device.format.uuid = udev_device_get_uuid(info)
>            self.device.deviceLinks = udev_device_get_symlinks(info)
>
> def cancel(self):
>        self.device.format = self.origFormat
>
> diff --git a/blivet/devices.py b/blivet/devices.py
> index 6650086..c278433 100644
> --- a/blivet/devices.py
> +++ b/blivet/devices.py
> @@ -3760,6 +3760,30 @@ class NoDevice(StorageDevice):
>          self._preDestroy()
>
> +class TmpFSDevice(NoDevice):
> def __init__(self, *args, **kwargs):
>    """Create a tmpfs device"""
>    format = kwargs.get('format')
>    NoDevice.__init__(self, format)
>    # the tmpfs device does not exist until mounted
>    self.exists = False
>    self._size = kwargs["size"]
>    self._targetSize = self._size
> @property
> def size(self):
>    if self._size is not None:
>        return self._size
>    elif self.format:
>        return self.format.size
>    else:
>        return None
> @property
> def fstabSpec(self):
>    return "tmpfs"
>
> class FileDevice(StorageDevice):
>      """ A file on a filesystem.
>
> diff --git a/blivet/formats/fs.py b/blivet/formats/fs.py
> index 13896c9..f7e1da8 100644
> --- a/blivet/formats/fs.py
> +++ b/blivet/formats/fs.py
> @@ -1406,7 +1406,7 @@ class NoDevFS(FS):
>      def __init__(self, *args, **kwargs):
>          FS.__init__(self, *args, **kwargs)
>          self.exists = True
>    self.device = self.type
>    self.device = self._type
>
> def _setDevice(self, devspec):
>        self._device = devspec
>
> @@ -1451,6 +1451,129 @@ register_device_format(SysFS)
>
> class TmpFS(NoDevFS):
>      _type = "tmpfs"
> _supported = True
> _resizable = True
> # as tmpfs is part of the Linux kernel,
> # I guess it is linux-native
> _linuxNative = True
> # empty tmpfs has zero overhead
> _minSize = 0
> _minInstanceSize = 0
> # tmpfs really does not occupy any space by itself
> _minSize = 0
> # in a sense, I guess tmpfs is formatable

^^^ double 't'
> # in the regard that the format is atuomatically created

^^^ typo
> # once mounted
> _formattable = True
> def __init__(self, *args, **kwargs):
>    NoDevFS.__init__(self, *args, **kwargs)
>    self.exists = True
>    self._device = "tmpfs"
>    # check if fixed filesystem size has been specified,
>    # if no size is specified, tmpfs will by default
>    # be limited to half of the system RAM
>    # (sizes of all tmpfs mounts are independent)
>    fsoptions = kwargs.get("mountopts")
>    grow = kwargs.get("grow")
>    maxsize = kwargs.get("maxsize")
>    size_option = ""
>    if fsoptions:
>        self._apendOptions(fsoptions)

^^^ should be appendOptions with double 'p'
>    if self._size:
>        self._apendOptions("size=%dm" % self._size)
>    elif grow:
>        # grow to 100% RAM
>        size_option = "size=100%%"
>        system_ram = util.total_memory()/1024  # kB to MB

It's better to use spaces around operators (applies to a few more
places).
>        fs_size = system_ram
>        if maxsize:
>            # check if maxsize is < RAM
>            if maxsize < system_ram:
>                # as maxsize is smaller than RAM, filesystem size = maxsize
>                fs_size = maxsize
>                # maxsize is less than size of RAM
>                # -> convert to % of RAM
>                size_fraction = maxsize/float(system_ram)
>                # "size=%1.0f%%" % (0.5555*100) -> "size=56%"
>                size_option = "size=%1.0f%%" % (size_fraction*100)
>        self._size = fs_size
>        kwargs["size"] = fs_size
>    if size_option:
>        self._apendOptions(size_option)
> def mount(self, *args, **kwargs):
>    """The filesystem doesn't need to be separately mounted"""
>    pass
> def create(self, *arg, **kwargs):
>    """A filesystem is created automatically once tmpfs is mounted"""
>    pass
> @property
> def mountable(self):
>    return True
> def _getOptions(self):
>    if self._options:
>        return self._options
>    else:
>        return "defaults"
> def _setOptions(self, options):
>    self._options = options
> # override the options property
> # so that the size and other options
> # are correctly added to fstab
> options = property(_getOptions, _setOptions)
> @property
> def size(self):
>    return self._size
> @property
> def minSize(self):
>    """ The minimum filesystem size in megabytes. """
>    return self._minInstanceSize
> @property
> def free(self):
>    free_space = 0
>    if self.mountpoint:
>        # we need both the output and the return code
>        rc, buf = util._run_program(['df', '-P', self.mountpoint])
>        # XXXX, what about changeroot ?
>        if rc == 0:
>            lines = buf.readlines()
>            if len(lines) == 2:  # header and a single data row
>                space_data = lines[0].split(" ")
>                if len(space_data) >= 6:
>                    free_space = int(space_data[3])

Comment describing the format of the 'df -P' output may help here.

Apart from those nitpicking comments this looks good and surprisingly
clean to me.