Re: [Qemu-devel] [PATCH COLO-Frame v13 38/39] colo: Use default buffer-filter to buffer and release packets

[Jason Wang]

On 12/29/2015 03:09 PM, zhanghailiang wrote:
> Enable default filter to buffer packets and release the
> packets after a checkpoint.
>
> Signed-off-by: zhanghailiang <address@hidden>
> Cc: Jason Wang <address@hidden>
> Cc: Yang Hongyang <address@hidden>
> ---
> v12:
> - Add a helper function to check if all netdev supports buffer packets.
> - Flush buffered packets when do failover.
> v11:
> - Use new helper functions to buffer and release packets.
> ---
>  include/net/net.h |  1 +
>  migration/colo.c  | 25 ++++++++++++++++++++++++-
>  net/net.c         | 17 +++++++++++++++++
>  3 files changed, 42 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/net.h b/include/net/net.h
> index 5c65c45..2eb9451 100644
> --- a/include/net/net.h
> +++ b/include/net/net.h
> @@ -129,6 +129,7 @@ typedef void (*qemu_netfilter_foreach)(NetFilterState 
> *nf, void *opaque,
>                                         Error **errp);
>  void qemu_foreach_netfilter(qemu_netfilter_foreach func, void *opaque,
>                              Error **errp);
> +bool qemu_netdev_support_netfilter(void);
>  int qemu_can_send_packet(NetClientState *nc);
>  ssize_t qemu_sendv_packet(NetClientState *nc, const struct iovec *iov,
>                            int iovcnt);
> diff --git a/migration/colo.c b/migration/colo.c
> index bd68e86..23e1131 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -18,6 +18,8 @@
>  #include "qemu/error-report.h"
>  #include "migration/failover.h"
>  #include "qapi-event.h"
> +#include "net/filter.h"
> +#include "net/net.h"
>  
>  static bool vmstate_loading;
>  
> @@ -129,6 +131,11 @@ static void primary_vm_do_failover(void)
>                       old_state);
>          return;
>      }
> +    /* Don't buffer any packets while exited COLO */
> +    qemu_set_default_filters_status(false);
> +    /* Flush the residuary buffered packts */
> +    qemu_release_default_filters_packets();

It's ok to depend on buffer filter explicitly. But the name of above two
functions need to be renamed (since it was rather generic).

> +
>      /* Notify COLO thread that failover work is finished */
>      qemu_sem_post(&s->colo_sem);
>  }
> @@ -335,6 +342,8 @@ static int colo_do_checkpoint_transaction(MigrationState 
> *s,
>          goto out;
>      }
>  
> +    qemu_release_default_filters_packets();
> +
>      if (colo_shutdown) {
>          colo_put_cmd(s->to_dst_file, COLO_COMMAND_GUEST_SHUTDOWN, 
> &local_err);
>          if (local_err) {
> @@ -379,6 +388,17 @@ static int colo_prepare_before_save(MigrationState *s)
>      return ret;
>  }
>  
> +static int colo_init_buffer_filters(void)
> +{
> +    if (!qemu_netdev_support_netfilter()) {
> +        return -EPERM;
> +    }

I think colo should fail to be started if any netdev doesn't support
filter. Instead of failing in the middle.

> +    /* Begin to buffer packets that sent by VM */
> +    qemu_set_default_filters_status(true);
> +
> +    return 0;
> +}
> +
>  static void colo_process_checkpoint(MigrationState *s)
>  {
>      QEMUSizedBuffer *buffer = NULL;
> @@ -387,7 +407,10 @@ static void colo_process_checkpoint(MigrationState *s)
>      int ret;
>  
>      failover_init_state();
> -
> +    ret = colo_init_buffer_filters();
> +    if (ret < 0) {
> +        goto out;
> +    }
>      s->rp_state.from_dst_file = qemu_file_get_return_path(s->to_dst_file);
>      if (!s->rp_state.from_dst_file) {
>          error_report("Open QEMUFile from_dst_file failed");
> diff --git a/net/net.c b/net/net.c
> index 30946c5..4678a6e 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -288,6 +288,23 @@ void qemu_foreach_netfilter(qemu_netfilter_foreach func, 
> void *opaque,
>      }
>  }
>  
> +bool qemu_netdev_support_netfilter(void)
> +{
> +    NetClientState *nc;
> +
> +    QTAILQ_FOREACH(nc, &net_clients, next) {
> +        if (nc->info->type == NET_CLIENT_OPTIONS_KIND_NIC) {
> +            continue;
> +        }
> +        if (QTAILQ_EMPTY(&nc->filters)) {
> +            error_report("netdev (%s) does not support filter", nc->name);
> +            return false;
> +        }
> +    }
> +
> +    return true;
> +}
> +
>  static void qemu_net_client_destructor(NetClientState *nc)
>  {
>      g_free(nc);