Re: [PATCH 0/2] block-backend: Retain permissions after migration
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
Re: [PATCH 0/2] block-backend: Retain permissions after migration
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
Re: [PATCH 0/2] block-backend: Retain permissions after migration
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
Re: [PATCH 0/2] block-backend: Retain permissions after migration
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
[PATCH 0/2] block-backend: Retain permissions after migration
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 -- 2.33.1
