[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 09/11] block: drive_backup transaction callba
From: |
John Snow |
Subject: |
Re: [Qemu-devel] [PATCH v4 09/11] block: drive_backup transaction callback support |
Date: |
Mon, 18 May 2015 11:53:55 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 |
On 05/18/2015 11:35 AM, Stefan Hajnoczi wrote:
> On Mon, May 11, 2015 at 07:04:24PM -0400, John Snow wrote:
>> +static void drive_backup_cb(BlkActionState *common) +{ +
>> BlkActionCallbackData *cb_data = common->cb_data; +
>> BlockDriverState *bs = cb_data->opaque; + DriveBackupState
>> *state = DO_UPCAST(DriveBackupState, common, common); + +
>> assert(state->bs == bs); + if (bs->job) { +
>> assert(state->job == bs->job); + }
>
> What is the purpose of the if statement?
>
> Why is it not okay for a new job to have started?
>
Hmm, maybe it's fine -- It was just my thought that it probably
/shouldn't/ occur under normal circumstances.
I think my assumption was that we want to impose an ordering that job
cleanup occurs before another job launches, in general.
I think, though, that you wanted to start allowing non-conflicting
jobs to run concurrently, though, so I'll just eye over this series
again to make sure it's okay for cleanup to happen after another job
starts ...
...Provided the second job does not fiddle with bitmaps, of course. We
should clean those up before another bitmap job starts, definitely.
>> + + state->aio_context = bdrv_get_aio_context(bs); +
>> aio_context_acquire(state->aio_context);
>
> The bs->job access above should be protected by
> aio_context_acquire().
>
Thanks,
--js
- [Qemu-devel] [PATCH v4 03/11] block: rename BlkTransactionState and BdrvActionOps, (continued)
- [Qemu-devel] [PATCH v4 03/11] block: rename BlkTransactionState and BdrvActionOps, John Snow, 2015/05/11
- [Qemu-devel] [PATCH v4 07/11] block: add delayed bitmap successor cleanup, John Snow, 2015/05/11
- [Qemu-devel] [PATCH v4 08/11] qmp: Add an implementation wrapper for qmp_drive_backup, John Snow, 2015/05/11
- [Qemu-devel] [PATCH v4 05/11] block: add transactional callbacks feature, John Snow, 2015/05/11
- [Qemu-devel] [PATCH v4 10/11] iotests: 124 - transactional failure test, John Snow, 2015/05/11
- [Qemu-devel] [PATCH v4 09/11] block: drive_backup transaction callback support, John Snow, 2015/05/11
- [Qemu-devel] [PATCH v4 11/11] qmp-commands.hx: Update the supported 'transaction' operations, John Snow, 2015/05/11