[PATCH] Add tmpfs support (#918621)
Martin Kolman
mkolman at redhat.com
Wed Oct 2 14:33:03 UTC 2013
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 <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.
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
> anaconda-patches at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
More information about the anaconda-patches
mailing list