Replace manual lock()/unlock() calls with lock guard macros
(QEMU_LOCK_GUARD/WITH_QEMU_LOCK_GUARD) in block/iscsi.c.

Signed-off-by: Gan Qixin <ganqixin@huawei.com>
---
 block/iscsi.c | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index e30a7e3606..f5f657b582 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -322,7 +322,7 @@ iscsi_aio_cancel(BlockAIOCB *blockacb)
     IscsiAIOCB *acb = (IscsiAIOCB *)blockacb;
     IscsiLun *iscsilun = acb->iscsilun;
 
-    qemu_mutex_lock(&iscsilun->mutex);
+    QEMU_LOCK_GUARD(&iscsilun->mutex);
 
     /* If it was cancelled or completed already, our work is done here */
     if (acb->cancelled || acb->status != -EINPROGRESS) {
@@ -339,8 +339,6 @@ iscsi_aio_cancel(BlockAIOCB *blockacb)
                                          iscsi_abort_task_cb, acb) < 0) {
         qemu_aio_unref(acb); /* since iscsi_abort_task_cb() won't be called */
     }
-
-    qemu_mutex_unlock(&iscsilun->mutex);
 }
 
 static const AIOCBInfo iscsi_aiocb_info = {
@@ -375,22 +373,22 @@ static void iscsi_timed_check_events(void *opaque)
 {
     IscsiLun *iscsilun = opaque;
 
-    qemu_mutex_lock(&iscsilun->mutex);
+    WITH_QEMU_LOCK_GUARD(&iscsilun->mutex) {
+        /* check for timed out requests */
+        iscsi_service(iscsilun->iscsi, 0);
 
-    /* check for timed out requests */
-    iscsi_service(iscsilun->iscsi, 0);
+        if (iscsilun->request_timed_out) {
+            iscsilun->request_timed_out = false;
+            iscsi_reconnect(iscsilun->iscsi);
+        }
 
-    if (iscsilun->request_timed_out) {
-        iscsilun->request_timed_out = false;
-        iscsi_reconnect(iscsilun->iscsi);
+        /*
+         * newer versions of libiscsi may return zero events. Ensure we are
+         * able to return to service once this situation changes.
+         */
+        iscsi_set_events(iscsilun);
     }
 
-    /* newer versions of libiscsi may return zero events. Ensure we are able
-     * to return to service once this situation changes. */
-    iscsi_set_events(iscsilun);
-
-    qemu_mutex_unlock(&iscsilun->mutex);
-
     timer_mod(iscsilun->event_timer,
               qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + EVENT_INTERVAL);
 }
-- 
2.23.0

Am 09.11.2020 um 16:43 hat Gan Qixin geschrieben:
> Replace manual lock()/unlock() calls with lock guard macros
> (QEMU_LOCK_GUARD/WITH_QEMU_LOCK_GUARD) in block/iscsi.c.
> 
> Signed-off-by: Gan Qixin <ganqixin@huawei.com>
> ---
>  block/iscsi.c | 28 +++++++++++++---------------
>  1 file changed, 13 insertions(+), 15 deletions(-)
> 
> diff --git a/block/iscsi.c b/block/iscsi.c
> index e30a7e3606..f5f657b582 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -322,7 +322,7 @@ iscsi_aio_cancel(BlockAIOCB *blockacb)
>      IscsiAIOCB *acb = (IscsiAIOCB *)blockacb;
>      IscsiLun *iscsilun = acb->iscsilun;
>  
> -    qemu_mutex_lock(&iscsilun->mutex);
> +    QEMU_LOCK_GUARD(&iscsilun->mutex);
>  
>      /* If it was cancelled or completed already, our work is done here */
>      if (acb->cancelled || acb->status != -EINPROGRESS) {
           qemu_mutex_unlock(&iscsilun->mutex);
           return;
       }

I don't think this qemu_mutex_unlock() is right any more now.

Kevin

> -----Original Message-----
> From: Kevin Wolf [mailto:kwolf@redhat.com]
> Sent: Wednesday, December 2, 2020 7:12 PM
> To: ganqixin <ganqixin@huawei.com>
> Cc: qemu-devel@nongnu.org; qemu-trivial@nongnu.org;
> pbonzini@redhat.com; mreitz@redhat.com; stefanha@redhat.com;
> dnbrdsky@gmail.com; Zhanghailiang <zhang.zhanghailiang@huawei.com>;
> Chenqun (kuhn) <kuhn.chenqun@huawei.com>
> Subject: Re: [PATCH 4/4] block/iscsi.c: Use lock guard macros
> 
> Am 09.11.2020 um 16:43 hat Gan Qixin geschrieben:
> > Replace manual lock()/unlock() calls with lock guard macros
> > (QEMU_LOCK_GUARD/WITH_QEMU_LOCK_GUARD) in block/iscsi.c.
> >
> > Signed-off-by: Gan Qixin <ganqixin@huawei.com>
> > ---
> >  block/iscsi.c | 28 +++++++++++++---------------
> >  1 file changed, 13 insertions(+), 15 deletions(-)
> >
> > diff --git a/block/iscsi.c b/block/iscsi.c index
> > e30a7e3606..f5f657b582 100644
> > --- a/block/iscsi.c
> > +++ b/block/iscsi.c
> > @@ -322,7 +322,7 @@ iscsi_aio_cancel(BlockAIOCB *blockacb)
> >      IscsiAIOCB *acb = (IscsiAIOCB *)blockacb;
> >      IscsiLun *iscsilun = acb->iscsilun;
> >
> > -    qemu_mutex_lock(&iscsilun->mutex);
> > +    QEMU_LOCK_GUARD(&iscsilun->mutex);
> >
> >      /* If it was cancelled or completed already, our work is done here */
> >      if (acb->cancelled || acb->status != -EINPROGRESS) {
>            qemu_mutex_unlock(&iscsilun->mutex);
>            return;
>        }
> 
> I don't think this qemu_mutex_unlock() is right any more now.

You are right, I ignored this qemu_mutex_unlock(). I will correct it and resubmit. :)

Thanks,
Gan Qixin

> 
> Kevin