On Wed, 12/02 18:41, Peter Xu wrote:
> On Wed, Dec 02, 2015 at 05:49:18PM +0800, Fam Zheng wrote:
> > On Wed, 12/02 16:49, Peter Xu wrote:
> > > On Wed, Dec 02, 2015 at 09:32:57AM +0800, Fam Zheng wrote:
> > > > > @@ -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?
> >
> > write_cache can fail after you increment it here, no?
>
> I am adding it at the end of loop. It looks like:
>
> while (get_next_page(&block_iter, &pfn_iter, &buf, s)) {
> if (is_zero_page(buf, TARGET_PAGE_SIZE)) {
> ret = write_cache(&page_desc, &pd_zero, sizeof(PageDescriptor),
> false);
> if (ret < 0) {
> error_setg(errp, "XXXXXXXX");
> goto out;
> }
> } else {
> ...
> ret = write_cache(&page_desc, &pd, sizeof(PageDescriptor), false);
> if (ret < 0) {
> error_setg(errp, "XXXXXXXX");
> goto out;
> }
> ...
> }
> s->written_size += TARGET_PAGE_SIZE;
> }
>
> Label "out" is out of the loop. So, when error happens, it sets the
> errp and directly jump out of the loop. Did I miss anything?
>
You are right, I misread the context of this incremental. Thanks.
Fam