| Message ID | 1443168480-5067-1-git-send-email-yanghy@cn.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
Hi Markus, If I missed something, please feel free to point out... On 09/25/2015 04:08 PM, Yang Hongyang wrote: > This patchset addressed Markus comment on netfilter patch > series, most of them are comment fixes. > > It is based on jason's net tree: > https://github.com/jasowang/qemu/tree/net > > Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com> > --- > include/net/filter.h | 2 ++ > net/filter-buffer.c | 15 +++++++++------ > net/filter.c | 2 +- > 3 files changed, 12 insertions(+), 7 deletions(-) > > diff --git a/include/net/filter.h b/include/net/filter.h > index 3fa80c9..d3ff569 100644 > --- a/include/net/filter.h > +++ b/include/net/filter.h > @@ -39,8 +39,10 @@ typedef ssize_t (FilterReceiveIOV)(NetFilterState *nc, > struct NetFilterClass { > ObjectClass parent_class; > > + /* optional */ > FilterSetup *setup; > FilterCleanup *cleanup; > + /* mandatory */ > FilterReceiveIOV *receive_iov; > }; > typedef struct NetFilterClass NetFilterClass; > diff --git a/net/filter-buffer.c b/net/filter-buffer.c > index ef94e91..a528737 100644 > --- a/net/filter-buffer.c > +++ b/net/filter-buffer.c > @@ -42,8 +42,10 @@ static void filter_buffer_flush(NetFilterState *nf) > static void filter_buffer_release_timer(void *opaque) > { > NetFilterState *nf = opaque; > + > FilterBufferState *s = FILTER_BUFFER(nf); > filter_buffer_flush(nf); > + /* Timer rearmed to fire again in s->interval microseconds. */ > timer_mod(&s->release_timer, > qemu_clock_get_us(QEMU_CLOCK_VIRTUAL) + s->interval); > } > @@ -59,9 +61,9 @@ static ssize_t filter_buffer_receive_iov(NetFilterState *nf, > FilterBufferState *s = FILTER_BUFFER(nf); > > /* > - * we return size when buffer a packet, the sender will take it as > - * a already sent packet, so sent_cb should not be called later > - * FIXME: even if guest can't receive packet for some reasons. Filter > + * We return size when buffer a packet, the sender will take it as > + * a already sent packet, so sent_cb should not be called later. > + * FIXME: Even if guest can't receive packet for some reasons. Filter > * can still accept packet until its internal queue is full. > */ > qemu_net_queue_append_iov(s->incoming_queue, sender, flags, > @@ -89,7 +91,7 @@ static void filter_buffer_setup(NetFilterState *nf, Error **errp) > FilterBufferState *s = FILTER_BUFFER(nf); > > /* > - * this check should be dropped when there're VM FT solutions like MC > + * We may want to accept zero interval when VM FT solutions like MC > * or COLO use this filter to release packets on demand. > */ > if (!s->interval) { > @@ -102,6 +104,7 @@ static void filter_buffer_setup(NetFilterState *nf, Error **errp) > if (s->interval) { > timer_init_us(&s->release_timer, QEMU_CLOCK_VIRTUAL, > filter_buffer_release_timer, nf); > + /* Timer armed to fire in s->interval microseconds. */ > timer_mod(&s->release_timer, > qemu_clock_get_us(QEMU_CLOCK_VIRTUAL) + s->interval); > } > @@ -137,8 +140,8 @@ static void filter_buffer_set_interval(Object *obj, Visitor *v, void *opaque, > goto out; > } > if (!value) { > - error_setg(&local_err, "Property '%s.%s' doesn't take value '%" > - PRIu32 "'", object_get_typename(obj), name, value); > + error_setg(&local_err, "Property '%s.%s' requires a positive value", > + object_get_typename(obj), name); > goto out; > } > s->interval = value; > diff --git a/net/filter.c b/net/filter.c > index a8adc89..7b2437a 100644 > --- a/net/filter.c > +++ b/net/filter.c > @@ -164,7 +164,7 @@ static void netfilter_complete(UserCreatable *uc, Error **errp) > "a network backend id"); > return; > } else if (queues > 1) { > - error_setg(errp, "Multi queue is not supported"); > + error_setg(errp, "multiqueue is not supported"); > return; > } > >
On 09/25/2015 02:08 AM, Yang Hongyang wrote: > This patchset addressed Markus comment on netfilter patch > series, most of them are comment fixes. > > It is based on jason's net tree: > https://github.com/jasowang/qemu/tree/net If that tree is not yet merged mainline, wouldn't it be better to squash these fixes directly into the offending patches, instead of using it as a followup patch? > /* > - * we return size when buffer a packet, the sender will take it as > - * a already sent packet, so sent_cb should not be called later > - * FIXME: even if guest can't receive packet for some reasons. Filter > + * We return size when buffer a packet, the sender will take it as > + * a already sent packet, so sent_cb should not be called later. s/a already/an already/
Yang Hongyang <yanghy@cn.fujitsu.com> writes: > This patchset addressed Markus comment on netfilter patch > series, most of them are comment fixes. > > It is based on jason's net tree: > https://github.com/jasowang/qemu/tree/net > > Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com> I think we have enough issues to warrant a full respin. Jason's call, of course.
On 09/25/2015 10:30 PM, Eric Blake wrote: > On 09/25/2015 02:08 AM, Yang Hongyang wrote: >> This patchset addressed Markus comment on netfilter patch >> series, most of them are comment fixes. >> >> It is based on jason's net tree: >> https://github.com/jasowang/qemu/tree/net > If that tree is not yet merged mainline, wouldn't it be better to squash > these fixes directly into the offending patches, instead of using it as > a followup patch? Yes, and maybe a new version. > >> /* >> - * we return size when buffer a packet, the sender will take it as >> - * a already sent packet, so sent_cb should not be called later >> - * FIXME: even if guest can't receive packet for some reasons. Filter >> + * We return size when buffer a packet, the sender will take it as >> + * a already sent packet, so sent_cb should not be called later. > s/a already/an already/ >
On 09/25/2015 11:33 PM, Markus Armbruster wrote: > Yang Hongyang <yanghy@cn.fujitsu.com> writes: > >> This patchset addressed Markus comment on netfilter patch >> series, most of them are comment fixes. >> >> It is based on jason's net tree: >> https://github.com/jasowang/qemu/tree/net >> >> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com> > I think we have enough issues to warrant a full respin. Jason's call, > of course. > I thought most of them were comments fixes but look not. Hongyang: I will drop the v11 from the tree, sorry. Please address the comments and send V12. Thanks
On 09/28/2015 02:15 PM, Jason Wang wrote: > > > On 09/25/2015 11:33 PM, Markus Armbruster wrote: >> Yang Hongyang <yanghy@cn.fujitsu.com> writes: >> >>> This patchset addressed Markus comment on netfilter patch >>> series, most of them are comment fixes. >>> >>> It is based on jason's net tree: >>> https://github.com/jasowang/qemu/tree/net >>> >>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com> >> I think we have enough issues to warrant a full respin. Jason's call, >> of course. >> > > I thought most of them were comments fixes but look not. > > Hongyang: > > I will drop the v11 from the tree, sorry. Please address the comments > and send V12. Ok, will do, thanks. > > Thanks >
diff --git a/include/net/filter.h b/include/net/filter.h index 3fa80c9..d3ff569 100644 --- a/include/net/filter.h +++ b/include/net/filter.h @@ -39,8 +39,10 @@ typedef ssize_t (FilterReceiveIOV)(NetFilterState *nc, struct NetFilterClass { ObjectClass parent_class; + /* optional */ FilterSetup *setup; FilterCleanup *cleanup; + /* mandatory */ FilterReceiveIOV *receive_iov; }; typedef struct NetFilterClass NetFilterClass; diff --git a/net/filter-buffer.c b/net/filter-buffer.c index ef94e91..a528737 100644 --- a/net/filter-buffer.c +++ b/net/filter-buffer.c @@ -42,8 +42,10 @@ static void filter_buffer_flush(NetFilterState *nf) static void filter_buffer_release_timer(void *opaque) { NetFilterState *nf = opaque; + FilterBufferState *s = FILTER_BUFFER(nf); filter_buffer_flush(nf); + /* Timer rearmed to fire again in s->interval microseconds. */ timer_mod(&s->release_timer, qemu_clock_get_us(QEMU_CLOCK_VIRTUAL) + s->interval); } @@ -59,9 +61,9 @@ static ssize_t filter_buffer_receive_iov(NetFilterState *nf, FilterBufferState *s = FILTER_BUFFER(nf); /* - * we return size when buffer a packet, the sender will take it as - * a already sent packet, so sent_cb should not be called later - * FIXME: even if guest can't receive packet for some reasons. Filter + * We return size when buffer a packet, the sender will take it as + * a already sent packet, so sent_cb should not be called later. + * FIXME: Even if guest can't receive packet for some reasons. Filter * can still accept packet until its internal queue is full. */ qemu_net_queue_append_iov(s->incoming_queue, sender, flags, @@ -89,7 +91,7 @@ static void filter_buffer_setup(NetFilterState *nf, Error **errp) FilterBufferState *s = FILTER_BUFFER(nf); /* - * this check should be dropped when there're VM FT solutions like MC + * We may want to accept zero interval when VM FT solutions like MC * or COLO use this filter to release packets on demand. */ if (!s->interval) { @@ -102,6 +104,7 @@ static void filter_buffer_setup(NetFilterState *nf, Error **errp) if (s->interval) { timer_init_us(&s->release_timer, QEMU_CLOCK_VIRTUAL, filter_buffer_release_timer, nf); + /* Timer armed to fire in s->interval microseconds. */ timer_mod(&s->release_timer, qemu_clock_get_us(QEMU_CLOCK_VIRTUAL) + s->interval); } @@ -137,8 +140,8 @@ static void filter_buffer_set_interval(Object *obj, Visitor *v, void *opaque, goto out; } if (!value) { - error_setg(&local_err, "Property '%s.%s' doesn't take value '%" - PRIu32 "'", object_get_typename(obj), name, value); + error_setg(&local_err, "Property '%s.%s' requires a positive value", + object_get_typename(obj), name); goto out; } s->interval = value; diff --git a/net/filter.c b/net/filter.c index a8adc89..7b2437a 100644 --- a/net/filter.c +++ b/net/filter.c @@ -164,7 +164,7 @@ static void netfilter_complete(UserCreatable *uc, Error **errp) "a network backend id"); return; } else if (queues > 1) { - error_setg(errp, "Multi queue is not supported"); + error_setg(errp, "multiqueue is not supported"); return; }
This patchset addressed Markus comment on netfilter patch series, most of them are comment fixes. It is based on jason's net tree: https://github.com/jasowang/qemu/tree/net Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com> --- include/net/filter.h | 2 ++ net/filter-buffer.c | 15 +++++++++------ net/filter.c | 2 +- 3 files changed, 12 insertions(+), 7 deletions(-)