| Message ID | 1686163139-256654-1-git-send-email-steven.sistare@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [V2] migration: file URI | expand |
Hi, Steve, On Wed, Jun 07, 2023 at 11:38:59AM -0700, Steve Sistare wrote: > Extend the migration URI to support file:<filename>. This can be used for > any migration scenario that does not require a reverse path. It can be used > as an alternative to 'exec:cat > file' in minimized containers that do not > contain /bin/sh, and it is easier to use than the fd:<fdname> URI. It can > be used in HMP commands, and as a qemu command-line parameter. I have similar question on the fixed-ram work, on whether we should assume the vm stopped before doing so. Again, it leaves us space for optimizations on top without breaking anyone. The other thing is considering a very busy guest, migration may not even converge for "file:" URI (the same to other URIs) but I think that doesn't make much sense to not converge for a "file:" URI. The user might be very confused too.
On 6/12/2023 2:44 PM, Peter Xu wrote: > Hi, Steve, > > On Wed, Jun 07, 2023 at 11:38:59AM -0700, Steve Sistare wrote: >> Extend the migration URI to support file:<filename>. This can be used for >> any migration scenario that does not require a reverse path. It can be used >> as an alternative to 'exec:cat > file' in minimized containers that do not >> contain /bin/sh, and it is easier to use than the fd:<fdname> URI. It can >> be used in HMP commands, and as a qemu command-line parameter. > > I have similar question on the fixed-ram work, Sorry, what is the "fixed-ram work"? > on whether we should assume > the vm stopped before doing so. Again, it leaves us space for > optimizations on top without breaking anyone. I do not assume the vm is stopped. The migration code will stop the vm in migration_iteration_finish. > The other thing is considering a very busy guest, migration may not even > converge for "file:" URI (the same to other URIs) but I think that doesn't > make much sense to not converge for a "file:" URI. The user might be very > confused too. The file URI is mainly intended for the case where guest ram is backed by shared memory and preserved in place, in which case writes are not tracked and convergence is not an issue. If not shared memory, the user should be advised to stop the machine first. I should document these notes in qemu-options and/or migration.json. - Steve
On Mon, Jun 12, 2023 at 03:39:34PM -0400, Steven Sistare wrote: > On 6/12/2023 2:44 PM, Peter Xu wrote: > > Hi, Steve, > > > > On Wed, Jun 07, 2023 at 11:38:59AM -0700, Steve Sistare wrote: > >> Extend the migration URI to support file:<filename>. This can be used for > >> any migration scenario that does not require a reverse path. It can be used > >> as an alternative to 'exec:cat > file' in minimized containers that do not > >> contain /bin/sh, and it is easier to use than the fd:<fdname> URI. It can > >> be used in HMP commands, and as a qemu command-line parameter. > > > > I have similar question on the fixed-ram work, > > Sorry, what is the "fixed-ram work"? https://lore.kernel.org/all/20230330180336.2791-1-farosas@suse.de It has similar requirement to migrate to a file, though slightly different use case. > > > on whether we should assume > > the vm stopped before doing so. Again, it leaves us space for > > optimizations on top without breaking anyone. > > I do not assume the vm is stopped. The migration code will stop the vm > in migration_iteration_finish. > > > The other thing is considering a very busy guest, migration may not even > > converge for "file:" URI (the same to other URIs) but I think that doesn't > > make much sense to not converge for a "file:" URI. The user might be very > > confused too. > > The file URI is mainly intended for the case where guest ram is backed by shared memory > and preserved in place, in which case writes are not tracked and convergence is not an > issue. If not shared memory, the user should be advised to stop the machine first. > I should document these notes in qemu-options and/or migration.json. My question was whether we should treat "file:" differently from most of other URIs. It makes the URI slightly tricky for sure, but it also does make sense to me because "file:" implies more than the rest URIs, where we're sure about the consequence of the migration (vm stops), in that case keeping vm live makes it less performant, and also weird. It doesn't need to be special in memory type being shared, e.g. what if there's a device that contains a lot of data to migrate in the future? Assuming "shared memory will always migrate very fast" may not hold true. Do you think it makes more sense to just always stop VM when migrating to file URI? Then if someone tries to restart the VM or cancel the migration, we always do both (cancel migration, then start VM).
Peter Xu <peterx@redhat.com> writes: > On Mon, Jun 12, 2023 at 03:39:34PM -0400, Steven Sistare wrote: >> On 6/12/2023 2:44 PM, Peter Xu wrote: >> > Hi, Steve, >> > >> > On Wed, Jun 07, 2023 at 11:38:59AM -0700, Steve Sistare wrote: >> >> Extend the migration URI to support file:<filename>. This can be used for >> >> any migration scenario that does not require a reverse path. It can be used >> >> as an alternative to 'exec:cat > file' in minimized containers that do not >> >> contain /bin/sh, and it is easier to use than the fd:<fdname> URI. It can >> >> be used in HMP commands, and as a qemu command-line parameter. >> > >> > I have similar question on the fixed-ram work, >> >> Sorry, what is the "fixed-ram work"? > > https://lore.kernel.org/all/20230330180336.2791-1-farosas@suse.de > > It has similar requirement to migrate to a file, though slightly different > use case. > >> >> > on whether we should assume >> > the vm stopped before doing so. Again, it leaves us space for >> > optimizations on top without breaking anyone. >> >> I do not assume the vm is stopped. The migration code will stop the vm >> in migration_iteration_finish. >> >> > The other thing is considering a very busy guest, migration may not even >> > converge for "file:" URI (the same to other URIs) but I think that doesn't >> > make much sense to not converge for a "file:" URI. The user might be very >> > confused too. >> >> The file URI is mainly intended for the case where guest ram is backed by shared memory >> and preserved in place, in which case writes are not tracked and convergence is not an >> issue. If not shared memory, the user should be advised to stop the machine first. >> I should document these notes in qemu-options and/or migration.json. > > My question was whether we should treat "file:" differently from most of > other URIs. It makes the URI slightly tricky for sure, but it also does > make sense to me because "file:" implies more than the rest URIs, where > we're sure about the consequence of the migration (vm stops), in that case > keeping vm live makes it less performant, and also weird. > > It doesn't need to be special in memory type being shared, e.g. what if > there's a device that contains a lot of data to migrate in the future? > Assuming "shared memory will always migrate very fast" may not hold true. > > Do you think it makes more sense to just always stop VM when migrating to > file URI? Then if someone tries to restart the VM or cancel the migration, > we always do both (cancel migration, then start VM). From our discussions in the other thread, I have implemented a MIGRATION_CAPABILITY_SUSPEND to allow the management layer to decide whether the guest should be stopped by QEMU before the migration. I'm not opposed to coupling file URI with a stopped VM, although I think, at least for fixed-ram, libvirt would prefer to be able to decide when to stop.
On Wed, Jun 14, 2023 at 12:47:41PM -0300, Fabiano Rosas wrote: > Peter Xu <peterx@redhat.com> writes: > > > On Mon, Jun 12, 2023 at 03:39:34PM -0400, Steven Sistare wrote: > >> On 6/12/2023 2:44 PM, Peter Xu wrote: > >> > Hi, Steve, > >> > > >> > On Wed, Jun 07, 2023 at 11:38:59AM -0700, Steve Sistare wrote: > >> >> Extend the migration URI to support file:<filename>. This can be used for > >> >> any migration scenario that does not require a reverse path. It can be used > >> >> as an alternative to 'exec:cat > file' in minimized containers that do not > >> >> contain /bin/sh, and it is easier to use than the fd:<fdname> URI. It can > >> >> be used in HMP commands, and as a qemu command-line parameter. > >> > > >> > I have similar question on the fixed-ram work, > >> > >> Sorry, what is the "fixed-ram work"? > > > > https://lore.kernel.org/all/20230330180336.2791-1-farosas@suse.de > > > > It has similar requirement to migrate to a file, though slightly different > > use case. > > > >> > >> > on whether we should assume > >> > the vm stopped before doing so. Again, it leaves us space for > >> > optimizations on top without breaking anyone. > >> > >> I do not assume the vm is stopped. The migration code will stop the vm > >> in migration_iteration_finish. > >> > >> > The other thing is considering a very busy guest, migration may not even > >> > converge for "file:" URI (the same to other URIs) but I think that doesn't > >> > make much sense to not converge for a "file:" URI. The user might be very > >> > confused too. > >> > >> The file URI is mainly intended for the case where guest ram is backed by shared memory > >> and preserved in place, in which case writes are not tracked and convergence is not an > >> issue. If not shared memory, the user should be advised to stop the machine first. > >> I should document these notes in qemu-options and/or migration.json. > > > > My question was whether we should treat "file:" differently from most of > > other URIs. It makes the URI slightly tricky for sure, but it also does > > make sense to me because "file:" implies more than the rest URIs, where > > we're sure about the consequence of the migration (vm stops), in that case > > keeping vm live makes it less performant, and also weird. > > > > It doesn't need to be special in memory type being shared, e.g. what if > > there's a device that contains a lot of data to migrate in the future? > > Assuming "shared memory will always migrate very fast" may not hold true. > > > > Do you think it makes more sense to just always stop VM when migrating to > > file URI? Then if someone tries to restart the VM or cancel the migration, > > we always do both (cancel migration, then start VM). > > From our discussions in the other thread, I have implemented a > MIGRATION_CAPABILITY_SUSPEND to allow the management layer to decide > whether the guest should be stopped by QEMU before the migration. > > I'm not opposed to coupling file URI with a stopped VM, although I > think, at least for fixed-ram, libvirt would prefer to be able to decide > when to stop. IIUC the best timing is when migration starts, not earlier, not later. If that's always the case, it's better qemu guarantee that? Or am I wrong that libvirt wants to not do it in some cases?
Peter Xu <peterx@redhat.com> writes: > On Wed, Jun 14, 2023 at 12:47:41PM -0300, Fabiano Rosas wrote: >> Peter Xu <peterx@redhat.com> writes: >> >> > On Mon, Jun 12, 2023 at 03:39:34PM -0400, Steven Sistare wrote: >> >> On 6/12/2023 2:44 PM, Peter Xu wrote: >> >> > Hi, Steve, >> >> > >> >> > On Wed, Jun 07, 2023 at 11:38:59AM -0700, Steve Sistare wrote: >> >> >> Extend the migration URI to support file:<filename>. This can be used for >> >> >> any migration scenario that does not require a reverse path. It can be used >> >> >> as an alternative to 'exec:cat > file' in minimized containers that do not >> >> >> contain /bin/sh, and it is easier to use than the fd:<fdname> URI. It can >> >> >> be used in HMP commands, and as a qemu command-line parameter. >> >> > >> >> > I have similar question on the fixed-ram work, >> >> >> >> Sorry, what is the "fixed-ram work"? >> > >> > https://lore.kernel.org/all/20230330180336.2791-1-farosas@suse.de >> > >> > It has similar requirement to migrate to a file, though slightly different >> > use case. >> > >> >> >> >> > on whether we should assume >> >> > the vm stopped before doing so. Again, it leaves us space for >> >> > optimizations on top without breaking anyone. >> >> >> >> I do not assume the vm is stopped. The migration code will stop the vm >> >> in migration_iteration_finish. >> >> >> >> > The other thing is considering a very busy guest, migration may not even >> >> > converge for "file:" URI (the same to other URIs) but I think that doesn't >> >> > make much sense to not converge for a "file:" URI. The user might be very >> >> > confused too. >> >> >> >> The file URI is mainly intended for the case where guest ram is backed by shared memory >> >> and preserved in place, in which case writes are not tracked and convergence is not an >> >> issue. If not shared memory, the user should be advised to stop the machine first. >> >> I should document these notes in qemu-options and/or migration.json. >> > >> > My question was whether we should treat "file:" differently from most of >> > other URIs. It makes the URI slightly tricky for sure, but it also does >> > make sense to me because "file:" implies more than the rest URIs, where >> > we're sure about the consequence of the migration (vm stops), in that case >> > keeping vm live makes it less performant, and also weird. >> > >> > It doesn't need to be special in memory type being shared, e.g. what if >> > there's a device that contains a lot of data to migrate in the future? >> > Assuming "shared memory will always migrate very fast" may not hold true. >> > >> > Do you think it makes more sense to just always stop VM when migrating to >> > file URI? Then if someone tries to restart the VM or cancel the migration, >> > we always do both (cancel migration, then start VM). >> >> From our discussions in the other thread, I have implemented a >> MIGRATION_CAPABILITY_SUSPEND to allow the management layer to decide >> whether the guest should be stopped by QEMU before the migration. >> >> I'm not opposed to coupling file URI with a stopped VM, although I >> think, at least for fixed-ram, libvirt would prefer to be able to decide >> when to stop. > > IIUC the best timing is when migration starts, not earlier, not later. > Sorry, I meant "when" as in "which migration instances". > If that's always the case, it's better qemu guarantee that? Or am I wrong > that libvirt wants to not do it in some cases? In this message Daniel mentions virDomainSnapshotXXX which would benefit from using the same "file" migration, but being done live: https://lore.kernel.org/r/ZD7MRGQ+4QsDBtKR@redhat.com And from your response here: https://lore.kernel.org/r/ZEA759BSs75ldW6Y@x1n I had understood that having a new SUSPEND cap to decide whether to do it live or non-live would be enough to cover all use-cases.
On Wed, Jun 14, 2023 at 02:59:54PM -0300, Fabiano Rosas wrote: > In this message Daniel mentions virDomainSnapshotXXX which would benefit > from using the same "file" migration, but being done live: > > https://lore.kernel.org/r/ZD7MRGQ+4QsDBtKR@redhat.com > > And from your response here: > https://lore.kernel.org/r/ZEA759BSs75ldW6Y@x1n > > I had understood that having a new SUSPEND cap to decide whether to do > it live or non-live would be enough to cover all use-cases. Oh, I probably lost some of the contexts there, sorry about that - so it's about not being able to live snapshot on !LINUX worlds properly, am I right? In the ideal world where we can always synchronously tracking guest pages (like what we do with userfaultfd wr-protections on modern Linux), the !SUSPEND case should always be covered by CAP_BACKGROUND_SNAPSHOT already in a more performant way. IOW, !SUSPEND seems to be not useful to Linux, because whenever we want to set !SUSPEND we should just use BG_SNAPSHOT. But I think indeed the live snapshot support is not good enough. Even on Linux, it lacks different memory type supports, multi-process support, and also no-go on very old kernels. So I assume the fallback makes sense, and then we can't always rely on that. Then I agree we can keep "file:" the same as others like proposed here, but I'd like to double check with all of us so we're on the same page.. And maybe we should mention some discussions into commit message or comments where proper in the code, so we can track what has happened easier. Thanks,
Peter Xu <peterx@redhat.com> writes: > On Wed, Jun 14, 2023 at 02:59:54PM -0300, Fabiano Rosas wrote: >> In this message Daniel mentions virDomainSnapshotXXX which would benefit >> from using the same "file" migration, but being done live: >> >> https://lore.kernel.org/r/ZD7MRGQ+4QsDBtKR@redhat.com >> >> And from your response here: >> https://lore.kernel.org/r/ZEA759BSs75ldW6Y@x1n >> >> I had understood that having a new SUSPEND cap to decide whether to do >> it live or non-live would be enough to cover all use-cases. > > Oh, I probably lost some of the contexts there, sorry about that - so it's > about not being able to live snapshot on !LINUX worlds properly, am I > right? > Right, so that gives us for now a reasonable use-case for keeping live migration behavior possible with "file:". > In the ideal world where we can always synchronously tracking guest pages > (like what we do with userfaultfd wr-protections on modern Linux), the > !SUSPEND case should always be covered by CAP_BACKGROUND_SNAPSHOT already > in a more performant way. IOW, !SUSPEND seems to be not useful to Linux, > because whenever we want to set !SUSPEND we should just use BG_SNAPSHOT. > I agree. > But I think indeed the live snapshot support is not good enough. Even on > Linux, it lacks different memory type supports, multi-process support, and > also no-go on very old kernels. So I assume the fallback makes sense, and > then we can't always rely on that. > > Then I agree we can keep "file:" the same as others like proposed here, but > I'd like to double check with all of us so we're on the same page.. +1 > And maybe we should mention some discussions into commit message or > comments where proper in the code, so we can track what has happened > easier. > I'll add some words where appropriate in my series as well. A v2 is already overdue with all the refactorings that have happened in the migration code.
On 6/15/2023 10:50 AM, Fabiano Rosas wrote: > Peter Xu <peterx@redhat.com> writes: > >> On Wed, Jun 14, 2023 at 02:59:54PM -0300, Fabiano Rosas wrote: >>> In this message Daniel mentions virDomainSnapshotXXX which would benefit >>> from using the same "file" migration, but being done live: >>> >>> https://lore.kernel.org/r/ZD7MRGQ+4QsDBtKR@redhat.com >>> >>> And from your response here: >>> https://lore.kernel.org/r/ZEA759BSs75ldW6Y@x1n >>> >>> I had understood that having a new SUSPEND cap to decide whether to do >>> it live or non-live would be enough to cover all use-cases. >> >> Oh, I probably lost some of the contexts there, sorry about that - so it's >> about not being able to live snapshot on !LINUX worlds properly, am I >> right? >> > > Right, so that gives us for now a reasonable use-case for keeping live > migration behavior possible with "file:". > >> In the ideal world where we can always synchronously tracking guest pages >> (like what we do with userfaultfd wr-protections on modern Linux), the >> !SUSPEND case should always be covered by CAP_BACKGROUND_SNAPSHOT already >> in a more performant way. IOW, !SUSPEND seems to be not useful to Linux, >> because whenever we want to set !SUSPEND we should just use BG_SNAPSHOT. >> > > I agree. > >> But I think indeed the live snapshot support is not good enough. Even on >> Linux, it lacks different memory type supports, multi-process support, and >> also no-go on very old kernels. So I assume the fallback makes sense, and >> then we can't always rely on that. >> >> Then I agree we can keep "file:" the same as others like proposed here, but >> I'd like to double check with all of us so we're on the same page.. > > +1 > >> And maybe we should mention some discussions into commit message or >> comments where proper in the code, so we can track what has happened >> easier. >> > > I'll add some words where appropriate in my series as well. A v2 is > already overdue with all the refactorings that have happened in the > migration code. Peter, should one of us proceed to submit the file URI as a stand-alone patch, since we both need it, and it has some value on its own? My version adds a watch on the incoming channel so we do not block monitor commands. It also adds tracepoints like the other URI's. Fabiano's version adds a nice unit test. Maybe we should submit a small series with both. - Steve
On Tue, Jun 20, 2023 at 02:36:58PM -0400, Steven Sistare wrote: > On 6/15/2023 10:50 AM, Fabiano Rosas wrote: > > Peter Xu <peterx@redhat.com> writes: > > > >> On Wed, Jun 14, 2023 at 02:59:54PM -0300, Fabiano Rosas wrote: > >>> In this message Daniel mentions virDomainSnapshotXXX which would benefit > >>> from using the same "file" migration, but being done live: > >>> > >>> https://lore.kernel.org/r/ZD7MRGQ+4QsDBtKR@redhat.com > >>> > >>> And from your response here: > >>> https://lore.kernel.org/r/ZEA759BSs75ldW6Y@x1n > >>> > >>> I had understood that having a new SUSPEND cap to decide whether to do > >>> it live or non-live would be enough to cover all use-cases. > >> > >> Oh, I probably lost some of the contexts there, sorry about that - so it's > >> about not being able to live snapshot on !LINUX worlds properly, am I > >> right? > >> > > > > Right, so that gives us for now a reasonable use-case for keeping live > > migration behavior possible with "file:". > > > >> In the ideal world where we can always synchronously tracking guest pages > >> (like what we do with userfaultfd wr-protections on modern Linux), the > >> !SUSPEND case should always be covered by CAP_BACKGROUND_SNAPSHOT already > >> in a more performant way. IOW, !SUSPEND seems to be not useful to Linux, > >> because whenever we want to set !SUSPEND we should just use BG_SNAPSHOT. > >> > > > > I agree. > > > >> But I think indeed the live snapshot support is not good enough. Even on > >> Linux, it lacks different memory type supports, multi-process support, and > >> also no-go on very old kernels. So I assume the fallback makes sense, and > >> then we can't always rely on that. > >> > >> Then I agree we can keep "file:" the same as others like proposed here, but > >> I'd like to double check with all of us so we're on the same page.. > > > > +1 > > > >> And maybe we should mention some discussions into commit message or > >> comments where proper in the code, so we can track what has happened > >> easier. > >> > > > > I'll add some words where appropriate in my series as well. A v2 is > > already overdue with all the refactorings that have happened in the > > migration code. > > Peter, should one of us proceed to submit the file URI as a stand-alone patch, > since we both need it, and it has some value on its own? > > My version adds a watch on the incoming channel so we do not block monitor commands. > It also adds tracepoints like the other URI's. > > Fabiano's version adds a nice unit test. > > Maybe we should submit a small series with both. I fully agree. I didn't check the details, but if we know the shared bits it'll be great if we arrange it before-hand, and then it might also be the best too for all sides. Thanks for raising this.
On Wed, Jun 07, 2023 at 11:38:59AM -0700, Steve Sistare wrote: > Extend the migration URI to support file:<filename>. This can be used for > any migration scenario that does not require a reverse path. It can be used > as an alternative to 'exec:cat > file' in minimized containers that do not > contain /bin/sh, and it is easier to use than the fd:<fdname> URI. It can > be used in HMP commands, and as a qemu command-line parameter. > > Signed-off-by: Steve Sistare <steven.sistare@oracle.com> > diff --git a/qemu-options.hx b/qemu-options.hx > index b37eb96..b93449d 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -4610,6 +4610,7 @@ DEF("incoming", HAS_ARG, QEMU_OPTION_incoming, \ > " prepare for incoming migration, listen on\n" \ > " specified protocol and socket address\n" \ > "-incoming fd:fd\n" \ > + "-incoming file:filename\n" \ > "-incoming exec:cmdline\n" \ > " accept incoming migration on given file descriptor\n" \ > " or from given external command\n" \ > @@ -4626,7 +4627,10 @@ SRST > Prepare for incoming migration, listen on a given unix socket. > > ``-incoming fd:fd`` > - Accept incoming migration from a given filedescriptor. > + Accept incoming migration from a given file descriptor. > + > +``-incoming file:filename`` > + Accept incoming migration from a given file. To be usable for libvirt we need to support an offset -incoming file:filename,offset=NNNN because for save/restore to disk,libvirt stores its own header and XML document in front of the QEMU save state, and we need to be able to tell QEMU to leave space (on save) or skip over it (on restore). With regards, Daniel
On Wed, Jun 07, 2023 at 11:38:59AM -0700, Steve Sistare wrote: > Extend the migration URI to support file:<filename>. This can be used for > any migration scenario that does not require a reverse path. It can be used > as an alternative to 'exec:cat > file' in minimized containers that do not > contain /bin/sh, and it is easier to use than the fd:<fdname> URI. It can > be used in HMP commands, and as a qemu command-line parameter. > > Signed-off-by: Steve Sistare <steven.sistare@oracle.com> In the cases where libvirt wants to save/restore QEMU migration state to a file, we also need to have libvirt header and XML document at the front of the file. IOW, if libvirt is to be able to use this new 'file:' protocol, then it neeeds to have the ability to specify an offset too. eg so libvirt can tell QEMU to start reading/writing at, for example, 4MB offset from the start. Should be fairly easy to add on top of this - just requires support for a URI parameter, and then a seek once the file is opened. > --- > migration/file.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++ > migration/file.h | 14 ++++++++++++ > migration/meson.build | 1 + > migration/migration.c | 5 ++++ > migration/trace-events | 4 ++++ > qemu-options.hx | 6 ++++- > 6 files changed, 91 insertions(+), 1 deletion(-) > create mode 100644 migration/file.c > create mode 100644 migration/file.h > > diff --git a/migration/file.c b/migration/file.c > new file mode 100644 > index 0000000..8e35827 > --- /dev/null > +++ b/migration/file.c > @@ -0,0 +1,62 @@ > +/* > + * Copyright (c) 2021-2023 Oracle and/or its affiliates. > + * > + * This work is licensed under the terms of the GNU GPL, version 2. > + * See the COPYING file in the top-level directory. > + */ > + > +#include "qemu/osdep.h" > +#include "channel.h" > +#include "file.h" > +#include "migration.h" > +#include "io/channel-file.h" > +#include "io/channel-util.h" > +#include "trace.h" > + > +void file_start_outgoing_migration(MigrationState *s, const char *filename, > + Error **errp) > +{ > + g_autoptr(QIOChannelFile) fioc = NULL; > + QIOChannel *ioc; > + > + trace_migration_file_outgoing(filename); > + > + fioc = qio_channel_file_new_path(filename, O_CREAT | O_WRONLY | O_TRUNC, > + 0600, errp); > + if (!fioc) { > + return; > + } > + > + ioc = QIO_CHANNEL(fioc); > + qio_channel_set_name(ioc, "migration-file-outgoing"); > + migration_channel_connect(s, ioc, NULL, NULL); > +} > + > +static gboolean file_accept_incoming_migration(QIOChannel *ioc, > + GIOCondition condition, > + gpointer opaque) > +{ > + migration_channel_process_incoming(ioc); > + object_unref(OBJECT(ioc)); > + return G_SOURCE_REMOVE; > +} > + > +void file_start_incoming_migration(const char *filename, Error **errp) > +{ > + QIOChannelFile *fioc = NULL; > + QIOChannel *ioc; > + > + trace_migration_file_incoming(filename); > + > + fioc = qio_channel_file_new_path(filename, O_RDONLY, 0, errp); > + if (!fioc) { > + return; > + } > + > + ioc = QIO_CHANNEL(fioc); > + qio_channel_set_name(QIO_CHANNEL(ioc), "migration-file-incoming"); > + qio_channel_add_watch_full(ioc, G_IO_IN, > + file_accept_incoming_migration, > + NULL, NULL, > + g_main_context_get_thread_default()); > +} > diff --git a/migration/file.h b/migration/file.h > new file mode 100644 > index 0000000..841b94a > --- /dev/null > +++ b/migration/file.h > @@ -0,0 +1,14 @@ > +/* > + * Copyright (c) 2021-2023 Oracle and/or its affiliates. > + * > + * This work is licensed under the terms of the GNU GPL, version 2. > + * See the COPYING file in the top-level directory. > + */ > + > +#ifndef QEMU_MIGRATION_FILE_H > +#define QEMU_MIGRATION_FILE_H > +void file_start_incoming_migration(const char *filename, Error **errp); > + > +void file_start_outgoing_migration(MigrationState *s, const char *filename, > + Error **errp); > +#endif > diff --git a/migration/meson.build b/migration/meson.build > index 8ba6e42..3af817e 100644 > --- a/migration/meson.build > +++ b/migration/meson.build > @@ -16,6 +16,7 @@ softmmu_ss.add(files( > 'dirtyrate.c', > 'exec.c', > 'fd.c', > + 'file.c', > 'global_state.c', > 'migration-hmp-cmds.c', > 'migration.c', > diff --git a/migration/migration.c b/migration/migration.c > index dc05c6f..cfbde86 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -20,6 +20,7 @@ > #include "migration/blocker.h" > #include "exec.h" > #include "fd.h" > +#include "file.h" > #include "socket.h" > #include "sysemu/runstate.h" > #include "sysemu/sysemu.h" > @@ -442,6 +443,8 @@ static void qemu_start_incoming_migration(const char *uri, Error **errp) > exec_start_incoming_migration(p, errp); > } else if (strstart(uri, "fd:", &p)) { > fd_start_incoming_migration(p, errp); > + } else if (strstart(uri, "file:", &p)) { > + file_start_incoming_migration(p, errp); > } else { > error_setg(errp, "unknown migration protocol: %s", uri); > } > @@ -1662,6 +1665,8 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk, > exec_start_outgoing_migration(s, p, &local_err); > } else if (strstart(uri, "fd:", &p)) { > fd_start_outgoing_migration(s, p, &local_err); > + } else if (strstart(uri, "file:", &p)) { > + file_start_outgoing_migration(s, p, &local_err); > } else { > if (!(has_resume && resume)) { > yank_unregister_instance(MIGRATION_YANK_INSTANCE); > diff --git a/migration/trace-events b/migration/trace-events > index cdaef7a..c8c1771 100644 > --- a/migration/trace-events > +++ b/migration/trace-events > @@ -307,6 +307,10 @@ migration_exec_incoming(const char *cmd) "cmd=%s" > migration_fd_outgoing(int fd) "fd=%d" > migration_fd_incoming(int fd) "fd=%d" > > +# file.c > +migration_file_outgoing(const char *filename) "filename=%s" > +migration_file_incoming(const char *filename) "filename=%s" > + > # socket.c > migration_socket_incoming_accepted(void) "" > migration_socket_outgoing_connected(const char *hostname) "hostname=%s" > diff --git a/qemu-options.hx b/qemu-options.hx > index b37eb96..b93449d 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -4610,6 +4610,7 @@ DEF("incoming", HAS_ARG, QEMU_OPTION_incoming, \ > " prepare for incoming migration, listen on\n" \ > " specified protocol and socket address\n" \ > "-incoming fd:fd\n" \ > + "-incoming file:filename\n" \ > "-incoming exec:cmdline\n" \ > " accept incoming migration on given file descriptor\n" \ > " or from given external command\n" \ > @@ -4626,7 +4627,10 @@ SRST > Prepare for incoming migration, listen on a given unix socket. > > ``-incoming fd:fd`` > - Accept incoming migration from a given filedescriptor. > + Accept incoming migration from a given file descriptor. > + > +``-incoming file:filename`` > + Accept incoming migration from a given file. > > ``-incoming exec:cmdline`` > Accept incoming migration as an output from specified external > -- > 1.8.3.1 > With regards, Daniel
Steve Sistare <steven.sistare@oracle.com> writes: > Extend the migration URI to support file:<filename>. This can be used for > any migration scenario that does not require a reverse path. It can be used > as an alternative to 'exec:cat > file' in minimized containers that do not > contain /bin/sh, and it is easier to use than the fd:<fdname> URI. It can > be used in HMP commands, and as a qemu command-line parameter. > > Signed-off-by: Steve Sistare <steven.sistare@oracle.com> Reviewed-by: Fabiano Rosas <farosas@suse.de> I'm ok with using this version over mine. I based my series on top of this and it works fine. I'm preparing a couple of patches with the test case. We'll need a fix to common migration code before it can work due to the latest migration-test.c changes.
On 6/22/2023 6:12 AM, Daniel P. Berrangé wrote: > On Wed, Jun 07, 2023 at 11:38:59AM -0700, Steve Sistare wrote: >> Extend the migration URI to support file:<filename>. This can be used for >> any migration scenario that does not require a reverse path. It can be used >> as an alternative to 'exec:cat > file' in minimized containers that do not >> contain /bin/sh, and it is easier to use than the fd:<fdname> URI. It can >> be used in HMP commands, and as a qemu command-line parameter. >> >> Signed-off-by: Steve Sistare <steven.sistare@oracle.com> > > In the cases where libvirt wants to save/restore QEMU migration state > to a file, we also need to have libvirt header and XML document at the > front of the file. > > IOW, if libvirt is to be able to use this new 'file:' protocol, then > it neeeds to have the ability to specify an offset too. eg so libvirt > can tell QEMU to start reading/writing at, for example, 4MB offset > from the start. > > Should be fairly easy to add on top of this - just requires support > for a URI parameter, and then a seek once the file is opened. Will do, probably today - steve
On 6/22/2023 8:20 AM, Fabiano Rosas wrote: > Steve Sistare <steven.sistare@oracle.com> writes: > >> Extend the migration URI to support file:<filename>. This can be used for >> any migration scenario that does not require a reverse path. It can be used >> as an alternative to 'exec:cat > file' in minimized containers that do not >> contain /bin/sh, and it is easier to use than the fd:<fdname> URI. It can >> be used in HMP commands, and as a qemu command-line parameter. >> >> Signed-off-by: Steve Sistare <steven.sistare@oracle.com> > > Reviewed-by: Fabiano Rosas <farosas@suse.de> > > I'm ok with using this version over mine. I based my series on top of > this and it works fine. > > I'm preparing a couple of patches with the test case. We'll need a fix > to common migration code before it can work due to the latest > migration-test.c changes. Hi Fabiano, I re-submitted my patch, along with an offset parameter requested by Daniel. Perhaps you can add a test case using the offset? - Steve
diff --git a/migration/file.c b/migration/file.c new file mode 100644 index 0000000..8e35827 --- /dev/null +++ b/migration/file.c @@ -0,0 +1,62 @@ +/* + * Copyright (c) 2021-2023 Oracle and/or its affiliates. + * + * This work is licensed under the terms of the GNU GPL, version 2. + * See the COPYING file in the top-level directory. + */ + +#include "qemu/osdep.h" +#include "channel.h" +#include "file.h" +#include "migration.h" +#include "io/channel-file.h" +#include "io/channel-util.h" +#include "trace.h" + +void file_start_outgoing_migration(MigrationState *s, const char *filename, + Error **errp) +{ + g_autoptr(QIOChannelFile) fioc = NULL; + QIOChannel *ioc; + + trace_migration_file_outgoing(filename); + + fioc = qio_channel_file_new_path(filename, O_CREAT | O_WRONLY | O_TRUNC, + 0600, errp); + if (!fioc) { + return; + } + + ioc = QIO_CHANNEL(fioc); + qio_channel_set_name(ioc, "migration-file-outgoing"); + migration_channel_connect(s, ioc, NULL, NULL); +} + +static gboolean file_accept_incoming_migration(QIOChannel *ioc, + GIOCondition condition, + gpointer opaque) +{ + migration_channel_process_incoming(ioc); + object_unref(OBJECT(ioc)); + return G_SOURCE_REMOVE; +} + +void file_start_incoming_migration(const char *filename, Error **errp) +{ + QIOChannelFile *fioc = NULL; + QIOChannel *ioc; + + trace_migration_file_incoming(filename); + + fioc = qio_channel_file_new_path(filename, O_RDONLY, 0, errp); + if (!fioc) { + return; + } + + ioc = QIO_CHANNEL(fioc); + qio_channel_set_name(QIO_CHANNEL(ioc), "migration-file-incoming"); + qio_channel_add_watch_full(ioc, G_IO_IN, + file_accept_incoming_migration, + NULL, NULL, + g_main_context_get_thread_default()); +} diff --git a/migration/file.h b/migration/file.h new file mode 100644 index 0000000..841b94a --- /dev/null +++ b/migration/file.h @@ -0,0 +1,14 @@ +/* + * Copyright (c) 2021-2023 Oracle and/or its affiliates. + * + * This work is licensed under the terms of the GNU GPL, version 2. + * See the COPYING file in the top-level directory. + */ + +#ifndef QEMU_MIGRATION_FILE_H +#define QEMU_MIGRATION_FILE_H +void file_start_incoming_migration(const char *filename, Error **errp); + +void file_start_outgoing_migration(MigrationState *s, const char *filename, + Error **errp); +#endif diff --git a/migration/meson.build b/migration/meson.build index 8ba6e42..3af817e 100644 --- a/migration/meson.build +++ b/migration/meson.build @@ -16,6 +16,7 @@ softmmu_ss.add(files( 'dirtyrate.c', 'exec.c', 'fd.c', + 'file.c', 'global_state.c', 'migration-hmp-cmds.c', 'migration.c', diff --git a/migration/migration.c b/migration/migration.c index dc05c6f..cfbde86 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -20,6 +20,7 @@ #include "migration/blocker.h" #include "exec.h" #include "fd.h" +#include "file.h" #include "socket.h" #include "sysemu/runstate.h" #include "sysemu/sysemu.h" @@ -442,6 +443,8 @@ static void qemu_start_incoming_migration(const char *uri, Error **errp) exec_start_incoming_migration(p, errp); } else if (strstart(uri, "fd:", &p)) { fd_start_incoming_migration(p, errp); + } else if (strstart(uri, "file:", &p)) { + file_start_incoming_migration(p, errp); } else { error_setg(errp, "unknown migration protocol: %s", uri); } @@ -1662,6 +1665,8 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk, exec_start_outgoing_migration(s, p, &local_err); } else if (strstart(uri, "fd:", &p)) { fd_start_outgoing_migration(s, p, &local_err); + } else if (strstart(uri, "file:", &p)) { + file_start_outgoing_migration(s, p, &local_err); } else { if (!(has_resume && resume)) { yank_unregister_instance(MIGRATION_YANK_INSTANCE); diff --git a/migration/trace-events b/migration/trace-events index cdaef7a..c8c1771 100644 --- a/migration/trace-events +++ b/migration/trace-events @@ -307,6 +307,10 @@ migration_exec_incoming(const char *cmd) "cmd=%s" migration_fd_outgoing(int fd) "fd=%d" migration_fd_incoming(int fd) "fd=%d" +# file.c +migration_file_outgoing(const char *filename) "filename=%s" +migration_file_incoming(const char *filename) "filename=%s" + # socket.c migration_socket_incoming_accepted(void) "" migration_socket_outgoing_connected(const char *hostname) "hostname=%s" diff --git a/qemu-options.hx b/qemu-options.hx index b37eb96..b93449d 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -4610,6 +4610,7 @@ DEF("incoming", HAS_ARG, QEMU_OPTION_incoming, \ " prepare for incoming migration, listen on\n" \ " specified protocol and socket address\n" \ "-incoming fd:fd\n" \ + "-incoming file:filename\n" \ "-incoming exec:cmdline\n" \ " accept incoming migration on given file descriptor\n" \ " or from given external command\n" \ @@ -4626,7 +4627,10 @@ SRST Prepare for incoming migration, listen on a given unix socket. ``-incoming fd:fd`` - Accept incoming migration from a given filedescriptor. + Accept incoming migration from a given file descriptor. + +``-incoming file:filename`` + Accept incoming migration from a given file. ``-incoming exec:cmdline`` Accept incoming migration as an output from specified external
Extend the migration URI to support file:<filename>. This can be used for any migration scenario that does not require a reverse path. It can be used as an alternative to 'exec:cat > file' in minimized containers that do not contain /bin/sh, and it is easier to use than the fd:<fdname> URI. It can be used in HMP commands, and as a qemu command-line parameter. Signed-off-by: Steve Sistare <steven.sistare@oracle.com> --- migration/file.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++ migration/file.h | 14 ++++++++++++ migration/meson.build | 1 + migration/migration.c | 5 ++++ migration/trace-events | 4 ++++ qemu-options.hx | 6 ++++- 6 files changed, 91 insertions(+), 1 deletion(-) create mode 100644 migration/file.c create mode 100644 migration/file.h