Re: [PATCH v3 2/3] block: add a s390-only biodasdinfo method

Christoph Hellwig
2020-05-08T15:53:10Z

On Fri, May 08, 2020 at 03:14:54PM +0200, Stefan Haberland wrote:
> From: Christoph Hellwig <hch@lst.de>
>
> The IBM partition parser needs to query the DASD driver for details that
> are very s390 specific. Instead of using ioctl_by_bdev with a fake user
> space pointer just add a s390-specific method to get the information
> directly.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> [sth@linux.ibm.com: remove fop, add gendisk check, export funcion]
> Signed-off-by: Stefan Haberland <sth@linux.ibm.com>

The subject and changelog need updates for your changes. I think you
should also claim authorship, even if a few bits are originally from me.
Probaby it would make sense to even just merge this into the next patch.

> index fa552f9f1666..6eac7b11c75b 100644
> --- a/drivers/s390/block/dasd_int.h
> +++ b/drivers/s390/block/dasd_int.h
> @@ -845,6 +845,7 @@ void dasd_destroy_partitions(struct dasd_block *);
>
> /* externals in dasd_ioctl.c */
> int dasd_ioctl(struct block_device *, fmode_t, unsigned int, unsigned long);
> +int dasd_biodasdinfo(struct gendisk *disk, struct dasd_information2_t *info);

I think this needs to go to a public include/linux/ header for the
partitioning code to share the prototype.

> +int dasd_biodasdinfo(struct gendisk *disk, struct dasd_information2_t *info)
> +{
> + struct dasd_device *base;
> + int error;
> +
> + /*
> + * we might get called externaly, so check if the gendisk belongs
> + * to a DASD by checking the fops pointer
> + */
> + if (disk->fops != &dasd_device_operations)
> + return -EINVAL;

I think a function comment (e.g. kernel doc) explaining the use case and
this detail might be useful.