[makedumpfile PATCH 2/2] print_info: show the remaining time of dump progress

[Pingfan Liu]

----- 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