Re: [PATCH hci-dev 3/6] scsi-disk: refresh the cached capacity from the resize callback #VSTOR-130961

[no-reply@sashiko.virtuozzo.com]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] scsi-disk: scsi_disk_refresh_max_lba() leaves max_lba 0 for a zero-size backend
--

commit 0d24fb2b136b4040fa253df9cd5bb8b0c4064335
Author: Denis V. Lunev <den@openvz.org>

scsi-disk: refresh the cached capacity from the resize callback

This commit moves the max_lba computation out of scsi_disk_reset() into a
new helper, scsi_disk_refresh_max_lba(). It then calls the helper from
scsi_disk_resize_cb() so the cached capacity follows a backend resize.

Message-ID: <20260812121500.1034178-3-den@openvz.org>

> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c

[ ... ]

> +static void scsi_disk_refresh_max_lba(SCSIDiskState *s)
>  {
>      uint64_t nb_sectors;
>  
>      blk_get_geometry(s->qdev.conf.blk, &nb_sectors);
>  
>      nb_sectors /= s->qdev.blocksize / BDRV_SECTOR_SIZE;
>      if (nb_sectors) {
>          nb_sectors--;
>      }
>      s->qdev.max_lba = nb_sectors;
> +}

[Severity: Low]
Can max_lba end up describing a one-block device when the backend has zero
sectors?

blk_get_geometry() maps a negative blk_nb_sectors() to 0. The helper only
decrements when nb_sectors is non-zero, so max_lba stays 0 in that case.

check_lba_range() then evaluates a request for LBA 0 with length 1 as:

    sector_num + nb_sectors <= s->qdev.max_lba + 1

which is 0 + 1 <= 0 + 1, so the request is accepted. It is not refused
with LOGICAL BLOCK ADDRESS OUT OF RANGE.

The guard in scsi_disk_dma_command() does not cover this:

    if (!blk_is_available(s->qdev.conf.blk)) {
        scsi_check_condition(r, SENSE_CODE(NO_MEDIUM));
        return 0;
    }

It only tests for a missing medium, not a present medium that was resized
to 0 sectors.

The READ CAPACITY handling in scsi_disk_emulate_command_complete() takes a
different approach for a zero-size backend:

    blk_get_geometry(s->qdev.conf.blk, &nb_sectors);
    if (!nb_sectors) {
        scsi_check_condition(r, SENSE_CODE(LUN_NOT_READY));
        return 0;
    }

That path returns LUN_NOT_READY and does not touch max_lba. The new helper
has no equivalent check.

The zero-size handling was already in scsi_disk_reset(). This patch newly
applies it at runtime from scsi_disk_resize_cb(), for example after a
resize to 0 sectors or a failed size query during a replace.

I did not check how the block layer treats the request after it passes
check_lba_range(). The result is at least a wrong sense code, and possibly
an error from the backend.

Should the helper handle the zero-sector case separately, so that
check_lba_range() refuses every request in that state?

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261104134539.1519657-1-den@openvz.org?part=3