[Virtio-fs] [PATCH v3 1/1] vhost-user-fs: add migration type property
Anton Kuchin
antonkuchin at yandex-team.ru
Wed Feb 22 14:21:01 UTC 2023
On 22/02/2023 14:20, Vladimir Sementsov-Ogievskiy wrote:
> On 17.02.23 20:00, Anton Kuchin wrote:
>> Migration of vhost-user-fs device requires transfer of FUSE internal
>> state
>> from backend. There is no standard way to do it now so by default
>> migration
>> must be blocked. But if this state can be externally transferred by
>> orchestrator give it an option to explicitly allow migration.
>>
>> Signed-off-by: Anton Kuchin <antonkuchin at yandex-team.ru>
>> ---
>> hw/core/qdev-properties-system.c | 10 +++++++++
>> hw/virtio/vhost-user-fs.c | 32 ++++++++++++++++++++++++++++-
>> include/hw/qdev-properties-system.h | 1 +
>> include/hw/virtio/vhost-user-fs.h | 2 ++
>> qapi/migration.json | 16 +++++++++++++++
>> 5 files changed, 60 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/core/qdev-properties-system.c
>> b/hw/core/qdev-properties-system.c
>> index d42493f630..d9b1aa2a5d 100644
>> --- a/hw/core/qdev-properties-system.c
>> +++ b/hw/core/qdev-properties-system.c
>> @@ -1143,3 +1143,13 @@ const PropertyInfo qdev_prop_uuid = {
>> .set = set_uuid,
>> .set_default_value = set_default_uuid_auto,
>> };
>> +
>> +const PropertyInfo qdev_prop_vhost_user_migration_type = {
>> + .name = "VhostUserMigrationType",
>> + .description = "none/external",
>> + .enum_table = &VhostUserMigrationType_lookup,
>> + .get = qdev_propinfo_get_enum,
>> + .set = qdev_propinfo_set_enum,
>> + .set_default_value = qdev_propinfo_set_default_value_enum,
>> + .realized_set_allowed = true,
>> +};
>> diff --git a/hw/virtio/vhost-user-fs.c b/hw/virtio/vhost-user-fs.c
>> index 83fc20e49e..7deb9df5ec 100644
>> --- a/hw/virtio/vhost-user-fs.c
>> +++ b/hw/virtio/vhost-user-fs.c
>> @@ -298,9 +298,35 @@ static struct vhost_dev
>> *vuf_get_vhost(VirtIODevice *vdev)
>> return &fs->vhost_dev;
>> }
>> +static int vhost_user_fs_pre_save(void *opaque)
>> +{
>> + VHostUserFS *fs = opaque;
>> + g_autofree char *path = object_get_canonical_path(OBJECT(fs));
>> +
>> + switch (fs->migration_type) {
>> + case VHOST_USER_MIGRATION_TYPE_NONE:
>> + error_report("Migration is blocked by device %s", path);
>> + break;
>> + case VHOST_USER_MIGRATION_TYPE_EXTERNAL:
>> + return 0;
>> + default:
>> + error_report("Migration type '%s' is not supported by device
>> %s",
>> + VhostUserMigrationType_str(fs->migration_type), path);
>> + break;
>> + }
>> +
>> + return -1;
>> +}
>
> Should we also add this as .pre_load, to force user select correct
> migration_type on target too?
Why do we need it? Enum forces user to select at least one of the sane
option
and I believe this is enough. As this property affects only save and
don't know
what we can do at load.
>
>> +
>> static const VMStateDescription vuf_vmstate = {
>> .name = "vhost-user-fs",
>> - .unmigratable = 1,
>> + .minimum_version_id = 0,
>> + .version_id = 0,
>> + .fields = (VMStateField[]) {
>> + VMSTATE_VIRTIO_DEVICE,
>> + VMSTATE_END_OF_LIST()
>> + },
>> + .pre_save = vhost_user_fs_pre_save,
>> };
>> static Property vuf_properties[] = {
>> @@ -309,6 +335,10 @@ static Property vuf_properties[] = {
>> DEFINE_PROP_UINT16("num-request-queues", VHostUserFS,
>> conf.num_request_queues, 1),
>> DEFINE_PROP_UINT16("queue-size", VHostUserFS, conf.queue_size,
>> 128),
>> + DEFINE_PROP_UNSIGNED("migration", VHostUserFS, migration_type,
>> + VHOST_USER_MIGRATION_TYPE_NONE,
>> + qdev_prop_vhost_user_migration_type,
>> + VhostUserMigrationType),
>
> 1. I see, other similar qdev_prop_* use DEFINE_PROP_SIGNED
I don't think this should be signed. Enum values are non-negative so
compilers
(at least gcc and clang that I checked) evaluate underlying enum type to
be unsigned int.
I don't know why other property types use signed, may be they have
reasons or just this
is how they were initially implemented.
> 2. All of them except only qdev_prop_fdc_drive_type, define also a
> convenient macro in include/hw/qdev-properties-system.h
This makes sense if property is used in more than one place, in this
case I don't see any
benefit from writing more code to handle this specific case. Maybe if
property finds its
usage in other devices this can be done.
>
> should we follow these patterns?
>
>
More information about the Virtio-fs
mailing list