Re: [Qemu-devel] [RFC] scsi-disk: add -device scsi-disk, slow=on property

[Stefan Hajnoczi <stefanha@redhat.com>]

On Tue, Oct 13, 2015 at 05:31:27PM +0300, Denis V. Lunev wrote:
> On 10/13/2015 05:10 PM, Stefan Hajnoczi wrote:
> >The 'slow' bool property simulates an 8 second delay in responding to
> >simple SCSI commands that do not transfer data.
> >
> >This means non-READ/WRITE commands will take 8 seconds to complete so
> >slow SCSI LUNs can be simulated.
> >
> >Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> >---
> >This is a quick hack and probably buggy too.  Not worth merging, but I wanted
> >to archive it on the mailing list in case someone wants to use it for debugging
> >in the future.
> >
> >  hw/scsi/scsi-disk.c | 28 +++++++++++++++++++++++++++-
> >  1 file changed, 27 insertions(+), 1 deletion(-)
> >
> >diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> >index bada9a7..3aaa215 100644
> >--- a/hw/scsi/scsi-disk.c
> >+++ b/hw/scsi/scsi-disk.c
> >@@ -88,6 +88,9 @@ struct SCSIDiskState
> >      char *product;
> >      bool tray_open;
> >      bool tray_locked;
> >+    bool is_slow;
> >+    QEMUTimer *slow_timer;
> >+    SCSIDiskReq *slow_req;
> >  };
> >  static int scsi_handle_rw_error(SCSIDiskReq *r, int error);
> >@@ -1833,6 +1836,17 @@ static void scsi_disk_emulate_write_data(SCSIRequest *req)
> >      }
> >  }
> >+static void scsi_disk_slow_timer_cb(void *opaque)
> >+{
> >+    SCSIDiskState *s = opaque;
> >+    SCSIDiskReq *r = s->slow_req;
> >+
> >+    s->slow_req = NULL;
> >+
> >+    fprintf(stderr, "%s called\n", __func__);
> >+    scsi_req_complete(&r->req, GOOD);
> >+}
> >+
> >  static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf)
> >  {
> >      SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
> >@@ -2092,7 +2106,15 @@ static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf)
> >      assert(!r->req.aiocb);
> >      r->iov.iov_len = MIN(r->buflen, req->cmd.xfer);
> >      if (r->iov.iov_len == 0) {
> >-        scsi_req_complete(&r->req, GOOD);
> >+        if (s->is_slow) {
> >+            fprintf(stderr, "delaying scsi_req_complete...\n");
> >+            assert(!s->slow_req);
> 
> general note.
> do we really such fprintf's?

This is just a hack for debugging, it's very useful to see when the
request is held back and when it completes.

> >+            s->slow_req = r;
> >+            timer_mod(s->slow_timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
> >+                                     8 * 1000ULL * 1000ULL * 1000ULL);
> 
> AFAIK SCSI has a FIFO for commands thus in this case the
> request that is already in the queue will be lagged
> additionally
> 
> This is a simple probably lame opinion for the question.
> 
> From my POW you should follow the way I have used in
> the deadline series. You should queue the request into
> the slow queue and start the timer. In timer callback
> you should complete expired requests and reschedule
> the timer with appropriate timeout.

I think you are right that this is not a good way to delay requests.  My
other thought was a per-request timer instead of per-disk timer, but I
didn't need it for the debugging I was doing.

Stefan