[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 1/1] vhost-user-fs: add property to allow migration
From: |
Juan Quintela |
Subject: |
Re: [PATCH v2 1/1] vhost-user-fs: add property to allow migration |
Date: |
Thu, 16 Feb 2023 15:14:05 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) |
Anton Kuchin <antonkuchin@yandex-team.ru> wrote:
> Now any vhost-user-fs device makes VM unmigratable, that also prevents
> qemu update without stopping the VM. In most cases that makes sense
> because qemu has no way to transfer FUSE session state.
>
> But it is good to have an option for orchestrator to tune this according to
> backend capabilities and migration configuration.
>
> This patch adds device property 'migration' that is 'none' by default
> to keep old behaviour but can be set to 'external' to explicitly allow
> migration with minimal virtio device state in migration stream if daemon
> has some way to sync FUSE state on src and dst without help from qemu.
>
> Signed-off-by: Anton Kuchin <antonkuchin@yandex-team.ru>
Reviewed-by: Juan Quintela <quintela@redhat.com>
The migration bits are correct.
And I can think a better way to explain that one device is migrated
externally.
If you have to respin:
> +static int vhost_user_fs_pre_save(void *opaque)
> +{
> + VHostUserFS *fs = (VHostUserFS *)opaque;
This hack is useless.
I know that there are still lots of code that still have it.
Now remember that I have no clue about vhost-user-fs.
But this looks fishy
> static const VMStateDescription vuf_vmstate = {
> .name = "vhost-user-fs",
> - .unmigratable = 1,
> + .minimum_version_id = 0,
> + .version_id = 0,
> + .fields = (VMStateField[]) {
> + VMSTATE_VIRTIO_DEVICE,
> + VMSTATE_UINT8(migration_type, VHostUserFS),
> + VMSTATE_END_OF_LIST()
> + },
> + .pre_save = vhost_user_fs_pre_save,
> };
>
> static Property vuf_properties[] = {
> @@ -309,6 +337,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,
> + uint8_t),
> DEFINE_PROP_END_OF_LIST(),
We have four properties here (5 with the new migration one), and you
only migrate one.
This looks fishy, but I don't know if it makes sense.
If they _have_ to be configured the same on source and destination, I
would transfer them and check in post_load that the values are correct.
Later, Juan.
- [PATCH v2 0/1] virtio-fs: implement option for stateless migration., Anton Kuchin, 2023/02/16
- [PATCH v2 1/1] vhost-user-fs: add property to allow migration, Anton Kuchin, 2023/02/16
- Re: [PATCH v2 1/1] vhost-user-fs: add property to allow migration,
Juan Quintela <=
- Re: [PATCH v2 1/1] vhost-user-fs: add property to allow migration, Michael S. Tsirkin, 2023/02/16
- Re: [PATCH v2 1/1] vhost-user-fs: add property to allow migration, Michael S. Tsirkin, 2023/02/16
- Re: [PATCH v2 1/1] vhost-user-fs: add property to allow migration, Juan Quintela, 2023/02/16
- Re: [PATCH v2 1/1] vhost-user-fs: add property to allow migration, Anton Kuchin, 2023/02/16
- Re: [PATCH v2 1/1] vhost-user-fs: add property to allow migration, Anton Kuchin, 2023/02/16
- Re: [PATCH v2 1/1] vhost-user-fs: add property to allow migration, Juan Quintela, 2023/02/16
Re: [PATCH v2 1/1] vhost-user-fs: add property to allow migration, Stefan Hajnoczi, 2023/02/16