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

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

On 2016/2/18 11:23, Jason Wang wrote:
>
>
> On 02/06/2016 05:28 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>
>> ---
>> v14:
>> - New patch
>> ---
>>   include/migration/colo.h |  2 ++
>>   include/net/filter.h     |  2 ++
>>   migration/colo-comm.c    |  5 +++++
>>   migration/colo.c         | 20 ++++++++++++++++++++
>>   net/filter-buffer.c      |  2 --
>>   stubs/migration-colo.c   |  4 ++++
>>   6 files changed, 33 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/migration/colo.h b/include/migration/colo.h
>> index 919b135..22b92c9 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(const char *netdev_id, void *opaque);
>> +
>>   #endif
>> diff --git a/include/net/filter.h b/include/net/filter.h
>> index 0159080..0da0d97 100644
>> --- a/include/net/filter.h
>> +++ b/include/net/filter.h
>> @@ -22,6 +22,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 3943e94..91d873e 100644
>> --- a/migration/colo-comm.c
>> +++ b/migration/colo-comm.c
>> @@ -13,6 +13,7 @@
>>
>>   #include <migration/colo.h>
>>   #include "trace.h"
>> +#include <net/net.h>
>>
>>   typedef struct {
>>        bool colo_requested;
>> @@ -58,6 +59,10 @@ 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 use colo-proxy */
>> +    if (colo_supported()) {
>> +        netdev_init_add_handler(colo_add_buffer_filter, NULL);
>> +    }
>>   }
>>
>>   bool migration_incoming_enable_colo(void)
>> diff --git a/migration/colo.c b/migration/colo.c
>> index 98a12fc..235578b 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/net.h"
>> +#include "net/filter.h"
>>
>>   static bool vmstate_loading;
>>
>> @@ -380,6 +382,24 @@ static int colo_prepare_before_save(MigrationState *s)
>>       return ret;
>>   }
>>
>> +void colo_add_buffer_filter(const char *netdev_id, void *opaque)
>> +{
>> +    NetFilterState *nf;
>> +    char filter_name[128];
>> +    Error *errp = NULL;
>> +
>> +    snprintf(filter_name, sizeof(filter_name),
>> +            "%scolo", netdev_id);
>> +    nf = netdev_add_filter(netdev_id, TYPE_FILTER_BUFFER, filter_name, false,
>> +                           &errp);
>
> Using netdev_add_filter() is too heavyweight and also a overkill. Why
> not simply call object_new_with_props() with what ever props you want
> (e.g interval=0) here?
>

Make sense, i will drop the helper and fix like what you said, thanks.

>> +    if (errp) {
>> +        error_report_err(errp);
>> +        return;
>> +    }
>> +    /* Only buffer the packets that sent out by VM */
>> +    nf->direction = NET_FILTER_DIRECTION_RX;
>> +}
>> +
>>   static void colo_process_checkpoint(MigrationState *s)
>>   {
>>       QEMUSizedBuffer *buffer = NULL;
>> diff --git a/net/filter-buffer.c b/net/filter-buffer.c
>> index 58cea8f..e70d156 100644
>> --- a/net/filter-buffer.c
>> +++ b/net/filter-buffer.c
>> @@ -16,8 +16,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 1996cd9..8e74acb 100644
>> --- a/stubs/migration-colo.c
>> +++ b/stubs/migration-colo.c
>> @@ -48,3 +48,7 @@ bool colo_shutdown(void)
>>   {
>>       return false;
>>   }
>> +
>> +void colo_add_buffer_filter(const char *netdev_id, void *opaque)
>> +{
>> +}
>
>
> .
>