All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Peter Xu <peterx@redhat.com>
Cc: Fabiano Rosas <farosas@suse.de>,
	qemu-devel@nongnu.org, armbru@redhat.com,
	Leonardo Bras <leobras@redhat.com>,
	Claudio Fontana <cfontana@suse.de>
Subject: Re: [RFC PATCH v3 00/30] migration: File based migration with multifd and fixed-ram
Date: Mon, 15 Jan 2024 08:11:40 +0000	[thread overview]
Message-ID: <ZaTog6-cNMNqyyFG@redhat.com> (raw)
In-Reply-To: <ZaTPNwFcfrM-JUlg@x1n>

On Mon, Jan 15, 2024 at 02:22:47PM +0800, Peter Xu wrote:
> On Thu, Jan 11, 2024 at 03:38:31PM -0300, Fabiano Rosas wrote:
> > Peter Xu <peterx@redhat.com> writes:
> > 
> > > On Mon, Nov 27, 2023 at 05:25:42PM -0300, Fabiano Rosas wrote:
> > >> Hi,
> > >> 
> > >> In this v3:
> > >> 
> > >> Added support for the "file:/dev/fdset/" syntax to receive multiple
> > >> file descriptors. This allows the management layer to open the
> > >> migration file beforehand and pass the file descriptors to QEMU. We
> > >> need more than one fd to be able to use O_DIRECT concurrently with
> > >> unaligned writes.
> > >> 
> > >> Dropped the auto-pause capability. That discussion was kind of
> > >> stuck. We can revisit optimizations for non-live scenarios once the
> > >> series is more mature/merged.
> > >> 
> > >> Changed the multifd incoming side to use a more generic data structure
> > >> instead of MultiFDPages_t. This allows multifd to restore the ram
> > >> using larger chunks.
> > >> 
> > >> The rest are minor changes, I have noted them in the patches
> > >> themselves.
> > >
> > > Fabiano,
> > >
> > > Could you always keep a section around in the cover letter (and also in the
> > > upcoming doc file fixed-ram.rst) on the benefits of this feature?
> > >
> > > Please bare with me - I can start to ask silly questions.
> > >
> > 
> > That's fine. Ask away!
> > 
> > > I thought it was about "keeping the snapshot file small".  But then when I
> > > was thinking the use case, iiuc fixed-ram migration should always suggest
> > > the user to stop the VM first before migration starts, then if the VM is
> > > stopped the ultimate image shouldn't be large either.
> > >
> > > Or is it about performance only?  Where did I miss?
> > 
> > Performance is the main benefit because fixed-ram enables the use of
> > multifd for file migration which would otherwise not be
> > parallelizable. To use multifd has been the direction for a while as you
> > know, so it makes sense.
> > 
> > A fast file migration is desirable because it could be used for
> > snapshots with a stopped vm and also to replace the "exec:cat" hack
> > (this last one I found out about recently, Juan mentioned it in this
> > thread: https://lore.kernel.org/r/87cyx5ty26.fsf@secure.mitica).
> 
> I digged again the history, and started to remember the "live" migration
> case for fixed-ram. IIUC that is what Dan mentioned in below email
> regarding to the "virDomainSnapshotXXX" use case:
> 
> https://lore.kernel.org/all/ZD7MRGQ+4QsDBtKR@redhat.com/
> 
> So IIUC "stopped VM" is not always the use case?
> 
> If you agree with this, we need to document these two use cases clearly in
> the doc update:
> 
>   - "Migrate a VM to file, then destroy the VM"
> 
>     It should be suggested to stop the VM first before triggering such
>     migration in this use case in the documents.
> 
>   - "Take a live snapshot of the VM"
> 
>     It'll be ideal if there is a portable interface to synchronously track
>     dirtying of guest pages, but we don't...
> 
>     So fixed-ram seems to be the solution for such a portable solution for
>     taking live snapshot across-platforms as long as async dirty tracking
>     is still supported on that OS (aka KVM_GET_DIRTY_LOG).  If async
>     tracking is not supported, snapshot cannot be done live on the OS then,
>     and one needs to use "snapshot-save".
> 
>     For this one, IMHO it would be good to mention (from QEMU perspective)
>     the existance of background-snapshot even though libvirt didn't support
>     it for some reason.  Currently background-snapshot lacks multi-thread
>     feature (nor O_DIRECT), though, so it may be less performant than
>     fixed-ram.  However if with all features there I believe that's even
>     more performant.  Please consider mention to a degree of detail on
>     this.
> 
> > 
> > The size aspect is just an interesting property, not necessarily a
> > reason.
> 
> See above on the 2nd "live" use case of fixed-ram. I think in that case,
> size is still a matter, then, because that one cannot stop the VM vcpus.
> 
> > It's about having the file bounded to the RAM size. So a running
> > guest would not produce a continuously growing file. This is in contrast
> > with previous experiments (libvirt code) in using a proxy to put
> > multifd-produced data into a file.
> > 
> > I'll add this^ information in a more organized matter to the docs and
> > cover letter. Let me know what else I need to clarify.
> 
> Thanks.
> 
> > 
> > Some notes about fixed-ram by itself:
> > 
> > This series also enables fixed-ram without multifd, which would only
> > take benefit of the size property. That is not part of our end goal
> > which is to have multifd + fixed-ram, but I kept it nonetheless because
> > it helps to debug/reason about the fixed-ram format without conflating
> > matters with multifd.
> 
> Yes, makes sense.
> 
> > 
> > Fixed-ram without multifd also allows the file migration to take benefit
> > of direct io because the data portion of the file (pages) will be
> > written with alignment. This version of the series does not yet support
> > it, but I have a simple patch for the next version.
> > 
> > I also had a - perhaps naive - idea that we could merge the io code +
> > fixed-ram first, to expedite things and later bring in the multifd and
> > directio enhancements, but the review process ended up not being that
> > modular.
> 
> What's the review process issue you're talking about?
> 
> If you can split the series that'll help merging for sure to me.  IIRC
> there's complexity on passing the o-direct fds around, and not sure whether
> that chunk can be put at the last, similarly to split the multifd bits.
> 
> One thing I just noticed is fixed-ram seems to be always preferred for
> "file:" migrations.  Then can we already imply fixed-ram for "file" URIs?
> 
> I'm even thinking whether we can make it the default and drop the fixed-ram
> capability: fixed-ram won't work besides file, and file won't make sense if
> not using offsets / fixed-ram.  There's at least one problem, where we have
> released 8.2 with "file:", so it means it could break users already using
> "file:" there.  I'm wondering whether that'll be worthwhile considering if
> we can drop the (seems redundant..) capability.  What do you think?

The 'fd' protocol should support 'fixed-ram' too if passed a seekable
FD.

The 'file' protocol should be able to create save images compatible with
older QEMU too IMHO.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



  reply	other threads:[~2024-01-15  8:12 UTC|newest]

Thread overview: 95+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-27 20:25 [RFC PATCH v3 00/30] migration: File based migration with multifd and fixed-ram Fabiano Rosas
2023-11-27 20:25 ` [RFC PATCH v3 01/30] io: add and implement QIO_CHANNEL_FEATURE_SEEKABLE for channel file Fabiano Rosas
2024-01-10  8:49   ` Peter Xu
2023-11-27 20:25 ` [RFC PATCH v3 02/30] io: Add generic pwritev/preadv interface Fabiano Rosas
2024-01-10  9:07   ` Daniel P. Berrangé
2024-01-11  6:59   ` Peter Xu
2023-11-27 20:25 ` [RFC PATCH v3 03/30] io: implement io_pwritev/preadv for QIOChannelFile Fabiano Rosas
2024-01-10  9:08   ` Daniel P. Berrangé
2024-01-11  7:04   ` Peter Xu
2023-11-27 20:25 ` [RFC PATCH v3 04/30] io: fsync before closing a file channel Fabiano Rosas
2024-01-10  9:04   ` Daniel P. Berrangé
2024-01-11  8:44   ` Peter Xu
2024-01-11 18:46     ` Fabiano Rosas
2024-01-12  0:01       ` Peter Xu
2024-01-12 10:40         ` Daniel P. Berrangé
2024-01-15  3:38           ` Peter Xu
2024-01-15  8:57       ` Peter Xu
2024-01-15  9:03         ` Daniel P. Berrangé
2024-01-15  9:31           ` Peter Xu
2023-11-27 20:25 ` [RFC PATCH v3 05/30] migration/qemu-file: add utility methods for working with seekable channels Fabiano Rosas
2024-01-11  9:57   ` Peter Xu
2024-01-11 18:49     ` Fabiano Rosas
2023-11-27 20:25 ` [RFC PATCH v3 06/30] migration/ram: Introduce 'fixed-ram' migration capability Fabiano Rosas
2023-12-22 10:35   ` Markus Armbruster
2024-01-11 10:43   ` Peter Xu
2023-11-27 20:25 ` [RFC PATCH v3 07/30] migration: Add fixed-ram URI compatibility check Fabiano Rosas
2024-01-15  9:01   ` Peter Xu
2024-01-23 19:07     ` Fabiano Rosas
2024-01-23 19:07     ` Fabiano Rosas
2023-11-27 20:25 ` [RFC PATCH v3 08/30] migration/ram: Add outgoing 'fixed-ram' migration Fabiano Rosas
2024-01-15  9:28   ` Peter Xu
2024-01-15 14:50     ` Fabiano Rosas
2023-11-27 20:25 ` [RFC PATCH v3 09/30] migration/ram: Add incoming " Fabiano Rosas
2024-01-15  9:49   ` Peter Xu
2024-01-15 16:43     ` Fabiano Rosas
2023-11-27 20:25 ` [RFC PATCH v3 10/30] tests/qtest: migration-test: Add tests for fixed-ram file-based migration Fabiano Rosas
2024-01-15 10:01   ` Peter Xu
2023-11-27 20:25 ` [RFC PATCH v3 11/30] migration/multifd: Allow multifd without packets Fabiano Rosas
2024-01-15 11:51   ` Peter Xu
2024-01-15 18:39     ` Fabiano Rosas
2024-01-15 23:01       ` Peter Xu
2023-11-27 20:25 ` [RFC PATCH v3 12/30] migration/multifd: Allow QIOTask error reporting without an object Fabiano Rosas
2024-01-15 12:06   ` Peter Xu
2023-11-27 20:25 ` [RFC PATCH v3 13/30] migration/multifd: Add outgoing QIOChannelFile support Fabiano Rosas
2024-01-16  4:05   ` Peter Xu
2024-01-16  7:25     ` Peter Xu
2024-01-16 13:37     ` Fabiano Rosas
2024-01-17  8:28       ` Peter Xu
2024-01-17 17:34         ` Fabiano Rosas
2024-01-18  7:11           ` Peter Xu
2023-11-27 20:25 ` [RFC PATCH v3 14/30] migration/multifd: Add incoming " Fabiano Rosas
2024-01-16  6:29   ` Peter Xu
2023-11-27 20:25 ` [RFC PATCH v3 15/30] io: Add a pwritev/preadv version that takes a discontiguous iovec Fabiano Rosas
2024-01-16  6:58   ` Peter Xu
2024-01-16 18:15     ` Fabiano Rosas
2024-01-17  9:48       ` Peter Xu
2024-01-17 18:06         ` Fabiano Rosas
2024-01-18  7:44           ` Peter Xu
2024-01-18 12:47             ` Fabiano Rosas
2024-01-19  0:22               ` Peter Xu
2024-01-17 12:39   ` Daniel P. Berrangé
2024-01-17 14:27     ` Daniel P. Berrangé
2024-01-17 18:09       ` Fabiano Rosas
2023-11-27 20:25 ` [RFC PATCH v3 16/30] multifd: Rename MultiFDSendParams::data to compress_data Fabiano Rosas
2024-01-16  7:03   ` Peter Xu
2023-11-27 20:25 ` [RFC PATCH v3 17/30] migration/multifd: Decouple recv method from pages Fabiano Rosas
2024-01-16  7:23   ` Peter Xu
2023-11-27 20:26 ` [RFC PATCH v3 18/30] migration/multifd: Allow receiving pages without packets Fabiano Rosas
2024-01-16  8:10   ` Peter Xu
2024-01-16 20:25     ` Fabiano Rosas
2024-01-19  0:20       ` Peter Xu
2024-01-19 12:57         ` Fabiano Rosas
2023-11-27 20:26 ` [RFC PATCH v3 19/30] migration/ram: Ignore multifd flush when doing fixed-ram migration Fabiano Rosas
2024-01-16  8:23   ` Peter Xu
2024-01-17 18:13     ` Fabiano Rosas
2024-01-19  1:33       ` Peter Xu
2023-11-27 20:26 ` [RFC PATCH v3 20/30] migration/multifd: Support outgoing fixed-ram stream format Fabiano Rosas
2023-11-27 20:26 ` [RFC PATCH v3 21/30] migration/multifd: Support incoming " Fabiano Rosas
2023-11-27 20:26 ` [RFC PATCH v3 22/30] tests/qtest: Add a multifd + fixed-ram migration test Fabiano Rosas
2023-11-27 20:26 ` [RFC PATCH v3 23/30] migration: Add direct-io parameter Fabiano Rosas
2023-12-22 10:38   ` Markus Armbruster
2023-11-27 20:26 ` [RFC PATCH v3 24/30] tests/qtest: Add a test for migration with direct-io and multifd Fabiano Rosas
2023-11-27 20:26 ` [RFC PATCH v3 25/30] monitor: Honor QMP request for fd removal immediately Fabiano Rosas
2023-11-27 20:26 ` [RFC PATCH v3 26/30] monitor: Extract fdset fd flags comparison into a function Fabiano Rosas
2023-11-27 20:26 ` [RFC PATCH v3 27/30] monitor: fdset: Match against O_DIRECT Fabiano Rosas
2023-11-27 20:26 ` [RFC PATCH v3 28/30] docs/devel/migration.rst: Document the file transport Fabiano Rosas
2023-11-27 20:26 ` [RFC PATCH v3 29/30] migration: Add support for fdset with multifd + file Fabiano Rosas
2023-11-27 20:26 ` [RFC PATCH v3 30/30] tests/qtest: Add a test for fixed-ram with passing of fds Fabiano Rosas
2024-01-11 10:50 ` [RFC PATCH v3 00/30] migration: File based migration with multifd and fixed-ram Peter Xu
2024-01-11 18:38   ` Fabiano Rosas
2024-01-15  6:22     ` Peter Xu
2024-01-15  8:11       ` Daniel P. Berrangé [this message]
2024-01-15  8:41         ` Peter Xu
2024-01-15 19:45       ` Fabiano Rosas
2024-01-15 23:20         ` Peter Xu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZaTog6-cNMNqyyFG@redhat.com \
    --to=berrange@redhat.com \
    --cc=armbru@redhat.com \
    --cc=cfontana@suse.de \
    --cc=farosas@suse.de \
    --cc=leobras@redhat.com \
    --cc=peterx@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.