Re: [Qemu-devel] [PATCH COLO-Frame v14 37/40] COLO: enable buffer filters for PVM

[Jason Wang]

On 02/18/2016 03:30 PM, Hailiang Zhang wrote:
> Hi Jason,
>
> On 2016/2/18 11:46, Hailiang Zhang wrote:
>> On 2016/2/18 11:31, Jason Wang wrote:
>>>
>>>
>>> On 02/06/2016 05:28 PM, zhanghailiang wrote:
>>>> Enable all buffer filters that added by COLO while
>>>> go into COLO process, and disable them while exit COLO.
>>>>
>>>> Signed-off-by: zhanghailiang <address@hidden>
>>>> Cc: Jason Wang <address@hidden>
>>>> Cc: Yang Hongyang <address@hidden>
>>>> ---
>>>> v14:
>>>> - New patch
>>>> ---
>>>>   migration/colo.c | 32 ++++++++++++++++++++++++++++++++
>>>>   1 file changed, 32 insertions(+)
>>>>
>>>> diff --git a/migration/colo.c b/migration/colo.c
>>>> index 235578b..86a7638 100644
>>>> --- a/migration/colo.c
>>>> +++ b/migration/colo.c
>>>> @@ -104,10 +104,26 @@ static void secondary_vm_do_failover(void)
>>>>       }
>>>>   }
>>>>
>>>> +static void colo_set_filter_status(NetFilterState *nf, void *opaque,
>>>> +                                   Error **errp)
>>>> +{
>>>> +    char colo_filter[128];
>>>> +    char *name = object_get_canonical_path_component(OBJECT(nf));
>>>> +    char *status = opaque;
>>>> +
>>>> +    snprintf(colo_filter, sizeof(colo_filter), "%scolo",
>>>> nf->netdev_id);
>>>> +    if (strcmp(colo_filter, name)) {
>>>> +        return;
>>>> +    }
>>>
>>> Checking by name is not elegant. As we've discussed last time, why not
>>> let filter-buffer track all filters with zero interval in a linked list
>>> and just export a helper to disable and enable them all? Things will be
>>> greatly simplified with this, and there's even no need for patch 36.
>>>
>>>
>>
>> Yes, i know what you mean, but we have to add another
>> 'QTAILQ_ENTRY() entry' like member into struct NetFilterState
>> if we do like that, is it acceptable ?
>>
>
> Sorry for the hasty reply, ;)
>
> We can use a list to store all the colo related buffer filters
> without adding any member to struct NetFilterState.
>
> The codes will be like:
>
> struct COLOListNode{
>     void *opaque;
>     QLIST_ENTRY(COLOListNode) node;
> };
> static QLIST_HEAD(, COLOListNode) COLOBufferFilters =
>    QLIST_HEAD_INITIALIZER(COLOBufferFilters);
>
> void colo_add_buffer_filter()
> {
>     struct COLOListNode *filternode;
>     ...
>
>     filter = object_new_with_props();
>
>     filternode = g_new0(struct COLOListNode, 1);
>     filternode->opaque = NETFILTER(filter);
>     QLIST_INSERT_HEAD(&COLOBufferFilters, filternode, node);
> }
>
> And we can track all the colo releated filters directly by
> visiting the *COLOBufferFilters* list.
>
> Thanks,
> Hailiang

Also fine, but looks suboptimal to my proposal (use a list private to
filter-buffer).

>
>>> .
>>>
>>
>
>
>