From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45832) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZmMNP-00057r-RK for qemu-devel@nongnu.org; Wed, 14 Oct 2015 09:45:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZmMNL-0006ss-Nr for qemu-devel@nongnu.org; Wed, 14 Oct 2015 09:45:55 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42364) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZmMNL-0006sj-Fw for qemu-devel@nongnu.org; Wed, 14 Oct 2015 09:45:51 -0400 Date: Wed, 14 Oct 2015 15:45:48 +0200 From: Stefan Hajnoczi Message-ID: <20151014134548.GA16162@stefanha-thinkpad.redhat.com> References: <1444745415-21718-1-git-send-email-stefanha@redhat.com> <561D15BF.3070004@parallels.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <561D15BF.3070004@parallels.com> Subject: Re: [Qemu-devel] [RFC] scsi-disk: add -device scsi-disk, slow=on property List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Denis V. Lunev" Cc: Paolo Bonzini , Fam Zheng , qemu-devel@nongnu.org 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 > >--- > >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