qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH V11 16/20] filter: Add handle_event method for N


From: Zhang Chen
Subject: Re: [Qemu-devel] [PATCH V11 16/20] filter: Add handle_event method for NetFilterClass
Date: Tue, 21 Aug 2018 17:25:51 +0800

On Tue, Aug 21, 2018 at 11:30 AM Jason Wang <address@hidden> 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 <address@hidden>
> > ---
> >   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];
>
>


reply via email to

[Prev in Thread] Current Thread [Next in Thread]