From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:33612) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SEGYf-0000mY-AC for qemu-devel@nongnu.org; Sun, 01 Apr 2012 04:54:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SEGYd-00067O-7j for qemu-devel@nongnu.org; Sun, 01 Apr 2012 04:54:44 -0400 Received: from mx1.redhat.com ([209.132.183.28]:1599) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SEGYc-00067I-W1 for qemu-devel@nongnu.org; Sun, 01 Apr 2012 04:54:43 -0400 Date: Sun, 1 Apr 2012 11:54:48 +0300 From: "Michael S. Tsirkin" Message-ID: <20120401085448.GA22071@redhat.com> References: <1324067607-17055-1-git-send-email-brogers@suse.com> <4F766A22.8030606@dreamhost.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4F766A22.8030606@dreamhost.com> Subject: Re: [Qemu-devel] [PATCH] vhost-net: Move asserts to after check for end < start List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Josh Durgin Cc: qemu-devel@nongnu.org, Bruce Rogers On Fri, Mar 30, 2012 at 07:21:22PM -0700, Josh Durgin wrote: > On 12/16/2011 12:33 PM, Bruce Rogers wrote: > >When migrating a vm using vhost-net we hit the following assertion: > > > >qemu-kvm: /usr/src/packages/BUILD/qemu-kvm-0.15.1/hw/vhost.c:30: > >vhost_dev_sync_region: Assertion `start / (0x1000 * (8 * > >sizeof(vhost_log_chunk_t)))< dev->log_size' failed. > > I consistently hit this assert while testing live migration with > qemu 1.0.1 and the master branch. Applying this patch allowed live > migration to complete successfully. Maybe it could be reviewed and > merged? Yes, thanks for the reminder. I've applied a patch by Alex Williamson that addresses this and other crashes. > >The cases which the end< start check is intended to catch, such as > >for vga video memory, will also likely trigger the assertion. > >Reorder the code to handle this correctly. > > > >Signed-off-by: Bruce Rogers > >--- > > hw/vhost.c | 4 ++-- > > 1 files changed, 2 insertions(+), 2 deletions(-) > > > >diff --git a/hw/vhost.c b/hw/vhost.c > >index 0870cb7..7309f71 100644 > >--- a/hw/vhost.c > >+++ b/hw/vhost.c > >@@ -26,11 +26,11 @@ static void vhost_dev_sync_region(struct vhost_dev *dev, > > vhost_log_chunk_t *to = dev->log + end / VHOST_LOG_CHUNK + 1; > > uint64_t addr = (start / VHOST_LOG_CHUNK) * VHOST_LOG_CHUNK; > > > >- assert(end / VHOST_LOG_CHUNK< dev->log_size); > >- assert(start / VHOST_LOG_CHUNK< dev->log_size); > > if (end< start) { > > return; > > } > >+ assert(end / VHOST_LOG_CHUNK< dev->log_size); > >+ assert(start / VHOST_LOG_CHUNK< dev->log_size); > > for (;from< to; ++from) { > > vhost_log_chunk_t log; > > int bit;