Message ID | 20230509181528.84212-1-quintela@redhat.com |
---|---|
State | New |
Headers | show |
Series | multifd: Add colo support | expand |
On Tue, 9 May 2023 20:15:28 +0200 Juan Quintela <quintela@redhat.com> wrote: > From: Lukas Straub <lukasstraub2@web.de> > > Like in the normal ram_load() path, put the received pages into the > colo cache and mark the pages in the bitmap so that they will be > flushed to the guest later. > > Signed-off-by: Lukas Straub <lukasstraub2@web.de> > > --- > > Hi Lukas > > What about this instead of your other three patches? I think it is > clearer, and I don't think that we are going to have anything else > that is going to hook there anytime soon. > > Notice that I put CONFIG_COLO waiting for Vladimir changes to get in > before I merge this. > > Notice also that I "lost" the line: > > p->host = p->block->host; > > In the error case. But in that case we are aborting the migration, so > we don't care. > > Can you check if it works for you? > Here it compiles, so it must be perfect. > > Thanks, Juan. This way is okay for now. Though I will send a patch. Regards, Lukas Straub > --- > migration/meson.build | 2 +- > migration/multifd-colo.c | 49 ++++++++++++++++++++++++++++++++++++++++ > migration/multifd-colo.h | 24 ++++++++++++++++++++ > 3 files changed, 74 insertions(+), 1 deletion(-) > create mode 100644 migration/multifd-colo.c > create mode 100644 migration/multifd-colo.h > > diff --git a/migration/meson.build b/migration/meson.build > index 75de868bb7..c9db40d4d4 100644 > --- a/migration/meson.build > +++ b/migration/meson.build > @@ -23,7 +23,7 @@ softmmu_ss.add(files( > 'migration.c', > 'multifd.c', > 'multifd-zlib.c', > - 'multifd-zlib.c', > + 'multifd-colo.c', > 'ram-compress.c', > 'options.c', > 'postcopy-ram.c', > diff --git a/migration/multifd-colo.c b/migration/multifd-colo.c > new file mode 100644 > index 0000000000..10fa1467fa > --- /dev/null > +++ b/migration/multifd-colo.c > @@ -0,0 +1,49 @@ > +/* > + * multifd colo implementation > + * > + * Copyright (c) Lukas Straub <lukasstraub2@web.de> > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > + * See the COPYING file in the top-level directory. > + */ > + > +#include "qemu/osdep.h" > +#include "exec/target_page.h" > +#include "exec/ramblock.h" > +#include "qemu/error-report.h" > +#include "qapi/error.h" > +#include "ram.h" > +#include "multifd.h" > +#include "options.h" > +#include "io/channel-socket.h" > +#include "migration/colo.h" > +#include "multifd-colo.h" > + > +void multifd_colo_prepare_recv_pages(MultiFDRecvParams *p) > +{ > + if (migrate_colo()) { > + /* > + * While we're still in precopy mode, we copy received pages to both guest > + * and cache. No need to set dirty bits, since guest and cache memory are > + * in sync. > + */ > + if (migration_incoming_in_colo_state()) { > + colo_record_bitmap(p->block, p->normal, p->normal_num); > + } > + p->host = p->block->colo_cache; > + } > +} > + > +void multifd_colo_process_recv_pages(MultiFDRecvParams *p) > +{ > + if (migrate_colo()) { > + if (!migration_incoming_in_colo_state()) { > + for (int i = 0; i < p->normal_num; i++) { > + void *guest = p->block->host + p->normal[i]; > + void *cache = p->host + p->normal[i]; > + memcpy(guest, cache, p->page_size); > + } > + } > + p->host = p->block->host; > + } > +} > diff --git a/migration/multifd-colo.h b/migration/multifd-colo.h > new file mode 100644 > index 0000000000..1636c617fc > --- /dev/null > +++ b/migration/multifd-colo.h > @@ -0,0 +1,24 @@ > +/* > + * multifd colo header > + * > + * Copyright (c) Lukas Straub <lukasstraub2@web.de> > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > + * See the COPYING file in the top-level directory. > + */ > + > +#ifndef QEMU_MIGRATION_MULTIFD_COLO_H > +#define QEMU_MIGRATION_MULTIFD_COLO_H > + > +#ifndef CONFIG_COLO > + > +void multifd_colo_prepare_recv_pages(MultiFDRecvParams *p); > +void multifd_colo_process_recv_pages(MultiFDRecvParams *p); > + > +#else > + > +static inline void multifd_colo_prepare_recv_pages(MultiFDRecvParams *p) {} > +static inline void multifd_colo_process_recv_pages(MultiFDRecvParams *p) {} > + > +#endif /* CONFIG_COLO */ > +#endif --
Lukas Straub <lukasstraub2@web.de> wrote: > On Tue, 9 May 2023 20:15:28 +0200 > Juan Quintela <quintela@redhat.com> wrote: > >> From: Lukas Straub <lukasstraub2@web.de> >> >> Like in the normal ram_load() path, put the received pages into the >> colo cache and mark the pages in the bitmap so that they will be >> flushed to the guest later. >> >> Signed-off-by: Lukas Straub <lukasstraub2@web.de> >> >> --- >> >> Hi Lukas >> >> What about this instead of your other three patches? I think it is >> clearer, and I don't think that we are going to have anything else >> that is going to hook there anytime soon. >> >> Notice that I put CONFIG_COLO waiting for Vladimir changes to get in >> before I merge this. >> >> Notice also that I "lost" the line: >> >> p->host = p->block->host; >> >> In the error case. But in that case we are aborting the migration, so >> we don't care. >> >> Can you check if it works for you? >> Here it compiles, so it must be perfect. >> >> Thanks, Juan. > > This way is okay for now. Though I will send a patch. If it works for you, you can add a review-by and I will integrate in next PULL. Later, Juan. > > Regards, > Lukas Straub > >> --- >> migration/meson.build | 2 +- >> migration/multifd-colo.c | 49 ++++++++++++++++++++++++++++++++++++++++ >> migration/multifd-colo.h | 24 ++++++++++++++++++++ >> 3 files changed, 74 insertions(+), 1 deletion(-) >> create mode 100644 migration/multifd-colo.c >> create mode 100644 migration/multifd-colo.h >> >> diff --git a/migration/meson.build b/migration/meson.build >> index 75de868bb7..c9db40d4d4 100644 >> --- a/migration/meson.build >> +++ b/migration/meson.build >> @@ -23,7 +23,7 @@ softmmu_ss.add(files( >> 'migration.c', >> 'multifd.c', >> 'multifd-zlib.c', >> - 'multifd-zlib.c', >> + 'multifd-colo.c', >> 'ram-compress.c', >> 'options.c', >> 'postcopy-ram.c', >> diff --git a/migration/multifd-colo.c b/migration/multifd-colo.c >> new file mode 100644 >> index 0000000000..10fa1467fa >> --- /dev/null >> +++ b/migration/multifd-colo.c >> @@ -0,0 +1,49 @@ >> +/* >> + * multifd colo implementation >> + * >> + * Copyright (c) Lukas Straub <lukasstraub2@web.de> >> + * >> + * This work is licensed under the terms of the GNU GPL, version 2 or later. >> + * See the COPYING file in the top-level directory. >> + */ >> + >> +#include "qemu/osdep.h" >> +#include "exec/target_page.h" >> +#include "exec/ramblock.h" >> +#include "qemu/error-report.h" >> +#include "qapi/error.h" >> +#include "ram.h" >> +#include "multifd.h" >> +#include "options.h" >> +#include "io/channel-socket.h" >> +#include "migration/colo.h" >> +#include "multifd-colo.h" >> + >> +void multifd_colo_prepare_recv_pages(MultiFDRecvParams *p) >> +{ >> + if (migrate_colo()) { >> + /* >> + * While we're still in precopy mode, we copy received pages to both guest >> + * and cache. No need to set dirty bits, since guest and cache memory are >> + * in sync. >> + */ >> + if (migration_incoming_in_colo_state()) { >> + colo_record_bitmap(p->block, p->normal, p->normal_num); >> + } >> + p->host = p->block->colo_cache; >> + } >> +} >> + >> +void multifd_colo_process_recv_pages(MultiFDRecvParams *p) >> +{ >> + if (migrate_colo()) { >> + if (!migration_incoming_in_colo_state()) { >> + for (int i = 0; i < p->normal_num; i++) { >> + void *guest = p->block->host + p->normal[i]; >> + void *cache = p->host + p->normal[i]; >> + memcpy(guest, cache, p->page_size); >> + } >> + } >> + p->host = p->block->host; >> + } >> +} >> diff --git a/migration/multifd-colo.h b/migration/multifd-colo.h >> new file mode 100644 >> index 0000000000..1636c617fc >> --- /dev/null >> +++ b/migration/multifd-colo.h >> @@ -0,0 +1,24 @@ >> +/* >> + * multifd colo header >> + * >> + * Copyright (c) Lukas Straub <lukasstraub2@web.de> >> + * >> + * This work is licensed under the terms of the GNU GPL, version 2 or later. >> + * See the COPYING file in the top-level directory. >> + */ >> + >> +#ifndef QEMU_MIGRATION_MULTIFD_COLO_H >> +#define QEMU_MIGRATION_MULTIFD_COLO_H >> + >> +#ifndef CONFIG_COLO >> + >> +void multifd_colo_prepare_recv_pages(MultiFDRecvParams *p); >> +void multifd_colo_process_recv_pages(MultiFDRecvParams *p); >> + >> +#else >> + >> +static inline void multifd_colo_prepare_recv_pages(MultiFDRecvParams *p) {} >> +static inline void multifd_colo_process_recv_pages(MultiFDRecvParams *p) {} >> + >> +#endif /* CONFIG_COLO */ >> +#endif
On Thu, 11 May 2023 11:52:55 +0200 Juan Quintela <quintela@redhat.com> wrote: > Lukas Straub <lukasstraub2@web.de> wrote: > > On Tue, 9 May 2023 20:15:28 +0200 > > Juan Quintela <quintela@redhat.com> wrote: > > > >> From: Lukas Straub <lukasstraub2@web.de> > >> > >> Like in the normal ram_load() path, put the received pages into the > >> colo cache and mark the pages in the bitmap so that they will be > >> flushed to the guest later. > >> > >> Signed-off-by: Lukas Straub <lukasstraub2@web.de> > >> > >> --- > >> > >> Hi Lukas > >> > >> What about this instead of your other three patches? I think it is > >> clearer, and I don't think that we are going to have anything else > >> that is going to hook there anytime soon. > >> > >> Notice that I put CONFIG_COLO waiting for Vladimir changes to get in > >> before I merge this. > >> > >> Notice also that I "lost" the line: > >> > >> p->host = p->block->host; > >> > >> In the error case. But in that case we are aborting the migration, so > >> we don't care. > >> > >> Can you check if it works for you? > >> Here it compiles, so it must be perfect. > >> > >> Thanks, Juan. > > > > This way is okay for now. Though I will send a patch. > > > If it works for you, you can add a review-by and I will integrate in > next PULL. Nack, There are quite few things wrong at a glance. I will cook up my own patch, similar to this one. > > Later, Juan. > > > > > Regards, > > Lukas Straub > > > >> --- > >> migration/meson.build | 2 +- > >> migration/multifd-colo.c | 49 ++++++++++++++++++++++++++++++++++++++++ > >> migration/multifd-colo.h | 24 ++++++++++++++++++++ > >> 3 files changed, 74 insertions(+), 1 deletion(-) > >> create mode 100644 migration/multifd-colo.c > >> create mode 100644 migration/multifd-colo.h > >> > >> diff --git a/migration/meson.build b/migration/meson.build > >> index 75de868bb7..c9db40d4d4 100644 > >> --- a/migration/meson.build > >> +++ b/migration/meson.build > >> @@ -23,7 +23,7 @@ softmmu_ss.add(files( > >> 'migration.c', > >> 'multifd.c', > >> 'multifd-zlib.c', > >> - 'multifd-zlib.c', ??? > >> + 'multifd-colo.c', > >> 'ram-compress.c', > >> 'options.c', > >> 'postcopy-ram.c', > >> diff --git a/migration/multifd-colo.c b/migration/multifd-colo.c With just this patch, this file is not compiled in at all. And multifd_colo_prepare_recv_pages and multifd_colo_process_recv_pages aren't called from anywhere. I guess they should be called before and after multifd_recv_state->ops->recv_pages(p, &local_err); ? > >> new file mode 100644 > >> index 0000000000..10fa1467fa > >> --- /dev/null > >> +++ b/migration/multifd-colo.c > >> @@ -0,0 +1,49 @@ > >> +/* > >> + * multifd colo implementation > >> + * > >> + * Copyright (c) Lukas Straub <lukasstraub2@web.de> > >> + * > >> + * This work is licensed under the terms of the GNU GPL, version 2 or later. > >> + * See the COPYING file in the top-level directory. > >> + */ > >> + > >> +#include "qemu/osdep.h" > >> +#include "exec/target_page.h" > >> +#include "exec/ramblock.h" > >> +#include "qemu/error-report.h" > >> +#include "qapi/error.h" > >> +#include "ram.h" > >> +#include "multifd.h" > >> +#include "options.h" > >> +#include "io/channel-socket.h" > >> +#include "migration/colo.h" > >> +#include "multifd-colo.h" > >> + > >> +void multifd_colo_prepare_recv_pages(MultiFDRecvParams *p) > >> +{ > >> + if (migrate_colo()) { > >> + /* > >> + * While we're still in precopy mode, we copy received pages to both guest > >> + * and cache. No need to set dirty bits, since guest and cache memory are > >> + * in sync. > >> + */ > >> + if (migration_incoming_in_colo_state()) { > >> + colo_record_bitmap(p->block, p->normal, p->normal_num); > >> + } > >> + p->host = p->block->colo_cache; > >> + } > >> +} > >> + > >> +void multifd_colo_process_recv_pages(MultiFDRecvParams *p) > >> +{ > >> + if (migrate_colo()) { > >> + if (!migration_incoming_in_colo_state()) { > >> + for (int i = 0; i < p->normal_num; i++) { > >> + void *guest = p->block->host + p->normal[i]; > >> + void *cache = p->host + p->normal[i]; > >> + memcpy(guest, cache, p->page_size); > >> + } > >> + } > >> + p->host = p->block->host; > >> + } > >> +} > >> diff --git a/migration/multifd-colo.h b/migration/multifd-colo.h > >> new file mode 100644 > >> index 0000000000..1636c617fc > >> --- /dev/null > >> +++ b/migration/multifd-colo.h > >> @@ -0,0 +1,24 @@ > >> +/* > >> + * multifd colo header > >> + * > >> + * Copyright (c) Lukas Straub <lukasstraub2@web.de> > >> + * > >> + * This work is licensed under the terms of the GNU GPL, version 2 or later. > >> + * See the COPYING file in the top-level directory. > >> + */ > >> + > >> +#ifndef QEMU_MIGRATION_MULTIFD_COLO_H > >> +#define QEMU_MIGRATION_MULTIFD_COLO_H > >> + > >> +#ifndef CONFIG_COLO Should be CONFIG_REPLICATION > >> + > >> +void multifd_colo_prepare_recv_pages(MultiFDRecvParams *p); > >> +void multifd_colo_process_recv_pages(MultiFDRecvParams *p); > >> + > >> +#else > >> + > >> +static inline void multifd_colo_prepare_recv_pages(MultiFDRecvParams *p) {} > >> +static inline void multifd_colo_process_recv_pages(MultiFDRecvParams *p) {} > >> + > >> +#endif /* CONFIG_COLO */ > >> +#endif > --
Lukas Straub <lukasstraub2@web.de> wrote: > On Thu, 11 May 2023 11:52:55 +0200 > Juan Quintela <quintela@redhat.com> wrote: > >> Lukas Straub <lukasstraub2@web.de> wrote: >> > On Tue, 9 May 2023 20:15:28 +0200 >> > Juan Quintela <quintela@redhat.com> wrote: >> > >> >> From: Lukas Straub <lukasstraub2@web.de> >> >> >> >> Like in the normal ram_load() path, put the received pages into the >> >> colo cache and mark the pages in the bitmap so that they will be >> >> flushed to the guest later. >> >> >> >> Signed-off-by: Lukas Straub <lukasstraub2@web.de> >> >> >> >> --- >> >> >> >> Hi Lukas >> >> >> >> What about this instead of your other three patches? I think it is >> >> clearer, and I don't think that we are going to have anything else >> >> that is going to hook there anytime soon. >> >> >> >> Notice that I put CONFIG_COLO waiting for Vladimir changes to get in >> >> before I merge this. >> >> >> >> Notice also that I "lost" the line: >> >> >> >> p->host = p->block->host; >> >> >> >> In the error case. But in that case we are aborting the migration, so >> >> we don't care. >> >> >> >> Can you check if it works for you? >> >> Here it compiles, so it must be perfect. >> >> >> >> Thanks, Juan. >> > >> > This way is okay for now. Though I will send a patch. >> >> >> If it works for you, you can add a review-by and I will integrate in >> next PULL. > > Nack, There are quite few things wrong at a glance. I will cook up my > own patch, similar to this one. Perfect. Thanks. >> >> @@ -23,7 +23,7 @@ softmmu_ss.add(files( >> >> 'migration.c', >> >> 'multifd.c', >> >> 'multifd-zlib.c', >> >> - 'multifd-zlib.c', > > ??? Patch sent upstream to fix it. Introduced (by me) when I rebased/merged your compression code changes. >> >> diff --git a/migration/multifd-colo.h b/migration/multifd-colo.h >> >> new file mode 100644 >> >> index 0000000000..1636c617fc >> >> --- /dev/null >> >> +++ b/migration/multifd-colo.h >> >> @@ -0,0 +1,24 @@ >> >> +/* >> >> + * multifd colo header >> >> + * >> >> + * Copyright (c) Lukas Straub <lukasstraub2@web.de> >> >> + * >> >> + * This work is licensed under the terms of the GNU GPL, version 2 or later. >> >> + * See the COPYING file in the top-level directory. >> >> + */ >> >> + >> >> +#ifndef QEMU_MIGRATION_MULTIFD_COLO_H >> >> +#define QEMU_MIGRATION_MULTIFD_COLO_H >> >> + >> >> +#ifndef CONFIG_COLO > > Should be CONFIG_REPLICATION Aha, thanks. Will wait for your patch. Later, Juan. >> >> + >> >> +void multifd_colo_prepare_recv_pages(MultiFDRecvParams *p); >> >> +void multifd_colo_process_recv_pages(MultiFDRecvParams *p); >> >> + >> >> +#else >> >> + >> >> +static inline void multifd_colo_prepare_recv_pages(MultiFDRecvParams *p) {} >> >> +static inline void multifd_colo_process_recv_pages(MultiFDRecvParams *p) {} >> >> + >> >> +#endif /* CONFIG_COLO */ >> >> +#endif >>
diff --git a/migration/meson.build b/migration/meson.build index 75de868bb7..c9db40d4d4 100644 --- a/migration/meson.build +++ b/migration/meson.build @@ -23,7 +23,7 @@ softmmu_ss.add(files( 'migration.c', 'multifd.c', 'multifd-zlib.c', - 'multifd-zlib.c', + 'multifd-colo.c', 'ram-compress.c', 'options.c', 'postcopy-ram.c', diff --git a/migration/multifd-colo.c b/migration/multifd-colo.c new file mode 100644 index 0000000000..10fa1467fa --- /dev/null +++ b/migration/multifd-colo.c @@ -0,0 +1,49 @@ +/* + * multifd colo implementation + * + * Copyright (c) Lukas Straub <lukasstraub2@web.de> + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +#include "qemu/osdep.h" +#include "exec/target_page.h" +#include "exec/ramblock.h" +#include "qemu/error-report.h" +#include "qapi/error.h" +#include "ram.h" +#include "multifd.h" +#include "options.h" +#include "io/channel-socket.h" +#include "migration/colo.h" +#include "multifd-colo.h" + +void multifd_colo_prepare_recv_pages(MultiFDRecvParams *p) +{ + if (migrate_colo()) { + /* + * While we're still in precopy mode, we copy received pages to both guest + * and cache. No need to set dirty bits, since guest and cache memory are + * in sync. + */ + if (migration_incoming_in_colo_state()) { + colo_record_bitmap(p->block, p->normal, p->normal_num); + } + p->host = p->block->colo_cache; + } +} + +void multifd_colo_process_recv_pages(MultiFDRecvParams *p) +{ + if (migrate_colo()) { + if (!migration_incoming_in_colo_state()) { + for (int i = 0; i < p->normal_num; i++) { + void *guest = p->block->host + p->normal[i]; + void *cache = p->host + p->normal[i]; + memcpy(guest, cache, p->page_size); + } + } + p->host = p->block->host; + } +} diff --git a/migration/multifd-colo.h b/migration/multifd-colo.h new file mode 100644 index 0000000000..1636c617fc --- /dev/null +++ b/migration/multifd-colo.h @@ -0,0 +1,24 @@ +/* + * multifd colo header + * + * Copyright (c) Lukas Straub <lukasstraub2@web.de> + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +#ifndef QEMU_MIGRATION_MULTIFD_COLO_H +#define QEMU_MIGRATION_MULTIFD_COLO_H + +#ifndef CONFIG_COLO + +void multifd_colo_prepare_recv_pages(MultiFDRecvParams *p); +void multifd_colo_process_recv_pages(MultiFDRecvParams *p); + +#else + +static inline void multifd_colo_prepare_recv_pages(MultiFDRecvParams *p) {} +static inline void multifd_colo_process_recv_pages(MultiFDRecvParams *p) {} + +#endif /* CONFIG_COLO */ +#endif