Ping <bait>just the final three patches left to review</bait>
On 22.05.19 19:03, Max Reitz wrote: > Hi, > > This series is mainly a fix for > https://bugzilla.redhat.com/show_bug.cgi?id=1703793. The problem > described there is that mirroring to a gluster volume, then switching > off the volume makes qemu crash. There are two problems here: > > (1) file-posix reopens the FD all the time because it thinks the FD it > has is RDONLY. It actually isn’t after the first reopen, we just > forgot to change the internal flags. That’s what patch 1 is for. > > (2) Even then, when mirror completes, it drops its write permission on > the FD. This requires a reopen, which will fail if the volume is > down. Mirror doesn’t expect that. Nobody ever expects that > dropping permissions can fail, and rightfully so because that’s what > I think we have generally agreed on. > Therefore, the block layer should hide this error. This is what the > last two patches are for. > > The penultimate patch adds two assertions: bdrv_replace_child() (for the > old BDS) and bdrv_inactivate_recurse() assume they only ever drop > assertions. This is now substantiated by these new assertions. > It turns out that this assumption was just plain wrong. Patches 3 to 5 > make it right. > > > v3: > - Received no reply to my “Hm, warnings break 'make check', so maybe we > should just keep quiet if loosening restrictions fails?” question, so > I assume silence means agreement. Changed patch 7 accordingly. > > - Added a test: The fact how make check kind-of-but-not-really broke > showed a nice reproducer: Launching qemu with some file, then deleting > that file, then quitting qemu. > > - Rebase “conflict” in patch 6: The forward declaration of > bdrv_get_cumulative_perm() is already in qemu thanks to commit > 481e0eeef4f. > > > git-backport-diff against v2: > > Key: > [----] : patches are identical > [####] : number of functional differences between upstream/downstream patch > [down] : patch is downstream-only > The flags [FC] indicate (F)unctional and (C)ontextual differences, > respectively > > 001/8:[----] [--] 'file-posix: Update open_flags in raw_set_perm()' > 002/8:[----] [--] 'block: Add bdrv_child_refresh_perms()' > 003/8:[----] [--] 'block/mirror: Fix child permissions' > 004/8:[----] [--] 'block/commit: Drop bdrv_child_try_set_perm()' > 005/8:[0018] [FC] 'block: Fix order in bdrv_replace_child()' > ^^^^ Again confuses my v2 patch with 8aecf1d1bd250a, should be: > [----] : patches are identical > 006/8:[0002] [FC] 'block: Add *tighten_restrictions to *check*_perm()' > 007/8:[0018] [FC] 'block: Ignore loosening perm restrictions failures' > 008/8:[down] 'iotests: Test failure to loosen restrictions' > > > Max Reitz (8): > file-posix: Update open_flags in raw_set_perm() > block: Add bdrv_child_refresh_perms() > block/mirror: Fix child permissions > block/commit: Drop bdrv_child_try_set_perm() > block: Fix order in bdrv_replace_child() > block: Add *tighten_restrictions to *check*_perm() > block: Ignore loosening perm restrictions failures > iotests: Test failure to loosen restrictions > > include/block/block_int.h | 15 ++++ > block.c | 153 +++++++++++++++++++++++++++++++------ > block/commit.c | 2 - > block/file-posix.c | 4 + > block/mirror.c | 32 +++++--- > tests/qemu-iotests/182 | 21 +++++ > tests/qemu-iotests/182.out | 6 ++ > 7 files changed, 198 insertions(+), 35 deletions(-) >
signature.asc
Description: OpenPGP digital signature