Re: [PATCH] scsi-generic: replace logical block count of response of READ CAPACITY

[Claudio Fontana]

Hi Paolo, Hannes,

any thoughts on the following issue?

Introduction:

When using SAN storage for providing block devices to guests, configured as 
SCSI-passthrough devices, increasing the space available in the VM is a use 
case.

To do it, it is currently necessary to:

1) expand storage on the actual SAN,
2) run a "virsh blockresize" or equivalent command to make QEMU aware of the 
new size, and finally
3) do a "rescan-scsi-bus.sh" or equivalent operation in the guest to make the 
running guest aware of the increased disk size.

The problem:

As of now, the administrator needs to make sure that step 3 won't be done 
before step 2 has been executed, or the resulting state will be inconsistent.
In practice this creates organizational issues to try to sync up host/storage 
admins and guest OS admin, and is therefore error prone (due to these human 
factors).

The proposal:

The patch I replied to here from Ma Lin tries to avoid the inconsistent state, 
by having "rescan-scsi-bus.sh" still report the old size in the guest until 
QEMU itself is aware of the new disk size.

The patch:

Before the patch, the SCSI READ_CAPACITY command in the guest os directly 
receives the unmodified response from the storage backend.

After the patch, QEMU intercepts the READ_CAPACITY response and replaces the 
maximum LBA with the information which is saved in QEMU.

This means: after resizing the storage on the SAN backend, the host 
administrator must explicitly notify about CAPACITY HAS CHANGED by issuing a 
block resize command through QMP or libvirt,
even for SCSI passthrough disks.

Any ideas on this patch or on possible alternatives?

Thanks,

Claudio


On 11/20/21 11:15 AM, Lin Ma wrote:
> While using SCSI passthrough, Following scenario makes qemu doesn't
> realized the capacity change of remote scsi target:
> 1. online resize the scsi target.
> 2. issue 'rescan-scsi-bus.sh -s ...' in host.
> 3. issue 'rescan-scsi-bus.sh -s ...' in vm.
> 
> In above scenario I used to experienced errors while accessing the
> additional disk space in vm. I think the reasonable operations should
> be:
> 1. online resize the scsi target.
> 2. issue 'rescan-scsi-bus.sh -s ...' in host.
> 3. issue 'block_resize' via qmp to notify qemu.
> 4. issue 'rescan-scsi-bus.sh -s ...' in vm.
> 
> The errors disappear once I notify qemu by block_resize via qmp.
> 
> So this patch replaces the number of logical blocks of READ CAPACITY
> response from scsi target by qemu's bs->total_sectors. If the user in
> vm wants to access the additional disk space, The administrator of
> host must notify qemu once resizeing the scsi target.
> 
> Bonus is that domblkinfo of libvirt can reflect the consistent capacity
> information between host and vm in case of missing block_resize in qemu.
> E.g:
> ...
>     <disk type='block' device='lun'>
>       <driver name='qemu' type='raw'/>
>       <source dev='/dev/sdc' index='1'/>
>       <backingStore/>
>       <target dev='sda' bus='scsi'/>
>       <alias name='scsi0-0-0-0'/>
>       <address type='drive' controller='0' bus='0' target='0' unit='0'/>
>     </disk>
> ...
> 
> Before:
> 1. online resize the scsi target.
> 2. host:~  # rescan-scsi-bus.sh -s /dev/sdc
> 3. guest:~ # rescan-scsi-bus.sh -s /dev/sda
> 4  host:~  # virsh domblkinfo --domain $DOMAIN --human --device sda
> Capacity:       4.000 GiB
> Allocation:     0.000 B
> Physical:       8.000 GiB
> 
> 5. guest:~ # lsblk /dev/sda
> NAME   MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
> sda      8:0    0   8G  0 disk
> └─sda1   8:1    0   2G  0 part
> 
> After:
> 1. online resize the scsi target.
> 2. host:~  # rescan-scsi-bus.sh -s /dev/sdc
> 3. guest:~ # rescan-scsi-bus.sh -s /dev/sda
> 4  host:~  # virsh domblkinfo --domain $DOMAIN --human --device sda
> Capacity:       4.000 GiB
> Allocation:     0.000 B
> Physical:       8.000 GiB
> 
> 5. guest:~ # lsblk /dev/sda
> NAME   MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
> sda      8:0    0   4G  0 disk
> └─sda1   8:1    0   2G  0 part
> 
> Signed-off-by: Lin Ma <lma@suse.com>
> ---
>  hw/scsi/scsi-generic.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
> index 0306ccc7b1..343b51c2c0 100644
> --- a/hw/scsi/scsi-generic.c
> +++ b/hw/scsi/scsi-generic.c
> @@ -315,11 +315,17 @@ static void scsi_read_complete(void * opaque, int ret)
>      if (r->req.cmd.buf[0] == READ_CAPACITY_10 &&
>          (ldl_be_p(&r->buf[0]) != 0xffffffffU || s->max_lba == 0)) {
>          s->blocksize = ldl_be_p(&r->buf[4]);
> -        s->max_lba = ldl_be_p(&r->buf[0]) & 0xffffffffULL;
> +        BlockBackend *blk = s->conf.blk;
> +        BlockDriverState *bs = blk_bs(blk);
> +        s->max_lba = bs->total_sectors - 1;
> +        stl_be_p(&r->buf[0], s->max_lba);
>      } else if (r->req.cmd.buf[0] == SERVICE_ACTION_IN_16 &&
>                 (r->req.cmd.buf[1] & 31) == SAI_READ_CAPACITY_16) {
>          s->blocksize = ldl_be_p(&r->buf[8]);
> -        s->max_lba = ldq_be_p(&r->buf[0]);
> +        BlockBackend *blk = s->conf.blk;
> +        BlockDriverState *bs = blk_bs(blk);
> +        s->max_lba = bs->total_sectors - 1;
> +        stq_be_p(&r->buf[0], s->max_lba);
>      }
>      blk_set_guest_block_size(s->conf.blk, s->blocksize);
>  
>