| Message ID | 20180811205924.4113-17-zhangckid@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | COLO: integrate colo frame with block replication and COLO proxy | expand |
On 2018年08月12日 04:59, Zhang Chen wrote: > Filter needs to process the event of checkpoint/failover or > other event passed by COLO frame. > > Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> > --- > include/net/filter.h | 5 +++++ > net/filter.c | 17 +++++++++++++++++ > net/net.c | 28 ++++++++++++++++++++++++++++ > 3 files changed, 50 insertions(+) > > diff --git a/include/net/filter.h b/include/net/filter.h > index 435acd6f82..49da666ac0 100644 > --- a/include/net/filter.h > +++ b/include/net/filter.h > @@ -38,6 +38,8 @@ typedef ssize_t (FilterReceiveIOV)(NetFilterState *nc, > > typedef void (FilterStatusChanged) (NetFilterState *nf, Error **errp); > > +typedef void (FilterHandleEvent) (NetFilterState *nf, int event, Error **errp); > + > typedef struct NetFilterClass { > ObjectClass parent_class; > > @@ -45,6 +47,7 @@ typedef struct NetFilterClass { > FilterSetup *setup; > FilterCleanup *cleanup; > FilterStatusChanged *status_changed; > + FilterHandleEvent *handle_event; > /* mandatory */ > FilterReceiveIOV *receive_iov; > } NetFilterClass; > @@ -77,4 +80,6 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender, > int iovcnt, > void *opaque); > > +void colo_notify_filters_event(int event, Error **errp); > + > #endif /* QEMU_NET_FILTER_H */ > diff --git a/net/filter.c b/net/filter.c > index 2fd7d7d663..0f17eba143 100644 > --- a/net/filter.c > +++ b/net/filter.c > @@ -17,6 +17,8 @@ > #include "net/vhost_net.h" > #include "qom/object_interfaces.h" > #include "qemu/iov.h" > +#include "net/colo.h" > +#include "migration/colo.h" > > static inline bool qemu_can_skip_netfilter(NetFilterState *nf) > { > @@ -245,11 +247,26 @@ static void netfilter_finalize(Object *obj) > g_free(nf->netdev_id); > } > > +static void dummy_handle_event(NetFilterState *nf, int event, Error **errp) > +{ It's in fact not a "dummy" handler, Maybe it's better to rename it as "default". > + switch (event) { > + case COLO_EVENT_CHECKPOINT: > + break; > + case COLO_EVENT_FAILOVER: > + object_property_set_str(OBJECT(nf), "off", "status", errp); I think filter is a generic infrastructure, so it's better not have COLO specific things like this. You can either add a generic name or have a dedicated helper to just disable all net filters. > + break; > + default: > + break; > + } > +} > + > static void netfilter_class_init(ObjectClass *oc, void *data) > { > UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc); > + NetFilterClass *nfc = NETFILTER_CLASS(oc); > > ucc->complete = netfilter_complete; > + nfc->handle_event = dummy_handle_event; > } > > static const TypeInfo netfilter_info = { > diff --git a/net/net.c b/net/net.c > index a77ea88fff..b4f6a2efb2 100644 > --- a/net/net.c > +++ b/net/net.c > @@ -1331,6 +1331,34 @@ void hmp_info_network(Monitor *mon, const QDict *qdict) > } > } > > +void colo_notify_filters_event(int event, Error **errp) > +{ > + NetClientState *nc, *peer; > + NetClientDriver type; > + NetFilterState *nf; > + NetFilterClass *nfc = NULL; > + Error *local_err = NULL; > + > + QTAILQ_FOREACH(nc, &net_clients, next) { > + peer = nc->peer; > + type = nc->info->type; > + if (!peer || type != NET_CLIENT_DRIVER_TAP) { > + continue; > + } The check the TAP is redundant with previous patch. > + QTAILQ_FOREACH(nf, &nc->filters, next) { > + nfc = NETFILTER_GET_CLASS(OBJECT(nf)); > + if (!nfc->handle_event) { Looks like this won't happen. Thanks > + continue; > + } > + nfc->handle_event(nf, event, &local_err); > + if (local_err) { > + error_propagate(errp, local_err); > + return; > + } > + } > + } > +} > + > void qmp_set_link(const char *name, bool up, Error **errp) > { > NetClientState *ncs[MAX_QUEUE_NUM];
On Tue, Aug 21, 2018 at 11:30 AM Jason Wang <jasowang@redhat.com> wrote: > > > On 2018年08月12日 04:59, Zhang Chen wrote: > > Filter needs to process the event of checkpoint/failover or > > other event passed by COLO frame. > > > > Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> > > --- > > include/net/filter.h | 5 +++++ > > net/filter.c | 17 +++++++++++++++++ > > net/net.c | 28 ++++++++++++++++++++++++++++ > > 3 files changed, 50 insertions(+) > > > > diff --git a/include/net/filter.h b/include/net/filter.h > > index 435acd6f82..49da666ac0 100644 > > --- a/include/net/filter.h > > +++ b/include/net/filter.h > > @@ -38,6 +38,8 @@ typedef ssize_t (FilterReceiveIOV)(NetFilterState *nc, > > > > typedef void (FilterStatusChanged) (NetFilterState *nf, Error **errp); > > > > +typedef void (FilterHandleEvent) (NetFilterState *nf, int event, Error > **errp); > > + > > typedef struct NetFilterClass { > > ObjectClass parent_class; > > > > @@ -45,6 +47,7 @@ typedef struct NetFilterClass { > > FilterSetup *setup; > > FilterCleanup *cleanup; > > FilterStatusChanged *status_changed; > > + FilterHandleEvent *handle_event; > > /* mandatory */ > > FilterReceiveIOV *receive_iov; > > } NetFilterClass; > > @@ -77,4 +80,6 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState > *sender, > > int iovcnt, > > void *opaque); > > > > +void colo_notify_filters_event(int event, Error **errp); > > + > > #endif /* QEMU_NET_FILTER_H */ > > diff --git a/net/filter.c b/net/filter.c > > index 2fd7d7d663..0f17eba143 100644 > > --- a/net/filter.c > > +++ b/net/filter.c > > @@ -17,6 +17,8 @@ > > #include "net/vhost_net.h" > > #include "qom/object_interfaces.h" > > #include "qemu/iov.h" > > +#include "net/colo.h" > > +#include "migration/colo.h" > > > > static inline bool qemu_can_skip_netfilter(NetFilterState *nf) > > { > > @@ -245,11 +247,26 @@ static void netfilter_finalize(Object *obj) > > g_free(nf->netdev_id); > > } > > > > +static void dummy_handle_event(NetFilterState *nf, int event, Error > **errp) > > +{ > > It's in fact not a "dummy" handler, Maybe it's better to rename it as > "default". > OK, I will rename it in next version. > > > + switch (event) { > > + case COLO_EVENT_CHECKPOINT: > > + break; > > + case COLO_EVENT_FAILOVER: > > + object_property_set_str(OBJECT(nf), "off", "status", errp); > > I think filter is a generic infrastructure, so it's better not have COLO > specific things like this. You can either add a generic name or have a > dedicated helper to just disable all net filters. > Maybe we can rename it to "EVENT_CHECKPOINT" and "EVENT_FAILOVER" looks better? > > > + break; > > + default: > > + break; > > + } > > +} > > + > > static void netfilter_class_init(ObjectClass *oc, void *data) > > { > > UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc); > > + NetFilterClass *nfc = NETFILTER_CLASS(oc); > > > > ucc->complete = netfilter_complete; > > + nfc->handle_event = dummy_handle_event; > > } > > > > static const TypeInfo netfilter_info = { > > diff --git a/net/net.c b/net/net.c > > index a77ea88fff..b4f6a2efb2 100644 > > --- a/net/net.c > > +++ b/net/net.c > > @@ -1331,6 +1331,34 @@ void hmp_info_network(Monitor *mon, const QDict > *qdict) > > } > > } > > > > +void colo_notify_filters_event(int event, Error **errp) > > +{ > > + NetClientState *nc, *peer; > > + NetClientDriver type; > > + NetFilterState *nf; > > + NetFilterClass *nfc = NULL; > > + Error *local_err = NULL; > > + > > + QTAILQ_FOREACH(nc, &net_clients, next) { > > + peer = nc->peer; > > + type = nc->info->type; > > + if (!peer || type != NET_CLIENT_DRIVER_TAP) { > > + continue; > > + } > > The check the TAP is redundant with previous patch. > OK, I will remove it. > > > + QTAILQ_FOREACH(nf, &nc->filters, next) { > > + nfc = NETFILTER_GET_CLASS(OBJECT(nf)); > > + if (!nfc->handle_event) { > > Looks like this won't happen. > It will happen. like filter-mirror and filter-redirector haven't this event. Thanks Zhang Chen > > Thanks > > > + continue; > > + } > > + nfc->handle_event(nf, event, &local_err); > > + if (local_err) { > > + error_propagate(errp, local_err); > > + return; > > + } > > + } > > + } > > +} > > + > > void qmp_set_link(const char *name, bool up, Error **errp) > > { > > NetClientState *ncs[MAX_QUEUE_NUM]; > >
On 2018年08月21日 17:25, Zhang Chen wrote: > On Tue, Aug 21, 2018 at 11:30 AM Jason Wang <jasowang@redhat.com> wrote: > >> >> On 2018年08月12日 04:59, Zhang Chen wrote: >>> Filter needs to process the event of checkpoint/failover or >>> other event passed by COLO frame. >>> >>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> >>> --- >>> include/net/filter.h | 5 +++++ >>> net/filter.c | 17 +++++++++++++++++ >>> net/net.c | 28 ++++++++++++++++++++++++++++ >>> 3 files changed, 50 insertions(+) >>> >>> diff --git a/include/net/filter.h b/include/net/filter.h >>> index 435acd6f82..49da666ac0 100644 >>> --- a/include/net/filter.h >>> +++ b/include/net/filter.h >>> @@ -38,6 +38,8 @@ typedef ssize_t (FilterReceiveIOV)(NetFilterState *nc, >>> >>> typedef void (FilterStatusChanged) (NetFilterState *nf, Error **errp); >>> >>> +typedef void (FilterHandleEvent) (NetFilterState *nf, int event, Error >> **errp); >>> + >>> typedef struct NetFilterClass { >>> ObjectClass parent_class; >>> >>> @@ -45,6 +47,7 @@ typedef struct NetFilterClass { >>> FilterSetup *setup; >>> FilterCleanup *cleanup; >>> FilterStatusChanged *status_changed; >>> + FilterHandleEvent *handle_event; >>> /* mandatory */ >>> FilterReceiveIOV *receive_iov; >>> } NetFilterClass; >>> @@ -77,4 +80,6 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState >> *sender, >>> int iovcnt, >>> void *opaque); >>> >>> +void colo_notify_filters_event(int event, Error **errp); >>> + >>> #endif /* QEMU_NET_FILTER_H */ >>> diff --git a/net/filter.c b/net/filter.c >>> index 2fd7d7d663..0f17eba143 100644 >>> --- a/net/filter.c >>> +++ b/net/filter.c >>> @@ -17,6 +17,8 @@ >>> #include "net/vhost_net.h" >>> #include "qom/object_interfaces.h" >>> #include "qemu/iov.h" >>> +#include "net/colo.h" >>> +#include "migration/colo.h" >>> >>> static inline bool qemu_can_skip_netfilter(NetFilterState *nf) >>> { >>> @@ -245,11 +247,26 @@ static void netfilter_finalize(Object *obj) >>> g_free(nf->netdev_id); >>> } >>> >>> +static void dummy_handle_event(NetFilterState *nf, int event, Error >> **errp) >>> +{ >> It's in fact not a "dummy" handler, Maybe it's better to rename it as >> "default". >> > OK, I will rename it in next version. > > >>> + switch (event) { >>> + case COLO_EVENT_CHECKPOINT: >>> + break; >>> + case COLO_EVENT_FAILOVER: >>> + object_property_set_str(OBJECT(nf), "off", "status", errp); >> I think filter is a generic infrastructure, so it's better not have COLO >> specific things like this. You can either add a generic name or have a >> dedicated helper to just disable all net filters. >> > Maybe we can rename it to "EVENT_CHECKPOINT" and "EVENT_FAILOVER" looks > better? > I think registering notifier to COLO is better. For disabling filter, we can have a generic dedicated helpers to do this. Btw, I remember we allow disabling filter through qmp, if it's true, we probably need some notification to management, but this can be done in the future. >>> + break; >>> + default: >>> + break; >>> + } >>> +} >>> + >>> static void netfilter_class_init(ObjectClass *oc, void *data) >>> { >>> UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc); >>> + NetFilterClass *nfc = NETFILTER_CLASS(oc); >>> >>> ucc->complete = netfilter_complete; >>> + nfc->handle_event = dummy_handle_event; >>> } >>> >>> static const TypeInfo netfilter_info = { >>> diff --git a/net/net.c b/net/net.c >>> index a77ea88fff..b4f6a2efb2 100644 >>> --- a/net/net.c >>> +++ b/net/net.c >>> @@ -1331,6 +1331,34 @@ void hmp_info_network(Monitor *mon, const QDict >> *qdict) >>> } >>> } >>> >>> +void colo_notify_filters_event(int event, Error **errp) >>> +{ >>> + NetClientState *nc, *peer; >>> + NetClientDriver type; >>> + NetFilterState *nf; >>> + NetFilterClass *nfc = NULL; >>> + Error *local_err = NULL; >>> + >>> + QTAILQ_FOREACH(nc, &net_clients, next) { >>> + peer = nc->peer; >>> + type = nc->info->type; >>> + if (!peer || type != NET_CLIENT_DRIVER_TAP) { >>> + continue; >>> + } >> The check the TAP is redundant with previous patch. >> > OK, I will remove it. > > >>> + QTAILQ_FOREACH(nf, &nc->filters, next) { >>> + nfc = NETFILTER_GET_CLASS(OBJECT(nf)); >>> + if (!nfc->handle_event) { >> Looks like this won't happen. >> > It will happen. like filter-mirror and filter-redirector haven't this event. But you do: static void netfilter_class_init(ObjectClass *oc, void *data) { UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc); + NetFilterClass *nfc = NETFILTER_CLASS(oc); ucc->complete = netfilter_complete; + nfc->handle_event = dummy_handle_event; } ? Thanks > > Thanks > Zhang Chen > > >> Thanks >> >>> + continue; >>> + } >>> + nfc->handle_event(nf, event, &local_err); >>> + if (local_err) { >>> + error_propagate(errp, local_err); >>> + return; >>> + } >>> + } >>> + } >>> +} >>> + >>> void qmp_set_link(const char *name, bool up, Error **errp) >>> { >>> NetClientState *ncs[MAX_QUEUE_NUM]; >>
On Wed, Aug 22, 2018 at 4:22 PM Jason Wang <jasowang@redhat.com> wrote: > > > On 2018年08月21日 17:25, Zhang Chen wrote: > > On Tue, Aug 21, 2018 at 11:30 AM Jason Wang <jasowang@redhat.com> wrote: > > > >> > >> On 2018年08月12日 04:59, Zhang Chen wrote: > >>> Filter needs to process the event of checkpoint/failover or > >>> other event passed by COLO frame. > >>> > >>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> > >>> --- > >>> include/net/filter.h | 5 +++++ > >>> net/filter.c | 17 +++++++++++++++++ > >>> net/net.c | 28 ++++++++++++++++++++++++++++ > >>> 3 files changed, 50 insertions(+) > >>> > >>> diff --git a/include/net/filter.h b/include/net/filter.h > >>> index 435acd6f82..49da666ac0 100644 > >>> --- a/include/net/filter.h > >>> +++ b/include/net/filter.h > >>> @@ -38,6 +38,8 @@ typedef ssize_t (FilterReceiveIOV)(NetFilterState > *nc, > >>> > >>> typedef void (FilterStatusChanged) (NetFilterState *nf, Error > **errp); > >>> > >>> +typedef void (FilterHandleEvent) (NetFilterState *nf, int event, Error > >> **errp); > >>> + > >>> typedef struct NetFilterClass { > >>> ObjectClass parent_class; > >>> > >>> @@ -45,6 +47,7 @@ typedef struct NetFilterClass { > >>> FilterSetup *setup; > >>> FilterCleanup *cleanup; > >>> FilterStatusChanged *status_changed; > >>> + FilterHandleEvent *handle_event; > >>> /* mandatory */ > >>> FilterReceiveIOV *receive_iov; > >>> } NetFilterClass; > >>> @@ -77,4 +80,6 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState > >> *sender, > >>> int iovcnt, > >>> void *opaque); > >>> > >>> +void colo_notify_filters_event(int event, Error **errp); > >>> + > >>> #endif /* QEMU_NET_FILTER_H */ > >>> diff --git a/net/filter.c b/net/filter.c > >>> index 2fd7d7d663..0f17eba143 100644 > >>> --- a/net/filter.c > >>> +++ b/net/filter.c > >>> @@ -17,6 +17,8 @@ > >>> #include "net/vhost_net.h" > >>> #include "qom/object_interfaces.h" > >>> #include "qemu/iov.h" > >>> +#include "net/colo.h" > >>> +#include "migration/colo.h" > >>> > >>> static inline bool qemu_can_skip_netfilter(NetFilterState *nf) > >>> { > >>> @@ -245,11 +247,26 @@ static void netfilter_finalize(Object *obj) > >>> g_free(nf->netdev_id); > >>> } > >>> > >>> +static void dummy_handle_event(NetFilterState *nf, int event, Error > >> **errp) > >>> +{ > >> It's in fact not a "dummy" handler, Maybe it's better to rename it as > >> "default". > >> > > OK, I will rename it in next version. > > > > > >>> + switch (event) { > >>> + case COLO_EVENT_CHECKPOINT: > >>> + break; > >>> + case COLO_EVENT_FAILOVER: > >>> + object_property_set_str(OBJECT(nf), "off", "status", errp); > >> I think filter is a generic infrastructure, so it's better not have COLO > >> specific things like this. You can either add a generic name or have a > >> dedicated helper to just disable all net filters. > >> > > Maybe we can rename it to "EVENT_CHECKPOINT" and "EVENT_FAILOVER" looks > > better? > > > > I think registering notifier to COLO is better. For disabling filter, we > can have a generic dedicated helpers to do this. > If we registering notifier to COLO (in migration/colo.c), filter-rewriter can not get the notify immediately when checkout occur. filter-reweiter didn't know when checkpoint happened, and I think loop to check the COLO event in filter-rewriter is a bad choice. Any thing I misunderstand? > > Btw, I remember we allow disabling filter through qmp, if it's true, we > probably need some notification to management, but this can be done in > the future. > > >>> + break; > >>> + default: > >>> + break; > >>> + } > >>> +} > >>> + > >>> static void netfilter_class_init(ObjectClass *oc, void *data) > >>> { > >>> UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc); > >>> + NetFilterClass *nfc = NETFILTER_CLASS(oc); > >>> > >>> ucc->complete = netfilter_complete; > >>> + nfc->handle_event = dummy_handle_event; > >>> } > >>> > >>> static const TypeInfo netfilter_info = { > >>> diff --git a/net/net.c b/net/net.c > >>> index a77ea88fff..b4f6a2efb2 100644 > >>> --- a/net/net.c > >>> +++ b/net/net.c > >>> @@ -1331,6 +1331,34 @@ void hmp_info_network(Monitor *mon, const QDict > >> *qdict) > >>> } > >>> } > >>> > >>> +void colo_notify_filters_event(int event, Error **errp) > >>> +{ > >>> + NetClientState *nc, *peer; > >>> + NetClientDriver type; > >>> + NetFilterState *nf; > >>> + NetFilterClass *nfc = NULL; > >>> + Error *local_err = NULL; > >>> + > >>> + QTAILQ_FOREACH(nc, &net_clients, next) { > >>> + peer = nc->peer; > >>> + type = nc->info->type; > >>> + if (!peer || type != NET_CLIENT_DRIVER_TAP) { > >>> + continue; > >>> + } > >> The check the TAP is redundant with previous patch. > >> > > OK, I will remove it. > > > > > >>> + QTAILQ_FOREACH(nf, &nc->filters, next) { > >>> + nfc = NETFILTER_GET_CLASS(OBJECT(nf)); > >>> + if (!nfc->handle_event) { > >> Looks like this won't happen. > >> > > It will happen. like filter-mirror and filter-redirector haven't this > event. > > But you do: > > static void netfilter_class_init(ObjectClass *oc, void *data) > { > UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc); > + NetFilterClass *nfc = NETFILTER_CLASS(oc); > > ucc->complete = netfilter_complete; > + nfc->handle_event = dummy_handle_event; > } > > ? > Oh, I forgot thiat. you are right, I will remove this in next version. Thanks Zhang Chen > > Thanks > > > > > Thanks > > Zhang Chen > > > > > >> Thanks > >> > >>> + continue; > >>> + } > >>> + nfc->handle_event(nf, event, &local_err); > >>> + if (local_err) { > >>> + error_propagate(errp, local_err); > >>> + return; > >>> + } > >>> + } > >>> + } > >>> +} > >>> + > >>> void qmp_set_link(const char *name, bool up, Error **errp) > >>> { > >>> NetClientState *ncs[MAX_QUEUE_NUM]; > >> > >
On 2018年08月24日 13:57, Zhang Chen wrote: > On Wed, Aug 22, 2018 at 4:22 PM Jason Wang<jasowang@redhat.com> wrote: > >> On 2018年08月21日 17:25, Zhang Chen wrote: >>> On Tue, Aug 21, 2018 at 11:30 AM Jason Wang<jasowang@redhat.com> wrote: >>> >>>> On 2018年08月12日 04:59, Zhang Chen wrote: >>>>> Filter needs to process the event of checkpoint/failover or >>>>> other event passed by COLO frame. >>>>> >>>>> Signed-off-by: zhanghailiang<zhang.zhanghailiang@huawei.com> >>>>> --- >>>>> include/net/filter.h | 5 +++++ >>>>> net/filter.c | 17 +++++++++++++++++ >>>>> net/net.c | 28 ++++++++++++++++++++++++++++ >>>>> 3 files changed, 50 insertions(+) >>>>> >>>>> diff --git a/include/net/filter.h b/include/net/filter.h >>>>> index 435acd6f82..49da666ac0 100644 >>>>> --- a/include/net/filter.h >>>>> +++ b/include/net/filter.h >>>>> @@ -38,6 +38,8 @@ typedef ssize_t (FilterReceiveIOV)(NetFilterState >> *nc, >>>>> typedef void (FilterStatusChanged) (NetFilterState *nf, Error >> **errp); >>>>> +typedef void (FilterHandleEvent) (NetFilterState *nf, int event, Error >>>> **errp); >>>>> + >>>>> typedef struct NetFilterClass { >>>>> ObjectClass parent_class; >>>>> >>>>> @@ -45,6 +47,7 @@ typedef struct NetFilterClass { >>>>> FilterSetup *setup; >>>>> FilterCleanup *cleanup; >>>>> FilterStatusChanged *status_changed; >>>>> + FilterHandleEvent *handle_event; >>>>> /* mandatory */ >>>>> FilterReceiveIOV *receive_iov; >>>>> } NetFilterClass; >>>>> @@ -77,4 +80,6 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState >>>> *sender, >>>>> int iovcnt, >>>>> void *opaque); >>>>> >>>>> +void colo_notify_filters_event(int event, Error **errp); >>>>> + >>>>> #endif /* QEMU_NET_FILTER_H */ >>>>> diff --git a/net/filter.c b/net/filter.c >>>>> index 2fd7d7d663..0f17eba143 100644 >>>>> --- a/net/filter.c >>>>> +++ b/net/filter.c >>>>> @@ -17,6 +17,8 @@ >>>>> #include "net/vhost_net.h" >>>>> #include "qom/object_interfaces.h" >>>>> #include "qemu/iov.h" >>>>> +#include "net/colo.h" >>>>> +#include "migration/colo.h" >>>>> >>>>> static inline bool qemu_can_skip_netfilter(NetFilterState *nf) >>>>> { >>>>> @@ -245,11 +247,26 @@ static void netfilter_finalize(Object *obj) >>>>> g_free(nf->netdev_id); >>>>> } >>>>> >>>>> +static void dummy_handle_event(NetFilterState *nf, int event, Error >>>> **errp) >>>>> +{ >>>> It's in fact not a "dummy" handler, Maybe it's better to rename it as >>>> "default". >>>> >>> OK, I will rename it in next version. >>> >>> >>>>> + switch (event) { >>>>> + case COLO_EVENT_CHECKPOINT: >>>>> + break; >>>>> + case COLO_EVENT_FAILOVER: >>>>> + object_property_set_str(OBJECT(nf), "off", "status", errp); >>>> I think filter is a generic infrastructure, so it's better not have COLO >>>> specific things like this. You can either add a generic name or have a >>>> dedicated helper to just disable all net filters. >>>> >>> Maybe we can rename it to "EVENT_CHECKPOINT" and "EVENT_FAILOVER" looks >>> better? >>> >> I think registering notifier to COLO is better. For disabling filter, we >> can have a generic dedicated helpers to do this. >> > If we registering notifier to COLO (in migration/colo.c), filter-rewriter > can not get the notify immediately when checkout occur. > filter-reweiter didn't know when checkpoint happened, I may miss something, isn't COLO which triggers the checkpoint even? > and I think loop to > check the COLO event in filter-rewriter is a bad choice. Well, anyway you need a loop somewhere to iterate all notifiers? Thanks > Any thing I misunderstand? > > >
On Tue, Aug 28, 2018 at 3:19 PM Jason Wang <jasowang@redhat.com> wrote: > > > On 2018年08月24日 13:57, Zhang Chen wrote: > > On Wed, Aug 22, 2018 at 4:22 PM Jason Wang<jasowang@redhat.com> wrote: > > > >> On 2018年08月21日 17:25, Zhang Chen wrote: > >>> On Tue, Aug 21, 2018 at 11:30 AM Jason Wang<jasowang@redhat.com> > wrote: > >>> > >>>> On 2018年08月12日 04:59, Zhang Chen wrote: > >>>>> Filter needs to process the event of checkpoint/failover or > >>>>> other event passed by COLO frame. > >>>>> > >>>>> Signed-off-by: zhanghailiang<zhang.zhanghailiang@huawei.com> > >>>>> --- > >>>>> include/net/filter.h | 5 +++++ > >>>>> net/filter.c | 17 +++++++++++++++++ > >>>>> net/net.c | 28 ++++++++++++++++++++++++++++ > >>>>> 3 files changed, 50 insertions(+) > >>>>> > >>>>> diff --git a/include/net/filter.h b/include/net/filter.h > >>>>> index 435acd6f82..49da666ac0 100644 > >>>>> --- a/include/net/filter.h > >>>>> +++ b/include/net/filter.h > >>>>> @@ -38,6 +38,8 @@ typedef ssize_t (FilterReceiveIOV)(NetFilterState > >> *nc, > >>>>> typedef void (FilterStatusChanged) (NetFilterState *nf, Error > >> **errp); > >>>>> +typedef void (FilterHandleEvent) (NetFilterState *nf, int event, > Error > >>>> **errp); > >>>>> + > >>>>> typedef struct NetFilterClass { > >>>>> ObjectClass parent_class; > >>>>> > >>>>> @@ -45,6 +47,7 @@ typedef struct NetFilterClass { > >>>>> FilterSetup *setup; > >>>>> FilterCleanup *cleanup; > >>>>> FilterStatusChanged *status_changed; > >>>>> + FilterHandleEvent *handle_event; > >>>>> /* mandatory */ > >>>>> FilterReceiveIOV *receive_iov; > >>>>> } NetFilterClass; > >>>>> @@ -77,4 +80,6 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState > >>>> *sender, > >>>>> int iovcnt, > >>>>> void *opaque); > >>>>> > >>>>> +void colo_notify_filters_event(int event, Error **errp); > >>>>> + > >>>>> #endif /* QEMU_NET_FILTER_H */ > >>>>> diff --git a/net/filter.c b/net/filter.c > >>>>> index 2fd7d7d663..0f17eba143 100644 > >>>>> --- a/net/filter.c > >>>>> +++ b/net/filter.c > >>>>> @@ -17,6 +17,8 @@ > >>>>> #include "net/vhost_net.h" > >>>>> #include "qom/object_interfaces.h" > >>>>> #include "qemu/iov.h" > >>>>> +#include "net/colo.h" > >>>>> +#include "migration/colo.h" > >>>>> > >>>>> static inline bool qemu_can_skip_netfilter(NetFilterState *nf) > >>>>> { > >>>>> @@ -245,11 +247,26 @@ static void netfilter_finalize(Object *obj) > >>>>> g_free(nf->netdev_id); > >>>>> } > >>>>> > >>>>> +static void dummy_handle_event(NetFilterState *nf, int event, Error > >>>> **errp) > >>>>> +{ > >>>> It's in fact not a "dummy" handler, Maybe it's better to rename it as > >>>> "default". > >>>> > >>> OK, I will rename it in next version. > >>> > >>> > >>>>> + switch (event) { > >>>>> + case COLO_EVENT_CHECKPOINT: > >>>>> + break; > >>>>> + case COLO_EVENT_FAILOVER: > >>>>> + object_property_set_str(OBJECT(nf), "off", "status", errp); > >>>> I think filter is a generic infrastructure, so it's better not have > COLO > >>>> specific things like this. You can either add a generic name or have a > >>>> dedicated helper to just disable all net filters. > >>>> > >>> Maybe we can rename it to "EVENT_CHECKPOINT" and "EVENT_FAILOVER" looks > >>> better? > >>> > >> I think registering notifier to COLO is better. For disabling filter, we > >> can have a generic dedicated helpers to do this. > >> > > If we registering notifier to COLO (in migration/colo.c), filter-rewriter > > can not get the notify immediately when checkout occur. > > filter-reweiter didn't know when checkpoint happened, > > I may miss something, isn't COLO which triggers the checkpoint even? > Yes, in detail, we have two way to trigger the checkpoint: case 1. COLO compare trigger. case 2. periodic checkpoint mechanism trigger. But we both solve the same problem that how to notify filter-rewriter do checkpoint in secondary node. Currently we do this job in that way: case 1. primary node COLO compare(net/colo-compare.c) ----> primary node COLO frame(migration/colo.c) ------> secondary node COLO frame -----------------> secondary node filter-rewriter. case 2. primary node COLO frame(migration/colo.c) ------> secondary node COLO frame -----------------> secondary node filter-rewriter. So, this patch just focus on "secondary node COLO frame -----------------> secondary node filter-rewriter". In filter-rewriter hard to get the checkpoint message from COLO frame. Thanks Zhang Chen > > > and I think loop to > > check the COLO event in filter-rewriter is a bad choice. > > Well, anyway you need a loop somewhere to iterate all notifiers? > > Thanks > > > Any thing I misunderstand? > > > > > > > >
diff --git a/include/net/filter.h b/include/net/filter.h index 435acd6f82..49da666ac0 100644 --- a/include/net/filter.h +++ b/include/net/filter.h @@ -38,6 +38,8 @@ typedef ssize_t (FilterReceiveIOV)(NetFilterState *nc, typedef void (FilterStatusChanged) (NetFilterState *nf, Error **errp); +typedef void (FilterHandleEvent) (NetFilterState *nf, int event, Error **errp); + typedef struct NetFilterClass { ObjectClass parent_class; @@ -45,6 +47,7 @@ typedef struct NetFilterClass { FilterSetup *setup; FilterCleanup *cleanup; FilterStatusChanged *status_changed; + FilterHandleEvent *handle_event; /* mandatory */ FilterReceiveIOV *receive_iov; } NetFilterClass; @@ -77,4 +80,6 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender, int iovcnt, void *opaque); +void colo_notify_filters_event(int event, Error **errp); + #endif /* QEMU_NET_FILTER_H */ diff --git a/net/filter.c b/net/filter.c index 2fd7d7d663..0f17eba143 100644 --- a/net/filter.c +++ b/net/filter.c @@ -17,6 +17,8 @@ #include "net/vhost_net.h" #include "qom/object_interfaces.h" #include "qemu/iov.h" +#include "net/colo.h" +#include "migration/colo.h" static inline bool qemu_can_skip_netfilter(NetFilterState *nf) { @@ -245,11 +247,26 @@ static void netfilter_finalize(Object *obj) g_free(nf->netdev_id); } +static void dummy_handle_event(NetFilterState *nf, int event, Error **errp) +{ + switch (event) { + case COLO_EVENT_CHECKPOINT: + break; + case COLO_EVENT_FAILOVER: + object_property_set_str(OBJECT(nf), "off", "status", errp); + break; + default: + break; + } +} + static void netfilter_class_init(ObjectClass *oc, void *data) { UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc); + NetFilterClass *nfc = NETFILTER_CLASS(oc); ucc->complete = netfilter_complete; + nfc->handle_event = dummy_handle_event; } static const TypeInfo netfilter_info = { diff --git a/net/net.c b/net/net.c index a77ea88fff..b4f6a2efb2 100644 --- a/net/net.c +++ b/net/net.c @@ -1331,6 +1331,34 @@ void hmp_info_network(Monitor *mon, const QDict *qdict) } } +void colo_notify_filters_event(int event, Error **errp) +{ + NetClientState *nc, *peer; + NetClientDriver type; + NetFilterState *nf; + NetFilterClass *nfc = NULL; + Error *local_err = NULL; + + QTAILQ_FOREACH(nc, &net_clients, next) { + peer = nc->peer; + type = nc->info->type; + if (!peer || type != NET_CLIENT_DRIVER_TAP) { + continue; + } + QTAILQ_FOREACH(nf, &nc->filters, next) { + nfc = NETFILTER_GET_CLASS(OBJECT(nf)); + if (!nfc->handle_event) { + continue; + } + nfc->handle_event(nf, event, &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } + } + } +} + void qmp_set_link(const char *name, bool up, Error **errp) { NetClientState *ncs[MAX_QUEUE_NUM];
Filter needs to process the event of checkpoint/failover or other event passed by COLO frame. Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> --- include/net/filter.h | 5 +++++ net/filter.c | 17 +++++++++++++++++ net/net.c | 28 ++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+)