On 2021/06/05 9:42, Damien Le Moal wrote:
> On 2021/06/04 21:18, Niklas Cassel wrote:
>> From: Niklas Cassel <niklas.cassel@xxxxxxx>
>>
>> For a job with zonemode=zbd, we do not want any file to be ignored.
>> Each file's file type in that job should be supported by either zbd.c
>> or the ioengine. If not, we should return an error.
>> This way, ZBD_IGNORE becomes redundant and can be removed.
>>
>> By removing ZBD_IGNORE, we know that all files belonging to a job that
>> has zonemode=zbd set, will either be a zoned block device, or emulate
>> a zoned block device.
>>
>> This means that for jobs that have zonemode=zbd, f->zbd_info will always
>> be non-NULL. This will make the zbd code slightly easier to reason about
>> and to maintain.
>>
>> When removing zbd_zoned_model ZBD_IGNORE, define the new first enum value
>> as 0x1, so that we avoid potential ABI problems with existing binaries.
>>
>> Signed-off-by: Niklas Cassel <niklas.cassel@xxxxxxx>
>> ---
>> engines/libzbc.c | 3 +--
>> engines/skeleton_external.c | 1 -
>> oslib/linux-blkzoned.c | 3 +--
>> zbd.c | 3 +--
>> zbd_types.h | 7 +++----
>> 5 files changed, 6 insertions(+), 11 deletions(-)
>>
>> diff --git a/engines/libzbc.c b/engines/libzbc.c
>> index 3dde93db..c3551bec 100644
>> --- a/engines/libzbc.c
>> +++ b/engines/libzbc.c
>> @@ -181,8 +181,7 @@ static int libzbc_get_zoned_model(struct thread_data *td, struct fio_file *f,
>> int ret;
>>
>> if (f->filetype != FIO_TYPE_BLOCK && f->filetype != FIO_TYPE_CHAR) {
>> - *model = ZBD_IGNORE;
>> - return 0;
>> + return -EINVAL;
>> }
>>
>> ret = libzbc_open_dev(td, f, &ld);
>> diff --git a/engines/skeleton_external.c b/engines/skeleton_external.c
>> index c79b6f11..cff83a10 100644
>> --- a/engines/skeleton_external.c
>> +++ b/engines/skeleton_external.c
>> @@ -156,7 +156,6 @@ static int fio_skeleton_close(struct thread_data *td, struct fio_file *f)
>> /*
>> * Hook for getting the zoned model of a zoned block device for zonemode=zbd.
>> * The zoned model can be one of (see zbd_types.h):
>> - * - ZBD_IGNORE: skip regular files
>> * - ZBD_NONE: regular block device (zone emulation will be used)
>> * - ZBD_HOST_AWARE: host aware zoned block device
>> * - ZBD_HOST_MANAGED: host managed zoned block device
>> diff --git a/oslib/linux-blkzoned.c b/oslib/linux-blkzoned.c
>> index 6f89ec6f..8cb9c53a 100644
>> --- a/oslib/linux-blkzoned.c
>> +++ b/oslib/linux-blkzoned.c
>> @@ -141,8 +141,7 @@ int blkzoned_get_zoned_model(struct thread_data *td, struct fio_file *f,
>> char *model_str = NULL;
>>
>> if (f->filetype != FIO_TYPE_BLOCK) {
>> - *model = ZBD_IGNORE;
>> - return 0;
>> + return -EINVAL;
>> }
>>
>> *model = ZBD_NONE;
>> diff --git a/zbd.c b/zbd.c
>> index fb20f6cc..7012cb93 100644
>> --- a/zbd.c
>> +++ b/zbd.c
>> @@ -661,8 +661,6 @@ static int zbd_create_zone_info(struct thread_data *td, struct fio_file *f)
>> return ret;
>>
>> switch (zbd_model) {
>> - case ZBD_IGNORE:
>> - return 0;
>> case ZBD_HOST_AWARE:
>> case ZBD_HOST_MANAGED:
>> ret = parse_zone_info(td, f);
>> @@ -680,6 +678,7 @@ static int zbd_create_zone_info(struct thread_data *td, struct fio_file *f)
>> return -EINVAL;
>> }
>>
>> + assert(f->zbd_info);
>> f->zbd_info->model = zbd_model;
>>
>> ret = zbd_set_max_open_zones(td, f);
>> diff --git a/zbd_types.h b/zbd_types.h
>> index d0f4c44e..0a8630cb 100644
>> --- a/zbd_types.h
>> +++ b/zbd_types.h
>> @@ -14,10 +14,9 @@
>> * Zoned block device models.
>> */
>> enum zbd_zoned_model {
>> - ZBD_IGNORE, /* Ignore file */
>> - ZBD_NONE, /* No zone support. Emulate zones. */
>> - ZBD_HOST_AWARE, /* Host-aware zoned block device */
>> - ZBD_HOST_MANAGED, /* Host-managed zoned block device */
>> + ZBD_NONE = 0x1, /* No zone support. Emulate zones. */
>> + ZBD_HOST_AWARE = 0x2, /* Host-aware zoned block device */
>> + ZBD_HOST_MANAGED = 0x3, /* Host-managed zoned block device */
>> };
>>
>> /*
>>
>
> Looks good.
>
> Reviewed-by: Damien Le Moal <damien.lemoal@xxxxxxx>
Scrap this. Rereading the patch and code, I have comments.
--
Damien Le Moal
Western Digital Research
