[PATCH V2 0/4] COLO net and runstate bugfix/optimization

[PATCH V2 0/4] COLO net and runstate bugfix/optimization

[Zhang Chen]

This series fix some COLO related issues in internal stress testing.

 - V2:
    - Add more comments in patch 2/4 commit log.

Zhang Chen (4):
  softmmu/runstate.c: add RunStateTransition support form COLO to
    PRELAUNCH
  net/colo: Fix a "double free" crash to clear the conn_list
  net/colo.c: No need to track conn_list for filter-rewriter
  net/colo.c: fix segmentation fault when packet is not parsed correctly

 net/colo-compare.c    |  2 +-
 net/colo.c            | 11 +++++++++--
 net/filter-rewriter.c |  2 +-
 net/trace-events      |  1 +
 softmmu/runstate.c    |  1 +
 5 files changed, 13 insertions(+), 4 deletions(-)

-- 
2.25.1

[PATCH V2 1/4] softmmu/runstate.c: add RunStateTransition support form COLO to PRELAUNCH

[Zhang Chen]

If the checkpoint occurs when the guest finishes restarting
but has not started running, the runstate_set() may reject
the transition from COLO to PRELAUNCH with the crash log:

{"timestamp": {"seconds": 1593484591, "microseconds": 26605},\
"event": "RESET", "data": {"guest": true, "reason": "guest-reset"}}
qemu-system-x86_64: invalid runstate transition: 'colo' -> 'prelaunch'

Long-term testing says that it's pretty safe.

Signed-off-by: Like Xu <like.xu@linux.intel.com>
Signed-off-by: Zhang Chen <chen.zhang@intel.com>
---
 softmmu/runstate.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/softmmu/runstate.c b/softmmu/runstate.c
index e0d869b21a..c021c56338 100644
--- a/softmmu/runstate.c
+++ b/softmmu/runstate.c
@@ -127,6 +127,7 @@ static const RunStateTransition runstate_transitions_def[] = {
     { RUN_STATE_RESTORE_VM, RUN_STATE_PRELAUNCH },
 
     { RUN_STATE_COLO, RUN_STATE_RUNNING },
+    { RUN_STATE_COLO, RUN_STATE_PRELAUNCH },
     { RUN_STATE_COLO, RUN_STATE_SHUTDOWN},
 
     { RUN_STATE_RUNNING, RUN_STATE_DEBUG },
-- 
2.25.1

Re: [PATCH V2 1/4] softmmu/runstate.c: add RunStateTransition support form COLO to PRELAUNCH

[Jason Wang]

On Fri, Apr 1, 2022 at 11:59 AM Zhang Chen <chen.zhang@intel.com> wrote:
>
> If the checkpoint occurs when the guest finishes restarting
> but has not started running, the runstate_set() may reject
> the transition from COLO to PRELAUNCH with the crash log:
>
> {"timestamp": {"seconds": 1593484591, "microseconds": 26605},\
> "event": "RESET", "data": {"guest": true, "reason": "guest-reset"}}
> qemu-system-x86_64: invalid runstate transition: 'colo' -> 'prelaunch'
>
> Long-term testing says that it's pretty safe.
>
> Signed-off-by: Like Xu <like.xu@linux.intel.com>
> Signed-off-by: Zhang Chen <chen.zhang@intel.com>

I'd expect this to get ack from the relevant maintainers.

Thanks

> ---
>  softmmu/runstate.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/softmmu/runstate.c b/softmmu/runstate.c
> index e0d869b21a..c021c56338 100644
> --- a/softmmu/runstate.c
> +++ b/softmmu/runstate.c
> @@ -127,6 +127,7 @@ static const RunStateTransition runstate_transitions_def[] = {
>      { RUN_STATE_RESTORE_VM, RUN_STATE_PRELAUNCH },
>
>      { RUN_STATE_COLO, RUN_STATE_RUNNING },
> +    { RUN_STATE_COLO, RUN_STATE_PRELAUNCH },
>      { RUN_STATE_COLO, RUN_STATE_SHUTDOWN},
>
>      { RUN_STATE_RUNNING, RUN_STATE_DEBUG },
> --
> 2.25.1
>

RE: [PATCH V2 1/4] softmmu/runstate.c: add RunStateTransition support form COLO to PRELAUNCH

[Zhang, Chen]

> -----Original Message-----
> From: Jason Wang <jasowang@redhat.com>
> Sent: Wednesday, April 27, 2022 4:57 PM
> To: Zhang, Chen <chen.zhang@intel.com>
> Cc: Li Zhijian <lizhijian@cn.fujitsu.com>; qemu-dev <qemu-
> devel@nongnu.org>; Like Xu <like.xu@linux.intel.com>
> Subject: Re: [PATCH V2 1/4] softmmu/runstate.c: add RunStateTransition
> support form COLO to PRELAUNCH
> 
> On Fri, Apr 1, 2022 at 11:59 AM Zhang Chen <chen.zhang@intel.com> wrote:
> >
> > If the checkpoint occurs when the guest finishes restarting but has
> > not started running, the runstate_set() may reject the transition from
> > COLO to PRELAUNCH with the crash log:
> >
> > {"timestamp": {"seconds": 1593484591, "microseconds": 26605},\
> > "event": "RESET", "data": {"guest": true, "reason": "guest-reset"}}
> > qemu-system-x86_64: invalid runstate transition: 'colo' -> 'prelaunch'
> >
> > Long-term testing says that it's pretty safe.
> >
> > Signed-off-by: Like Xu <like.xu@linux.intel.com>
> > Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> 
> I'd expect this to get ack from the relevant maintainers.
> 

The scripts/get_maintainer.pl can't find relevant maintainers for this patch.
Maybe Paolo have time to cover this simple patch related to runstate? 

Thanks
Chen

> Thanks
> 
> > ---
> >  softmmu/runstate.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/softmmu/runstate.c b/softmmu/runstate.c index
> > e0d869b21a..c021c56338 100644
> > --- a/softmmu/runstate.c
> > +++ b/softmmu/runstate.c
> > @@ -127,6 +127,7 @@ static const RunStateTransition
> runstate_transitions_def[] = {
> >      { RUN_STATE_RESTORE_VM, RUN_STATE_PRELAUNCH },
> >
> >      { RUN_STATE_COLO, RUN_STATE_RUNNING },
> > +    { RUN_STATE_COLO, RUN_STATE_PRELAUNCH },
> >      { RUN_STATE_COLO, RUN_STATE_SHUTDOWN},
> >
> >      { RUN_STATE_RUNNING, RUN_STATE_DEBUG },
> > --
> > 2.25.1
> >

RE: [PATCH V2 1/4] softmmu/runstate.c: add RunStateTransition support form COLO to PRELAUNCH

[Zhang, Chen]

> -----Original Message-----
> From: Zhang, Chen
> Sent: Wednesday, April 27, 2022 5:26 PM
> To: Jason Wang <jasowang@redhat.com>; Paolo Bonzini
> <pbonzini@redhat.com>
> Cc: Li Zhijian <lizhijian@cn.fujitsu.com>; qemu-dev <qemu-
> devel@nongnu.org>; Like Xu <like.xu@linux.intel.com>
> Subject: RE: [PATCH V2 1/4] softmmu/runstate.c: add RunStateTransition
> support form COLO to PRELAUNCH
> 
> 
> 
> > -----Original Message-----
> > From: Jason Wang <jasowang@redhat.com>
> > Sent: Wednesday, April 27, 2022 4:57 PM
> > To: Zhang, Chen <chen.zhang@intel.com>
> > Cc: Li Zhijian <lizhijian@cn.fujitsu.com>; qemu-dev <qemu-
> > devel@nongnu.org>; Like Xu <like.xu@linux.intel.com>
> > Subject: Re: [PATCH V2 1/4] softmmu/runstate.c: add RunStateTransition
> > support form COLO to PRELAUNCH
> >
> > On Fri, Apr 1, 2022 at 11:59 AM Zhang Chen <chen.zhang@intel.com> wrote:
> > >
> > > If the checkpoint occurs when the guest finishes restarting but has
> > > not started running, the runstate_set() may reject the transition
> > > from COLO to PRELAUNCH with the crash log:
> > >
> > > {"timestamp": {"seconds": 1593484591, "microseconds": 26605},\
> > > "event": "RESET", "data": {"guest": true, "reason": "guest-reset"}}
> > > qemu-system-x86_64: invalid runstate transition: 'colo' -> 'prelaunch'
> > >
> > > Long-term testing says that it's pretty safe.
> > >
> > > Signed-off-by: Like Xu <like.xu@linux.intel.com>
> > > Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> >
> > I'd expect this to get ack from the relevant maintainers.
> >
> 
> The scripts/get_maintainer.pl can't find relevant maintainers for this patch.
> Maybe Paolo have time to cover this simple patch related to runstate?

No news for a while, any comments for unmaintained files changes ?
Ping...

Thanks
Chen

> 
> Thanks
> Chen
> 
> > Thanks
> >
> > > ---
> > >  softmmu/runstate.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/softmmu/runstate.c b/softmmu/runstate.c index
> > > e0d869b21a..c021c56338 100644
> > > --- a/softmmu/runstate.c
> > > +++ b/softmmu/runstate.c
> > > @@ -127,6 +127,7 @@ static const RunStateTransition
> > runstate_transitions_def[] = {
> > >      { RUN_STATE_RESTORE_VM, RUN_STATE_PRELAUNCH },
> > >
> > >      { RUN_STATE_COLO, RUN_STATE_RUNNING },
> > > +    { RUN_STATE_COLO, RUN_STATE_PRELAUNCH },
> > >      { RUN_STATE_COLO, RUN_STATE_SHUTDOWN},
> > >
> > >      { RUN_STATE_RUNNING, RUN_STATE_DEBUG },
> > > --
> > > 2.25.1
> > >

Re: [PATCH V2 1/4] softmmu/runstate.c: add RunStateTransition support form COLO to PRELAUNCH

[Jason Wang]

On Sat, May 7, 2022 at 10:03 AM Zhang, Chen <chen.zhang@intel.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Zhang, Chen
> > Sent: Wednesday, April 27, 2022 5:26 PM
> > To: Jason Wang <jasowang@redhat.com>; Paolo Bonzini
> > <pbonzini@redhat.com>
> > Cc: Li Zhijian <lizhijian@cn.fujitsu.com>; qemu-dev <qemu-
> > devel@nongnu.org>; Like Xu <like.xu@linux.intel.com>
> > Subject: RE: [PATCH V2 1/4] softmmu/runstate.c: add RunStateTransition
> > support form COLO to PRELAUNCH
> >
> >
> >
> > > -----Original Message-----
> > > From: Jason Wang <jasowang@redhat.com>
> > > Sent: Wednesday, April 27, 2022 4:57 PM
> > > To: Zhang, Chen <chen.zhang@intel.com>
> > > Cc: Li Zhijian <lizhijian@cn.fujitsu.com>; qemu-dev <qemu-
> > > devel@nongnu.org>; Like Xu <like.xu@linux.intel.com>
> > > Subject: Re: [PATCH V2 1/4] softmmu/runstate.c: add RunStateTransition
> > > support form COLO to PRELAUNCH
> > >
> > > On Fri, Apr 1, 2022 at 11:59 AM Zhang Chen <chen.zhang@intel.com> wrote:
> > > >
> > > > If the checkpoint occurs when the guest finishes restarting but has
> > > > not started running, the runstate_set() may reject the transition
> > > > from COLO to PRELAUNCH with the crash log:
> > > >
> > > > {"timestamp": {"seconds": 1593484591, "microseconds": 26605},\
> > > > "event": "RESET", "data": {"guest": true, "reason": "guest-reset"}}
> > > > qemu-system-x86_64: invalid runstate transition: 'colo' -> 'prelaunch'
> > > >
> > > > Long-term testing says that it's pretty safe.
> > > >
> > > > Signed-off-by: Like Xu <like.xu@linux.intel.com>
> > > > Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> > >
> > > I'd expect this to get ack from the relevant maintainers.
> > >
> >
> > The scripts/get_maintainer.pl can't find relevant maintainers for this patch.
> > Maybe Paolo have time to cover this simple patch related to runstate?
>
> No news for a while, any comments for unmaintained files changes ?
> Ping...

Adding David and Juan.

Thanks

>
> Thanks
> Chen
>
> >
> > Thanks
> > Chen
> >
> > > Thanks
> > >
> > > > ---
> > > >  softmmu/runstate.c | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/softmmu/runstate.c b/softmmu/runstate.c index
> > > > e0d869b21a..c021c56338 100644
> > > > --- a/softmmu/runstate.c
> > > > +++ b/softmmu/runstate.c
> > > > @@ -127,6 +127,7 @@ static const RunStateTransition
> > > runstate_transitions_def[] = {
> > > >      { RUN_STATE_RESTORE_VM, RUN_STATE_PRELAUNCH },
> > > >
> > > >      { RUN_STATE_COLO, RUN_STATE_RUNNING },
> > > > +    { RUN_STATE_COLO, RUN_STATE_PRELAUNCH },
> > > >      { RUN_STATE_COLO, RUN_STATE_SHUTDOWN},
> > > >
> > > >      { RUN_STATE_RUNNING, RUN_STATE_DEBUG },
> > > > --
> > > > 2.25.1
> > > >
>

Re: [PATCH V2 1/4] softmmu/runstate.c: add RunStateTransition support form COLO to PRELAUNCH

[Dr. David Alan Gilbert]

* Jason Wang (jasowang@redhat.com) wrote:
> On Sat, May 7, 2022 at 10:03 AM Zhang, Chen <chen.zhang@intel.com> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Zhang, Chen
> > > Sent: Wednesday, April 27, 2022 5:26 PM
> > > To: Jason Wang <jasowang@redhat.com>; Paolo Bonzini
> > > <pbonzini@redhat.com>
> > > Cc: Li Zhijian <lizhijian@cn.fujitsu.com>; qemu-dev <qemu-
> > > devel@nongnu.org>; Like Xu <like.xu@linux.intel.com>
> > > Subject: RE: [PATCH V2 1/4] softmmu/runstate.c: add RunStateTransition
> > > support form COLO to PRELAUNCH
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Jason Wang <jasowang@redhat.com>
> > > > Sent: Wednesday, April 27, 2022 4:57 PM
> > > > To: Zhang, Chen <chen.zhang@intel.com>
> > > > Cc: Li Zhijian <lizhijian@cn.fujitsu.com>; qemu-dev <qemu-
> > > > devel@nongnu.org>; Like Xu <like.xu@linux.intel.com>
> > > > Subject: Re: [PATCH V2 1/4] softmmu/runstate.c: add RunStateTransition
> > > > support form COLO to PRELAUNCH
> > > >
> > > > On Fri, Apr 1, 2022 at 11:59 AM Zhang Chen <chen.zhang@intel.com> wrote:
> > > > >
> > > > > If the checkpoint occurs when the guest finishes restarting but has
> > > > > not started running, the runstate_set() may reject the transition
> > > > > from COLO to PRELAUNCH with the crash log:
> > > > >
> > > > > {"timestamp": {"seconds": 1593484591, "microseconds": 26605},\
> > > > > "event": "RESET", "data": {"guest": true, "reason": "guest-reset"}}
> > > > > qemu-system-x86_64: invalid runstate transition: 'colo' -> 'prelaunch'
> > > > >
> > > > > Long-term testing says that it's pretty safe.
> > > > >
> > > > > Signed-off-by: Like Xu <like.xu@linux.intel.com>
> > > > > Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> > > >
> > > > I'd expect this to get ack from the relevant maintainers.
> > > >
> > >
> > > The scripts/get_maintainer.pl can't find relevant maintainers for this patch.
> > > Maybe Paolo have time to cover this simple patch related to runstate?
> >
> > No news for a while, any comments for unmaintained files changes ?
> > Ping...
> 
> Adding David and Juan.

This looks OK to me;

Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

it should be fine to merge it along with the pull that takes the other
patches.

Dave

> Thanks
> 
> >
> > Thanks
> > Chen
> >
> > >
> > > Thanks
> > > Chen
> > >
> > > > Thanks
> > > >
> > > > > ---
> > > > >  softmmu/runstate.c | 1 +
> > > > >  1 file changed, 1 insertion(+)
> > > > >
> > > > > diff --git a/softmmu/runstate.c b/softmmu/runstate.c index
> > > > > e0d869b21a..c021c56338 100644
> > > > > --- a/softmmu/runstate.c
> > > > > +++ b/softmmu/runstate.c
> > > > > @@ -127,6 +127,7 @@ static const RunStateTransition
> > > > runstate_transitions_def[] = {
> > > > >      { RUN_STATE_RESTORE_VM, RUN_STATE_PRELAUNCH },
> > > > >
> > > > >      { RUN_STATE_COLO, RUN_STATE_RUNNING },
> > > > > +    { RUN_STATE_COLO, RUN_STATE_PRELAUNCH },
> > > > >      { RUN_STATE_COLO, RUN_STATE_SHUTDOWN},
> > > > >
> > > > >      { RUN_STATE_RUNNING, RUN_STATE_DEBUG },
> > > > > --
> > > > > 2.25.1
> > > > >
> >
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH V2 1/4] softmmu/runstate.c: add RunStateTransition support form COLO to PRELAUNCH

[Jason Wang]

On Wed, May 18, 2022 at 9:09 PM Dr. David Alan Gilbert
<dgilbert@redhat.com> wrote:
>
> * Jason Wang (jasowang@redhat.com) wrote:
> > On Sat, May 7, 2022 at 10:03 AM Zhang, Chen <chen.zhang@intel.com> wrote:
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Zhang, Chen
> > > > Sent: Wednesday, April 27, 2022 5:26 PM
> > > > To: Jason Wang <jasowang@redhat.com>; Paolo Bonzini
> > > > <pbonzini@redhat.com>
> > > > Cc: Li Zhijian <lizhijian@cn.fujitsu.com>; qemu-dev <qemu-
> > > > devel@nongnu.org>; Like Xu <like.xu@linux.intel.com>
> > > > Subject: RE: [PATCH V2 1/4] softmmu/runstate.c: add RunStateTransition
> > > > support form COLO to PRELAUNCH
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Jason Wang <jasowang@redhat.com>
> > > > > Sent: Wednesday, April 27, 2022 4:57 PM
> > > > > To: Zhang, Chen <chen.zhang@intel.com>
> > > > > Cc: Li Zhijian <lizhijian@cn.fujitsu.com>; qemu-dev <qemu-
> > > > > devel@nongnu.org>; Like Xu <like.xu@linux.intel.com>
> > > > > Subject: Re: [PATCH V2 1/4] softmmu/runstate.c: add RunStateTransition
> > > > > support form COLO to PRELAUNCH
> > > > >
> > > > > On Fri, Apr 1, 2022 at 11:59 AM Zhang Chen <chen.zhang@intel.com> wrote:
> > > > > >
> > > > > > If the checkpoint occurs when the guest finishes restarting but has
> > > > > > not started running, the runstate_set() may reject the transition
> > > > > > from COLO to PRELAUNCH with the crash log:
> > > > > >
> > > > > > {"timestamp": {"seconds": 1593484591, "microseconds": 26605},\
> > > > > > "event": "RESET", "data": {"guest": true, "reason": "guest-reset"}}
> > > > > > qemu-system-x86_64: invalid runstate transition: 'colo' -> 'prelaunch'
> > > > > >
> > > > > > Long-term testing says that it's pretty safe.
> > > > > >
> > > > > > Signed-off-by: Like Xu <like.xu@linux.intel.com>
> > > > > > Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> > > > >
> > > > > I'd expect this to get ack from the relevant maintainers.
> > > > >
> > > >
> > > > The scripts/get_maintainer.pl can't find relevant maintainers for this patch.
> > > > Maybe Paolo have time to cover this simple patch related to runstate?
> > >
> > > No news for a while, any comments for unmaintained files changes ?
> > > Ping...
> >
> > Adding David and Juan.
>
> This looks OK to me;
>
> Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Great.

>
> it should be fine to merge it along with the pull that takes the other
> patches.

Yes, I've queued this series.

Thanks

>
> Dave
>
> > Thanks
> >
> > >
> > > Thanks
> > > Chen
> > >
> > > >
> > > > Thanks
> > > > Chen
> > > >
> > > > > Thanks
> > > > >
> > > > > > ---
> > > > > >  softmmu/runstate.c | 1 +
> > > > > >  1 file changed, 1 insertion(+)
> > > > > >
> > > > > > diff --git a/softmmu/runstate.c b/softmmu/runstate.c index
> > > > > > e0d869b21a..c021c56338 100644
> > > > > > --- a/softmmu/runstate.c
> > > > > > +++ b/softmmu/runstate.c
> > > > > > @@ -127,6 +127,7 @@ static const RunStateTransition
> > > > > runstate_transitions_def[] = {
> > > > > >      { RUN_STATE_RESTORE_VM, RUN_STATE_PRELAUNCH },
> > > > > >
> > > > > >      { RUN_STATE_COLO, RUN_STATE_RUNNING },
> > > > > > +    { RUN_STATE_COLO, RUN_STATE_PRELAUNCH },
> > > > > >      { RUN_STATE_COLO, RUN_STATE_SHUTDOWN},
> > > > > >
> > > > > >      { RUN_STATE_RUNNING, RUN_STATE_DEBUG },
> > > > > > --
> > > > > > 2.25.1
> > > > > >
> > >
> >
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>

RE: [PATCH V2 1/4] softmmu/runstate.c: add RunStateTransition support form COLO to PRELAUNCH

[Zhang, Chen]

> > > > > > On Fri, Apr 1, 2022 at 11:59 AM Zhang Chen <chen.zhang@intel.com>
> wrote:
> > > > > > >
> > > > > > > If the checkpoint occurs when the guest finishes restarting
> > > > > > > but has not started running, the runstate_set() may reject
> > > > > > > the transition from COLO to PRELAUNCH with the crash log:
> > > > > > >
> > > > > > > {"timestamp": {"seconds": 1593484591, "microseconds":
> > > > > > > 26605},\
> > > > > > > "event": "RESET", "data": {"guest": true, "reason":
> > > > > > > "guest-reset"}}
> > > > > > > qemu-system-x86_64: invalid runstate transition: 'colo' ->
> 'prelaunch'
> > > > > > >
> > > > > > > Long-term testing says that it's pretty safe.
> > > > > > >
> > > > > > > Signed-off-by: Like Xu <like.xu@linux.intel.com>
> > > > > > > Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> > > > > >
> > > > > > I'd expect this to get ack from the relevant maintainers.
> > > > > >
> > > > >
> > > > > The scripts/get_maintainer.pl can't find relevant maintainers for this
> patch.
> > > > > Maybe Paolo have time to cover this simple patch related to runstate?
> > > >
> > > > No news for a while, any comments for unmaintained files changes ?
> > > > Ping...
> > >
> > > Adding David and Juan.
> >
> > This looks OK to me;
> >
> > Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> 
> Great.
> 
> >
> > it should be fine to merge it along with the pull that takes the other
> > patches.
> 
> Yes, I've queued this series.

Hi Jason,  did this series get lost in the net queue branch?

Thanks
Chen

> 
> Thanks
> 
> >
> > Dave
> >
> > > Thanks
> > >
> > > >
> >

Re: [PATCH V2 1/4] softmmu/runstate.c: add RunStateTransition support form COLO to PRELAUNCH

[Jason Wang]

On Wed, Jul 6, 2022 at 1:02 PM Zhang, Chen <chen.zhang@intel.com> wrote:
>
> > > > > > > On Fri, Apr 1, 2022 at 11:59 AM Zhang Chen <chen.zhang@intel.com>
> > wrote:
> > > > > > > >
> > > > > > > > If the checkpoint occurs when the guest finishes restarting
> > > > > > > > but has not started running, the runstate_set() may reject
> > > > > > > > the transition from COLO to PRELAUNCH with the crash log:
> > > > > > > >
> > > > > > > > {"timestamp": {"seconds": 1593484591, "microseconds":
> > > > > > > > 26605},\
> > > > > > > > "event": "RESET", "data": {"guest": true, "reason":
> > > > > > > > "guest-reset"}}
> > > > > > > > qemu-system-x86_64: invalid runstate transition: 'colo' ->
> > 'prelaunch'
> > > > > > > >
> > > > > > > > Long-term testing says that it's pretty safe.
> > > > > > > >
> > > > > > > > Signed-off-by: Like Xu <like.xu@linux.intel.com>
> > > > > > > > Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> > > > > > >
> > > > > > > I'd expect this to get ack from the relevant maintainers.
> > > > > > >
> > > > > >
> > > > > > The scripts/get_maintainer.pl can't find relevant maintainers for this
> > patch.
> > > > > > Maybe Paolo have time to cover this simple patch related to runstate?
> > > > >
> > > > > No news for a while, any comments for unmaintained files changes ?
> > > > > Ping...
> > > >
> > > > Adding David and Juan.
> > >
> > > This looks OK to me;
> > >
> > > Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> >
> > Great.
> >
> > >
> > > it should be fine to merge it along with the pull that takes the other
> > > patches.
> >
> > Yes, I've queued this series.
>
> Hi Jason,  did this series get lost in the net queue branch?

Unfortunately, yes.

It will be included in the net pull request.

Sorry.

>
> Thanks
> Chen
>
> >
> > Thanks
> >
> > >
> > > Dave
> > >
> > > > Thanks
> > > >
> > > > >
> > >
>

[PATCH V2 2/4] net/colo: Fix a "double free" crash to clear the conn_list

[Zhang Chen]

We notice the QEMU may crash when the guest has too many
incoming network connections with the following log:

15197@1593578622.668573:colo_proxy_main : colo proxy connection hashtable full, clear it
free(): invalid pointer
[1]    15195 abort (core dumped)  qemu-system-x86_64 ....

This is because we create the s->connection_track_table with
g_hash_table_new_full() which is defined as:

GHashTable * g_hash_table_new_full (GHashFunc hash_func,
                       GEqualFunc key_equal_func,
                       GDestroyNotify key_destroy_func,
                       GDestroyNotify value_destroy_func);

The fourth parameter connection_destroy() will be called to free the
memory allocated for all 'Connection' values in the hashtable when
we call g_hash_table_remove_all() in the connection_hashtable_reset().

But both connection_track_table and conn_list reference to the same
conn instance. It will trigger double free in conn_list clear. So this
patch remove free action on hash table side to avoid double free the
conn.

Signed-off-by: Like Xu <like.xu@linux.intel.com>
Signed-off-by: Zhang Chen <chen.zhang@intel.com>
---
 net/colo-compare.c    | 2 +-
 net/filter-rewriter.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 62554b5b3c..ab054cfd21 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -1324,7 +1324,7 @@ static void colo_compare_complete(UserCreatable *uc, Error **errp)
     s->connection_track_table = g_hash_table_new_full(connection_key_hash,
                                                       connection_key_equal,
                                                       g_free,
-                                                      connection_destroy);
+                                                      NULL);
 
     colo_compare_iothread(s);
 
diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c
index bf05023dc3..c18c4c2019 100644
--- a/net/filter-rewriter.c
+++ b/net/filter-rewriter.c
@@ -383,7 +383,7 @@ static void colo_rewriter_setup(NetFilterState *nf, Error **errp)
     s->connection_track_table = g_hash_table_new_full(connection_key_hash,
                                                       connection_key_equal,
                                                       g_free,
-                                                      connection_destroy);
+                                                      NULL);
     s->incoming_queue = qemu_new_net_queue(qemu_netfilter_pass_to_next, nf);
 }
 
-- 
2.25.1

[PATCH V2 3/4] net/colo.c: No need to track conn_list for filter-rewriter

[Zhang Chen]

Filter-rewriter no need to track connection in conn_list.
This patch fix the glib g_queue_is_empty assertion when COLO guest
keep a lot of network connection.

Signed-off-by: Zhang Chen <chen.zhang@intel.com>
Reviewed-by: Li Zhijian <lizhijian@fujitsu.com>
---
 net/colo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/colo.c b/net/colo.c
index 1f8162f59f..694f3c93ef 100644
--- a/net/colo.c
+++ b/net/colo.c
@@ -218,7 +218,7 @@ Connection *connection_get(GHashTable *connection_track_table,
             /*
              * clear the conn_list
              */
-            while (!g_queue_is_empty(conn_list)) {
+            while (conn_list && !g_queue_is_empty(conn_list)) {
                 connection_destroy(g_queue_pop_head(conn_list));
             }
         }
-- 
2.25.1

[PATCH V2 4/4] net/colo.c: fix segmentation fault when packet is not parsed correctly

[Zhang Chen]

When COLO use only one vnet_hdr_support parameter between
filter-redirector and filter-mirror(or colo-compare), COLO will crash
with segmentation fault. Back track as follow:

Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
0x0000555555cb200b in eth_get_l2_hdr_length (p=0x0)
    at /home/tao/project/COLO/colo-qemu/include/net/eth.h:296
296         uint16_t proto = be16_to_cpu(PKT_GET_ETH_HDR(p)->h_proto);
(gdb) bt
0  0x0000555555cb200b in eth_get_l2_hdr_length (p=0x0)
    at /home/tao/project/COLO/colo-qemu/include/net/eth.h:296
1  0x0000555555cb22b4 in parse_packet_early (pkt=0x555556a44840) at
net/colo.c:49
2  0x0000555555cb2b91 in is_tcp_packet (pkt=0x555556a44840) at
net/filter-rewriter.c:63

So wrong vnet_hdr_len will cause pkt->data become NULL. Add check to
raise error and add trace-events to track vnet_hdr_len.

Signed-off-by: Tao Xu <tao3.xu@intel.com>
Signed-off-by: Zhang Chen <chen.zhang@intel.com>
Reviewed-by: Li Zhijian <lizhijian@fujitsu.com>
---
 net/colo.c       | 9 ++++++++-
 net/trace-events | 1 +
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/net/colo.c b/net/colo.c
index 694f3c93ef..6b0ff562ad 100644
--- a/net/colo.c
+++ b/net/colo.c
@@ -46,7 +46,14 @@ int parse_packet_early(Packet *pkt)
     static const uint8_t vlan[] = {0x81, 0x00};
     uint8_t *data = pkt->data + pkt->vnet_hdr_len;
     uint16_t l3_proto;
-    ssize_t l2hdr_len = eth_get_l2_hdr_length(data);
+    ssize_t l2hdr_len;
+
+    if (data == NULL) {
+        trace_colo_proxy_main_vnet_info("This packet is not parsed correctly, "
+                                        "pkt->vnet_hdr_len", pkt->vnet_hdr_len);
+        return 1;
+    }
+    l2hdr_len = eth_get_l2_hdr_length(data);
 
     if (pkt->size < ETH_HLEN + pkt->vnet_hdr_len) {
         trace_colo_proxy_main("pkt->size < ETH_HLEN");
diff --git a/net/trace-events b/net/trace-events
index d7a17256cc..6af927b4b9 100644
--- a/net/trace-events
+++ b/net/trace-events
@@ -9,6 +9,7 @@ vhost_user_event(const char *chr, int event) "chr: %s got event: %d"
 
 # colo.c
 colo_proxy_main(const char *chr) ": %s"
+colo_proxy_main_vnet_info(const char *sta, int size) ": %s = %d"
 
 # colo-compare.c
 colo_compare_main(const char *chr) ": %s"
-- 
2.25.1

RE: [PATCH V2 0/4] COLO net and runstate bugfix/optimization

[Zhang, Chen]

No update for a while. Ping...

Thanks
Chen

> -----Original Message-----
> From: Zhang, Chen <chen.zhang@intel.com>
> Sent: Friday, April 1, 2022 11:47 AM
> To: Jason Wang <jasowang@redhat.com>; Li Zhijian <lizhijian@cn.fujitsu.com>
> Cc: Zhang, Chen <chen.zhang@intel.com>; qemu-dev <qemu-
> devel@nongnu.org>
> Subject: [PATCH V2 0/4] COLO net and runstate bugfix/optimization
> 
> This series fix some COLO related issues in internal stress testing.
> 
>  - V2:
>     - Add more comments in patch 2/4 commit log.
> 
> Zhang Chen (4):
>   softmmu/runstate.c: add RunStateTransition support form COLO to
>     PRELAUNCH
>   net/colo: Fix a "double free" crash to clear the conn_list
>   net/colo.c: No need to track conn_list for filter-rewriter
>   net/colo.c: fix segmentation fault when packet is not parsed correctly
> 
>  net/colo-compare.c    |  2 +-
>  net/colo.c            | 11 +++++++++--
>  net/filter-rewriter.c |  2 +-
>  net/trace-events      |  1 +
>  softmmu/runstate.c    |  1 +
>  5 files changed, 13 insertions(+), 4 deletions(-)
> 
> --
> 2.25.1

Re: [PATCH V2 0/4] COLO net and runstate bugfix/optimization

[Jason Wang]

On Thu, Apr 14, 2022 at 9:52 AM Zhang, Chen <chen.zhang@intel.com> wrote:
>
> No update for a while. Ping...
>
> Thanks
> Chen

Hi:

It's near to release, I think we can queue this for 7.1?

Thanks

>
> > -----Original Message-----
> > From: Zhang, Chen <chen.zhang@intel.com>
> > Sent: Friday, April 1, 2022 11:47 AM
> > To: Jason Wang <jasowang@redhat.com>; Li Zhijian <lizhijian@cn.fujitsu.com>
> > Cc: Zhang, Chen <chen.zhang@intel.com>; qemu-dev <qemu-
> > devel@nongnu.org>
> > Subject: [PATCH V2 0/4] COLO net and runstate bugfix/optimization
> >
> > This series fix some COLO related issues in internal stress testing.
> >
> >  - V2:
> >     - Add more comments in patch 2/4 commit log.
> >
> > Zhang Chen (4):
> >   softmmu/runstate.c: add RunStateTransition support form COLO to
> >     PRELAUNCH
> >   net/colo: Fix a "double free" crash to clear the conn_list
> >   net/colo.c: No need to track conn_list for filter-rewriter
> >   net/colo.c: fix segmentation fault when packet is not parsed correctly
> >
> >  net/colo-compare.c    |  2 +-
> >  net/colo.c            | 11 +++++++++--
> >  net/filter-rewriter.c |  2 +-
> >  net/trace-events      |  1 +
> >  softmmu/runstate.c    |  1 +
> >  5 files changed, 13 insertions(+), 4 deletions(-)
> >
> > --
> > 2.25.1
>

RE: [PATCH V2 0/4] COLO net and runstate bugfix/optimization

[Zhang, Chen]

> -----Original Message-----
> From: Jason Wang <jasowang@redhat.com>
> Sent: Thursday, April 14, 2022 11:12 AM
> To: Zhang, Chen <chen.zhang@intel.com>
> Cc: Li Zhijian <lizhijian@cn.fujitsu.com>; qemu-dev <qemu-
> devel@nongnu.org>
> Subject: Re: [PATCH V2 0/4] COLO net and runstate bugfix/optimization
> 
> On Thu, Apr 14, 2022 at 9:52 AM Zhang, Chen <chen.zhang@intel.com>
> wrote:
> >
> > No update for a while. Ping...
> >
> > Thanks
> > Chen
> 
> Hi:
> 
> It's near to release, I think we can queue this for 7.1?

Sure. I forgot to add "for 7.1" tag.

Thanks
Chen

> 
> Thanks
> 
> >
> > > -----Original Message-----
> > > From: Zhang, Chen <chen.zhang@intel.com>
> > > Sent: Friday, April 1, 2022 11:47 AM
> > > To: Jason Wang <jasowang@redhat.com>; Li Zhijian
> > > <lizhijian@cn.fujitsu.com>
> > > Cc: Zhang, Chen <chen.zhang@intel.com>; qemu-dev <qemu-
> > > devel@nongnu.org>
> > > Subject: [PATCH V2 0/4] COLO net and runstate bugfix/optimization
> > >
> > > This series fix some COLO related issues in internal stress testing.
> > >
> > >  - V2:
> > >     - Add more comments in patch 2/4 commit log.
> > >
> > > Zhang Chen (4):
> > >   softmmu/runstate.c: add RunStateTransition support form COLO to
> > >     PRELAUNCH
> > >   net/colo: Fix a "double free" crash to clear the conn_list
> > >   net/colo.c: No need to track conn_list for filter-rewriter
> > >   net/colo.c: fix segmentation fault when packet is not parsed
> > > correctly
> > >
> > >  net/colo-compare.c    |  2 +-
> > >  net/colo.c            | 11 +++++++++--
> > >  net/filter-rewriter.c |  2 +-
> > >  net/trace-events      |  1 +
> > >  softmmu/runstate.c    |  1 +
> > >  5 files changed, 13 insertions(+), 4 deletions(-)
> > >
> > > --
> > > 2.25.1
> >