On 28.02.2025 11:05, Cédric Le Goater wrote:
On 2/27/25 23:01, Maciej S. Szmigiero wrote:
On 27.02.2025 07:59, Cédric Le Goater wrote:
On 2/19/25 21:34, Maciej S. Szmigiero wrote:
From: "Maciej S. Szmigiero" <maciej.szmigi...@oracle.com>
Update the VFIO documentation at docs/devel/migration describing the
changes brought by the multifd device state transfer.
Signed-off-by: Maciej S. Szmigiero <maciej.szmigi...@oracle.com>
---
docs/devel/migration/vfio.rst | 80 +++++++++++++++++++++++++++++++----
1 file changed, 71 insertions(+), 9 deletions(-)
diff --git a/docs/devel/migration/vfio.rst b/docs/devel/migration/vfio.rst
index c49482eab66d..d9b169d29921 100644
--- a/docs/devel/migration/vfio.rst
+++ b/docs/devel/migration/vfio.rst
@@ -16,6 +16,37 @@ helps to reduce the total downtime of the VM. VFIO devices
opt-in to pre-copy
support by reporting the VFIO_MIGRATION_PRE_COPY flag in the
VFIO_DEVICE_FEATURE_MIGRATION ioctl.
Please add a new "multifd" documentation subsection at the end of the file
with this part :
+Starting from QEMU version 10.0 there's a possibility to transfer VFIO device
+_STOP_COPY state via multifd channels. This helps reduce downtime - especially
+with multiple VFIO devices or with devices having a large migration state.
+As an additional benefit, setting the VFIO device to _STOP_COPY state and
+saving its config space is also parallelized (run in a separate thread) in
+such migration mode.
+
+The multifd VFIO device state transfer is controlled by
+"x-migration-multifd-transfer" VFIO device property. This property defaults to
+AUTO, which means that VFIO device state transfer via multifd channels is
+attempted in configurations that otherwise support it.
+
Done - I also moved the parts about x-migration-max-queued-buffers
and x-migration-load-config-after-iter description there since
obviously they wouldn't make sense being left alone in the top section.
I was expecting a much more detailed explanation on the design too :
* in the cover letter
* in the hw/vfio/migration-multifd.c
* in some new file under docs/devel/migration/
I forgot to add :
* guide on how to use this new feature from QEMU and libvirt.
something we can refer to for tests. That's a must have.
So basically a user's guide.
That's something I plan to write after the code is ready.
* usage scenarios
There are some benefits but it is not obvious a user would
like to use multiple VFs in one VM, please explain.
Hmm, this patch set does not bring ability to use multiple VFs
in a single VM - that ability is already in QEMU even without this
patch set.
As Yanghang has measured the downtime improvement happens even
with a single VF, although with more VFs one can additionality see
the scalability benefits of this patch set.
This is a major addition which needs justification anyhow
* pros and cons
The biggest advantage is obviously the downtime performance.
I'm not sure if there are any obvious disadvantages (assuming
the setup supports the multifd migration in the first place),
besides maybe slightly bigger memory usage for in-flight buffers?
But we have an option for capping that if someone is concerned
about it.
I'm not sure what descriptions you exactly want in these places,
Looking from the VFIO subsystem, the way this series works is very opaque.
There are a couple of a new migration handlers,
I've added descriptions of these 3 new migration handlers to
docs/devel/migration/vfio.rst.
They are also described in struct SaveVMHandlers in include/migration/register.h
and also in the commit messages that introduce them.
new threads,
A total of two of these, their function is described in
docs/devel/migration/vfio.rst
and also in the commit messages that introduce them.
new channels,
I think you meant "new data type for multifd channel" here but that's
in migration core, not VFIO.
etc. It has been discussed several times with migration folks, please provide
a summary for a new reader as ignorant as everyone would be when looking at
a new file.
I can certainly include all these in the new version cover letter if that's
easier for a new reader.
but since
that's just documentation (not code) it could be added after the code freeze...
That's the risk of not getting any ! and the initial proposal should be
discussed before code freeze.
For the general framework, I was expecting an extension of a "multifd"
subsection under :
https://qemu.readthedocs.io/en/v9.2.0/devel/migration/features.html
but it doesn't exist :/
Looking at the source file for this page at docs/devel/migration/features.rst
the "multifd" section should appear on this page automatically after
I added it to docs/devel/migration/vfio.rst.
So, for now, let's use the new "multifd" subsection of
https://qemu.readthedocs.io/en/v9.2.0/devel/migration/vfio.html
Okay.
This section :
+Since the target QEMU needs to load device state buffers in-order it needs to
+queue incoming buffers until they can be loaded into the device.
+This means that a malicious QEMU source could theoretically cause the target
+QEMU to allocate unlimited amounts of memory for such buffers-in-flight.
+
+The "x-migration-max-queued-buffers" property allows capping the maximum count
+of these VFIO device state buffers queued at the destination.
+
+Because a malicious QEMU source causing OOM on the target is not expected to be
+a realistic threat in most of VFIO live migration use cases and the right value
+depends on the particular setup by default this queued buffers limit is
+disabled by setting it to UINT64_MAX.
should be in patch 34. It is not obvious it will be merged.
...which brings us to this point.
I think by this point in time (less then 2 weeks to code freeze) we should
finally decide what is going to be included in the patch set.
> This way this patch set could be well tested in its final form rather than
having significant parts taken out of it at the eleventh hour.
If the final form is known also the documentation can be adjusted accordingly
and user/admin documentation eventually written once the code is considered
okay.
I though we discussed a few times the rationale behind both
x-migration-max-queued-buffers and x-migration-load-config-after-iter properties
but if you still have some concerns there please let me know before I prepare
the next version of this patch set so I know whether to include these.
Patch 34, not sure yet.
Patch 35 is for next cycle IMO.
For QEMU 10.0, let's focus on x86 first and see how it goes. We can add
ARM support in QEMU 10.1 if nothing new arises. We will need the virt-arm
folks in cc: then.
Please keep patch 35 in v6 nevertheless, it is good for reference if
someone wants to apply on an out of tree QEMU.
If we are to drop/skip adding the "x-migration-load-config-after-iter"
option for now then let's do it now so the next version could be already
tested in its target shape.
After this "x-migration-load-config-after-iter" option is proposed
once again in QEMU 10.1 cycle then it obviously will be forward ported
to whatever the code looks at that point and tested again.
The patch itself is not going to suddenly disappear :) - it's on the
mailing list and in my repository here:
https://gitlab.com/maciejsszmigiero/qemu/-/commit/6582ac5ac338c40ad74ec60820e85b06c4509a2a
Thanks,
C.
Thanks,
Maciej