* Re: [PATCH] Skip resizing image to the same size [not found] ` <Z45ANK-m2XZazDi3@redhat.com> @ 2025-01-20 20:17 ` Fahrzin Hemmati 2025-01-20 21:49 ` Michael Tokarev 0 siblings, 1 reply; 5+ messages in thread From: Fahrzin Hemmati @ 2025-01-20 20:17 UTC (permalink / raw) To: Kevin Wolf; +Cc: qemu-devel, Hanna Reitz, qemu-block [-- Attachment #1: Type: text/plain, Size: 3782 bytes --] My apologies, I saw the Signed-off-by requirement at first, but as I followed the docs I got to "git publish" and didn't realize "git publish" was actually able to send emails on my system (I don't remember setting up any SMTP settings). By that time, I forgot and thought this patch was short enough to not warrant much of a commit message. The main practical advantage is for users that call "qemu-img resize" via scripts or other systems (like Packer in my case) that don't check the image size ahead of time. I can upstream this change into them (by using "qemu-img info —output=json ...") but I figured it would be useful to more users here. This could trivially be added to block/io.c:bdrv_co_truncate(), as well as blockdev.c:qmp_block_resize() with a little more work. I'm not familiar with those workflows, but if needed I can do that as well. Here's the new patch: From 17f5c5f03d930c4816b92b97e0e54db0725d7b94 Mon Sep 17 00:00:00 2001 From: Fahrzin Hemmati <fahhem@fahhem.com> Date: Mon, 20 Jan 2025 01:56:24 -0800 Subject: [PATCH] Skip resizing image to the same size Higher-level software, such as Packer, blindly call "qemu-img resize" even when the size is the same. This change can be pushed to those users, but it's very cheap to check and can save many other users more time here. Signed-off-by: Fahrzin Hemmati <fahhem@fahhem.com> --- qemu-img.c | 6 ++++++ tests/qemu-iotests/061 | 8 ++++++++ tests/qemu-iotests/061.out | 9 +++++++++ 3 files changed, 23 insertions(+) diff --git a/qemu-img.c b/qemu-img.c index 2f2fac90e8..3345c4e63f 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -4184,6 +4184,12 @@ static int img_resize(int argc, char **argv) goto out; } + if (total_size == current_size) { + qprintf(quiet, "Image already has the desired size.\n"); + ret = 0; + goto out; + } + /* * The user expects the image to have the desired size after * resizing, so pass @exact=true. It is of no use to report diff --git a/tests/qemu-iotests/061 b/tests/qemu-iotests/061 index b71ac097d1..88aec4ebc6 100755 --- a/tests/qemu-iotests/061 +++ b/tests/qemu-iotests/061 @@ -150,6 +150,14 @@ _qcow2_dump_header | grep '^\(version\|size\|nb_snap\)' _check_test_img +echo +echo "=== Testing resize to same size ===" +echo +_make_test_img -o "compat=0.10" 32M +$QEMU_IMG resize "$TEST_IMG" 32M || + echo "unexpected fail" +_qcow2_dump_header | grep '^\(version\|size\|nb_snap\)' +_check_test_img echo echo "=== Testing dirty lazy_refcounts=off ===" diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out index 24c33add7c..d94eb9d513 100644 --- a/tests/qemu-iotests/061.out +++ b/tests/qemu-iotests/061.out @@ -299,6 +299,15 @@ size 33554432 nb_snapshots 1 No errors were found on the image. +=== Testing resize to same size === + +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33554432 +Image already has the desired size. +version 2 +size 33554432 +nb_snapshots 0 +No errors were found on the image. + === Testing dirty lazy_refcounts=off === Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 -- 2.43.0 On Mon Jan 20, 2025, 12:23 PM GMT, Kevin Wolf <kwolf@redhat.com> wrote: Am 20.01.2025 um 11:37 hat Fahrzin Hemmati geschrieben: --- qemu-img.c | 6 ++++++ tests/qemu-iotests/061 | 8 ++++++++ tests/qemu-iotests/061.out | 9 +++++++++ 3 files changed, 23 insertions(+) This is lacking a commit message. Please describe in it why this is change is made, i.e. which practical advantages the change brings, and why having it in qemu-img, but not in other places like the block_resize monitor command is desirable. Also, without a Signed-off-by line, with which you sign the DCO, patches can't be accepted into QEMU. Kevin [-- Attachment #2: Type: text/html, Size: 6153 bytes --] ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] Skip resizing image to the same size 2025-01-20 20:17 ` [PATCH] Skip resizing image to the same size Fahrzin Hemmati @ 2025-01-20 21:49 ` Michael Tokarev 2025-01-20 22:21 ` Fahrzin Hemmati 0 siblings, 1 reply; 5+ messages in thread From: Michael Tokarev @ 2025-01-20 21:49 UTC (permalink / raw) To: Fahrzin Hemmati, Kevin Wolf; +Cc: qemu-devel, Hanna Reitz, qemu-block 20.01.2025 23:17, Fahrzin Hemmati wrote: > My apologies, I saw the Signed-off-by requirement at first, but as I followed the docs I got to "git publish" and didn't realize "git publish" was > actually able to send emails on my system (I don't remember setting up any SMTP settings). By that time, I forgot and thought this patch was short > enough to not warrant much of a commit message. > > The main practical advantage is for users that call "qemu-img resize" via scripts or other systems (like Packer in my case) that don't check the image > size ahead of time. I can upstream this change into them (by using "qemu-img info —output=json ...") but I figured it would be useful to more users here. > > This could trivially be added to block/io.c:bdrv_co_truncate(), as well as blockdev.c:qmp_block_resize() with a little more work. I'm not familiar > with those workflows, but if needed I can do that as well. > > Here's the new patch: > > From 17f5c5f03d930c4816b92b97e0e54db0725d7b94 Mon Sep 17 00:00:00 2001 > From: Fahrzin Hemmati <fahhem@fahhem.com <mailto:fahhem@fahhem.com>> > Date: Mon, 20 Jan 2025 01:56:24 -0800 > Subject: [PATCH] Skip resizing image to the same size > > Higher-level software, such as Packer, blindly call "qemu-img resize" > even when the size is the same. This change can be pushed to those > users, but it's very cheap to check and can save many other users more > time here. > > Signed-off-by: Fahrzin Hemmati <fahhem@fahhem.com <mailto:fahhem@fahhem.com>> > --- > qemu-img.c | 6 ++++++ > tests/qemu-iotests/061 | 8 ++++++++ > tests/qemu-iotests/061.out | 9 +++++++++ > 3 files changed, 23 insertions(+) > > diff --git a/qemu-img.c b/qemu-img.c > index 2f2fac90e8..3345c4e63f 100644 > --- a/qemu-img.c > +++ b/qemu-img.c > @@ -4184,6 +4184,12 @@ static int img_resize(int argc, char **argv) > goto out; > } > > + if (total_size == current_size) { > + qprintf(quiet, "Image already has the desired size.\n"); > + ret = 0; > + goto out; > + } I don't think this is necessary: the actual operation is a no-op anyway, there's no need to special-case it. Please also refrain from changing qemu-img until my refresh patchset is either accepted or rejected. It was a large work and it'd be sad to lose it. Thanks, /mjt ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Skip resizing image to the same size 2025-01-20 21:49 ` Michael Tokarev @ 2025-01-20 22:21 ` Fahrzin Hemmati 2025-01-21 11:40 ` Kevin Wolf 0 siblings, 1 reply; 5+ messages in thread From: Fahrzin Hemmati @ 2025-01-20 22:21 UTC (permalink / raw) To: Kevin Wolf, Michael Tokarev; +Cc: qemu-devel, Hanna Reitz, qemu-block [-- Attachment #1: Type: text/plain, Size: 2520 bytes --] Happy to wait until your patchset is in. Yes, this is a no-op, but it reads the entire disk image to perform that no-op, so this is merely a time-saving improvement, not a behavior change. On Mon Jan 20, 2025, 09:49 PM GMT, Michael Tokarev <mjt@tls.msk.ru> wrote: 20.01.2025 23:17, Fahrzin Hemmati wrote: My apologies, I saw the Signed-off-by requirement at first, but as I followed the docs I got to "git publish" and didn't realize "git publish" was actually able to send emails on my system (I don't remember setting up any SMTP settings). By that time, I forgot and thought this patch was short enough to not warrant much of a commit message. The main practical advantage is for users that call "qemu-img resize" via scripts or other systems (like Packer in my case) that don't check the image size ahead of time. I can upstream this change into them (by using "qemu-img info —output=json ...") but I figured it would be useful to more users here. This could trivially be added to block/io.c:bdrv_co_truncate(), as well as blockdev.c:qmp_block_resize() with a little more work. I'm not familiar with those workflows, but if needed I can do that as well. Here's the new patch: From 17f5c5f03d930c4816b92b97e0e54db0725d7b94 Mon Sep 17 00:00:00 2001 From: Fahrzin Hemmati <fahhem@fahhem.com <mailto:fahhem@fahhem.com>> Date: Mon, 20 Jan 2025 01:56:24 -0800 Subject: [PATCH] Skip resizing image to the same size Higher-level software, such as Packer, blindly call "qemu-img resize" even when the size is the same. This change can be pushed to those users, but it's very cheap to check and can save many other users more time here. Signed-off-by: Fahrzin Hemmati <fahhem@fahhem.com <mailto:fahhem@fahhem.com >> --- qemu-img.c | 6 ++++++ tests/qemu-iotests/061 | 8 ++++++++ tests/qemu-iotests/061.out | 9 +++++++++ 3 files changed, 23 insertions(+) diff --git a/qemu-img.c b/qemu-img.c index 2f2fac90e8..3345c4e63f 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -4184,6 +4184,12 @@ static int img_resize(int argc, char **argv) goto out; } + if (total_size == current_size) { + qprintf(quiet, "Image already has the desired size.\n"); + ret = 0; + goto out; + } I don't think this is necessary: the actual operation is a no-op anyway, there's no need to special-case it. Please also refrain from changing qemu-img until my refresh patchset is either accepted or rejected. It was a large work and it'd be sad to lose it. Thanks, /mjt [-- Attachment #2: Type: text/html, Size: 3220 bytes --] ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] Skip resizing image to the same size 2025-01-20 22:21 ` Fahrzin Hemmati @ 2025-01-21 11:40 ` Kevin Wolf 2025-01-21 15:30 ` Fahrzin Hemmati 0 siblings, 1 reply; 5+ messages in thread From: Kevin Wolf @ 2025-01-21 11:40 UTC (permalink / raw) To: Fahrzin Hemmati; +Cc: Michael Tokarev, qemu-devel, Hanna Reitz, qemu-block Am 20.01.2025 um 23:21 hat Fahrzin Hemmati geschrieben: > Happy to wait until your patchset is in. > > Yes, this is a no-op, but it reads the entire disk image to perform that > no-op, so this is merely a time-saving improvement, not a behavior change. Can you give more context on what exactly you're doing that it reads the entire disk image just to resize it? This sounds completely unexpected and if it does, there may be a different problem to be solved. In my test, for a raw image, I see a single ftruncate() call, which is unnecessary, but shouldn't cause any measurable time difference for the qemu-img run. qcow2 has a little more code in QEMU to figure out that there is nothing to do, but it doesn't involve any syscall and certainly not reading the whole image. Kevin > On Mon Jan 20, 2025, 09:49 PM GMT, Michael Tokarev <mjt@tls.msk.ru> wrote: > > 20.01.2025 23:17, Fahrzin Hemmati wrote: > > My apologies, I saw the Signed-off-by requirement at first, but as I > followed the docs I got to "git publish" and didn't realize "git publish" > was > actually able to send emails on my system (I don't remember setting up any > SMTP settings). By that time, I forgot and thought this patch was short > enough to not warrant much of a commit message. > > The main practical advantage is for users that call "qemu-img resize" via > scripts or other systems (like Packer in my case) that don't check the > image > size ahead of time. I can upstream this change into them (by using > "qemu-img info —output=json ...") but I figured it would be useful to more > users here. > > This could trivially be added to block/io.c:bdrv_co_truncate(), as well as > blockdev.c:qmp_block_resize() with a little more work. I'm not familiar > with those workflows, but if needed I can do that as well. > > Here's the new patch: > > From 17f5c5f03d930c4816b92b97e0e54db0725d7b94 Mon Sep 17 00:00:00 2001 > From: Fahrzin Hemmati <fahhem@fahhem.com <mailto:fahhem@fahhem.com>> > Date: Mon, 20 Jan 2025 01:56:24 -0800 > Subject: [PATCH] Skip resizing image to the same size > > Higher-level software, such as Packer, blindly call "qemu-img resize" > even when the size is the same. This change can be pushed to those > users, but it's very cheap to check and can save many other users more > time here. > > Signed-off-by: Fahrzin Hemmati <fahhem@fahhem.com <mailto:fahhem@fahhem.com > >> > --- > qemu-img.c | 6 ++++++ > tests/qemu-iotests/061 | 8 ++++++++ > tests/qemu-iotests/061.out | 9 +++++++++ > 3 files changed, 23 insertions(+) > > diff --git a/qemu-img.c b/qemu-img.c > index 2f2fac90e8..3345c4e63f 100644 > --- a/qemu-img.c > +++ b/qemu-img.c > @@ -4184,6 +4184,12 @@ static int img_resize(int argc, char **argv) > goto out; > } > > + if (total_size == current_size) { > + qprintf(quiet, "Image already has the desired size.\n"); > + ret = 0; > + goto out; > + } > > > > I don't think this is necessary: the actual operation is a no-op > anyway, there's no need to special-case it. > > Please also refrain from changing qemu-img until my refresh > patchset is either accepted or rejected. It was a large work > and it'd be sad to lose it. > > Thanks, > > /mjt ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Skip resizing image to the same size 2025-01-21 11:40 ` Kevin Wolf @ 2025-01-21 15:30 ` Fahrzin Hemmati 0 siblings, 0 replies; 5+ messages in thread From: Fahrzin Hemmati @ 2025-01-21 15:30 UTC (permalink / raw) To: Kevin Wolf; +Cc: Michael Tokarev, qemu-devel, Hanna Reitz, qemu-block [-- Attachment #1: Type: text/plain, Size: 3632 bytes --] I'm running packer, and with a qcow2 source image sized at 75000MB (but only 5GB on disk) when it runs "qemu-img resize ... 75000MB" it takes about 10 seconds on my system. I guess it's not reading the whole disk since me nvme isn't that fast, but it's a non-trivial amount of work. It also runs a qemu-img convert call (again, from qcow2 to qcow2) that seems more like a cp, and that takes the same 10 seconds on my system. On Tue Jan 21, 2025, 11:40 AM GMT, Kevin Wolf <kwolf@redhat.com> wrote: Am 20.01.2025 um 23:21 hat Fahrzin Hemmati geschrieben: Happy to wait until your patchset is in. Yes, this is a no-op, but it reads the entire disk image to perform that no-op, so this is merely a time-saving improvement, not a behavior change. Can you give more context on what exactly you're doing that it reads the entire disk image just to resize it? This sounds completely unexpected and if it does, there may be a different problem to be solved. In my test, for a raw image, I see a single ftruncate() call, which is unnecessary, but shouldn't cause any measurable time difference for the qemu-img run. qcow2 has a little more code in QEMU to figure out that there is nothing to do, but it doesn't involve any syscall and certainly not reading the whole image. Kevin On Mon Jan 20, 2025, 09:49 PM GMT, Michael Tokarev <mjt@tls.msk.ru> wrote: 20.01.2025 23:17, Fahrzin Hemmati wrote: My apologies, I saw the Signed-off-by requirement at first, but as I followed the docs I got to "git publish" and didn't realize "git publish" was actually able to send emails on my system (I don't remember setting up any SMTP settings). By that time, I forgot and thought this patch was short enough to not warrant much of a commit message. The main practical advantage is for users that call "qemu-img resize" via scripts or other systems (like Packer in my case) that don't check the image size ahead of time. I can upstream this change into them (by using "qemu-img info —output=json ...") but I figured it would be useful to more users here. This could trivially be added to block/io.c:bdrv_co_truncate(), as well as blockdev.c:qmp_block_resize() with a little more work. I'm not familiar with those workflows, but if needed I can do that as well. Here's the new patch: From 17f5c5f03d930c4816b92b97e0e54db0725d7b94 Mon Sep 17 00:00:00 2001 From: Fahrzin Hemmati <fahhem@fahhem.com <mailto:fahhem@fahhem.com>> Date: Mon, 20 Jan 2025 01:56:24 -0800 Subject: [PATCH] Skip resizing image to the same size Higher-level software, such as Packer, blindly call "qemu-img resize" even when the size is the same. This change can be pushed to those users, but it's very cheap to check and can save many other users more time here. Signed-off-by: Fahrzin Hemmati <fahhem@fahhem.com <mailto:fahhem@fahhem.com >> --- qemu-img.c | 6 ++++++ tests/qemu-iotests/061 | 8 ++++++++ tests/qemu-iotests/061.out | 9 +++++++++ 3 files changed, 23 insertions(+) diff --git a/qemu-img.c b/qemu-img.c index 2f2fac90e8..3345c4e63f 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -4184,6 +4184,12 @@ static int img_resize(int argc, char **argv) goto out; } + if (total_size == current_size) { + qprintf(quiet, "Image already has the desired size.\n"); + ret = 0; + goto out; + } I don't think this is necessary: the actual operation is a no-op anyway, there's no need to special-case it. Please also refrain from changing qemu-img until my refresh patchset is either accepted or rejected. It was a large work and it'd be sad to lose it. Thanks, /mjt [-- Attachment #2: Type: text/html, Size: 4472 bytes --] ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-01-21 15:32 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20250120103711.836753-1-fahhem@fahhem.com>
[not found] ` <Z45ANK-m2XZazDi3@redhat.com>
2025-01-20 20:17 ` [PATCH] Skip resizing image to the same size Fahrzin Hemmati
2025-01-20 21:49 ` Michael Tokarev
2025-01-20 22:21 ` Fahrzin Hemmati
2025-01-21 11:40 ` Kevin Wolf
2025-01-21 15:30 ` Fahrzin Hemmati
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.