On Wed, Apr 19, 2017 at 05:43:40PM +0800, Fam Zheng wrote: > v2: Address Stefan's comments: > > - Clean up redundancy in bdrv_format_default_perms change. > - Add a test case to check both success/failure cases. > A failure case is not possible at user interface level because of other > checks we have, so write a unit test in tests/test-blk-perm.c. > > Eject / change of scsi-cd on a virtio-scsi dataplane bus causes abort() > because > the new BDS doesn't get proper bdrv_set_aio_context(). > > Store the AioContext in BB and do it in blk_insert_bs. That is done by > Vladimir's patch. > > Other patches are to make sure such a bdrv_set_aio_context() doesn't interfere > with other BBs using other nodes from this graph.
Looks pretty good. I had two comments that apply across all patches: First, it is not safe to enable the new permission without registering an aio notifier. Another user could look up the BDS and call bdrv_set_aio_context() on it. I believe this bug is present for block jobs that have additional BDSes like base/target/etc. Second, patches that post-pone bdrv_set_aio_context() must take care to acquire the AioContext for BDS accesses that happen before the next bdrv_set_aio_context() call.
signature.asc
Description: PGP signature