This way we can change the packet size in the future and everything
will work.  We choose an arbitrary big number (100 times configured
size) as a limit about how big we will reallocate.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/ram.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index e22d02760b..75a8fc21f8 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -723,13 +723,13 @@ static void multifd_pages_clear(MultiFDPages_t *pages)
 static void multifd_send_fill_packet(MultiFDSendParams *p)
 {
     MultiFDPacket_t *packet = p->packet;
-    uint32_t page_count = MULTIFD_PACKET_SIZE / qemu_target_page_size();
+    uint32_t page_max = MULTIFD_PACKET_SIZE / qemu_target_page_size();
     int i;
 
     packet->magic = cpu_to_be32(MULTIFD_MAGIC);
     packet->version = cpu_to_be32(MULTIFD_VERSION);
     packet->flags = cpu_to_be32(p->flags);
-    packet->pages_alloc = cpu_to_be32(page_count);
+    packet->pages_alloc = cpu_to_be32(page_max);
     packet->pages_used = cpu_to_be32(p->pages->used);
     packet->next_packet_size = cpu_to_be32(p->next_packet_size);
     packet->packet_num = cpu_to_be64(p->packet_num);
@@ -746,7 +746,7 @@ static void multifd_send_fill_packet(MultiFDSendParams *p)
 static int multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp)
 {
     MultiFDPacket_t *packet = p->packet;
-    uint32_t page_count = MULTIFD_PACKET_SIZE / qemu_target_page_size();
+    uint32_t pages_max = MULTIFD_PACKET_SIZE / qemu_target_page_size();
     RAMBlock *block;
     int i;
 
@@ -769,12 +769,24 @@ static int multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp)
     p->flags = be32_to_cpu(packet->flags);
 
     packet->pages_alloc = be32_to_cpu(packet->pages_alloc);
-    if (packet->pages_alloc > page_count) {
+    /*
+     * If we recevied a packet that is 100 times bigger than expected
+     * just stop migration.  It is a magic number.
+     */
+    if (packet->pages_alloc > pages_max * 100) {
         error_setg(errp, "multifd: received packet "
-                   "with size %d and expected maximum size %d",
-                   packet->pages_alloc, page_count) ;
+                   "with size %d and expected size %d",
+                   packet->pages_alloc, pages_max) ;
         return -1;
     }
+    /*
+     * We received a packet that is bigger than expected but inside
+     * reasonable limits (see previous comment).  Just reallocate.
+     */
+    if (packet->pages_alloc > p->pages->allocated) {
+        multifd_pages_clear(p->pages);
+        multifd_pages_init(packet->pages_alloc);
+    }
 
     p->pages->used = be32_to_cpu(packet->pages_used);
     if (p->pages->used > packet->pages_alloc) {
-- 
2.20.1

* Juan Quintela (quintela@redhat.com) wrote:
> This way we can change the packet size in the future and everything
> will work.  We choose an arbitrary big number (100 times configured
> size) as a limit about how big we will reallocate.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  migration/ram.c | 24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index e22d02760b..75a8fc21f8 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -723,13 +723,13 @@ static void multifd_pages_clear(MultiFDPages_t *pages)
>  static void multifd_send_fill_packet(MultiFDSendParams *p)
>  {
>      MultiFDPacket_t *packet = p->packet;
> -    uint32_t page_count = MULTIFD_PACKET_SIZE / qemu_target_page_size();
> +    uint32_t page_max = MULTIFD_PACKET_SIZE / qemu_target_page_size();
>      int i;
>  
>      packet->magic = cpu_to_be32(MULTIFD_MAGIC);
>      packet->version = cpu_to_be32(MULTIFD_VERSION);
>      packet->flags = cpu_to_be32(p->flags);
> -    packet->pages_alloc = cpu_to_be32(page_count);
> +    packet->pages_alloc = cpu_to_be32(page_max);
>      packet->pages_used = cpu_to_be32(p->pages->used);
>      packet->next_packet_size = cpu_to_be32(p->next_packet_size);
>      packet->packet_num = cpu_to_be64(p->packet_num);
> @@ -746,7 +746,7 @@ static void multifd_send_fill_packet(MultiFDSendParams *p)
>  static int multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp)
>  {
>      MultiFDPacket_t *packet = p->packet;
> -    uint32_t page_count = MULTIFD_PACKET_SIZE / qemu_target_page_size();
> +    uint32_t pages_max = MULTIFD_PACKET_SIZE / qemu_target_page_size();
>      RAMBlock *block;
>      int i;
>  
> @@ -769,12 +769,24 @@ static int multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp)
>      p->flags = be32_to_cpu(packet->flags);
>  
>      packet->pages_alloc = be32_to_cpu(packet->pages_alloc);
> -    if (packet->pages_alloc > page_count) {
> +    /*
> +     * If we recevied a packet that is 100 times bigger than expected
> +     * just stop migration.  It is a magic number.
> +     */
> +    if (packet->pages_alloc > pages_max * 100) {
>          error_setg(errp, "multifd: received packet "
> -                   "with size %d and expected maximum size %d",
> -                   packet->pages_alloc, page_count) ;
> +                   "with size %d and expected size %d",
> +                   packet->pages_alloc, pages_max) ;

Should that end with pages_max * 100 ?

>          return -1;
>      }
> +    /*
> +     * We received a packet that is bigger than expected but inside
> +     * reasonable limits (see previous comment).  Just reallocate.
> +     */
> +    if (packet->pages_alloc > p->pages->allocated) {
> +        multifd_pages_clear(p->pages);
> +        multifd_pages_init(packet->pages_alloc);
> +    }
>  
>      p->pages->used = be32_to_cpu(packet->pages_used);
>      if (p->pages->used > packet->pages_alloc) {

Other than that error message, I think it's OK, although the names get
very confusing (max, alloc, allocated)


Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> -- 
> 2.20.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * Juan Quintela (quintela@redhat.com) wrote:
>> This way we can change the packet size in the future and everything
>> will work.  We choose an arbitrary big number (100 times configured
>> size) as a limit about how big we will reallocate.
>> 
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  migration/ram.c | 24 ++++++++++++++++++------
>>  1 file changed, 18 insertions(+), 6 deletions(-)
>> 
>> diff --git a/migration/ram.c b/migration/ram.c
>> index e22d02760b..75a8fc21f8 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -723,13 +723,13 @@ static void multifd_pages_clear(MultiFDPages_t *pages)
>>  static void multifd_send_fill_packet(MultiFDSendParams *p)
>>  {
>>      MultiFDPacket_t *packet = p->packet;
>> -    uint32_t page_count = MULTIFD_PACKET_SIZE / qemu_target_page_size();
>> +    uint32_t page_max = MULTIFD_PACKET_SIZE / qemu_target_page_size();
>>      int i;
>>  
>>      packet->magic = cpu_to_be32(MULTIFD_MAGIC);
>>      packet->version = cpu_to_be32(MULTIFD_VERSION);
>>      packet->flags = cpu_to_be32(p->flags);
>> -    packet->pages_alloc = cpu_to_be32(page_count);
>> +    packet->pages_alloc = cpu_to_be32(page_max);
>>      packet->pages_used = cpu_to_be32(p->pages->used);
>>      packet->next_packet_size = cpu_to_be32(p->next_packet_size);
>>      packet->packet_num = cpu_to_be64(p->packet_num);
>> @@ -746,7 +746,7 @@ static void multifd_send_fill_packet(MultiFDSendParams *p)
>>  static int multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp)
>>  {
>>      MultiFDPacket_t *packet = p->packet;
>> -    uint32_t page_count = MULTIFD_PACKET_SIZE / qemu_target_page_size();
>> +    uint32_t pages_max = MULTIFD_PACKET_SIZE / qemu_target_page_size();
>>      RAMBlock *block;
>>      int i;
>>  
>> @@ -769,12 +769,24 @@ static int multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp)
>>      p->flags = be32_to_cpu(packet->flags);
>>  
>>      packet->pages_alloc = be32_to_cpu(packet->pages_alloc);
>> -    if (packet->pages_alloc > page_count) {
>> +    /*
>> +     * If we recevied a packet that is 100 times bigger than expected
>> +     * just stop migration.  It is a magic number.
>> +     */
>> +    if (packet->pages_alloc > pages_max * 100) {
>>          error_setg(errp, "multifd: received packet "
>> -                   "with size %d and expected maximum size %d",
>> -                   packet->pages_alloc, page_count) ;
>> +                   "with size %d and expected size %d",
>> +                   packet->pages_alloc, pages_max) ;
>
> Should that end with pages_max * 100 ?

Not sure.

The *allocated* by defaault size is pages_max.  If we receive
bigger packets, we update it, but until a limit (arbitrary, I am open to
other limits).

So, what multifd is expecting here is pages_max.  But it will cope with
anything that is smaller than pages_max * 100.  So, what I should put on
the error message?  100 * pages_max or pages_max?

It appears that for you it is simpler to understand pages_max * 100, and
as I don't care, I am just changing it.

>>          return -1;
>>      }
>> +    /*
>> +     * We received a packet that is bigger than expected but inside
>> +     * reasonable limits (see previous comment).  Just reallocate.
>> +     */
>> +    if (packet->pages_alloc > p->pages->allocated) {
>> +        multifd_pages_clear(p->pages);
>> +        multifd_pages_init(packet->pages_alloc);
>> +    }
>>  
>>      p->pages->used = be32_to_cpu(packet->pages_used);
>>      if (p->pages->used > packet->pages_alloc) {
>
> Other than that error message, I think it's OK, although the names get
> very confusing (max, alloc, allocated)

I am open to suggestions.  I just got out of names :-(

>
>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Thanks.