On Wed 16-05-18 10:54:10, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@xxxxxx>
>
> Currently, for an invalid swap file, we print the same error message
> regardless of the reason. This isn't very useful for an admin, who will
> likely want to know why exactly they can't use their swap file. So,
> let's add specific error messages for each reason, and also move the
> bdev check after the flags checks, since the latter are more
> fundamental.
>
> Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> Signed-off-by: Omar Sandoval <osandov@xxxxxx>
Yup, looks good to me. You can add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
Honza
> ---
> fs/iomap.c | 40 ++++++++++++++++++++++++----------------
> 1 file changed, 24 insertions(+), 16 deletions(-)
>
> diff --git a/fs/iomap.c b/fs/iomap.c
> index d193390a1c20..89517442e296 100644
> --- a/fs/iomap.c
> +++ b/fs/iomap.c
> @@ -1214,26 +1214,37 @@ static loff_t iomap_swapfile_activate_actor(struct inode *inode, loff_t pos,
> struct iomap_swapfile_info *isi = data;
> int error;
>
> + /* No inline data. */
> + if (iomap->flags & IOMAP_F_DATA_INLINE) {
> + pr_err("swapon: file is inline\n");
> + return -EINVAL;
> + }
> +
> /* Skip holes. */
> if (iomap->type == IOMAP_HOLE)
> goto out;
>
> - /* Only one bdev per swap file. */
> - if (iomap->bdev != isi->sis->bdev)
> - goto err;
> -
> /* Only real or unwritten extents. */
> - if (iomap->type != IOMAP_MAPPED && iomap->type != IOMAP_UNWRITTEN)
> - goto err;
> + if (iomap->type != IOMAP_MAPPED && iomap->type != IOMAP_UNWRITTEN) {
> + pr_err("swapon: file has unallocated extents\n");
> + return -EINVAL;
> + }
>
> - /* No uncommitted metadata or shared blocks or inline data. */
> - if (iomap->flags & (IOMAP_F_DIRTY | IOMAP_F_SHARED |
> - IOMAP_F_DATA_INLINE))
> - goto err;
> + /* No uncommitted metadata or shared blocks. */
> + if (iomap->flags & IOMAP_F_DIRTY) {
> + pr_err("swapon: file is not committed\n");
> + return -EINVAL;
> + }
> + if (iomap->flags & IOMAP_F_SHARED) {
> + pr_err("swapon: file has shared extents\n");
> + return -EINVAL;
> + }
>
> - /* No null physical addresses. */
> - if (iomap->addr == IOMAP_NULL_ADDR)
> - goto err;
> + /* Only one bdev per swap file. */
> + if (iomap->bdev != isi->sis->bdev) {
> + pr_err("swapon: file is on multiple devices\n");
> + return -EINVAL;
> + }
>
> if (isi->iomap.length == 0) {
> /* No accumulated extent, so just store it. */
> @@ -1250,9 +1261,6 @@ static loff_t iomap_swapfile_activate_actor(struct inode *inode, loff_t pos,
> }
> out:
> return count;
> -err:
> - pr_err("swapon: file cannot be used for swap\n");
> - return -EINVAL;
> }
>
> /*
> --
> 2.17.0
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
