qemu-kvm aborts - vhost_dev_unassign_memory: Assertion `to >= 0' failed.

qemu-kvm aborts - vhost_dev_unassign_memory: Assertion `to >= 0' failed.

[David Ahern]

qemu-kvm.git as of:

commit dacdc4b10bafbb21120e1c24a9665444768ef999
Merge: 7b69d4f 0af4922
Author: Avi Kivity <avi@redhat.com>
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

$ git bisect bad
00cb2a99f5e7f73c4fff54ae16c7b6acf463ab5c is the first bad commit
commit 00cb2a99f5e7f73c4fff54ae16c7b6acf463ab5c
Author: Avi Kivity <avi@redhat.com>
Date:   Tue Jul 26 14:26:17 2011 +0300

    pc: convert pc_memory_init() to memory API

    Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
    Signed-off-by: Avi Kivity <avi@redhat.com>
    Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

:040000 040000 3d709c2cab75b934030fb9fbdfa99024c855b2a6
720d362f9702f15d16519093555182169d0b09bd M	hw

Re: qemu-kvm aborts - vhost_dev_unassign_memory: Assertion `to >= 0' failed.

[David Ahern]

On 08/01/2011 11:44 AM, David Ahern wrote:
> qemu-kvm.git as of:
> 
> commit dacdc4b10bafbb21120e1c24a9665444768ef999
> Merge: 7b69d4f 0af4922
> Author: Avi Kivity <avi@redhat.com>
> 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

BTW, happens when I pass a kernel and initrd on the command line.
qemu-kvm  -kernel vmlinuz -initrd initrd.img ...

David

> 
> $ git bisect bad
> 00cb2a99f5e7f73c4fff54ae16c7b6acf463ab5c is the first bad commit
> commit 00cb2a99f5e7f73c4fff54ae16c7b6acf463ab5c
> Author: Avi Kivity <avi@redhat.com>
> Date:   Tue Jul 26 14:26:17 2011 +0300
> 
>     pc: convert pc_memory_init() to memory API
> 
>     Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
>     Signed-off-by: Avi Kivity <avi@redhat.com>
>     Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> 
> :040000 040000 3d709c2cab75b934030fb9fbdfa99024c855b2a6
> 720d362f9702f15d16519093555182169d0b09bd M	hw
> 

Re: qemu-kvm aborts - vhost_dev_unassign_memory: Assertion `to >= 0' failed.

[Avi Kivity]

On 08/01/2011 08:44 PM, David Ahern wrote:
> qemu-kvm.git as of:
>
> commit dacdc4b10bafbb21120e1c24a9665444768ef999
> Merge: 7b69d4f 0af4922
> Author: Avi Kivity<avi@redhat.com>
> 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

Full command line please?

-- 
error compiling committee.c: too many arguments to function

Re: qemu-kvm aborts - vhost_dev_unassign_memory: Assertion `to >= 0' failed.

[Avi Kivity]

On 08/01/2011 08:44 PM, David Ahern wrote:
> qemu-kvm.git as of:
>
> commit dacdc4b10bafbb21120e1c24a9665444768ef999
> Merge: 7b69d4f 0af4922
> Author: Avi Kivity<avi@redhat.com>
> 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?

-- 
error compiling committee.c: too many arguments to function

Re: qemu-kvm aborts - vhost_dev_unassign_memory: Assertion `to >= 0' failed.

[Michael S. Tsirkin]

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<avi@redhat.com>
> >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 <mst@redhat.com>

----

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;
         }

Re: qemu-kvm aborts - vhost_dev_unassign_memory: Assertion `to >= 0' failed.

[David Ahern]

On 08/03/2011 06:24 AM, Michael S. Tsirkin wrote:
> 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<avi@redhat.com>
>>> 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 <mst@redhat.com>
> 
> ----
> 
> 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;
>          }
> 

Removing the assert appears to work fine.
Tested-by: David Ahern <daahern@cisco.com>

David

Re: qemu-kvm aborts - vhost_dev_unassign_memory: Assertion `to >= 0' failed.

[Michael S. Tsirkin]

On Wed, Aug 03, 2011 at 07:55:47AM -0600, David Ahern wrote:
> Tested-by: David Ahern <daahern@cisco.com>
> 
> David

Applied, thanks very much.

Re: qemu-kvm aborts - vhost_dev_unassign_memory: Assertion `to >= 0' failed.

[David Ahern]

On 08/03/2011 09:00 AM, Michael S. Tsirkin wrote:
> On Wed, Aug 03, 2011 at 07:55:47AM -0600, David Ahern wrote:
>> Tested-by: David Ahern <daahern@cisco.com>
>>
>> David
> 
> Applied, thanks very much.
> 

I assume this will make 0.15 since it is a regression? haven't seen the
patch applied to that branch yet.

David

Re: qemu-kvm aborts - vhost_dev_unassign_memory: Assertion `to >= 0' failed.

[Michael S. Tsirkin]

On Thu, Aug 04, 2011 at 12:48:49PM -0600, David Ahern wrote:
> 
> 
> On 08/03/2011 09:00 AM, Michael S. Tsirkin wrote:
> > On Wed, Aug 03, 2011 at 07:55:47AM -0600, David Ahern wrote:
> >> Tested-by: David Ahern <daahern@cisco.com>
> >>
> >> David
> > 
> > Applied, thanks very much.
> > 
> 
> I assume this will make 0.15 since it is a regression?

How is this a regression in 0.15? I thought it's an old bug
exposed by the memory API which I expect is not going into 0.15.

> haven't seen the
> patch applied to that branch yet.
> 
> David

My branch will have to get merged in trunk first.

-- 
MST

Re: qemu-kvm aborts - vhost_dev_unassign_memory: Assertion `to >= 0' failed.

[David Ahern]

On 08/04/2011 01:17 PM, Michael S. Tsirkin wrote:
> On Thu, Aug 04, 2011 at 12:48:49PM -0600, David Ahern wrote:
>>
>>
>> On 08/03/2011 09:00 AM, Michael S. Tsirkin wrote:
>>> On Wed, Aug 03, 2011 at 07:55:47AM -0600, David Ahern wrote:
>>>> Tested-by: David Ahern <daahern@cisco.com>
>>>>
>>>> David
>>>
>>> Applied, thanks very much.
>>>
>>
>> I assume this will make 0.15 since it is a regression?
> 
> How is this a regression in 0.15? I thought it's an old bug
> exposed by the memory API which I expect is not going into 0.15.

Is qemu-kvm.git not tagged as 0.15 rc?

David

> 
>> haven't seen the
>> patch applied to that branch yet.
>>
>> David
> 
> My branch will have to get merged in trunk first.
> 

Re: [Qemu-devel] qemu-kvm aborts - vhost_dev_unassign_memory: Assertion `to >= 0' failed.

[Michael S. Tsirkin]

On Mon, Aug 01, 2011 at 11:44:23AM -0600, David Ahern wrote:
> qemu-kvm.git as of:
> 
> commit dacdc4b10bafbb21120e1c24a9665444768ef999
> Merge: 7b69d4f 0af4922
> Author: Avi Kivity <avi@redhat.com>
> 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
> 
> $ git bisect bad
> 00cb2a99f5e7f73c4fff54ae16c7b6acf463ab5c is the first bad commit
> commit 00cb2a99f5e7f73c4fff54ae16c7b6acf463ab5c
> Author: Avi Kivity <avi@redhat.com>
> Date:   Tue Jul 26 14:26:17 2011 +0300
> 
>     pc: convert pc_memory_init() to memory API
> 
>     Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
>     Signed-off-by: Avi Kivity <avi@redhat.com>
>     Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> 
> :040000 040000 3d709c2cab75b934030fb9fbdfa99024c855b2a6
> 720d362f9702f15d16519093555182169d0b09bd M	hw
> 

Thanks for the report.
As Avi pointed out it's a vhost bug that got exposed
by a memory API change.
I have posted a patch which should fix that problem -
could you please try it and let me know?

Thanks!

-- 
MST