On Thu, Aug 17, 2023 at 02:50:02PM +0200, Kevin Wolf wrote: > When the permission related BlockDriver callbacks are called, we are in > the middle of an operation traversing the block graph. Polling in such a > place is a very bad idea because the graph could change in unexpected > ways. In the future, callers will also hold the graph lock, which is > likely to turn polling into a deadlock.
One I'm sure you encountered before writing this patch ;) > > So we need to get rid of calls to functions like bdrv_getlength() or > bdrv_truncate() there as these functions poll internally. They are > currently used so that when no parent has write/resize permissions on > the image any more, the preallocate filter drops the extra preallocated > area in the image file and gives up write/resize permissions itself. Sounds like a sane plan, and the patch matches the implementation spelled out here. > > In order to achieve this without polling in .bdrv_check_perm, don't > immediately truncate the image, but only schedule a BH to do so. The > filter keeps the write/resize permissions a bit longer now until the BH > has executed. > > There is one case in which delaying doesn't work: Reopening the image > read-only. In this case, bs->file will likely be reopened read-only, > too, so keeping write permissions a bit longer on it doesn't work. But > we can already cover this case in preallocate_reopen_prepare() and not > rely on the permission updates for it. > > Signed-off-by: Kevin Wolf <kw...@redhat.com> > --- > block/preallocate.c | 89 +++++++++++++++++++++++++++++++++++---------- > 1 file changed, 69 insertions(+), 20 deletions(-) > Reviewed-by: Eric Blake <ebl...@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. Virtualization: qemu.org | libguestfs.org
