Re: [Qemu-devel] [PATCH v2 07/12] Rework ram_control_load_hook to hook during block load

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Juan Quintela (quintela@redhat.com) wrote:
> "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >
> > We need the names of RAMBlocks as they're loaded for RDMA,
> > reuse a slightly modified ram_control_load_hook:
> >   a) Pass a 'data' parameter to use for the name in the block-reg
> >      case
> >   b) Only some hook types now require the presence of a hook function.
> >
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> 
> > @@ -1569,6 +1569,8 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
> >                                  error_report_err(local_err);
> >                              }
> >                          }
> > +                        ram_control_load_hook(f, RAM_CONTROL_BLOCK_REG,
> > +                                              block->idstr);
> >                          break;
> >                      }
> >                  }
> > @@ -1637,7 +1639,7 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
> >              break;
> >          default:
> >              if (flags & RAM_SAVE_FLAG_HOOK) {
> > -                ram_control_load_hook(f, flags);
> > +                ram_control_load_hook(f, RAM_CONTROL_HOOK, NULL);
> 
> Using a function in only two places, and passing two additional
> parameters for that ....
> 
> > +static int rdma_load_hook(QEMUFile *f, void *opaque, uint64_t flags, void *data)
> > +{
> > +    switch (flags) {
> > +    case RAM_CONTROL_BLOCK_REG:
> > +        /* TODO A later patch */
> > +        return 0;
> > +        break;
> > +
> > +    case RAM_CONTROL_HOOK:
> > +        return qemu_rdma_registration_handle(f, opaque);
> > +
> > +    default:
> > +        /* Shouldn't be called with any other values */
> > +        abort();
> > +    }
> 
> And you are doing two completely different things depending of the flag ....

The other way would be to add a new function pointer to qemu_file and
wire up the new pointer.  It didn't seem any prettier.

Dave

> Later, Juan.
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK