| Message ID | 1495649128-10529-4-git-send-email-vyasevic@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 2017年05月25日 02:05, Vladislav Yasevich wrote: > Switch qemu_announce_self and virtio annoucements to use > the announcement timer framework. This makes sure that both > timers use the same timeouts and number of annoucement attempts > > Based on work by Dr. David Alan Gilbert <dgilbert@redhat.com> > > Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> > --- > hw/net/virtio-net.c | 28 ++++++++++++++++------------ > include/hw/virtio/virtio-net.h | 3 +-- > include/migration/vmstate.h | 17 +++++++++++------ > include/sysemu/sysemu.h | 2 +- > migration/migration.c | 2 +- > migration/savevm.c | 28 ++++++++++++++-------------- > 6 files changed, 44 insertions(+), 36 deletions(-) > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > index 7d091c9..1c65825 100644 > --- a/hw/net/virtio-net.c > +++ b/hw/net/virtio-net.c > @@ -25,6 +25,7 @@ > #include "qapi/qmp/qjson.h" > #include "qapi-event.h" > #include "hw/virtio/virtio-access.h" > +#include "migration/migration.h" > > #define VIRTIO_NET_VM_VERSION 11 > > @@ -115,7 +116,7 @@ static void virtio_net_announce_timer(void *opaque) > VirtIONet *n = opaque; > VirtIODevice *vdev = VIRTIO_DEVICE(n); > > - n->announce_counter--; > + n->announce_timer->round--; > n->status |= VIRTIO_NET_S_ANNOUNCE; > virtio_notify_config(vdev); > } > @@ -427,8 +428,8 @@ static void virtio_net_reset(VirtIODevice *vdev) > n->nobcast = 0; > /* multiqueue is disabled by default */ > n->curr_queues = 1; > - timer_del(n->announce_timer); > - n->announce_counter = 0; > + timer_del(n->announce_timer->tm); > + n->announce_timer->round = 0; > n->status &= ~VIRTIO_NET_S_ANNOUNCE; > > /* Flush any MAC and VLAN filter table state */ > @@ -872,10 +873,10 @@ static int virtio_net_handle_announce(VirtIONet *n, uint8_t cmd, > if (cmd == VIRTIO_NET_CTRL_ANNOUNCE_ACK && > n->status & VIRTIO_NET_S_ANNOUNCE) { > n->status &= ~VIRTIO_NET_S_ANNOUNCE; > - if (n->announce_counter) { > - timer_mod(n->announce_timer, > + if (n->announce_timer->round) { > + timer_mod(n->announce_timer->tm, > qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + > - self_announce_delay(n->announce_counter)); > + self_announce_delay(n->announce_timer)); > } > return VIRTIO_NET_OK; > } else { > @@ -1615,8 +1616,8 @@ static int virtio_net_post_load_device(void *opaque, int version_id) > > if (virtio_vdev_has_feature(vdev, VIRTIO_NET_F_GUEST_ANNOUNCE) && > virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ)) { > - n->announce_counter = SELF_ANNOUNCE_ROUNDS; > - timer_mod(n->announce_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL)); > + n->announce_timer->round = n->announce_timer->params.rounds; > + timer_mod(n->announce_timer->tm, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL)); > } > > return 0; > @@ -1938,8 +1939,10 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp) > qemu_macaddr_default_if_unset(&n->nic_conf.macaddr); > memcpy(&n->mac[0], &n->nic_conf.macaddr, sizeof(n->mac)); > n->status = VIRTIO_NET_S_LINK_UP; > - n->announce_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL, > - virtio_net_announce_timer, n); > + n->announce_timer = qemu_announce_timer_new(qemu_get_announce_params(), > + QEMU_CLOCK_VIRTUAL); > + n->announce_timer->tm = timer_new_ms(QEMU_CLOCK_VIRTUAL, > + virtio_net_announce_timer, n); > > if (n->netclient_type) { > /* > @@ -2001,8 +2004,9 @@ static void virtio_net_device_unrealize(DeviceState *dev, Error **errp) > virtio_net_del_queue(n, i); > } > > - timer_del(n->announce_timer); > - timer_free(n->announce_timer); > + timer_del(n->announce_timer->tm); > + timer_free(n->announce_timer->tm); > + g_free(n->announce_timer); > g_free(n->vqs); > qemu_del_nic(n->nic); > virtio_cleanup(vdev); > diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h > index 1eec9a2..51dd4c3 100644 > --- a/include/hw/virtio/virtio-net.h > +++ b/include/hw/virtio/virtio-net.h > @@ -94,8 +94,7 @@ typedef struct VirtIONet { > char *netclient_name; > char *netclient_type; > uint64_t curr_guest_offloads; > - QEMUTimer *announce_timer; > - int announce_counter; > + AnnounceTimer *announce_timer; > bool needs_vnet_hdr_swap; > } VirtIONet; > > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h > index a6bf84d..f8aed9b 100644 > --- a/include/migration/vmstate.h > +++ b/include/migration/vmstate.h > @@ -1022,8 +1022,6 @@ extern const VMStateInfo vmstate_info_qtailq; > #define VMSTATE_END_OF_LIST() \ > {} > > -#define SELF_ANNOUNCE_ROUNDS 5 > - > void loadvm_free_handlers(MigrationIncomingState *mis); > > int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd, > @@ -1071,11 +1069,18 @@ AnnounceTimer *qemu_announce_timer_create(AnnounceParameters *params, > QEMUTimerCB *cb); > > static inline > -int64_t self_announce_delay(int round) > +int64_t self_announce_delay(AnnounceTimer *timer) > { > - assert(round < SELF_ANNOUNCE_ROUNDS && round > 0); > - /* delay 50ms, 150ms, 250ms, ... */ > - return 50 + (SELF_ANNOUNCE_ROUNDS - round - 1) * 100; > + int64_t ret; > + > + ret = timer->params.initial + > + (timer->params.rounds - timer->round - 1) * > + timer->params.step; > + > + if (ret < 0 || ret > timer->params.max) { > + ret = timer->params.max; > + } Can we move this check to qemu_validate_announce_parameters()? > + return ret; > } > > void dump_vmstate_json_to_file(FILE *out_fp); > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h > index 7fd49c4..2ef1687 100644 > --- a/include/sysemu/sysemu.h > +++ b/include/sysemu/sysemu.h > @@ -85,7 +85,7 @@ bool qemu_validate_announce_parameters(AnnounceParameters *params, > Error **errp); > void qemu_set_announce_parameters(AnnounceParameters *announce_params, > AnnounceParameters *params); > -void qemu_announce_self(void); > +void qemu_announce_self(AnnounceParameters *params); > > /* Subcommands for QEMU_VM_COMMAND */ > enum qemu_vm_cmd { > diff --git a/migration/migration.c b/migration/migration.c > index 0304c01..987c1cf 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -345,7 +345,7 @@ static void process_incoming_migration_bh(void *opaque) > * This must happen after all error conditions are dealt with and > * we're sure the VM is going to be running on this host. > */ > - qemu_announce_self(); > + qemu_announce_self(qemu_get_announce_params()); > > /* If global state section was not received or we are in running > state, we need to obey autostart. Any other state is set with > diff --git a/migration/savevm.c b/migration/savevm.c > index 607b090..555157a 100644 > --- a/migration/savevm.c > +++ b/migration/savevm.c > @@ -212,21 +212,19 @@ static void qemu_announce_self_iter(NICState *nic, void *opaque) > qemu_send_packet_raw(qemu_get_queue(nic), buf, len); > } > > - > static void qemu_announce_self_once(void *opaque) > { > - static int count = SELF_ANNOUNCE_ROUNDS; > - QEMUTimer *timer = *(QEMUTimer **)opaque; > + AnnounceTimer *timer = (AnnounceTimer *)opaque; > > qemu_foreach_nic(qemu_announce_self_iter, NULL); > > - if (--count) { > - /* delay 50ms, 150ms, 250ms, ... */ > - timer_mod(timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + > - self_announce_delay(count)); > + if (--timer->round) { > + timer_mod(timer->tm, qemu_clock_get_ms(timer->type) + > + self_announce_delay(timer)); > } else { > - timer_del(timer); > - timer_free(timer); > + timer_del(timer->tm); > + timer_free(timer->tm); > + g_free(timer); > } > } > > @@ -252,11 +250,13 @@ AnnounceTimer *qemu_announce_timer_create(AnnounceParameters *params, > return timer; > } > > -void qemu_announce_self(void) > +void qemu_announce_self(AnnounceParameters *params) > { > - static QEMUTimer *timer; > - timer = timer_new_ms(QEMU_CLOCK_REALTIME, qemu_announce_self_once, &timer); > - qemu_announce_self_once(&timer); > + AnnounceTimer *timer; > + > + timer = qemu_announce_timer_create(params, QEMU_CLOCK_REALTIME, > + qemu_announce_self_once); > + qemu_announce_self_once(timer); > } > > /***********************************************************/ > @@ -1730,7 +1730,7 @@ static void loadvm_postcopy_handle_run_bh(void *opaque) > */ > cpu_synchronize_all_post_init(); > > - qemu_announce_self(); > + qemu_announce_self(qemu_get_announce_params()); > > /* Make sure all file formats flush their mutable metadata. > * If we get an error here, just don't restart the VM yet. */ Reviewed-by: Jason Wang <jasowang@redhat.com>
On 05/26/2017 12:16 AM, Jason Wang wrote: > > > On 2017年05月25日 02:05, Vladislav Yasevich wrote: >> Switch qemu_announce_self and virtio annoucements to use >> the announcement timer framework. This makes sure that both >> timers use the same timeouts and number of annoucement attempts >> >> Based on work by Dr. David Alan Gilbert <dgilbert@redhat.com> >> >> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> >> --- >> hw/net/virtio-net.c | 28 ++++++++++++++++------------ >> include/hw/virtio/virtio-net.h | 3 +-- >> include/migration/vmstate.h | 17 +++++++++++------ >> include/sysemu/sysemu.h | 2 +- >> migration/migration.c | 2 +- >> migration/savevm.c | 28 ++++++++++++++-------------- >> 6 files changed, 44 insertions(+), 36 deletions(-) >> >> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c >> index 7d091c9..1c65825 100644 >> --- a/hw/net/virtio-net.c >> +++ b/hw/net/virtio-net.c >> @@ -25,6 +25,7 @@ >> #include "qapi/qmp/qjson.h" >> #include "qapi-event.h" >> #include "hw/virtio/virtio-access.h" >> +#include "migration/migration.h" >> #define VIRTIO_NET_VM_VERSION 11 >> @@ -115,7 +116,7 @@ static void virtio_net_announce_timer(void *opaque) >> VirtIONet *n = opaque; >> VirtIODevice *vdev = VIRTIO_DEVICE(n); >> - n->announce_counter--; >> + n->announce_timer->round--; >> n->status |= VIRTIO_NET_S_ANNOUNCE; >> virtio_notify_config(vdev); >> } >> @@ -427,8 +428,8 @@ static void virtio_net_reset(VirtIODevice *vdev) >> n->nobcast = 0; >> /* multiqueue is disabled by default */ >> n->curr_queues = 1; >> - timer_del(n->announce_timer); >> - n->announce_counter = 0; >> + timer_del(n->announce_timer->tm); >> + n->announce_timer->round = 0; >> n->status &= ~VIRTIO_NET_S_ANNOUNCE; >> /* Flush any MAC and VLAN filter table state */ >> @@ -872,10 +873,10 @@ static int virtio_net_handle_announce(VirtIONet *n, uint8_t cmd, >> if (cmd == VIRTIO_NET_CTRL_ANNOUNCE_ACK && >> n->status & VIRTIO_NET_S_ANNOUNCE) { >> n->status &= ~VIRTIO_NET_S_ANNOUNCE; >> - if (n->announce_counter) { >> - timer_mod(n->announce_timer, >> + if (n->announce_timer->round) { >> + timer_mod(n->announce_timer->tm, >> qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + >> - self_announce_delay(n->announce_counter)); >> + self_announce_delay(n->announce_timer)); >> } >> return VIRTIO_NET_OK; >> } else { >> @@ -1615,8 +1616,8 @@ static int virtio_net_post_load_device(void *opaque, int version_id) >> if (virtio_vdev_has_feature(vdev, VIRTIO_NET_F_GUEST_ANNOUNCE) && >> virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ)) { >> - n->announce_counter = SELF_ANNOUNCE_ROUNDS; >> - timer_mod(n->announce_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL)); >> + n->announce_timer->round = n->announce_timer->params.rounds; >> + timer_mod(n->announce_timer->tm, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL)); >> } >> return 0; >> @@ -1938,8 +1939,10 @@ static void virtio_net_device_realize(DeviceState *dev, Error >> **errp) >> qemu_macaddr_default_if_unset(&n->nic_conf.macaddr); >> memcpy(&n->mac[0], &n->nic_conf.macaddr, sizeof(n->mac)); >> n->status = VIRTIO_NET_S_LINK_UP; >> - n->announce_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL, >> - virtio_net_announce_timer, n); >> + n->announce_timer = qemu_announce_timer_new(qemu_get_announce_params(), >> + QEMU_CLOCK_VIRTUAL); >> + n->announce_timer->tm = timer_new_ms(QEMU_CLOCK_VIRTUAL, >> + virtio_net_announce_timer, n); >> if (n->netclient_type) { >> /* >> @@ -2001,8 +2004,9 @@ static void virtio_net_device_unrealize(DeviceState *dev, Error >> **errp) >> virtio_net_del_queue(n, i); >> } >> - timer_del(n->announce_timer); >> - timer_free(n->announce_timer); >> + timer_del(n->announce_timer->tm); >> + timer_free(n->announce_timer->tm); >> + g_free(n->announce_timer); >> g_free(n->vqs); >> qemu_del_nic(n->nic); >> virtio_cleanup(vdev); >> diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h >> index 1eec9a2..51dd4c3 100644 >> --- a/include/hw/virtio/virtio-net.h >> +++ b/include/hw/virtio/virtio-net.h >> @@ -94,8 +94,7 @@ typedef struct VirtIONet { >> char *netclient_name; >> char *netclient_type; >> uint64_t curr_guest_offloads; >> - QEMUTimer *announce_timer; >> - int announce_counter; >> + AnnounceTimer *announce_timer; >> bool needs_vnet_hdr_swap; >> } VirtIONet; >> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h >> index a6bf84d..f8aed9b 100644 >> --- a/include/migration/vmstate.h >> +++ b/include/migration/vmstate.h >> @@ -1022,8 +1022,6 @@ extern const VMStateInfo vmstate_info_qtailq; >> #define VMSTATE_END_OF_LIST() \ >> {} >> -#define SELF_ANNOUNCE_ROUNDS 5 >> - >> void loadvm_free_handlers(MigrationIncomingState *mis); >> int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd, >> @@ -1071,11 +1069,18 @@ AnnounceTimer *qemu_announce_timer_create(AnnounceParameters >> *params, >> QEMUTimerCB *cb); >> static inline >> -int64_t self_announce_delay(int round) >> +int64_t self_announce_delay(AnnounceTimer *timer) >> { >> - assert(round < SELF_ANNOUNCE_ROUNDS && round > 0); >> - /* delay 50ms, 150ms, 250ms, ... */ >> - return 50 + (SELF_ANNOUNCE_ROUNDS - round - 1) * 100; >> + int64_t ret; >> + >> + ret = timer->params.initial + >> + (timer->params.rounds - timer->round - 1) * >> + timer->params.step; >> + >> + if (ret < 0 || ret > timer->params.max) { >> + ret = timer->params.max; >> + } > > Can we move this check to qemu_validate_announce_parameters()? Not really. Consider a situation where you don't really want to wait for more the then say 5 seconds between announcements, but you want to increase the number of announcement attempts (since you are not sure how long it will take to plumb the whole network path). In this case, you will hit the max delay before hitting max rounds, and the code above makes sure that we don't exceed max delay. I can update the names to make it clearer, if it will help. Thanks -vlad > >> + return ret; >> } >> void dump_vmstate_json_to_file(FILE *out_fp); >> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h >> index 7fd49c4..2ef1687 100644 >> --- a/include/sysemu/sysemu.h >> +++ b/include/sysemu/sysemu.h >> @@ -85,7 +85,7 @@ bool qemu_validate_announce_parameters(AnnounceParameters *params, >> Error **errp); >> void qemu_set_announce_parameters(AnnounceParameters *announce_params, >> AnnounceParameters *params); >> -void qemu_announce_self(void); >> +void qemu_announce_self(AnnounceParameters *params); >> /* Subcommands for QEMU_VM_COMMAND */ >> enum qemu_vm_cmd { >> diff --git a/migration/migration.c b/migration/migration.c >> index 0304c01..987c1cf 100644 >> --- a/migration/migration.c >> +++ b/migration/migration.c >> @@ -345,7 +345,7 @@ static void process_incoming_migration_bh(void *opaque) >> * This must happen after all error conditions are dealt with and >> * we're sure the VM is going to be running on this host. >> */ >> - qemu_announce_self(); >> + qemu_announce_self(qemu_get_announce_params()); >> /* If global state section was not received or we are in running >> state, we need to obey autostart. Any other state is set with >> diff --git a/migration/savevm.c b/migration/savevm.c >> index 607b090..555157a 100644 >> --- a/migration/savevm.c >> +++ b/migration/savevm.c >> @@ -212,21 +212,19 @@ static void qemu_announce_self_iter(NICState *nic, void *opaque) >> qemu_send_packet_raw(qemu_get_queue(nic), buf, len); >> } >> - >> static void qemu_announce_self_once(void *opaque) >> { >> - static int count = SELF_ANNOUNCE_ROUNDS; >> - QEMUTimer *timer = *(QEMUTimer **)opaque; >> + AnnounceTimer *timer = (AnnounceTimer *)opaque; >> qemu_foreach_nic(qemu_announce_self_iter, NULL); >> - if (--count) { >> - /* delay 50ms, 150ms, 250ms, ... */ >> - timer_mod(timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + >> - self_announce_delay(count)); >> + if (--timer->round) { >> + timer_mod(timer->tm, qemu_clock_get_ms(timer->type) + >> + self_announce_delay(timer)); >> } else { >> - timer_del(timer); >> - timer_free(timer); >> + timer_del(timer->tm); >> + timer_free(timer->tm); >> + g_free(timer); >> } >> } >> @@ -252,11 +250,13 @@ AnnounceTimer *qemu_announce_timer_create(AnnounceParameters >> *params, >> return timer; >> } >> -void qemu_announce_self(void) >> +void qemu_announce_self(AnnounceParameters *params) >> { >> - static QEMUTimer *timer; >> - timer = timer_new_ms(QEMU_CLOCK_REALTIME, qemu_announce_self_once, &timer); >> - qemu_announce_self_once(&timer); >> + AnnounceTimer *timer; >> + >> + timer = qemu_announce_timer_create(params, QEMU_CLOCK_REALTIME, >> + qemu_announce_self_once); >> + qemu_announce_self_once(timer); >> } >> /***********************************************************/ >> @@ -1730,7 +1730,7 @@ static void loadvm_postcopy_handle_run_bh(void *opaque) >> */ >> cpu_synchronize_all_post_init(); >> - qemu_announce_self(); >> + qemu_announce_self(qemu_get_announce_params()); >> /* Make sure all file formats flush their mutable metadata. >> * If we get an error here, just don't restart the VM yet. */ > > Reviewed-by: Jason Wang <jasowang@redhat.com> >
Vladislav Yasevich <vyasevic@redhat.com> wrote: > Switch qemu_announce_self and virtio annoucements to use > the announcement timer framework. This makes sure that both > timers use the same timeouts and number of annoucement attempts > > Based on work by Dr. David Alan Gilbert <dgilbert@redhat.com> > > Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> > static void qemu_announce_self_once(void *opaque) > { > - static int count = SELF_ANNOUNCE_ROUNDS; > - QEMUTimer *timer = *(QEMUTimer **)opaque; > + AnnounceTimer *timer = (AnnounceTimer *)opaque; Cast from void * is never needed.
On 05/30/2017 06:10 AM, Juan Quintela wrote: > Vladislav Yasevich <vyasevic@redhat.com> wrote: >> Switch qemu_announce_self and virtio annoucements to use >> the announcement timer framework. This makes sure that both >> timers use the same timeouts and number of annoucement attempts >> >> Based on work by Dr. David Alan Gilbert <dgilbert@redhat.com> >> >> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> > >> static void qemu_announce_self_once(void *opaque) >> { >> - static int count = SELF_ANNOUNCE_ROUNDS; >> - QEMUTimer *timer = *(QEMUTimer **)opaque; >> + AnnounceTimer *timer = (AnnounceTimer *)opaque; > > Cast from void * is never needed. > OK Thanks -vlad
* Vladislav Yasevich (vyasevic@redhat.com) wrote: > Switch qemu_announce_self and virtio annoucements to use > the announcement timer framework. This makes sure that both > timers use the same timeouts and number of annoucement attempts > > Based on work by Dr. David Alan Gilbert <dgilbert@redhat.com> > > Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> > --- > hw/net/virtio-net.c | 28 ++++++++++++++++------------ > include/hw/virtio/virtio-net.h | 3 +-- > include/migration/vmstate.h | 17 +++++++++++------ > include/sysemu/sysemu.h | 2 +- > migration/migration.c | 2 +- > migration/savevm.c | 28 ++++++++++++++-------------- > 6 files changed, 44 insertions(+), 36 deletions(-) > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > index 7d091c9..1c65825 100644 > --- a/hw/net/virtio-net.c > +++ b/hw/net/virtio-net.c > @@ -25,6 +25,7 @@ > #include "qapi/qmp/qjson.h" > #include "qapi-event.h" > #include "hw/virtio/virtio-access.h" > +#include "migration/migration.h" > > #define VIRTIO_NET_VM_VERSION 11 > > @@ -115,7 +116,7 @@ static void virtio_net_announce_timer(void *opaque) > VirtIONet *n = opaque; > VirtIODevice *vdev = VIRTIO_DEVICE(n); > > - n->announce_counter--; > + n->announce_timer->round--; > n->status |= VIRTIO_NET_S_ANNOUNCE; > virtio_notify_config(vdev); > } > @@ -427,8 +428,8 @@ static void virtio_net_reset(VirtIODevice *vdev) > n->nobcast = 0; > /* multiqueue is disabled by default */ > n->curr_queues = 1; > - timer_del(n->announce_timer); > - n->announce_counter = 0; > + timer_del(n->announce_timer->tm); > + n->announce_timer->round = 0; > n->status &= ~VIRTIO_NET_S_ANNOUNCE; > > /* Flush any MAC and VLAN filter table state */ > @@ -872,10 +873,10 @@ static int virtio_net_handle_announce(VirtIONet *n, uint8_t cmd, > if (cmd == VIRTIO_NET_CTRL_ANNOUNCE_ACK && > n->status & VIRTIO_NET_S_ANNOUNCE) { > n->status &= ~VIRTIO_NET_S_ANNOUNCE; > - if (n->announce_counter) { > - timer_mod(n->announce_timer, > + if (n->announce_timer->round) { > + timer_mod(n->announce_timer->tm, > qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + > - self_announce_delay(n->announce_counter)); > + self_announce_delay(n->announce_timer)); > } > return VIRTIO_NET_OK; > } else { > @@ -1615,8 +1616,8 @@ static int virtio_net_post_load_device(void *opaque, int version_id) > > if (virtio_vdev_has_feature(vdev, VIRTIO_NET_F_GUEST_ANNOUNCE) && > virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ)) { > - n->announce_counter = SELF_ANNOUNCE_ROUNDS; > - timer_mod(n->announce_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL)); > + n->announce_timer->round = n->announce_timer->params.rounds; > + timer_mod(n->announce_timer->tm, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL)); Do you think it's worth having a qemu_announce_timer_init(AnnounceTimer *, QEMU_CLOCK_*) that would initialise the round and any other values and do the initial timer_mod ? > } > > return 0; > @@ -1938,8 +1939,10 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp) > qemu_macaddr_default_if_unset(&n->nic_conf.macaddr); > memcpy(&n->mac[0], &n->nic_conf.macaddr, sizeof(n->mac)); > n->status = VIRTIO_NET_S_LINK_UP; > - n->announce_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL, > - virtio_net_announce_timer, n); > + n->announce_timer = qemu_announce_timer_new(qemu_get_announce_params(), > + QEMU_CLOCK_VIRTUAL); > + n->announce_timer->tm = timer_new_ms(QEMU_CLOCK_VIRTUAL, > + virtio_net_announce_timer, n); > > if (n->netclient_type) { > /* > @@ -2001,8 +2004,9 @@ static void virtio_net_device_unrealize(DeviceState *dev, Error **errp) > virtio_net_del_queue(n, i); > } > > - timer_del(n->announce_timer); > - timer_free(n->announce_timer); > + timer_del(n->announce_timer->tm); > + timer_free(n->announce_timer->tm); > + g_free(n->announce_timer); Hmm, why is this all safe - I guess this is in the BQL? > g_free(n->vqs); > qemu_del_nic(n->nic); > virtio_cleanup(vdev); > diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h > index 1eec9a2..51dd4c3 100644 > --- a/include/hw/virtio/virtio-net.h > +++ b/include/hw/virtio/virtio-net.h > @@ -94,8 +94,7 @@ typedef struct VirtIONet { > char *netclient_name; > char *netclient_type; > uint64_t curr_guest_offloads; > - QEMUTimer *announce_timer; > - int announce_counter; > + AnnounceTimer *announce_timer; > bool needs_vnet_hdr_swap; > } VirtIONet; > > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h > index a6bf84d..f8aed9b 100644 > --- a/include/migration/vmstate.h > +++ b/include/migration/vmstate.h > @@ -1022,8 +1022,6 @@ extern const VMStateInfo vmstate_info_qtailq; > #define VMSTATE_END_OF_LIST() \ > {} > > -#define SELF_ANNOUNCE_ROUNDS 5 > - > void loadvm_free_handlers(MigrationIncomingState *mis); > > int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd, > @@ -1071,11 +1069,18 @@ AnnounceTimer *qemu_announce_timer_create(AnnounceParameters *params, > QEMUTimerCB *cb); > > static inline > -int64_t self_announce_delay(int round) > +int64_t self_announce_delay(AnnounceTimer *timer) > { > - assert(round < SELF_ANNOUNCE_ROUNDS && round > 0); > - /* delay 50ms, 150ms, 250ms, ... */ > - return 50 + (SELF_ANNOUNCE_ROUNDS - round - 1) * 100; > + int64_t ret; > + > + ret = timer->params.initial + > + (timer->params.rounds - timer->round - 1) * > + timer->params.step; > + > + if (ret < 0 || ret > timer->params.max) { > + ret = timer->params.max; > + } > + return ret; > } This feels like it should be with the rest of your code somewhere? > void dump_vmstate_json_to_file(FILE *out_fp); > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h > index 7fd49c4..2ef1687 100644 > --- a/include/sysemu/sysemu.h > +++ b/include/sysemu/sysemu.h > @@ -85,7 +85,7 @@ bool qemu_validate_announce_parameters(AnnounceParameters *params, > Error **errp); > void qemu_set_announce_parameters(AnnounceParameters *announce_params, > AnnounceParameters *params); > -void qemu_announce_self(void); > +void qemu_announce_self(AnnounceParameters *params); > > /* Subcommands for QEMU_VM_COMMAND */ > enum qemu_vm_cmd { > diff --git a/migration/migration.c b/migration/migration.c > index 0304c01..987c1cf 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -345,7 +345,7 @@ static void process_incoming_migration_bh(void *opaque) > * This must happen after all error conditions are dealt with and > * we're sure the VM is going to be running on this host. > */ > - qemu_announce_self(); > + qemu_announce_self(qemu_get_announce_params()); > > /* If global state section was not received or we are in running > state, we need to obey autostart. Any other state is set with > diff --git a/migration/savevm.c b/migration/savevm.c > index 607b090..555157a 100644 > --- a/migration/savevm.c > +++ b/migration/savevm.c > @@ -212,21 +212,19 @@ static void qemu_announce_self_iter(NICState *nic, void *opaque) > qemu_send_packet_raw(qemu_get_queue(nic), buf, len); > } > > - > static void qemu_announce_self_once(void *opaque) > { > - static int count = SELF_ANNOUNCE_ROUNDS; > - QEMUTimer *timer = *(QEMUTimer **)opaque; > + AnnounceTimer *timer = (AnnounceTimer *)opaque; > > qemu_foreach_nic(qemu_announce_self_iter, NULL); > > - if (--count) { > - /* delay 50ms, 150ms, 250ms, ... */ > - timer_mod(timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + > - self_announce_delay(count)); > + if (--timer->round) { > + timer_mod(timer->tm, qemu_clock_get_ms(timer->type) + > + self_announce_delay(timer)); > } else { > - timer_del(timer); > - timer_free(timer); > + timer_del(timer->tm); > + timer_free(timer->tm); > + g_free(timer); That's kind of odd in that it doesn't cause the thing the opaque pointed to, to be zero'd, so problem if someone free'd it in an exit path? But probably OK in this case. Dave > } > } > > @@ -252,11 +250,13 @@ AnnounceTimer *qemu_announce_timer_create(AnnounceParameters *params, > return timer; > } > > -void qemu_announce_self(void) > +void qemu_announce_self(AnnounceParameters *params) > { > - static QEMUTimer *timer; > - timer = timer_new_ms(QEMU_CLOCK_REALTIME, qemu_announce_self_once, &timer); > - qemu_announce_self_once(&timer); > + AnnounceTimer *timer; > + > + timer = qemu_announce_timer_create(params, QEMU_CLOCK_REALTIME, > + qemu_announce_self_once); > + qemu_announce_self_once(timer); > } > > /***********************************************************/ > @@ -1730,7 +1730,7 @@ static void loadvm_postcopy_handle_run_bh(void *opaque) > */ > cpu_synchronize_all_post_init(); > > - qemu_announce_self(); > + qemu_announce_self(qemu_get_announce_params()); > > /* Make sure all file formats flush their mutable metadata. > * If we get an error here, just don't restart the VM yet. */ > -- > 2.7.4 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On 05/30/2017 03:19 PM, Dr. David Alan Gilbert wrote: > * Vladislav Yasevich (vyasevic@redhat.com) wrote: >> Switch qemu_announce_self and virtio annoucements to use >> the announcement timer framework. This makes sure that both >> timers use the same timeouts and number of annoucement attempts >> >> Based on work by Dr. David Alan Gilbert <dgilbert@redhat.com> >> >> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> >> --- >> hw/net/virtio-net.c | 28 ++++++++++++++++------------ >> include/hw/virtio/virtio-net.h | 3 +-- >> include/migration/vmstate.h | 17 +++++++++++------ >> include/sysemu/sysemu.h | 2 +- >> migration/migration.c | 2 +- >> migration/savevm.c | 28 ++++++++++++++-------------- >> 6 files changed, 44 insertions(+), 36 deletions(-) >> >> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c >> index 7d091c9..1c65825 100644 >> --- a/hw/net/virtio-net.c >> +++ b/hw/net/virtio-net.c >> @@ -25,6 +25,7 @@ >> #include "qapi/qmp/qjson.h" >> #include "qapi-event.h" >> #include "hw/virtio/virtio-access.h" >> +#include "migration/migration.h" >> >> #define VIRTIO_NET_VM_VERSION 11 >> >> @@ -115,7 +116,7 @@ static void virtio_net_announce_timer(void *opaque) >> VirtIONet *n = opaque; >> VirtIODevice *vdev = VIRTIO_DEVICE(n); >> >> - n->announce_counter--; >> + n->announce_timer->round--; >> n->status |= VIRTIO_NET_S_ANNOUNCE; >> virtio_notify_config(vdev); >> } >> @@ -427,8 +428,8 @@ static void virtio_net_reset(VirtIODevice *vdev) >> n->nobcast = 0; >> /* multiqueue is disabled by default */ >> n->curr_queues = 1; >> - timer_del(n->announce_timer); >> - n->announce_counter = 0; >> + timer_del(n->announce_timer->tm); >> + n->announce_timer->round = 0; >> n->status &= ~VIRTIO_NET_S_ANNOUNCE; >> >> /* Flush any MAC and VLAN filter table state */ >> @@ -872,10 +873,10 @@ static int virtio_net_handle_announce(VirtIONet *n, uint8_t cmd, >> if (cmd == VIRTIO_NET_CTRL_ANNOUNCE_ACK && >> n->status & VIRTIO_NET_S_ANNOUNCE) { >> n->status &= ~VIRTIO_NET_S_ANNOUNCE; >> - if (n->announce_counter) { >> - timer_mod(n->announce_timer, >> + if (n->announce_timer->round) { >> + timer_mod(n->announce_timer->tm, >> qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + >> - self_announce_delay(n->announce_counter)); >> + self_announce_delay(n->announce_timer)); >> } >> return VIRTIO_NET_OK; >> } else { >> @@ -1615,8 +1616,8 @@ static int virtio_net_post_load_device(void *opaque, int version_id) >> >> if (virtio_vdev_has_feature(vdev, VIRTIO_NET_F_GUEST_ANNOUNCE) && >> virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ)) { >> - n->announce_counter = SELF_ANNOUNCE_ROUNDS; >> - timer_mod(n->announce_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL)); >> + n->announce_timer->round = n->announce_timer->params.rounds; >> + timer_mod(n->announce_timer->tm, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL)); > > Do you think it's worth having a > qemu_announce_timer_init(AnnounceTimer *, QEMU_CLOCK_*) > that would initialise the round and any other values and > do the initial timer_mod ? I had that initially, but ended up with just 1 user (virtio). So removed. I think I might put it back. It's really a announce_timer_start() type thing. May be it'll convert both places to use that api to make it cleaner. > >> } >> >> return 0; >> @@ -1938,8 +1939,10 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp) >> qemu_macaddr_default_if_unset(&n->nic_conf.macaddr); >> memcpy(&n->mac[0], &n->nic_conf.macaddr, sizeof(n->mac)); >> n->status = VIRTIO_NET_S_LINK_UP; >> - n->announce_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL, >> - virtio_net_announce_timer, n); >> + n->announce_timer = qemu_announce_timer_new(qemu_get_announce_params(), >> + QEMU_CLOCK_VIRTUAL); >> + n->announce_timer->tm = timer_new_ms(QEMU_CLOCK_VIRTUAL, >> + virtio_net_announce_timer, n); >> >> if (n->netclient_type) { >> /* >> @@ -2001,8 +2004,9 @@ static void virtio_net_device_unrealize(DeviceState *dev, Error **errp) >> virtio_net_del_queue(n, i); >> } >> >> - timer_del(n->announce_timer); >> - timer_free(n->announce_timer); >> + timer_del(n->announce_timer->tm); >> + timer_free(n->announce_timer->tm); >> + g_free(n->announce_timer); > > Hmm, why is this all safe - I guess this is in the BQL? Well, this is virito's explicit timer. I didn't check, but it really no different then destroying the virtio-specific QEMU timer created for the devices. > >> g_free(n->vqs); >> qemu_del_nic(n->nic); >> virtio_cleanup(vdev); >> diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h >> index 1eec9a2..51dd4c3 100644 >> --- a/include/hw/virtio/virtio-net.h >> +++ b/include/hw/virtio/virtio-net.h >> @@ -94,8 +94,7 @@ typedef struct VirtIONet { >> char *netclient_name; >> char *netclient_type; >> uint64_t curr_guest_offloads; >> - QEMUTimer *announce_timer; >> - int announce_counter; >> + AnnounceTimer *announce_timer; >> bool needs_vnet_hdr_swap; >> } VirtIONet; >> >> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h >> index a6bf84d..f8aed9b 100644 >> --- a/include/migration/vmstate.h >> +++ b/include/migration/vmstate.h >> @@ -1022,8 +1022,6 @@ extern const VMStateInfo vmstate_info_qtailq; >> #define VMSTATE_END_OF_LIST() \ >> {} >> >> -#define SELF_ANNOUNCE_ROUNDS 5 >> - >> void loadvm_free_handlers(MigrationIncomingState *mis); >> >> int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd, >> @@ -1071,11 +1069,18 @@ AnnounceTimer *qemu_announce_timer_create(AnnounceParameters *params, >> QEMUTimerCB *cb); >> >> static inline >> -int64_t self_announce_delay(int round) >> +int64_t self_announce_delay(AnnounceTimer *timer) >> { >> - assert(round < SELF_ANNOUNCE_ROUNDS && round > 0); >> - /* delay 50ms, 150ms, 250ms, ... */ >> - return 50 + (SELF_ANNOUNCE_ROUNDS - round - 1) * 100; >> + int64_t ret; >> + >> + ret = timer->params.initial + >> + (timer->params.rounds - timer->round - 1) * >> + timer->params.step; >> + >> + if (ret < 0 || ret > timer->params.max) { >> + ret = timer->params.max; >> + } >> + return ret; >> } > > This feels like it should be with the rest of your code somewhere? You had it moved into the vmstate.c with your patches. I left it in vmstate.h, but as Juan mentioned, this should all be moved together under net somewhere. I think I'll do that for the next series. > >> void dump_vmstate_json_to_file(FILE *out_fp); >> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h >> index 7fd49c4..2ef1687 100644 >> --- a/include/sysemu/sysemu.h >> +++ b/include/sysemu/sysemu.h >> @@ -85,7 +85,7 @@ bool qemu_validate_announce_parameters(AnnounceParameters *params, >> Error **errp); >> void qemu_set_announce_parameters(AnnounceParameters *announce_params, >> AnnounceParameters *params); >> -void qemu_announce_self(void); >> +void qemu_announce_self(AnnounceParameters *params); >> >> /* Subcommands for QEMU_VM_COMMAND */ >> enum qemu_vm_cmd { >> diff --git a/migration/migration.c b/migration/migration.c >> index 0304c01..987c1cf 100644 >> --- a/migration/migration.c >> +++ b/migration/migration.c >> @@ -345,7 +345,7 @@ static void process_incoming_migration_bh(void *opaque) >> * This must happen after all error conditions are dealt with and >> * we're sure the VM is going to be running on this host. >> */ >> - qemu_announce_self(); >> + qemu_announce_self(qemu_get_announce_params()); >> >> /* If global state section was not received or we are in running >> state, we need to obey autostart. Any other state is set with >> diff --git a/migration/savevm.c b/migration/savevm.c >> index 607b090..555157a 100644 >> --- a/migration/savevm.c >> +++ b/migration/savevm.c >> @@ -212,21 +212,19 @@ static void qemu_announce_self_iter(NICState *nic, void *opaque) >> qemu_send_packet_raw(qemu_get_queue(nic), buf, len); >> } >> >> - >> static void qemu_announce_self_once(void *opaque) >> { >> - static int count = SELF_ANNOUNCE_ROUNDS; >> - QEMUTimer *timer = *(QEMUTimer **)opaque; >> + AnnounceTimer *timer = (AnnounceTimer *)opaque; >> >> qemu_foreach_nic(qemu_announce_self_iter, NULL); >> >> - if (--count) { >> - /* delay 50ms, 150ms, 250ms, ... */ >> - timer_mod(timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + >> - self_announce_delay(count)); >> + if (--timer->round) { >> + timer_mod(timer->tm, qemu_clock_get_ms(timer->type) + >> + self_announce_delay(timer)); >> } else { >> - timer_del(timer); >> - timer_free(timer); >> + timer_del(timer->tm); >> + timer_free(timer->tm); >> + g_free(timer); > > That's kind of odd in that it doesn't cause the thing the opaque > pointed to, to be zero'd, so problem if someone free'd it > in an exit path? But probably OK in this case. > In this case, the opaque is the AnnounceTimer itself. So we end up: +-> Announce_timer: +----> QemuTimer: | tm -----------+ opaque --+ | | +----------------------------------------+ zero-ing opaque is a really qemu-timers job, but it doesn't do so, and in this case, it's OK (sort of), since we call timer_free(). Thanks -vlad > Dave > >> } >> } >> >> @@ -252,11 +250,13 @@ AnnounceTimer *qemu_announce_timer_create(AnnounceParameters *params, >> return timer; >> } >> >> -void qemu_announce_self(void) >> +void qemu_announce_self(AnnounceParameters *params) >> { >> - static QEMUTimer *timer; >> - timer = timer_new_ms(QEMU_CLOCK_REALTIME, qemu_announce_self_once, &timer); >> - qemu_announce_self_once(&timer); >> + AnnounceTimer *timer; >> + >> + timer = qemu_announce_timer_create(params, QEMU_CLOCK_REALTIME, >> + qemu_announce_self_once); >> + qemu_announce_self_once(timer); >> } >> >> /***********************************************************/ >> @@ -1730,7 +1730,7 @@ static void loadvm_postcopy_handle_run_bh(void *opaque) >> */ >> cpu_synchronize_all_post_init(); >> >> - qemu_announce_self(); >> + qemu_announce_self(qemu_get_announce_params()); >> >> /* Make sure all file formats flush their mutable metadata. >> * If we get an error here, just don't restart the VM yet. */ >> -- >> 2.7.4 >> > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK >
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 7d091c9..1c65825 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -25,6 +25,7 @@ #include "qapi/qmp/qjson.h" #include "qapi-event.h" #include "hw/virtio/virtio-access.h" +#include "migration/migration.h" #define VIRTIO_NET_VM_VERSION 11 @@ -115,7 +116,7 @@ static void virtio_net_announce_timer(void *opaque) VirtIONet *n = opaque; VirtIODevice *vdev = VIRTIO_DEVICE(n); - n->announce_counter--; + n->announce_timer->round--; n->status |= VIRTIO_NET_S_ANNOUNCE; virtio_notify_config(vdev); } @@ -427,8 +428,8 @@ static void virtio_net_reset(VirtIODevice *vdev) n->nobcast = 0; /* multiqueue is disabled by default */ n->curr_queues = 1; - timer_del(n->announce_timer); - n->announce_counter = 0; + timer_del(n->announce_timer->tm); + n->announce_timer->round = 0; n->status &= ~VIRTIO_NET_S_ANNOUNCE; /* Flush any MAC and VLAN filter table state */ @@ -872,10 +873,10 @@ static int virtio_net_handle_announce(VirtIONet *n, uint8_t cmd, if (cmd == VIRTIO_NET_CTRL_ANNOUNCE_ACK && n->status & VIRTIO_NET_S_ANNOUNCE) { n->status &= ~VIRTIO_NET_S_ANNOUNCE; - if (n->announce_counter) { - timer_mod(n->announce_timer, + if (n->announce_timer->round) { + timer_mod(n->announce_timer->tm, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + - self_announce_delay(n->announce_counter)); + self_announce_delay(n->announce_timer)); } return VIRTIO_NET_OK; } else { @@ -1615,8 +1616,8 @@ static int virtio_net_post_load_device(void *opaque, int version_id) if (virtio_vdev_has_feature(vdev, VIRTIO_NET_F_GUEST_ANNOUNCE) && virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ)) { - n->announce_counter = SELF_ANNOUNCE_ROUNDS; - timer_mod(n->announce_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL)); + n->announce_timer->round = n->announce_timer->params.rounds; + timer_mod(n->announce_timer->tm, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL)); } return 0; @@ -1938,8 +1939,10 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp) qemu_macaddr_default_if_unset(&n->nic_conf.macaddr); memcpy(&n->mac[0], &n->nic_conf.macaddr, sizeof(n->mac)); n->status = VIRTIO_NET_S_LINK_UP; - n->announce_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL, - virtio_net_announce_timer, n); + n->announce_timer = qemu_announce_timer_new(qemu_get_announce_params(), + QEMU_CLOCK_VIRTUAL); + n->announce_timer->tm = timer_new_ms(QEMU_CLOCK_VIRTUAL, + virtio_net_announce_timer, n); if (n->netclient_type) { /* @@ -2001,8 +2004,9 @@ static void virtio_net_device_unrealize(DeviceState *dev, Error **errp) virtio_net_del_queue(n, i); } - timer_del(n->announce_timer); - timer_free(n->announce_timer); + timer_del(n->announce_timer->tm); + timer_free(n->announce_timer->tm); + g_free(n->announce_timer); g_free(n->vqs); qemu_del_nic(n->nic); virtio_cleanup(vdev); diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h index 1eec9a2..51dd4c3 100644 --- a/include/hw/virtio/virtio-net.h +++ b/include/hw/virtio/virtio-net.h @@ -94,8 +94,7 @@ typedef struct VirtIONet { char *netclient_name; char *netclient_type; uint64_t curr_guest_offloads; - QEMUTimer *announce_timer; - int announce_counter; + AnnounceTimer *announce_timer; bool needs_vnet_hdr_swap; } VirtIONet; diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h index a6bf84d..f8aed9b 100644 --- a/include/migration/vmstate.h +++ b/include/migration/vmstate.h @@ -1022,8 +1022,6 @@ extern const VMStateInfo vmstate_info_qtailq; #define VMSTATE_END_OF_LIST() \ {} -#define SELF_ANNOUNCE_ROUNDS 5 - void loadvm_free_handlers(MigrationIncomingState *mis); int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd, @@ -1071,11 +1069,18 @@ AnnounceTimer *qemu_announce_timer_create(AnnounceParameters *params, QEMUTimerCB *cb); static inline -int64_t self_announce_delay(int round) +int64_t self_announce_delay(AnnounceTimer *timer) { - assert(round < SELF_ANNOUNCE_ROUNDS && round > 0); - /* delay 50ms, 150ms, 250ms, ... */ - return 50 + (SELF_ANNOUNCE_ROUNDS - round - 1) * 100; + int64_t ret; + + ret = timer->params.initial + + (timer->params.rounds - timer->round - 1) * + timer->params.step; + + if (ret < 0 || ret > timer->params.max) { + ret = timer->params.max; + } + return ret; } void dump_vmstate_json_to_file(FILE *out_fp); diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index 7fd49c4..2ef1687 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -85,7 +85,7 @@ bool qemu_validate_announce_parameters(AnnounceParameters *params, Error **errp); void qemu_set_announce_parameters(AnnounceParameters *announce_params, AnnounceParameters *params); -void qemu_announce_self(void); +void qemu_announce_self(AnnounceParameters *params); /* Subcommands for QEMU_VM_COMMAND */ enum qemu_vm_cmd { diff --git a/migration/migration.c b/migration/migration.c index 0304c01..987c1cf 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -345,7 +345,7 @@ static void process_incoming_migration_bh(void *opaque) * This must happen after all error conditions are dealt with and * we're sure the VM is going to be running on this host. */ - qemu_announce_self(); + qemu_announce_self(qemu_get_announce_params()); /* If global state section was not received or we are in running state, we need to obey autostart. Any other state is set with diff --git a/migration/savevm.c b/migration/savevm.c index 607b090..555157a 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -212,21 +212,19 @@ static void qemu_announce_self_iter(NICState *nic, void *opaque) qemu_send_packet_raw(qemu_get_queue(nic), buf, len); } - static void qemu_announce_self_once(void *opaque) { - static int count = SELF_ANNOUNCE_ROUNDS; - QEMUTimer *timer = *(QEMUTimer **)opaque; + AnnounceTimer *timer = (AnnounceTimer *)opaque; qemu_foreach_nic(qemu_announce_self_iter, NULL); - if (--count) { - /* delay 50ms, 150ms, 250ms, ... */ - timer_mod(timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + - self_announce_delay(count)); + if (--timer->round) { + timer_mod(timer->tm, qemu_clock_get_ms(timer->type) + + self_announce_delay(timer)); } else { - timer_del(timer); - timer_free(timer); + timer_del(timer->tm); + timer_free(timer->tm); + g_free(timer); } } @@ -252,11 +250,13 @@ AnnounceTimer *qemu_announce_timer_create(AnnounceParameters *params, return timer; } -void qemu_announce_self(void) +void qemu_announce_self(AnnounceParameters *params) { - static QEMUTimer *timer; - timer = timer_new_ms(QEMU_CLOCK_REALTIME, qemu_announce_self_once, &timer); - qemu_announce_self_once(&timer); + AnnounceTimer *timer; + + timer = qemu_announce_timer_create(params, QEMU_CLOCK_REALTIME, + qemu_announce_self_once); + qemu_announce_self_once(timer); } /***********************************************************/ @@ -1730,7 +1730,7 @@ static void loadvm_postcopy_handle_run_bh(void *opaque) */ cpu_synchronize_all_post_init(); - qemu_announce_self(); + qemu_announce_self(qemu_get_announce_params()); /* Make sure all file formats flush their mutable metadata. * If we get an error here, just don't restart the VM yet. */
Switch qemu_announce_self and virtio annoucements to use the announcement timer framework. This makes sure that both timers use the same timeouts and number of annoucement attempts Based on work by Dr. David Alan Gilbert <dgilbert@redhat.com> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> --- hw/net/virtio-net.c | 28 ++++++++++++++++------------ include/hw/virtio/virtio-net.h | 3 +-- include/migration/vmstate.h | 17 +++++++++++------ include/sysemu/sysemu.h | 2 +- migration/migration.c | 2 +- migration/savevm.c | 28 ++++++++++++++-------------- 6 files changed, 44 insertions(+), 36 deletions(-)