qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 1/1] vhost-user-fs: add migration type property


From: Anton Kuchin
Subject: Re: [PATCH v3 1/1] vhost-user-fs: add migration type property
Date: Fri, 24 Feb 2023 06:14:47 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.7.1

On 23/02/2023 23:24, Stefan Hajnoczi wrote:
On Thu, Feb 23, 2023 at 02:36:33AM -0500, Michael S. Tsirkin wrote:
On Wed, Feb 22, 2023 at 03:21:42PM -0500, Michael S. Tsirkin wrote:
On Wed, Feb 22, 2023 at 08:25:19PM +0200, Anton Kuchin wrote:
On 22/02/2023 19:12, Michael S. Tsirkin wrote:
On Wed, Feb 22, 2023 at 07:05:47PM +0200, Anton Kuchin wrote:
On 22/02/2023 18:51, Michael S. Tsirkin wrote:
On Wed, Feb 22, 2023 at 06:49:10PM +0200, Anton Kuchin wrote:
On 22/02/2023 17:14, Vladimir Sementsov-Ogievskiy wrote:
On 22.02.23 17:25, Anton Kuchin wrote:
+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?
In fact, I would claim we only want pre_load.
When qemu is started on destination we know where it's migrated
from so this flag can be set.
When qemu is started on source we generally do not yet know so
we don't know whether it's safe to set this flag.
But destination is a "source" for next migration, so there shouldn't be
real difference.
The new property has ".realized_set_allowed = true", so, as I understand
it may be changed at any time, so that's not a problem.
Yes, exactly. So destination's property sets not how it will handle this
incoming
migration but the future outgoing one.
How do you know where you are going to migrate though?
I think you don't.
Setting it on source is better since we know where we
are migrating from.
Yes, I don't know where I'm going to migrate to. This is why property
affects only how source saves state on outgoing migration.
Um. I don't get the logic.
For this feature to work we need orchestrator to manage the migration. And
we
generally assume that it is responsibility of orchestrator to ensure
matching
properties on source and destination.
As orchestrator manages both sides of migration it can set option (and we
can
check it) on either source or destination. Now it's not important which side
we
select, because now the option is essentially binary allow/deny (but IMHO it
is much better to refuse source to migrate than find later that state can't
be
loaded by destination, in case of file migration this becomes especially
painful).

But there are plans to add internal migration option (extract FUSE state
from
backend and transfer it in QEMU migration stream), and that's where
setting/checking
on source becomes important because it will rely on this property to decide
if
extra state form backend needs to be put in the migration stream subsection.

If we do internal migration that will be a different property
which has to match on source *and* destination.


If you are concerned about orchestrator breaking assumption of matching
properties
on source and destination this is not really supported AFAIK but I don't
think we
need to punish it for this, maybe it has its reasons: I can imagine scenario
where orchestrator could want to migrate from source with
'migration=external'
to destination with 'migration=none' to ensure that destination can't be
migrated further.
No. I am concerned about a simple practical matter:
- I decide to restart qemu on the same host - so I need to enable
   migration
- Later I decide to migrate qemu to another host - this should be
   blocked


Property on source does not satisfy both at the same time.
Property on destination does.

Stefan what's your take on this? Should we move this from
save to load hook?
This property can be changed on the source at runtime via qom-set, so
you don't need to predict the future. The device can be started from an
incoming migration with "external" and then set to "stateful" migration
to migrate to another host later on.

Anton, can you share the virtiofsd patches so we have a full picture of
how "external" migration works? I'd like to understand the workflow and
also how it can be extended when "stateful" migration is added.

Unfortunately internal implementation is relying heavily on our infrastructure, and rust virtiofsd still lacks dirty tracking so it is not ready yet. But I did
have a PoC for deprecated now C virtiofsd that I didn't bother to prepare
for upstreaming because C version was declared unsupported. It essentially adds
reconnect and this was the only thing required from virtiofsd to support
migration via file.

If this helps I'll try to find patches or recreate then and will be happy to share.




This property selects if VM can migrate and if it can what should
qemu put
to the migration stream. So we select on source what type of
migration is
allowed for this VM, destination can't check anything at load time.
OK, so the new field "migration" regulates only outgoing migration and
do nothing for incoming. On incoming migration the migration stream
itself defines the type of device migration.
Worth mentioning in doc?
Good point. I don't think this deserves a respin but if I have to send v4
I'll include
clarification in it.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]