On Wed, Dec 02, 2015 at 09:32:57AM +0800, Fam Zheng wrote: > On Tue, 12/01 21:28, Peter Xu wrote: > > @@ -333,6 +333,8 @@ static void write_data(DumpState *s, void *buf, int > > length, Error **errp) > > if (ret < 0) { > > error_setg(errp, "dump: failed to save memory"); > > } > > + > > + s->written_size += length; > > If (ret < 0), the incremental is inaccurate, do we want an accurate > written_size in that case? I.e. put this in "else" branch?
Yes, I think I'd better put it into an else block... Thanks. Actually even that could not be accurate, since the write() could be partly done and failed. But I hope that's good enough for dump status queries. :) > > > } > > > > /* write the memory to vmcore. 1 page per I/O. */ > > @@ -1301,6 +1303,7 @@ static void write_dump_pages(DumpState *s, Error > > **errp) > > goto out; > > } > > } > > + s->written_size += TARGET_PAGE_SIZE; > > The same question applies here. For kdump case, it is using "goto out" when error happens. So it seems to have no problem here? > > > } > > > > ret = write_cache(&page_desc, NULL, 0, true); > > @@ -1433,6 +1436,30 @@ bool dump_in_progress(void) > > return (state->status == DUMP_STATUS_ACTIVE); > > } > > > > +/* calculate total size of memory to be dumped (taking filter into > > + * acoount.) */ > > +static size_t dump_calculate_size(DumpState *s) > > Is size_t big enough for 64 bit guest on 32 bit host (with 4 bytes size_t)? Thanks to point out. Will use int64_t here (and also written_size and total_size). > > + size_t written_size; /* written memory size (in bytes), > > + * this could be used to calculate > > + * how many work we have > > s/many/much/ Will fix it. Thanks! Peter