Re: [Qemu-devel] [PATCH v4 08/32] migration: allow send_rq to fail

[Peter Xu]

On Thu, Nov 30, 2017 at 12:13:57PM +0000, Dr. David Alan Gilbert wrote:
> * Peter Xu (address@hidden) wrote:
> > We will not allow failures to happen when sending data from destination
> > to source via the return path. However it is possible that there can be
> > errors along the way.  This patch allows the migrate_send_rp_message()
> > to return error when it happens, and further extended it to
> > migrate_send_rp_req_pages().
> > 
> > Reviewed-by: Dr. David Alan Gilbert <address@hidden>
> > Signed-off-by: Peter Xu <address@hidden>
> > ---
> >  migration/migration.c | 38 ++++++++++++++++++++++++++++++--------
> >  migration/migration.h |  2 +-
> >  2 files changed, 31 insertions(+), 9 deletions(-)
> > 
> > diff --git a/migration/migration.c b/migration/migration.c
> > index 8d93b891e3..db896233f6 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -199,17 +199,35 @@ static void deferred_incoming_migration(Error **errp)
> >   * Send a message on the return channel back to the source
> >   * of the migration.
> >   */
> > -static void migrate_send_rp_message(MigrationIncomingState *mis,
> > -                                    enum mig_rp_message_type message_type,
> > -                                    uint16_t len, void *data)
> > +static int migrate_send_rp_message(MigrationIncomingState *mis,
> > +                                   enum mig_rp_message_type message_type,
> > +                                   uint16_t len, void *data)
> >  {
> > +    int ret = 0;
> > +
> >      trace_migrate_send_rp_message((int)message_type, len);
> >      qemu_mutex_lock(&mis->rp_mutex);
> > +
> > +    /*
> > +     * It's possible that the file handle got lost due to network
> > +     * failures.
> > +     */
> > +    if (!mis->to_src_file) {
> > +        ret = -EIO;
> > +        goto error;
> > +    }
> > +
> >      qemu_put_be16(mis->to_src_file, (unsigned int)message_type);
> >      qemu_put_be16(mis->to_src_file, len);
> >      qemu_put_buffer(mis->to_src_file, data, len);
> >      qemu_fflush(mis->to_src_file);
> > +
> > +    /* It's possible that qemu file got error during sending */
> > +    ret = qemu_file_get_error(mis->to_src_file);
> > +
> > +error:
> >      qemu_mutex_unlock(&mis->rp_mutex);
> > +    return ret;
> >  }
> >  
> >  /* Request a range of pages from the source VM at the given
> > @@ -219,26 +237,30 @@ static void 
> > migrate_send_rp_message(MigrationIncomingState *mis,
> >   *   Start: Address offset within the RB
> >   *   Len: Length in bytes required - must be a multiple of pagesize
> >   */
> > -void migrate_send_rp_req_pages(MigrationIncomingState *mis, const char 
> > *rbname,
> > -                               ram_addr_t start, size_t len)
> > +int migrate_send_rp_req_pages(MigrationIncomingState *mis, const char 
> > *rbname,
> > +                              ram_addr_t start, size_t len)
> >  {
> >      uint8_t bufc[12 + 1 + 255]; /* start (8), len (4), rbname up to 256 */
> >      size_t msglen = 12; /* start + len */
> > +    int rbname_len;
> > +    enum mig_rp_message_type msg_type;
> >  
> >      *(uint64_t *)bufc = cpu_to_be64((uint64_t)start);
> >      *(uint32_t *)(bufc + 8) = cpu_to_be32((uint32_t)len);
> >  
> >      if (rbname) {
> > -        int rbname_len = strlen(rbname);
> > +        rbname_len = strlen(rbname);
> 
> I don't think that move of the declaration of rbname_len is necessary;
> it's only msglen that you need to keep for longer.

Yes it's not necessary.  I'll avoid touching it.  Thanks,

-- 
Peter Xu