[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 03/21] preallocate: Don't poll during permission updates
From: |
Eric Blake |
Subject: |
Re: [PATCH 03/21] preallocate: Don't poll during permission updates |
Date: |
Fri, 18 Aug 2023 10:34:36 -0500 |
User-agent: |
NeoMutt/20230517 |
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 <kwolf@redhat.com>
> ---
> block/preallocate.c | 89 +++++++++++++++++++++++++++++++++++----------
> 1 file changed, 69 insertions(+), 20 deletions(-)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
- [PATCH 00/21] Graph locking part 4 (node management), Kevin Wolf, 2023/08/17
- [PATCH 02/21] preallocate: Factor out preallocate_truncate_to_real_size(), Kevin Wolf, 2023/08/17
- [PATCH 01/21] block: Remove unused BlockReopenQueueEntry.perms_checked, Kevin Wolf, 2023/08/17
- [PATCH 03/21] preallocate: Don't poll during permission updates, Kevin Wolf, 2023/08/17
- [PATCH 11/21] block: Call transaction callbacks with lock held, Kevin Wolf, 2023/08/17
- [PATCH 04/21] block: Take AioContext lock for bdrv_append() more consistently, Kevin Wolf, 2023/08/17
- [PATCH 08/21] block: Mark bdrv_replace_child_noperm() GRAPH_WRLOCK, Kevin Wolf, 2023/08/17