[blivet][rhel7-branch][PATCH] Add tmpfs support (#918621)

[Martin Kolman]

This patch adds support for working with tmpfs "mounts" (well they are not
really devices and don't really have a separate filesystem).

Adding lines to fstab works fine (being the main point of the feature request)
and other operations, such as mounting the filesystem, also work.

I've been even able to start an installation to a tmpfs mount, with:
part / --size=3000 --fstype=tmpfs
And it got to about 3/4 before crashing the VM. :)

BTW, regarding the feature request:
* in Fedora, the tmps mount to /tmp is done by a systemd unit
* this unit is also present in RHEL7, but it is disabled
* there doesn't seem to be any straightforward way to set a size of such mount
Therefore I've decided to handle this by extending blivet and the part kickstart
command to support tmpfs, because it appears to be much more flexible:
* you can easily set the size of the tmpfs mount with --size
* you can use --grow and --maxsize
* tmpfs options can be passed with --fsoptions
* an arbitrary number of tmpfs partitions can be created, not just /tmp

Martin Kolman (1):
  Add tmpfs support (#918621)

blivet/__init__.py     |   4 ++
 blivet/deviceaction.py |   7 ++-
 blivet/devices.py      |  24 ++++++++++
 blivet/formats/fs.py   | 125 ++++++++++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 157 insertions(+), 3 deletions(-)

[PATCH] Add tmpfs support (#918621)

[Martin Kolman]

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
+    # 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:
+                    # 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])
+                        # 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")
+
+    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)

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

[PATCH] Add tmpfs support (#918621)

[David Lehman]

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

[PATCH] Add tmpfs support (#918621)

[Martin Kolman]

On Fri, 2013-09-27 at 09:12 -0500, David Lehman wrote:
> 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.

Yeah, that might indeed by an issue - for example partition size.
Theoretically, there is no limit the size of a tmps partition.
That could do nasty things with the displayed total space/available
space - I guess it probably should not be counted into either ?

Also setting the size of the tmpfs mount could be an issue, as it can be
done both as an absolute value and as a % of RAM (more about this
further down).
> > 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?

Oh, that should be "50% of system RAM 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
>
> 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.

Well, yeah, this might be kinda confusing. :)
Ill try to summarize all the ways that can be used to set the size of a
tmpfs mount:

* not specifying the size - size will correspond to 50% of system RAM
Kickstart example:
part /tmp --fstype=tmpfs

* setting a fixed size in megabytes - the tmpfs mount will have a fixed
size corresponding to the given number of megabytes.
Kickstart example:
part /tmp --size=3000 --fstype=tmpfs

* setting directly setting the percentage with --fsoptions - the tmpf
mount will have a size in the % of system RAM given. 
Kickstart example:
part /tmp --fstype=tmpfs --fsoptions="size=25%"
-> this tmpfs mount will have a size of 25% of the system RAM

* using the --grow option without --maxsize - the tmpfs mount size will
correspond to 100% of system RAM
Kickstart example:
part /tmp --fstype=tmpfs --grow
BTW, the end result is the same as using:
part /tmp --fstype=tmpfs --fsoptions="size=100%"

* using the --grow option with --maxsize - it gets a bit more
complicated:
- if maxsize > system RAM - the size of the tmpfs mount will be 100% RAM
- if maxsize < system RAM - the size of the tmpfs mount will be <100%
RAM
- as long as --grow is used, the size is always specified in %
- also --grow always grows only up to 100% od system RAM, never above,
even if the number given by --maxsize is larger than system RAM
Kickstart example for a system with 1GB RAM
part /tmp --fstype=tmpfs --maxsize=3000 --grow
-> the size of the tmpfs mount will be 100% RAM
part /tmp --fstype=tmpfs --maxsize=512 --grow
-> the size of the tmpfs mount will be 50% RAM

This is how the size works, and it has some consequences:
* there are some "undefined" combinations for the kickstart command,
such as for example:
part /tmp --fstype=tmpfs --fsoptions="size=25%" --size=300
part /tmp --fstype=tmpfs --fsoptions="size=25%" --grow
part /tmp --fstype=tmpfs --fsoptions="size=25%" --grow --maxisze=300
part /tmp --fstype=tmpfs --size=300 --grow
part /tmp --fstype=tmpfs --size=300 --grow --maxsize=500
part /tmp --fstype=tmpfs --size=300 --grow --maxsize=500
-fsoptions="size=25%"

The question is, how to handle these combinations ? Just document
the correct usage and leave the behavior undefined or throw a kickstart
parsing error if such combination is found in the kickstart file (as has
been recently done for combining autopart and other partitioning
commands)?

* --size is the only option that can set the size of the tmpfs mount
to an amount larger than the size of the system RAM

* also the --size option is the only one it can set the size to an
absolute value, all the other options (and the 50% default) all work
with % sizes

* if the size is specified in percent and RAM is added/removed, the size
of the tmpfs mounts automatically grows/shrinks accordingly
> > 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):
>
> You could safely set _type to "tmpfs" here so that .type is meaningful.

OK.
> > 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 :)

Oops. :)
> > # 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.

Oh, now I see it - good point.
> >                # 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 the tmpfs --size kickstart option is not used, the "size" is never
set kwargs, which breaks stuff such as checking the size of the
filesystem. 
So in this case the % is converted to a number (based on the current
system RAM) and set in kwargs.
Now when I think about this, this should be also done in case no
explicit size is specified and the 50%-of-system-RAM default is used.
> >    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?

Yes, at least for the bugreport doing this should be enough.
> 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.

OK, I'll do that.
> > @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.

OK, good to know. :)
> >        # 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.

Well, yeah, that makes sense - it would always be "tmpfs", for all tmpfs
mounts so a read-only property should do just fine.
> > 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)
>
> anaconda-patches mailing list
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches

[PATCH] Add tmpfs support (#918621)

[David Lehman]

On Wed, 2013-10-02 at 16:33 +0200, Martin Kolman wrote:
> On Fri, 2013-09-27 at 09:12 -0500, David Lehman wrote:
> > 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.
>
> Yeah, that might indeed by an issue - for example partition size.
> Theoretically, there is no limit the size of a tmps partition.
> That could do nasty things with the displayed total space/available
> space - I guess it probably should not be counted into either ?

Tmpfs mountpoints should not have any effect on the total/free space in
custom. That makes me wonder, though -- do your patches handle the case
where the user tries to create multiple tmpfs mountpoints with a total
of >100% ? Also, surely specifying 100% of RAM for tmpfs size is madness
-- no?
> Also setting the size of the tmpfs mount could be an issue, as it can be
> done both as an absolute value and as a % of RAM (more about this
> further down).

I would consider making it so that for tmpfs the size is always treated
as a percentage.
> > > 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?
>
> Oh, that should be "50% of system RAM 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
> >
> > 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.
>
> Well, yeah, this might be kinda confusing. :)
> Ill try to summarize all the ways that can be used to set the size of a
> tmpfs mount:
> not specifying the size - size will correspond to 50% of system RAM
>
> Kickstart example:
> part /tmp --fstype=tmpfs
> setting a fixed size in megabytes - the tmpfs mount will have a fixed
>
> size corresponding to the given number of megabytes.
> Kickstart example:
> part /tmp --size=3000 --fstype=tmpfs
> setting directly setting the percentage with --fsoptions - the tmpf
>
> mount will have a size in the % of system RAM given. 
> Kickstart example:
> part /tmp --fstype=tmpfs --fsoptions="size=25%"
> -> this tmpfs mount will have a size of 25% of the system RAM
> using the --grow option without --maxsize - the tmpfs mount size will
>
> correspond to 100% of system RAM
> Kickstart example:
> part /tmp --fstype=tmpfs --grow
> BTW, the end result is the same as using:
> part /tmp --fstype=tmpfs --fsoptions="size=100%"
> using the --grow option with --maxsize - it gets a bit more
>
> complicated:
> if maxsize > system RAM - the size of the tmpfs mount will be 100% RAM
> if maxsize < system RAM - the size of the tmpfs mount will be <100%
>
> RAM
> as long as --grow is used, the size is always specified in %
> also --grow always grows only up to 100% od system RAM, never above,
>
> even if the number given by --maxsize is larger than system RAM
> Kickstart example for a system with 1GB RAM
> part /tmp --fstype=tmpfs --maxsize=3000 --grow
> -> the size of the tmpfs mount will be 100% RAM
> part /tmp --fstype=tmpfs --maxsize=512 --grow
> -> the size of the tmpfs mount will be 50% RAM
>
> This is how the size works, and it has some consequences:
> there are some "undefined" combinations for the kickstart command,
>
> such as for example:
> part /tmp --fstype=tmpfs --fsoptions="size=25%" --size=300
> part /tmp --fstype=tmpfs --fsoptions="size=25%" --grow
> part /tmp --fstype=tmpfs --fsoptions="size=25%" --grow --maxisze=300
> part /tmp --fstype=tmpfs --size=300 --grow
> part /tmp --fstype=tmpfs --size=300 --grow --maxsize=500
> part /tmp --fstype=tmpfs --size=300 --grow --maxsize=500
> -fsoptions="size=25%"
>
> The question is, how to handle these combinations ? Just document
> the correct usage and leave the behavior undefined or throw a kickstart
> parsing error if such combination is found in the kickstart file (as has
> been recently done for combining autopart and other partitioning
> commands)?
> --size is the only option that can set the size of the tmpfs mount
>
> to an amount larger than the size of the system RAM
> also the --size option is the only one it can set the size to an
>
> absolute value, all the other options (and the 50% default) all work
> with % sizes
> if the size is specified in percent and RAM is added/removed, the size
>
> of the tmpfs mounts automatically grows/shrinks accordingly
> > > 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):
> >
> > You could safely set _type to "tmpfs" here so that .type is meaningful.
>
> OK.
> > > 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 :)
>
> Oops. :)
> > > # 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.
>
> Oh, now I see it - good point.
> > >                # 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 the tmpfs --size kickstart option is not used, the "size" is never
> set kwargs, which breaks stuff such as checking the size of the
> filesystem. 
> So in this case the % is converted to a number (based on the current
> system RAM) and set in kwargs.
> Now when I think about this, this should be also done in case no
> explicit size is specified and the 50%-of-system-RAM default is used.

My question is related more to the fact that nothing will look at
kwargs["size"] after you set it there (or am I missing something).
You've already run the superclass constructors, and you're at the end of
the TmpFS constructor, so who's going to see that size you're putting in
there?
> > >    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?
>
> Yes, at least for the bugreport doing this should be enough.
> > 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.
>
> OK, I'll do that.
> > > @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.
>
> OK, good to know. :)
> > >        # 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.
>
> Well, yeah, that makes sense - it would always be "tmpfs", for all tmpfs
> mounts so a read-only property should do just fine.
> > > 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)
> >
> > anaconda-patches mailing list
> > [e-mail]
> > https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
>
> anaconda-patches mailing list
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches

[PATCH] Add tmpfs support (#918621)

[Martin Kolman]

On Wed, 2013-10-02 at 12:09 -0500, David Lehman wrote:
> On Wed, 2013-10-02 at 16:33 +0200, Martin Kolman wrote:
> > On Fri, 2013-09-27 at 09:12 -0500, David Lehman wrote:
> > > 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.
> >
> > Yeah, that might indeed by an issue - for example partition size.
> > Theoretically, there is no limit the size of a tmps partition.
> > That could do nasty things with the displayed total space/available
> > space - I guess it probably should not be counted into either ?
>
> Tmpfs mountpoints should not have any effect on the total/free space in
> custom. That makes me wonder, though -- do your patches handle the case
> where the user tries to create multiple tmpfs mountpoints with a total
> of >100% ? Also, surely specifying 100% of RAM for tmpfs size is madness
> -- no?

No, that might still be a valid usecase - tmpfs uses swap if there is no
space left in system RAM. So even when a tmpfs mount is over-committed
to say 200% of system RAM (or there are multiple mounts set to 100%,
etc.), as long as there is enough swap to cover the case of all the
tmpfs mounts filling up, everything should work just fine. So I can
imagine people with a lot of RAM making multiple tmpf temporary data
partitions and throwing a big swap partition to back it all up.

I can see this working quite nicely - as long as enough RAM is free, you
have a really fast storage and once RAM runs out, your tmpfs mounts will
just allocate more space from the swap partition page pool, without the
need to allocate a fixed backing storage partition for every mount.

Of course, if there is no swap or if swap is too small or full and can't
cover for the tmpfs mount/mounts filling up, the system will pretty much
lock up.

I've tried what it does on a VM without swap with a tmpfs partition
bigger than the system RAM. I've started filling the tmpf partition and
once RAM run out, the system became pretty much unusable. It was not
even possible to run any utilities to correct the issue - top, mount,
umount, df, nothing.

So one possibility would be to get RAM & swap size and then check if
there are any tmpfs mounts that are bigger than RAM & swap combined. 
But even this is not bulletproof, because the RAM & swap can be
independently filled by normal usage and the system still can lock-up if
the tmpfs mounts start to fill-up and there are no more pages to
allocate from RAM or SWAP.

BTW, having multiple mounts that are individually smaller than RAM +
swap, but combined bigger than RAM + swap might still be a valid
usecase, as long as the user makes sure the mounts don't all fill-up at
once. A rather extreme situation though.

On the other hand, no tmpfs mounts are created by default and the user
needs to explicitly specify he wants to have a tmpfs mount with a given
size.
So I guess describing all the possible issues/shortfalls in the docs and
expecting the user user does an informed decision might be enough ?
> > Also setting the size of the tmpfs mount could be an issue, as it can be
> > done both as an absolute value and as a % of RAM (more about this
> > further down).
>
> I would consider making it so that for tmpfs the size is always treated
> as a percentage.

I've done some additional tests and it appears this would actually be
possible - the size= option used in fstab or when mounting a tmpfs mount
accepts percentages >100%. If the device has 1GB RAM and a tmpfs mount
uses size=200%, the size of the mount will be 2GB.

But I'm not really sure this would be the behavior expected by the user
when he specifies a fixed size for the mount. In sucha a case, he might
not expect the size of the mount to automatically expand/contract if RAM
is added/removed.
> > > > 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?
> >
> > Oh, that should be "50% of system RAM 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
> > >
> > > 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.
> >
> > Well, yeah, this might be kinda confusing. :)
> > Ill try to summarize all the ways that can be used to set the size of a
> > tmpfs mount:
> > not specifying the size - size will correspond to 50% of system RAM
> >
> > Kickstart example:
> > part /tmp --fstype=tmpfs
> > setting a fixed size in megabytes - the tmpfs mount will have a fixed
> >
> > size corresponding to the given number of megabytes.
> > Kickstart example:
> > part /tmp --size=3000 --fstype=tmpfs
> > setting directly setting the percentage with --fsoptions - the tmpf
> >
> > mount will have a size in the % of system RAM given. 
> > Kickstart example:
> > part /tmp --fstype=tmpfs --fsoptions="size=25%"
> > -> this tmpfs mount will have a size of 25% of the system RAM
> > using the --grow option without --maxsize - the tmpfs mount size will
> >
> > correspond to 100% of system RAM
> > Kickstart example:
> > part /tmp --fstype=tmpfs --grow
> > BTW, the end result is the same as using:
> > part /tmp --fstype=tmpfs --fsoptions="size=100%"
> > using the --grow option with --maxsize - it gets a bit more
> >
> > complicated:
> > if maxsize > system RAM - the size of the tmpfs mount will be 100% RAM
> > if maxsize < system RAM - the size of the tmpfs mount will be <100%
> >
> > RAM
> > as long as --grow is used, the size is always specified in %
> > also --grow always grows only up to 100% od system RAM, never above,
> >
> > even if the number given by --maxsize is larger than system RAM
> > Kickstart example for a system with 1GB RAM
> > part /tmp --fstype=tmpfs --maxsize=3000 --grow
> > -> the size of the tmpfs mount will be 100% RAM
> > part /tmp --fstype=tmpfs --maxsize=512 --grow
> > -> the size of the tmpfs mount will be 50% RAM
> >
> > This is how the size works, and it has some consequences:
> > there are some "undefined" combinations for the kickstart command,
> >
> > such as for example:
> > part /tmp --fstype=tmpfs --fsoptions="size=25%" --size=300
> > part /tmp --fstype=tmpfs --fsoptions="size=25%" --grow
> > part /tmp --fstype=tmpfs --fsoptions="size=25%" --grow --maxisze=300
> > part /tmp --fstype=tmpfs --size=300 --grow
> > part /tmp --fstype=tmpfs --size=300 --grow --maxsize=500
> > part /tmp --fstype=tmpfs --size=300 --grow --maxsize=500
> > -fsoptions="size=25%"
> >
> > The question is, how to handle these combinations ? Just document
> > the correct usage and leave the behavior undefined or throw a kickstart
> > parsing error if such combination is found in the kickstart file (as has
> > been recently done for combining autopart and other partitioning
> > commands)?
> > --size is the only option that can set the size of the tmpfs mount
> >
> > to an amount larger than the size of the system RAM
> > also the --size option is the only one it can set the size to an
> >
> > absolute value, all the other options (and the 50% default) all work
> > with % sizes
> > if the size is specified in percent and RAM is added/removed, the size
> >
> > of the tmpfs mounts automatically grows/shrinks accordingly
> > > > 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):
> > >
> > > You could safely set _type to "tmpfs" here so that .type is meaningful.
> >
> > OK.
> > > > 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 :)
> >
> > Oops. :)
> > > > # 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.
> >
> > Oh, now I see it - good point.
> > > >                # 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 the tmpfs --size kickstart option is not used, the "size" is never
> > set kwargs, which breaks stuff such as checking the size of the
> > filesystem. 
> > So in this case the % is converted to a number (based on the current
> > system RAM) and set in kwargs.
> > Now when I think about this, this should be also done in case no
> > explicit size is specified and the 50%-of-system-RAM default is used.
>
> My question is related more to the fact that nothing will look at
> kwargs["size"] after you set it there (or am I missing something).
> You've already run the superclass constructors, and you're at the end of
> the TmpFS constructor, so who's going to see that size you're putting in
> there?

Oh! Yes, you are right, it is not needed. Also looks like even if it was
assigned before superclass constructors were run, all relevant size
related stuff has been overridden anyway. So I'll just remove it.
> > > >    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?
> >
> > Yes, at least for the bugreport doing this should be enough.
> > > 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.
> >
> > OK, I'll do that.
> > > > @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.
> >
> > OK, good to know. :)
> > > >        # 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.
> >
> > Well, yeah, that makes sense - it would always be "tmpfs", for all tmpfs
> > mounts so a read-only property should do just fine.
> > > > 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)
> > >
> > > anaconda-patches mailing list
> > > [e-mail]
> > > https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
> >
> > anaconda-patches mailing list
> > [e-mail]
> > https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
>
> anaconda-patches mailing list
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches