16.01.2019 19:02, Max Reitz wrote:
> On 29.12.18 13:20, Vladimir Sementsov-Ogievskiy wrote:
>> Backup-top filter does copy-before-write operation. It should be
>> inserted above active disk and has a target node for CBW, like the
>> following:
>>
>> +-------+
>> | Guest |
>> +---+---+
>> |r,w
>> v
>> +---+-----------+ target +---------------+
>> | backup_top |---------->| target(qcow2) |
>> +---+-----------+ CBW +---+-----------+
>> |
>> backing |r,w
>> v
>> +---+---------+
>> | Active disk |
>> +-------------+
>>
>> The driver will be used in backup instead of write-notifiers.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
>> ---
>> block/backup-top.h | 43 +++++++
>> block/backup-top.c | 306 ++++++++++++++++++++++++++++++++++++++++++++
>> block/Makefile.objs | 2 +
>> 3 files changed, 351 insertions(+)
>> create mode 100644 block/backup-top.h
>> create mode 100644 block/backup-top.c
>
[..]
>> +BlockDriverState *bdrv_backup_top_append(BlockDriverState *source,
>> + BlockDriverState *target,
>> + HBitmap *copy_bitmap,
>> + Error **errp)
>> +{
>> + Error *local_err = NULL;
>> + BDRVBackupTopState *state;
>> + BlockDriverState *top = bdrv_new_open_driver(&bdrv_backup_top_filter,
>> + NULL, BDRV_O_RDWR, errp);
>> +
>> + if (!top) {
>> + return NULL;
>> + }
>> +
>> + top->implicit = true;
>> + top->total_sectors = source->total_sectors;
>> + top->opaque = state = g_new0(BDRVBackupTopState, 1);
>> + state->copy_bitmap = copy_bitmap;
>> +
>> + bdrv_ref(target);
>> + state->target = bdrv_attach_child(top, target, "target", &child_file,
>> errp);
>> + if (!state->target) {
>> + bdrv_unref(target);
>> + bdrv_unref(top);
>> + return NULL;
>> + }
>> +
>> + bdrv_set_aio_context(top, bdrv_get_aio_context(source));
>> + bdrv_set_aio_context(target, bdrv_get_aio_context(source));
>> +
>> + bdrv_drained_begin(source);
>> +
>> + bdrv_ref(top);
>> + bdrv_append(top, source, &local_err);
>> +
>> + if (local_err) {
>> + bdrv_unref(top);
>
> This is done automatically by bdrv_append().
>
>> + }
>> +
>> + bdrv_drained_end(source);
>> +
>> + if (local_err) {
>> + bdrv_unref_child(top, state->target);
>> + bdrv_unref(top);
>> + error_propagate(errp, local_err);
>> + return NULL;
>> + }
>> +
>> + return top;
>> +}
>> +
>> +void bdrv_backup_top_drop(BlockDriverState *bs)
>> +{
>> + BDRVBackupTopState *s = bs->opaque;
>> +
>> + AioContext *aio_context = bdrv_get_aio_context(bs);
>> +
>> + aio_context_acquire(aio_context);
>> +
>> + bdrv_drained_begin(bs);
>> +
>> + bdrv_child_try_set_perm(bs->backing, 0, BLK_PERM_ALL, &error_abort);
>> + bdrv_replace_node(bs, backing_bs(bs), &error_abort);
>> + bdrv_set_backing_hd(bs, NULL, &error_abort);
>
> This is done automatically in bdrv_close(), and after bs has been
> replaced by backing_bs(bs), I don't think new requests should come in,
> so I don't think this needs to be done here.
Following movement of backup_top back to job->blk becomes impossible then,
if we don't share WRITE on source in backup_top_child_perm.
And I think, this function should drop all relations created by
bdrv_backup_top_append.
>
>> +
>> + bdrv_drained_end(bs);
>> +
>> + if (s->target) {
>> + bdrv_unref_child(bs, s->target);
>> + }
>
> And this should be done in a .bdrv_close() implementation, I think.
>
> Max
>
>> + bdrv_unref(bs);
>> +
>> + aio_context_release(aio_context);
>> +}
>> +
--
Best regards,
Vladimir
