Re: [PATCH v2] cdrom, scsi: sr: propagate read-only status to block layer via set_disk_ro()

[Phillip Potter <phil@philpotter.co.uk>]

On Wed, Apr 22, 2026 at 11:32:06AM +0000, Daan De Meyer wrote:
> 
>  drivers/cdrom/cdrom.c | 73 ++++++++++++++++++++++++++++---------------
>  drivers/scsi/sr.c     | 11 ++-----
>  drivers/scsi/sr.h     |  1 -
>  include/linux/cdrom.h |  1 +
>  4 files changed, 51 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
> index fc049612d6dc..62934cf4b10d 100644
> --- a/drivers/cdrom/cdrom.c
> +++ b/drivers/cdrom/cdrom.c
> @@ -631,6 +631,16 @@ int register_cdrom(struct gendisk *disk, struct cdrom_device_info *cdi)
>  
>  	WARN_ON(!cdo->generic_packet);
>  
> +	/*
> +	 * Propagate the drive's write support to the block layer so BLKROGET
> +	 * reflects actual write capability. Drivers that use GET CONFIGURATION
> +	 * features (CDC_MRW_W, CDC_RAM) must have called
> +	 * cdrom_probe_write_features() before register_cdrom() so the mask is
> +	 * complete here.
> +	 */
> +	set_disk_ro(disk, !CDROM_CAN(CDC_DVD_RAM | CDC_MRW_W | CDC_RAM |
> +				     CDC_CD_RW));
> +
>  	cd_dbg(CD_REG_UNREG, "drive \"/dev/%s\" registered\n", cdi->name);
>  	mutex_lock(&cdrom_mutex);
>  	list_add(&cdi->list, &cdrom_list);
> @@ -742,6 +752,44 @@ static int cdrom_is_random_writable(struct cdrom_device_info *cdi, int *write)
>  	return 0;
>  }
>  
> +/*
> + * Probe write-related MMC features via GET CONFIGURATION and update
> + * cdi->mask accordingly. Drivers that populate cdi->mask from the MODE SENSE
> + * capabilities page (e.g. sr) should call this after those MODE SENSE bits
> + * have been set but before register_cdrom(), so that the full set of
> + * write-capability bits is known by the time register_cdrom() decides on the
> + * initial read-only state of the disk.
> + */
> +void cdrom_probe_write_features(struct cdrom_device_info *cdi)
> +{
> +	int mrw, mrw_write, ram_write;
> +
> +	mrw = 0;
> +	if (!cdrom_is_mrw(cdi, &mrw_write))
> +		mrw = 1;
> +
> +	if (CDROM_CAN(CDC_MO_DRIVE))
> +		ram_write = 1;
> +	else
> +		(void) cdrom_is_random_writable(cdi, &ram_write);
> +
> +	if (mrw)
> +		cdi->mask &= ~CDC_MRW;
> +	else
> +		cdi->mask |= CDC_MRW;
> +
> +	if (mrw_write)
> +		cdi->mask &= ~CDC_MRW_W;
> +	else
> +		cdi->mask |= CDC_MRW_W;
> +
> +	if (ram_write)
> +		cdi->mask &= ~CDC_RAM;
> +	else
> +		cdi->mask |= CDC_RAM;
> +}
> +EXPORT_SYMBOL(cdrom_probe_write_features);
> +
>  static int cdrom_media_erasable(struct cdrom_device_info *cdi)
>  {
>  	disc_information di;
> @@ -894,33 +942,8 @@ static int cdrom_is_dvd_rw(struct cdrom_device_info *cdi)
>   */
>  static int cdrom_open_write(struct cdrom_device_info *cdi)
>  {
> -	int mrw, mrw_write, ram_write;
>  	int ret = 1;
>  
> -	mrw = 0;
> -	if (!cdrom_is_mrw(cdi, &mrw_write))
> -		mrw = 1;
> -
> -	if (CDROM_CAN(CDC_MO_DRIVE))
> -		ram_write = 1;
> -	else
> -		(void) cdrom_is_random_writable(cdi, &ram_write);
> -	
> -	if (mrw)
> -		cdi->mask &= ~CDC_MRW;
> -	else
> -		cdi->mask |= CDC_MRW;
> -
> -	if (mrw_write)
> -		cdi->mask &= ~CDC_MRW_W;
> -	else
> -		cdi->mask |= CDC_MRW_W;
> -
> -	if (ram_write)
> -		cdi->mask &= ~CDC_RAM;
> -	else
> -		cdi->mask |= CDC_RAM;
> -
>  	if (CDROM_CAN(CDC_MRW_W))
>  		ret = cdrom_mrw_open_write(cdi);
>  	else if (CDROM_CAN(CDC_DVD_RAM))
> diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
> index 7adb2573f50d..c36c54ecd354 100644
> --- a/drivers/scsi/sr.c
> +++ b/drivers/scsi/sr.c
> @@ -395,7 +395,7 @@ static blk_status_t sr_init_command(struct scsi_cmnd *SCpnt)
>  
>  	switch (req_op(rq)) {
>  	case REQ_OP_WRITE:
> -		if (!cd->writeable)
> +		if (get_disk_ro(cd->disk))
>  			goto out;
>  		SCpnt->cmnd[0] = WRITE_10;
>  		cd->cdi.media_written = 1;
> @@ -681,6 +681,7 @@ static int sr_probe(struct scsi_device *sdev)
>  	error = -ENOMEM;
>  	if (get_capabilities(cd))
>  		goto fail_minor;
> +	cdrom_probe_write_features(&cd->cdi);
>  	sr_vendor_init(cd);
>  
>  	set_capacity(disk, cd->capacity);
> @@ -899,14 +900,6 @@ static int get_capabilities(struct scsi_cd *cd)
>  	/*else    I don't think it can close its tray
>  		cd->cdi.mask |= CDC_CLOSE_TRAY; */
>  
> -	/*
> -	 * if DVD-RAM, MRW-W or CD-RW, we are randomly writable
> -	 */
> -	if ((cd->cdi.mask & (CDC_DVD_RAM | CDC_MRW_W | CDC_RAM | CDC_CD_RW)) !=
> -			(CDC_DVD_RAM | CDC_MRW_W | CDC_RAM | CDC_CD_RW)) {
> -		cd->writeable = 1;
> -	}
> -
>  	kfree(buffer);
>  	return 0;
>  }
> diff --git a/drivers/scsi/sr.h b/drivers/scsi/sr.h
> index dc899277b3a4..2d92f9cb6fec 100644
> --- a/drivers/scsi/sr.h
> +++ b/drivers/scsi/sr.h
> @@ -35,7 +35,6 @@ typedef struct scsi_cd {
>  	struct scsi_device *device;
>  	unsigned int vendor;	/* vendor code, see sr_vendor.c         */
>  	unsigned long ms_offset;	/* for reading multisession-CD's        */
> -	unsigned writeable : 1;
>  	unsigned use:1;		/* is this device still supportable     */
>  	unsigned xa_flag:1;	/* CD has XA sectors ? */
>  	unsigned readcd_known:1;	/* drive supports READ_CD (0xbe) */
> diff --git a/include/linux/cdrom.h b/include/linux/cdrom.h
> index b907e6c2307d..260d7968cf72 100644
> --- a/include/linux/cdrom.h
> +++ b/include/linux/cdrom.h
> @@ -108,6 +108,7 @@ int cdrom_ioctl(struct cdrom_device_info *cdi, struct block_device *bdev,
>  extern unsigned int cdrom_check_events(struct cdrom_device_info *cdi,
>  				       unsigned int clearing);
>  
> +extern void cdrom_probe_write_features(struct cdrom_device_info *cdi);
>  extern int register_cdrom(struct gendisk *disk, struct cdrom_device_info *cdi);
>  extern void unregister_cdrom(struct cdrom_device_info *cdi);
>  
> -- 
> 2.53.0
>

Hi Daan,

I've looked through the patch and it looks good to me. Looks like a
decent change. I think in this case too, it's unlikely this historically
broken behaviour is being relied upon (i.e. it seems unlikely to me that
fixing it would break anything).

In addition, I've build tested and booted/run some read/write tests with
your patch which worked fine for me too.

Reviewed-by: Phillip Potter <phil@philpotter.co.uk>

One final question from me though, and a purely procedural one:
The patch was submitted from your gmail address, but signed off by your
corporate address. Are you happy for me to adjust the submission so that
the author appears as your corporate address when sending on for
inclusion? Let me know, thanks.

Regards,
Phil