Re: [Qemu-devel] [PULL 21/25] scsi-disk: fix VERIFY for scsi-block

[Peter Maydell]

On 22 December 2016 at 15:22, Paolo Bonzini <address@hidden> wrote:
> When a scsi-disk object receives VERIFY command with BYTCHK bit being zero,
> scsi_block_is_passthrough returns false and finally makes req being proceeded
> by scsi_block_dma_command. Because scsi_block_dma_command has removed process
> of VERIFY, QEMU will abort in this function.
>
> Reported-by: Junlian Bell <address@hidden>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  hw/scsi/scsi-disk.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index a963191..bdd1e5f 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c
> @@ -2157,6 +2157,13 @@ static int32_t scsi_disk_dma_command(SCSIRequest *req, 
> uint8_t *buf)
>          DPRINTF("Write %s(sector %" PRId64 ", count %u)\n",
>                  (command & 0xe) == 0xe ? "And Verify " : "",
>                  r->req.cmd.lba, len);
> +    case VERIFY_10:
> +    case VERIFY_12:
> +    case VERIFY_16:
> +        /* We get here only for BYTCHK == 0x01 and only for scsi-block.
> +         * As far as DMA is concerned, we can treat it the same as a write;
> +         * scsi_block_do_sgio will send VERIFY commands.
> +         */
>          if (r->req.cmd.buf[1] & 0xe0) {
>              goto illegal_request;
>          }

Coverity now complains about the fallthrough into the case statements
added here. I assume from the patch that the fallthrough is deliberate,
in which case a /* fall through */ comment would assist future
readers...

thanks
-- PMM