[PATCH 2/2] Add tmpfs support (#918621)

Brian C. Lane bcl at redhat.com
Mon Oct 21 18:54:57 UTC 2013


On Wed, Oct 09, 2013 at 03:49:22PM +0200, Martin Kolman wrote:

> +    bits = arch.bits()
> +    if bits == 32:
> +        _maxSize = 16 * 1024 * 1024
> +    elif bits == 64:
> +        _maxSize = 16 * 1024 * 1024 * 1024 * 1024

We shouldn't call things from inside the class like this, I also don't
like seeing logic here -- it should be in .init or in a property
(possibly cached).

> +    def _getOptions(self):
> +        # if the size option string is defined,
> +        # append it to options
> +        # (we need to separate the size option string
> +        # from the other options, as the size might change
> +        # when the filesystem is resized;
> +        # replacing the size option in the otherwise free-form
> +        # options string would get messy fast)
> +        option_string = self._options
> +        if self._size_option:
> +            if option_string:
> +                # append
> +                option_string = "%s,%s" % (self._options, self._size_option)
> +            else:
> +                # assign
> +                option_string = self._size_option
> +        if option_string:
> +            return option_string
> +        else:
> +            return "defaults"

A more compact way to do this would be:

opts = ",".join([o for o in [self._options, self._size_option] if o])
return opts or "defaults"

So you get one or the other (or neither) and if both joined by ,

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

[snip]

> +    @property
> +    def device(self):
> +        """ All the tmpfs mounts use the same "tmpfs" device. """
> +        return self._type
> +
> +    @device.setter
> +    def device(self, value):
> +        # the DeviceFormat parent class does a
> +        # self.device = kwargs["device"]
> +        # assignment, so we need a setter for the
> +        # device property, but as the device is always the
> +        # same, nothing actually needs to be set
> +        pass

For options you used property() which matches current blivet style, but
here use @foo.setter (which I like better) but we should probably follow
the current style and just use property() everywhere.

-- 
Brian C. Lane | Anaconda Team | IRC: bcl #anaconda | Port Orchard, WA (PST8PDT)


More information about the anaconda-patches mailing list