On 1/9/23 16:12, Avihai Horon wrote:
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.
also, in vfio_migration_query_flags() :
+static int vfio_migration_query_flags(VFIODevice *vbasedev, uint64_t
*mig_flags)
+{
+ uint64_t buf[DIV_ROUND_UP(sizeof(struct vfio_device_feature) +
+ sizeof(struct
vfio_device_feature_migration),
+ sizeof(uint64_t))] = {};
+ struct vfio_device_feature *feature = (struct vfio_device_feature *)buf;
+ struct vfio_device_feature_migration *mig =
+ (struct vfio_device_feature_migration *)feature->data;
+
+ feature->argsz = sizeof(buf);
+ feature->flags = VFIO_DEVICE_FEATURE_GET | VFIO_DEVICE_FEATURE_MIGRATION;
+ if (ioctl(vbasedev->fd, VFIO_DEVICE_FEATURE, feature)) {
+ return -EOPNOTSUPP;
+ }
+
+ *mig_flags = mig->flags;
+
+ return 0;
+}
The code is using any possible error returned by the VFIO_DEVICE_FEATURE
ioctl to distinguish protocol v1 from v2.
Couldn't we use ENOTTY ? I think a pre-v6.0 kernel would return this errno.
Some error report would be good to have.
Thanks,
C.
