[PATCH 0/2] tools: virsh-domain: display progress with enhanced granularity
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 | 18 ++++++++---------- 1 file changed, 8 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 | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 165aa0ee0f19..9f82722b2ac6 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -1722,21 +1722,21 @@ static void virshPrintJobProgress(const char *label, unsigned long long remaining, unsigned long long total) { - int progress = 100; + double progress = 100.0; /* 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.0 - (remaining * 100.0 / total); + if (progress >= 100.0) { /* migration has not completed, do not print [100 %] */ - progress = 99; + progress = 99.0; } } /* see comments in vshError about why we must flush */ fflush(stdout); - fprintf(stderr, "\r%s: [%3d %%]", label, progress); + fprintf(stderr, "\r%s: [%5.2f %%]", label, (int)(progress*100)/100.0); fflush(stderr); } -- 2.31.1
On Wed, Apr 26, 2023 at 10:56:56AM +0530, Shaleen Bathla wrote:
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 | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 165aa0ee0f19..9f82722b2ac6 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -1722,21 +1722,21 @@ static void virshPrintJobProgress(const char *label, unsigned long long remaining, unsigned long long total) { - int progress = 100; + double progress = 100.0;
/* 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.0 - (remaining * 100.0 / total);
The parentheses are redundant here, but that's fine.
+ if (progress >= 100.0) { /* migration has not completed, do not print [100 %] */ - progress = 99; + progress = 99.0;
Maybe we want 99.99 here?
} }
/* see comments in vshError about why we must flush */ fflush(stdout); - fprintf(stderr, "\r%s: [%3d %%]", label, progress); + fprintf(stderr, "\r%s: [%5.2f %%]", label, (int)(progress*100)/100.0);
So this one took me a while, but I might've figured it out. Are you doing this so that the progress is not rounded up to 100.00 without linking against math library? If yes, then I'd suggest adding a comment here just so we know once we look at it again in few years time. Other than that the patches look fine to me.
fflush(stderr); }
-- 2.31.1
On 4/26/23 13:05, Martin Kletzander wrote:
On Wed, Apr 26, 2023 at 10:56:56AM +0530, Shaleen Bathla wrote:
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 | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 165aa0ee0f19..9f82722b2ac6 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -1722,21 +1722,21 @@ static void virshPrintJobProgress(const char *label, unsigned long long remaining, unsigned long long total) { - int progress = 100; + double progress = 100.0;
/* 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.0 - (remaining * 100.0 / total);
The parentheses are redundant here, but that's fine.
+ if (progress >= 100.0) { /* migration has not completed, do not print [100 %] */ - progress = 99; + progress = 99.0;
Maybe we want 99.99 here?
} }
/* see comments in vshError about why we must flush */ fflush(stdout); - fprintf(stderr, "\r%s: [%3d %%]", label, progress); + fprintf(stderr, "\r%s: [%5.2f %%]", label, (int)(progress*100)/100.0);
So this one took me a while, but I might've figured it out. Are you doing this so that the progress is not rounded up to 100.00 without linking against math library? If yes, then I'd suggest adding a comment here just so we know once we look at it again in few years time.
We already link with math library and for a good reason: we do use it internally: libvirt.git $ git grep math\.h but I don't mind this code. Michal
participants (3)
-
Martin Kletzander -
Michal Prívozník -
Shaleen Bathla