Re: [PATCH v4 14/25] memory: Add Error** argument to the global_dirty_log routines

[Cédric Le Goater]

> > --- a/migration/ram.c
> > +++ b/migration/ram.c
> > @@ -2836,18 +2836,31 @@ static void 
> > migration_bitmap_clear_discarded_pages(RAMState *rs)
> >   
> >   static void ram_init_bitmaps(RAMState *rs)
> >   {
> > +    Error *local_err = NULL;
> > +    bool ret = true;
> > +
> >       qemu_mutex_lock_ramlist();
> >   
> >       WITH_RCU_READ_LOCK_GUARD() {
> >           ram_list_init_bitmaps();
> >           /* We don't use dirty log with background snapshots */
> >           if (!migrate_background_snapshot()) {
> > -            memory_global_dirty_log_start(GLOBAL_DIRTY_MIGRATION);
> > +            ret = memory_global_dirty_log_start(GLOBAL_DIRTY_MIGRATION,
> > +                                                &local_err);
> > +            if (!ret) {
> > +                error_report_err(local_err);
> > +                goto out_unlock;
>
> Here we may need to free the bitmaps created in ram_list_init_bitmaps().
>
> We can have a helper ram_bitmaps_destroy() for that.
>
> One thing be careful is the new file_bmap can be created but missing in the
> ram_save_cleanup(), it's because it's freed earlier.  IMHO if we will have
> a new ram_bitmaps_destroy() we can unconditionally free file_bmap there
> too, as if it's freed early g_free() is noop.

OK. Let's do that in a new prereq patch. I will change ram_state_init()
and xbzrle_init() to take an Error ** argument while at it.


Thanks,

C.