| Message ID | 1257519486-14786-1-git-send-email-quintela@redhat.com |
|---|---|
| State | New |
| Headers | show |
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.
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.
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.
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.
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.
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, ... */
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(-)