Re: [Qemu-devel] [PATCH v4 14/14] tests: add BlockJobTxn unit test

[Fam Zheng]

On Mon, 08/03 20:45, Max Reitz wrote:
> >+static void test_single_job(int expected)
> >+{
> >+    BlockJob *job;
> >+    BlockJobTxn *txn;
> >+    int result = -EINPROGRESS;
> >+
> >+    txn = block_job_txn_new();
> >+    job = test_block_job_start(1, true, expected, &result);
> 
> I think I'd like it better if 0 was passed if expected == -ECANCELED. This
> way, we don't know whether the test really was cancelled or whether it just
> exited normally, returning -ECANCELED because that was what was passed here.

The job will yield right after starting, so I think it is OK.

> >+    /* Now make job2 finish before the main loop kicks jobs.  This means
> >+     * simulates the race between a pending kick and another job completing.
> 
> Perhaps the "means" should be removed?

Yes.

> 
> Even if you don't decide to address any of my comments:
> 
> Reviewed-by: Max Reitz <address@hidden>

Thanks,

Fam