curtin-dev team mailing list archive

[Paride Legovini <paride.legovini@xxxxxxxxxxxxx>]

Review: Needs Fixing

LGTM modulo one too much "they".

Diff comments:

> diff --git a/curtin/swap.py b/curtin/swap.py
> index d3f29dc..18628d9 100644
> --- a/curtin/swap.py
> +++ b/curtin/swap.py
> @@ -65,6 +122,24 @@ def setup_swapfile(target, fstab=None, swapfile=None, size=None, maxsize=None):
>      if not swapfile.startswith("/"):
>          swapfile = "/" + swapfile
>  
> +    # query the directory in which swapfile will reside
> +    fstype = get_fstype(target, os.path.dirname(swapfile))
> +    try:
> +        can_use_swapfile(target, fstype)
> +    except RuntimeError as err:
> +        if force:
> +            LOG.warning('swapfile may not work: %s', err)
> +        else:
> +            LOG.debug('Not creating swap: %s', err)
> +            return
> +
> +    allocate_cmd = 'fallocate -l "${2}M" "$1"'
> +    # fallocate uses IOCTLs to allocate space in a filesystem, however they
> +    # it's not clear that this creates non-sparse files as required by mkswap

s/they//

> +    # so we'll skip for now and use dd.
> +    if fstype in ['btrfs', 'xfs']:
> +        allocate_cmd = 'dd if=/dev/zero "of=$1" bs=1M "count=$2"'
> +
>      mbsize = str(int(size / (2 ** 20)))
>      msg = "creating swap file '%s' of %sMB" % (swapfile, mbsize)
>      fpath = os.path.sep.join([target, swapfile])
> diff --git a/doc/topics/config.rst b/doc/topics/config.rst
> index 72cd683..90e467e 100644
> --- a/doc/topics/config.rst
> +++ b/doc/topics/config.rst
> @@ -752,13 +752,25 @@ Configure the max size of the swapfile, defaults to 8GB
>  Configure the exact size of the swapfile.  Setting ``size`` to 0 will
>  disable swap.
>  
> +**force**: *<boolean>*
> +
> +Force the creation of swapfile even if curtin detects it may not work.
> +In some target filesystems, e.g. btrfs, xfs, zfs; use of a swap file has

nit: colon instead of semicolon?

> +restrictions.  If curtin detects that there may be issues it will refuse
> +to create the swapfile.  Users can force this by passing ``force: true``.
> +
>  **Example**::
>  
>    swap:
>      filename: swap.img
> -    size: None
> +    size: 1GB
>      maxsize: 4GB
>  
> +  swap:
> +    filename: btrfs_swapfile.img
> +    size: 1GB
> +    force: true
> +
>  
>  system_upgrade
>  ~~~~~~~~~~~~~~


-- 
https://code.launchpad.net/~raharper/curtin/+git/curtin/+merge/386117
Your team curtin developers is subscribed to branch curtin:master.