Re: [Qemu-devel] [PATCH COLO-Frame v16 33/35] COLO/filter: add each netdev a buffer filter

[Hailiang Zhang <zhang.zhanghailiang@huawei.com>]

On 2016/4/26 14:58, Jason Wang wrote:
>
>
> On 04/08/2016 02:26 PM, zhanghailiang wrote:
>> For COLO periodic mode, it need to buffer packets that
>> sent by VM, and we will not release these packets until
>> finish a checkpoint.
>>
>> Here, we add each netdev a buffer-filter that will be controlled
>> by COLO. It is disabled by default, and the packets will not pass
>> through these filters. If users don't enable COLO while configure
>> qemu, these buffer-filters will not be added.
>>
>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>> Cc: Jason Wang <jasowang@redhat.com>
>> Cc: Yang Hongyang <hongyang.yang@easystack.cn>
>> ---
>> v16:
>> - Remove the useless check in colo_add_buffer_filter()
>> v15:
>> - call object_new_with_props() directly to add filter in
>>    colo_add_buffer_filter. (Jason's suggestion)
>> v14:
>> - New patch
>> ---
>>   include/migration/colo.h |  2 ++
>>   include/net/filter.h     |  2 ++
>>   migration/colo-comm.c    |  7 +++++++
>>   migration/colo.c         | 39 +++++++++++++++++++++++++++++++++++++++
>>   net/filter-buffer.c      |  2 --
>>   stubs/migration-colo.c   |  4 ++++
>>   6 files changed, 54 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/migration/colo.h b/include/migration/colo.h
>> index 919b135..87ea6d2 100644
>> --- a/include/migration/colo.h
>> +++ b/include/migration/colo.h
>> @@ -37,4 +37,6 @@ COLOMode get_colo_mode(void);
>>   void colo_do_failover(MigrationState *s);
>>
>>   bool colo_shutdown(void);
>> +void colo_add_buffer_filter(Notifier *notifier, void *data);
>> +
>>   #endif
>> diff --git a/include/net/filter.h b/include/net/filter.h
>> index 0c4a2ea..817eaf4 100644
>> --- a/include/net/filter.h
>> +++ b/include/net/filter.h
>> @@ -21,6 +21,8 @@
>>   #define NETFILTER_CLASS(klass) \
>>       OBJECT_CLASS_CHECK(NetFilterClass, (klass), TYPE_NETFILTER)
>>
>> +#define TYPE_FILTER_BUFFER "filter-buffer"
>> +
>>   typedef void (FilterSetup) (NetFilterState *nf, Error **errp);
>>   typedef void (FilterCleanup) (NetFilterState *nf);
>>   /*
>> diff --git a/migration/colo-comm.c b/migration/colo-comm.c
>> index 716a4f7..a90f1cb 100644
>> --- a/migration/colo-comm.c
>> +++ b/migration/colo-comm.c
>> @@ -14,12 +14,14 @@
>>   #include "qemu/osdep.h"
>>   #include <migration/colo.h>
>>   #include "trace.h"
>> +#include <net/net.h>
>>
>>   typedef struct {
>>        bool colo_requested;
>>   } COLOInfo;
>>
>>   static COLOInfo colo_info;
>> +static Notifier netdev_init_notifier;
>>
>>   COLOMode get_colo_mode(void)
>>   {
>> @@ -59,6 +61,11 @@ static const VMStateDescription colo_state = {
>>   void colo_info_init(void)
>>   {
>>       vmstate_register(NULL, 0, &colo_state, &colo_info);
>> +    /* FIXME: Remove this after COLO switch to using colo-proxy */
>> +    if (colo_supported()) {
>> +        netdev_init_notifier.notify = colo_add_buffer_filter;
>> +        netdev_init_add_notifier(&netdev_init_notifier);
>> +    }
>>   }
>>
>>   bool migration_incoming_enable_colo(void)
>> diff --git a/migration/colo.c b/migration/colo.c
>> index c16a7d7..7a60bc1 100644
>> --- a/migration/colo.c
>> +++ b/migration/colo.c
>> @@ -19,12 +19,23 @@
>>   #include "qapi/error.h"
>>   #include "migration/failover.h"
>>   #include "qapi-event.h"
>> +#include "net/net.h"
>> +#include "net/filter.h"
>> +#include "net/vhost_net.h"
>>
>>   static bool vmstate_loading;
>>
>>   /* colo buffer */
>>   #define COLO_BUFFER_BASE_SIZE (4 * 1024 * 1024)
>>
>> +typedef struct COLOListNode {
>> +    void *opaque;
>> +    QLIST_ENTRY(COLOListNode) node;
>> +} COLOListNode;
>> +
>> +static QLIST_HEAD(, COLOListNode) COLOBufferFilters =
>> +    QLIST_HEAD_INITIALIZER(COLOBufferFilters);
>> +
>>   bool colo_supported(void)
>>   {
>>       return true;
>> @@ -384,6 +395,34 @@ static int colo_prepare_before_save(MigrationState *s)
>>       return ret;
>>   }
>>
>> +void colo_add_buffer_filter(Notifier *notifier, void *data)
>> +{
>> +    char *netdev_id = data;
>> +    NetFilterState *nf;
>> +    char filter_name[128];
>> +    Object *filter;
>> +    COLOListNode *filternode;
>> +
>> +    snprintf(filter_name, sizeof(filter_name),
>> +            "%scolo", netdev_id);
>> +
>> +    filter = object_new_with_props(TYPE_FILTER_BUFFER,
>> +                        object_get_objects_root(),
>> +                        filter_name, NULL,
>> +                        "netdev", netdev_id,
>> +                        "status", "off",
>> +                        NULL);
>> +    if (!filter) {
>> +        return;
>
> Looks like at least we need a warn here?
>

I'm not quite sure about this, since we add this default netfilters
automatically, we'd better to keep silence if some error
happened during this process ...

>> +    }
>> +    nf =  NETFILTER(filter);
>> +    /* Only buffer the packets that sent out by VM */
>> +    nf->direction = NET_FILTER_DIRECTION_RX;
>> +    filternode = g_new0(COLOListNode, 1);
>> +    filternode->opaque = nf;
>> +    QLIST_INSERT_HEAD(&COLOBufferFilters, filternode, node);
>> +}
>> +
>>   static void colo_process_checkpoint(MigrationState *s)
>>   {
>>       QEMUSizedBuffer *buffer = NULL;
>> diff --git a/net/filter-buffer.c b/net/filter-buffer.c
>> index 346306a..d1cf595 100644
>> --- a/net/filter-buffer.c
>> +++ b/net/filter-buffer.c
>> @@ -17,8 +17,6 @@
>>   #include "qapi-visit.h"
>>   #include "qom/object.h"
>>
>> -#define TYPE_FILTER_BUFFER "filter-buffer"
>> -
>>   #define FILTER_BUFFER(obj) \
>>       OBJECT_CHECK(FilterBufferState, (obj), TYPE_FILTER_BUFFER)
>>
>> diff --git a/stubs/migration-colo.c b/stubs/migration-colo.c
>> index e507e17..65b1f49 100644
>> --- a/stubs/migration-colo.c
>> +++ b/stubs/migration-colo.c
>> @@ -49,3 +49,7 @@ bool colo_shutdown(void)
>>   {
>>       return false;
>>   }
>> +
>> +void colo_add_buffer_filter(Notifier *notifier, void *data)
>> +{
>> +}
>
>
> .
>