[PATCH] Add tmpfs support (#918621)

David Lehman dlehman at redhat.com
Fri Sep 27 14:12:36 UTC 2013


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.

This looks good overall. I have several comments/questions. It's worth
noting that this is going to be the first nodev filesystem that anaconda
supports direct configuration of. That will present some challenges --
particularly in the custom spoke.

> 
> 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

50% of what?

> - 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

When I read this the first time I thought you meant maxsize would be
treated as a percentage -- not that you have to convert it to a
percentage to specify for the mount options.

> 
> Signed-off-by: Martin Kolman <mkolman at redhat.com>
> ---
>  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):

You could safely set _type to "tmpfs" here so that .type is meaningful.

> +    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

You've got minSize in here twice :)

> +    # in a sense, I guess tmpfs is formatable
> +    # in the regard that the format is atuomatically created
> +    # 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)
> +        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
> +            fs_size = system_ram
> +            if maxsize:
> +                # check if maxsize is < RAM
> +                if maxsize < system_ram:

Not necessary, but you could combine the two if statements above.

> +                    # 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

What the above line for?

> +
> +        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

So we don't do anything with tmpfs mounts except put them in /etc/fstab?
This may be fine when flags.installer_mode is True, but for blivet it
will have to be possible to mount and unmount tmpfs as well. My advice
would be to stub create and destroy and just let mount/umount do the
work.

> +
> +    @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:

I think you want self._mountpoint throughout this method -- it holds the
current/active mountpoint and provides an implicit status check since it
is only set when the filesystem is actually mounted somewhere.

> +            # 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])
> +                        # convert to MB
> +                        free_space = math.ceil(size / 1024.0)
> +        return free_space
> +
> +    def _setDevice(self, devspec):
> +        self._device = devspec
> +
> +    def _getDevice(self):
> +        return self._device
> +
> +    device = property(lambda f: f._getDevice(),
> +                      lambda f,d: f._setDevice(d),
> +                      doc="Full path the device this format occupies")

Why not just use a plain attribute for self.device if the get/set are as
above? It seems like we might want to use a read-only property here,
actually.

> +
> +    def _apendOptions(self, new_options):
> +        """Append free form option string
> +        to the options (used among others in fstab)
> +        """
> +        if self._options:  # append to existing options
> +            self._options = "%s,%s" % (self._options, new_options)
> +        else:  # not yet set, just assign it
> +            self._options = new_options
>  
>  register_device_format(TmpFS)
>  




More information about the anaconda-patches mailing list