Re: [PATCH v2 4/4] linux-user: Add support for btrfs ioctls used to scrub a filesystem

[Laurent Vivier]

Le 17/07/2020 à 16:44, Filip Bozuta a écrit :
> This patch implements functionality for following ioctls:
> 
> BTRFS_IOC_SCRUB - Starting a btrfs filesystem scrub
> 
>     Start a btrfs filesystem scrub. The third ioctls argument
>     is a pointer to a following type:
> 
>     struct btrfs_ioctl_scrub_args {
>       __u64 devid;                            /* in */
>       __u64 start;                            /* in */
>       __u64 end;                              /* in */
>       __u64 flags;                            /* in */
>       struct btrfs_scrub_progress progress;   /* out */
>       /* pad to 1k */
>       __u64 unused[(1024-32-sizeof(struct btrfs_scrub_progress))/8];
>     };
> 
>     Before calling this ioctl, field 'devid' should be filled
>     with value that represents the device id of the btrfs filesystem
>     for which the scrub is to be started.
> 
> BTRFS_IOC_SCRUB_CANCEL - Canceling scrub of a btrfs filesystem
> 
>     Cancel a btrfs filesystem scrub if it is running. The third
>     ioctls argument is ignored.
> 
> BTRFS_IOC_SCRUB_PROGRESS - Getting status of a running scrub
> 
>     Read the status of a running btrfs filesystem scrub. The third
>     ioctls argument is a pointer to the above mentioned
>     'struct btrfs_ioctl_scrub_args'. Similarly as with 'BTRFS_IOC_SCRUB',
>     the 'devid' field should be filled with value that represents the
>     id of the btrfs device for which the scrub has started. The status
>     of a running scrub is returned in the field 'progress' which is
>     of type 'struct btrfs_scrub_progress' and its definition can be
>     found at:
>     
> https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/btrfs.h#L150
> 
> Implementation nots:
> 
>     Ioctls in this patch use type 'struct btrfs_ioctl_scrub_args' as their
>     third argument. That is the reason why an aproppriate thunk type
>     definition is added in file 'syscall_types.h'.
> 
> Signed-off-by: Filip Bozuta <Filip.Bozuta@syrmia.com>
> ---
>  linux-user/ioctls.h        | 11 +++++++++++
>  linux-user/syscall_defs.h  |  6 ++++++
>  linux-user/syscall_types.h | 26 ++++++++++++++++++++++++++
>  3 files changed, 43 insertions(+)
> 
> diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
> index 8665f504bf..bf80615438 100644
> --- a/linux-user/ioctls.h
> +++ b/linux-user/ioctls.h
> @@ -215,6 +215,17 @@
>  #ifdef BTRFS_IOC_SUBVOL_SETFLAGS
>       IOCTL(BTRFS_IOC_SUBVOL_SETFLAGS, IOC_W, MK_PTR(TYPE_ULONGLONG))
>  #endif
> +#ifdef BTRFS_IOC_SCRUB
> +     IOCTL(BTRFS_IOC_SCRUB, IOC_RW,
> +           MK_PTR(MK_STRUCT(STRUCT_btrfs_ioctl_scrub_args)))
> +#endif
> +#ifdef BTRFS_IOC_SCRUB_CANCEL
> +     IOCTL(BTRFS_IOC_SCRUB_CANCEL, 0, TYPE_NULL)
> +#endif
> +#ifdef BTRFS_IOC_SCRUB_PROGRESS
> +     IOCTL(BTRFS_IOC_SCRUB_PROGRESS, IOC_RW,
> +           MK_PTR(MK_STRUCT(STRUCT_btrfs_ioctl_scrub_args)))
> +#endif
>  #ifdef BTRFS_IOC_DEV_INFO
>       IOCTL(BTRFS_IOC_DEV_INFO, IOC_RW,
>             MK_PTR(MK_STRUCT(STRUCT_btrfs_ioctl_dev_info_args)))
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index 3f771ae5d1..589ec3e9b0 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -990,6 +990,12 @@ struct target_rtc_pll_info {
>                                                             25, abi_ullong)
>  #define TARGET_BTRFS_IOC_SUBVOL_SETFLAGS        
> TARGET_IOW(BTRFS_IOCTL_MAGIC, \
>                                                             26, abi_ullong)
> +#define TARGET_BTRFS_IOC_SCRUB                  
> TARGET_IOWR(BTRFS_IOCTL_MAGIC,\
> +                                             27, struct 
> btrfs_ioctl_scrub_args)
> +#define TARGET_BTRFS_IOC_SCRUB_CANCEL           TARGET_IO(BTRFS_IOCTL_MAGIC, 
>  \
> +                                                          28)
> +#define TARGET_BTRFS_IOC_SCRUB_PROGRESS         
> TARGET_IOWR(BTRFS_IOCTL_MAGIC,\
> +                                             29, struct 
> btrfs_ioctl_scrub_args)

Use the 'U' variant.

>  #define TARGET_BTRFS_IOC_DEV_INFO               
> TARGET_IOWR(BTRFS_IOCTL_MAGIC,\
>                                            30, struct 
> btrfs_ioctl_dev_info_args)
>  #define TARGET_BTRFS_IOC_INO_PATHS              
> TARGET_IOWR(BTRFS_IOCTL_MAGIC,\
> diff --git a/linux-user/syscall_types.h b/linux-user/syscall_types.h
> index b4f462b5c6..345193270c 100644
> --- a/linux-user/syscall_types.h
> +++ b/linux-user/syscall_types.h
> @@ -373,6 +373,32 @@ STRUCT(btrfs_ioctl_ino_lookup_user_args,
>         MK_ARRAY(TYPE_CHAR, BTRFS_VOL_NAME_MAX + 1), /* name */
>         MK_ARRAY(TYPE_CHAR, BTRFS_INO_LOOKUP_USER_PATH_MAX)) /* path */
>  
> +STRUCT(btrfs_scrub_progress,
> +       TYPE_ULONGLONG, /* data_extents_scrubbed */
> +       TYPE_ULONGLONG, /* tree_extents_scrubbed */
> +       TYPE_ULONGLONG, /* data_bytes_scrubbed */
> +       TYPE_ULONGLONG, /* tree_bytes_scrubbed */
> +       TYPE_ULONGLONG, /* read_errors */
> +       TYPE_ULONGLONG, /* csum_errors */
> +       TYPE_ULONGLONG, /* verify_errors */
> +       TYPE_ULONGLONG, /* no_csum */
> +       TYPE_ULONGLONG, /* csum_discards */
> +       TYPE_ULONGLONG, /* super_errors */
> +       TYPE_ULONGLONG, /* malloc_errors */
> +       TYPE_ULONGLONG, /* uncorrectable_errors */
> +       TYPE_ULONGLONG, /* corrected_er */
> +       TYPE_ULONGLONG, /* last_physical */
> +       TYPE_ULONGLONG) /* unverified_errors */
> +
> +STRUCT(btrfs_ioctl_scrub_args,
> +  TYPE_ULONGLONG, /* devid */
> +  TYPE_ULONGLONG, /* start */
> +  TYPE_ULONGLONG, /* end */
> +  TYPE_ULONGLONG, /* flags */
> +  MK_STRUCT(STRUCT_btrfs_scrub_progress), /* progress */
> +  MK_ARRAY(TYPE_ULONGLONG,
> +           (1024 - 32 - sizeof(struct btrfs_scrub_progress)) / 8)) /* unused 
> */
>

The clean way would be to define a target_btrfs_scrub_progress and to
use it in the sizeof(). But I don't know if it's worth to define it as
it is only made of u64 and the result will be the same for all the guest
types (and the same as for the host one).

With the 'U' variant:

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Thanks,
Laurent