On Fri, Aug 5, 2011 at 10:07 AM, Kevin Wolf <kw...@redhat.com> wrote: > Am 05.08.2011 10:40, schrieb Stefan Hajnoczi: >> We've discussed safe methods for reopening image files (e.g. useful for >> changing the hostcache parameter). The problem is that closing the file >> first >> and then opening it again exposes us to the error case where the open fails. >> At that point we cannot get to the file anymore and our options are to >> terminate QEMU, pause the VM, or offline the block device. >> >> This window of vulnerability can be eliminated by keeping the file descriptor >> around and falling back to it should the open fail. >> >> The challenge for the file descriptor approach is that image formats, like >> VMDK, can span multiple files. Therefore the solution is not as simple as >> stashing a single file descriptor and reopening from it. > > So far I agree. The rest I believe is wrong because you can't assume > that every backend uses file descriptors. The qemu block layer is based > on BlockDriverStates, not fds. They are a concept that should be hidden > in raw-posix. > > I think something like this could do: > > struct BDRVReopenState { > BlockDriverState *bs; > /* can be extended by block drivers */ > }; > > .bdrv_reopen(BlockDriverState *bs, BDRVReopenState **reopen_state, int > flags); > .bdrv_reopen_commit(BDRVReopenState *reopen_state); > .bdrv_reopen_abort(BDRVReopenState *reopen_state); > > raw-posix would store the old file descriptor in its reopen_state. On > commit, it closes the old descriptors, on abort it reverts to the old > one and closes the newly opened one. > > Makes things a bit more complicated than the simple bdrv_reopen I had in > mind before, but it allows VMDK to get an all-or-nothing semantics.
Can you show how bdrv_reopen() would use these new interfaces? I'm not 100% clear on the idea. Stefan