[PATCH] block/qcow2-cluster: remove dead code

[Elena Afanasova]

Since handle_dependencies() returns 0 or -EAGAIN the following case can be 
removed.
Spotted by PVS-Studio.

Signed-off-by: Elena Afanasova 
---
 block/qcow2-cluster.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index aa87d3e99b..e2e0db0cc9 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1806,8 +1806,6 @@ again:
  * structs before starting over. */
 assert(*m == NULL);
 goto again;
-} else if (ret < 0) {
-return ret;
 } else if (cur_bytes == 0) {
 break;
 } else {
-- 
2.25.1

[PATCH] hw/scsi: remove dead code

[Elena Afanasova]

Since MEGASAS_MAX_SGE is defined to be 128 the following if statement can be 
removed.
Spotted by PVS-Studio.

Signed-off-by: Elena Afanasova 
---
 hw/scsi/megasas.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index e24c12d7ee..6dcaad55fa 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -2393,8 +2393,6 @@ static void megasas_scsi_realize(PCIDevice *dev, Error 
**errp)
 }
 if (s->fw_sge >= MEGASAS_MAX_SGE - MFI_PASS_FRAME_SIZE) {
 s->fw_sge = MEGASAS_MAX_SGE - MFI_PASS_FRAME_SIZE;
-} else if (s->fw_sge >= 128 - MFI_PASS_FRAME_SIZE) {
-s->fw_sge = 128 - MFI_PASS_FRAME_SIZE;
 } else {
 s->fw_sge = 64 - MFI_PASS_FRAME_SIZE;
 }
-- 
2.25.1

[PATCH] block/blkdebug: fix memory leak

[Elena Afanasova]

Spotted by PVS-Studio

Signed-off-by: Elena Afanasova 
---
 block/blkdebug.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index eecbf3e5c4..54da719dd1 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -215,6 +215,7 @@ static int add_rule(void *opaque, QemuOpts *opts, Error 
**errp)
  BLKDEBUG_IO_TYPE__MAX, _error);
 if (local_error) {
 error_propagate(errp, local_error);
+g_free(rule);
 return -1;
 }
 if (iotype != BLKDEBUG_IO_TYPE__MAX) {
-- 
2.25.1

Re: [PATCH] job: delete job_{lock,unlock} functions and replace them with lock guard

[Elena Afanasova]

On Tue, 2020-09-29 at 14:04 -0400, John Snow wrote:
> On 9/29/20 9:42 AM, Elena Afanasova wrote:
> > Signed-off-by: Elena Afanasova 
> 
> Hi, can I have a commit message here, please?
> 
> > ---
> >   job.c | 46 +-
> >   1 file changed, 17 insertions(+), 29 deletions(-)
> > 
> > diff --git a/job.c b/job.c
> > index 8fecf38960..89ceb53434 100644
> > --- a/job.c
> > +++ b/job.c
> > @@ -79,16 +79,6 @@ struct JobTxn {
> >* job_enter. */
> >   static QemuMutex job_mutex;
> >   
> > -static void job_lock(void)
> > -{
> > -qemu_mutex_lock(_mutex);
> > -}
> > -
> > -static void job_unlock(void)
> > -{
> > -qemu_mutex_unlock(_mutex);
> > -}
> > -
> >   static void __attribute__((__constructor__)) job_init(void)
> >   {
> >   qemu_mutex_init(_mutex);
> > @@ -437,21 +427,19 @@ void job_enter_cond(Job *job, bool(*fn)(Job
> > *job))
> >   return;
> >   }
> >   
> > -job_lock();
> > -if (job->busy) {
> > -job_unlock();
> > -return;
> > -}
> > +WITH_QEMU_LOCK_GUARD(_mutex) {
> > +if (job->busy) {
> > +return;
> > +}
> >   
> > -if (fn && !fn(job)) {
> > -job_unlock();
> > -return;
> > -}
> > +if (fn && !fn(job)) {
> > +return;
> > +}
> >   
> > -assert(!job->deferred_to_main_loop);
> > -timer_del(>sleep_timer);
> > -job->busy = true;
> > -job_unlock();
> > +assert(!job->deferred_to_main_loop);
> > +timer_del(>sleep_timer);
> > +job->busy = true;
> > +}
> >   aio_co_enter(job->aio_context, job->co);
> >   }
> >   
> > @@ -468,13 +456,13 @@ void job_enter(Job *job)
> >* called explicitly. */
> >   static void coroutine_fn job_do_yield(Job *job, uint64_t ns)
> >   {
> > -job_lock();
> > -if (ns != -1) {
> > -timer_mod(>sleep_timer, ns);
> > +WITH_QEMU_LOCK_GUARD(_mutex) {
> > +if (ns != -1) {
> > +timer_mod(>sleep_timer, ns);
> > +}
> > +job->busy = false;
> > +job_event_idle(job);
> 
> Is this new macro safe to use in a coroutine context?

Hi, I suppose it's safe. It would be nice to get some more opinions
here.

> >   }
> > -job->busy = false;
> > -job_event_idle(job);
> > -job_unlock();
> >   qemu_coroutine_yield();
> >   
> >   /* Set by job_enter_cond() before re-entering the
> > coroutine.  */
> > 
> 
> I haven't looked into WITH_QEMU_LOCK_GUARD before, I assume it's new.
> If 
> it works like I think it does, this change seems good.
> 
> (I'm assuming it works like a Python context manager and it drops
> the 
> lock when it leaves the scope of the macro using GCC/Clang language 
> extensions.)
> 

[PATCH] job: delete job_{lock, unlock} functions and replace them with lock guard

[Elena Afanasova]

Signed-off-by: Elena Afanasova 
---
 job.c | 46 +-
 1 file changed, 17 insertions(+), 29 deletions(-)

diff --git a/job.c b/job.c
index 8fecf38960..89ceb53434 100644
--- a/job.c
+++ b/job.c
@@ -79,16 +79,6 @@ struct JobTxn {
  * job_enter. */
 static QemuMutex job_mutex;
 
-static void job_lock(void)
-{
-qemu_mutex_lock(_mutex);
-}
-
-static void job_unlock(void)
-{
-qemu_mutex_unlock(_mutex);
-}
-
 static void __attribute__((__constructor__)) job_init(void)
 {
 qemu_mutex_init(_mutex);
@@ -437,21 +427,19 @@ void job_enter_cond(Job *job, bool(*fn)(Job *job))
 return;
 }
 
-job_lock();
-if (job->busy) {
-job_unlock();
-return;
-}
+WITH_QEMU_LOCK_GUARD(_mutex) {
+if (job->busy) {
+return;
+}
 
-if (fn && !fn(job)) {
-job_unlock();
-return;
-}
+if (fn && !fn(job)) {
+return;
+}
 
-assert(!job->deferred_to_main_loop);
-timer_del(>sleep_timer);
-job->busy = true;
-job_unlock();
+assert(!job->deferred_to_main_loop);
+timer_del(>sleep_timer);
+job->busy = true;
+}
 aio_co_enter(job->aio_context, job->co);
 }
 
@@ -468,13 +456,13 @@ void job_enter(Job *job)
  * called explicitly. */
 static void coroutine_fn job_do_yield(Job *job, uint64_t ns)
 {
-job_lock();
-if (ns != -1) {
-timer_mod(>sleep_timer, ns);
+WITH_QEMU_LOCK_GUARD(_mutex) {
+if (ns != -1) {
+timer_mod(>sleep_timer, ns);
+}
+job->busy = false;
+job_event_idle(job);
 }
-job->busy = false;
-job_event_idle(job);
-job_unlock();
 qemu_coroutine_yield();
 
 /* Set by job_enter_cond() before re-entering the coroutine.  */
-- 
2.25.1