Re: [Qemu-devel] [PATCH v9 04/12] migration: Start of multiple fd work

["Daniel P. Berrange" <berrange@redhat.com>]

On Wed, Oct 04, 2017 at 12:46:28PM +0200, Juan Quintela wrote:
> We create new channels for each new thread created. We send through
> them a string containing <uuid> multifd <channel number> so we are
> sure that we connect the right channels in both sides.

This message needs updating now that we send a struct.


> diff --git a/migration/ram.c b/migration/ram.c
> index b83f8977c5..b57006594b 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -414,9 +425,27 @@ int multifd_save_cleanup(Error **errp)
>      return ret;
>  }
>  
> +typedef struct {
> +    uint32_t version;
> +    uint8_t id;
> +    char uuid[UUID_FMT_LEN];
> +} MigrateMultiFDInit_t;

We add an __attribute__((packed)) here since we send it directly
on the wire. Perhaps put 'uuid' field before 'id' when doing that
so 'uuid' gets a more natural alignment.

If we use 'unsigned char uuid[16]' then you don't need to convert
from string format either...

>  static void *multifd_send_thread(void *opaque)
>  {
>      MultiFDSendParams *p = opaque;
> +    MigrateMultiFDInit_t msg;
> +    Error *local_err = NULL;
> +    size_t ret;
> +
> +    msg.version = 1;
> +    msg.id = p->id;
> +    qemu_uuid_unparse(&qemu_uuid, (char *)&msg.uuid);

eg this could be   memcpy(msg.uuid, qemu_uuid.data, sizeof(msg.uuid))

> +    ret = qio_channel_write_all(p->c, (char *)&msg, sizeof(msg), &local_err);
> +    if (ret != 0) {
> +        terminate_multifd_send_threads(local_err);
> +        return NULL;
> +    }
>  
>      while (true) {
>          qemu_mutex_lock(&p->mutex);
> @@ -431,6 +460,27 @@ static void *multifd_send_thread(void *opaque)
>      return NULL;
>  }

> +void multifd_new_channel(QIOChannel *ioc)
> +{
> +    MultiFDRecvParams *p;
> +    MigrateMultiFDInit_t msg;
> +    Error *local_err = NULL;
> +    char *uuid;
> +    size_t ret;
> +
> +    ret = qio_channel_read_all(ioc, (char *)&msg, sizeof(msg), &local_err);
> +    if (ret != 0) {
> +        terminate_multifd_recv_threads(local_err);
> +        return;
> +    }
> +
> +    uuid = qemu_uuid_unparse_strdup(&qemu_uuid);

...and here we would avoid need to unparse, instead..

> +
> +    if (strcmp(msg.uuid, uuid)) {

  memcmp(msg.uuid, qemu_uuid.data, sizeof(msg.uuid) != 0

> +        g_free(uuid);
> +        error_setg(&local_err, "multifd: received uuid '%s' and expected "
> +                   "uuid '%s' for channel %hhd", msg.uuid, uuid, msg.id);
> +        terminate_multifd_recv_threads(local_err);
> +        return;
> +    }
> +    g_free(uuid);
> +

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 :|