On 11/18/2015 02:25 PM, Juan Quintela wrote:
"Denis V. Lunev" <d...@openvz.org> wrote:
basically all bdrv_* operations must be called under aio_context_acquire
except ones with bdrv_all prefix.
Signed-off-by: Denis V. Lunev <d...@openvz.org>
Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com>
Reviewed-by: Fam Zheng <f...@redhat.com>
CC: Juan Quintela <quint...@redhat.com>
CC: Kevin Wolf <kw...@redhat.com>
I will preffer that migration code don't know about aiocontexts.
@@ -2048,9 +2054,12 @@ int load_vmstate(const char *name)
error_report("No block device supports snapshots");
return -ENOTSUP;
}
+ aio_context = bdrv_get_aio_context(bs);
/* Don't even try to load empty VM states */
+ aio_context_acquire(aio_context);
ret = bdrv_snapshot_find(bs_vm_state, &sn, name);
+ aio_context_release(aio_context);
Why are we dropping it here?
I can understand doing it on the error case.
acquire == lock
release == unlock
The lock should be dropped....
if (ret < 0) {
return ret;
} else if (sn.vm_state_size == 0) {
@@ -2078,9 +2087,12 @@ int load_vmstate(const char *name)
qemu_system_reset(VMRESET_SILENT);
migration_incoming_state_new(f);
- ret = qemu_loadvm_state(f);
+ aio_context_acquire(aio_context);
We have done a qemu_fopen_bdrv() without acquiring the context, not sure
if we really need it (or not). My understanding of locking is that we
should get the context on first use and maintain it until last use.
Does it work in a different way?
I was requested to drop that code in one of the versions.
Stefan, Juan, can you pls come into agreement.
+ ret = qemu_loadvm_state(f);
qemu_fclose(f);
+ aio_context_release(aio_context);
+
migration_incoming_state_destroy();
if (ret < 0) {
error_report("Error %d while loading VM state", ret);
@@ -2111,14 +2123,19 @@ void hmp_info_snapshots(Monitor *mon, const QDict
*qdict)
int nb_sns, i;
int total;
int *available_snapshots;
+ AioContext *aio_context;
bs = bdrv_all_find_vmstate_bs();
if (!bs) {
monitor_printf(mon, "No available block device supports snapshots\n");
return;
}
+ aio_context = bdrv_get_aio_context(bs);
+ aio_context_acquire(aio_context);
nb_sns = bdrv_snapshot_list(bs, &sn_tab);
+ aio_context_release(aio_context);
+
if (nb_sns < 0) {
monitor_printf(mon, "bdrv_snapshot_list: error %d\n", nb_sns);
return;
I will very much preffer to have a:
bdrv_snapshot_list_full_whatever()
That does the two operations and don't make me know about aio_contexts.
What I need there really is just the block layer to fill the sn_tab and
to told me if there is a problem, no real need of knowing about
contexts, no?
Thanks, Juan.
I know :( This is the most thing I can do at the moment keeping
agreement from Stefan :(
Den
