Re: 答复: [PATCH 5/6] migration/qemu-file.c: Don't ratelimit a shutdown fd

2020-05-18 Thread Dr. David Alan Gilbert
* Zhanghailiang (zhang.zhanghaili...@huawei.com) wrote:
> > This causes the migration thread to hang if we failover during checkpoint. A
> > shutdown fd won't cause network traffic anyway.
> > 
> 
> I'm not quite sure if this modification can take side effect on normal 
> migration process or not,
> There are several places calling it.
> 
> Maybe Juan and Dave can help ;)
> 
> > Signed-off-by: Lukas Straub 
> > ---
> >  migration/qemu-file.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/migration/qemu-file.c b/migration/qemu-file.c index
> > 1c3a358a14..0748b5810f 100644
> > --- a/migration/qemu-file.c
> > +++ b/migration/qemu-file.c
> > @@ -660,7 +660,7 @@ int64_t qemu_ftell(QEMUFile *f)  int
> > qemu_file_rate_limit(QEMUFile *f)  {
> >  if (f->shutdown) {
> > -return 1;
> > +return 0;
> >  }

This looks wrong to me; I'd be curious to understand how it's hanging
for you.
'1' means 'stop what you're doing', 0 means carry on; carrying on with a
shutdown fd sounds wrong.

If we look at ram.c we have:

while ((ret = qemu_file_rate_limit(f)) == 0 ||
!QSIMPLEQ_EMPTY(&rs->src_page_requests)) {
int pages;


so if it returns '1', as it does at the moment it should cause it to
exit the ram_save_iterate loop - which is what we want if it's failing.
Thus I think you need to find the actual place it's stuck in this case -
I suspect it's repeatedly calling ram_save_iterate and then exiting it,
but if that's happening perhaps we're missing a qemu_file_get_error
check somewhere.

Dave

> >  if (qemu_file_get_error(f)) {
> >  return 1;
> > --
> > 2.20.1
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK




答复: [PATCH 5/6] migration/qemu-file.c: Don't ratelimit a shutdown fd

2020-05-14 Thread Zhanghailiang
> This causes the migration thread to hang if we failover during checkpoint. A
> shutdown fd won't cause network traffic anyway.
> 

I'm not quite sure if this modification can take side effect on normal 
migration process or not,
There are several places calling it.

Maybe Juan and Dave can help ;)

> Signed-off-by: Lukas Straub 
> ---
>  migration/qemu-file.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c index
> 1c3a358a14..0748b5810f 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -660,7 +660,7 @@ int64_t qemu_ftell(QEMUFile *f)  int
> qemu_file_rate_limit(QEMUFile *f)  {
>  if (f->shutdown) {
> -return 1;
> +return 0;
>  }
>  if (qemu_file_get_error(f)) {
>  return 1;
> --
> 2.20.1