[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] backup bug or question
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
Re: [Qemu-devel] backup bug or question |
Date: |
Mon, 12 Aug 2019 16:09:45 +0000 |
12.08.2019 16:23, Kevin Wolf wrote:
> Am 09.08.2019 um 15:18 hat Vladimir Sementsov-Ogievskiy geschrieben:
>> Hi!
>>
>> Hmm, hacking around backup I have a question:
>>
>> What prevents guest write request after job_start but before setting
>> write notifier?
>>
>> code path:
>>
>> qmp_drive_backup or transaction with backup
>>
>> job_start
>> aio_co_enter(job_co_entry) /* may only schedule execution, isn't it
>> ? */
>>
>> ....
>>
>> job_co_entry
>> job_pause_point() /* it definitely yields, isn't it bad? */
>> job->driver->run() /* backup_run */
>>
>> ----
>>
>> backup_run()
>> bdrv_add_before_write_notifier()
>>
>> ...
>>
>> And what guarantees we give to the user? Is it guaranteed that write
>> notifier is
>> set when qmp command returns?
>>
>> And I guess, if we start several backups in a transaction it should be
>> guaranteed
>> that the set of backups is consistent and correspond to one point in time...
>
> Do the patches to switch backup to a filter node solve this
> automatically because that node would be inserted in
> backup_job_create()?
>
Hmm, great, looks like they should. At least it moves scope of the problem to
do_drive_backup
and do_blockdev_backup functions..
Am I right that aio_context_acquire/aio_context_release guarantees no new
request created during
the section? Or should we add drained_begin/drained_end pair, or at least
drain() at start of
qmp_blockdev_backup and qmp_drive_backup?
Assume scenario like the this,
1. fsfreeze
2. qmp backup
3. fsthaw
to make sure that backup starting point is consistent. So in our qmp command we
should:
1. complete all current requests to make drives corresponding to fsfreeze point
2. initialize write-notifiers or filter before any new guest request, i.e.
before fsthaw,
i.e. before qmp command return.
Transactions should be OK, as they use drained_begin/drained_end pairs, and
additional
aio_context_acquire/aio_context_release pairs.
--
Best regards,
Vladimir