----- Original Message -----
> From: "Atsushi Kumagai" <ats-kumagai at wm.jp.nec.com>
> To: "Pingfan Liu" <piliu at redhat.com>
> Cc: kexec at lists.infradead.org
> Sent: Wednesday, June 7, 2017 3:06:07 PM
> Subject: RE: [makedumpfile PATCH 2/2] print_info: show the remaining time of dump progress
>
> Hello Pingfan,
>
> >If the dump progress is slow, it is better to show an estimate of
> >remaining time of the progress to the user. The estimator simply uses
> >the average speed to work.
>
> I like it, thanks for your work.
> I have a minor comment.
>
> [snip]
> > void
> >-print_progress(const char *msg, unsigned long current, unsigned long end)
> >+print_progress(const char *msg, unsigned long current, unsigned long end,
> >struct timeval *start)
> > {
> > float progress;
> > time_t tm;
> > static time_t last_time = 0;
> > static unsigned int lapse = 0;
> > static const char *spinner = "/|\\-";
> >+ struct timeval delta;
> >+ double eta;
> >+ char eta_msg[32] = "eta: ";
>
> I thinks this "eta: " should be set in eta_to_human_short(), because...
>
> > if (current < end) {
> > tm = time(NULL);
> >@@ -368,13 +388,19 @@ print_progress(const char *msg, unsigned long current,
> >unsigned long end)
> > } else
> > progress = 100;
> >
> >+ if (start != NULL) {
> >+ calc_delta(start, &delta);
> >+ eta = delta.tv_sec + delta.tv_usec/1e6;
> >+ eta = (100 - progress) * eta/progress;
> >+ eta_to_human_short(eta, eta_msg+5);
>
> the offset for "eta: " is here as a magic number.
> It has low maintainability since the number is independent of
> the actual string length.
>
>
Thanks for your review. I will modify according to your comment.
Regards,
Pingfan
