[Qemu-devel] [PATCH] Fix incoming migration

[Qemu-devel] [PATCH] Fix incoming migration

[Juan Quintela]

commit b04c4134d6de28c249277de19e523bfbe4aebbd6
broke incoming migration.  After talking with Gleb, code was intended
to be the way is in this fix.  This fixes migration here.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 savevm.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/savevm.c b/savevm.c
index b7abf43..fd98ccd 100644
--- a/savevm.c
+++ b/savevm.c
@@ -126,6 +126,8 @@ static int announce_self_create(uint8_t *buf,
 static void qemu_announce_self_once(void *opaque)
 {
     int i, len;
+    VLANState *vlan;
+    VLANClientState *vc;
     uint8_t buf[60];
     static int count = SELF_ANNOUNCE_ROUNDS;
     QEMUTimer *timer = *(QEMUTimer **)opaque;
@@ -134,7 +136,10 @@ static void qemu_announce_self_once(void *opaque)
         if (!nd_table[i].used)
             continue;
         len = announce_self_create(buf, nd_table[i].macaddr);
-        qemu_send_packet_raw(nd_table[i].vc, buf, len);
+        vlan = nd_table[i].vlan;
+        QTAILQ_FOREACH(vc, &vlan->clients, next) {
+            qemu_send_packet_raw(vc, buf, len);
+        }
     }
     if (--count) {
         /* delay 50ms, 150ms, 250ms, ... */
-- 
1.6.2.5

Re: [Qemu-devel] [PATCH] Fix incoming migration

[Mark McLoughlin]

On Fri, 2009-11-06 at 15:58 +0100, Juan Quintela wrote:
> commit b04c4134d6de28c249277de19e523bfbe4aebbd6
> broke incoming migration.  After talking with Gleb, code was intended
> to be the way is in this fix.  This fixes migration here.

Tried to reproduce and it works fine for me. More details?

> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  savevm.c |    7 ++++++-
>  1 files changed, 6 insertions(+), 1 deletions(-)
> 
> diff --git a/savevm.c b/savevm.c
> index b7abf43..fd98ccd 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -126,6 +126,8 @@ static int announce_self_create(uint8_t *buf,
>  static void qemu_announce_self_once(void *opaque)
>  {
>      int i, len;
> +    VLANState *vlan;
> +    VLANClientState *vc;
>      uint8_t buf[60];
>      static int count = SELF_ANNOUNCE_ROUNDS;
>      QEMUTimer *timer = *(QEMUTimer **)opaque;
> @@ -134,7 +136,10 @@ static void qemu_announce_self_once(void *opaque)
>          if (!nd_table[i].used)
>              continue;
>          len = announce_self_create(buf, nd_table[i].macaddr);
> -        qemu_send_packet_raw(nd_table[i].vc, buf, len);
> +        vlan = nd_table[i].vlan;
> +        QTAILQ_FOREACH(vc, &vlan->clients, next) {
> +            qemu_send_packet_raw(vc, buf, len);
> +        }

A NIC isn't necessarily connected to a vlan any more, which is why the
change was made.

With your patch, I'd expect it to crash if you used -netdev rather than
-net

Cheers,
Mark.

[Qemu-devel] Re: [PATCH] Fix incoming migration

[Juan Quintela]

Mark McLoughlin <markmc@redhat.com> wrote:
> On Fri, 2009-11-06 at 15:58 +0100, Juan Quintela wrote:
>> commit b04c4134d6de28c249277de19e523bfbe4aebbd6
>> broke incoming migration.  After talking with Gleb, code was intended
>> to be the way is in this fix.  This fixes migration here.
>
> Tried to reproduce and it works fine for me. More details?
>
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  savevm.c |    7 ++++++-
>>  1 files changed, 6 insertions(+), 1 deletions(-)
>> 
>> diff --git a/savevm.c b/savevm.c
>> index b7abf43..fd98ccd 100644
>> --- a/savevm.c
>> +++ b/savevm.c
>> @@ -126,6 +126,8 @@ static int announce_self_create(uint8_t *buf,
>>  static void qemu_announce_self_once(void *opaque)
>>  {
>>      int i, len;
>> +    VLANState *vlan;
>> +    VLANClientState *vc;
>>      uint8_t buf[60];
>>      static int count = SELF_ANNOUNCE_ROUNDS;
>>      QEMUTimer *timer = *(QEMUTimer **)opaque;
>> @@ -134,7 +136,10 @@ static void qemu_announce_self_once(void *opaque)
>>          if (!nd_table[i].used)
>>              continue;
>>          len = announce_self_create(buf, nd_table[i].macaddr);
>> -        qemu_send_packet_raw(nd_table[i].vc, buf, len);
>> +        vlan = nd_table[i].vlan;
>> +        QTAILQ_FOREACH(vc, &vlan->clients, next) {
>> +            qemu_send_packet_raw(vc, buf, len);
>> +        }
>
> A NIC isn't necessarily connected to a vlan any more, which is why the
> change was made.
>
> With your patch, I'd expect it to crash if you used -netdev rather than
> -net

Without this patch, migration don't work at all.  nd_table[i].vc is NULL
at this point.  Any better idea?

Later, Juan.

Re: [Qemu-devel] [PATCH] Fix incoming migration

[Glauber Costa]

On Tue, Nov 10, 2009 at 04:47:59PM +0000, Mark McLoughlin wrote:
> On Fri, 2009-11-06 at 15:58 +0100, Juan Quintela wrote:
> > commit b04c4134d6de28c249277de19e523bfbe4aebbd6
> > broke incoming migration.  After talking with Gleb, code was intended
> > to be the way is in this fix.  This fixes migration here.
> 
> Tried to reproduce and it works fine for me. More details?
> 
> > Signed-off-by: Juan Quintela <quintela@redhat.com>
> > ---
> >  savevm.c |    7 ++++++-
> >  1 files changed, 6 insertions(+), 1 deletions(-)
> > 
> > diff --git a/savevm.c b/savevm.c
> > index b7abf43..fd98ccd 100644
> > --- a/savevm.c
> > +++ b/savevm.c
> > @@ -126,6 +126,8 @@ static int announce_self_create(uint8_t *buf,
> >  static void qemu_announce_self_once(void *opaque)
> >  {
> >      int i, len;
> > +    VLANState *vlan;
> > +    VLANClientState *vc;
> >      uint8_t buf[60];
> >      static int count = SELF_ANNOUNCE_ROUNDS;
> >      QEMUTimer *timer = *(QEMUTimer **)opaque;
> > @@ -134,7 +136,10 @@ static void qemu_announce_self_once(void *opaque)
> >          if (!nd_table[i].used)
> >              continue;
> >          len = announce_self_create(buf, nd_table[i].macaddr);
> > -        qemu_send_packet_raw(nd_table[i].vc, buf, len);
> > +        vlan = nd_table[i].vlan;
> > +        QTAILQ_FOREACH(vc, &vlan->clients, next) {
> > +            qemu_send_packet_raw(vc, buf, len);
> > +        }
> 
> A NIC isn't necessarily connected to a vlan any more, which is why the
> change was made.
> 
> With your patch, I'd expect it to crash if you used -netdev rather than
> -net
> 
It crashes for me too, and I acknowledge that this patch fixes it.

I was able to reproduce it with my default network settings, which includes
a tap device, if it matters.

[Qemu-devel] Re: [PATCH] Fix incoming migration

[Mark McLoughlin]

On Tue, 2009-11-10 at 18:03 +0100, Juan Quintela wrote:
> Mark McLoughlin <markmc@redhat.com> wrote:
> > On Fri, 2009-11-06 at 15:58 +0100, Juan Quintela wrote:
> >> commit b04c4134d6de28c249277de19e523bfbe4aebbd6
> >> broke incoming migration.  After talking with Gleb, code was intended
> >> to be the way is in this fix.  This fixes migration here.
> >
> > Tried to reproduce and it works fine for me. More details?
> >
> >> Signed-off-by: Juan Quintela <quintela@redhat.com>
> >> ---
> >>  savevm.c |    7 ++++++-
> >>  1 files changed, 6 insertions(+), 1 deletions(-)
> >> 
> >> diff --git a/savevm.c b/savevm.c
> >> index b7abf43..fd98ccd 100644
> >> --- a/savevm.c
> >> +++ b/savevm.c
> >> @@ -126,6 +126,8 @@ static int announce_self_create(uint8_t *buf,
> >>  static void qemu_announce_self_once(void *opaque)
> >>  {
> >>      int i, len;
> >> +    VLANState *vlan;
> >> +    VLANClientState *vc;
> >>      uint8_t buf[60];
> >>      static int count = SELF_ANNOUNCE_ROUNDS;
> >>      QEMUTimer *timer = *(QEMUTimer **)opaque;
> >> @@ -134,7 +136,10 @@ static void qemu_announce_self_once(void *opaque)
> >>          if (!nd_table[i].used)
> >>              continue;
> >>          len = announce_self_create(buf, nd_table[i].macaddr);
> >> -        qemu_send_packet_raw(nd_table[i].vc, buf, len);
> >> +        vlan = nd_table[i].vlan;
> >> +        QTAILQ_FOREACH(vc, &vlan->clients, next) {
> >> +            qemu_send_packet_raw(vc, buf, len);
> >> +        }
> >
> > A NIC isn't necessarily connected to a vlan any more, which is why the
> > change was made.
> >
> > With your patch, I'd expect it to crash if you used -netdev rather than
> > -net
> 
> Without this patch, migration don't work at all.  nd_table[i].vc is NULL
> at this point.  Any better idea?

Right, I can't reproduce that. Exactly what command line did you use?

Thanks,
Mark.

[Qemu-devel] Re: [PATCH] Fix incoming migration

[Juan Quintela]

Mark McLoughlin <markmc@redhat.com> wrote:
> On Tue, 2009-11-10 at 18:03 +0100, Juan Quintela wrote:
>> Mark McLoughlin <markmc@redhat.com> wrote:
>> > On Fri, 2009-11-06 at 15:58 +0100, Juan Quintela wrote:
>> >> commit b04c4134d6de28c249277de19e523bfbe4aebbd6
>> >> broke incoming migration.  After talking with Gleb, code was intended
>> >> to be the way is in this fix.  This fixes migration here.
>> >
>> > Tried to reproduce and it works fine for me. More details?
>> >
>> >> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> >> ---
>> >>  savevm.c |    7 ++++++-
>> >>  1 files changed, 6 insertions(+), 1 deletions(-)
>> >> 
>> >> diff --git a/savevm.c b/savevm.c
>> >> index b7abf43..fd98ccd 100644
>> >> --- a/savevm.c
>> >> +++ b/savevm.c
>> >> @@ -126,6 +126,8 @@ static int announce_self_create(uint8_t *buf,
>> >>  static void qemu_announce_self_once(void *opaque)
>> >>  {
>> >>      int i, len;
>> >> +    VLANState *vlan;
>> >> +    VLANClientState *vc;
>> >>      uint8_t buf[60];
>> >>      static int count = SELF_ANNOUNCE_ROUNDS;
>> >>      QEMUTimer *timer = *(QEMUTimer **)opaque;
>> >> @@ -134,7 +136,10 @@ static void qemu_announce_self_once(void *opaque)
>> >>          if (!nd_table[i].used)
>> >>              continue;
>> >>          len = announce_self_create(buf, nd_table[i].macaddr);
>> >> -        qemu_send_packet_raw(nd_table[i].vc, buf, len);
>> >> +        vlan = nd_table[i].vlan;
>> >> +        QTAILQ_FOREACH(vc, &vlan->clients, next) {
>> >> +            qemu_send_packet_raw(vc, buf, len);
>> >> +        }
>> >
>> > A NIC isn't necessarily connected to a vlan any more, which is why the
>> > change was made.
>> >
>> > With your patch, I'd expect it to crash if you used -netdev rather than
>> > -net
>> 
>> Without this patch, migration don't work at all.  nd_table[i].vc is NULL
>> at this point.  Any better idea?
>
> Right, I can't reproduce that. Exactly what command line did you use?

/scratch/qemu/x86_64-softmmu/qemu-system-x86_64 -M pc -m 512 -smp 1
-name test -drive file=/scratch/tmp/f11b.img,if=ide -drive
file=/scratch/tmp/temp.img,if=virtio -net
nic,macaddr=54:52:00:53:7e:5b,vlan=0,model=virtio -net
tap,script=/etc/kvm-ifup,vlan=0,ifname=vnet1,downscript=no --monitor
stdio --serial telnet:0:5553,server --virtioconsole telnet:0:5554,server
--enable-kvm -usbdevice tablet  -soundhw ac97 -loadvm virtio

virtio is an image saved with the same command line.

(I am rebasing to upstream, but I am getting a different error)

qemu) 
Program received signal SIGSEGV, Segmentation fault.
0x00000000004ddb53 in qdev_reset (opaque=0xd32cc0)
    at /scratch/qemu/hw/qdev.c:228
228	    if (dev->info->reset)
(gdb) bt
#0  0x00000000004ddb53 in qdev_reset (opaque=0xd32cc0)
    at /scratch/qemu/hw/qdev.c:228
#1  0x000000000040ce9d in qemu_system_reset () at /scratch/qemu/vl.c:3240
#2  0x000000000040da71 in main_loop () at /scratch/qemu/vl.c:4047
#3  0x0000000000411452 in main (argc=30, argv=0x7fffffffe388, 
    envp=0x7fffffffe480) at /scratch/qemu/vl.c:5909
(gdb) p *dev
$1 = {id = 0x0, 
state = DEV_STATE_INITIALIZED, opts = 0x0, hotplugged = 0, 
  info = 0x10000007f7ee2a0, parent_bus = 0x137f80, num_gpio_out = 0, 
  gpio_out = 0x0, num_gpio_in = 0, gpio_in = 0x0, child_bus = {
    lh_first = 0x0}, num_child_bus = 0, sibling = {le_next = 0xd0a8d0, 
    le_prev = 0xcf6400}}
(gdb) 

Will try again once that I have fixed that one.

Later, Juan.