Re: [PATCH v3 2/3] cdrom: gdrom: update gendisk capacity on open

On 2026-04-23 21:41, Florian Fuchs wrote:
> Update the gendisk capacity of the media. Without the capacity, the 
> block
> reads fail before reaching the request queue, which prevented ISO9660
> mounts. Refresh the capacity from the TOC leadout in gdrom_bdops_open()
> so it checks the inserted media.
> 
> Signed-off-by: Florian Fuchs <[email protected]>

Hey Florian,
thanks for v3. Verified on real hardware.

> ---
> v2->v3: Also add quirk to handle proprietary GDROMs, using the same
>         mechanic like in gdrom_get_last_session() try session 1 first
> 	for GDROM, then session 0 for CDROMs. Dropped Acked-By due to
> 	code change.
> v1->v2: no change for gdrom_update_capacity(), but for
>         gdrom_bdops_open(): handle the failure case when
>         gdrom_update_capacity() fails but previous cdrom_open() 
> succeeded,
>         to cleanup the successful cdrom_open() with cdrom_release()
> 
> v2: 
> https://lore.kernel.org/linux-sh/[email protected]/
> v1: 
> https://lore.kernel.org/linux-sh/[email protected]/
> 
>  drivers/cdrom/gdrom.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/cdrom/gdrom.c b/drivers/cdrom/gdrom.c
> index 094d55b2d004..1e73617b39ac 100644
> --- a/drivers/cdrom/gdrom.c
> +++ b/drivers/cdrom/gdrom.c
> @@ -474,6 +474,27 @@ static const struct cdrom_device_ops gdrom_ops = {
>  				  CDC_RESET | CDC_DRIVE_STATUS | CDC_CD_R,
>  };
> 
> +static int gdrom_update_capacity(void)
> +{
> +	sector_t cap;
> +	int ret;
> +
> +	if (gdrom_drivestatus(gd.cd_info, CDSL_CURRENT) != CDS_DISC_OK) {
> +		set_capacity(gd.disk, 0);
> +		return -ENOMEDIUM;
> +	}
> +	ret = gdrom_readtoc_cmd(gd.toc, 1);
> +	if (ret)
> +		ret = gdrom_readtoc_cmd(gd.toc, 0);
> +	if (ret) {
> +		set_capacity(gd.disk, 0);
> +		return ret;
> +	}

Since you already clobber the return value of
gdrom_readtoc_cmd(gd.toc, 1), then how about make it even simpler:

>        if (gdrom_readtoc_cmd(gd.toc, 1) && gdrom_readtoc_cmd(gd.toc, 
> 0)) {
>                set_capacity(gd.disk, 0);
>                return -EINVAL;
>        }

With or without the above change:

Acked-by: Artur Rojek <[email protected]>

Cheers,
Artur

> +	cap = (sector_t)get_entry_lba(gd.toc->leadout) * GD_TO_BLK;
> +	set_capacity(gd.disk, cap);
> +	return 0;
> +}
> +
>  static int gdrom_bdops_open(struct gendisk *disk, blk_mode_t mode)
>  {
>  	int ret;
> @@ -482,6 +503,12 @@ static int gdrom_bdops_open(struct gendisk *disk, 
> blk_mode_t mode)
> 
>  	mutex_lock(&gdrom_mutex);
>  	ret = cdrom_open(gd.cd_info, mode);
> +	if (ret)
> +		goto out;
> +	ret = gdrom_update_capacity();
> +	if (ret)
> +		cdrom_release(gd.cd_info);
> +out:
>  	mutex_unlock(&gdrom_mutex);
>  	return ret;
>  }