Re: [Qemu-devel] [PATCH v7 28/42] Postcopy: Postcopy startup in migration thread

[Dr. David Alan Gilbert]

* Juan Quintela (address@hidden) wrote:
> "Dr. David Alan Gilbert" <address@hidden> wrote:

> >> > +     * We need to leave the fd free for page transfers during the
> >> > +     * loading of the device state, so wrap all the remaining
> >> > +     * commands and state into a package that gets sent in one go
> >> > +     */
> >> > +    QEMUFile *fb = qemu_bufopen("w", NULL);
> >> > +    if (!fb) {
> >> > +        error_report("Failed to create buffered file");
> >> > +        goto fail;
> >> > +    }
> >> > +
> >> > +    qemu_savevm_state_complete_precopy(fb);
> >> > +    qemu_savevm_send_ping(fb, 3);
> >> > +
> >> > +    qemu_savevm_send_postcopy_run(fb);
> >> > +
> >> > +    /* <><> end of stuff going into the package */
> >> > +    qsb = qemu_buf_get(fb);
> >> > +
> >> > +    /* Now send that blob */
> >> > +    if (qemu_savevm_send_packaged(ms->file, qsb)) {
> >> > +        goto fail_closefb;
> >> > +    }
> >> > +    qemu_fclose(fb);
> >> 
> >> Why can't we send this directly without the extra copy?
> >> I guess that there are some missing/extra section starts/end whatever?
> >> Anything specific?
> >
> > The problem is that the destination has to be able to read the chunk
> > of migration stream off the fd and leave the fd free for page requests
> > that may be required during loading the device state.
> > Since the migration-stream is unstructured, there is no way to read
> > a chunk of stream off without knowing the length of that chunk, and the
> > only way to know that chunk is to write it to a buffer and then see
> > how big it is.
> 
> Arghhh.  ok.  Comment?

I've changed the comment at the start of that section to:

     * While loading the device state we may trigger page transfer
     * requests and the fd must be free to process those, and thus
     * the destination must read the whole device state off the fd before
     * it starts processing it.  Unfortunately the ad-hoc migration format
     * doesn't allow the destination to know the size to read without fully
     * parsing it through each devices load-state code (especially the open
     * coded devices that use get/put).
     * So we wrap the device state up in a package with a length at the start;
     * to do this we use a qemu_buf to hold the whole of the device state.

Dave

> 
> >
> >> > +    ms->downtime =  qemu_clock_get_ms(QEMU_CLOCK_REALTIME) - 
> >> > time_at_stop;
> >> 
> >> Now, that we are here, is there a counter of the time that takes the
> >> postcopy stage?  Just curious.
> >
> > No, not separate.
> >
> >> > +/*
> >> >   * Master migration thread on the source VM.
> >> >   * It drives the migration and pumps the data down the outgoing channel.
> >> >   */
> >> >  static void *migration_thread(void *opaque)
> >> >  {
> >> >      MigrationState *s = opaque;
> >> > +    /* Used by the bandwidth calcs, updated later */
> >> >      int64_t initial_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
> >> >      int64_t setup_start = qemu_clock_get_ms(QEMU_CLOCK_HOST);
> >> >      int64_t initial_bytes = 0;
> >> >      int64_t max_size = 0;
> >> >      int64_t start_time = initial_time;
> >> >      bool old_vm_running = false;
> >> > +    bool entered_postcopy = false;
> >> > +    /* The active state we expect to be in; ACTIVE or POSTCOPY_ACTIVE */
> >> > +    enum MigrationStatus current_active_type = MIGRATION_STATUS_ACTIVE;
> >> 
> >> current_active_state?
> >
> > Changed.
> >
> > Dave
> >
> > --
> > Dr. David Alan Gilbert / address@hidden / Manchester, UK
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK