| Message ID | 20211125135317.186576-1-hreitz@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | block-backend: Retain permissions after migration | expand |
Am 25.11.2021 um 14:53 hat Hanna Reitz geschrieben: > Hi, > > Peng Liang has reported an issue regarding migration of raw images here: > https://lists.nongnu.org/archive/html/qemu-block/2021-11/msg00673.html > > It turns out that after migrating, all permissions are shared when they > weren’t before. The cause of the problem is that we deliberately delay > restricting the shared permissions until migration is really done (until > the runstate is no longer INMIGRATE) and first share all permissions; > but this causes us to lose the original shared permission mask and > overwrites it with BLK_PERM_ALL, so once we do try to restrict the > shared permissions, we only again share them all. > > Fix this by saving the set of shared permissions through the first > blk_perm_set() call that shares all; and add a regression test. > > > I don’t believe we have to fix this in 6.2, because I think this bug has > existed for four years now. (I.e. it isn’t critical, and it’s no > regression.) Feels a bit like a hack, but I guess as long as it works... :-) Reviewed-by: Kevin Wolf <kwolf@redhat.com>
On 11/25/2021 9:53 PM, Hanna Reitz wrote: > Hi, > > Peng Liang has reported an issue regarding migration of raw images here: > https://lists.nongnu.org/archive/html/qemu-block/2021-11/msg00673.html > > It turns out that after migrating, all permissions are shared when they > weren’t before. The cause of the problem is that we deliberately delay > restricting the shared permissions until migration is really done (until > the runstate is no longer INMIGRATE) and first share all permissions; > but this causes us to lose the original shared permission mask and > overwrites it with BLK_PERM_ALL, so once we do try to restrict the > shared permissions, we only again share them all. > > Fix this by saving the set of shared permissions through the first > blk_perm_set() call that shares all; and add a regression test. > > > I don’t believe we have to fix this in 6.2, because I think this bug has > existed for four years now. (I.e. it isn’t critical, and it’s no > regression.) > > > Hanna Reitz (2): > block-backend: Retain permissions after migration > iotests/migration-permissions: New test > > block/block-backend.c | 11 ++ > .../qemu-iotests/tests/migration-permissions | 101 ++++++++++++++++++ > .../tests/migration-permissions.out | 5 + > 3 files changed, 117 insertions(+) > create mode 100755 tests/qemu-iotests/tests/migration-permissions > create mode 100644 tests/qemu-iotests/tests/migration-permissions.out > Hi Hanna, QEMU 6.3 development tree has been opened. Will this fix be merged in 6.3? Thanks, Peng
On 25.11.21 14:53, Hanna Reitz wrote: > Hi, > > Peng Liang has reported an issue regarding migration of raw images here: > https://lists.nongnu.org/archive/html/qemu-block/2021-11/msg00673.html > > It turns out that after migrating, all permissions are shared when they > weren’t before. The cause of the problem is that we deliberately delay > restricting the shared permissions until migration is really done (until > the runstate is no longer INMIGRATE) and first share all permissions; > but this causes us to lose the original shared permission mask and > overwrites it with BLK_PERM_ALL, so once we do try to restrict the > shared permissions, we only again share them all. > > Fix this by saving the set of shared permissions through the first > blk_perm_set() call that shares all; and add a regression test. > > > I don’t believe we have to fix this in 6.2, because I think this bug has > existed for four years now. (I.e. it isn’t critical, and it’s no > regression.) Thanks for the reviews, applied to my block branch: https://gitlab.com/hreitz/qemu/-/commits/block Hanna
On 10.01.22 12:51, Peng Liang wrote: > On 11/25/2021 9:53 PM, Hanna Reitz wrote: >> Hi, >> >> Peng Liang has reported an issue regarding migration of raw images here: >> https://lists.nongnu.org/archive/html/qemu-block/2021-11/msg00673.html >> >> It turns out that after migrating, all permissions are shared when they >> weren’t before. The cause of the problem is that we deliberately delay >> restricting the shared permissions until migration is really done (until >> the runstate is no longer INMIGRATE) and first share all permissions; >> but this causes us to lose the original shared permission mask and >> overwrites it with BLK_PERM_ALL, so once we do try to restrict the >> shared permissions, we only again share them all. >> >> Fix this by saving the set of shared permissions through the first >> blk_perm_set() call that shares all; and add a regression test. >> >> >> I don’t believe we have to fix this in 6.2, because I think this bug has >> existed for four years now. (I.e. it isn’t critical, and it’s no >> regression.) >> >> >> Hanna Reitz (2): >> block-backend: Retain permissions after migration >> iotests/migration-permissions: New test >> >> block/block-backend.c | 11 ++ >> .../qemu-iotests/tests/migration-permissions | 101 ++++++++++++++++++ >> .../tests/migration-permissions.out | 5 + >> 3 files changed, 117 insertions(+) >> create mode 100755 tests/qemu-iotests/tests/migration-permissions >> create mode 100644 tests/qemu-iotests/tests/migration-permissions.out >> > Hi Hanna, > QEMU 6.3 development tree has been opened. Will this fix be merged in 6.3? Oh, yes, right. Thanks for the reminder! :) Hanna