Re: [PATCH RFC] hw/scsi: Add SCSI tape device emulation

["Daniel P. Berrangé" <berrange@redhat.com>]

On Thu, Apr 16, 2026 at 08:34:48PM +0300, rft0 wrote:
> Add initial emulation of SCSI tape device supporting basic
> INQUIRY operation for now.
> 
> Hi, I am a GSoC 2026 applicant and this is my first patch.
> Any feedback or corrections appreciated.

When you have any notes that are not intended for long
term preservation in git history, it is preferrable to
put them after the "---"....

> 
> Signed-off-by: rft0 <rafettaskindev@gmail.com>
> ---

...just here...

>  hw/scsi/meson.build |   1 +
>  hw/scsi/scsi-tape.c | 334 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 335 insertions(+)
>  create mode 100644 hw/scsi/scsi-tape.c
> 
> diff --git a/hw/scsi/meson.build b/hw/scsi/meson.build
> index 69fde0cf84..1699b6e591 100644
> --- a/hw/scsi/meson.build
> +++ b/hw/scsi/meson.build
> @@ -7,6 +7,7 @@ scsi_ss.add(files(
>    'scsi-bus.c',
>    'scsi-disk.c',
>    'scsi-generic.c',
> +  'scsi-tape.c',
>  ))

I wonder if we shold add a "CONFIG_SCSI_TAPE"   KConfig
flag so that downstreams can selectively enable the tape
emulation


> +static int scsi_disk_emulate_vpd_page(SCSIRequest *req, uint8_t *outbuf)
> +{
> +    SCSITapeState *s = DO_UPCAST(SCSITapeState, qdev, req->dev);
> +    uint8_t page_code = req->cmd.buf[2];
> +
> +    outbuf[0] = TYPE_TAPE;
> +    outbuf[1] = page_code;
> +    outbuf[2] = 0x00;
> +    outbuf[3] = 0x00;
> +
> +    switch (page_code) {
> +    case 0x00: /* Supported VPD pages */
> +        outbuf[4] = 0x00;   /* page 0x00 (this page) */
> +        if (s->serial) {
> +            outbuf[5] = 0x80;   /* page 0x80 (serial number) */
> +            outbuf[3] = 2;      /* page data length */
> +            return 6;
> +        }
> +        outbuf[3] = 1;
> +        return 5;
> +
> +    case 0x80: /* Unit Serial Number */
> +        if (!s->serial) {
> +            return -1;   /* not supported, caller sends INVALID_FIELD */
> +        }
> +        {
> +            int l = strlen(s->serial);
> +            if (l > 36) {
> +                l = 36;
> +            }

Rather than silently truncating it here, we should check the length
in realize() and report an error at that point, so this code can
assume the length is already within bounds.

> +            outbuf[3] = l;
> +            memcpy(&outbuf[4], s->serial, l);
> +            return 4 + l;
> +        }
> +
> +    default:
> +        return -1;  /* unsupported VPD page */
> +    }
> +}
> +
> +static int scsi_tape_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf)
> +{
> +    SCSITapeState *s = DO_UPCAST(SCSITapeState, qdev, req->dev);
> +    int buflen;
> +
> +    if (req->cmd.buf[1] & 0x1) {
> +        return scsi_disk_emulate_vpd_page(req, outbuf);
> +    }
> +
> +    /* Standard INQUIRY, not a VPD request */
> +    if (req->cmd.buf[2] != 0) {
> +        return -1;
> +    }
> +
> +    /* PAGE_CODE == 0 */
> +    buflen = req->cmd.xfer;
> +    if (buflen > SCSI_MAX_INQUIRY_LEN) {
> +        buflen = SCSI_MAX_INQUIRY_LEN;
> +    }
> +
> +    outbuf[0] = TYPE_TAPE;      /* 0x01 = Tape */
> +    outbuf[1] = 0x80;           /* Always removable */
> +    outbuf[2] = 0x05;           /* SPC-3 */
> +    outbuf[3] = 0x02 | 0x10;    /* Format 2, HiSup */
> +
> +    if (buflen > 36) {
> +        outbuf[4] = buflen - 5;
> +    } else {
> +        outbuf[4] = 36 - 5;
> +    }
> +
> +    outbuf[7] = 0x10 | (req->bus->info->tcq ? 0x02 : 0);
> +
> +    strpadcpy((char *)&outbuf[16], 16, s->product, ' ');
> +    strpadcpy((char *)&outbuf[8], 8, s->vendor, ' ');
> +
> +    memset(&outbuf[32], 0, 4);
> +    memcpy(&outbuf[32], s->version, MIN(4, strlen(s->version)));

product/vendor/version should be length checked too.

> +
> +    return buflen;
> +}
> +

> +static void scsi_tape_realize(SCSIDevice *dev, Error **errp)
> +{
> +    SCSITapeState *s = DO_UPCAST(SCSITapeState, qdev, dev);
> +
> +    dev->type = TYPE_TAPE;
> +
> +    if (!s->qdev.conf.blk) {
> +        error_setg(errp, "Drive property not set");
> +        return;
> +    }
> +
> +    if (!blk_is_inserted(s->qdev.conf.blk)) {
> +        error_setg(errp, "Device needs media, but drive is empty");
> +        return;
> +    }
> +
> +    if (!s->vendor) {
> +        s->vendor = g_strdup("QEMU");
> +    }
> +    if (!s->product) {
> +        s->product = g_strdup("QEMU TAPE");
> +    }
> +    if (!s->version) {
> +        s->version = g_strdup(QEMU_HW_VERSION);
> +    }

Here you can validate length of vendor, product, version
and serial.

> +}
> +

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|