qemu-img.c | 18 ++++-------------- tests/qemu-iotests/122 | 1 + tests/qemu-iotests/122.out | 6 ++++-- 3 files changed, 9 insertions(+), 16 deletions(-)
Peter, three years ago you changed 'qemu-img convert' to sacrifice some sparsification in order to get aligned requests on the target image. At the time, I thought the impact would be small, but it turns out that this can end up wasting gigabytes of storagee (like converting a fully zeroed 10 GB image taking 2.8 GB instead of a few kilobytes). https://bugzilla.redhat.com/show_bug.cgi?id=1882917 I'm not entirely sure how to attack this best since this is a tradeoff, but maybe the approach in this series is still good enough for the case that you wanted to fix back then? Of course, it would be possible to have a more complete fix like looking forward a few blocks more before writing data, but that would probably not be entirely trivial because you would have to merge blocks with ZERO block status with DATA blocks that contain only zeros. I'm not sure if it's worth this complication of the code. Kevin Wolf (2): iotests: Test qemu-img convert of zeroed data cluster qemu-img convert: Fix sparseness detection qemu-img.c | 18 ++++-------------- tests/qemu-iotests/122 | 1 + tests/qemu-iotests/122.out | 6 ++++-- 3 files changed, 9 insertions(+), 16 deletions(-) -- 2.30.2
> Am 15.04.2021 um 17:22 schrieb Kevin Wolf <kwolf@redhat.com>: > > Peter, three years ago you changed 'qemu-img convert' to sacrifice some > sparsification in order to get aligned requests on the target image. At > the time, I thought the impact would be small, but it turns out that > this can end up wasting gigabytes of storagee (like converting a fully > zeroed 10 GB image taking 2.8 GB instead of a few kilobytes). > > https://bugzilla.redhat.com/show_bug.cgi?id=1882917 > > I'm not entirely sure how to attack this best since this is a tradeoff, > but maybe the approach in this series is still good enough for the case > that you wanted to fix back then? > > Of course, it would be possible to have a more complete fix like looking > forward a few blocks more before writing data, but that would probably > not be entirely trivial because you would have to merge blocks with ZERO > block status with DATA blocks that contain only zeros. I'm not sure if > it's worth this complication of the code. I will try to look into this asap. Is there a hint which FS I need to set the extent hint when creating the raw image? I was not able to do that. Peter
Am 19.04.2021 um 10:36 hat Peter Lieven geschrieben: > > > > Am 15.04.2021 um 17:22 schrieb Kevin Wolf <kwolf@redhat.com>: > > > > Peter, three years ago you changed 'qemu-img convert' to sacrifice some > > sparsification in order to get aligned requests on the target image. At > > the time, I thought the impact would be small, but it turns out that > > this can end up wasting gigabytes of storagee (like converting a fully > > zeroed 10 GB image taking 2.8 GB instead of a few kilobytes). > > > > https://bugzilla.redhat.com/show_bug.cgi?id=1882917 > > > > I'm not entirely sure how to attack this best since this is a tradeoff, > > but maybe the approach in this series is still good enough for the case > > that you wanted to fix back then? > > > > Of course, it would be possible to have a more complete fix like looking > > forward a few blocks more before writing data, but that would probably > > not be entirely trivial because you would have to merge blocks with ZERO > > block status with DATA blocks that contain only zeros. I'm not sure if > > it's worth this complication of the code. > > I will try to look into this asap. > > Is there a hint which FS I need to set the extent hint when creating > the raw image? I was not able to do that. Grepping the current kernel source, it seems extent size hints still work only on XFS. But I don't think it's necessary for reproducing this bug. In fact, disabling the extent size hint should cause a lot more fragmentation, which should make the problem more visible. Kevin
> Am 19.04.2021 um 10:36 schrieb Peter Lieven <pl@kamp.de>: > > > >> Am 15.04.2021 um 17:22 schrieb Kevin Wolf <kwolf@redhat.com>: >> >> Peter, three years ago you changed 'qemu-img convert' to sacrifice some >> sparsification in order to get aligned requests on the target image. At >> the time, I thought the impact would be small, but it turns out that >> this can end up wasting gigabytes of storagee (like converting a fully >> zeroed 10 GB image taking 2.8 GB instead of a few kilobytes). >> >> https://bugzilla.redhat.com/show_bug.cgi?id=1882917 >> >> I'm not entirely sure how to attack this best since this is a tradeoff, >> but maybe the approach in this series is still good enough for the case >> that you wanted to fix back then? >> >> Of course, it would be possible to have a more complete fix like looking >> forward a few blocks more before writing data, but that would probably >> not be entirely trivial because you would have to merge blocks with ZERO >> block status with DATA blocks that contain only zeros. I'm not sure if >> it's worth this complication of the code. > > I will try to look into this asap. Besides from the reproducer described in the ticket, I retried my old conversion test in our environment: Before commit 8dcd3c9b91: reads 4608 writes 14959 After commit 8dcd3c9b91: reads 0 writes 14924 With Kevins patch: reads 110 writes 14924 I think this is a good result if it avoids other issues. Peter
Am 19.04.2021 um 11:13 hat Peter Lieven geschrieben: > > > > Am 19.04.2021 um 10:36 schrieb Peter Lieven <pl@kamp.de>: > > > > > > > >> Am 15.04.2021 um 17:22 schrieb Kevin Wolf <kwolf@redhat.com>: > >> > >> Peter, three years ago you changed 'qemu-img convert' to sacrifice some > >> sparsification in order to get aligned requests on the target image. At > >> the time, I thought the impact would be small, but it turns out that > >> this can end up wasting gigabytes of storagee (like converting a fully > >> zeroed 10 GB image taking 2.8 GB instead of a few kilobytes). > >> > >> https://bugzilla.redhat.com/show_bug.cgi?id=1882917 > >> > >> I'm not entirely sure how to attack this best since this is a tradeoff, > >> but maybe the approach in this series is still good enough for the case > >> that you wanted to fix back then? > >> > >> Of course, it would be possible to have a more complete fix like looking > >> forward a few blocks more before writing data, but that would probably > >> not be entirely trivial because you would have to merge blocks with ZERO > >> block status with DATA blocks that contain only zeros. I'm not sure if > >> it's worth this complication of the code. > > > > I will try to look into this asap. > > Besides from the reproducer described in the ticket, I retried my old > conversion test in our environment: > > Before commit 8dcd3c9b91: reads 4608 writes 14959 > After commit 8dcd3c9b91: reads 0 writes 14924 > With Kevins patch: reads 110 writes 14924 > > I think this is a good result if it avoids other issues. Sounds like a promising way to make the tradeoff. Thanks for testing! Kevin
Von meinem iPhone gesendet > Am 19.04.2021 um 14:31 schrieb Kevin Wolf <kwolf@redhat.com>: > > Am 19.04.2021 um 11:13 hat Peter Lieven geschrieben: >> >> >>>> Am 19.04.2021 um 10:36 schrieb Peter Lieven <pl@kamp.de>: >>> >>> >>> >>>> Am 15.04.2021 um 17:22 schrieb Kevin Wolf <kwolf@redhat.com>: >>>> >>>> Peter, three years ago you changed 'qemu-img convert' to sacrifice some >>>> sparsification in order to get aligned requests on the target image. At >>>> the time, I thought the impact would be small, but it turns out that >>>> this can end up wasting gigabytes of storagee (like converting a fully >>>> zeroed 10 GB image taking 2.8 GB instead of a few kilobytes). >>>> >>>> https://bugzilla.redhat.com/show_bug.cgi?id=1882917 >>>> >>>> I'm not entirely sure how to attack this best since this is a tradeoff, >>>> but maybe the approach in this series is still good enough for the case >>>> that you wanted to fix back then? >>>> >>>> Of course, it would be possible to have a more complete fix like looking >>>> forward a few blocks more before writing data, but that would probably >>>> not be entirely trivial because you would have to merge blocks with ZERO >>>> block status with DATA blocks that contain only zeros. I'm not sure if >>>> it's worth this complication of the code. >>> >>> I will try to look into this asap. >> >> Besides from the reproducer described in the ticket, I retried my old >> conversion test in our environment: >> >> Before commit 8dcd3c9b91: reads 4608 writes 14959 >> After commit 8dcd3c9b91: reads 0 writes 14924 >> With Kevins patch: reads 110 writes 14924 >> >> I think this is a good result if it avoids other issues. > > Sounds like a promising way to make the tradeoff. Thanks for testing! is this sth for 6.0-rc4? Peter
Am 19.04.2021 um 19:12 hat Peter Lieven geschrieben: > > > Von meinem iPhone gesendet > > > Am 19.04.2021 um 14:31 schrieb Kevin Wolf <kwolf@redhat.com>: > > > > Am 19.04.2021 um 11:13 hat Peter Lieven geschrieben: > >> > >> > >>>> Am 19.04.2021 um 10:36 schrieb Peter Lieven <pl@kamp.de>: > >>> > >>> > >>> > >>>> Am 15.04.2021 um 17:22 schrieb Kevin Wolf <kwolf@redhat.com>: > >>>> > >>>> Peter, three years ago you changed 'qemu-img convert' to sacrifice some > >>>> sparsification in order to get aligned requests on the target image. At > >>>> the time, I thought the impact would be small, but it turns out that > >>>> this can end up wasting gigabytes of storagee (like converting a fully > >>>> zeroed 10 GB image taking 2.8 GB instead of a few kilobytes). > >>>> > >>>> https://bugzilla.redhat.com/show_bug.cgi?id=1882917 > >>>> > >>>> I'm not entirely sure how to attack this best since this is a tradeoff, > >>>> but maybe the approach in this series is still good enough for the case > >>>> that you wanted to fix back then? > >>>> > >>>> Of course, it would be possible to have a more complete fix like looking > >>>> forward a few blocks more before writing data, but that would probably > >>>> not be entirely trivial because you would have to merge blocks with ZERO > >>>> block status with DATA blocks that contain only zeros. I'm not sure if > >>>> it's worth this complication of the code. > >>> > >>> I will try to look into this asap. > >> > >> Besides from the reproducer described in the ticket, I retried my old > >> conversion test in our environment: > >> > >> Before commit 8dcd3c9b91: reads 4608 writes 14959 > >> After commit 8dcd3c9b91: reads 0 writes 14924 > >> With Kevins patch: reads 110 writes 14924 > >> > >> I think this is a good result if it avoids other issues. > > > > Sounds like a promising way to make the tradeoff. Thanks for testing! > > is this sth for 6.0-rc4? No, certainly not. It would be for the first 6.1 pull request. Kevin
© 2016 - 2024 Red Hat, Inc.