From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:52134)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <dgilbert@redhat.com>) id 1WEFBP-00075G-QD
	for qemu-devel@nongnu.org; Fri, 14 Feb 2014 04:35:48 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <dgilbert@redhat.com>) id 1WEFBK-0007Jw-Kx
	for qemu-devel@nongnu.org; Fri, 14 Feb 2014 04:35:43 -0500
Received: from mx1.redhat.com ([209.132.183.28]:60849)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <dgilbert@redhat.com>) id 1WEFBK-0007J1-3U
	for qemu-devel@nongnu.org; Fri, 14 Feb 2014 04:35:38 -0500
Date: Fri, 14 Feb 2014 09:35:14 +0000
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Message-ID: <20140214093514.GB2316@work-vm>
References: <1392320685-20609-1-git-send-email-dgilbert@redhat.com>
	<33183CC9F5247A488A2544077AF19020815C26F8@SZXEMA503-MBS.china.huawei.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <33183CC9F5247A488A2544077AF19020815C26F8@SZXEMA503-MBS.china.huawei.com>
Subject: Re: [Qemu-devel] [PATCH] Fix two XBZRLE corruption issues
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: "Gonglei (Arei)" <arei.gonglei@huawei.com>
Cc: "owasserm@redhat.com" <owasserm@redhat.com>, "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>, "quintela@redhat.com" <quintela@redhat.com>

* Gonglei (Arei) (arei.gonglei@huawei.com) wrote:
> 
> Best regards,
> -Gonglei
> 
> 

<snip>

> >              } else if (!ram_bulk_stage && migrate_use_xbzrle()) {
> > -                current_addr = block->offset + offset;
> >                  bytes_sent = save_xbzrle_page(f, p, current_addr, block,
> >                                                offset, cont,
> > last_stage);
> >                  if (!last_stage) {
> > +                    /* We must send exactly what's in the xbzrle cache
> > +                     * even if the page wasn't xbzrle compressed, so
> > that
> > +                     * it's right next time.
> > +                     */
> >                      p = get_cached_data(XBZRLE.cache, current_addr);
> > +
> > +                    /* Can't send this cached data async, since the cache
> > page
> > +                     * might get updated before it gets to the wire
> > +                     */
> > +                    send_async = false;
> >                  }
> >              }
> > 
> >              /* XBZRLE overflow or normal page */
> >              if (bytes_sent == -1) {
> >                  bytes_sent = save_block_hdr(f, block, offset, cont,
> > RAM_SAVE_FLAG_PAGE);
> > -                qemu_put_buffer_async(f, p, TARGET_PAGE_SIZE);
> > +                if (send_async) {
> > +                    qemu_put_buffer_async(f, p, TARGET_PAGE_SIZE);
> > +                } else {
> > +                    qemu_put_buffer(f, p, TARGET_PAGE_SIZE);
> > +                }
> >                  bytes_sent += TARGET_PAGE_SIZE;
> >                  acct_info.norm_pages++;
> >              }
> 
> if a page that was cached by XBZRLE but XBZRLE overflow,qemu should send the page in the cache rather then original page.Because the original page might change .

Which it already does - see the line above which is:
    p = get_cached_data(XBZRLE.cache, current_addr);

That changes the 'p' to point to the page in the cache and thus forces that
qemu_put_buffer to send the page in the cache; my patch doesn't change that,
it just stops it using the qemu_put_buffer_async so that the cache is read
immediately not at some point in the future when the cache may have changed.

Dave
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK