On Fri, Mar 16, 2018 at 05:49:07PM +0000, Daniel P. Berrangé wrote:
> On Fri, Mar 16, 2018 at 12:53:49PM +0100, Juan Quintela wrote:
> > Signed-off-by: Juan Quintela <quintela@redhat.com>
> > ---
> > migration/ram.c | 20 ++++++++++++++++++++
> > 1 file changed, 20 insertions(+)
> >
> > diff --git a/migration/ram.c b/migration/ram.c
> > index 7266351fd0..1b8095a358 100644
> > --- a/migration/ram.c
> > +++ b/migration/ram.c
> > @@ -414,6 +414,16 @@ static void terminate_multifd_send_threads(Error *errp)
> > {
> > int i;
> >
> > + if (errp) {
> > + MigrationState *s = migrate_get_current();
> > + migrate_set_error(s, errp);
>
> This doesn't look quiet right. You're checking if 'errp' is a non-NULL,
> which just tells you if the caller wants to collect the error, not
> whether an error has happened. For the latter you need
>
> if (errp && *errp)
>
> seems a little strange though for the caller to pass an error into this
> method for reporting.
Oh wait, I'm being mislead by the unusual parameter name.
An "errp" name should only ever be used for a "Error **", but we
only have an "Error *" here.
So just fix the parameter name to be "err" instead of "errp".
> > + if (s->state == MIGRATION_STATUS_SETUP ||
> > + s->state == MIGRATION_STATUS_ACTIVE) {
> > + migrate_set_state(&s->state, s->state,
> > + MIGRATION_STATUS_FAILED);
> > + }
> > + }
> > +
> > for (i = 0; i < multifd_send_state->count; i++) {
> > MultiFDSendParams *p = &multifd_send_state->params[i];
> >
> > @@ -514,6 +524,16 @@ static void terminate_multifd_recv_threads(Error *errp)
This parameter name needs fixing too.
These are actually a pre-existing problem in current GIT, so worth fixing
in a separate patch.
> > {
> > int i;
> >
> > + if (errp) {
> > + MigrationState *s = migrate_get_current();
> > + migrate_set_error(s, errp);
> > + if (s->state == MIGRATION_STATUS_SETUP ||
> > + s->state == MIGRATION_STATUS_ACTIVE) {
> > + migrate_set_state(&s->state, s->state,
> > + MIGRATION_STATUS_FAILED);
> > + }
> > + }
> > +
> > for (i = 0; i < multifd_recv_state->count; i++) {
> > MultiFDRecvParams *p = &multifd_recv_state->params[i];
> >
> > --
> > 2.14.3
> >
> >
>
> Regards,
> Daniel
> --
> |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o- https://fstop138.berrange.com :|
> |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|