Re: [PATCH v5 10/14] vfio/migration: Implement VFIO migration protocol v2

[Avihai Horon <avihaih@nvidia.com>]

On 09/01/2023 12:20, Cédric Le Goater wrote:
> External email: Use caution opening links or attachments
>
>
> Hello Avihai,
>
>
> On 12/29/22 12:03, Avihai Horon wrote:
>>
>> +static int vfio_save_setup(QEMUFile *f, void *opaque)
>> +{
>> +    VFIODevice *vbasedev = opaque;
>> +    VFIOMigration *migration = vbasedev->migration;
>> +    uint64_t stop_copy_size;
>> +
>> +    qemu_put_be64(f, VFIO_MIG_FLAG_DEV_SETUP_STATE);
>> +
>> +    if (vfio_query_stop_copy_size(vbasedev, &stop_copy_size)) {
>> +        stop_copy_size = VFIO_MIG_DEFAULT_DATA_BUFFER_SIZE;
>> +    }
>> +    migration->data_buffer_size = 
>> MIN(VFIO_MIG_DEFAULT_DATA_BUFFER_SIZE,
>> +                                      stop_copy_size);
>> +    migration->data_buffer = 
>> g_try_malloc0(migration->data_buffer_size);
>> +    if (!migration->data_buffer) {
>> +        error_report("%s: Failed to allocate migration data buffer",
>> +                     vbasedev->name);
>> +        return -ENOMEM;
>> +    }
>> +
>> +    trace_vfio_save_setup(vbasedev->name, migration->data_buffer_size);
>> +
>> +    qemu_put_be64(f, VFIO_MIG_FLAG_END_OF_STATE);
>> +
>> +    return qemu_file_get_error(f);
>> +}
>> +
>
> This fails to compile with :
>
>   gcc version 12.2.1 20221121 (Red Hat 12.2.1-4) (GCC) complains with :
>
>
>   ../include/qemu/osdep.h:315:22: error: ‘stop_copy_size’ may be used 
> uninitialized [-Werror=maybe-uninitialized]
>     315 |         _a < _b ? _a : _b;                              \
>         |                      ^
>   ../hw/vfio/migration.c:262:14: note: ‘stop_copy_size’ was declared here
>     262 |     uint64_t stop_copy_size;
>         |              ^~~~~~~~~~~~~~
>   cc1: all warnings being treated as errors
>
> May be rework the code slightly to avoid the breakage :
>
> +++ qemu.git/hw/vfio/migration.c
> @@ -259,13 +259,11 @@ static int vfio_save_setup(QEMUFile *f,
>  {
>      VFIODevice *vbasedev = opaque;
>      VFIOMigration *migration = vbasedev->migration;
> -    uint64_t stop_copy_size;
> +    uint64_t stop_copy_size = VFIO_MIG_DEFAULT_DATA_BUFFER_SIZE;
>
>      qemu_put_be64(f, VFIO_MIG_FLAG_DEV_SETUP_STATE);
>
> -    if (vfio_query_stop_copy_size(vbasedev, &stop_copy_size)) {
> -        stop_copy_size = VFIO_MIG_DEFAULT_DATA_BUFFER_SIZE;
> -    }
> +    vfio_query_stop_copy_size(vbasedev, &stop_copy_size);
>      migration->data_buffer_size = MIN(VFIO_MIG_DEFAULT_DATA_BUFFER_SIZE,
>                                        stop_copy_size);
>      migration->data_buffer = g_try_malloc0(migration->data_buffer_size);
>
>
> and report the error in vfio_query_stop_copy_size()
>
Thanks, Cedric.

There is another similar case in vfio_save_pending().
I will fix both of them.

> Thanks,
>
> C.