Re: [PATCH 02/10] block: virtio-blk: check logical block size
- To: Maxim Levitsky <mlevitsk@redhat.com>
- Cc: "Martin K. Petersen" <martin.petersen@oracle.com>, Christoph Hellwig <hch@lst.de>, linux-kernel@vger.kernel.org, Keith Busch <kbusch@kernel.org>, Josef Bacik <josef@toxicpanda.com>, "open list:BLOCK LAYER" <linux-block@vger.kernel.org>, Sagi Grimberg <sagi@grimberg.me>, Jens Axboe <axboe@kernel.dk>, "open list:NVM EXPRESS DRIVER" <linux-nvme@lists.infradead.org>, "open list:SCSI CDROM DRIVER" <linux-scsi@vger.kernel.org>, Tejun Heo <tj@kernel.org>, Bart Van Assche <bvanassche@acm.org>, Damien Le Moal <damien.lemoal@wdc.com>, Jason Wang <jasowang@redhat.com>, Maxim Levitsky <maximlevitsky@gmail.com>, Stefan Hajnoczi <stefanha@redhat.com>, Colin Ian King <colin.king@canonical.com>, "Michael S. Tsirkin" <mst@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Ulf Hansson <ulf.hansson@linaro.org>, Ajay Joshi <ajay.joshi@wdc.com>, Ming Lei <ming.lei@redhat.com>, "open list:SONY MEMORYSTICK SUBSYSTEM" <linux-mmc@vger.kernel.org>, Satya Tangirala <satyat@google.com>, "open list:NETWORK BLOCK DEVICE (NBD)" <nbd@other.debian.org>, Hou Tao <houtao1@huawei.com>, Jens Axboe <axboe@fb.com>, "open list:VIRTIO CORE AND NET DRIVERS" <virtualization@lists.linux-foundation.org>, "James E.J. Bottomley" <jejb@linux.ibm.com>, Alex Dubov <oakad@yahoo.com>
- Subject: Re: [PATCH 02/10] block: virtio-blk: check logical block size
- From: "Martin K. Petersen" <martin.petersen@oracle.com>
- Date: Wed, 29 Jul 2020 00:32:55 -0400
- Message-id: <[🔎] yq1bljz7y9q.fsf@ca-mkp.ca.oracle.com>
- In-reply-to: <[🔎] f16aba1020019530564f0869a67951282104a5d2.camel@redhat.com> (Maxim Levitsky's message of "Wed, 22 Jul 2020 12:11:17 +0300")
- References: <[🔎] 20200721105239.8270-1-mlevitsk@redhat.com> <[🔎] 20200721105239.8270-3-mlevitsk@redhat.com> <20200721151437.GB10620@lst.de> <[🔎] yq1zh7sfedj.fsf@ca-mkp.ca.oracle.com> <[🔎] f16aba1020019530564f0869a67951282104a5d2.camel@redhat.com>
Hi Maxim,
> Instead of this adding blk_is_valid_logical_block_size allowed me to
> trivially convert most of the uses.
>
> For RFC I converted only some drivers that I am more familiar with
> and/or can test but I can remove the driver's own checks in most other
> drivers with low chance of introducing a bug, even if I can't test the
> driver.
>
> What do you think?
>
> I can also both make blk_queue_logical_block_size return an error
> value, and have blk_is_valid_logical_block_size and use either of
> these checks, depending on the driver with eventual goal of
> un-exporting blk_is_valid_logical_block_size.
My concern is that blk_is_valid_logical_block_size() deviates from the
regular block layer convention where the function to set a given queue
limit will either adjust the passed value or print a warning.
I guess I won't entirely object to having the actual check in a helper
function that drivers with a peculiar initialization pattern can
use. And then make blk_queue_logical_block_size() call that helper as
well to validate the lbs.
But I do think that blk_queue_logical_block_size() should be the
preferred interface and that, where possible, drivers should be updated
to check the return value of that.
--
Martin K. Petersen Oracle Linux Engineering
Reply to: