Re: [Qemu-devel] [PATCH v1 1/3] scsi-block: emulate missing Block Limits response

[Paolo Bonzini]

On 08/06/2018 22:07, Daniel Henrique Barboza wrote:
> +    unsigned int unmap_sectors = s->conf.discard_granularity / s->blocksize;
> +    unsigned int min_io_size = s->conf.min_io_size / s->blocksize;
> +    unsigned int opt_io_size = s->conf.opt_io_size / s->blocksize;
> +    unsigned int max_unmap_sectors = DEFAULT_MAX_UNMAP_SIZE / s->blocksize;
> +    unsigned int max_io_sectors =  DEFAULT_MAX_IO_SIZE / s->blocksize;
> +
> +    int max_transfer_blk = blk_get_max_transfer(s->conf.blk);
> +    int max_io_sectors_blk = max_transfer_blk / s->blocksize;
> +
> +    max_io_sectors =  MIN_NON_ZERO(max_io_sectors_blk, max_io_sectors);
> +
> +    /* min_io_size and opt_io_size can't be greater than max_io_sectors */
> +    if (min_io_size) {
> +        min_io_size = MIN(min_io_size, max_io_sectors);
> +    }
> +    if (opt_io_size) {
> +        opt_io_size = MIN(opt_io_size, max_io_sectors);
> +    }
> +
> +    /* required VPD size with unmap support */
> +    buflen = 0x40;
> +    memset(outbuf + 4, 0, buflen - 4);
> +
> +    outbuf[4] = 0x1; /* wsnz */
> +
> +    /* optimal transfer length granularity */
> +    outbuf[6] = (min_io_size >> 8) & 0xff;
> +    outbuf[7] = min_io_size & 0xff;
> +
> +    /* maximum transfer length */
> +    outbuf[8] = (max_io_sectors >> 24) & 0xff;
> +    outbuf[9] = (max_io_sectors >> 16) & 0xff;
> +    outbuf[10] = (max_io_sectors >> 8) & 0xff;
> +    outbuf[11] = max_io_sectors & 0xff;
> +
> +    /* optimal transfer length */
> +    outbuf[12] = (opt_io_size >> 24) & 0xff;
> +    outbuf[13] = (opt_io_size >> 16) & 0xff;
> +    outbuf[14] = (opt_io_size >> 8) & 0xff;
> +    outbuf[15] = opt_io_size & 0xff;
> +
> +    /* max unmap LBA count, default is 1GB */
> +    outbuf[20] = (max_unmap_sectors >> 24) & 0xff;
> +    outbuf[21] = (max_unmap_sectors >> 16) & 0xff;
> +    outbuf[22] = (max_unmap_sectors >> 8) & 0xff;
> +    outbuf[23] = max_unmap_sectors & 0xff;
> +
> +    /* max unmap descriptors, 255 fit in 4 kb with an 8-byte header.  */
> +    outbuf[24] = 0;
> +    outbuf[25] = 0;
> +    outbuf[26] = 0;
> +    outbuf[27] = 255;
> +
> +    /* optimal unmap granularity */
> +    outbuf[28] = (unmap_sectors >> 24) & 0xff;
> +    outbuf[29] = (unmap_sectors >> 16) & 0xff;
> +    outbuf[30] = (unmap_sectors >> 8) & 0xff;
> +    outbuf[31] = unmap_sectors & 0xff;
> +
> +    /* max write same size */
> +    outbuf[36] = 0;
> +    outbuf[37] = 0;
> +    outbuf[38] = 0;
> +    outbuf[39] = 0;
> +
> +    outbuf[40] = (max_io_sectors >> 24) & 0xff;
> +    outbuf[41] = (max_io_sectors >> 16) & 0xff;
> +    outbuf[42] = (max_io_sectors >> 8) & 0xff;
> +    outbuf[43] = max_io_sectors & 0xff;
> +
> +    /* done with EVPD */
> +    assert(buflen - start <= 255);
> +    outbuf[start - 1] = buflen - start;
> +
> +    return buflen;
> +}
> +

Please share this code with the existing block limits stuff in scsi-disk.c.

Also please move the inquiry handling to a separate function, since
you're adding more stuff to it in patch 2.

In fact, I would structure the series like this: patch 1 is just
refactoring, everything else can be done in a single patch.

Paolo