From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:59247) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QoaUa-0003CG-Rr for qemu-devel@nongnu.org; Wed, 03 Aug 2011 08:24:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QoaUZ-0000jq-JZ for qemu-devel@nongnu.org; Wed, 03 Aug 2011 08:24:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:6645) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QoaUZ-0000jj-6x for qemu-devel@nongnu.org; Wed, 03 Aug 2011 08:24:07 -0400 Date: Wed, 3 Aug 2011 15:24:41 +0300 From: "Michael S. Tsirkin" Message-ID: <20110803122441.GB10538@redhat.com> References: <4E36E5F7.6060202@cisco.com> <4E393575.4090003@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4E393575.4090003@redhat.com> Subject: Re: [Qemu-devel] qemu-kvm aborts - vhost_dev_unassign_memory: Assertion `to >= 0' failed. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Avi Kivity Cc: aliguori@us.ibm.com, qemu-devel , David Ahern , KVM mailing list On Wed, Aug 03, 2011 at 02:48:05PM +0300, Avi Kivity wrote: > On 08/01/2011 08:44 PM, David Ahern wrote: > >qemu-kvm.git as of: > > > >commit dacdc4b10bafbb21120e1c24a9665444768ef999 > >Merge: 7b69d4f 0af4922 > >Author: Avi Kivity > >Date: Sun Jul 31 11:42:26 2011 +0300 > > > > Merge branch 'upstream-merge' into next > > > >is aborting with the error: > > > >qemu-kvm: qemu-kvm.git/hw/vhost.c:123: vhost_dev_unassign_memory: > >Assertion `to>= 0' failed. > >Aborted > > > > It's a bug in vhost: > > /* Assign/unassign. Keep an unsorted array of non-overlapping > * memory regions in dev->mem. */ > static void vhost_dev_unassign_memory(struct vhost_dev *dev, > uint64_t start_addr, > uint64_t size) > { > int from, to, n = dev->mem->nregions; > /* Track overlapping/split regions for sanity checking. */ > int overlap_start = 0, overlap_end = 0, overlap_middle = 0, split = 0; > > for (from = 0, to = 0; from < n; ++from, ++to) { > struct vhost_memory_region *reg = dev->mem->regions + to; > uint64_t reglast; > uint64_t memlast; > uint64_t change; > > /* clone old region */ > if (to != from) { > memcpy(reg, dev->mem->regions + from, sizeof *reg); > } > > /* No overlap is simple */ > if (!ranges_overlap(reg->guest_phys_addr, reg->memory_size, > start_addr, size)) { > continue; > } > > /* Split only happens if supplied region > * is in the middle of an existing one. Thus it can not > * overlap with any other existing region. */ > assert(!split); > > reglast = range_get_last(reg->guest_phys_addr, reg->memory_size); > memlast = range_get_last(start_addr, size); > > /* Remove whole region */ > if (start_addr <= reg->guest_phys_addr && memlast >= reglast) { > --dev->mem->nregions; > --to; > assert(to >= 0); > ++overlap_middle; > continue; > } > > > We're removing the first region, and 'to' goes negative. Michael? Yes, that assert is wrong. ---> Subject: vhost: remove an incorrect assert The 'to' can go negative when the first region gets removed (it gets incremented by to 0 immediately afterward), which makes the assertion fail. Nothing breaks if to < 0 here so just remove the assert. Signed-off-by: Michael S. Tsirkin ---- diff --git a/hw/vhost.c b/hw/vhost.c index c3d8821..19e7255 100644 --- a/hw/vhost.c +++ b/hw/vhost.c @@ -120,7 +120,6 @@ static void vhost_dev_unassign_memory(struct vhost_dev *dev, if (start_addr <= reg->guest_phys_addr && memlast >= reglast) { --dev->mem->nregions; --to; - assert(to >= 0); ++overlap_middle; continue; }