On 11.09.23 12:46, 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.
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.
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.
Hmm, now I found one more "future" case.
I now try to rebase my "[PATCH v7 0/7] blockdev-replace"
https://patchew.org/QEMU/20230421114102.884457-1-vsement...@yandex-team.ru/
And it breaks after this commit.
By accident, blockdev-replace series uses exactly "preallocate" filter to test
insertion/removing of filters. And removing is broken now.
Removing is done as follows:
1. We have filter inserted: disk0 -file-> filter -file-> file0
2. blockdev-replace, replaces file child of disk0, so we should get the picture*:
disk0 -file-> file0 <-file- filter
3. blockdev-del filter
But step [2] fails, as now preallocate filter doesn't drop permissions during
the operation (postponing this for a while) and the picture* is impossible.
Permission check fails.
Hmmm... Any idea how blockdev-replace and preallocate filter should work :) ?
Maybe, doing truncation in .drain_begin() will help? Will try
Signed-off-by: Kevin Wolf <kw...@redhat.com>
Reviewed-by: Eric Blake <ebl...@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com>
---
--
Best regards,
Vladimir
