[Qemu-devel] [PATCH 07/10] blockjob: strengthen a bit test-blockjob-txn

Paolo Bonzini posted 10 patches 8 years, 10 months ago
There is a newer version of this series
[Qemu-devel] [PATCH 07/10] blockjob: strengthen a bit test-blockjob-txn
Posted by Paolo Bonzini 8 years, 10 months ago
Unlike test-blockjob-txn, QMP releases the reference to the transaction
before the jobs finish.  Thus, while working on the next patch,
qemu-iotest 124 showed a failure that the unit tests did not have.
Make the unit test just a little nastier, so that it fails too.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/test-blockjob-txn.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c
index 4ccbda1..bfc2aaa 100644
--- a/tests/test-blockjob-txn.c
+++ b/tests/test-blockjob-txn.c
@@ -165,6 +165,11 @@ static void test_pair_jobs(int expected1, int expected2)
     job2 = test_block_job_start(2, true, expected2, &result2);
     block_job_txn_add_job(txn, job2);
 
+    /* Release our reference now to trigger as many nice
+     * use-after-free bugs as possible.
+     */
+    block_job_txn_unref(txn);
+
     if (expected1 == -ECANCELED) {
         block_job_cancel(job1);
     }
@@ -185,8 +190,6 @@ static void test_pair_jobs(int expected1, int expected2)
 
     g_assert_cmpint(result1, ==, expected1);
     g_assert_cmpint(result2, ==, expected2);
-
-    block_job_txn_unref(txn);
 }
 
 static void test_pair_jobs_success(void)
-- 
2.9.3



Re: [Qemu-devel] [PATCH 07/10] blockjob: strengthen a bit test-blockjob-txn
Posted by John Snow 8 years, 10 months ago

On 03/23/2017 01:39 PM, Paolo Bonzini wrote:
> Unlike test-blockjob-txn, QMP releases the reference to the transaction
> before the jobs finish.  Thus, while working on the next patch,
> qemu-iotest 124 showed a failure that the unit tests did not have.
> Make the unit test just a little nastier, so that it fails too.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  tests/test-blockjob-txn.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c
> index 4ccbda1..bfc2aaa 100644
> --- a/tests/test-blockjob-txn.c
> +++ b/tests/test-blockjob-txn.c
> @@ -165,6 +165,11 @@ static void test_pair_jobs(int expected1, int expected2)
>      job2 = test_block_job_start(2, true, expected2, &result2);
>      block_job_txn_add_job(txn, job2);
>  

^ Oh, this might cause you grief too. Should be add-add-start-start, not
start-add-start-add. Fam sent a patch fixing this recently.

> +    /* Release our reference now to trigger as many nice
> +     * use-after-free bugs as possible.
> +     */
> +    block_job_txn_unref(txn);
> +

This is fine, though.

>      if (expected1 == -ECANCELED) {
>          block_job_cancel(job1);
>      }
> @@ -185,8 +190,6 @@ static void test_pair_jobs(int expected1, int expected2)
>  
>      g_assert_cmpint(result1, ==, expected1);
>      g_assert_cmpint(result2, ==, expected2);
> -
> -    block_job_txn_unref(txn);
>  }
>  
>  static void test_pair_jobs_success(void)
> 

Reviewed-by: John Snow <jsnow@redhat.com>

Re: [Qemu-devel] [PATCH 07/10] blockjob: strengthen a bit test-blockjob-txn
Posted by Stefan Hajnoczi 8 years, 10 months ago
On Thu, Mar 23, 2017 at 06:39:25PM +0100, Paolo Bonzini wrote:
> Unlike test-blockjob-txn, QMP releases the reference to the transaction
> before the jobs finish.  Thus, while working on the next patch,
> qemu-iotest 124 showed a failure that the unit tests did not have.
> Make the unit test just a little nastier, so that it fails too.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  tests/test-blockjob-txn.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>