qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH RFC v2 4/5] migration: remove unused &local_err


From: Philippe Mathieu-Daudé
Subject: Re: [Qemu-devel] [PATCH RFC v2 4/5] migration: remove unused &local_err parameter in multifd_save_cleanup
Date: Thu, 29 Nov 2018 15:52:34 +0100

On Thu, Nov 29, 2018 at 3:50 PM Philippe Mathieu-Daudé
<address@hidden> wrote:
> On 29/11/18 11:03, Fei Li wrote:
> > Always call migrate_set_error() to set the error state without relying
> > on whether multifd_save_cleanup() succeeds.  As the passed &local_err
> > is never used in multifd_save_cleanup(), remove it. And make the
> > function be: void multifd_save_cleanup(void).
>
> Reading this after your patch 1/5, maybe multifd_save_cleanup() lacks

I wanted to write "2/5: qemu_thread_join: fix segmentation fault"

> error reporting on failure. So the actual prototype is correct, we just
> need to use **errp in that function (like reporting invalid thread
> instead of calling qemu_thread_join()).
>
> >
> > Signed-off-by: Fei Li <address@hidden>
> > Reviewed-by: Juan Quintela <address@hidden>
> > ---
> >  migration/migration.c |  5 +----
> >  migration/ram.c       | 11 ++++-------
> >  migration/ram.h       |  2 +-
> >  3 files changed, 6 insertions(+), 12 deletions(-)
> >
> > diff --git a/migration/migration.c b/migration/migration.c
> > index 72106bddf0..0537fc0c26 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -1386,7 +1386,6 @@ static void migrate_fd_cleanup(void *opaque)
> >      qemu_savevm_state_cleanup();
> >
> >      if (s->to_dst_file) {
> > -        Error *local_err = NULL;
> >          QEMUFile *tmp;
> >
> >          trace_migrate_fd_cleanup();
> > @@ -1397,9 +1396,7 @@ static void migrate_fd_cleanup(void *opaque)
> >          }
> >          qemu_mutex_lock_iothread();
> >
> > -        if (multifd_save_cleanup(&local_err) != 0) {
> > -            error_report_err(local_err);
> > -        }
> > +        multifd_save_cleanup();
> >          qemu_mutex_lock(&s->qemu_file_lock);
> >          tmp = s->to_dst_file;
> >          s->to_dst_file = NULL;
> > diff --git a/migration/ram.c b/migration/ram.c
> > index e13b9349d0..c44cb6f56d 100644
> > --- a/migration/ram.c
> > +++ b/migration/ram.c
> > @@ -917,13 +917,12 @@ static void multifd_send_terminate_threads(Error *err)
> >      }
> >  }
> >
> > -int multifd_save_cleanup(Error **errp)
> > +void multifd_save_cleanup(void)
> >  {
> >      int i;
> > -    int ret = 0;
> >
> >      if (!migrate_use_multifd()) {
> > -        return 0;
> > +        return;
> >      }
> >      multifd_send_terminate_threads(NULL);
> >      for (i = 0; i < migrate_multifd_channels(); i++) {
> > @@ -953,7 +952,6 @@ int multifd_save_cleanup(Error **errp)
> >      multifd_send_state->pages = NULL;
> >      g_free(multifd_send_state);
> >      multifd_send_state = NULL;
> > -    return ret;
> >  }
> >
> >  static void multifd_send_sync_main(void)
> > @@ -1071,9 +1069,8 @@ static void multifd_new_send_channel_async(QIOTask 
> > *task, gpointer opaque)
> >      Error *local_err = NULL;
> >
> >      if (qio_task_propagate_error(task, &local_err)) {
> > -        if (multifd_save_cleanup(&local_err) != 0) {
> > -            migrate_set_error(migrate_get_current(), local_err);
> > -        }
> > +        migrate_set_error(migrate_get_current(), local_err);
> > +        multifd_save_cleanup();
> >      } else {
> >          p->c = QIO_CHANNEL(sioc);
> >          qio_channel_set_delay(p->c, false);
> > diff --git a/migration/ram.h b/migration/ram.h
> > index 046d3074be..936177b3e9 100644
> > --- a/migration/ram.h
> > +++ b/migration/ram.h
> > @@ -43,7 +43,7 @@ uint64_t ram_bytes_remaining(void);
> >  uint64_t ram_bytes_total(void);
> >
> >  int multifd_save_setup(void);
> > -int multifd_save_cleanup(Error **errp);
> > +void multifd_save_cleanup(void);
> >  int multifd_load_setup(void);
> >  int multifd_load_cleanup(Error **errp);
> >  bool multifd_recv_all_channels_created(void);
> >



reply via email to

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