Re: [Qemu-devel] [PATCH COLO-Frame v13 17/39] COLO: Load VMState into qsb before restore it

[Hailiang Zhang]

On 2016/1/5 3:00, Dr. David Alan Gilbert wrote:
> * zhanghailiang (address@hidden) wrote:
> > We should not destroy the state of SVM (Secondary VM) until we receive the whole
> > state from the PVM (Primary VM), in case the primary fails in the middle of 
> > sending
> > the state, so, here we cache the device state in Secondary before restore it.
> >
> > Besides, we should call qemu_system_reset() before load VM state,
> > which can ensure the data is intact.
> >
> > Signed-off-by: zhanghailiang <address@hidden>
> > Signed-off-by: Li Zhijian <address@hidden>
> > Signed-off-by: Gonglei <address@hidden>
> > Reviewed-by: Dr. David Alan Gilbert <address@hidden>
> > Cc: Dr. David Alan Gilbert <address@hidden>
> > ---
> > v13:
> > - Fix the define of colo_get_cmd_value() to use 'Error **errp' instead of
> >    return value.
> > v12:
> > - Use the new helper colo_get_cmd_value() instead of colo_ctl_get()
> > ---
> >   migration/colo.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> >   1 file changed, 72 insertions(+), 2 deletions(-)
> >
> > diff --git a/migration/colo.c b/migration/colo.c
> > index 925eb3c..8414feb 100644
> > --- a/migration/colo.c
> > +++ b/migration/colo.c
> > @@ -114,6 +114,28 @@ static void colo_get_check_cmd(QEMUFile *f, COLOCommand 
> > expect_cmd,
> >       }
> >   }
> >
> > +static uint64_t colo_get_cmd_value(QEMUFile *f, uint32_t expect_cmd,
> > +                                   Error **errp)
> > +{
> > +    Error *local_err = NULL;
> > +    uint64_t value;
> > +    int ret;
> > +
> > +    colo_get_check_cmd(f, expect_cmd, &local_err);
> > +    if (local_err) {
> > +        error_propagate(errp, local_err);
> > +        return 0;
> > +    }
> > +
> > +    value = qemu_get_be64(f);
> > +    ret = qemu_file_get_error(f);
> > +    if (ret < 0) {
> > +        error_setg_errno(errp, -ret, "Failed to get value for COlO commnd: %s",
>
> Tiny typo; you've used 'CO*l*O' rather than 'COLO' - actually all your other 
> errors say 'colo'
> so it's probably best to standardise.

OK, i will fix them all in next version, thanks.

> Dave
>
> > +                         COLOCommand_lookup[expect_cmd]);
> > +    }
> > +    return value;
> > +}
> > +
> >   static int colo_do_checkpoint_transaction(MigrationState *s,
> >                                             QEMUSizedBuffer *buffer)
> >   {
> > @@ -297,6 +319,10 @@ static void colo_wait_handle_cmd(QEMUFile *f, int 
> > *checkpoint_request,
> >   void *colo_process_incoming_thread(void *opaque)
> >   {
> >       MigrationIncomingState *mis = opaque;
> > +    QEMUFile *fb = NULL;
> > +    QEMUSizedBuffer *buffer = NULL; /* Cache incoming device state */
> > +    uint64_t total_size;
> > +    uint64_t value;
> >       Error *local_err = NULL;
> >       int ret;
> >
> > @@ -320,6 +346,12 @@ void *colo_process_incoming_thread(void *opaque)
> >           goto out;
> >       }
> >
> > +    buffer = qsb_create(NULL, COLO_BUFFER_BASE_SIZE);
> > +    if (buffer == NULL) {
> > +        error_report("Failed to allocate colo buffer!");
> > +        goto out;
> > +    }
> > +
> >       colo_put_cmd(mis->to_src_file, COLO_COMMAND_CHECKPOINT_READY,
> >                    &local_err);
> >       if (local_err) {
> > @@ -347,7 +379,21 @@ void *colo_process_incoming_thread(void *opaque)
> >               goto out;
> >           }
> >
> > -        /* TODO: read migration data into colo buffer */
> > +        /* read the VM state total size first */
> > +        value = colo_get_cmd_value(mis->from_src_file,
> > +                                 COLO_COMMAND_VMSTATE_SIZE, &local_err);
> > +        if (local_err) {
> > +            goto out;
> > +        }
> > +
> > +        /* read vm device state into colo buffer */
> > +        total_size = qsb_fill_buffer(buffer, mis->from_src_file, value);
> > +        if (total_size != value) {
> > +            error_report("Got %lu VMState data, less than expected %lu",
> > +                         total_size, value);
> > +            ret = -EINVAL;
> > +            goto out;
> > +        }
> >
> >           colo_put_cmd(mis->to_src_file, COLO_COMMAND_VMSTATE_RECEIVED,
> >                        &local_err);
> > @@ -355,13 +401,32 @@ void *colo_process_incoming_thread(void *opaque)
> >               goto out;
> >           }
> >
> > -        /* TODO: load vm state */
> > +        /* open colo buffer for read */
> > +        fb = qemu_bufopen("r", buffer);
> > +        if (!fb) {
> > +            error_report("Can't open colo buffer for read");
> > +            goto out;
> > +        }
> > +
> > +        qemu_mutex_lock_iothread();
> > +        qemu_system_reset(VMRESET_SILENT);
> > +        if (qemu_loadvm_state(fb) < 0) {
> > +            error_report("COLO: loadvm failed");
> > +            qemu_mutex_unlock_iothread();
> > +            goto out;
> > +        }
> > +        qemu_mutex_unlock_iothread();
> > +
> > +        /* TODO: flush vm state */
> >
> >           colo_put_cmd(mis->to_src_file, COLO_COMMAND_VMSTATE_LOADED,
> >                        &local_err);
> >           if (local_err) {
> >               goto out;
> >           }
> > +
> > +        qemu_fclose(fb);
> > +        fb = NULL;
> >       }
> >
> >   out:
> > @@ -370,6 +435,11 @@ out:
> >           error_report_err(local_err);
> >       }
> >
> > +    if (fb) {
> > +        qemu_fclose(fb);
> > +    }
> > +    qsb_free(buffer);
> > +
> >       qemu_mutex_lock_iothread();
> >       colo_release_ram_cache();
> >       qemu_mutex_unlock_iothread();
> > --
> > 1.8.3.1
> --
> Dr. David Alan Gilbert / address@hidden / Manchester, UK
>
> .