[PATCH 0/2] tools: virsh-domain: display progress with enhanced granularity

Changes since v1 ================================================================================ - Remove superfluous parenthesis - Update progress as 99.99 instead of 99.00 - Add comment for auto-round-off prevention logic for double Description ================================================================================== While working with active blockcommit using virsh/libvirt, we found that in case of heavy IO, the progress may stay stuck at 99% for quite a while. It will be much better if we show progress upto 2 decimal places more precise which should be sufficient for most scenarios for now. This will give a user, a better understanding of the progress of blockjob/blockcommit etc This patch intends to do exactly that by adding more granularity to blockjob/blockcommit etc progress. Shaleen Bathla (2): tools: virsh-domain: refactor variable initialization tools: virsh-domain: display progress with enhanced granularity tools/virsh-domain.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) -- 2.31.1

Signed-off-by: Shaleen Bathla <shaleen.bathla@oracle.com> --- tools/virsh-domain.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 6850843a259f..165aa0ee0f19 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -1722,12 +1722,10 @@ static void virshPrintJobProgress(const char *label, unsigned long long remaining, unsigned long long total) { - int progress; + int progress = 100; - if (remaining == 0) { - /* migration has completed */ - progress = 100; - } else { + /* if remaining == 0 migration has completed */ + if (remaining != 0) { /* use float to avoid overflow */ progress = (int)(100.0 - remaining * 100.0 / total); if (progress >= 100) { -- 2.31.1

Switch from int to double for displaying job progress upto 2 decimal places. Signed-off-by: Shaleen Bathla <shaleen.bathla@oracle.com> --- tools/virsh-domain.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 165aa0ee0f19..af2c0842079c 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -1722,21 +1722,22 @@ static void virshPrintJobProgress(const char *label, unsigned long long remaining, unsigned long long total) { - int progress = 100; + double progress = 100.00; /* if remaining == 0 migration has completed */ if (remaining != 0) { - /* use float to avoid overflow */ - progress = (int)(100.0 - remaining * 100.0 / total); - if (progress >= 100) { + /* use double to avoid overflow */ + progress = 100.00 - remaining * 100.00 / total; + if (progress >= 100.00) { /* migration has not completed, do not print [100 %] */ - progress = 99; + progress = 99.99; } } /* see comments in vshError about why we must flush */ fflush(stdout); - fprintf(stderr, "\r%s: [%3d %%]", label, progress); + /* avoid auto-round-off of double by keeping only 2 decimals */ + fprintf(stderr, "\r%s: [%5.2f %%]", label, (int)(progress*100)/100.0); fflush(stderr); } -- 2.31.1

Missed adding V2 in subject title. Can someone please guide, if I should resend the patch series with the title correction or this can be taken forward ? Thanks and Regards, Shaleen Bathla -----Original Message----- From: Shaleen Bathla <shaleen.bathla@oracle.com> Sent: Wednesday, April 26, 2023 4:59 PM To: libvir-list@redhat.com Subject: [PATCH 0/2] tools: virsh-domain: display progress with enhanced granularity Changes since v1 ================================================================================ - Remove superfluous parenthesis - Update progress as 99.99 instead of 99.00 - Add comment for auto-round-off prevention logic for double Description ================================================================================== While working with active blockcommit using virsh/libvirt, we found that in case of heavy IO, the progress may stay stuck at 99% for quite a while. It will be much better if we show progress upto 2 decimal places more precise which should be sufficient for most scenarios for now. This will give a user, a better understanding of the progress of blockjob/blockcommit etc This patch intends to do exactly that by adding more granularity to blockjob/blockcommit etc progress. Shaleen Bathla (2): tools: virsh-domain: refactor variable initialization tools: virsh-domain: display progress with enhanced granularity tools/virsh-domain.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) -- 2.31.1

On Wed, Apr 26, 2023 at 11:51:28AM +0000, Shaleen Bathla wrote:
Missed adding V2 in subject title. Can someone please guide, if I should resend the patch series with the title correction or this can be taken forward ?
That's fine, no need to resend just for this, that can happen to anyone.
Thanks and Regards, Shaleen Bathla
-----Original Message----- From: Shaleen Bathla <shaleen.bathla@oracle.com> Sent: Wednesday, April 26, 2023 4:59 PM To: libvir-list@redhat.com Subject: [PATCH 0/2] tools: virsh-domain: display progress with enhanced granularity
Changes since v1 ================================================================================ - Remove superfluous parenthesis - Update progress as 99.99 instead of 99.00 - Add comment for auto-round-off prevention logic for double
Description ================================================================================== While working with active blockcommit using virsh/libvirt, we found that in case of heavy IO, the progress may stay stuck at 99% for quite a while. It will be much better if we show progress upto 2 decimal places more precise which should be sufficient for most scenarios for now. This will give a user, a better understanding of the progress of blockjob/blockcommit etc This patch intends to do exactly that by adding more granularity to blockjob/blockcommit etc progress.
Shaleen Bathla (2): tools: virsh-domain: refactor variable initialization tools: virsh-domain: display progress with enhanced granularity
tools/virsh-domain.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-)
-- 2.31.1

On 4/26/23 13:28, Shaleen Bathla wrote:
Changes since v1 ================================================================================ - Remove superfluous parenthesis - Update progress as 99.99 instead of 99.00 - Add comment for auto-round-off prevention logic for double
Description ================================================================================== While working with active blockcommit using virsh/libvirt, we found that in case of heavy IO, the progress may stay stuck at 99% for quite a while. It will be much better if we show progress upto 2 decimal places more precise which should be sufficient for most scenarios for now. This will give a user, a better understanding of the progress of blockjob/blockcommit etc This patch intends to do exactly that by adding more granularity to blockjob/blockcommit etc progress.
Shaleen Bathla (2): tools: virsh-domain: refactor variable initialization tools: virsh-domain: display progress with enhanced granularity
tools/virsh-domain.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> I'll let Martin share his thoughts though, since he reviewed v1. Michal

On Thu, Apr 27, 2023 at 09:56:39AM +0200, Michal Prívozník wrote:
On 4/26/23 13:28, Shaleen Bathla wrote:
Changes since v1 ================================================================================ - Remove superfluous parenthesis - Update progress as 99.99 instead of 99.00 - Add comment for auto-round-off prevention logic for double
Description ================================================================================== While working with active blockcommit using virsh/libvirt, we found that in case of heavy IO, the progress may stay stuck at 99% for quite a while. It will be much better if we show progress upto 2 decimal places more precise which should be sufficient for most scenarios for now. This will give a user, a better understanding of the progress of blockjob/blockcommit etc This patch intends to do exactly that by adding more granularity to blockjob/blockcommit etc progress.
Shaleen Bathla (2): tools: virsh-domain: refactor variable initialization tools: virsh-domain: display progress with enhanced granularity
tools/virsh-domain.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
I'll let Martin share his thoughts though, since he reviewed v1.
My review comments were worked in, so from me Reviewed-by: Martin Kletzander <mkletzan@redhat.com> and I'll push it after the release.
Michal
participants (3)
-
Martin Kletzander
-
Michal Prívozník
-
Shaleen Bathla