Re: [libvirt] [RFC PATCH] qcow2: Fix race in cache invalidation

Am 25.09.2014 um 11:55 hat Alexey Kardashevskiy geschrieben: > Right. Cool. So is below what was suggested? I am doublechecking as it does > not solve the original issue - the bottomhalf is called first and then > nbd_trip() crashes in qcow2_co_flush_to_os(). > > diff --git a/block.c b/block.c > index d06dd51..1e6dfd1 100644 > --- a/block.c > +++ b/block.c > @@ -5037,20 +5037,22 @@ void bdrv_invalidate_cache(BlockDriverState *bs, > Error **errp) > if (local_err) { > error_propagate(errp, local_err); > return; > } > > ret = refresh_total_sectors(bs, bs->total_sectors); > if (ret < 0) { > error_setg_errno(errp, -ret, "Could not refresh total sector count"); > return; > } > + > + bdrv_drain_all(); > } Try moving the bdrv_drain_all() call to the top of the function (at least it must be called before bs->drv->bdrv_invalidate_cache).

> +static QEMUBH *migration_complete_bh; > +static void process_incoming_migration_complete(void *opaque); > + > static void process_incoming_migration_co(void *opaque) > { > QEMUFile *f = opaque; > - Error *local_err = NULL; > int ret; > > ret = qemu_loadvm_state(f); > qemu_fclose(f); Paolo suggested to move eveything starting from here, but as far as I can tell, leaving the next few lines here shouldn't hurt.

> free_xbzrle_decoded_buf(); > if (ret < 0) { > error_report("load of migration failed: %s", strerror(-ret)); > exit(EXIT_FAILURE); > } > qemu_announce_self(); > > bdrv_clear_incoming_migration_all(); > + > + migration_complete_bh = aio_bh_new(qemu_get_aio_context(), > + process_incoming_migration_complete, > + NULL); > + qemu_bh_schedule(migration_complete_bh); > +} > + > +static void process_incoming_migration_complete(void *opaque) > +{ > + Error *local_err = NULL; > + > /* Make sure all file formats flush their mutable metadata */ > bdrv_invalidate_cache_all(&local_err); > if (local_err) { > qerror_report_err(local_err); > error_free(local_err); > exit(EXIT_FAILURE); > } > > if (autostart) { > vm_start(); > } else { > runstate_set(RUN_STATE_PAUSED); > } > + qemu_bh_delete(migration_complete_bh); > + migration_complete_bh = NULL; > } That part looks good to me. I hope moving bdrv_drain_all() does the trick, otherwise there's somthing wrong with our reasoning. Kevin