Patchwork [5/7] scsi-disk: more assertions and resets for aiocb

login
register
mail settings
Submitter Paolo Bonzini
Date Aug. 9, 2012, 1:38 p.m.
Message ID <1344519511-18147-6-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/176132/
State New
Headers show

Comments

Paolo Bonzini - Aug. 9, 2012, 1:38 p.m.
Leaving the aiocb to a non-NULL value leads to an assertion failure when
rerror/werror are set to stop or enospc, and the operation is retried.
scsi-disk checks that the aiocb member is NULL before filling it.

This patch correctly resets the aiocb to NULL values everywhere,
and adds the dual assertion that the aiocb was non-NULL before
calling bdrv_acct_done.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi-disk.c | 16 ++++++++--------
 1 file modificato, 8 inserzioni(+), 8 rimozioni(-)
Michael Tokarev - Aug. 9, 2012, 5:16 p.m.
On 09.08.2012 17:38, Paolo Bonzini wrote:
> Leaving the aiocb to a non-NULL value leads to an assertion failure when
> rerror/werror are set to stop or enospc, and the operation is retried.
> scsi-disk checks that the aiocb member is NULL before filling it.
> 
> This patch correctly resets the aiocb to NULL values everywhere,
> and adds the dual assertion that the aiocb was non-NULL before
> calling bdrv_acct_done.

Stable matherial?

Thanks,

/mjt
Paolo Bonzini - Aug. 9, 2012, 6:07 p.m.
Il 09/08/2012 19:16, Michael Tokarev ha scritto:
>> > Leaving the aiocb to a non-NULL value leads to an assertion failure when
>> > rerror/werror are set to stop or enospc, and the operation is retried.
>> > scsi-disk checks that the aiocb member is NULL before filling it.
>> > 
>> > This patch correctly resets the aiocb to NULL values everywhere,
>> > and adds the dual assertion that the aiocb was non-NULL before
>> > calling bdrv_acct_done.
> Stable matherial?

Possibly, but quite unlikely to trigger (since this would happen only on
flushes, basically).

Paolo

Patch

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index a9c7279..3baa238 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -175,6 +175,8 @@  static void scsi_aio_complete(void *opaque, int ret)
     SCSIDiskReq *r = (SCSIDiskReq *)opaque;
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
 
+    assert(r->req.aiocb != NULL);
+    r->req.aiocb = NULL;
     bdrv_acct_done(s->qdev.conf.bs, &r->acct);
 
     if (ret < 0) {
@@ -238,10 +240,9 @@  static void scsi_dma_complete(void *opaque, int ret)
     SCSIDiskReq *r = (SCSIDiskReq *)opaque;
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
 
-    if (r->req.aiocb != NULL) {
-        r->req.aiocb = NULL;
-        bdrv_acct_done(s->qdev.conf.bs, &r->acct);
-    }
+    assert(r->req.aiocb != NULL);
+    r->req.aiocb = NULL;
+    bdrv_acct_done(s->qdev.conf.bs, &r->acct);
 
     if (ret < 0) {
         if (scsi_handle_rw_error(r, -ret)) {
@@ -270,10 +271,9 @@  static void scsi_read_complete(void * opaque, int ret)
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
     int n;
 
-    if (r->req.aiocb != NULL) {
-        r->req.aiocb = NULL;
-        bdrv_acct_done(s->qdev.conf.bs, &r->acct);
-    }
+    assert(r->req.aiocb != NULL);
+    r->req.aiocb = NULL;
+    bdrv_acct_done(s->qdev.conf.bs, &r->acct);
 
     if (ret < 0) {
         if (scsi_handle_rw_error(r, -ret)) {