Re: [PATCH v6 09/10] migration/yank: Keep track of registered yank instances
On Mon, 25 Sep 2023 09:20:58 -0300
Fabiano Rosas wrote:
> CC: Daniel for the QIOChannel discussion
>
> Lukas Straub writes:
> > On Thu, 14 Sep 2023 10:57:47 -0400
> > Peter Xu wrote:
> >
> >> On Thu, Sep 14, 2023 at 10:23:38AM -0300, Fabiano Rosas wrote:
> >> > Peter Xu writes:
> >> >
> >> > > On Wed, Sep 13, 2023 at 06:53:20PM -0300, Fabiano Rosas wrote:
> >> > >> Peter Xu writes:
> >> > >>
> >> > >> > On Mon, Sep 11, 2023 at 02:13:19PM -0300, Fabiano Rosas wrote:
> >> > >> >> The core yank code is strict about balanced registering and
> >> > >> >> unregistering of yank functions.
> >> > >> >>
> >> > >> >> This creates a difficulty because the migration code registers one
> >> > >> >> yank function per QIOChannel, but each QIOChannel can be
> >> > >> >> referenced by
> >> > >> >> more than one QEMUFile. The yank function should not be removed
> >> > >> >> until
> >> > >> >> all QEMUFiles have been closed.
> >> > >> >>
> >> > >> >> Keep a reference count of how many QEMUFiles are using a QIOChannel
> >> > >> >> that has a yank function. Only unregister the yank function when
> >> > >> >> all
> >> > >> >> QEMUFiles have been closed.
> >> > >> >>
> >> > >> >> This improves the current code by removing the need for the
> >> > >> >> programmer
> >> > >> >> to know which QEMUFile is the last one to be cleaned up and fixes
> >> > >> >> the
> >> > >> >> theoretical issue of removing the yank function while another
> >> > >> >> QEMUFile
> >> > >> >> could still be using the ioc and require a yank.
> >> > >> >>
> >> > >> >> Signed-off-by: Fabiano Rosas
> >> > >> >> ---
> >> > >> >> migration/yank_functions.c | 81
> >> > >> >> ++
> >> > >> >> migration/yank_functions.h | 8
> >> > >> >> 2 files changed, 81 insertions(+), 8 deletions(-)
> >> > >> >
> >> > >> > I worry this over-complicate things.
> >> > >>
> >> > >> It does. We ran out of simple options.
> >> > >>
> >> > >> > If you prefer the cleaness that we operate always on qemufile
> >> > >> > level, can we
> >> > >> > just register each yank function per-qemufile?
> >> > >>
> >> > >> "just" hehe
> >> > >>
> >> > >> we could, but:
> >> > >>
> >> > >> i) the yank is a per-channel operation, so this is even more
> >> > >> unintuitive;
> >> > >
> >> > > I mean we can provide something like:
> >> > >
> >> > > void migration_yank_qemufile(void *opaque)
> >> > > {
> >> > > QEMUFile *file = opaque;
> >> > > QIOChannel *ioc = file->ioc;
> >> > >
> >> > > qio_channel_shutdown(ioc, QIO_CHANNEL_SHUTDOWN_BOTH, NULL);
> >> > > }
> >> > >
> >> > > void migration_qemufile_register_yank(QEMUFile *file)
> >> > > {
> >> > > if (migration_ioc_yank_supported(file->ioc)) {
> >> > > yank_register_function(MIGRATION_YANK_INSTANCE,
> >> > >migration_yank_qemufile,
> >> > >file);
> >> > > }
> >> > > }
> >> >
> >> > Sure, this is what I was thinking as well. IMO it will be yet another
> >> > operation that happens on the channel, but it performed via the
> >> > file. Just like qio_channel_close() at qemu_fclose(). Not the end of the
> >> > world, of course, I just find it error-prone.
> >> >
> >> > >>
> >> > >> ii) multifd doesn't have a QEMUFile, so it will have to continue using
> >> > >> the ioc;
> >> > >
> >> > > We
Re: [PATCH v6 09/10] migration/yank: Keep track of registered yank instances
On Thu, 14 Sep 2023 10:57:47 -0400
Peter Xu wrote:
> On Thu, Sep 14, 2023 at 10:23:38AM -0300, Fabiano Rosas wrote:
> > Peter Xu writes:
> >
> > > On Wed, Sep 13, 2023 at 06:53:20PM -0300, Fabiano Rosas wrote:
> > >> Peter Xu writes:
> > >>
> > >> > On Mon, Sep 11, 2023 at 02:13:19PM -0300, Fabiano Rosas wrote:
> > >> >> The core yank code is strict about balanced registering and
> > >> >> unregistering of yank functions.
> > >> >>
> > >> >> This creates a difficulty because the migration code registers one
> > >> >> yank function per QIOChannel, but each QIOChannel can be referenced by
> > >> >> more than one QEMUFile. The yank function should not be removed until
> > >> >> all QEMUFiles have been closed.
> > >> >>
> > >> >> Keep a reference count of how many QEMUFiles are using a QIOChannel
> > >> >> that has a yank function. Only unregister the yank function when all
> > >> >> QEMUFiles have been closed.
> > >> >>
> > >> >> This improves the current code by removing the need for the programmer
> > >> >> to know which QEMUFile is the last one to be cleaned up and fixes the
> > >> >> theoretical issue of removing the yank function while another QEMUFile
> > >> >> could still be using the ioc and require a yank.
> > >> >>
> > >> >> Signed-off-by: Fabiano Rosas
> > >> >> ---
> > >> >> migration/yank_functions.c | 81
> > >> >> ++
> > >> >> migration/yank_functions.h | 8
> > >> >> 2 files changed, 81 insertions(+), 8 deletions(-)
> > >> >
> > >> > I worry this over-complicate things.
> > >>
> > >> It does. We ran out of simple options.
> > >>
> > >> > If you prefer the cleaness that we operate always on qemufile level,
> > >> > can we
> > >> > just register each yank function per-qemufile?
> > >>
> > >> "just" hehe
> > >>
> > >> we could, but:
> > >>
> > >> i) the yank is a per-channel operation, so this is even more
> > >> unintuitive;
> > >
> > > I mean we can provide something like:
> > >
> > > void migration_yank_qemufile(void *opaque)
> > > {
> > > QEMUFile *file = opaque;
> > > QIOChannel *ioc = file->ioc;
> > >
> > > qio_channel_shutdown(ioc, QIO_CHANNEL_SHUTDOWN_BOTH, NULL);
> > > }
> > >
> > > void migration_qemufile_register_yank(QEMUFile *file)
> > > {
> > > if (migration_ioc_yank_supported(file->ioc)) {
> > > yank_register_function(MIGRATION_YANK_INSTANCE,
> > >migration_yank_qemufile,
> > >file);
> > > }
> > > }
> >
> > Sure, this is what I was thinking as well. IMO it will be yet another
> > operation that happens on the channel, but it performed via the
> > file. Just like qio_channel_close() at qemu_fclose(). Not the end of the
> > world, of course, I just find it error-prone.
> >
> > >>
> > >> ii) multifd doesn't have a QEMUFile, so it will have to continue using
> > >> the ioc;
> > >
> > > We can keep using migration_ioc_[un]register_yank() for them if there's no
> > > qemufile attached. As long as the function will all be registered under
> > > MIGRATION_YANK_INSTANCE we should be fine having different yank func.
> > >
> >
> > ok
> >
> > >>
> > >> iii) we'll have to add a yank to every new QEMUFile created during the
> > >> incoming migration (colo, rdma, etc), otherwise the incoming side
> > >> will be left using iocs while the src uses the QEMUFile;
> > >
> > > For RDMA, IIUC it'll simply be a noop as migration_ioc_yank_supported()
> > > will be a noop for it for either reg/unreg.
> > >
> > > Currently it seems we will also unreg the ioc even for RDMA (even though
> > > we
> > > don't reg for it). But since unreg will be a noop it seems all fine even
> > > if not paired.. maybe we should still try to pair it, e.g. register also
> > > in
> > > rdma_start_outgoing_migration() for the rdma ioc so at least they're
> > > paired.
> > >
> > > I don't see why COLO is special here, though. Maybe I missed something.
> >
> > For colo I was thinking we'd have to register the yank just to be sure
> > that all paths unregistering it have something to unregister.
> >
> > Maybe I should move the register into qemu_file_new_impl() with a
> > matching unregister at qemu_fclose().
>
> Sounds good. Or...
>
> >
> > >>
> > >> iv) this is a functional change of the yank feature for which we have no
> > >> tests.
> > >
> > > Having yank tested should be preferrable. Lukas is in the loop, let's see
> > > whether he has something. We can still smoke test it before a selftest
> > > being there.
> > >
Hi All,
Sorry for the late reply.
Yes, testing missing. I'll work on it.
> > > Taking one step back.. I doubt whether anyone is using yank for migration?
> > > Knowing that migration already have migrate-cancel (for precopy) and
> > > migrate-pause (for postcopy).
> >
> > Right, both already call qio_channel_shutdown().
> >
> > > I never used it myself, and I don't think
> > > it's
Re: Tips for local testing guestfwd
CC'ing SLIRP and net maintainer. On Sun, 25 Jun 2023 22:58:36 -0700 Felix Wu wrote: > Hi all, > > TL,DR: I am working on QEMU ipv6 guestfwd feature and finished coding, and > would like to learn the best practice to test it. > Context: in slirp side this task is tracking by [1]. > Currently, I have done following: > i. made char parse + guestfwd functions happy with ipv6 address. > ii. enabled debug print and made sure the ip and port are inserted into the > forward list in libslirp. > To sufficiently verify it's working, I do have three questions: > 1. I want to forward a port in the guest (OS in QEMU) to a port 22 on the > host OS, and ssh from guest back to host, > does this sound reasonable? If this is not a good idea, what method is > recommended? > 2. I want to understand what ip I should use. Currently I have following > formats for the QEMU invocation in ipv6: > ``` > guestfwd=tcp:[::1]:1234-tcp:[my:host:ip:from:ifconfig]:22 > ``` > I know the general form is `guestfwd=tcp:server:port-dev`, where > server:port is for guest, dev is for host. > Adding [] in my implementation will let QEMU know it's ipv6. > Is the aforementioned invocation correct? Or in this case [::1] is the > local host address and I should put qemu address > for it instead? > 3. Is there a default ipv6 address for QEMU instance? I think I need it in > the invocation. > > Thanks in advance! Felix > > [1]. https://gitlab.freedesktop.org/slirp/libslirp/-/issues/67 pgp7peayMUmmF.pgp Description: OpenPGP digital signature
[PATCH 6/8] colo: Reject colo with postcopy capability enabled
Signed-off-by: Lukas Straub
---
migration/migration.c | 5 +++--
migration/options.c | 9 +
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index a954ff4f7d..9860f960f2 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2333,9 +2333,10 @@ static void migration_completion(MigrationState *s)
goto fail;
}
-if (migrate_colo() && s->state == MIGRATION_STATUS_ACTIVE) {
+if (migrate_colo()) {
/* COLO does not support postcopy */
-migrate_set_state(>state, MIGRATION_STATUS_ACTIVE,
+assert(s->state != MIGRATION_STATUS_POSTCOPY_ACTIVE);
+migrate_set_state(>state, current_active_state,
MIGRATION_STATUS_COLO);
} else {
migrate_set_state(>state, current_active_state,
diff --git a/migration/options.c b/migration/options.c
index b62ab30cd5..d3d4525d40 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -445,8 +445,17 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps,
Error **errp)
error_append_hint(errp, "Please enable replication before COLO.\n");
return false;
}
+#else
+if (new_caps[MIGRATION_CAPABILITY_X_COLO]) {
+if (new_caps[MIGRATION_CAPABILITY_POSTCOPY_RAM]) {
+error_setg(errp, "COLO is not compatible with postcopy");
+return false;
+}
+}
#endif
+
+
if (new_caps[MIGRATION_CAPABILITY_POSTCOPY_RAM]) {
/* This check is reasonably expensive, so only when it's being
* set the first time, also it's only the destination that needs
--
2.39.2
pgpIIW_MCwLei.pgp
Description: OpenPGP digital signature
[PATCH 5/8] colo: Don't send ENABLE_COLO command
We should only migrate to qemu with newer version, and there it's
not needed anymore.
Signed-off-by: Lukas Straub
---
migration/migration.c | 5 -
migration/savevm.c | 6 --
migration/savevm.h | 1 -
migration/trace-events | 1 -
4 files changed, 13 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 1d347533f9..a954ff4f7d 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2929,11 +2929,6 @@ static void *migration_thread(void *opaque)
qemu_savevm_send_postcopy_advise(s->to_dst_file);
}
-if (migrate_colo()) {
-/* Notify migration destination that we enable COLO */
-qemu_savevm_send_colo_enable(s->to_dst_file);
-}
-
qemu_savevm_state_setup(s->to_dst_file);
qemu_savevm_wait_unplug(s, MIGRATION_STATUS_SETUP,
diff --git a/migration/savevm.c b/migration/savevm.c
index 3a1de15bd0..5986f852b2 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1035,12 +1035,6 @@ static void qemu_savevm_command_send(QEMUFile *f,
qemu_fflush(f);
}
-void qemu_savevm_send_colo_enable(QEMUFile *f)
-{
-trace_savevm_send_colo_enable();
-qemu_savevm_command_send(f, MIG_CMD_ENABLE_COLO, 0, NULL);
-}
-
void qemu_savevm_send_ping(QEMUFile *f, uint32_t value)
{
uint32_t buf;
diff --git a/migration/savevm.h b/migration/savevm.h
index fb636735f0..cb3318e9e2 100644
--- a/migration/savevm.h
+++ b/migration/savevm.h
@@ -57,7 +57,6 @@ void qemu_savevm_send_postcopy_ram_discard(QEMUFile *f, const
char *name,
uint16_t len,
uint64_t *start_list,
uint64_t *length_list);
-void qemu_savevm_send_colo_enable(QEMUFile *f);
void qemu_savevm_live_state(QEMUFile *f);
int qemu_save_device_state(QEMUFile *f);
diff --git a/migration/trace-events b/migration/trace-events
index cdaef7a1ea..bd3ec40e31 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -36,7 +36,6 @@ savevm_send_ping(uint32_t val) "0x%x"
savevm_send_postcopy_listen(void) ""
savevm_send_postcopy_run(void) ""
savevm_send_postcopy_resume(void) ""
-savevm_send_colo_enable(void) ""
savevm_send_recv_bitmap(char *name) "%s"
savevm_state_setup(void) ""
savevm_state_resume_prepare(void) ""
--
2.39.2
pgpu0tet5w3yp.pgp
Description: OpenPGP digital signature
[PATCH 8/8] ram: Remove useless colo special-casing
This is not needed, as colo primary side everything is more or less
a new migration for every checkpoint.
Also, we only enter colo mode after the precopy migration is finished
so this if is always taken. Still add an assert just in case.
Signed-off-by: Lukas Straub
---
migration/ram.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index e3eadd08cd..e5c1146360 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2994,17 +2994,17 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
RAMBlock *block;
int ret;
+assert(!migration_in_colo_state());
+
if (compress_threads_save_setup()) {
return -1;
}
-/* migration has already setup the bitmap, reuse it. */
-if (!migration_in_colo_state()) {
-if (ram_init_all(rsp) != 0) {
-compress_threads_save_cleanup();
-return -1;
-}
+if (ram_init_all(rsp) != 0) {
+compress_threads_save_cleanup();
+return -1;
}
+
(*rsp)->pss[RAM_CHANNEL_PRECOPY].pss_channel = f;
WITH_RCU_READ_LOCK_GUARD() {
--
2.39.2
pgp9IHHnYXBdP.pgp
Description: OpenPGP digital signature
[PATCH 3/8] colo: Replace migration_incoming_colo_enabled() with migrate_colo()
Signed-off-by: Lukas Straub
---
include/migration/colo.h | 1 -
migration/colo.c | 2 +-
migration/migration.c| 7 +--
migration/ram.c | 2 +-
4 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/include/migration/colo.h b/include/migration/colo.h
index eaac07f26d..4a1955067b 100644
--- a/include/migration/colo.h
+++ b/include/migration/colo.h
@@ -27,7 +27,6 @@ bool migration_in_colo_state(void);
/* loadvm */
int migration_incoming_enable_colo(void);
void migration_incoming_disable_colo(void);
-bool migration_incoming_colo_enabled(void);
bool migration_incoming_in_colo_state(void);
COLOMode get_colo_mode(void);
diff --git a/migration/colo.c b/migration/colo.c
index 72f4f7b37e..8b0e7c9af3 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -927,7 +927,7 @@ int coroutine_fn colo_incoming_co(void)
assert(qemu_mutex_iothread_locked());
-if (!migration_incoming_colo_enabled()) {
+if (!migrate_colo()) {
return 0;
}
diff --git a/migration/migration.c b/migration/migration.c
index 050bd8ffc8..2506fd63f7 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -379,11 +379,6 @@ int migrate_send_rp_req_pages(MigrationIncomingState *mis,
}
static bool migration_colo_enabled;
-bool migration_incoming_colo_enabled(void)
-{
-return migration_colo_enabled;
-}
-
void migration_incoming_disable_colo(void)
{
ram_block_discard_disable(false);
@@ -484,7 +479,7 @@ static void process_incoming_migration_bh(void *opaque)
} else {
runstate_set(RUN_STATE_PAUSED);
}
-} else if (migration_incoming_colo_enabled()) {
+} else if (migrate_colo()) {
migration_incoming_disable_colo();
vm_start();
} else {
diff --git a/migration/ram.c b/migration/ram.c
index 5283a75f02..e3eadd08cd 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3892,7 +3892,7 @@ static int ram_load_precopy(QEMUFile *f)
* speed of the migration, but it obviously reduce the downtime of
* back-up all SVM'S memory in COLO preparing stage.
*/
-if (migration_incoming_colo_enabled()) {
+if (migrate_colo()) {
if (migration_incoming_in_colo_state()) {
/* In COLO stage, put all pages into cache temporarily */
host = colo_cache_from_block_offset(block, addr, true);
--
2.39.2
pgphszwTyTASq.pgp
Description: OpenPGP digital signature
[PATCH 7/8] colo: Reject colo with block migration capability enabled
Signed-off-by: Lukas Straub
---
migration/migration.c | 4
migration/options.c | 5 +
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 9860f960f2..270130579f 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1571,10 +1571,6 @@ static bool migrate_prepare(MigrationState *s, bool blk,
bool blk_inc,
}
if (blk || blk_inc) {
-if (migrate_colo()) {
-error_setg(errp, "No disk migration is required in COLO mode");
-return false;
-}
if (migrate_block() || migrate_block_incremental()) {
error_setg(errp, "Command options are incompatible with "
"current migration capabilities");
diff --git a/migration/options.c b/migration/options.c
index d3d4525d40..1e9659fcb3 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -451,6 +451,11 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps,
Error **errp)
error_setg(errp, "COLO is not compatible with postcopy");
return false;
}
+
+if (new_caps[MIGRATION_CAPABILITY_BLOCK]) {
+error_setg(errp, "COLO is not compatible with block migration");
+return false;
+}
}
#endif
--
2.39.2
pgpY2K49RL2ZB.pgp
Description: OpenPGP digital signature
[PATCH 4/8] colo: Remove ENABLE_COLO loadvm command functions
No need for it anymore now that x-colo capability is required
on incoming side. Still accept the command as noop, for
compatibility with older qemu.
Signed-off-by: Lukas Straub
---
include/migration/colo.h | 2 --
migration/migration.c| 26 --
migration/savevm.c | 7 +--
3 files changed, 1 insertion(+), 34 deletions(-)
diff --git a/include/migration/colo.h b/include/migration/colo.h
index 4a1955067b..addbc24fcf 100644
--- a/include/migration/colo.h
+++ b/include/migration/colo.h
@@ -25,8 +25,6 @@ void migrate_start_colo_process(MigrationState *s);
bool migration_in_colo_state(void);
/* loadvm */
-int migration_incoming_enable_colo(void);
-void migration_incoming_disable_colo(void);
bool migration_incoming_in_colo_state(void);
COLOMode get_colo_mode(void);
diff --git a/migration/migration.c b/migration/migration.c
index 2506fd63f7..1d347533f9 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -378,31 +378,6 @@ int migrate_send_rp_req_pages(MigrationIncomingState *mis,
return migrate_send_rp_message_req_pages(mis, rb, start);
}
-static bool migration_colo_enabled;
-void migration_incoming_disable_colo(void)
-{
-ram_block_discard_disable(false);
-migration_colo_enabled = false;
-}
-
-int migration_incoming_enable_colo(void)
-{
-#ifndef CONFIG_REPLICATION
-error_report("ENABLE_COLO command come in migration stream, but COLO "
- "module is not built in");
-return -ENOTSUP;
-#endif
-
-if (!migrate_colo()) {
-error_report("ENABLE_COLO command come in migration stream, but c-colo
"
- "capability is not set");
-return -EINVAL;
-}
-
-migration_colo_enabled = true;
-return 0;
-}
-
void migrate_add_address(SocketAddress *address)
{
MigrationIncomingState *mis = migration_incoming_get_current();
@@ -480,7 +455,6 @@ static void process_incoming_migration_bh(void *opaque)
runstate_set(RUN_STATE_PAUSED);
}
} else if (migrate_colo()) {
-migration_incoming_disable_colo();
vm_start();
} else {
runstate_set(global_state_get_runstate());
diff --git a/migration/savevm.c b/migration/savevm.c
index 155abb0fda..3a1de15bd0 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2300,11 +2300,6 @@ static int
loadvm_handle_recv_bitmap(MigrationIncomingState *mis,
return 0;
}
-static int loadvm_process_enable_colo(MigrationIncomingState *mis)
-{
-return migration_incoming_enable_colo();
-}
-
/*
* Process an incoming 'QEMU_VM_COMMAND'
* 0 just a normal return
@@ -2387,7 +2382,7 @@ static int loadvm_process_command(QEMUFile *f)
return loadvm_handle_recv_bitmap(mis, len);
case MIG_CMD_ENABLE_COLO:
-return loadvm_process_enable_colo(mis);
+return 0;
}
return 0;
--
2.39.2
pgphOc2njFMxw.pgp
Description: OpenPGP digital signature
[PATCH 2/8] colo: Setup ram cache in normal migration path
Now that x-colo capability needs to be always enabled on the
incoming side we can use that to initialize the ram cache in
the normal migration path.
Signed-off-by: Lukas Straub
---
migration/migration.c | 16
migration/savevm.c| 10 +-
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index dc05c6f6ea..050bd8ffc8 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -404,10 +404,6 @@ int migration_incoming_enable_colo(void)
return -EINVAL;
}
-if (ram_block_discard_disable(true)) {
-error_report("COLO: cannot disable RAM discard");
-return -EBUSY;
-}
migration_colo_enabled = true;
return 0;
}
@@ -519,6 +515,18 @@ process_incoming_migration_co(void *opaque)
goto fail;
}
+if (migrate_colo()) {
+if (ram_block_discard_disable(true)) {
+error_report("COLO: cannot disable RAM discard");
+goto fail;
+}
+
+if (colo_init_ram_cache() < 0) {
+error_report("Init ram cache failed");
+goto fail;
+}
+}
+
mis->largest_page_size = qemu_ram_pagesize_largest();
postcopy_state_set(POSTCOPY_INCOMING_NONE);
migrate_set_state(>state, MIGRATION_STATUS_NONE,
diff --git a/migration/savevm.c b/migration/savevm.c
index bc284087f9..155abb0fda 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2302,15 +2302,7 @@ static int
loadvm_handle_recv_bitmap(MigrationIncomingState *mis,
static int loadvm_process_enable_colo(MigrationIncomingState *mis)
{
-int ret = migration_incoming_enable_colo();
-
-if (!ret) {
-ret = colo_init_ram_cache();
-if (ret) {
-migration_incoming_disable_colo();
-}
-}
-return ret;
+return migration_incoming_enable_colo();
}
/*
--
2.39.2
pgpdfT0wpZtDw.pgp
Description: OpenPGP digital signature
[PATCH 0/8] colo migration cleanups
Hello Everyone, Here are some cleanups around the colo migration code. Notably, we only support the same qemu version on both sides for now. Though in this patchset I still try to be support migration to a newer qemu version. Lukas Straub (8): colo: Only support the same qemu version on source and destination colo: Setup ram cache in normal migration path colo: Replace migration_incoming_colo_enabled() with migrate_colo() colo: Remove ENABLE_COLO loadvm command functions colo: Don't send ENABLE_COLO command colo: Reject colo with postcopy capability enabled colo: Reject colo with block migration capability enabled ram: Remove useless colo special-casing docs/COLO-FT.txt | 2 ++ include/migration/colo.h | 3 -- migration/colo.c | 2 +- migration/migration.c| 63 ++-- migration/options.c | 14 + migration/ram.c | 14 - migration/savevm.c | 21 +- migration/savevm.h | 1 - migration/trace-events | 1 - 9 files changed, 41 insertions(+), 80 deletions(-) -- 2.39.2 pgpoWWYDRMi4v.pgp Description: OpenPGP digital signature
[PATCH 1/8] colo: Only support the same qemu version on source and destination
Signed-off-by: Lukas Straub --- docs/COLO-FT.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/COLO-FT.txt b/docs/COLO-FT.txt index 2e760a4aee..8e64480dbd 100644 --- a/docs/COLO-FT.txt +++ b/docs/COLO-FT.txt @@ -148,6 +148,8 @@ in test procedure. Note: Here we are running both instances on the same host for testing, change the IP Addresses if you want to run it on two hosts. Initially 127.0.0.1 is the Primary Host and 127.0.0.2 is the Secondary Host. +Note: COLO is a experimental feature, so currently is should only be used +with the same qemu version on sourcee and target. == Startup qemu == 1. Primary: -- 2.39.2 pgpJGsWLsZtng.pgp Description: OpenPGP digital signature
Re: [PATCH 18/18] migration: Remove not needed file parameter for save_zero_page*
On Tue, 13 Jun 2023 16:57:57 +0200
Juan Quintela wrote:
> Signed-off-by: Juan Quintela
Reviewed-by: Lukas Straub
> ---
> migration/ram.c | 13 ++---
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/migration/ram.c b/migration/ram.c
> index 0259c33da7..ae11d0e992 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -1069,16 +1069,15 @@ void ram_release_page(const char *rbname, uint64_t
> offset)
> * @pss: current PSS channel
> * @offset: offset inside the block for the page
> */
> -static int save_zero_page_to_file(PageSearchStatus *pss, QEMUFile *file,
> - ram_addr_t offset)
> +static int save_zero_page_to_file(PageSearchStatus *pss, ram_addr_t offset)
> {
> uint8_t *p = pss->block->host + offset;
> int len = 0;
>
> if (buffer_is_zero(p, TARGET_PAGE_SIZE)) {
> -len += save_page_header(pss, file, pss->block,
> +len += save_page_header(pss, pss->pss_channel, pss->block,
> offset | RAM_SAVE_FLAG_ZERO);
> -qemu_put_byte(file, 0);
> +qemu_put_byte(pss->pss_channel, 0);
> len += 1;
> ram_release_page(pss->block->idstr, offset);
> }
> @@ -1093,9 +1092,9 @@ static int save_zero_page_to_file(PageSearchStatus
> *pss, QEMUFile *file,
> * @pss: current PSS channel
> * @offset: offset inside the block for the page
> */
> -static int save_zero_page(PageSearchStatus *pss, QEMUFile *f, ram_addr_t
> offset)
> +static int save_zero_page(PageSearchStatus *pss, ram_addr_t offset)
> {
> -int len = save_zero_page_to_file(pss, f, offset);
> +int len = save_zero_page_to_file(pss, offset);
>
> if (len) {
> stat64_add(_stats.zero_pages, 1);
> @@ -2031,7 +2030,7 @@ static int ram_save_target_page_legacy(RAMState *rs,
> PageSearchStatus *pss)
> return 1;
> }
>
> -res = save_zero_page(pss, pss->pss_channel, offset);
> +res = save_zero_page(pss, offset);
> if (res > 0) {
> /* Must let xbzrle know, otherwise a previous (now 0'd) cached
> * page would be stale
pgpxSHMFFFx5H.pgp
Description: OpenPGP digital signature
Re: [PATCH 17/18] migration: Remove not needed block parameter for save_zero_page*
On Tue, 13 Jun 2023 16:57:56 +0200
Juan Quintela wrote:
> Signed-off-by: Juan Quintela
Reviewed-by: Lukas Straub
> ---
> migration/ram.c | 18 --
> 1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/migration/ram.c b/migration/ram.c
> index 293ea87a70..0259c33da7 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -1067,20 +1067,20 @@ void ram_release_page(const char *rbname, uint64_t
> offset)
> * a zero page
> *
> * @pss: current PSS channel
> - * @block: block that contains the page we want to send
> * @offset: offset inside the block for the page
> */
> static int save_zero_page_to_file(PageSearchStatus *pss, QEMUFile *file,
> - RAMBlock *block, ram_addr_t offset)
> + ram_addr_t offset)
> {
> -uint8_t *p = block->host + offset;
> +uint8_t *p = pss->block->host + offset;
> int len = 0;
>
> if (buffer_is_zero(p, TARGET_PAGE_SIZE)) {
> -len += save_page_header(pss, file, block, offset |
> RAM_SAVE_FLAG_ZERO);
> +len += save_page_header(pss, file, pss->block,
> +offset | RAM_SAVE_FLAG_ZERO);
> qemu_put_byte(file, 0);
> len += 1;
> -ram_release_page(block->idstr, offset);
> +ram_release_page(pss->block->idstr, offset);
> }
> return len;
> }
> @@ -1091,13 +1091,11 @@ static int save_zero_page_to_file(PageSearchStatus
> *pss, QEMUFile *file,
> * Returns the number of pages written.
> *
> * @pss: current PSS channel
> - * @block: block that contains the page we want to send
> * @offset: offset inside the block for the page
> */
> -static int save_zero_page(PageSearchStatus *pss, QEMUFile *f, RAMBlock
> *block,
> - ram_addr_t offset)
> +static int save_zero_page(PageSearchStatus *pss, QEMUFile *f, ram_addr_t
> offset)
> {
> -int len = save_zero_page_to_file(pss, f, block, offset);
> +int len = save_zero_page_to_file(pss, f, offset);
>
> if (len) {
> stat64_add(_stats.zero_pages, 1);
> @@ -2033,7 +2031,7 @@ static int ram_save_target_page_legacy(RAMState *rs,
> PageSearchStatus *pss)
> return 1;
> }
>
> -res = save_zero_page(pss, pss->pss_channel, block, offset);
> +res = save_zero_page(pss, pss->pss_channel, offset);
> if (res > 0) {
> /* Must let xbzrle know, otherwise a previous (now 0'd) cached
> * page would be stale
pgpkKep3QsVzA.pgp
Description: OpenPGP digital signature
Re: [PATCH 16/18] migration: control_save_page() can take block through pss
On Tue, 13 Jun 2023 16:57:55 +0200
Juan Quintela wrote:
> Signed-off-by: Juan Quintela
Reviewed-by: Lukas Straub
> ---
> migration/ram.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/migration/ram.c b/migration/ram.c
> index e3ff98a6ce..293ea87a70 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -1114,14 +1114,14 @@ static int save_zero_page(PageSearchStatus *pss,
> QEMUFile *f, RAMBlock *block,
> *
> * Return true if the pages has been saved, otherwise false is returned.
> */
> -static bool control_save_page(PageSearchStatus *pss, RAMBlock *block,
> +static bool control_save_page(PageSearchStatus *pss,
>ram_addr_t offset, int *pages)
> {
> uint64_t bytes_xmit = 0;
> int ret;
>
> *pages = -1;
> -ret = ram_control_save_page(pss->pss_channel, block->offset, offset,
> +ret = ram_control_save_page(pss->pss_channel, pss->block->offset, offset,
> TARGET_PAGE_SIZE, _xmit);
> if (ret == RAM_SAVE_CONTROL_NOT_SUPP) {
> return false;
> @@ -2025,7 +2025,7 @@ static int ram_save_target_page_legacy(RAMState *rs,
> PageSearchStatus *pss)
> ram_addr_t offset = ((ram_addr_t)pss->page) << TARGET_PAGE_BITS;
> int res;
>
> -if (control_save_page(pss, block, offset, )) {
> +if (control_save_page(pss, offset, )) {
> return res;
> }
>
Re: [PATCH 15/18] migration: save_compress_page() can take block through pss
On Tue, 13 Jun 2023 16:57:54 +0200
Juan Quintela wrote:
> Signed-off-by: Juan Quintela
Reviewed-by: Lukas Straub
> ---
> migration/ram.c | 9 +
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/migration/ram.c b/migration/ram.c
> index 1bd586c23a..e3ff98a6ce 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -1986,7 +1986,7 @@ int ram_save_queue_pages(const char *rbname, ram_addr_t
> start, ram_addr_t len)
> * paths to handle it
> */
> static bool save_compress_page(RAMState *rs, PageSearchStatus *pss,
> - RAMBlock *block, ram_addr_t offset)
> + ram_addr_t offset)
> {
> if (!migrate_compress()) {
> return false;
> @@ -2002,12 +2002,13 @@ static bool save_compress_page(RAMState *rs,
> PageSearchStatus *pss,
> * We post the fist page as normal page as compression will take
> * much CPU resource.
> */
> -if (block != pss->last_sent_block) {
> +if (pss->block != pss->last_sent_block) {
> ram_flush_compressed_data();
> return false;
> }
>
> -return compress_page_with_multi_thread(block, offset, send_queued_data);
> +return compress_page_with_multi_thread(pss->block, offset,
> + send_queued_data);
> }
>
> /**
> @@ -2028,7 +2029,7 @@ static int ram_save_target_page_legacy(RAMState *rs,
> PageSearchStatus *pss)
> return res;
> }
>
> -if (save_compress_page(rs, pss, block, offset)) {
> +if (save_compress_page(rs, pss, offset)) {
> return 1;
> }
>
pgp46CodrEjPg.pgp
Description: OpenPGP digital signature
Re: [PATCH 14/18] migration: Use "i" as an for index in ram-compress.c
On Tue, 13 Jun 2023 16:57:53 +0200
Juan Quintela wrote:
> It is used everywhere else in C. Once there, make sure that we don't
> use the index outside of the for declaring the variable there.
>
> Signed-off-by: Juan Quintela
Reviewed-by: Lukas Straub
> ---
> migration/ram-compress.c | 57 ++--
> 1 file changed, 25 insertions(+), 32 deletions(-)
>
> diff --git a/migration/ram-compress.c b/migration/ram-compress.c
> index cf3a6c238d..a23531606e 100644
> --- a/migration/ram-compress.c
> +++ b/migration/ram-compress.c
> @@ -243,27 +243,25 @@ static inline void compress_reset_result(CompressParam
> *param)
>
> void flush_compressed_data(int (send_queued_data(CompressParam *)))
> {
> -int idx, thread_count;
> -
> -thread_count = migrate_compress_threads();
> +int thread_count = migrate_compress_threads();
>
> qemu_mutex_lock(_done_lock);
> -for (idx = 0; idx < thread_count; idx++) {
> -while (!comp_param[idx].done) {
> +for (int i = 0; i < thread_count; i++) {
> +while (!comp_param[i].done) {
> qemu_cond_wait(_done_cond, _done_lock);
> }
> }
> qemu_mutex_unlock(_done_lock);
>
> -for (idx = 0; idx < thread_count; idx++) {
> -qemu_mutex_lock(_param[idx].mutex);
> -if (!comp_param[idx].quit) {
> -CompressParam *param = _param[idx];
> +for (int i = 0; i < thread_count; i++) {
> +qemu_mutex_lock(_param[i].mutex);
> +if (!comp_param[i].quit) {
> +CompressParam *param = _param[i];
> send_queued_data(param);
> assert(qemu_file_buffer_empty(param->file));
> compress_reset_result(param);
> }
> -qemu_mutex_unlock(_param[idx].mutex);
> +qemu_mutex_unlock(_param[i].mutex);
> }
> }
>
> @@ -281,16 +279,15 @@ static inline void set_compress_params(CompressParam
> *param, RAMBlock *block,
> bool compress_page_with_multi_thread(RAMBlock *block, ram_addr_t offset,
> int (send_queued_data(CompressParam *)))
> {
> -int idx, thread_count;
> bool wait = migrate_compress_wait_thread();
>
> -thread_count = migrate_compress_threads();
> +int thread_count = migrate_compress_threads();
> qemu_mutex_lock(_done_lock);
>
> while (true) {
> -for (idx = 0; idx < thread_count; idx++) {
> -if (comp_param[idx].done) {
> -CompressParam *param = _param[idx];
> +for (int i = 0; i < thread_count; i++) {
> +if (comp_param[i].done) {
> +CompressParam *param = _param[i];
> qemu_mutex_lock(>mutex);
> param->done = false;
> send_queued_data(param);
> @@ -384,16 +381,14 @@ static void *do_data_decompress(void *opaque)
>
> int wait_for_decompress_done(void)
> {
> -int idx, thread_count;
> -
> if (!migrate_compress()) {
> return 0;
> }
>
> -thread_count = migrate_decompress_threads();
> +int thread_count = migrate_decompress_threads();
> qemu_mutex_lock(_done_lock);
> -for (idx = 0; idx < thread_count; idx++) {
> -while (!decomp_param[idx].done) {
> +for (int i = 0; i < thread_count; i++) {
> +while (!decomp_param[i].done) {
> qemu_cond_wait(_done_cond, _done_lock);
> }
> }
> @@ -484,20 +479,18 @@ exit:
>
> void decompress_data_with_multi_threads(QEMUFile *f, void *host, int len)
> {
> -int idx, thread_count;
> -
> -thread_count = migrate_decompress_threads();
> +int thread_count = migrate_decompress_threads();
> QEMU_LOCK_GUARD(_done_lock);
> while (true) {
> -for (idx = 0; idx < thread_count; idx++) {
> -if (decomp_param[idx].done) {
> -decomp_param[idx].done = false;
> -qemu_mutex_lock(_param[idx].mutex);
> -qemu_get_buffer(f, decomp_param[idx].compbuf, len);
> -decomp_param[idx].des = host;
> -decomp_param[idx].len = len;
> -qemu_cond_signal(_param[idx].cond);
> -qemu_mutex_unlock(_param[idx].mutex);
> +for (int i = 0; i < thread_count; i++) {
> +if (decomp_param[i].done) {
> +decomp_param[i].done = false;
> +qemu_mutex_lock(_param[i].mutex);
> +qemu_get_buffer(f, decomp_param[i].compbuf, len);
> +decomp_param[i].des = host;
> +decomp_param[i].len = len;
> +qemu_cond_signal(_param[i].cond);
> +qemu_mutex_unlock(_param[i].mutex);
> return;
> }
> }
pgpdSc1LF1MWx.pgp
Description: OpenPGP digital signature
Re: [PATCH 13/18] migration: Simplify decompress_data_with_multi_threads()
On Tue, 13 Jun 2023 16:57:52 +0200
Juan Quintela wrote:
> Doing a break to do another break is just confused. Just call return
> when we know we want to return.
>
> Signed-off-by: Juan Quintela
Reviewed-by: Lukas Straub
> ---
> migration/ram-compress.c | 8 ++--
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/migration/ram-compress.c b/migration/ram-compress.c
> index 5e1bb82fcd..cf3a6c238d 100644
> --- a/migration/ram-compress.c
> +++ b/migration/ram-compress.c
> @@ -498,14 +498,10 @@ void decompress_data_with_multi_threads(QEMUFile *f,
> void *host, int len)
> decomp_param[idx].len = len;
> qemu_cond_signal(_param[idx].cond);
> qemu_mutex_unlock(_param[idx].mutex);
> -break;
> +return;
> }
> }
> -if (idx < thread_count) {
> -break;
> -} else {
> -qemu_cond_wait(_done_cond, _done_lock);
> -}
> +qemu_cond_wait(_done_cond, _done_lock);
> }
> }
>
pgp6BJUA6IaB5.pgp
Description: OpenPGP digital signature
Re: [PATCH 12/18] migration: Create compression_update_rates()
On Tue, 13 Jun 2023 16:57:51 +0200
Juan Quintela wrote:
> So we can move more compression_counters stuff to ram-compress.c.
> Create compression_counters struct to add the stuff that was on
> MigrationState.
>
> Signed-off-by: Juan Quintela
With the comment below fixed:
Reviewed-by: Lukas Straub
> ---
> migration/ram-compress.h | 1 +
> migration/ram.h | 1 -
> migration/ram-compress.c | 41 +++-
> migration/ram.c | 29 +---
> 4 files changed, 42 insertions(+), 30 deletions(-)
>
> diff --git a/migration/ram-compress.h b/migration/ram-compress.h
> index b228640092..a7eb028849 100644
> --- a/migration/ram-compress.h
> +++ b/migration/ram-compress.h
> @@ -71,5 +71,6 @@ void decompress_data_with_multi_threads(QEMUFile *f, void
> *host, int len);
> void populate_compress(MigrationInfo *info);
> uint64_t ram_compressed_pages(void);
> void update_compress_thread_counts(const CompressParam *param, int
> bytes_xmit);
> +void compression_update_rates(uint64_t page_count);
>
> #endif
> diff --git a/migration/ram.h b/migration/ram.h
> index ea1f3c25b5..60bc4c9e3a 100644
> --- a/migration/ram.h
> +++ b/migration/ram.h
> @@ -34,7 +34,6 @@
> #include "io/channel.h"
>
> extern XBZRLECacheStats xbzrle_counters;
> -extern CompressionStats compression_counters;
>
> bool ramblock_is_ignored(RAMBlock *block);
> /* Should be holding either ram_list.mutex, or the RCU lock. */
> diff --git a/migration/ram-compress.c b/migration/ram-compress.c
> index 2652cdee8b..5e1bb82fcd 100644
> --- a/migration/ram-compress.c
> +++ b/migration/ram-compress.c
> @@ -41,7 +41,20 @@
> #include "ram.h"
> #include "migration-stats.h"
>
> -CompressionStats compression_counters;
> +static struct {
> +int64_t pages;
> +int64_t busy;
> +double busy_rate;
> +int64_t compressed_size;
> +double compression_rate;
> +/* compression statistics since the beginning of the period */
> +/* amount of count that no free thread to compress data */
> +uint64_t compress_thread_busy_prev;
> +/* amount bytes after compression */
> +uint64_t compressed_size_prev;
> +/* amount of compressed pages */
> +uint64_t compress_pages_prev;
> +} compression_counters;
>
> static CompressParam *comp_param;
> static QemuThread *compress_threads;
> @@ -529,3 +542,29 @@ void update_compress_thread_counts(const CompressParam
> *param, int bytes_xmit)
> compression_counters.pages++;
> }
>
> +void compression_update_rates(uint64_t page_count)
> +{
> +if (migrate_compress()) {
Use early return.
> +compression_counters.busy_rate = (double)(compression_counters.busy -
> +compression_counters.compress_thread_busy_prev) / page_count;
> +compression_counters.compress_thread_busy_prev =
> +compression_counters.busy;
> +
> +double compressed_size = compression_counters.compressed_size -
> + compression_counters.compressed_size_prev;
> +if (compressed_size) {
> +double uncompressed_size = (compression_counters.pages -
> +compression_counters.compress_pages_prev) *
> +qemu_target_page_size();
> +
> +/* Compression-Ratio = Uncompressed-size / Compressed-size */
> +compression_counters.compression_rate =
> +uncompressed_size / compressed_size;
> +
> +compression_counters.compress_pages_prev =
> +compression_counters.pages;
> +compression_counters.compressed_size_prev =
> +compression_counters.compressed_size;
> +}
> +}
> +}
> diff --git a/migration/ram.c b/migration/ram.c
> index 60f24006bc..1bd586c23a 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -364,13 +364,6 @@ struct RAMState {
> bool xbzrle_started;
> /* Are we on the last stage of migration */
> bool last_stage;
> -/* compression statistics since the beginning of the period */
> -/* amount of count that no free thread to compress data */
> -uint64_t compress_thread_busy_prev;
> -/* amount bytes after compression */
> -uint64_t compressed_size_prev;
> -/* amount of compressed pages */
> -uint64_t compress_pages_prev;
>
> /* total handled target pages at the beginning of period */
> uint64_t target_page_count_prev;
> @@ -933,7 +926,6 @@ uint64_t ram_get_total_transferred_pages(void)
> static void migration_update_rates(RAMState *rs, int64_t end_time)
> {
>
Re: [PATCH 10/18] migration: Simplify compress_page_with_multithread()
On Tue, 13 Jun 2023 16:57:49 +0200
Juan Quintela wrote:
> Move the goto to a while true.
>
> Signed-off-by: Juan Quintela
Reviewed-by: Lukas Straub
> ---
> migration/ram-compress.c | 48
> 1 file changed, 24 insertions(+), 24 deletions(-)
>
> diff --git a/migration/ram-compress.c b/migration/ram-compress.c
> index 8ce9519f3d..3c44803132 100644
> --- a/migration/ram-compress.c
> +++ b/migration/ram-compress.c
> @@ -273,35 +273,35 @@ bool compress_page_with_multi_thread(RAMBlock *block,
> ram_addr_t offset,
>
> thread_count = migrate_compress_threads();
> qemu_mutex_lock(_done_lock);
> -retry:
> -for (idx = 0; idx < thread_count; idx++) {
> -if (comp_param[idx].done) {
> -CompressParam *param = _param[idx];
> -qemu_mutex_lock(>mutex);
> -param->done = false;
> -send_queued_data(param);
> -assert(qemu_file_buffer_empty(param->file));
> -compress_reset_result(param);
> -set_compress_params(param, block, offset);
>
> -qemu_cond_signal(>cond);
> -qemu_mutex_unlock(>mutex);
> +while (true) {
> +for (idx = 0; idx < thread_count; idx++) {
> +if (comp_param[idx].done) {
> +CompressParam *param = _param[idx];
> +qemu_mutex_lock(>mutex);
> +param->done = false;
> +send_queued_data(param);
> +assert(qemu_file_buffer_empty(param->file));
> +compress_reset_result(param);
> +set_compress_params(param, block, offset);
> +
> +qemu_cond_signal(>cond);
> +qemu_mutex_unlock(>mutex);
> +qemu_mutex_unlock(_done_lock);
> +return true;
> +}
> +}
> +if (!wait) {
> qemu_mutex_unlock(_done_lock);
> -return true;
> +return false;
> }
> -}
> -
> -/*
> - * wait for the free thread if the user specifies 'compress-wait-thread',
> - * otherwise we will post the page out in the main thread as normal page.
> - */
> -if (wait) {
> +/*
> + * wait for a free thread if the user specifies
> + * 'compress-wait-thread', otherwise we will post the page out
> + * in the main thread as normal page.
> + */
> qemu_cond_wait(_done_cond, _done_lock);
> -goto retry;
> }
> -qemu_mutex_unlock(_done_lock);
> -
> -return false;
> }
>
> /* return the size after decompression, or negative value on error */
pgp16RF7BPBS3.pgp
Description: OpenPGP digital signature
Re: [PATCH 11/18] migration: Move busy++ to migrate_with_multithread
On Tue, 13 Jun 2023 16:57:50 +0200
Juan Quintela wrote:
> And now we can simplify save_compress_page().
>
> Signed-off-by: Juan Quintela
Reviewed-by: Lukas Straub
> ---
> migration/ram-compress.c | 1 +
> migration/ram.c | 7 +--
> 2 files changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/migration/ram-compress.c b/migration/ram-compress.c
> index 3c44803132..2652cdee8b 100644
> --- a/migration/ram-compress.c
> +++ b/migration/ram-compress.c
> @@ -293,6 +293,7 @@ bool compress_page_with_multi_thread(RAMBlock *block,
> ram_addr_t offset,
> }
> if (!wait) {
> qemu_mutex_unlock(_done_lock);
> +compression_counters.busy++;
> return false;
> }
> /*
> diff --git a/migration/ram.c b/migration/ram.c
> index 2a3ff2c36e..60f24006bc 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -2034,12 +2034,7 @@ static bool save_compress_page(RAMState *rs,
> PageSearchStatus *pss,
> return false;
> }
>
> -if (compress_page_with_multi_thread(block, offset, send_queued_data)) {
> -return true;
> -}
> -
> -compression_counters.busy++;
> -return false;
> +return compress_page_with_multi_thread(block, offset, send_queued_data);
> }
>
> /**
pgpLtlzWtK8YG.pgp
Description: OpenPGP digital signature
Re: [PATCH 09/18] migration: Make compress_data_with_multithreads return bool
On Tue, 13 Jun 2023 16:57:48 +0200
Juan Quintela wrote:
> Signed-off-by: Juan Quintela
Nit:
Probably makes sense to rename it to try_compress_page or similar.
And probably for zero_page and xbzrle (when factored out in it's own
function) too.
> ---
> migration/ram-compress.h | 4 ++--
> migration/ram-compress.c | 17 ++---
> migration/ram.c | 2 +-
> 3 files changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/migration/ram-compress.h b/migration/ram-compress.h
> index e55d3b50bd..b228640092 100644
> --- a/migration/ram-compress.h
> +++ b/migration/ram-compress.h
> @@ -60,8 +60,8 @@ void compress_threads_save_cleanup(void);
> int compress_threads_save_setup(void);
>
> void flush_compressed_data(int (send_queued_data(CompressParam *)));
> -int compress_page_with_multi_thread(RAMBlock *block, ram_addr_t offset,
> -int (send_queued_data(CompressParam *)));
> +bool compress_page_with_multi_thread(RAMBlock *block, ram_addr_t offset,
> + int (send_queued_data(CompressParam
> *)));
>
> int wait_for_decompress_done(void);
> void compress_threads_load_cleanup(void);
> diff --git a/migration/ram-compress.c b/migration/ram-compress.c
> index 47b600d62b..8ce9519f3d 100644
> --- a/migration/ram-compress.c
> +++ b/migration/ram-compress.c
> @@ -262,10 +262,13 @@ static inline void set_compress_params(CompressParam
> *param, RAMBlock *block,
> param->trigger = true;
> }
>
> -int compress_page_with_multi_thread(RAMBlock *block, ram_addr_t offset,
> -int (send_queued_data(CompressParam *)))
> +/*
> + * Return true when it compress a page
> + */
> +bool compress_page_with_multi_thread(RAMBlock *block, ram_addr_t offset,
> + int (send_queued_data(CompressParam *)))
> {
> -int idx, thread_count, pages = -1;
> +int idx, thread_count;
> bool wait = migrate_compress_wait_thread();
>
> thread_count = migrate_compress_threads();
> @@ -283,8 +286,8 @@ retry:
>
> qemu_cond_signal(>cond);
> qemu_mutex_unlock(>mutex);
> -pages = 1;
> -break;
> +qemu_mutex_unlock(_done_lock);
> +return true;
> }
> }
>
> @@ -292,13 +295,13 @@ retry:
> * wait for the free thread if the user specifies 'compress-wait-thread',
> * otherwise we will post the page out in the main thread as normal page.
> */
> -if (pages < 0 && wait) {
> +if (wait) {
> qemu_cond_wait(_done_cond, _done_lock);
> goto retry;
> }
> qemu_mutex_unlock(_done_lock);
>
> -return pages;
> +return false;
> }
>
> /* return the size after decompression, or negative value on error */
> diff --git a/migration/ram.c b/migration/ram.c
> index 53dec4d305..2a3ff2c36e 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -2034,7 +2034,7 @@ static bool save_compress_page(RAMState *rs,
> PageSearchStatus *pss,
> return false;
> }
>
> -if (compress_page_with_multi_thread(block, offset, send_queued_data) >
> 0) {
> +if (compress_page_with_multi_thread(block, offset, send_queued_data)) {
> return true;
> }
>
pgpS8MmMBtnrD.pgp
Description: OpenPGP digital signature
Re: [PATCH 08/18] migration: Move update_compress_threads_counts() to ram-compress.c
On Tue, 13 Jun 2023 16:57:47 +0200
Juan Quintela wrote:
> Signed-off-by: Juan Quintela
Reviewed-by: Lukas Straub
> ---
> migration/ram-compress.h | 1 +
> migration/ram-compress.c | 17 +
> migration/ram.c | 15 ---
> 3 files changed, 18 insertions(+), 15 deletions(-)
>
> diff --git a/migration/ram-compress.h b/migration/ram-compress.h
> index 77465dad4b..e55d3b50bd 100644
> --- a/migration/ram-compress.h
> +++ b/migration/ram-compress.h
> @@ -70,5 +70,6 @@ void decompress_data_with_multi_threads(QEMUFile *f, void
> *host, int len);
>
> void populate_compress(MigrationInfo *info);
> uint64_t ram_compressed_pages(void);
> +void update_compress_thread_counts(const CompressParam *param, int
> bytes_xmit);
>
> #endif
> diff --git a/migration/ram-compress.c b/migration/ram-compress.c
> index c48d5c53f6..47b600d62b 100644
> --- a/migration/ram-compress.c
> +++ b/migration/ram-compress.c
> @@ -32,11 +32,14 @@
> #include "ram-compress.h"
>
> #include "qemu/error-report.h"
> +#include "qemu/stats64.h"
> #include "migration.h"
> #include "options.h"
> #include "io/channel-null.h"
> #include "exec/target_page.h"
> #include "exec/ramblock.h"
> +#include "ram.h"
> +#include "migration-stats.h"
>
> CompressionStats compression_counters;
>
> @@ -508,3 +511,17 @@ uint64_t ram_compressed_pages(void)
> return compression_counters.pages;
> }
>
> +void update_compress_thread_counts(const CompressParam *param, int
> bytes_xmit)
> +{
> +ram_transferred_add(bytes_xmit);
> +
> +if (param->result == RES_ZEROPAGE) {
> +stat64_add(_stats.zero_pages, 1);
> +return;
> +}
> +
> +/* 8 means a header with RAM_SAVE_FLAG_CONTINUE. */
> +compression_counters.compressed_size += bytes_xmit - 8;
> +compression_counters.pages++;
> +}
> +
> diff --git a/migration/ram.c b/migration/ram.c
> index 13c518f81a..53dec4d305 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -1259,21 +1259,6 @@ static int ram_save_multifd_page(QEMUFile *file,
> RAMBlock *block,
> return 1;
> }
>
> -static void
> -update_compress_thread_counts(const CompressParam *param, int bytes_xmit)
> -{
> -ram_transferred_add(bytes_xmit);
> -
> -if (param->result == RES_ZEROPAGE) {
> -stat64_add(_stats.zero_pages, 1);
> -return;
> -}
> -
> -/* 8 means a header with RAM_SAVE_FLAG_CONTINUE. */
> -compression_counters.compressed_size += bytes_xmit - 8;
> -compression_counters.pages++;
> -}
> -
> static int send_queued_data(CompressParam *param)
> {
> PageSearchStatus *pss = _state->pss[RAM_CHANNEL_PRECOPY];
pgpOJ0VTKMmQW.pgp
Description: OpenPGP digital signature
Re: [PATCH 07/18] migration: Create ram_compressed_pages()
On Tue, 13 Jun 2023 16:57:46 +0200
Juan Quintela wrote:
> Signed-off-by: Juan Quintela
>
> fix ram_compress
>
> Signed-off-by: Juan Quintela
Commit message is off. Other than that:
Reviewed-by: Lukas Straub
> ---
> migration/ram-compress.h | 1 +
> migration/ram-compress.c | 6 ++
> migration/ram.c | 2 +-
> 3 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/migration/ram-compress.h b/migration/ram-compress.h
> index 43ff44c0ba..77465dad4b 100644
> --- a/migration/ram-compress.h
> +++ b/migration/ram-compress.h
> @@ -69,5 +69,6 @@ int compress_threads_load_setup(QEMUFile *f);
> void decompress_data_with_multi_threads(QEMUFile *f, void *host, int len);
>
> void populate_compress(MigrationInfo *info);
> +uint64_t ram_compressed_pages(void);
>
> #endif
> diff --git a/migration/ram-compress.c b/migration/ram-compress.c
> index a91c32588e..c48d5c53f6 100644
> --- a/migration/ram-compress.c
> +++ b/migration/ram-compress.c
> @@ -502,3 +502,9 @@ void populate_compress(MigrationInfo *info)
> compression_counters.compression_rate;
> }
> }
> +
> +uint64_t ram_compressed_pages(void)
> +{
> +return compression_counters.pages;
> +}
> +
> diff --git a/migration/ram.c b/migration/ram.c
> index d4943b982f..13c518f81a 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -927,7 +927,7 @@ uint64_t ram_get_total_transferred_pages(void)
> {
> return stat64_get(_stats.normal_pages) +
> stat64_get(_stats.zero_pages) +
> -compression_counters.pages + xbzrle_counters.pages;
> +ram_compressed_pages() + xbzrle_counters.pages;
> }
>
> static void migration_update_rates(RAMState *rs, int64_t end_time)
pgpuSYaow68dz.pgp
Description: OpenPGP digital signature
Re: [PATCH 06/18] migration: Create populate_compress()
On Tue, 13 Jun 2023 16:57:45 +0200
Juan Quintela wrote:
> So we don't have to access compression_counters from outside
> ram-compress.c.
>
> Signed-off-by: Juan Quintela
With the comment below fixed:
Reviewed-by: Lukas Straub
> ---
> migration/ram-compress.h | 3 +++
> migration/migration.c| 11 +--
> migration/ram-compress.c | 14 ++
> 3 files changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/migration/ram-compress.h b/migration/ram-compress.h
> index 6f7fe2f472..43ff44c0ba 100644
> --- a/migration/ram-compress.h
> +++ b/migration/ram-compress.h
> @@ -30,6 +30,7 @@
> #define QEMU_MIGRATION_COMPRESS_H
>
> #include "qemu-file.h"
> +#include "qapi/qapi-types-migration.h"
>
> enum CompressResult {
> RES_NONE = 0,
> @@ -67,4 +68,6 @@ void compress_threads_load_cleanup(void);
> int compress_threads_load_setup(QEMUFile *f);
> void decompress_data_with_multi_threads(QEMUFile *f, void *host, int len);
>
> +void populate_compress(MigrationInfo *info);
> +
> #endif
> diff --git a/migration/migration.c b/migration/migration.c
> index 5105ad6825..402bcd0d67 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -956,16 +956,7 @@ static void populate_ram_info(MigrationInfo *info,
> MigrationState *s)
> info->xbzrle_cache->overflow = xbzrle_counters.overflow;
> }
>
> -if (migrate_compress()) {
> -info->compression = g_malloc0(sizeof(*info->compression));
> -info->compression->pages = compression_counters.pages;
> -info->compression->busy = compression_counters.busy;
> -info->compression->busy_rate = compression_counters.busy_rate;
> -info->compression->compressed_size =
> -compression_counters.compressed_size;
> -info->compression->compression_rate =
> -compression_counters.compression_rate;
> -}
> +populate_compress(info);
>
> if (cpu_throttle_active()) {
> info->has_cpu_throttle_percentage = true;
> diff --git a/migration/ram-compress.c b/migration/ram-compress.c
> index a0f6a56470..a91c32588e 100644
> --- a/migration/ram-compress.c
> +++ b/migration/ram-compress.c
> @@ -488,3 +488,17 @@ void decompress_data_with_multi_threads(QEMUFile *f,
> void *host, int len)
> }
> }
> }
> +
> +void populate_compress(MigrationInfo *info)
> +{
> +if (migrate_compress()) {
Use early return:
if (!migrate_compress()) {
return;
}
> +info->compression = g_malloc0(sizeof(*info->compression));
> +info->compression->pages = compression_counters.pages;
> +info->compression->busy = compression_counters.busy;
> +info->compression->busy_rate = compression_counters.busy_rate;
> +info->compression->compressed_size =
> +compression_counters.compressed_size;
> +info->compression->compression_rate =
> +compression_counters.compression_rate;
> +}
> +}
pgpPjj4ifUJDX.pgp
Description: OpenPGP digital signature
Re: [PATCH 05/18] migration: Move compression_counters cleanup ram-compress.c
On Tue, 13 Jun 2023 16:57:44 +0200 Juan Quintela wrote: > Signed-off-by: Juan Quintela Reviewed-by: Lukas Straub > --- > migration/migration.c| 4 +--- > migration/ram-compress.c | 5 + > migration/savevm.c | 1 - > 3 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/migration/migration.c b/migration/migration.c > index 84551f040c..5105ad6825 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -1628,11 +1628,9 @@ static bool migrate_prepare(MigrationState *s, bool > blk, bool blk_inc, > > migrate_init(s); > /* > - * set mig_stats compression_counters memory to zero for a > - * new migration > + * set mig_stats memory to zero for a new migration > */ > memset(_stats, 0, sizeof(mig_stats)); > -memset(_counters, 0, sizeof(compression_counters)); > > return true; > } > diff --git a/migration/ram-compress.c b/migration/ram-compress.c > index 06254d8c69..a0f6a56470 100644 > --- a/migration/ram-compress.c > +++ b/migration/ram-compress.c > @@ -430,6 +430,11 @@ int compress_threads_load_setup(QEMUFile *f) > return 0; > } > > +/* > + * set compression_counters memory to zero for a new migration > + */ > +memset(_counters, 0, sizeof(compression_counters)); > + > thread_count = migrate_decompress_threads(); > decompress_threads = g_new0(QemuThread, thread_count); > decomp_param = g_new0(DecompressParam, thread_count); > diff --git a/migration/savevm.c b/migration/savevm.c > index bc284087f9..aa31d29f23 100644 > --- a/migration/savevm.c > +++ b/migration/savevm.c > @@ -1621,7 +1621,6 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp) > > migrate_init(ms); > memset(_stats, 0, sizeof(mig_stats)); > -memset(_counters, 0, sizeof(compression_counters)); > ms->to_dst_file = f; > > qemu_mutex_unlock_iothread(); pgpB_gHFkzE0w.pgp Description: OpenPGP digital signature
Re: [PATCH 03/18] migration: RDMA is not compatible with anything else
On Tue, 13 Jun 2023 16:57:42 +0200
Juan Quintela wrote:
> So give an error instead of just ignoring the other methods.
>
> Signed-off-by: Juan Quintela
Reviewed-by: Lukas Straub
> ---
> migration/migration.c | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index dc05c6f6ea..84551f040c 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -436,6 +436,18 @@ static void qemu_start_incoming_migration(const char
> *uri, Error **errp)
> socket_start_incoming_migration(p ? p : uri, errp);
> #ifdef CONFIG_RDMA
> } else if (strstart(uri, "rdma:", )) {
> +if (migrate_compress()) {
> +error_setg(errp, "RDMA and compression can't be used together");
> +return;
> +}
> +if (migrate_xbzrle()) {
> +error_setg(errp, "RDMA and XBZRLE can't be used together");
> +return;
> +}
> +if (migrate_multifd()) {
> +error_setg(errp, "RDMA and multifd can't be used together");
> +return;
> +}
> rdma_start_incoming_migration(p, errp);
> #endif
> } else if (strstart(uri, "exec:", )) {
pgpgZCKRQl6cS.pgp
Description: OpenPGP digital signature
Re: [PATCH 02/18] migration: Give one error if trying to set COMPRESSION and XBZRLE
On Tue, 13 Jun 2023 16:57:41 +0200
Juan Quintela wrote:
> As we have disable to use both together in the previous patch, we can
> remove this check.
This commit message is off.
>
> Signed-off-by: Juan Quintela
> ---
> migration/options.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/migration/options.c b/migration/options.c
> index c6674a4753..a18689c314 100644
> --- a/migration/options.c
> +++ b/migration/options.c
> @@ -554,6 +554,13 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps,
> Error **errp)
> }
> }
>
> +if (new_caps[MIGRATION_CAPABILITY_COMPRESS]) {
> +if (new_caps[MIGRATION_CAPABILITY_XBZRLE]) {
> +error_setg(errp, "Compression is not compatible with xbzrle");
> +return false;
> +}
> +}
> +
> return true;
> }
>
Re: [PATCH 01/18] migration: Give one error if trying to set MULTIFD and XBZRLE
On Tue, 13 Jun 2023 16:57:40 +0200
Juan Quintela wrote:
> Signed-off-by: Juan Quintela
> ---
> migration/options.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/migration/options.c b/migration/options.c
> index b62ab30cd5..c6674a4753 100644
> --- a/migration/options.c
> +++ b/migration/options.c
> @@ -547,6 +547,13 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps,
> Error **errp)
> }
> }
>
> +if (new_caps[MIGRATION_CAPABILITY_MULTIFD]) {
> +if (new_caps[MIGRATION_CAPABILITY_XBZRLE]) {
> +error_setg(errp, "Multifd is not compatible with xbzrle");
> +return false;
> +}
> +}
> +
> return true;
> }
>
Hmm, I think supporting xbzrle + multifd might be worthwhile as an
alternative to xbzrle + compress.
I don't use it, but it should be easy to do by just letting xbzlre try
to handle the page before multifd.
There shouldn't be any incompatibilities other than that, as long as
xbzlre sees the pages before multifd and the zero-page case is still
handled the old way.
Best Regards,
Lukas Straub
pgpzt4_NdWd_7.pgp
Description: OpenPGP digital signature
[PATCH 2/2] savevm: Disallow devices being legacy and iterable at the same time
Signed-off-by: Lukas Straub
---
migration/savevm.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/migration/savevm.c b/migration/savevm.c
index 5986f852b2..8d3cf7d675 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -774,6 +774,8 @@ int register_savevm_live(const char *idstr,
se->is_ram = 1;
}
+assert(!(se->ops->save_setup && se->ops->save_state));
+
pstrcat(se->idstr, sizeof(se->idstr), idstr);
if (instance_id == VMSTATE_INSTANCE_ID_ANY) {
--
2.39.2
pgpTvwTgJFqKw.pgp
Description: OpenPGP digital signature
[PATCH 1/2] vfio: Don't be a iterable and legacy device at the same time
Legacy savevm devices only implement save_state() and load_state().
Iterable devices shouldn't implement save_state() or else they are
handled both as an iterable and legacy device in the savevm code.
Signed-off-by: Lukas Straub
---
Note: this patch is completely untested.
hw/vfio/migration.c | 38 +++---
1 file changed, 11 insertions(+), 27 deletions(-)
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 6b58dddb88..8d7f22dbd3 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -166,23 +166,6 @@ static int vfio_load_buffer(QEMUFile *f, VFIODevice
*vbasedev,
return ret;
}
-static int vfio_save_device_config_state(QEMUFile *f, void *opaque)
-{
-VFIODevice *vbasedev = opaque;
-
-qemu_put_be64(f, VFIO_MIG_FLAG_DEV_CONFIG_STATE);
-
-if (vbasedev->ops && vbasedev->ops->vfio_save_config) {
-vbasedev->ops->vfio_save_config(vbasedev, f);
-}
-
-qemu_put_be64(f, VFIO_MIG_FLAG_END_OF_STATE);
-
-trace_vfio_save_device_config_state(vbasedev->name);
-
-return qemu_file_get_error(f);
-}
-
static int vfio_load_device_config_state(QEMUFile *f, void *opaque)
{
VFIODevice *vbasedev = opaque;
@@ -351,7 +334,6 @@ static int vfio_save_complete_precopy(QEMUFile *f, void
*opaque)
}
} while (!ret);
-qemu_put_be64(f, VFIO_MIG_FLAG_END_OF_STATE);
ret = qemu_file_get_error(f);
if (ret) {
return ret;
@@ -365,20 +347,23 @@ static int vfio_save_complete_precopy(QEMUFile *f, void
*opaque)
VFIO_DEVICE_STATE_ERROR);
trace_vfio_save_complete_precopy(vbasedev->name, ret);
-return ret;
-}
+qemu_put_be64(f, VFIO_MIG_FLAG_DEV_CONFIG_STATE);
-static void vfio_save_state(QEMUFile *f, void *opaque)
-{
-VFIODevice *vbasedev = opaque;
-int ret;
+if (vbasedev->ops && vbasedev->ops->vfio_save_config) {
+vbasedev->ops->vfio_save_config(vbasedev, f);
+}
+
+qemu_put_be64(f, VFIO_MIG_FLAG_END_OF_STATE);
+
+trace_vfio_save_device_config_state(vbasedev->name);
-ret = vfio_save_device_config_state(f, opaque);
+ret = qemu_file_get_error(f);
if (ret) {
error_report("%s: Failed to save device config space",
vbasedev->name);
-qemu_file_set_error(f, ret);
}
+
+return ret;
}
static int vfio_load_setup(QEMUFile *f, void *opaque)
@@ -458,7 +443,6 @@ static const SaveVMHandlers savevm_vfio_handlers = {
.save_cleanup = vfio_save_cleanup,
.state_pending_exact = vfio_state_pending_exact,
.save_live_complete_precopy = vfio_save_complete_precopy,
-.save_state = vfio_save_state,
.load_setup = vfio_load_setup,
.load_cleanup = vfio_load_cleanup,
.load_state = vfio_load_state,
--
2.39.2
pgp0NPw7SfPNG.pgp
Description: OpenPGP digital signature
[PATCH 2/2] ide: Explicitly poll for BHs on cancel
When we still have an AIOCB registered for DMA operations, we try to
settle the respective operation by draining the BlockBackend associated
with the IDE device.
However, this assumes that every DMA operation is associated with some
I/O operation on the BlockBackend, and so settling the latter will
settle the former. That is not the case; for example, the guest is free
to issue a zero-length TRIM operation that will not result in any I/O
operation forwarded to the BlockBackend. In such a case, blk_drain()
will be a no-op if no other operations are in flight.
It is clear that if blk_drain() is a no-op, the value of
s->bus->dma->aiocb will not change between checking it in the `if`
condition and asserting that it is NULL after blk_drain().
To settle the DMA operation, we will thus need to explicitly invoke
aio_poll() ourselves, which will run any outstanding BHs (like
ide_trim_bh_cb()), until s->bus->dma->aiocb is NULL. To stop this from
being an infinite loop, assert that we made progress with every
aio_poll() call (i.e., invoked some BH).
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2029980
Signed-off-by: Hanna Reitz
Signed-off-by: Lukas Straub
---
hw/ide/core.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index d172e70f1e..a5fd89ebdd 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -736,7 +736,17 @@ void ide_cancel_dma_sync(IDEState *s)
if (s->bus->dma->aiocb) {
trace_ide_cancel_dma_sync_remaining();
blk_drain(s->blk);
-assert(s->bus->dma->aiocb == NULL);
+
+/*
+ * Wait for potentially still-scheduled BHs, like ide_trim_bh_cb()
+ * (blk_drain() will only poll if there are in-flight requests on the
+ * BlockBackend, which there may not necessarily be, e.g. when the
+ * guest has issued a zero-length TRIM request)
+ */
+while (s->bus->dma->aiocb) {
+bool progress = aio_poll(qemu_get_aio_context(), true);
+assert(progress);
+}
}
}
--
2.39.2
pgpimE5CmA6Oq.pgp
Description: OpenPGP digital signature
[PATCH 1/2] ide: Fix a rare hang during block draining
If the guest issues a discard during a block drain section, the
blk_aio_pdiscard() may not be processed, but queued instead.
And so the callback will never be called to issue the bh and decrease
the BB in-flight number again.
This causes a hang in the drain code, since it will wait forever for the
BB in-flight counter to decrease.
This reverts commit 7e5cdb34 "ide: Increment BB in-flight counter for TRIM BH"
to fix this hang. The bug fixed by that commit will be fixed differently
in the next commit.
Signed-off-by: Lukas Straub
---
hw/ide/core.c | 7 ---
1 file changed, 7 deletions(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index de48ff9f86..d172e70f1e 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -436,16 +436,12 @@ static const AIOCBInfo trim_aiocb_info = {
static void ide_trim_bh_cb(void *opaque)
{
TrimAIOCB *iocb = opaque;
-BlockBackend *blk = iocb->s->blk;
iocb->common.cb(iocb->common.opaque, iocb->ret);
qemu_bh_delete(iocb->bh);
iocb->bh = NULL;
qemu_aio_unref(iocb);
-
-/* Paired with an increment in ide_issue_trim() */
-blk_dec_in_flight(blk);
}
static void ide_issue_trim_cb(void *opaque, int ret)
@@ -516,9 +512,6 @@ BlockAIOCB *ide_issue_trim(
IDEDevice *dev = s->unit ? s->bus->slave : s->bus->master;
TrimAIOCB *iocb;
-/* Paired with a decrement in ide_trim_bh_cb() */
-blk_inc_in_flight(s->blk);
-
iocb = blk_aio_get(_aiocb_info, s->blk, cb, cb_opaque);
iocb->s = s;
iocb->bh = qemu_bh_new_guarded(ide_trim_bh_cb, iocb,
--
2.39.2
pgp2YGl9QWr0N.pgp
Description: OpenPGP digital signature
[PATCH v3] multifd: Add colo support
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: Juan Quintela
Signed-off-by: Lukas Straub
---
migration/meson.build| 1 +
migration/multifd-colo.c | 53
migration/multifd-colo.h | 24 ++
migration/multifd.c | 5
4 files changed, 83 insertions(+)
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 1ae28523a1..063e0e0a8c 100644
--- a/migration/meson.build
+++ b/migration/meson.build
@@ -21,6 +21,7 @@ system_ss.add(files(
'migration.c',
'multifd.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 00..4872dc6d01
--- /dev/null
+++ b/migration/multifd-colo.c
@@ -0,0 +1,53 @@
+/*
+ * multifd colo implementation
+ *
+ * Copyright (c) Lukas Straub
+ *
+ * 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())
+return;
+
+assert(p->block->colo_cache);
+
+/*
+ * While we're still in precopy state (not yet in colo state), 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())
+return;
+
+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 00..58920a0583
--- /dev/null
+++ b/migration/multifd-colo.h
@@ -0,0 +1,24 @@
+/*
+ * multifd colo header
+ *
+ * Copyright (c) Lukas Straub
+ *
+ * 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
+
+#ifdef 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
+#endif
diff --git a/migration/multifd.c b/migration/multifd.c
index 3387d8277f..6b031c1fd2 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -25,6 +25,7 @@
#include "qemu-file.h"
#include "trace.h"
#include "multifd.h"
+#include "multifd-colo.h"
#include "threadinfo.h"
#include "options.h"
#include "qemu/yank.h"
@@ -1134,10 +1135,14 @@ static void *multifd_recv_thread(void *opaque)
qemu_mutex_unlock(>mutex);
if (p->normal_num) {
+multifd_colo_prepare_recv_pages(p);
+
ret = multifd_recv_state->ops->recv_pages(p, _err);
if (ret != 0) {
break;
}
+
+multifd_colo_process_recv_pages(p);
}
if (flags & MULTIFD_FLAG_SYNC) {
--
2.39.2
pgpF_Xz8KX6Oc.pgp
Description: OpenPGP digital signature
Re: [PATCH] multifd: Add colo support
On Thu, 11 May 2023 11:52:55 +0200
Juan Quintela wrote:
> Lukas Straub wrote:
> > On Tue, 9 May 2023 20:15:28 +0200
> > Juan Quintela wrote:
> >
> >> From: Lukas Straub
> >>
> >> 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
> >>
> >> ---
> >>
> >> 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, _err);
?
> >> new file mode 100644
> >> index 00..10fa1467fa
> >> --- /dev/null
> >> +++ b/migration/multifd-colo.c
> >> @@ -0,0 +1,49 @@
> >> +/*
> >> + * multifd colo implementation
> >> + *
> >> + * Copyright (c) Lukas Straub
> >> + *
> >> + * 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)
> >> +{
> >> +
Re: [PATCH] multifd: Add colo support
On Tue, 9 May 2023 20:15:28 +0200
Juan Quintela wrote:
> From: Lukas Straub
>
> 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
>
> ---
>
> 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 00..10fa1467fa
> --- /dev/null
> +++ b/migration/multifd-colo.c
> @@ -0,0 +1,49 @@
> +/*
> + * multifd colo implementation
> + *
> + * Copyright (c) Lukas Straub
> + *
> + * 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 00..1636c617fc
> --- /dev/null
> +++ b/migration/multifd-colo.h
> @@ -0,0 +1,24 @@
> +/*
> + * multifd colo header
> + *
> + * Copyright (c) Lukas Straub
> + *
> + * 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
--
pgpd7RjCOcblA.pgp
Description: OpenPGP digital signature
[PATCH v2 6/6] multifd: Add colo support
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
---
migration/multifd-colo.c | 30 +-
1 file changed, 29 insertions(+), 1 deletion(-)
diff --git a/migration/multifd-colo.c b/migration/multifd-colo.c
index c035d15e87..305a1b7000 100644
--- a/migration/multifd-colo.c
+++ b/migration/multifd-colo.c
@@ -15,13 +15,41 @@
#include "ram.h"
#include "multifd.h"
#include "io/channel-socket.h"
+#include "migration/colo.h"
#define MULTIFD_INTERNAL
#include "multifd-internal.h"
static int multifd_colo_recv_pages(MultiFDRecvParams *p, Error **errp)
{
-return multifd_recv_state->ops->recv_pages(p, errp);
+int ret = 0;
+
+/*
+ * 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;
+ret = multifd_recv_state->ops->recv_pages(p, errp);
+if (ret != 0) {
+p->host = p->block->host;
+return ret;
+}
+
+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;
+return ret;
}
int multifd_colo_load_setup(Error **errp)
--
2.39.2
pgplE9D31XYvU.pgp
Description: OpenPGP digital signature
[PATCH v2 3/6] multifd: Introduce multifd-internal.h
Introduce multifd-internal.h so code that would normally go into
multifd.c can go into an extra file. This way, multifd.c hopefully
won't grow to 4000 lines like ram.c
This will be used in the next commits to add colo support to multifd.
Signed-off-by: Lukas Straub
---
migration/multifd-internal.h | 34 ++
migration/multifd.c | 15 ---
2 files changed, 38 insertions(+), 11 deletions(-)
create mode 100644 migration/multifd-internal.h
diff --git a/migration/multifd-internal.h b/migration/multifd-internal.h
new file mode 100644
index 00..6eeaa028e7
--- /dev/null
+++ b/migration/multifd-internal.h
@@ -0,0 +1,34 @@
+/*
+ * Internal Multifd header
+ *
+ * Copyright (c) 2019-2020 Red Hat Inc
+ *
+ * Authors:
+ * Juan Quintela
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifdef QEMU_MIGRATION_MULTIFD_INTERNAL_H
+#error Only include this header directly
+#endif
+#define QEMU_MIGRATION_MULTIFD_INTERNAL_H
+
+#ifndef MULTIFD_INTERNAL
+#error This header is internal to multifd
+#endif
+
+struct MultiFDRecvState {
+MultiFDRecvParams *params;
+/* number of created threads */
+int count;
+/* syncs main thread and channels */
+QemuSemaphore sem_sync;
+/* global number of generated multifd packets */
+uint64_t packet_num;
+/* multifd ops */
+MultiFDMethods *ops;
+};
+
+extern struct MultiFDRecvState *multifd_recv_state;
diff --git a/migration/multifd.c b/migration/multifd.c
index 4e71c19292..f6bad69b6c 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -31,6 +31,9 @@
#include "io/channel-socket.h"
#include "yank_functions.h"
+#define MULTIFD_INTERNAL
+#include "multifd-internal.h"
+
/* Multiple fd's */
#define MULTIFD_MAGIC 0x11223344U
@@ -967,17 +970,7 @@ int multifd_save_setup(Error **errp)
return 0;
}
-struct {
-MultiFDRecvParams *params;
-/* number of created threads */
-int count;
-/* syncs main thread and channels */
-QemuSemaphore sem_sync;
-/* global number of generated multifd packets */
-uint64_t packet_num;
-/* multifd ops */
-MultiFDMethods *ops;
-} *multifd_recv_state;
+struct MultiFDRecvState *multifd_recv_state;
static void multifd_recv_terminate_threads(Error *err)
{
--
2.39.2
pgpxmCxzwCgUP.pgp
Description: OpenPGP digital signature
[PATCH v2 4/6] multifd: Introduce a overridable revc_pages method
This allows to override the behaviour around recv_pages. Think of
it like a "multifd_colo" child class of multifd.
This will be used in the next commits to add colo support to multifd.
Signed-off-by: Lukas Straub
---
migration/meson.build| 1 +
migration/multifd-colo.c | 39 +
migration/multifd-internal.h | 5
migration/multifd.c | 48
4 files changed, 83 insertions(+), 10 deletions(-)
create mode 100644 migration/multifd-colo.c
diff --git a/migration/meson.build b/migration/meson.build
index da1897fadf..22ab6c6d73 100644
--- a/migration/meson.build
+++ b/migration/meson.build
@@ -23,6 +23,7 @@ softmmu_ss.add(files(
'migration.c',
'multifd.c',
'multifd-zlib.c',
+ 'multifd-colo.c',
'options.c',
'postcopy-ram.c',
'savevm.c',
diff --git a/migration/multifd-colo.c b/migration/multifd-colo.c
new file mode 100644
index 00..c035d15e87
--- /dev/null
+++ b/migration/multifd-colo.c
@@ -0,0 +1,39 @@
+/*
+ * multifd colo implementation
+ *
+ * Copyright (c) Lukas Straub
+ *
+ * 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 "io/channel-socket.h"
+
+#define MULTIFD_INTERNAL
+#include "multifd-internal.h"
+
+static int multifd_colo_recv_pages(MultiFDRecvParams *p, Error **errp)
+{
+return multifd_recv_state->ops->recv_pages(p, errp);
+}
+
+int multifd_colo_load_setup(Error **errp)
+{
+int ret;
+
+ret = _multifd_load_setup(errp);
+if (ret) {
+return ret;
+}
+
+multifd_recv_state->recv_pages = multifd_colo_recv_pages;
+
+return 0;
+}
diff --git a/migration/multifd-internal.h b/migration/multifd-internal.h
index 6eeaa028e7..82357f1d88 100644
--- a/migration/multifd-internal.h
+++ b/migration/multifd-internal.h
@@ -29,6 +29,11 @@ struct MultiFDRecvState {
uint64_t packet_num;
/* multifd ops */
MultiFDMethods *ops;
+/* overridable recv method */
+int (*recv_pages)(MultiFDRecvParams *p, Error **errp);
};
extern struct MultiFDRecvState *multifd_recv_state;
+
+int _multifd_load_setup(Error **errp);
+int multifd_colo_load_setup(Error **errp);
diff --git a/migration/multifd.c b/migration/multifd.c
index f6bad69b6c..fb5e8859de 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -1126,7 +1126,7 @@ static void *multifd_recv_thread(void *opaque)
qemu_mutex_unlock(>mutex);
if (p->normal_num) {
-ret = multifd_recv_state->ops->recv_pages(p, _err);
+ret = multifd_recv_state->recv_pages(p, _err);
if (ret != 0) {
break;
}
@@ -1152,20 +1152,12 @@ static void *multifd_recv_thread(void *opaque)
return NULL;
}
-int multifd_load_setup(Error **errp)
+int _multifd_load_setup(Error **errp)
{
int thread_count;
uint32_t page_count = MULTIFD_PACKET_SIZE / qemu_target_page_size();
uint8_t i;
-/*
- * Return successfully if multiFD recv state is already initialised
- * or multiFD is not enabled.
- */
-if (multifd_recv_state || !migrate_multifd()) {
-return 0;
-}
-
thread_count = migrate_multifd_channels();
multifd_recv_state = g_malloc0(sizeof(*multifd_recv_state));
multifd_recv_state->params = g_new0(MultiFDRecvParams, thread_count);
@@ -1204,6 +1196,42 @@ int multifd_load_setup(Error **errp)
return 0;
}
+static int multifd_normal_recv_pages(MultiFDRecvParams *p, Error **errp)
+{
+return multifd_recv_state->ops->recv_pages(p, errp);
+}
+
+static int multifd_normal_load_setup(Error **errp)
+{
+int ret;
+
+ret = _multifd_load_setup(errp);
+if (ret) {
+return ret;
+}
+
+multifd_recv_state->recv_pages = multifd_normal_recv_pages;
+
+return 0;
+}
+
+int multifd_load_setup(Error **errp)
+{
+/*
+ * Return successfully if multiFD recv state is already initialised
+ * or multiFD is not enabled.
+ */
+if (multifd_recv_state || !migrate_multifd()) {
+return 0;
+}
+
+if (migrate_colo()) {
+return multifd_colo_load_setup(errp);
+} else {
+return multifd_normal_load_setup(errp);
+}
+}
+
bool multifd_recv_all_channels_created(void)
{
int thread_count = migrate_multifd_channels();
--
2.39.2
pgpW27tKPR0AL.pgp
Description: OpenPGP digital signature
[PATCH v2 1/6] ram: Add public helper to set colo bitmap
The overhead of the mutex in non-multifd mode is negligible,
because in that case its just the single thread taking the mutex.
This will be used in the next commits to add colo support to multifd.
Signed-off-by: Lukas Straub
---
migration/ram.c | 17 ++---
migration/ram.h | 1 +
2 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index 5e7bf20ca5..2d3fd2112a 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3633,6 +3633,18 @@ static ram_addr_t
host_page_offset_from_ram_block_offset(RAMBlock *block,
return ((uintptr_t)block->host + offset) & (block->page_size - 1);
}
+void colo_record_bitmap(RAMBlock *block, ram_addr_t *normal, uint normal_num)
+{
+qemu_mutex_lock(_state->bitmap_mutex);
+for (int i = 0; i < normal_num; i++) {
+ram_addr_t offset = normal[i];
+ram_state->migration_dirty_pages += !test_and_set_bit(
+offset >> TARGET_PAGE_BITS,
+block->bmap);
+}
+qemu_mutex_unlock(_state->bitmap_mutex);
+}
+
static inline void *colo_cache_from_block_offset(RAMBlock *block,
ram_addr_t offset, bool record_bitmap)
{
@@ -3650,9 +3662,8 @@ static inline void *colo_cache_from_block_offset(RAMBlock
*block,
* It help us to decide which pages in ram cache should be flushed
* into VM's RAM later.
*/
-if (record_bitmap &&
-!test_and_set_bit(offset >> TARGET_PAGE_BITS, block->bmap)) {
-ram_state->migration_dirty_pages++;
+if (record_bitmap) {
+colo_record_bitmap(block, , 1);
}
return block->colo_cache + offset;
}
diff --git a/migration/ram.h b/migration/ram.h
index 6fffbeb5f1..887d1fbae6 100644
--- a/migration/ram.h
+++ b/migration/ram.h
@@ -82,6 +82,7 @@ int colo_init_ram_cache(void);
void colo_flush_ram_cache(void);
void colo_release_ram_cache(void);
void colo_incoming_start_dirty_log(void);
+void colo_record_bitmap(RAMBlock *block, ram_addr_t *normal, uint normal_num);
/* Background snapshot */
bool ram_write_tracking_available(void);
--
2.39.2
pgp53mPpYP4Hq.pgp
Description: OpenPGP digital signature
[PATCH v2 2/6] ram: Let colo_flush_ram_cache take the bitmap_mutex
This is not required, colo_flush_ram_cache does not run concurrently
with the multifd threads since the cache is only flushed after
everything has been received. But it makes me more comfortable.
This will be used in the next commits to add colo support to multifd.
Signed-off-by: Lukas Straub
---
migration/ram.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/migration/ram.c b/migration/ram.c
index 2d3fd2112a..f9e7aeda12 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -4230,6 +4230,7 @@ void colo_flush_ram_cache(void)
unsigned long offset = 0;
memory_global_dirty_log_sync();
+qemu_mutex_lock(_state->bitmap_mutex);
WITH_RCU_READ_LOCK_GUARD() {
RAMBLOCK_FOREACH_NOT_IGNORED(block) {
ramblock_sync_dirty_bitmap(ram_state, block);
@@ -4264,6 +4265,7 @@ void colo_flush_ram_cache(void)
}
}
}
+qemu_mutex_unlock(_state->bitmap_mutex);
trace_colo_flush_ram_cache_end();
}
--
2.39.2
pgpg5HTiURpEc.pgp
Description: OpenPGP digital signature
[PATCH v2 5/6] multifd: Add the ramblock to MultiFDRecvParams
This will be used in the next commits to add colo support to multifd.
Signed-off-by: Lukas Straub
---
migration/multifd.c | 11 +--
migration/multifd.h | 2 ++
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/migration/multifd.c b/migration/multifd.c
index fb5e8859de..fddbf86596 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -284,7 +284,6 @@ static void multifd_send_fill_packet(MultiFDSendParams *p)
static int multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp)
{
MultiFDPacket_t *packet = p->packet;
-RAMBlock *block;
int i;
packet->magic = be32_to_cpu(packet->magic);
@@ -334,21 +333,21 @@ static int multifd_recv_unfill_packet(MultiFDRecvParams
*p, Error **errp)
/* make sure that ramblock is 0 terminated */
packet->ramblock[255] = 0;
-block = qemu_ram_block_by_name(packet->ramblock);
-if (!block) {
+p->block = qemu_ram_block_by_name(packet->ramblock);
+if (!p->block) {
error_setg(errp, "multifd: unknown ram block %s",
packet->ramblock);
return -1;
}
-p->host = block->host;
+p->host = p->block->host;
for (i = 0; i < p->normal_num; i++) {
uint64_t offset = be64_to_cpu(packet->offset[i]);
-if (offset > (block->used_length - p->page_size)) {
+if (offset > (p->block->used_length - p->page_size)) {
error_setg(errp, "multifd: offset too long %" PRIu64
" (max " RAM_ADDR_FMT ")",
- offset, block->used_length);
+ offset, p->block->used_length);
return -1;
}
p->normal[i] = offset;
diff --git a/migration/multifd.h b/migration/multifd.h
index 7cfc265148..a835643b48 100644
--- a/migration/multifd.h
+++ b/migration/multifd.h
@@ -175,6 +175,8 @@ typedef struct {
uint32_t next_packet_size;
/* packets sent through this channel */
uint64_t num_packets;
+/* ramblock */
+RAMBlock *block;
/* ramblock host address */
uint8_t *host;
/* non zero pages recv through this channel */
--
2.39.2
pgpYmu_18sDuf.pgp
Description: OpenPGP digital signature
[PATCH v2 0/6] multifd: Add colo support
Hello Everyone, These patches add support for colo to multifd. -v2: - Split out addition of p->block - Add more comments Lukas Straub (6): ram: Add public helper to set colo bitmap ram: Let colo_flush_ram_cache take the bitmap_mutex multifd: Introduce multifd-internal.h multifd: Introduce a overridable revc_pages method multifd: Add the ramblock to MultiFDRecvParams multifd: Add colo support migration/meson.build| 1 + migration/multifd-colo.c | 67 migration/multifd-internal.h | 39 +++ migration/multifd.c | 74 +++- migration/multifd.h | 2 + migration/ram.c | 19 +++-- migration/ram.h | 1 + 7 files changed, 173 insertions(+), 30 deletions(-) create mode 100644 migration/multifd-colo.c create mode 100644 migration/multifd-internal.h -- 2.39.2 pgpUUAHlLbth_.pgp Description: OpenPGP digital signature
Re: [PULL 00/21] Migration 20230428 patches
On Mon, 08 May 2023 10:12:35 +0200 Juan Quintela wrote: > Lukas Straub wrote: > > On Tue, 02 May 2023 12:39:12 +0200 > > Juan Quintela wrote: > > > >> [...] > >> > >> my patches are only code movement and cleanups, so Lukas any clue? > >> > >> Lukas, I am going to drop the compress code for now and pass the other > >> patches. In the meanwhile, I am going to try to setup some machine > >> where I can run the upstream tests and see if I can reproduce there. > >> BTW, I would be happy if you double check that I did the rebase > >> correctly, they didn't apply correctly, but as said, the tests have been > >> running for two/three days on all my daily testing, so I thought that I > >> did the things correctly. > > Hi > > > Hi, > > I rebased the series here and got exactly the same files as in this > > pull request. And I can't reproduce these failures either. > > Nice > > > Maybe you can run the CI just on the newly added compress tests and see > > if it already blows up without the refactoring? > > It does, I don't have to check O:-) > > The initial reason that I did the compression code on top of multifd was > that I was not able to get the old compression code to run "reliabely" > on my testing. > > > Anyway, this series is not so important anymore... > > What about: > - I add the series as they are, because the code is better than what we > have before (and being in a different file makes it easier to > deprecate, not compile, ...) > - I just disable the tests until we find something that works. > > Richard, Lukas? That is fine with me. > > Later, Juan. > > >> Richard, once that we are here, one of the problem that we are having is > >> that the test is exiting with an abort, so we have no clue what is > >> happening. Is there a way to get a backtrace, or at least the number > >> > >> Later, Juan. > >> > pgpysUxiVaor9.pgp Description: OpenPGP digital signature
Re: [PULL 00/21] Migration 20230428 patches
On Tue, 02 May 2023 12:39:12 +0200 Juan Quintela wrote: > [...] > > my patches are only code movement and cleanups, so Lukas any clue? > > Lukas, I am going to drop the compress code for now and pass the other > patches. In the meanwhile, I am going to try to setup some machine > where I can run the upstream tests and see if I can reproduce there. > BTW, I would be happy if you double check that I did the rebase > correctly, they didn't apply correctly, but as said, the tests have been > running for two/three days on all my daily testing, so I thought that I > did the things correctly. Hi, I rebased the series here and got exactly the same files as in this pull request. And I can't reproduce these failures either. Maybe you can run the CI just on the newly added compress tests and see if it already blows up without the refactoring? Anyway, this series is not so important anymore... > Richard, once that we are here, one of the problem that we are having is > that the test is exiting with an abort, so we have no clue what is > happening. Is there a way to get a backtrace, or at least the number > > Later, Juan. > -- pgpyt9nj2ware.pgp Description: OpenPGP digital signature
[PATCH 5/5] multifd: Add colo support
Signed-off-by: Lukas Straub
---
migration/multifd-colo.c | 30 +-
migration/multifd.c | 11 +--
migration/multifd.h | 2 ++
3 files changed, 36 insertions(+), 7 deletions(-)
diff --git a/migration/multifd-colo.c b/migration/multifd-colo.c
index c035d15e87..305a1b7000 100644
--- a/migration/multifd-colo.c
+++ b/migration/multifd-colo.c
@@ -15,13 +15,41 @@
#include "ram.h"
#include "multifd.h"
#include "io/channel-socket.h"
+#include "migration/colo.h"
#define MULTIFD_INTERNAL
#include "multifd-internal.h"
static int multifd_colo_recv_pages(MultiFDRecvParams *p, Error **errp)
{
-return multifd_recv_state->ops->recv_pages(p, errp);
+int ret = 0;
+
+/*
+ * 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;
+ret = multifd_recv_state->ops->recv_pages(p, errp);
+if (ret != 0) {
+p->host = p->block->host;
+return ret;
+}
+
+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;
+return ret;
}
int multifd_colo_load_setup(Error **errp)
diff --git a/migration/multifd.c b/migration/multifd.c
index fb5e8859de..fddbf86596 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -284,7 +284,6 @@ static void multifd_send_fill_packet(MultiFDSendParams *p)
static int multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp)
{
MultiFDPacket_t *packet = p->packet;
-RAMBlock *block;
int i;
packet->magic = be32_to_cpu(packet->magic);
@@ -334,21 +333,21 @@ static int multifd_recv_unfill_packet(MultiFDRecvParams
*p, Error **errp)
/* make sure that ramblock is 0 terminated */
packet->ramblock[255] = 0;
-block = qemu_ram_block_by_name(packet->ramblock);
-if (!block) {
+p->block = qemu_ram_block_by_name(packet->ramblock);
+if (!p->block) {
error_setg(errp, "multifd: unknown ram block %s",
packet->ramblock);
return -1;
}
-p->host = block->host;
+p->host = p->block->host;
for (i = 0; i < p->normal_num; i++) {
uint64_t offset = be64_to_cpu(packet->offset[i]);
-if (offset > (block->used_length - p->page_size)) {
+if (offset > (p->block->used_length - p->page_size)) {
error_setg(errp, "multifd: offset too long %" PRIu64
" (max " RAM_ADDR_FMT ")",
- offset, block->used_length);
+ offset, p->block->used_length);
return -1;
}
p->normal[i] = offset;
diff --git a/migration/multifd.h b/migration/multifd.h
index 7cfc265148..a835643b48 100644
--- a/migration/multifd.h
+++ b/migration/multifd.h
@@ -175,6 +175,8 @@ typedef struct {
uint32_t next_packet_size;
/* packets sent through this channel */
uint64_t num_packets;
+/* ramblock */
+RAMBlock *block;
/* ramblock host address */
uint8_t *host;
/* non zero pages recv through this channel */
--
2.39.2
pgpnfgwbRBpCg.pgp
Description: OpenPGP digital signature
[PATCH 3/5] multifd: Introduce multifd-internal.h
Introduce multifd-internal.h so code that would normally go into
multifd.c can go into an extra file. This way, multifd.c hopefully
won't grow to 4000 lines like ram.c
This will be used in the next commits to add colo support to multifd.
Signed-off-by: Lukas Straub
---
migration/multifd-internal.h | 34 ++
migration/multifd.c | 15 ---
2 files changed, 38 insertions(+), 11 deletions(-)
create mode 100644 migration/multifd-internal.h
diff --git a/migration/multifd-internal.h b/migration/multifd-internal.h
new file mode 100644
index 00..6eeaa028e7
--- /dev/null
+++ b/migration/multifd-internal.h
@@ -0,0 +1,34 @@
+/*
+ * Internal Multifd header
+ *
+ * Copyright (c) 2019-2020 Red Hat Inc
+ *
+ * Authors:
+ * Juan Quintela
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifdef QEMU_MIGRATION_MULTIFD_INTERNAL_H
+#error Only include this header directly
+#endif
+#define QEMU_MIGRATION_MULTIFD_INTERNAL_H
+
+#ifndef MULTIFD_INTERNAL
+#error This header is internal to multifd
+#endif
+
+struct MultiFDRecvState {
+MultiFDRecvParams *params;
+/* number of created threads */
+int count;
+/* syncs main thread and channels */
+QemuSemaphore sem_sync;
+/* global number of generated multifd packets */
+uint64_t packet_num;
+/* multifd ops */
+MultiFDMethods *ops;
+};
+
+extern struct MultiFDRecvState *multifd_recv_state;
diff --git a/migration/multifd.c b/migration/multifd.c
index 4e71c19292..f6bad69b6c 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -31,6 +31,9 @@
#include "io/channel-socket.h"
#include "yank_functions.h"
+#define MULTIFD_INTERNAL
+#include "multifd-internal.h"
+
/* Multiple fd's */
#define MULTIFD_MAGIC 0x11223344U
@@ -967,17 +970,7 @@ int multifd_save_setup(Error **errp)
return 0;
}
-struct {
-MultiFDRecvParams *params;
-/* number of created threads */
-int count;
-/* syncs main thread and channels */
-QemuSemaphore sem_sync;
-/* global number of generated multifd packets */
-uint64_t packet_num;
-/* multifd ops */
-MultiFDMethods *ops;
-} *multifd_recv_state;
+struct MultiFDRecvState *multifd_recv_state;
static void multifd_recv_terminate_threads(Error *err)
{
--
2.39.2
pgp3yx8xIkN_S.pgp
Description: OpenPGP digital signature
[PATCH 0/5] multifd: Add colo support
Hello Everyone, These patches add support for colo to multifd. Lukas Straub (5): ram: Add public helper to set colo bitmap ram: Let colo_flush_ram_cache take the bitmap_mutex multifd: Introduce multifd-internal.h multifd: Introduce a overridable revc_pages method multifd: Add colo support migration/meson.build| 1 + migration/multifd-colo.c | 67 migration/multifd-internal.h | 39 +++ migration/multifd.c | 74 +++- migration/multifd.h | 2 + migration/ram.c | 19 +++-- migration/ram.h | 1 + 7 files changed, 173 insertions(+), 30 deletions(-) create mode 100644 migration/multifd-colo.c create mode 100644 migration/multifd-internal.h -- 2.39.2 pgp_fTl_C5p8k.pgp Description: OpenPGP digital signature
[PATCH 1/5] ram: Add public helper to set colo bitmap
The overhead of the mutex in non-multifd mode is negligible,
because in that case its just the single thread taking the mutex.
This will be used in the next commits to add colo support to multifd.
Signed-off-by: Lukas Straub
---
migration/ram.c | 17 ++---
migration/ram.h | 1 +
2 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index 5e7bf20ca5..2d3fd2112a 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3633,6 +3633,18 @@ static ram_addr_t
host_page_offset_from_ram_block_offset(RAMBlock *block,
return ((uintptr_t)block->host + offset) & (block->page_size - 1);
}
+void colo_record_bitmap(RAMBlock *block, ram_addr_t *normal, uint normal_num)
+{
+qemu_mutex_lock(_state->bitmap_mutex);
+for (int i = 0; i < normal_num; i++) {
+ram_addr_t offset = normal[i];
+ram_state->migration_dirty_pages += !test_and_set_bit(
+offset >> TARGET_PAGE_BITS,
+block->bmap);
+}
+qemu_mutex_unlock(_state->bitmap_mutex);
+}
+
static inline void *colo_cache_from_block_offset(RAMBlock *block,
ram_addr_t offset, bool record_bitmap)
{
@@ -3650,9 +3662,8 @@ static inline void *colo_cache_from_block_offset(RAMBlock
*block,
* It help us to decide which pages in ram cache should be flushed
* into VM's RAM later.
*/
-if (record_bitmap &&
-!test_and_set_bit(offset >> TARGET_PAGE_BITS, block->bmap)) {
-ram_state->migration_dirty_pages++;
+if (record_bitmap) {
+colo_record_bitmap(block, , 1);
}
return block->colo_cache + offset;
}
diff --git a/migration/ram.h b/migration/ram.h
index 6fffbeb5f1..887d1fbae6 100644
--- a/migration/ram.h
+++ b/migration/ram.h
@@ -82,6 +82,7 @@ int colo_init_ram_cache(void);
void colo_flush_ram_cache(void);
void colo_release_ram_cache(void);
void colo_incoming_start_dirty_log(void);
+void colo_record_bitmap(RAMBlock *block, ram_addr_t *normal, uint normal_num);
/* Background snapshot */
bool ram_write_tracking_available(void);
--
2.39.2
pgpaPQOpK3JXV.pgp
Description: OpenPGP digital signature
[PATCH 4/5] multifd: Introduce a overridable revc_pages method
This allows to override the behaviour around recv_pages. Think of
it like a "multifd_colo" child class of multifd.
This will be used in the next commits to add colo support to multifd.
Signed-off-by: Lukas Straub
---
migration/meson.build| 1 +
migration/multifd-colo.c | 39 +
migration/multifd-internal.h | 5
migration/multifd.c | 48
4 files changed, 83 insertions(+), 10 deletions(-)
create mode 100644 migration/multifd-colo.c
diff --git a/migration/meson.build b/migration/meson.build
index da1897fadf..22ab6c6d73 100644
--- a/migration/meson.build
+++ b/migration/meson.build
@@ -23,6 +23,7 @@ softmmu_ss.add(files(
'migration.c',
'multifd.c',
'multifd-zlib.c',
+ 'multifd-colo.c',
'options.c',
'postcopy-ram.c',
'savevm.c',
diff --git a/migration/multifd-colo.c b/migration/multifd-colo.c
new file mode 100644
index 00..c035d15e87
--- /dev/null
+++ b/migration/multifd-colo.c
@@ -0,0 +1,39 @@
+/*
+ * multifd colo implementation
+ *
+ * Copyright (c) Lukas Straub
+ *
+ * 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 "io/channel-socket.h"
+
+#define MULTIFD_INTERNAL
+#include "multifd-internal.h"
+
+static int multifd_colo_recv_pages(MultiFDRecvParams *p, Error **errp)
+{
+return multifd_recv_state->ops->recv_pages(p, errp);
+}
+
+int multifd_colo_load_setup(Error **errp)
+{
+int ret;
+
+ret = _multifd_load_setup(errp);
+if (ret) {
+return ret;
+}
+
+multifd_recv_state->recv_pages = multifd_colo_recv_pages;
+
+return 0;
+}
diff --git a/migration/multifd-internal.h b/migration/multifd-internal.h
index 6eeaa028e7..82357f1d88 100644
--- a/migration/multifd-internal.h
+++ b/migration/multifd-internal.h
@@ -29,6 +29,11 @@ struct MultiFDRecvState {
uint64_t packet_num;
/* multifd ops */
MultiFDMethods *ops;
+/* overridable recv method */
+int (*recv_pages)(MultiFDRecvParams *p, Error **errp);
};
extern struct MultiFDRecvState *multifd_recv_state;
+
+int _multifd_load_setup(Error **errp);
+int multifd_colo_load_setup(Error **errp);
diff --git a/migration/multifd.c b/migration/multifd.c
index f6bad69b6c..fb5e8859de 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -1126,7 +1126,7 @@ static void *multifd_recv_thread(void *opaque)
qemu_mutex_unlock(>mutex);
if (p->normal_num) {
-ret = multifd_recv_state->ops->recv_pages(p, _err);
+ret = multifd_recv_state->recv_pages(p, _err);
if (ret != 0) {
break;
}
@@ -1152,20 +1152,12 @@ static void *multifd_recv_thread(void *opaque)
return NULL;
}
-int multifd_load_setup(Error **errp)
+int _multifd_load_setup(Error **errp)
{
int thread_count;
uint32_t page_count = MULTIFD_PACKET_SIZE / qemu_target_page_size();
uint8_t i;
-/*
- * Return successfully if multiFD recv state is already initialised
- * or multiFD is not enabled.
- */
-if (multifd_recv_state || !migrate_multifd()) {
-return 0;
-}
-
thread_count = migrate_multifd_channels();
multifd_recv_state = g_malloc0(sizeof(*multifd_recv_state));
multifd_recv_state->params = g_new0(MultiFDRecvParams, thread_count);
@@ -1204,6 +1196,42 @@ int multifd_load_setup(Error **errp)
return 0;
}
+static int multifd_normal_recv_pages(MultiFDRecvParams *p, Error **errp)
+{
+return multifd_recv_state->ops->recv_pages(p, errp);
+}
+
+static int multifd_normal_load_setup(Error **errp)
+{
+int ret;
+
+ret = _multifd_load_setup(errp);
+if (ret) {
+return ret;
+}
+
+multifd_recv_state->recv_pages = multifd_normal_recv_pages;
+
+return 0;
+}
+
+int multifd_load_setup(Error **errp)
+{
+/*
+ * Return successfully if multiFD recv state is already initialised
+ * or multiFD is not enabled.
+ */
+if (multifd_recv_state || !migrate_multifd()) {
+return 0;
+}
+
+if (migrate_colo()) {
+return multifd_colo_load_setup(errp);
+} else {
+return multifd_normal_load_setup(errp);
+}
+}
+
bool multifd_recv_all_channels_created(void)
{
int thread_count = migrate_multifd_channels();
--
2.39.2
pgpGZXBJHUHgp.pgp
Description: OpenPGP digital signature
[PATCH 2/5] ram: Let colo_flush_ram_cache take the bitmap_mutex
This will be used in the next commits to add colo support to multifd.
Signed-off-by: Lukas Straub
---
migration/ram.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/migration/ram.c b/migration/ram.c
index 2d3fd2112a..f9e7aeda12 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -4230,6 +4230,7 @@ void colo_flush_ram_cache(void)
unsigned long offset = 0;
memory_global_dirty_log_sync();
+qemu_mutex_lock(_state->bitmap_mutex);
WITH_RCU_READ_LOCK_GUARD() {
RAMBLOCK_FOREACH_NOT_IGNORED(block) {
ramblock_sync_dirty_bitmap(ram_state, block);
@@ -4264,6 +4265,7 @@ void colo_flush_ram_cache(void)
}
}
}
+qemu_mutex_unlock(_state->bitmap_mutex);
trace_colo_flush_ram_cache_end();
}
--
2.39.2
pgpHvMrbbIh6t.pgp
Description: OpenPGP digital signature
Re: [PATCH v4 10/10] migration: block incoming colo when capability is disabled
On Fri, 5 May 2023 01:30:34 +0300
Vladimir Sementsov-Ogievskiy wrote:
> On 05.05.23 01:10, Lukas Straub wrote:
> > On Fri, 28 Apr 2023 22:49:28 +0300
> > Vladimir Sementsov-Ogievskiy wrote:
> >
> >> We generally require same set of capabilities on source and target.
> >> Let's require x-colo capability to use COLO on target.
> >>
> >> Signed-off-by: Vladimir Sementsov-Ogievskiy
> >
> > Good patch, this is needed anyway for COLO with multifd.
> >
> > Also, it allows to remove a some code, like
> > migration_incoming_enable_colo(), qemu_savevm_send_colo_enable() etc.
> > I will send patches for that.
>
> Great! But with such changes we should be careful to not break compatibility
> between versions.. On the other hand, x-colo - is still experimental with
> that x-, so there is no guarantee how it works.
Given COLO's usecase, I think it should only be run with the same qemu
version on both sides anyway. Maybe we should even enforce that
somehow, while we're at it doing breaking changes.
For upgrading qemu without downtime, normal migration can still be used.
Zhang Cheng, what do you think?
> > Or you can do it if you like.
>
> To be honest, I don't :)
>
> >
> > Please update the docs like below, then:
> > Reviewed-by: Lukas Straub
> >
> > diff --git a/docs/COLO-FT.txt b/docs/COLO-FT.txt
> > index 8ec653f81c..2e760a4aee 100644
> > --- a/docs/COLO-FT.txt
> > +++ b/docs/COLO-FT.txt
> > @@ -210,6 +210,7 @@ children.0=childs0 \
> >
> > 3. On Secondary VM's QEMU monitor, issue command
> > {"execute":"qmp_capabilities"}
> > +{"execute": "migrate-set-capabilities", "arguments": {"capabilities": [
> > {"capability": "x-colo", "state": true } ] } }
> > {"execute": "nbd-server-start", "arguments": {"addr": {"type": "inet",
> > "data": {"host": "0.0.0.0", "port": ""} } } }
> > {"execute": "nbd-server-add", "arguments": {"device": "parent0",
> > "writable": true } }
> >
>
> I'll resend with this addition, thanks for reviewing!
>
> >
> >> ---
> >> migration/migration.c | 6 ++
> >> 1 file changed, 6 insertions(+)
> >>
> >> diff --git a/migration/migration.c b/migration/migration.c
> >> index 8c5bbf3e94..5e162c0622 100644
> >> --- a/migration/migration.c
> >> +++ b/migration/migration.c
> >> @@ -395,6 +395,12 @@ int migration_incoming_enable_colo(void)
> >> return -ENOTSUP;
> >> #endif
> >>
> >> +if (!migrate_colo()) {
> >> +error_report("ENABLE_COLO command come in migration stream, but
> >> c-colo "
> >> + "capability is not set");
> >> +return -EINVAL;
> >> +}
> >> +
> >> if (ram_block_discard_disable(true)) {
> >> error_report("COLO: cannot disable RAM discard");
> >> return -EBUSY;
> >
> >
> >
>
--
pgpI9ICjZOOON.pgp
Description: OpenPGP digital signature
Re: [PATCH v4 10/10] migration: block incoming colo when capability is disabled
On Fri, 28 Apr 2023 22:49:28 +0300
Vladimir Sementsov-Ogievskiy wrote:
> We generally require same set of capabilities on source and target.
> Let's require x-colo capability to use COLO on target.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
Good patch, this is needed anyway for COLO with multifd.
Also, it allows to remove a some code, like
migration_incoming_enable_colo(), qemu_savevm_send_colo_enable() etc.
I will send patches for that. Or you can do it if you like.
Please update the docs like below, then:
Reviewed-by: Lukas Straub
diff --git a/docs/COLO-FT.txt b/docs/COLO-FT.txt
index 8ec653f81c..2e760a4aee 100644
--- a/docs/COLO-FT.txt
+++ b/docs/COLO-FT.txt
@@ -210,6 +210,7 @@ children.0=childs0 \
3. On Secondary VM's QEMU monitor, issue command
{"execute":"qmp_capabilities"}
+{"execute": "migrate-set-capabilities", "arguments": {"capabilities": [
{"capability": "x-colo", "state": true } ] } }
{"execute": "nbd-server-start", "arguments": {"addr": {"type": "inet", "data":
{"host": "0.0.0.0", "port": ""} } } }
{"execute": "nbd-server-add", "arguments": {"device": "parent0", "writable":
true } }
> ---
> migration/migration.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 8c5bbf3e94..5e162c0622 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -395,6 +395,12 @@ int migration_incoming_enable_colo(void)
> return -ENOTSUP;
> #endif
>
> +if (!migrate_colo()) {
> +error_report("ENABLE_COLO command come in migration stream, but
> c-colo "
> + "capability is not set");
> +return -EINVAL;
> +}
> +
> if (ram_block_discard_disable(true)) {
> error_report("COLO: cannot disable RAM discard");
> return -EBUSY;
--
pgpjYhhH7vk31.pgp
Description: OpenPGP digital signature
Re: [PULL 00/21] Migration 20230428 patches
On Sat, 29 Apr 2023 19:45:07 +0100 Richard Henderson wrote: > On 4/28/23 20:11, Juan Quintela wrote: > > The following changes since commit 05d50ba2d4668d43a835c5a502efdec9b92646e6: > > > >Merge tag 'migration-20230427-pull-request' of > > https://gitlab.com/juan.quintela/qemu into staging (2023-04-28 08:35:06 > > +0100) > > > > are available in the Git repository at: > > > >https://gitlab.com/juan.quintela/qemu.git > > tags/migration-20230428-pull-request > > > > for you to fetch changes up to 05ecac612ec6a4bdb358e68554b4406ac2c8e25a: > > > >migration: Initialize and cleanup decompression in migration.c > > (2023-04-28 20:54:53 +0200) > > > > > > Migration Pull request (20230429 vintage) > > > > Hi > > > > In this series: > > - compression code cleanup (lukas) > >nice, nice, nice. > > - drop useless parameters from migration_tls* (juan) > > - first part of remove QEMUFileHooks series (juan) > > > > Please apply. > > > > > > > > Juan Quintela (8): > >multifd: We already account for this packet on the multifd thread > >migration: Move ram_stats to its own file migration-stats.[ch] > >migration: Rename ram_counters to mig_stats > >migration: Rename RAMStats to MigrationAtomicStats > >migration/rdma: Split the zero page case from acct_update_position > >migration/rdma: Unfold last user of acct_update_position() > >migration: Drop unused parameter for migration_tls_get_creds() > >migration: Drop unused parameter for migration_tls_client_create() > > > > Lukas Straub (13): > >qtest/migration-test.c: Add tests with compress enabled > >qtest/migration-test.c: Add postcopy tests with compress enabled > >ram.c: Let the compress threads return a CompressResult enum > >ram.c: Dont change param->block in the compress thread > >ram.c: Reset result after sending queued data > >ram.c: Do not call save_page_header() from compress threads > >ram.c: Call update_compress_thread_counts from > > compress_send_queued_data > >ram.c: Remove last ram.c dependency from the core compress code > >ram.c: Move core compression code into its own file > >ram.c: Move core decompression code into its own file > >ram compress: Assert that the file buffer matches the result > >ram-compress.c: Make target independent > >migration: Initialize and cleanup decompression in migration.c > > There are a bunch of migration failures in CI: > > https://gitlab.com/qemu-project/qemu/-/jobs/4201998343#L375 > https://gitlab.com/qemu-project/qemu/-/jobs/4201998342#L428 > https://gitlab.com/qemu-project/qemu/-/jobs/4201998340#L459 > https://gitlab.com/qemu-project/qemu/-/jobs/4201998336#L4883 > > > r~ Hmm, it doesn't always fail. Any way to get the testlog from the failed jobs? -- pgp4LdaxTgzzk.pgp Description: OpenPGP digital signature
Re: [PATCH 17/17] qapi: Reformat doc comments to conform to current conventions
On Fri, 28 Apr 2023 12:54:29 +0200 Markus Armbruster wrote: > Change > > # @name: Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed > #do eiusmod tempor incididunt ut labore et dolore magna aliqua. > > to > > # @name: Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed > # do eiusmod tempor incididunt ut labore et dolore magna aliqua. > > See recent commit "qapi: Relax doc string @name: description > indentation rules" for rationale. > > Reflow paragraphs to 70 columns width, and consistently use two spaces > to separate sentences. > > To check the generated documentation does not change, I compared the > generated HTML before and after this commit with "wdiff -3". Finds no > differences. Comparing with diff is not useful, as the reflown > paragraphs are visible there. > > Signed-off-by: Markus Armbruster Acked-by: Lukas Straub > --- > qapi/acpi.json | 50 +- > qapi/audio.json | 85 +- > qapi/authz.json | 29 +- > qapi/block-core.json | 2801 -- > qapi/block-export.json | 242 ++-- > qapi/block.json | 214 +-- > qapi/char.json | 134 +- > qapi/common.json | 19 +- > qapi/compat.json | 13 +- > qapi/control.json| 59 +- > qapi/crypto.json | 261 ++-- > qapi/cryptodev.json |3 + > qapi/cxl.json| 74 +- > qapi/dump.json | 78 +- > qapi/error.json |6 +- > qapi/introspect.json | 89 +- > qapi/job.json| 139 +- > qapi/machine-target.json | 303 +++-- > qapi/machine.json| 389 +++--- > qapi/migration.json | 1117 --- > qapi/misc-target.json| 67 +- > qapi/misc.json | 180 ++- > qapi/net.json| 260 ++-- > qapi/pci.json| 35 +- > qapi/qapi-schema.json| 25 +- > qapi/qdev.json | 63 +- > qapi/qom.json| 404 +++--- > qapi/rdma.json |1 - > qapi/replay.json | 48 +- > qapi/rocker.json | 20 +- > qapi/run-state.json | 215 +-- > qapi/sockets.json| 50 +- > qapi/stats.json | 83 +- > qapi/tpm.json| 20 +- > qapi/trace.json | 34 +- > qapi/transaction.json| 87 +- > qapi/ui.json | 435 +++--- > qapi/virtio.json | 84 +- > qapi/yank.json | 42 +- > 39 files changed, 4322 insertions(+), 3936 deletions(-) > > [...] > > diff --git a/qapi/yank.json b/qapi/yank.json > index 1639744ada..87ec7cab96 100644 > --- a/qapi/yank.json > +++ b/qapi/yank.json > @@ -9,7 +9,7 @@ > ## > # @YankInstanceType: > # > -# An enumeration of yank instance types. See @YankInstance for more > +# An enumeration of yank instance types. See @YankInstance for more > # information. > # > # Since: 6.0 > @@ -20,8 +20,8 @@ > ## > # @YankInstanceBlockNode: > # > -# Specifies which block graph node to yank. See @YankInstance for more > -# information. > +# Specifies which block graph node to yank. See @YankInstance for > +# more information. > # > # @node-name: the name of the block graph node > # > @@ -33,8 +33,8 @@ > ## > # @YankInstanceChardev: > # > -# Specifies which character device to yank. See @YankInstance for more > -# information. > +# Specifies which character device to yank. See @YankInstance for > +# more information. > # > # @id: the chardev's ID > # > @@ -46,21 +46,18 @@ > ## > # @YankInstance: > # > -# A yank instance can be yanked with the @yank qmp command to recover from a > -# hanging QEMU. > +# A yank instance can be yanked with the @yank qmp command to recover > +# from a hanging QEMU. > # > # Currently implemented yank instances: > # > -# - nbd block device: > -# Yanking it will shut down the connection to the nbd server without > -# attempting to reconnect. > -# - socket chardev: > -# Yanking it will shut down the connected socket. > -# - migration: > -# Yanking it will shut down all migration connections. Unlike > -# @migrate_cancel, it will not notify the migration process, so migration > -# will go into @failed state, instead of @cancelled state. @yank should be > -# used to recover from hangs. > +# - nbd block device: Yanking it will shut down the connection to the > +# nbd server without attempting to reconnect. > +# - socket chardev: Yanking it will shut down the connected socket. > +# - migration: Yanking it will shut down all migration connections. > +# Unlike @migrate_cancel, it will n
Re: [PATCH v3 4/4] configure: add --disable-colo-proxy option
On Thu, 27 Apr 2023 23:29:46 +0300
Vladimir Sementsov-Ogievskiy wrote:
> Add option to not build filter-mirror, filter-rewriter and
> colo-compare when they are not needed.
>
> There could be more agile configuration, for example add separate
> options for each filter, but that may be done in future on demand. The
> aim of this patch is to make possible to disable the whole COLO Proxy
> subsystem.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
> meson_options.txt | 2 ++
> net/meson.build | 14 ++
> scripts/meson-buildoptions.sh | 3 +++
> stubs/colo-compare.c | 7 +++
> stubs/meson.build | 1 +
> 5 files changed, 23 insertions(+), 4 deletions(-)
> create mode 100644 stubs/colo-compare.c
>
> diff --git a/meson_options.txt b/meson_options.txt
> index 2471dd02da..b59e7ae342 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -289,6 +289,8 @@ option('live_block_migration', type: 'feature', value:
> 'auto',
> description: 'block migration in the main migration stream')
> option('replication', type: 'feature', value: 'auto',
> description: 'replication support')
> +option('colo_proxy', type: 'feature', value: 'auto',
> + description: 'colo-proxy support')
> option('bochs', type: 'feature', value: 'auto',
> description: 'bochs image format support')
> option('cloop', type: 'feature', value: 'auto',
> diff --git a/net/meson.build b/net/meson.build
> index 87afca3e93..4cfc850c69 100644
> --- a/net/meson.build
> +++ b/net/meson.build
> @@ -1,13 +1,9 @@
> softmmu_ss.add(files(
>'announce.c',
>'checksum.c',
> - 'colo-compare.c',
> - 'colo.c',
>'dump.c',
>'eth.c',
>'filter-buffer.c',
> - 'filter-mirror.c',
> - 'filter-rewriter.c',
>'filter.c',
>'hub.c',
>'net-hmp-cmds.c',
> @@ -19,6 +15,16 @@ softmmu_ss.add(files(
>'util.c',
> ))
>
> +if get_option('replication').allowed() or \
> +get_option('colo_proxy').allowed()
> + softmmu_ss.add(files('colo-compare.c'))
> + softmmu_ss.add(files('colo.c'))
> +endif
> +
> +if get_option('colo_proxy').allowed()
> + softmmu_ss.add(files('filter-mirror.c', 'filter-rewriter.c'))
> +endif
> +
The last discussion didn't really come to a conclusion, but I still
think that 'filter-mirror.c' (which also contains filter-redirect)
should be left unchanged.
> softmmu_ss.add(when: 'CONFIG_TCG', if_true: files('filter-replay.c'))
>
> if have_l2tpv3
> diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
> index d4369a3ad8..036047ce6f 100644
> --- a/scripts/meson-buildoptions.sh
> +++ b/scripts/meson-buildoptions.sh
> @@ -83,6 +83,7 @@ meson_options_help() {
>printf "%s\n" ' capstoneWhether and how to find the capstone
> library'
>printf "%s\n" ' cloop cloop image format support'
>printf "%s\n" ' cocoa Cocoa user interface (macOS only)'
> + printf "%s\n" ' colo-proxy colo-proxy support'
>printf "%s\n" ' coreaudio CoreAudio sound support'
>printf "%s\n" ' crypto-afalgLinux AF_ALG crypto backend driver'
>printf "%s\n" ' curlCURL block device driver'
> @@ -236,6 +237,8 @@ _meson_option_parse() {
> --disable-cloop) printf "%s" -Dcloop=disabled ;;
> --enable-cocoa) printf "%s" -Dcocoa=enabled ;;
> --disable-cocoa) printf "%s" -Dcocoa=disabled ;;
> +--enable-colo-proxy) printf "%s" -Dcolo_proxy=enabled ;;
> +--disable-colo-proxy) printf "%s" -Dcolo_proxy=disabled ;;
> --enable-coreaudio) printf "%s" -Dcoreaudio=enabled ;;
> --disable-coreaudio) printf "%s" -Dcoreaudio=disabled ;;
> --enable-coroutine-pool) printf "%s" -Dcoroutine_pool=true ;;
> diff --git a/stubs/colo-compare.c b/stubs/colo-compare.c
> new file mode 100644
> index 00..ec726665be
> --- /dev/null
> +++ b/stubs/colo-compare.c
> @@ -0,0 +1,7 @@
> +#include "qemu/osdep.h"
> +#include "qemu/notify.h"
> +#include "net/colo-compare.h"
> +
> +void colo_compare_cleanup(void)
> +{
> +}
> diff --git a/stubs/meson.build b/stubs/meson.build
> index 8412cad15f..a56645e2f7 100644
> --- a/stubs/meson.build
> +++ b/stubs/meson.build
> @@ -46,6 +46,7 @@ stub_ss.add(files('target-monitor-defs.c'))
> stub_ss.add(files('trace-control.c'))
> stub_ss.add(files('uuid.c'))
> stub_ss.add(files('colo.c'))
> +stub_ss.add(files('colo-compare.c'))
> stub_ss.add(files('vmstate.c'))
> stub_ss.add(files('vm-stop.c'))
> stub_ss.add(files('win32-kbd-hook.c'))
--
pgpY19OfJBwmH.pgp
Description: OpenPGP digital signature
Re: [PATCH v3 3/4] build: move COLO under CONFIG_REPLICATION
On Thu, 27 Apr 2023 23:29:45 +0300
Vladimir Sementsov-Ogievskiy wrote:
> We don't allow to use x-colo capability when replication is not
> configured. So, no reason to build COLO when replication is disabled,
> it's unusable in this case.
>
> Note also that the check in migrate_caps_check() is not the only
> restriction: some functions in migration/colo.c will just abort if
> called with not defined CONFIG_REPLICATION, for example:
>
> migration_iteration_finish()
>case MIGRATION_STATUS_COLO:
>migrate_start_colo_process()
>colo_process_checkpoint()
>abort()
>
> It could probably make sense to have possibility to enable COLO without
> REPLICATION, but this requires deeper audit of colo & replication code,
> which may be done later if needed.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Lukas Straub
> ---
> hmp-commands.hx| 2 ++
> migration/colo.c | 28 -
> migration/meson.build | 6 --
> migration/migration-hmp-cmds.c | 2 ++
> migration/options.c| 17
> qapi/migration.json| 12 +++
> stubs/colo.c | 37 ++
> stubs/meson.build | 1 +
> 8 files changed, 62 insertions(+), 43 deletions(-)
> create mode 100644 stubs/colo.c
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index bb85ee1d26..fbd0932232 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1035,6 +1035,7 @@ SRST
>migration (or once already in postcopy).
> ERST
>
> +#ifdef CONFIG_REPLICATION
> {
> .name = "x_colo_lost_heartbeat",
> .args_type = "",
> @@ -1043,6 +1044,7 @@ ERST
>"a failover or takeover is needed.",
> .cmd = hmp_x_colo_lost_heartbeat,
> },
> +#endif
>
> SRST
> ``x_colo_lost_heartbeat``
> diff --git a/migration/colo.c b/migration/colo.c
> index 07bfa21fea..e4af47eeeb 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -26,9 +26,7 @@
> #include "qemu/rcu.h"
> #include "migration/failover.h"
> #include "migration/ram.h"
> -#ifdef CONFIG_REPLICATION
> #include "block/replication.h"
> -#endif
> #include "net/colo-compare.h"
> #include "net/colo.h"
> #include "block/block.h"
> @@ -68,7 +66,6 @@ static bool colo_runstate_is_stopped(void)
> static void secondary_vm_do_failover(void)
> {
> /* COLO needs enable block-replication */
> -#ifdef CONFIG_REPLICATION
> int old_state;
> MigrationIncomingState *mis = migration_incoming_get_current();
> Error *local_err = NULL;
> @@ -133,14 +130,10 @@ static void secondary_vm_do_failover(void)
> if (mis->migration_incoming_co) {
> qemu_coroutine_enter(mis->migration_incoming_co);
> }
> -#else
> -abort();
> -#endif
> }
>
> static void primary_vm_do_failover(void)
> {
> -#ifdef CONFIG_REPLICATION
> MigrationState *s = migrate_get_current();
> int old_state;
> Error *local_err = NULL;
> @@ -181,9 +174,6 @@ static void primary_vm_do_failover(void)
>
> /* Notify COLO thread that failover work is finished */
> qemu_sem_post(>colo_exit_sem);
> -#else
> -abort();
> -#endif
> }
>
> COLOMode get_colo_mode(void)
> @@ -217,7 +207,6 @@ void colo_do_failover(void)
> }
> }
>
> -#ifdef CONFIG_REPLICATION
> void qmp_xen_set_replication(bool enable, bool primary,
> bool has_failover, bool failover,
> Error **errp)
> @@ -271,7 +260,6 @@ void qmp_xen_colo_do_checkpoint(Error **errp)
> /* Notify all filters of all NIC to do checkpoint */
> colo_notify_filters_event(COLO_EVENT_CHECKPOINT, errp);
> }
> -#endif
>
> COLOStatus *qmp_query_colo_status(Error **errp)
> {
> @@ -435,15 +423,11 @@ static int
> colo_do_checkpoint_transaction(MigrationState *s,
> }
> qemu_mutex_lock_iothread();
>
> -#ifdef CONFIG_REPLICATION
> replication_do_checkpoint_all(_err);
> if (local_err) {
> qemu_mutex_unlock_iothread();
> goto out;
> }
> -#else
> -abort();
> -#endif
>
> colo_send_message(s->to_dst_file, COLO_MESSAGE_VMSTATE_SEND, _err);
> if (local_err) {
> @@ -561,15 +545,11 @@ static void colo_process_checkpoint(MigrationState *s)
> object_unref(OBJECT(bioc));
>
> qemu_mutex_lock_iothread();
> -#ifdef CONFIG
Re: [PATCH v3 1/4] block/meson.build: prefer positive condition for replication
On Thu, 27 Apr 2023 23:29:43 +0300
Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> Reviewed-by: Juan Quintela
> Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Lukas Straub
> ---
> block/meson.build | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/block/meson.build b/block/meson.build
> index 382bec0e7d..b9a72e219b 100644
> --- a/block/meson.build
> +++ b/block/meson.build
> @@ -84,7 +84,7 @@ block_ss.add(when: 'CONFIG_WIN32', if_true:
> files('file-win32.c', 'win32-aio.c')
> block_ss.add(when: 'CONFIG_POSIX', if_true: [files('file-posix.c'), coref,
> iokit])
> block_ss.add(when: libiscsi, if_true: files('iscsi-opts.c'))
> block_ss.add(when: 'CONFIG_LINUX', if_true: files('nvme.c'))
> -if not get_option('replication').disabled()
> +if get_option('replication').allowed()
>block_ss.add(files('replication.c'))
> endif
> block_ss.add(when: libaio, if_true: files('linux-aio.c'))
--
pgpgT54Ez08iK.pgp
Description: OpenPGP digital signature
Re: [PATCH 06/19] migration/rdma: Unfold last user of acct_update_position()
On Thu, 27 Apr 2023 18:34:36 +0200
Juan Quintela wrote:
> Signed-off-by: Juan Quintela
Reviewed-by: Lukas Straub
> ---
> migration/ram.c | 9 -
> migration/ram.h | 1 -
> migration/rdma.c | 4 +++-
> 3 files changed, 3 insertions(+), 11 deletions(-)
>
> diff --git a/migration/ram.c b/migration/ram.c
> index c249a1f468..7d81c4a39e 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -2629,15 +2629,6 @@ static int ram_find_and_save_block(RAMState *rs)
> return pages;
> }
>
> -void acct_update_position(QEMUFile *f, size_t size)
> -{
> -uint64_t pages = size / TARGET_PAGE_SIZE;
> -
> -stat64_add(_stats.normal_pages, pages);
> -ram_transferred_add(size);
> -qemu_file_credit_transfer(f, size);
> -}
> -
> static uint64_t ram_bytes_total_with_ignored(void)
> {
> RAMBlock *block;
> diff --git a/migration/ram.h b/migration/ram.h
> index 3804753ca3..6fffbeb5f1 100644
> --- a/migration/ram.h
> +++ b/migration/ram.h
> @@ -53,7 +53,6 @@ void mig_throttle_counter_reset(void);
>
> uint64_t ram_pagesize_summary(void);
> int ram_save_queue_pages(const char *rbname, ram_addr_t start, ram_addr_t
> len);
> -void acct_update_position(QEMUFile *f, size_t size);
> void ram_postcopy_migrated_memory_release(MigrationState *ms);
> /* For outgoing discard bitmap */
> void ram_postcopy_send_discard_bitmap(MigrationState *ms);
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 7a9b284c3f..7e747b2595 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -2231,7 +2231,9 @@ retry:
> }
>
> set_bit(chunk, block->transit_bitmap);
> -acct_update_position(f, sge.length);
> +stat64_add(_stats.normal_pages, sge.length /
> qemu_target_page_size());
> +ram_transferred_add(sge.length);
> +qemu_file_credit_transfer(f, sge.length);
> rdma->total_writes++;
>
> return 0;
--
pgpgacnKIsu4F.pgp
Description: OpenPGP digital signature
Re: [PATCH 05/19] migration/rdma: Split the zero page case from acct_update_position
On Thu, 27 Apr 2023 18:34:35 +0200
Juan Quintela wrote:
> Now that we have atomic counters, we can do it on the place that we
> need it, no need to do it inside ram.c.
>
> Signed-off-by: Juan Quintela
Reviewed-by: Lukas Straub
> ---
> migration/ram.c | 12
> migration/ram.h | 2 +-
> migration/rdma.c | 7 +--
> 3 files changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/migration/ram.c b/migration/ram.c
> index c3981f64e4..c249a1f468 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -2629,17 +2629,13 @@ static int ram_find_and_save_block(RAMState *rs)
> return pages;
> }
>
> -void acct_update_position(QEMUFile *f, size_t size, bool zero)
> +void acct_update_position(QEMUFile *f, size_t size)
> {
> uint64_t pages = size / TARGET_PAGE_SIZE;
>
> -if (zero) {
> -stat64_add(_stats.zero_pages, pages);
> -} else {
> -stat64_add(_stats.normal_pages, pages);
> -ram_transferred_add(size);
> -qemu_file_credit_transfer(f, size);
> -}
> +stat64_add(_stats.normal_pages, pages);
> +ram_transferred_add(size);
> +qemu_file_credit_transfer(f, size);
> }
>
> static uint64_t ram_bytes_total_with_ignored(void)
> diff --git a/migration/ram.h b/migration/ram.h
> index 8692de6ba0..3804753ca3 100644
> --- a/migration/ram.h
> +++ b/migration/ram.h
> @@ -53,7 +53,7 @@ void mig_throttle_counter_reset(void);
>
> uint64_t ram_pagesize_summary(void);
> int ram_save_queue_pages(const char *rbname, ram_addr_t start, ram_addr_t
> len);
> -void acct_update_position(QEMUFile *f, size_t size, bool zero);
> +void acct_update_position(QEMUFile *f, size_t size);
> void ram_postcopy_migrated_memory_release(MigrationState *ms);
> /* For outgoing discard bitmap */
> void ram_postcopy_send_discard_bitmap(MigrationState *ms);
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 0af5e944f0..7a9b284c3f 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -17,8 +17,10 @@
> #include "qemu/osdep.h"
> #include "qapi/error.h"
> #include "qemu/cutils.h"
> +#include "exec/target_page.h"
> #include "rdma.h"
> #include "migration.h"
> +#include "migration-stats.h"
> #include "qemu-file.h"
> #include "ram.h"
> #include "qemu/error-report.h"
> @@ -2120,7 +2122,8 @@ retry:
> return -EIO;
> }
>
> -acct_update_position(f, sge.length, true);
> +stat64_add(_stats.zero_pages,
> + sge.length / qemu_target_page_size());
>
> return 1;
> }
> @@ -2228,7 +2231,7 @@ retry:
> }
>
> set_bit(chunk, block->transit_bitmap);
> -acct_update_position(f, sge.length, false);
> +acct_update_position(f, sge.length);
> rdma->total_writes++;
>
> return 0;
--
pgpnzzlkFwkoC.pgp
Description: OpenPGP digital signature
Re: [PATCH 04/19] migration: Rename RAMStats to MigrationAtomicStats
On Thu, 27 Apr 2023 18:34:34 +0200
Juan Quintela wrote:
> It is lousely based on MigrationStats, but that name is taken, so this
> is the best one that I came with.
>
> Signed-off-by: Juan Quintela
Reviewed-by: Lukas Straub
> ---
>
> If you have any good suggestion for the name, I am all ears.
> ---
> migration/migration-stats.c | 2 +-
> migration/migration-stats.h | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/migration/migration-stats.c b/migration/migration-stats.c
> index 8c0af9b80a..2f2cea965c 100644
> --- a/migration/migration-stats.c
> +++ b/migration/migration-stats.c
> @@ -14,4 +14,4 @@
> #include "qemu/stats64.h"
> #include "migration-stats.h"
>
> -RAMStats mig_stats;
> +MigrationAtomicStats mig_stats;
> diff --git a/migration/migration-stats.h b/migration/migration-stats.h
> index 197374b4f6..149af932d7 100644
> --- a/migration/migration-stats.h
> +++ b/migration/migration-stats.h
> @@ -34,8 +34,8 @@ typedef struct {
> Stat64 postcopy_requests;
> Stat64 precopy_bytes;
> Stat64 transferred;
> -} RAMStats;
> +} MigrationAtomicStats;
>
> -extern RAMStats mig_stats;
> +extern MigrationAtomicStats mig_stats;
>
> #endif
--
pgp8vEdIsWNgM.pgp
Description: OpenPGP digital signature
Re: [PATCH 03/19] migration: Rename ram_counters to mig_stats
On Thu, 27 Apr 2023 18:34:33 +0200
Juan Quintela wrote:
> migration_stats is just too long, and it is going to have more than
> ram counters in the near future.
>
> Signed-off-by: Juan Quintela
Reviewed-by: Lukas Straub
> ---
> migration/migration-stats.c | 2 +-
> migration/migration-stats.h | 2 +-
> migration/migration.c | 32 -
> migration/multifd.c | 6 ++---
> migration/ram.c | 48 ++---
> migration/savevm.c | 2 +-
> 6 files changed, 46 insertions(+), 46 deletions(-)
>
> diff --git a/migration/migration-stats.c b/migration/migration-stats.c
> index b0eb5ae73c..8c0af9b80a 100644
> --- a/migration/migration-stats.c
> +++ b/migration/migration-stats.c
> @@ -14,4 +14,4 @@
> #include "qemu/stats64.h"
> #include "migration-stats.h"
>
> -RAMStats ram_counters;
> +RAMStats mig_stats;
> diff --git a/migration/migration-stats.h b/migration/migration-stats.h
> index 2edea0c779..197374b4f6 100644
> --- a/migration/migration-stats.h
> +++ b/migration/migration-stats.h
> @@ -36,6 +36,6 @@ typedef struct {
> Stat64 transferred;
> } RAMStats;
>
> -extern RAMStats ram_counters;
> +extern RAMStats mig_stats;
>
> #endif
> diff --git a/migration/migration.c b/migration/migration.c
> index 5ecf3dc381..feb5ab7493 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -909,26 +909,26 @@ static void populate_ram_info(MigrationInfo *info,
> MigrationState *s)
> size_t page_size = qemu_target_page_size();
>
> info->ram = g_malloc0(sizeof(*info->ram));
> -info->ram->transferred = stat64_get(_counters.transferred);
> +info->ram->transferred = stat64_get(_stats.transferred);
> info->ram->total = ram_bytes_total();
> -info->ram->duplicate = stat64_get(_counters.zero_pages);
> +info->ram->duplicate = stat64_get(_stats.zero_pages);
> /* legacy value. It is not used anymore */
> info->ram->skipped = 0;
> -info->ram->normal = stat64_get(_counters.normal_pages);
> +info->ram->normal = stat64_get(_stats.normal_pages);
> info->ram->normal_bytes = info->ram->normal * page_size;
> info->ram->mbps = s->mbps;
> info->ram->dirty_sync_count =
> -stat64_get(_counters.dirty_sync_count);
> +stat64_get(_stats.dirty_sync_count);
> info->ram->dirty_sync_missed_zero_copy =
> -stat64_get(_counters.dirty_sync_missed_zero_copy);
> +stat64_get(_stats.dirty_sync_missed_zero_copy);
> info->ram->postcopy_requests =
> -stat64_get(_counters.postcopy_requests);
> +stat64_get(_stats.postcopy_requests);
> info->ram->page_size = page_size;
> -info->ram->multifd_bytes = stat64_get(_counters.multifd_bytes);
> +info->ram->multifd_bytes = stat64_get(_stats.multifd_bytes);
> info->ram->pages_per_second = s->pages_per_second;
> -info->ram->precopy_bytes = stat64_get(_counters.precopy_bytes);
> -info->ram->downtime_bytes = stat64_get(_counters.downtime_bytes);
> -info->ram->postcopy_bytes = stat64_get(_counters.postcopy_bytes);
> +info->ram->precopy_bytes = stat64_get(_stats.precopy_bytes);
> +info->ram->downtime_bytes = stat64_get(_stats.downtime_bytes);
> +info->ram->postcopy_bytes = stat64_get(_stats.postcopy_bytes);
>
> if (migrate_xbzrle()) {
> info->xbzrle_cache = g_malloc0(sizeof(*info->xbzrle_cache));
> @@ -960,7 +960,7 @@ static void populate_ram_info(MigrationInfo *info,
> MigrationState *s)
> if (s->state != MIGRATION_STATUS_COMPLETED) {
> info->ram->remaining = ram_bytes_remaining();
> info->ram->dirty_pages_rate =
> - stat64_get(_counters.dirty_pages_rate);
> + stat64_get(_stats.dirty_pages_rate);
> }
> }
>
> @@ -1613,10 +1613,10 @@ static bool migrate_prepare(MigrationState *s, bool
> blk, bool blk_inc,
>
> migrate_init(s);
> /*
> - * set ram_counters compression_counters memory to zero for a
> + * set mig_stats compression_counters memory to zero for a
> * new migration
> */
> -memset(_counters, 0, sizeof(ram_counters));
> +memset(_stats, 0, sizeof(mig_stats));
> memset(_counters, 0, sizeof(compression_counters));
>
> return true;
> @@ -2627,7 +2627,7 @@ static MigThrError
> migration_detect_error(MigrationState *s)
> static uint64_t migration_total_bytes(MigrationState *s)
> {
> return qemu_file_total_tr
Re: [PATCH 02/19] migration: Move ram_stats to its own file migration-stats.[ch]
On Thu, 27 Apr 2023 18:34:32 +0200
Juan Quintela wrote:
> There is already include/qemu/stats.h, so stats.h was a bad idea.
> We want this file to not depend on anything else, we will move all the
> migration counters/stats to this struct.
>
> Signed-off-by: Juan Quintela
Reviewed-by: Lukas Straub
> ---
> migration/meson.build | 1 +
> migration/migration-stats.c | 17 +++
> migration/migration-stats.h | 41 +
> migration/migration.c | 1 +
> migration/multifd.c | 1 +
> migration/ram.c | 3 +--
> migration/ram.h | 23 -
> migration/savevm.c | 1 +
> 8 files changed, 63 insertions(+), 25 deletions(-)
> create mode 100644 migration/migration-stats.c
> create mode 100644 migration/migration-stats.h
>
> diff --git a/migration/meson.build b/migration/meson.build
> index 480ff6854a..da1897fadf 100644
> --- a/migration/meson.build
> +++ b/migration/meson.build
> @@ -19,6 +19,7 @@ softmmu_ss.add(files(
>'fd.c',
>'global_state.c',
>'migration-hmp-cmds.c',
> + 'migration-stats.c',
>'migration.c',
>'multifd.c',
>'multifd-zlib.c',
> diff --git a/migration/migration-stats.c b/migration/migration-stats.c
> new file mode 100644
> index 00..b0eb5ae73c
> --- /dev/null
> +++ b/migration/migration-stats.c
> @@ -0,0 +1,17 @@
> +/*
> + * Migration stats
> + *
> + * Copyright (c) 2012-2023 Red Hat Inc
> + *
> + * Authors:
> + * Juan Quintela
> + *
> + * 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 "qemu/stats64.h"
> +#include "migration-stats.h"
> +
> +RAMStats ram_counters;
> diff --git a/migration/migration-stats.h b/migration/migration-stats.h
> new file mode 100644
> index 00..2edea0c779
> --- /dev/null
> +++ b/migration/migration-stats.h
> @@ -0,0 +1,41 @@
> +/*
> + * Migration stats
> + *
> + * Copyright (c) 2012-2023 Red Hat Inc
> + *
> + * Authors:
> + * Juan Quintela
> + *
> + * 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_STATS_H
> +#define QEMU_MIGRATION_STATS_H
> +
> +#include "qemu/stats64.h"
> +
> +/*
> + * These are the ram migration statistic counters. It is loosely
> + * based on MigrationStats. We change to Stat64 any counter that
> + * needs to be updated using atomic ops (can be accessed by more than
> + * one thread).
> + */
> +typedef struct {
> +Stat64 dirty_bytes_last_sync;
> +Stat64 dirty_pages_rate;
> +Stat64 dirty_sync_count;
> +Stat64 dirty_sync_missed_zero_copy;
> +Stat64 downtime_bytes;
> +Stat64 zero_pages;
> +Stat64 multifd_bytes;
> +Stat64 normal_pages;
> +Stat64 postcopy_bytes;
> +Stat64 postcopy_requests;
> +Stat64 precopy_bytes;
> +Stat64 transferred;
> +} RAMStats;
> +
> +extern RAMStats ram_counters;
> +
> +#endif
> diff --git a/migration/migration.c b/migration/migration.c
> index abcadbb619..5ecf3dc381 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -29,6 +29,7 @@
> #include "migration/global_state.h"
> #include "migration/misc.h"
> #include "migration.h"
> +#include "migration-stats.h"
> #include "savevm.h"
> #include "qemu-file.h"
> #include "channel.h"
> diff --git a/migration/multifd.c b/migration/multifd.c
> index 6053012ad9..347999f84a 100644
> --- a/migration/multifd.c
> +++ b/migration/multifd.c
> @@ -19,6 +19,7 @@
> #include "qapi/error.h"
> #include "ram.h"
> #include "migration.h"
> +#include "migration-stats.h"
> #include "socket.h"
> #include "tls.h"
> #include "qemu-file.h"
> diff --git a/migration/ram.c b/migration/ram.c
> index 89be3e3320..a6d5478ef8 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -36,6 +36,7 @@
> #include "xbzrle.h"
> #include "ram.h"
> #include "migration.h"
> +#include "migration-stats.h"
> #include "migration/register.h"
> #include "migration/misc.h"
> #include "qemu-file.h"
> @@ -460,8 +461,6 @@ uint64_t ram_bytes_remaining(void)
> 0;
> }
>
> -RAMStats ram_counters;
> -
> void ram_transferred_add(uint64_t byte
Re: [PATCH 01/19] multifd: We already account for this packet on the multifd thread
On Thu, 27 Apr 2023 18:34:31 +0200
Juan Quintela wrote:
> Signed-off-by: Juan Quintela
Reviewed-by: Lukas Straub
> ---
> migration/multifd.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/migration/multifd.c b/migration/multifd.c
> index 6a59c03dd2..6053012ad9 100644
> --- a/migration/multifd.c
> +++ b/migration/multifd.c
> @@ -626,10 +626,7 @@ int multifd_send_sync_main(QEMUFile *f)
> p->packet_num = multifd_send_state->packet_num++;
> p->flags |= MULTIFD_FLAG_SYNC;
> p->pending_job++;
> -qemu_file_acct_rate_limit(f, p->packet_len);
> qemu_mutex_unlock(>mutex);
> -stat64_add(_counters.transferred, p->packet_len);
> -stat64_add(_counters.multifd_bytes, p->packet_len);
> qemu_sem_post(>sem);
> }
> for (i = 0; i < migrate_multifd_channels(); i++) {
--
pgpR1ZFCUM3fw.pgp
Description: OpenPGP digital signature
Re: [PATCH v2 01/13] qtest/migration-test.c: Add postcopy tests with compress enabled
On Thu, 20 Apr 2023 12:20:25 +0200
Juan Quintela wrote:
> Lukas Straub wrote:
> > Add postcopy tests with compress enabled to ensure nothing breaks
> > with the refactoring in the next commits.
> >
> > preempt+compress is blocked, so no test needed for that case.
> >
> > Signed-off-by: Lukas Straub
>
> Reviewed-by: Juan Quintela
>
> And I wanted to removed the old compression code and it gets new users.
> Sniff.
Who know how many compress threads users are out there...
By the way, I'm not against deprecating compress threads in the long
run. I'm already working on (cleanly :)) adding colo support with
multifd.
> > ---
> > tests/qtest/migration-test.c | 83 +++-
> > 1 file changed, 53 insertions(+), 30 deletions(-)
> >
> > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> > index 1f2a019ce0..930cb4f29d 100644
> > --- a/tests/qtest/migration-test.c
> > +++ b/tests/qtest/migration-test.c
> > @@ -1127,6 +1127,36 @@ test_migrate_tls_x509_finish(QTestState *from,
> > #endif /* CONFIG_TASN1 */
> > #endif /* CONFIG_GNUTLS */
> >
> > +static void *
> > +test_migrate_compress_start(QTestState *from,
> > +QTestState *to)
> > +{
> > +migrate_set_parameter_int(from, "compress-level", 1);
> > +migrate_set_parameter_int(from, "compress-threads", 4);
> > +migrate_set_parameter_bool(from, "compress-wait-thread", true);
> > +migrate_set_parameter_int(to, "decompress-threads", 4);
> > +
> > +migrate_set_capability(from, "compress", true);
> > +migrate_set_capability(to, "compress", true);
> > +
> > +return NULL;
> > +}
>
> Independently of this patch, we need to change this test to use 4
> compression tests and 3 decompression or anything that is not the same
> number in both sides.
>
> I was complaining about this and when I arrived to the end of the path
> found that this was code movement.
>
> Later, Juan.
>
Oops, forgot to mention, the test is based on this patch
https://lore.kernel.org/qemu-devel/2f4abb67cf5f3e1591b276462a93bdd20bbc.1680618040.git.lukasstra...@web.de/
Will probably carry the patch with this series then. So you mean 4
compress _threads_ and 3 decompress _threads_?
--
pgp0cwxzugLWj.pgp
Description: OpenPGP digital signature
Re: [PATCH v2 4/4] configure: add --disable-colo-filters option
On Thu, 20 Apr 2023 09:09:48 +
"Zhang, Chen" wrote:
> > -Original Message-
> > From: Vladimir Sementsov-Ogievskiy
> > Sent: Thursday, April 20, 2023 6:53 AM
> > To: qemu-devel@nongnu.org
> > Cc: qemu-bl...@nongnu.org; michael.r...@amd.com; arm...@redhat.com;
> > ebl...@redhat.com; jasow...@redhat.com; quint...@redhat.com; Zhang,
> > Hailiang ; phi...@linaro.org;
> > th...@redhat.com; berra...@redhat.com; marcandre.lur...@redhat.com;
> > pbonz...@redhat.com; d...@treblig.org; hre...@redhat.com;
> > kw...@redhat.com; Zhang, Chen ;
> > lizhij...@fujitsu.com; Vladimir Sementsov-Ogievskiy > team.ru>
> > Subject: [PATCH v2 4/4] configure: add --disable-colo-filters option
> >
> > Add option to not build COLO Proxy subsystem if it is not needed.
>
> I think no need to add the --disable-colo-filter option.
> Net-filters just general infrastructure. Another example is COLO also
> use the -chardev socket to connect each filters. No need to add the
> --disable-colo-chardev
> Please drop this patch.
> But for COLO network part, still something need to do:
> You can add --disable-colo-proxy not to build the net/colo-compare.c if it
> is not needed.
> This file just for COLO and not belong to network filters.
And net/filter-rewriter.c is just for COLO too.
So in summary just drop net/filter-mirror.c from this patch?
>
> Thanks
> Chen
>
> >
> > Signed-off-by: Vladimir Sementsov-Ogievskiy
> > ---
> > meson.build | 1 +
> > meson_options.txt | 2 ++
> > net/meson.build | 11 ---
> > scripts/meson-buildoptions.sh | 3 +++
> > 4 files changed, 14 insertions(+), 3 deletions(-)
> >
> > diff --git a/meson.build b/meson.build
> > index c44d05a13f..5b2fdfbd3a 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -1962,6 +1962,7 @@ config_host_data.set('CONFIG_GPROF',
> > get_option('gprof'))
> > config_host_data.set('CONFIG_LIVE_BLOCK_MIGRATION',
> > get_option('live_block_migration').allowed())
> > config_host_data.set('CONFIG_QOM_CAST_DEBUG',
> > get_option('qom_cast_debug'))
> > config_host_data.set('CONFIG_REPLICATION',
> > get_option('replication').allowed())
> > +config_host_data.set('CONFIG_COLO_FILTERS',
> > +get_option('colo_filters').allowed())
> >
> > # has_header
> > config_host_data.set('CONFIG_EPOLL', cc.has_header('sys/epoll.h')) diff
> > --git
> > a/meson_options.txt b/meson_options.txt index fc9447d267..ffe81317cb
> > 100644
> > --- a/meson_options.txt
> > +++ b/meson_options.txt
> > @@ -289,6 +289,8 @@ option('live_block_migration', type: 'feature', value:
> > 'auto',
> > description: 'block migration in the main migration stream')
> > option('replication', type: 'feature', value: 'auto',
> > description: 'replication support')
> > +option('colo_filters', type: 'feature', value: 'auto',
> > + description: 'colo_filters support')
> > option('bochs', type: 'feature', value: 'auto',
> > description: 'bochs image format support') option('cloop', type:
> > 'feature',
> > value: 'auto', diff --git a/net/meson.build b/net/meson.build index
> > 38d50b8c96..7e54744aea 100644
> > --- a/net/meson.build
> > +++ b/net/meson.build
> > @@ -1,12 +1,9 @@
> > softmmu_ss.add(files(
> >'announce.c',
> >'checksum.c',
> > - 'colo.c',
> >'dump.c',
> >'eth.c',
> >'filter-buffer.c',
> > - 'filter-mirror.c',
> > - 'filter-rewriter.c',
> >'filter.c',
> >'hub.c',
> >'net-hmp-cmds.c',
> > @@ -22,6 +19,14 @@ if get_option('replication').allowed()
> >softmmu_ss.add(files('colo-compare.c'))
> > endif
> >
> > +if get_option('replication').allowed() or
> > +get_option('colo_filters').allowed()
> > + softmmu_ss.add(files('colo.c'))
> > +endif
> > +
> > +if get_option('colo_filters').allowed()
> > + softmmu_ss.add(files('filter-mirror.c', 'filter-rewriter.c')) endif
> > +
> > softmmu_ss.add(when: 'CONFIG_TCG', if_true: files('filter-replay.c'))
> >
> > if have_l2tpv3
> > diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
> > index 009fab1515..cf9d23369f 100644
> > --- a/scripts/meson-buildoptions.sh
> > +++ b/scripts/meson-buildoptions.sh
> > @@ -83,6 +83,7 @@ meson_options_help() {
> >printf "%s\n" ' capstoneWhether and how to find the capstone
> > library'
> >printf "%s\n" ' cloop cloop image format support'
> >printf "%s\n" ' cocoa Cocoa user interface (macOS only)'
> > + printf "%s\n" ' colo-filterscolo_filters support'
> >printf "%s\n" ' coreaudio CoreAudio sound support'
> >printf "%s\n" ' crypto-afalgLinux AF_ALG crypto backend driver'
> >printf "%s\n" ' curlCURL block device driver'
> > @@ -236,6 +237,8 @@ _meson_option_parse() {
> > --disable-cloop) printf "%s" -Dcloop=disabled ;;
> > --enable-cocoa) printf "%s" -Dcocoa=enabled ;;
> > --disable-cocoa) printf "%s" -Dcocoa=disabled ;;
> > +--enable-colo-filters) printf "%s"
[PATCH v2 14/13] migration: Initialize and cleanup decompression in migration.c
This fixes compress with colo.
Signed-off-by: Lukas Straub
---
Oops, this one slipped trough
migration/migration.c | 9 +
migration/ram.c | 5 -
2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index bda4789193..e7d082a208 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -26,6 +26,7 @@
#include "sysemu/cpu-throttle.h"
#include "rdma.h"
#include "ram.h"
+#include "ram-compress.h"
#include "migration/global_state.h"
#include "migration/misc.h"
#include "migration.h"
@@ -316,6 +317,7 @@ void migration_incoming_state_destroy(void)
struct MigrationIncomingState *mis = migration_incoming_get_current();
multifd_load_cleanup();
+compress_threads_load_cleanup();
if (mis->to_src_file) {
/* Tell source that we are done */
@@ -598,6 +600,12 @@ process_incoming_migration_co(void *opaque)
Error *local_err = NULL;
assert(mis->from_src_file);
+
+if (compress_threads_load_setup(mis->from_src_file)) {
+error_report("Failed to setup decompress threads");
+goto fail;
+}
+
mis->migration_incoming_co = qemu_coroutine_self();
mis->largest_page_size = qemu_ram_pagesize_largest();
postcopy_state_set(POSTCOPY_INCOMING_NONE);
@@ -663,6 +671,7 @@ fail:
qemu_fclose(mis->from_src_file);
multifd_load_cleanup();
+compress_threads_load_cleanup();
exit(EXIT_FAILURE);
}
diff --git a/migration/ram.c b/migration/ram.c
index ccfedf4fb5..344f326065 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3560,10 +3560,6 @@ void colo_release_ram_cache(void)
*/
static int ram_load_setup(QEMUFile *f, void *opaque)
{
-if (compress_threads_load_setup(f)) {
-return -1;
-}
-
xbzrle_load_setup();
ramblock_recv_map_init();
@@ -3579,7 +3575,6 @@ static int ram_load_cleanup(void *opaque)
}
xbzrle_load_cleanup();
-compress_threads_load_cleanup();
RAMBLOCK_FOREACH_NOT_IGNORED(rb) {
g_free(rb->receivedmap);
--
2.40.0
pgpy06FSzQYDY.pgp
Description: OpenPGP digital signature
[PATCH v2 13/13] ram-compress.c: Make target independent
Make ram-compress.c target independent.
Signed-off-by: Lukas Straub
---
migration/meson.build| 2 +-
migration/ram-compress.c | 17 ++---
2 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/migration/meson.build b/migration/meson.build
index 262e3c9754..16f642031c 100644
--- a/migration/meson.build
+++ b/migration/meson.build
@@ -22,6 +22,7 @@ softmmu_ss.add(files(
'migration.c',
'multifd.c',
'multifd-zlib.c',
+ 'ram-compress.c',
'postcopy-ram.c',
'savevm.c',
'socket.c',
@@ -38,5 +39,4 @@ softmmu_ss.add(when: zstd, if_true: files('multifd-zstd.c'))
specific_ss.add(when: 'CONFIG_SOFTMMU',
if_true: files('dirtyrate.c',
'ram.c',
- 'ram-compress.c',
'target.c'))
diff --git a/migration/ram-compress.c b/migration/ram-compress.c
index b75a9d2b9a..9f03c3cc66 100644
--- a/migration/ram-compress.c
+++ b/migration/ram-compress.c
@@ -34,7 +34,8 @@
#include "qemu/error-report.h"
#include "migration.h"
#include "io/channel-null.h"
-#include "exec/ram_addr.h"
+#include "exec/target_page.h"
+#include "exec/ramblock.h"
CompressionStats compression_counters;
@@ -155,7 +156,7 @@ int compress_threads_save_setup(void)
qemu_cond_init(_done_cond);
qemu_mutex_init(_done_lock);
for (i = 0; i < thread_count; i++) {
-comp_param[i].originbuf = g_try_malloc(TARGET_PAGE_SIZE);
+comp_param[i].originbuf = g_try_malloc(qemu_target_page_size());
if (!comp_param[i].originbuf) {
goto exit;
}
@@ -191,11 +192,12 @@ static CompressResult do_compress_ram_page(QEMUFile *f,
z_stream *stream,
uint8_t *source_buf)
{
uint8_t *p = block->host + offset;
+size_t page_size = qemu_target_page_size();
int ret;
assert(qemu_file_buffer_empty(f));
-if (buffer_is_zero(p, TARGET_PAGE_SIZE)) {
+if (buffer_is_zero(p, page_size)) {
return RES_ZEROPAGE;
}
@@ -204,8 +206,8 @@ static CompressResult do_compress_ram_page(QEMUFile *f,
z_stream *stream,
* so that we can catch up the error during compression and
* decompression
*/
-memcpy(source_buf, p, TARGET_PAGE_SIZE);
-ret = qemu_put_compression_data(f, stream, source_buf, TARGET_PAGE_SIZE);
+memcpy(source_buf, p, page_size);
+ret = qemu_put_compression_data(f, stream, source_buf, page_size);
if (ret < 0) {
qemu_file_set_error(migrate_get_current()->to_dst_file, ret);
error_report("compressed data failed!");
@@ -335,7 +337,7 @@ static void *do_data_decompress(void *opaque)
param->des = 0;
qemu_mutex_unlock(>mutex);
-pagesize = TARGET_PAGE_SIZE;
+pagesize = qemu_target_page_size();
ret = qemu_uncompress_data(>stream, des, pagesize,
param->compbuf, len);
@@ -438,7 +440,8 @@ int compress_threads_load_setup(QEMUFile *f)
goto exit;
}
-decomp_param[i].compbuf = g_malloc0(compressBound(TARGET_PAGE_SIZE));
+size_t compbuf_size = compressBound(qemu_target_page_size());
+decomp_param[i].compbuf = g_malloc0(compbuf_size);
qemu_mutex_init(_param[i].mutex);
qemu_cond_init(_param[i].cond);
decomp_param[i].done = true;
--
2.40.0
pgp1z5Q1WXAjW.pgp
Description: OpenPGP digital signature
[PATCH v2 07/13] ram.c: Remove last ram.c dependency from the core compress code
Make compression interfaces take send_queued_data() as an argument.
Remove save_page_use_compression() from flush_compressed_data().
This removes the last ram.c dependency from the core compress code.
Signed-off-by: Lukas Straub
---
migration/ram.c | 27 +--
1 file changed, 17 insertions(+), 10 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index 3894d0ae79..bd3773d4c4 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1556,13 +1556,10 @@ static int send_queued_data(CompressParam *param)
return len;
}
-static void flush_compressed_data(RAMState *rs)
+static void flush_compressed_data(int (send_queued_data(CompressParam *)))
{
int idx, thread_count;
-if (!save_page_use_compression(rs)) {
-return;
-}
thread_count = migrate_compress_threads();
qemu_mutex_lock(_done_lock);
@@ -1584,6 +1581,15 @@ static void flush_compressed_data(RAMState *rs)
}
}
+static void ram_flush_compressed_data(RAMState *rs)
+{
+if (!save_page_use_compression(rs)) {
+return;
+}
+
+flush_compressed_data(send_queued_data);
+}
+
static inline void set_compress_params(CompressParam *param, RAMBlock *block,
ram_addr_t offset)
{
@@ -1592,7 +1598,8 @@ static inline void set_compress_params(CompressParam
*param, RAMBlock *block,
param->trigger = true;
}
-static int compress_page_with_multi_thread(RAMBlock *block, ram_addr_t offset)
+static int compress_page_with_multi_thread(RAMBlock *block, ram_addr_t offset,
+int (send_queued_data(CompressParam *)))
{
int idx, thread_count, pages = -1;
bool wait = migrate_compress_wait_thread();
@@ -1673,7 +1680,7 @@ static int find_dirty_block(RAMState *rs,
PageSearchStatus *pss)
* Also If xbzrle is on, stop using the data compression at this
* point. In theory, xbzrle can do better than compression.
*/
-flush_compressed_data(rs);
+ram_flush_compressed_data(rs);
/* Hit the end of the list */
pss->block = QLIST_FIRST_RCU(_list.blocks);
@@ -2363,11 +2370,11 @@ static bool save_compress_page(RAMState *rs,
PageSearchStatus *pss,
* much CPU resource.
*/
if (block != pss->last_sent_block) {
-flush_compressed_data(rs);
+ram_flush_compressed_data(rs);
return false;
}
-if (compress_page_with_multi_thread(block, offset) > 0) {
+if (compress_page_with_multi_thread(block, offset, send_queued_data) > 0) {
return true;
}
@@ -3419,7 +3426,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
* page is sent in one chunk.
*/
if (migrate_postcopy_ram()) {
-flush_compressed_data(rs);
+ram_flush_compressed_data(rs);
}
/*
@@ -3512,7 +3519,7 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
}
qemu_mutex_unlock(>bitmap_mutex);
-flush_compressed_data(rs);
+ram_flush_compressed_data(rs);
ram_control_after_iterate(f, RAM_CONTROL_FINISH);
}
--
2.40.0
pgpDqDbP2wU1k.pgp
Description: OpenPGP digital signature
[PATCH v2 09/13] ram.c: Move core compression code into its own file
No functional changes intended.
Signed-off-by: Lukas Straub
---
migration/meson.build| 5 +-
migration/ram-compress.c | 273 +++
migration/ram-compress.h | 65 ++
migration/ram.c | 256 +---
4 files changed, 343 insertions(+), 256 deletions(-)
create mode 100644 migration/ram-compress.c
create mode 100644 migration/ram-compress.h
diff --git a/migration/meson.build b/migration/meson.build
index 0d1bb9f96e..262e3c9754 100644
--- a/migration/meson.build
+++ b/migration/meson.build
@@ -36,4 +36,7 @@ endif
softmmu_ss.add(when: zstd, if_true: files('multifd-zstd.c'))
specific_ss.add(when: 'CONFIG_SOFTMMU',
-if_true: files('dirtyrate.c', 'ram.c', 'target.c'))
+if_true: files('dirtyrate.c',
+ 'ram.c',
+ 'ram-compress.c',
+ 'target.c'))
diff --git a/migration/ram-compress.c b/migration/ram-compress.c
new file mode 100644
index 00..77902a1d65
--- /dev/null
+++ b/migration/ram-compress.c
@@ -0,0 +1,273 @@
+/*
+ * QEMU System Emulator
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ * Copyright (c) 2011-2015 Red Hat Inc
+ *
+ * Authors:
+ * Juan Quintela
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/cutils.h"
+
+#include "ram-compress.h"
+
+#include "qemu/error-report.h"
+#include "migration.h"
+#include "io/channel-null.h"
+#include "exec/ram_addr.h"
+
+CompressionStats compression_counters;
+
+static CompressParam *comp_param;
+static QemuThread *compress_threads;
+/* comp_done_cond is used to wake up the migration thread when
+ * one of the compression threads has finished the compression.
+ * comp_done_lock is used to co-work with comp_done_cond.
+ */
+static QemuMutex comp_done_lock;
+static QemuCond comp_done_cond;
+
+static CompressResult do_compress_ram_page(QEMUFile *f, z_stream *stream,
+ RAMBlock *block, ram_addr_t offset,
+ uint8_t *source_buf);
+
+static void *do_data_compress(void *opaque)
+{
+CompressParam *param = opaque;
+RAMBlock *block;
+ram_addr_t offset;
+CompressResult result;
+
+qemu_mutex_lock(>mutex);
+while (!param->quit) {
+if (param->trigger) {
+block = param->block;
+offset = param->offset;
+param->trigger = false;
+qemu_mutex_unlock(>mutex);
+
+result = do_compress_ram_page(param->file, >stream,
+ block, offset, param->originbuf);
+
+qemu_mutex_lock(_done_lock);
+param->done = true;
+param->result = result;
+qemu_cond_signal(_done_cond);
+qemu_mutex_unlock(_done_lock);
+
+qemu_mutex_lock(>mutex);
+} else {
+qemu_cond_wait(>cond, >mutex);
+}
+}
+qemu_mutex_unlock(>mutex);
+
+return NULL;
+}
+
+void compress_threads_save_cleanup(void)
+{
+int i, thread_count;
+
+if (!migrate_use_compression() || !comp_param) {
+return;
+}
+
+thread_count = migrate_compress_threads();
+for (i = 0; i < thread_count; i++) {
+/*
+ * we use it as a indicator which shows if the thread is
+ * properly init'd or not
+ */
+if (!comp_param[i].file) {
+break;
+}
+
+qemu_mutex_lock(_param[i].mutex);
+comp_param[i].quit = true;
+qemu_cond_signal(_param[i].cond);
+qemu_mutex_unlock(_param[i].mutex);
+
+qemu_thread_join(compress_threads + i);
+qemu_mutex_destroy(_param[i].mutex);
+qemu_cond_dest
[PATCH v2 10/13] ram.c: Remove whitespace (squash with previous patch)
Signed-off-by: Lukas Straub --- migration/ram.c | 18 -- 1 file changed, 18 deletions(-) diff --git a/migration/ram.c b/migration/ram.c index 42d6a54132..7be09c18e3 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -529,24 +529,6 @@ static bool pss_overlap(PageSearchStatus *pss1, PageSearchStatus *pss2) (pss1->host_page_start == pss2->host_page_start); } - - - - -/* split */ - - - - - - - -/* split */ - - - - - /** * save_page_header: write page header to wire * -- 2.40.0 pgpDcXN1bqlRJ.pgp Description: OpenPGP digital signature
[PATCH v2 03/13] ram.c: Dont change param->block in the compress thread
Instead introduce a extra parameter to trigger the compress thread.
Now, when the compress thread is done, we know what RAMBlock and
offset it did compress.
This will be used in the next commits to move save_page_header()
out of compress code.
Signed-off-by: Lukas Straub
---
migration/ram.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index ade6638a96..820b4ebaeb 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -503,6 +503,7 @@ typedef enum CompressResult CompressResult;
struct CompressParam {
bool done;
bool quit;
+bool trigger;
CompressResult result;
QEMUFile *file;
QemuMutex mutex;
@@ -576,10 +577,10 @@ static void *do_data_compress(void *opaque)
qemu_mutex_lock(>mutex);
while (!param->quit) {
-if (param->block) {
+if (param->trigger) {
block = param->block;
offset = param->offset;
-param->block = NULL;
+param->trigger = false;
qemu_mutex_unlock(>mutex);
result = do_compress_ram_page(param->file, >stream,
@@ -1556,6 +1557,7 @@ static inline void set_compress_params(CompressParam
*param, RAMBlock *block,
{
param->block = block;
param->offset = offset;
+param->trigger = true;
}
static int compress_page_with_multi_thread(RAMBlock *block, ram_addr_t offset)
--
2.40.0
pgpkxhFpqGVhT.pgp
Description: OpenPGP digital signature
[PATCH v2 04/13] ram.c: Reset result after sending queued data
And take the param->mutex lock for the whole section to ensure
thread-safety.
Now, it is explicitly clear if there is no queued data to send.
Before, this was handled by param->file stream being empty and thus
qemu_put_qemu_file() not sending anything.
This will be used in the next commits to move save_page_header()
out of compress code.
Signed-off-by: Lukas Straub
---
migration/ram.c | 32 ++--
1 file changed, 22 insertions(+), 10 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index 820b4ebaeb..5ca0f115cf 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1519,6 +1519,13 @@ update_compress_thread_counts(const CompressParam
*param, int bytes_xmit)
static bool save_page_use_compression(RAMState *rs);
+static inline void compress_reset_result(CompressParam *param)
+{
+param->result = RES_NONE;
+param->block = NULL;
+param->offset = 0;
+}
+
static void flush_compressed_data(RAMState *rs)
{
MigrationState *ms = migrate_get_current();
@@ -1540,13 +1547,16 @@ static void flush_compressed_data(RAMState *rs)
for (idx = 0; idx < thread_count; idx++) {
qemu_mutex_lock(_param[idx].mutex);
if (!comp_param[idx].quit) {
-len = qemu_put_qemu_file(ms->to_dst_file, comp_param[idx].file);
+CompressParam *param = _param[idx];
+len = qemu_put_qemu_file(ms->to_dst_file, param->file);
+compress_reset_result(param);
+
/*
* it's safe to fetch zero_page without holding comp_done_lock
* as there is no further request submitted to the thread,
* i.e, the thread should be waiting for a request at this point.
*/
-update_compress_thread_counts(_param[idx], len);
+update_compress_thread_counts(param, len);
}
qemu_mutex_unlock(_param[idx].mutex);
}
@@ -1571,15 +1581,17 @@ static int compress_page_with_multi_thread(RAMBlock
*block, ram_addr_t offset)
retry:
for (idx = 0; idx < thread_count; idx++) {
if (comp_param[idx].done) {
-comp_param[idx].done = false;
-bytes_xmit = qemu_put_qemu_file(ms->to_dst_file,
-comp_param[idx].file);
-qemu_mutex_lock(_param[idx].mutex);
-set_compress_params(_param[idx], block, offset);
-qemu_cond_signal(_param[idx].cond);
-qemu_mutex_unlock(_param[idx].mutex);
+CompressParam *param = _param[idx];
+qemu_mutex_lock(>mutex);
+param->done = false;
+bytes_xmit = qemu_put_qemu_file(ms->to_dst_file, param->file);
+compress_reset_result(param);
+set_compress_params(param, block, offset);
+
+update_compress_thread_counts(param, bytes_xmit);
+qemu_cond_signal(>cond);
+qemu_mutex_unlock(>mutex);
pages = 1;
-update_compress_thread_counts(_param[idx], bytes_xmit);
break;
}
}
--
2.40.0
pgpOdTcrh92DD.pgp
Description: OpenPGP digital signature
[PATCH v2 01/13] qtest/migration-test.c: Add postcopy tests with compress enabled
Add postcopy tests with compress enabled to ensure nothing breaks
with the refactoring in the next commits.
preempt+compress is blocked, so no test needed for that case.
Signed-off-by: Lukas Straub
---
tests/qtest/migration-test.c | 83 +++-
1 file changed, 53 insertions(+), 30 deletions(-)
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 1f2a019ce0..930cb4f29d 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -1127,6 +1127,36 @@ test_migrate_tls_x509_finish(QTestState *from,
#endif /* CONFIG_TASN1 */
#endif /* CONFIG_GNUTLS */
+static void *
+test_migrate_compress_start(QTestState *from,
+QTestState *to)
+{
+migrate_set_parameter_int(from, "compress-level", 1);
+migrate_set_parameter_int(from, "compress-threads", 4);
+migrate_set_parameter_bool(from, "compress-wait-thread", true);
+migrate_set_parameter_int(to, "decompress-threads", 4);
+
+migrate_set_capability(from, "compress", true);
+migrate_set_capability(to, "compress", true);
+
+return NULL;
+}
+
+static void *
+test_migrate_compress_nowait_start(QTestState *from,
+ QTestState *to)
+{
+migrate_set_parameter_int(from, "compress-level", 9);
+migrate_set_parameter_int(from, "compress-threads", 1);
+migrate_set_parameter_bool(from, "compress-wait-thread", false);
+migrate_set_parameter_int(to, "decompress-threads", 1);
+
+migrate_set_capability(from, "compress", true);
+migrate_set_capability(to, "compress", true);
+
+return NULL;
+}
+
static int migrate_postcopy_prepare(QTestState **from_ptr,
QTestState **to_ptr,
MigrateCommon *args)
@@ -1204,6 +1234,15 @@ static void test_postcopy(void)
test_postcopy_common();
}
+static void test_postcopy_compress(void)
+{
+MigrateCommon args = {
+.start_hook = test_migrate_compress_start
+};
+
+test_postcopy_common();
+}
+
static void test_postcopy_preempt(void)
{
MigrateCommon args = {
@@ -1305,6 +1344,15 @@ static void test_postcopy_recovery(void)
test_postcopy_recovery_common();
}
+static void test_postcopy_recovery_compress(void)
+{
+MigrateCommon args = {
+.start_hook = test_migrate_compress_start
+};
+
+test_postcopy_recovery_common();
+}
+
#ifdef CONFIG_GNUTLS
static void test_postcopy_recovery_tls_psk(void)
{
@@ -1338,6 +1386,7 @@ static void test_postcopy_preempt_all(void)
test_postcopy_recovery_common();
}
+
#endif
static void test_baddest(void)
@@ -1559,21 +1608,6 @@ static void test_precopy_unix_xbzrle(void)
test_precopy_common();
}
-static void *
-test_migrate_compress_start(QTestState *from,
-QTestState *to)
-{
-migrate_set_parameter_int(from, "compress-level", 1);
-migrate_set_parameter_int(from, "compress-threads", 4);
-migrate_set_parameter_bool(from, "compress-wait-thread", true);
-migrate_set_parameter_int(to, "decompress-threads", 4);
-
-migrate_set_capability(from, "compress", true);
-migrate_set_capability(to, "compress", true);
-
-return NULL;
-}
-
static void test_precopy_unix_compress(void)
{
g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
@@ -1591,21 +1625,6 @@ static void test_precopy_unix_compress(void)
test_precopy_common();
}
-static void *
-test_migrate_compress_nowait_start(QTestState *from,
- QTestState *to)
-{
-migrate_set_parameter_int(from, "compress-level", 9);
-migrate_set_parameter_int(from, "compress-threads", 1);
-migrate_set_parameter_bool(from, "compress-wait-thread", false);
-migrate_set_parameter_int(to, "decompress-threads", 1);
-
-migrate_set_capability(from, "compress", true);
-migrate_set_capability(to, "compress", true);
-
-return NULL;
-}
-
static void test_precopy_unix_compress_nowait(void)
{
g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
@@ -2604,8 +2623,12 @@ int main(int argc, char **argv)
if (has_uffd) {
qtest_add_func("/migration/postcopy/plain", test_postcopy);
+qtest_add_func("/migration/postcopy/compress/plain",
+ test_postcopy_compress);
qtest_add_func("/migration/postcopy/recovery/plain",
test_postcopy_recovery);
+qtest_add_func("/migration/postcopy/recovery/compress/plain",
+ test_postcopy_recovery_compress);
qtest_add_func("/migration/postcopy/preempt/plain",
test_postcopy_preempt);
qtest_add_func("/migration/postcopy/preempt/recovery/plain",
test_postcopy_preempt_recovery);
--
2.40.0
pgpDntdkxE0pZ.pgp
Description: OpenPGP digital signature
[PATCH v2 11/13] ram.c: Move core decompression code into its own file
No functional changes intended.
Signed-off-by: Lukas Straub
Reviewed-by: Philippe Mathieu-Daudé
---
migration/ram-compress.c | 203 ++
migration/ram-compress.h | 5 +
migration/ram.c | 204 ---
3 files changed, 208 insertions(+), 204 deletions(-)
diff --git a/migration/ram-compress.c b/migration/ram-compress.c
index 77902a1d65..f75b8c3079 100644
--- a/migration/ram-compress.c
+++ b/migration/ram-compress.c
@@ -47,6 +47,24 @@ static QemuThread *compress_threads;
static QemuMutex comp_done_lock;
static QemuCond comp_done_cond;
+struct DecompressParam {
+bool done;
+bool quit;
+QemuMutex mutex;
+QemuCond cond;
+void *des;
+uint8_t *compbuf;
+int len;
+z_stream stream;
+};
+typedef struct DecompressParam DecompressParam;
+
+static QEMUFile *decomp_file;
+static DecompressParam *decomp_param;
+static QemuThread *decompress_threads;
+static QemuMutex decomp_done_lock;
+static QemuCond decomp_done_cond;
+
static CompressResult do_compress_ram_page(QEMUFile *f, z_stream *stream,
RAMBlock *block, ram_addr_t offset,
uint8_t *source_buf);
@@ -271,3 +289,188 @@ retry:
return pages;
}
+
+/* return the size after decompression, or negative value on error */
+static int
+qemu_uncompress_data(z_stream *stream, uint8_t *dest, size_t dest_len,
+ const uint8_t *source, size_t source_len)
+{
+int err;
+
+err = inflateReset(stream);
+if (err != Z_OK) {
+return -1;
+}
+
+stream->avail_in = source_len;
+stream->next_in = (uint8_t *)source;
+stream->avail_out = dest_len;
+stream->next_out = dest;
+
+err = inflate(stream, Z_NO_FLUSH);
+if (err != Z_STREAM_END) {
+return -1;
+}
+
+return stream->total_out;
+}
+
+static void *do_data_decompress(void *opaque)
+{
+DecompressParam *param = opaque;
+unsigned long pagesize;
+uint8_t *des;
+int len, ret;
+
+qemu_mutex_lock(>mutex);
+while (!param->quit) {
+if (param->des) {
+des = param->des;
+len = param->len;
+param->des = 0;
+qemu_mutex_unlock(>mutex);
+
+pagesize = TARGET_PAGE_SIZE;
+
+ret = qemu_uncompress_data(>stream, des, pagesize,
+ param->compbuf, len);
+if (ret < 0 && migrate_get_current()->decompress_error_check) {
+error_report("decompress data failed");
+qemu_file_set_error(decomp_file, ret);
+}
+
+qemu_mutex_lock(_done_lock);
+param->done = true;
+qemu_cond_signal(_done_cond);
+qemu_mutex_unlock(_done_lock);
+
+qemu_mutex_lock(>mutex);
+} else {
+qemu_cond_wait(>cond, >mutex);
+}
+}
+qemu_mutex_unlock(>mutex);
+
+return NULL;
+}
+
+int wait_for_decompress_done(void)
+{
+int idx, thread_count;
+
+if (!migrate_use_compression()) {
+return 0;
+}
+
+thread_count = migrate_decompress_threads();
+qemu_mutex_lock(_done_lock);
+for (idx = 0; idx < thread_count; idx++) {
+while (!decomp_param[idx].done) {
+qemu_cond_wait(_done_cond, _done_lock);
+}
+}
+qemu_mutex_unlock(_done_lock);
+return qemu_file_get_error(decomp_file);
+}
+
+void compress_threads_load_cleanup(void)
+{
+int i, thread_count;
+
+if (!migrate_use_compression()) {
+return;
+}
+thread_count = migrate_decompress_threads();
+for (i = 0; i < thread_count; i++) {
+/*
+ * we use it as a indicator which shows if the thread is
+ * properly init'd or not
+ */
+if (!decomp_param[i].compbuf) {
+break;
+}
+
+qemu_mutex_lock(_param[i].mutex);
+decomp_param[i].quit = true;
+qemu_cond_signal(_param[i].cond);
+qemu_mutex_unlock(_param[i].mutex);
+}
+for (i = 0; i < thread_count; i++) {
+if (!decomp_param[i].compbuf) {
+break;
+}
+
+qemu_thread_join(decompress_threads + i);
+qemu_mutex_destroy(_param[i].mutex);
+qemu_cond_destroy(_param[i].cond);
+inflateEnd(_param[i].stream);
+g_free(decomp_param[i].compbuf);
+decomp_param[i].compbuf = NULL;
+}
+g_free(decompress_threads);
+g_free(decomp_param);
+decompress_threads = NULL;
+decomp_param = NULL;
+decomp_file = NULL;
+}
+
+int compress_threads_load_setup(QEMUFile *f)
+{
+int i, thread_count;
+
+if (!migrate_use_compression()) {
+return 0;
+}
+
+thread_count = migrate_decompress_threads();
+decompress_threads = g_new0(QemuThread, thread_count);
+decomp_param = g_new0(Decomp
[PATCH v2 02/13] ram.c: Let the compress threads return a CompressResult enum
This will be used in the next commits to move save_page_header()
out of compress code.
Signed-off-by: Lukas Straub
---
migration/ram.c | 34 ++
1 file changed, 22 insertions(+), 12 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index 79d881f735..ade6638a96 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -493,10 +493,17 @@ MigrationOps *migration_ops;
CompressionStats compression_counters;
+enum CompressResult {
+RES_NONE = 0,
+RES_ZEROPAGE = 1,
+RES_COMPRESS = 2
+};
+typedef enum CompressResult CompressResult;
+
struct CompressParam {
bool done;
bool quit;
-bool zero_page;
+CompressResult result;
QEMUFile *file;
QemuMutex mutex;
QemuCond cond;
@@ -538,8 +545,9 @@ static QemuCond decomp_done_cond;
static int ram_save_host_page_urgent(PageSearchStatus *pss);
-static bool do_compress_ram_page(QEMUFile *f, z_stream *stream, RAMBlock
*block,
- ram_addr_t offset, uint8_t *source_buf);
+static CompressResult do_compress_ram_page(QEMUFile *f, z_stream *stream,
+ RAMBlock *block, ram_addr_t offset,
+ uint8_t *source_buf);
/* NOTE: page is the PFN not real ram_addr_t. */
static void pss_init(PageSearchStatus *pss, RAMBlock *rb, ram_addr_t page)
@@ -564,7 +572,7 @@ static void *do_data_compress(void *opaque)
CompressParam *param = opaque;
RAMBlock *block;
ram_addr_t offset;
-bool zero_page;
+CompressResult result;
qemu_mutex_lock(>mutex);
while (!param->quit) {
@@ -574,12 +582,12 @@ static void *do_data_compress(void *opaque)
param->block = NULL;
qemu_mutex_unlock(>mutex);
-zero_page = do_compress_ram_page(param->file, >stream,
- block, offset, param->originbuf);
+result = do_compress_ram_page(param->file, >stream,
+ block, offset, param->originbuf);
qemu_mutex_lock(_done_lock);
param->done = true;
-param->zero_page = zero_page;
+param->result = result;
qemu_cond_signal(_done_cond);
qemu_mutex_unlock(_done_lock);
@@ -1463,8 +1471,9 @@ static int ram_save_multifd_page(QEMUFile *file, RAMBlock
*block,
return 1;
}
-static bool do_compress_ram_page(QEMUFile *f, z_stream *stream, RAMBlock
*block,
- ram_addr_t offset, uint8_t *source_buf)
+static CompressResult do_compress_ram_page(QEMUFile *f, z_stream *stream,
+ RAMBlock *block, ram_addr_t offset,
+ uint8_t *source_buf)
{
RAMState *rs = ram_state;
PageSearchStatus *pss = >pss[RAM_CHANNEL_PRECOPY];
@@ -1472,7 +1481,7 @@ static bool do_compress_ram_page(QEMUFile *f, z_stream
*stream, RAMBlock *block,
int ret;
if (save_zero_page_to_file(pss, f, block, offset)) {
-return true;
+return RES_ZEROPAGE;
}
save_page_header(pss, f, block, offset | RAM_SAVE_FLAG_COMPRESS_PAGE);
@@ -1487,8 +1496,9 @@ static bool do_compress_ram_page(QEMUFile *f, z_stream
*stream, RAMBlock *block,
if (ret < 0) {
qemu_file_set_error(migrate_get_current()->to_dst_file, ret);
error_report("compressed data failed!");
+return RES_NONE;
}
-return false;
+return RES_COMPRESS;
}
static void
@@ -1496,7 +1506,7 @@ update_compress_thread_counts(const CompressParam *param,
int bytes_xmit)
{
ram_transferred_add(bytes_xmit);
-if (param->zero_page) {
+if (param->result == RES_ZEROPAGE) {
stat64_add(_atomic_counters.duplicate, 1);
return;
}
--
2.40.0
pgp8GyYygDtfx.pgp
Description: OpenPGP digital signature
[PATCH v2 08/13] ram.c: Introduce whitespace (squash with next patch)
Introduce whitespace to make it easier to reroll the series.
Signed-off-by: Lukas Straub
---
migration/ram.c | 12
1 file changed, 12 insertions(+)
diff --git a/migration/ram.c b/migration/ram.c
index bd3773d4c4..b95c5c720d 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -602,6 +602,12 @@ static void *do_data_compress(void *opaque)
return NULL;
}
+
+
+/* split */
+
+
+
static void compress_threads_save_cleanup(void)
{
int i, thread_count;
@@ -641,6 +647,12 @@ static void compress_threads_save_cleanup(void)
comp_param = NULL;
}
+
+
+/* split */
+
+
+
static int compress_threads_save_setup(void)
{
int i, thread_count;
--
2.40.0
pgpSPuZoCWdq0.pgp
Description: OpenPGP digital signature
[PATCH v2 00/13] migration/ram.c: Refactor compress code
This series refactors the ram compress code. It first removes ram.c dependencies from the core compress code, then moves it out to its own file. Finally, on the migration destination side the initialisation and cleanup of compress threads is moved out of ram.c to migration.c. This allows using COLO with compress enabled. Changes since v1: - Add postcopy tests with compress enabled - Use page_size variable in "ram-compress.c: Make target independent" - Squash "ram.c: Remove unused include after moving out code" with "ram.c: Move core compression code into its own file" - Add Reviewed-by: Tags Lukas Straub (13): qtest/migration-test.c: Add postcopy tests with compress enabled ram.c: Let the compress threads return a CompressResult enum ram.c: Dont change param->block in the compress thread ram.c: Reset result after sending queued data ram.c: Do not call save_page_header() from compress threads ram.c: Call update_compress_thread_counts from compress_send_queued_data ram.c: Remove last ram.c dependency from the core compress code ram.c: Introduce whitespace (squash with next patch) ram.c: Move core compression code into its own file ram.c: Remove whitespace (squash with previous patch) ram.c: Move core decompression code into its own file ram compress: Assert that the file buffer matches the result ram-compress.c: Make target independent migration/meson.build| 5 +- migration/qemu-file.c| 11 + migration/qemu-file.h| 1 + migration/ram-compress.c | 484 ++ migration/ram-compress.h | 70 + migration/ram.c | 485 +++ tests/qtest/migration-test.c | 83 +++--- 7 files changed, 660 insertions(+), 479 deletions(-) create mode 100644 migration/ram-compress.c create mode 100644 migration/ram-compress.h -- 2.40.0 pgp_Hn5trCuB2.pgp Description: OpenPGP digital signature
[PATCH v2 12/13] ram compress: Assert that the file buffer matches the result
Before this series, "nothing to send" was handled by the file buffer
being empty. Now it is tracked via param->result.
Assert that the file buffer state matches the result.
Signed-off-by: Lukas Straub
---
migration/qemu-file.c| 11 +++
migration/qemu-file.h| 1 +
migration/ram-compress.c | 5 +
migration/ram.c | 2 ++
4 files changed, 19 insertions(+)
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 102ab3b439..2b3f3f8549 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -887,6 +887,17 @@ int qemu_put_qemu_file(QEMUFile *f_des, QEMUFile *f_src)
return len;
}
+/*
+ * Check if the writable buffer is empty
+ */
+
+bool qemu_file_buffer_empty(QEMUFile *file)
+{
+assert(qemu_file_is_writable(file));
+
+return !file->iovcnt;
+}
+
/*
* Get a string whose length is determined by a single preceding byte
* A preallocated 256 byte buffer must be passed in.
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index 9d0155a2a1..15e5f189f0 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -113,6 +113,7 @@ size_t qemu_get_buffer_in_place(QEMUFile *f, uint8_t **buf,
size_t size);
ssize_t qemu_put_compression_data(QEMUFile *f, z_stream *stream,
const uint8_t *p, size_t size);
int qemu_put_qemu_file(QEMUFile *f_des, QEMUFile *f_src);
+bool qemu_file_buffer_empty(QEMUFile *file);
/*
* Note that you can only peek continuous bytes from where the current pointer
diff --git a/migration/ram-compress.c b/migration/ram-compress.c
index f75b8c3079..b75a9d2b9a 100644
--- a/migration/ram-compress.c
+++ b/migration/ram-compress.c
@@ -193,6 +193,8 @@ static CompressResult do_compress_ram_page(QEMUFile *f,
z_stream *stream,
uint8_t *p = block->host + offset;
int ret;
+assert(qemu_file_buffer_empty(f));
+
if (buffer_is_zero(p, TARGET_PAGE_SIZE)) {
return RES_ZEROPAGE;
}
@@ -207,6 +209,7 @@ static CompressResult do_compress_ram_page(QEMUFile *f,
z_stream *stream,
if (ret < 0) {
qemu_file_set_error(migrate_get_current()->to_dst_file, ret);
error_report("compressed data failed!");
+qemu_fflush(f);
return RES_NONE;
}
return RES_COMPRESS;
@@ -238,6 +241,7 @@ void flush_compressed_data(int
(send_queued_data(CompressParam *)))
if (!comp_param[idx].quit) {
CompressParam *param = _param[idx];
send_queued_data(param);
+assert(qemu_file_buffer_empty(param->file));
compress_reset_result(param);
}
qemu_mutex_unlock(_param[idx].mutex);
@@ -267,6 +271,7 @@ retry:
qemu_mutex_lock(>mutex);
param->done = false;
send_queued_data(param);
+assert(qemu_file_buffer_empty(param->file));
compress_reset_result(param);
set_compress_params(param, block, offset);
diff --git a/migration/ram.c b/migration/ram.c
index 1e5dede6f0..ccfedf4fb5 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1332,11 +1332,13 @@ static int send_queued_data(CompressParam *param)
assert(block == pss->last_sent_block);
if (param->result == RES_ZEROPAGE) {
+assert(qemu_file_buffer_empty(param->file));
len += save_page_header(pss, file, block, offset | RAM_SAVE_FLAG_ZERO);
qemu_put_byte(file, 0);
len += 1;
ram_release_page(block->idstr, offset);
} else if (param->result == RES_COMPRESS) {
+assert(!qemu_file_buffer_empty(param->file));
len += save_page_header(pss, file, block,
offset | RAM_SAVE_FLAG_COMPRESS_PAGE);
len += qemu_put_qemu_file(file, param->file);
--
2.40.0
pgp0cqKaov7hE.pgp
Description: OpenPGP digital signature
[PATCH v2 06/13] ram.c: Call update_compress_thread_counts from compress_send_queued_data
This makes the core compress code more independend from ram.c.
Signed-off-by: Lukas Straub
---
migration/ram.c | 18 ++
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index d248e1f062..3894d0ae79 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1551,12 +1551,14 @@ static int send_queued_data(CompressParam *param)
abort();
}
+update_compress_thread_counts(param, len);
+
return len;
}
static void flush_compressed_data(RAMState *rs)
{
-int idx, len, thread_count;
+int idx, thread_count;
if (!save_page_use_compression(rs)) {
return;
@@ -1575,15 +1577,8 @@ static void flush_compressed_data(RAMState *rs)
qemu_mutex_lock(_param[idx].mutex);
if (!comp_param[idx].quit) {
CompressParam *param = _param[idx];
-len = send_queued_data(param);
+send_queued_data(param);
compress_reset_result(param);
-
-/*
- * it's safe to fetch zero_page without holding comp_done_lock
- * as there is no further request submitted to the thread,
- * i.e, the thread should be waiting for a request at this point.
- */
-update_compress_thread_counts(param, len);
}
qemu_mutex_unlock(_param[idx].mutex);
}
@@ -1599,7 +1594,7 @@ static inline void set_compress_params(CompressParam
*param, RAMBlock *block,
static int compress_page_with_multi_thread(RAMBlock *block, ram_addr_t offset)
{
-int idx, thread_count, bytes_xmit = -1, pages = -1;
+int idx, thread_count, pages = -1;
bool wait = migrate_compress_wait_thread();
thread_count = migrate_compress_threads();
@@ -1610,11 +1605,10 @@ retry:
CompressParam *param = _param[idx];
qemu_mutex_lock(>mutex);
param->done = false;
-bytes_xmit = send_queued_data(param);
+send_queued_data(param);
compress_reset_result(param);
set_compress_params(param, block, offset);
-update_compress_thread_counts(param, bytes_xmit);
qemu_cond_signal(>cond);
qemu_mutex_unlock(>mutex);
pages = 1;
--
2.40.0
pgptC0qHkxCXC.pgp
Description: OpenPGP digital signature
[PATCH v2 05/13] ram.c: Do not call save_page_header() from compress threads
save_page_header() accesses several global variables, so calling it
from multiple threads is pretty ugly.
Instead, call save_page_header() before writing out the compressed
data from the compress buffer to the migration stream.
This also makes the core compress code more independend from ram.c.
Signed-off-by: Lukas Straub
---
migration/ram.c | 44 +++-
1 file changed, 35 insertions(+), 9 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index 5ca0f115cf..d248e1f062 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1476,17 +1476,13 @@ static CompressResult do_compress_ram_page(QEMUFile *f,
z_stream *stream,
RAMBlock *block, ram_addr_t offset,
uint8_t *source_buf)
{
-RAMState *rs = ram_state;
-PageSearchStatus *pss = >pss[RAM_CHANNEL_PRECOPY];
uint8_t *p = block->host + offset;
int ret;
-if (save_zero_page_to_file(pss, f, block, offset)) {
+if (buffer_is_zero(p, TARGET_PAGE_SIZE)) {
return RES_ZEROPAGE;
}
-save_page_header(pss, f, block, offset | RAM_SAVE_FLAG_COMPRESS_PAGE);
-
/*
* copy it to a internal buffer to avoid it being modified by VM
* so that we can catch up the error during compression and
@@ -1526,9 +1522,40 @@ static inline void compress_reset_result(CompressParam
*param)
param->offset = 0;
}
-static void flush_compressed_data(RAMState *rs)
+static int send_queued_data(CompressParam *param)
{
+PageSearchStatus *pss = _state->pss[RAM_CHANNEL_PRECOPY];
MigrationState *ms = migrate_get_current();
+QEMUFile *file = ms->to_dst_file;
+int len = 0;
+
+RAMBlock *block = param->block;
+ram_addr_t offset = param->offset;
+
+if (param->result == RES_NONE) {
+return 0;
+}
+
+assert(block == pss->last_sent_block);
+
+if (param->result == RES_ZEROPAGE) {
+len += save_page_header(pss, file, block, offset | RAM_SAVE_FLAG_ZERO);
+qemu_put_byte(file, 0);
+len += 1;
+ram_release_page(block->idstr, offset);
+} else if (param->result == RES_COMPRESS) {
+len += save_page_header(pss, file, block,
+offset | RAM_SAVE_FLAG_COMPRESS_PAGE);
+len += qemu_put_qemu_file(file, param->file);
+} else {
+abort();
+}
+
+return len;
+}
+
+static void flush_compressed_data(RAMState *rs)
+{
int idx, len, thread_count;
if (!save_page_use_compression(rs)) {
@@ -1548,7 +1575,7 @@ static void flush_compressed_data(RAMState *rs)
qemu_mutex_lock(_param[idx].mutex);
if (!comp_param[idx].quit) {
CompressParam *param = _param[idx];
-len = qemu_put_qemu_file(ms->to_dst_file, param->file);
+len = send_queued_data(param);
compress_reset_result(param);
/*
@@ -1574,7 +1601,6 @@ static int compress_page_with_multi_thread(RAMBlock
*block, ram_addr_t offset)
{
int idx, thread_count, bytes_xmit = -1, pages = -1;
bool wait = migrate_compress_wait_thread();
-MigrationState *ms = migrate_get_current();
thread_count = migrate_compress_threads();
qemu_mutex_lock(_done_lock);
@@ -1584,7 +1610,7 @@ retry:
CompressParam *param = _param[idx];
qemu_mutex_lock(>mutex);
param->done = false;
-bytes_xmit = qemu_put_qemu_file(ms->to_dst_file, param->file);
+bytes_xmit = send_queued_data(param);
compress_reset_result(param);
set_compress_params(param, block, offset);
--
2.40.0
pgpszFBdTmWSK.pgp
Description: OpenPGP digital signature
Re: [PATCH v2 0/4] COLO: improve build options
On Thu, 20 Apr 2023 01:52:28 +0300 Vladimir Sementsov-Ogievskiy wrote: > Hi all! > > COLO substem seems to be useless when CONFIG_REPLICATION is unset, as we > simply don't allow to set x-colo capability in this case. So, let's not > compile in unreachable code and interface we cannot use when > CONFIG_REPLICATION is unset. > > Also, provide personal configure option for COLO Proxy subsystem. > > v1 was > [PATCH] replication: compile out some staff when replication is not configured > Supersedes: <20230411145112.497785-1-vsement...@yandex-team.ru> Hey, This series is a good idea, and looks fine to me. Maybe you can remove the #ifdef CONFIG_REPLICATION/#ifndef CONFIG_REPLICATION from migration/colo.c too while you are at it. Regards, Lukas Straub > Vladimir Sementsov-Ogievskiy (4): > block/meson.build: prefer positive condition for replication > scripts/qapi: allow optional experimental enum values > build: move COLO under CONFIG_REPLICATION > configure: add --disable-colo-filters option > > block/meson.build | 2 +- > hmp-commands.hx| 2 ++ > meson.build| 1 + > meson_options.txt | 2 ++ > migration/colo.c | 6 + > migration/meson.build | 6 +++-- > migration/migration-hmp-cmds.c | 2 ++ > migration/migration.c | 19 +++--- > net/meson.build| 16 +--- > qapi/migration.json| 12 ++--- > scripts/meson-buildoptions.sh | 3 +++ > scripts/qapi/types.py | 2 ++ > stubs/colo.c | 47 ++ > stubs/meson.build | 1 + > 14 files changed, 95 insertions(+), 26 deletions(-) > create mode 100644 stubs/colo.c > -- pgpAPWC4CS0Os.pgp Description: OpenPGP digital signature
Re: [PATCH v2] migration: Handle block device inactivation failures better
On Fri, 14 Apr 2023 10:33:58 -0500
Eric Blake wrote:
> Consider what happens when performing a migration between two host
> machines connected to an NFS server serving multiple block devices to
> the guest, when the NFS server becomes unavailable. The migration
> attempts to inactivate all block devices on the source (a necessary
> step before the destination can take over); but if the NFS server is
> non-responsive, the attempt to inactivate can itself fail. When that
> happens, the destination fails to get the migrated guest (good,
> because the source wasn't able to flush everything properly):
>
> (qemu) qemu-kvm: load of migration failed: Input/output error
>
> at which point, our only hope for the guest is for the source to take
> back control. With the current code base, the host outputs a message, but
> then appears to resume:
>
> (qemu) qemu-kvm: qemu_savevm_state_complete_precopy_non_iterable:
> bdrv_inactivate_all() failed (-1)
>
> (src qemu)info status
>VM status: running
>
> but a second migration attempt now asserts:
>
> (src qemu) qemu-kvm: ../block.c:6738: int
> bdrv_inactivate_recurse(BlockDriverState *): Assertion `!(bs->open_flags &
> BDRV_O_INACTIVE)' failed.
>
> Whether the guest is recoverable on the source after the first failure
> is debatable, but what we do not want is to have qemu itself fail due
> to an assertion. It looks like the problem is as follows:
>
> In migration.c:migration_completion(), the source sets 'inactivate' to
> true (since COLO is not enabled), then tries
> savevm.c:qemu_savevm_state_complete_precopy() with a request to
> inactivate block devices. In turn, this calls
> block.c:bdrv_inactivate_all(), which fails when flushing runs up
> against the non-responsive NFS server. With savevm failing, we are
> now left in a state where some, but not all, of the block devices have
> been inactivated; but migration_completion() then jumps to 'fail'
> rather than 'fail_invalidate' and skips an attempt to reclaim those
> those disks by calling bdrv_activate_all(). Even if we do attempt to
> reclaim disks, we aren't taking note of failure there, either.
>
> Thus, we have reached a state where the migration engine has forgotten
> all state about whether a block device is inactive, because we did not
> set s->block_inactive in enough places; so migration allows the source
> to reach vm_start() and resume execution, violating the block layer
> invariant that the guest CPUs should not be restarted while a device
> is inactive. Note that the code in migration.c:migrate_fd_cancel()
> will also try to reactivate all block devices if s->block_inactive was
> set, but because we failed to set that flag after the first failure,
> the source assumes it has reclaimed all devices, even though it still
> has remaining inactivated devices and does not try again. Normally,
> qmp_cont() will also try to reactivate all disks (or correctly fail if
> the disks are not reclaimable because NFS is not yet back up), but the
> auto-resumption of the source after a migration failure does not go
> through qmp_cont(). And because we have left the block layer in an
> inconsistent state with devices still inactivated, the later migration
> attempt is hitting the assertion failure.
>
> Since it is important to not resume the source with inactive disks,
> this patch marks s->block_inactive before attempting inactivation,
> rather than after succeeding, in order to prevent any vm_start() until
> it has successfully reactivated all devices.
>
> See also https://bugzilla.redhat.com/show_bug.cgi?id=2058982
>
> Signed-off-by: Eric Blake
Looks good to me from colo side of things.
Acked-by: Lukas Straub
Tested-by: Lukas Straub
Best Regards,
Lukas Straub
> ---
>
> v2: Set s->block_inactive sooner [Juan]
> ---
> migration/migration.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index bda47891933..cb0d42c0610 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -3444,13 +3444,11 @@ static void migration_completion(MigrationState *s)
> MIGRATION_STATUS_DEVICE);
> }
> if (ret >= 0) {
> +s->block_inactive = inactivate;
> qemu_file_set_rate_limit(s->to_dst_file, INT64_MAX);
> ret = qemu_savevm_state_complete_precopy(s->to_dst_file,
> false,
> inactivate);
> }
> -if (inactivate && ret >= 0) {
> -s->block_inactive = true;
> -
Re: [PATCH for-8.0 v2 2/2] migration/ram.c: Fix migration with compress enabled
Ping...
On Thu, 6 Apr 2023 15:21:55 +
Lukas Straub wrote:
> Ping? This should go in rc4, there is not much time left to prepare a
> PULL...
>
> Best Regards,
> Lukas Straub
>
> On Tue, 4 Apr 2023 14:36:03 +
> Lukas Straub wrote:
>
> > Since ec6f3ab9, migration with compress enabled was broken, because
> > the compress threads use a dummy QEMUFile which just acts as a
> > buffer and that commit accidentally changed it to use the outgoing
> > migration channel instead.
> >
> > Fix this by using the dummy file again in the compress threads.
> >
> > Fixes: ec6f3ab9f4 ("migration: Move last_sent_block into PageSearchStatus")
> > Signed-off-by: Lukas Straub
> > Reviewed-by: Peter Xu
> > ---
> > v2:
> > - Add Fixed: and Reviewed-by: tags
> >
> > migration/ram.c | 24 +++-
> > 1 file changed, 11 insertions(+), 13 deletions(-)
> >
> > diff --git a/migration/ram.c b/migration/ram.c
> > index 96e8a19a58..9d1817ab7b 100644
> > --- a/migration/ram.c
> > +++ b/migration/ram.c
> > @@ -688,12 +688,11 @@ exit:
> > * @offset: offset inside the block for the page
> > * in the lower bits, it contains flags
> > */
> > -static size_t save_page_header(PageSearchStatus *pss, RAMBlock *block,
> > - ram_addr_t offset)
> > +static size_t save_page_header(PageSearchStatus *pss, QEMUFile *f,
> > + RAMBlock *block, ram_addr_t offset)
> > {
> > size_t size, len;
> > bool same_block = (block == pss->last_sent_block);
> > -QEMUFile *f = pss->pss_channel;
> >
> > if (same_block) {
> > offset |= RAM_SAVE_FLAG_CONTINUE;
> > @@ -867,7 +866,7 @@ static int save_xbzrle_page(RAMState *rs,
> > PageSearchStatus *pss,
> > }
> >
> > /* Send XBZRLE based compressed page */
> > -bytes_xbzrle = save_page_header(pss, block,
> > +bytes_xbzrle = save_page_header(pss, pss->pss_channel, block,
> > offset | RAM_SAVE_FLAG_XBZRLE);
> > qemu_put_byte(file, ENCODING_FLAG_XBZRLE);
> > qemu_put_be16(file, encoded_len);
> > @@ -1302,15 +1301,14 @@ void ram_release_page(const char *rbname, uint64_t
> > offset)
> > * @block: block that contains the page we want to send
> > * @offset: offset inside the block for the page
> > */
> > -static int save_zero_page_to_file(PageSearchStatus *pss,
> > +static int save_zero_page_to_file(PageSearchStatus *pss, QEMUFile *file,
> >RAMBlock *block, ram_addr_t offset)
> > {
> > uint8_t *p = block->host + offset;
> > -QEMUFile *file = pss->pss_channel;
> > int len = 0;
> >
> > if (buffer_is_zero(p, TARGET_PAGE_SIZE)) {
> > -len += save_page_header(pss, block, offset | RAM_SAVE_FLAG_ZERO);
> > +len += save_page_header(pss, file, block, offset |
> > RAM_SAVE_FLAG_ZERO);
> > qemu_put_byte(file, 0);
> > len += 1;
> > ram_release_page(block->idstr, offset);
> > @@ -1327,10 +1325,10 @@ static int save_zero_page_to_file(PageSearchStatus
> > *pss,
> > * @block: block that contains the page we want to send
> > * @offset: offset inside the block for the page
> > */
> > -static int save_zero_page(PageSearchStatus *pss, RAMBlock *block,
> > +static int save_zero_page(PageSearchStatus *pss, QEMUFile *f, RAMBlock
> > *block,
> >ram_addr_t offset)
> > {
> > -int len = save_zero_page_to_file(pss, block, offset);
> > +int len = save_zero_page_to_file(pss, f, block, offset);
> >
> > if (len) {
> > stat64_add(_atomic_counters.duplicate, 1);
> > @@ -1394,7 +1392,7 @@ static int save_normal_page(PageSearchStatus *pss,
> > RAMBlock *block,
> > {
> > QEMUFile *file = pss->pss_channel;
> >
> > -ram_transferred_add(save_page_header(pss, block,
> > +ram_transferred_add(save_page_header(pss, pss->pss_channel, block,
> > offset | RAM_SAVE_FLAG_PAGE));
> > if (async) {
> > qemu_put_buffer_async(file, buf, TARGET_PAGE_SIZE,
> > @@ -1473,11 +1471,11 @@ static bool do_compress_ram_page(QEMUFile *f,
> > z_stream *stream, RAMBlock *block,
> > uint8_t *p = block->host + offset;
> > int ret;
> >
> > -if (save_zero_page_to_file(pss, block, offset
Re: [PATCH for-8.0 v2 2/2] migration/ram.c: Fix migration with compress enabled
Ping? This should go in rc4, there is not much time left to prepare a
PULL...
Best Regards,
Lukas Straub
On Tue, 4 Apr 2023 14:36:03 +
Lukas Straub wrote:
> Since ec6f3ab9, migration with compress enabled was broken, because
> the compress threads use a dummy QEMUFile which just acts as a
> buffer and that commit accidentally changed it to use the outgoing
> migration channel instead.
>
> Fix this by using the dummy file again in the compress threads.
>
> Fixes: ec6f3ab9f4 ("migration: Move last_sent_block into PageSearchStatus")
> Signed-off-by: Lukas Straub
> Reviewed-by: Peter Xu
> ---
> v2:
> - Add Fixed: and Reviewed-by: tags
>
> migration/ram.c | 24 +++-
> 1 file changed, 11 insertions(+), 13 deletions(-)
>
> diff --git a/migration/ram.c b/migration/ram.c
> index 96e8a19a58..9d1817ab7b 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -688,12 +688,11 @@ exit:
> * @offset: offset inside the block for the page
> * in the lower bits, it contains flags
> */
> -static size_t save_page_header(PageSearchStatus *pss, RAMBlock *block,
> - ram_addr_t offset)
> +static size_t save_page_header(PageSearchStatus *pss, QEMUFile *f,
> + RAMBlock *block, ram_addr_t offset)
> {
> size_t size, len;
> bool same_block = (block == pss->last_sent_block);
> -QEMUFile *f = pss->pss_channel;
>
> if (same_block) {
> offset |= RAM_SAVE_FLAG_CONTINUE;
> @@ -867,7 +866,7 @@ static int save_xbzrle_page(RAMState *rs,
> PageSearchStatus *pss,
> }
>
> /* Send XBZRLE based compressed page */
> -bytes_xbzrle = save_page_header(pss, block,
> +bytes_xbzrle = save_page_header(pss, pss->pss_channel, block,
> offset | RAM_SAVE_FLAG_XBZRLE);
> qemu_put_byte(file, ENCODING_FLAG_XBZRLE);
> qemu_put_be16(file, encoded_len);
> @@ -1302,15 +1301,14 @@ void ram_release_page(const char *rbname, uint64_t
> offset)
> * @block: block that contains the page we want to send
> * @offset: offset inside the block for the page
> */
> -static int save_zero_page_to_file(PageSearchStatus *pss,
> +static int save_zero_page_to_file(PageSearchStatus *pss, QEMUFile *file,
>RAMBlock *block, ram_addr_t offset)
> {
> uint8_t *p = block->host + offset;
> -QEMUFile *file = pss->pss_channel;
> int len = 0;
>
> if (buffer_is_zero(p, TARGET_PAGE_SIZE)) {
> -len += save_page_header(pss, block, offset | RAM_SAVE_FLAG_ZERO);
> +len += save_page_header(pss, file, block, offset |
> RAM_SAVE_FLAG_ZERO);
> qemu_put_byte(file, 0);
> len += 1;
> ram_release_page(block->idstr, offset);
> @@ -1327,10 +1325,10 @@ static int save_zero_page_to_file(PageSearchStatus
> *pss,
> * @block: block that contains the page we want to send
> * @offset: offset inside the block for the page
> */
> -static int save_zero_page(PageSearchStatus *pss, RAMBlock *block,
> +static int save_zero_page(PageSearchStatus *pss, QEMUFile *f, RAMBlock
> *block,
>ram_addr_t offset)
> {
> -int len = save_zero_page_to_file(pss, block, offset);
> +int len = save_zero_page_to_file(pss, f, block, offset);
>
> if (len) {
> stat64_add(_atomic_counters.duplicate, 1);
> @@ -1394,7 +1392,7 @@ static int save_normal_page(PageSearchStatus *pss,
> RAMBlock *block,
> {
> QEMUFile *file = pss->pss_channel;
>
> -ram_transferred_add(save_page_header(pss, block,
> +ram_transferred_add(save_page_header(pss, pss->pss_channel, block,
> offset | RAM_SAVE_FLAG_PAGE));
> if (async) {
> qemu_put_buffer_async(file, buf, TARGET_PAGE_SIZE,
> @@ -1473,11 +1471,11 @@ static bool do_compress_ram_page(QEMUFile *f,
> z_stream *stream, RAMBlock *block,
> uint8_t *p = block->host + offset;
> int ret;
>
> -if (save_zero_page_to_file(pss, block, offset)) {
> +if (save_zero_page_to_file(pss, f, block, offset)) {
> return true;
> }
>
> -save_page_header(pss, block, offset | RAM_SAVE_FLAG_COMPRESS_PAGE);
> +save_page_header(pss, f, block, offset | RAM_SAVE_FLAG_COMPRESS_PAGE);
>
> /*
> * copy it to a internal buffer to avoid it being modified by VM
> @@ -2355,7 +2353,7 @@ static int ram_save_target_page_legacy(RAMState *rs,
> PageSearchStatus *pss)
> return 1;
> }
>
> -res = save_zero_page(pss, block, offset);
> +res = save_zero_page(pss, pss->pss_channel, block, offset);
> if (res > 0) {
> /* Must let xbzrle know, otherwise a previous (now 0'd) cached
> * page would be stale
--
pgpHxxY0tmy_c.pgp
Description: OpenPGP digital signature
Re: [PATCH 00/14] migration/ram.c: Refactor compress code
On Sun, 2 Apr 2023 17:55:59 + Lukas Straub wrote: > This series refactors the ram compress code. > > It first removes ram.c dependencies from the core compress code, then > moves it out to its own file. Finally, on the migration destination side > the initialisation and cleanup of compress threads is moved out of ram.c > to migration.c. This allows using COLO with compress enabled. > > This series is based on the following series: > https://lore.kernel.org/qemu-devel/af76761aa6978071c5b8e9b872b697db465a5520.1680457631.git.lukasstra...@web.de/T/#t > Hmm, I'm wondering if I should add postcopy+compress tests in this series? Because if we start this, the test matrix really will explode. Regards, Lukas Straub -- pgpeJDh9uUWFw.pgp Description: OpenPGP digital signature
[PATCH for-8.0 v2 1/2] qtest/migration-test.c: Add tests with compress enabled
There has never been tests for migration with compress enabled.
Add suitable tests, testing with compress-wait-thread = false
too.
Signed-off-by: Lukas Straub
---
v2:
- Split into 2 tests, one with compress-wait-thread = true and faster
compress options. And one with compress-wait-thread = false and slower
compress options, so it definitely sends some pages uncompressed.
- Add comment for iterations = 2.
tests/qtest/migration-test.c | 103 +++
1 file changed, 103 insertions(+)
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 3b615b0da9..1f2a019ce0 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -406,6 +406,41 @@ static void migrate_set_parameter_str(QTestState *who,
const char *parameter,
migrate_check_parameter_str(who, parameter, value);
}
+static long long migrate_get_parameter_bool(QTestState *who,
+ const char *parameter)
+{
+QDict *rsp;
+int result;
+
+rsp = wait_command(who, "{ 'execute': 'query-migrate-parameters' }");
+result = qdict_get_bool(rsp, parameter);
+qobject_unref(rsp);
+return !!result;
+}
+
+static void migrate_check_parameter_bool(QTestState *who, const char
*parameter,
+int value)
+{
+int result;
+
+result = migrate_get_parameter_bool(who, parameter);
+g_assert_cmpint(result, ==, value);
+}
+
+static void migrate_set_parameter_bool(QTestState *who, const char *parameter,
+ int value)
+{
+QDict *rsp;
+
+rsp = qtest_qmp(who,
+"{ 'execute': 'migrate-set-parameters',"
+"'arguments': { %s: %i } }",
+parameter, value);
+g_assert(qdict_haskey(rsp, "return"));
+qobject_unref(rsp);
+migrate_check_parameter_bool(who, parameter, value);
+}
+
static void migrate_ensure_non_converge(QTestState *who)
{
/* Can't converge with 1ms downtime + 3 mbs bandwidth limit */
@@ -1524,6 +1559,70 @@ static void test_precopy_unix_xbzrle(void)
test_precopy_common();
}
+static void *
+test_migrate_compress_start(QTestState *from,
+QTestState *to)
+{
+migrate_set_parameter_int(from, "compress-level", 1);
+migrate_set_parameter_int(from, "compress-threads", 4);
+migrate_set_parameter_bool(from, "compress-wait-thread", true);
+migrate_set_parameter_int(to, "decompress-threads", 4);
+
+migrate_set_capability(from, "compress", true);
+migrate_set_capability(to, "compress", true);
+
+return NULL;
+}
+
+static void test_precopy_unix_compress(void)
+{
+g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
+MigrateCommon args = {
+.connect_uri = uri,
+.listen_uri = uri,
+.start_hook = test_migrate_compress_start,
+/*
+ * Test that no invalid thread state is left over from
+ * the previous iteration.
+ */
+.iterations = 2,
+};
+
+test_precopy_common();
+}
+
+static void *
+test_migrate_compress_nowait_start(QTestState *from,
+ QTestState *to)
+{
+migrate_set_parameter_int(from, "compress-level", 9);
+migrate_set_parameter_int(from, "compress-threads", 1);
+migrate_set_parameter_bool(from, "compress-wait-thread", false);
+migrate_set_parameter_int(to, "decompress-threads", 1);
+
+migrate_set_capability(from, "compress", true);
+migrate_set_capability(to, "compress", true);
+
+return NULL;
+}
+
+static void test_precopy_unix_compress_nowait(void)
+{
+g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
+MigrateCommon args = {
+.connect_uri = uri,
+.listen_uri = uri,
+.start_hook = test_migrate_compress_nowait_start,
+/*
+ * Test that no invalid thread state is left over from
+ * the previous iteration.
+ */
+.iterations = 2,
+};
+
+test_precopy_common();
+}
+
static void test_precopy_tcp_plain(void)
{
MigrateCommon args = {
@@ -2515,6 +2614,10 @@ int main(int argc, char **argv)
qtest_add_func("/migration/bad_dest", test_baddest);
qtest_add_func("/migration/precopy/unix/plain", test_precopy_unix_plain);
qtest_add_func("/migration/precopy/unix/xbzrle", test_precopy_unix_xbzrle);
+qtest_add_func("/migration/precopy/unix/compress/wait",
+ test_precopy_unix_compress);
+qtest_add_func("/migration/precopy/unix/compress/nowait",
+ test_precopy_unix_compress_nowait);
#ifdef CONFIG_GNUTLS
qtest_add_func("/migration/precopy/unix/tls/psk",
test_precopy_unix_tls_psk);
--
2.40.0
pgpuy2I02evKX.pgp
Description: OpenPGP digital signature
[PATCH for-8.0 v2 2/2] migration/ram.c: Fix migration with compress enabled
Since ec6f3ab9, migration with compress enabled was broken, because
the compress threads use a dummy QEMUFile which just acts as a
buffer and that commit accidentally changed it to use the outgoing
migration channel instead.
Fix this by using the dummy file again in the compress threads.
Fixes: ec6f3ab9f4 ("migration: Move last_sent_block into PageSearchStatus")
Signed-off-by: Lukas Straub
Reviewed-by: Peter Xu
---
v2:
- Add Fixed: and Reviewed-by: tags
migration/ram.c | 24 +++-
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index 96e8a19a58..9d1817ab7b 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -688,12 +688,11 @@ exit:
* @offset: offset inside the block for the page
* in the lower bits, it contains flags
*/
-static size_t save_page_header(PageSearchStatus *pss, RAMBlock *block,
- ram_addr_t offset)
+static size_t save_page_header(PageSearchStatus *pss, QEMUFile *f,
+ RAMBlock *block, ram_addr_t offset)
{
size_t size, len;
bool same_block = (block == pss->last_sent_block);
-QEMUFile *f = pss->pss_channel;
if (same_block) {
offset |= RAM_SAVE_FLAG_CONTINUE;
@@ -867,7 +866,7 @@ static int save_xbzrle_page(RAMState *rs, PageSearchStatus
*pss,
}
/* Send XBZRLE based compressed page */
-bytes_xbzrle = save_page_header(pss, block,
+bytes_xbzrle = save_page_header(pss, pss->pss_channel, block,
offset | RAM_SAVE_FLAG_XBZRLE);
qemu_put_byte(file, ENCODING_FLAG_XBZRLE);
qemu_put_be16(file, encoded_len);
@@ -1302,15 +1301,14 @@ void ram_release_page(const char *rbname, uint64_t
offset)
* @block: block that contains the page we want to send
* @offset: offset inside the block for the page
*/
-static int save_zero_page_to_file(PageSearchStatus *pss,
+static int save_zero_page_to_file(PageSearchStatus *pss, QEMUFile *file,
RAMBlock *block, ram_addr_t offset)
{
uint8_t *p = block->host + offset;
-QEMUFile *file = pss->pss_channel;
int len = 0;
if (buffer_is_zero(p, TARGET_PAGE_SIZE)) {
-len += save_page_header(pss, block, offset | RAM_SAVE_FLAG_ZERO);
+len += save_page_header(pss, file, block, offset | RAM_SAVE_FLAG_ZERO);
qemu_put_byte(file, 0);
len += 1;
ram_release_page(block->idstr, offset);
@@ -1327,10 +1325,10 @@ static int save_zero_page_to_file(PageSearchStatus *pss,
* @block: block that contains the page we want to send
* @offset: offset inside the block for the page
*/
-static int save_zero_page(PageSearchStatus *pss, RAMBlock *block,
+static int save_zero_page(PageSearchStatus *pss, QEMUFile *f, RAMBlock *block,
ram_addr_t offset)
{
-int len = save_zero_page_to_file(pss, block, offset);
+int len = save_zero_page_to_file(pss, f, block, offset);
if (len) {
stat64_add(_atomic_counters.duplicate, 1);
@@ -1394,7 +1392,7 @@ static int save_normal_page(PageSearchStatus *pss,
RAMBlock *block,
{
QEMUFile *file = pss->pss_channel;
-ram_transferred_add(save_page_header(pss, block,
+ram_transferred_add(save_page_header(pss, pss->pss_channel, block,
offset | RAM_SAVE_FLAG_PAGE));
if (async) {
qemu_put_buffer_async(file, buf, TARGET_PAGE_SIZE,
@@ -1473,11 +1471,11 @@ static bool do_compress_ram_page(QEMUFile *f, z_stream
*stream, RAMBlock *block,
uint8_t *p = block->host + offset;
int ret;
-if (save_zero_page_to_file(pss, block, offset)) {
+if (save_zero_page_to_file(pss, f, block, offset)) {
return true;
}
-save_page_header(pss, block, offset | RAM_SAVE_FLAG_COMPRESS_PAGE);
+save_page_header(pss, f, block, offset | RAM_SAVE_FLAG_COMPRESS_PAGE);
/*
* copy it to a internal buffer to avoid it being modified by VM
@@ -2355,7 +2353,7 @@ static int ram_save_target_page_legacy(RAMState *rs,
PageSearchStatus *pss)
return 1;
}
-res = save_zero_page(pss, block, offset);
+res = save_zero_page(pss, pss->pss_channel, block, offset);
if (res > 0) {
/* Must let xbzrle know, otherwise a previous (now 0'd) cached
* page would be stale
--
2.40.0
pgp7ahzxffZlY.pgp
Description: OpenPGP digital signature
Re: [PATCH RESEND 1/2] qtest/migration-test.c: Add test with compress enabled
On Mon, 3 Apr 2023 17:17:52 -0400
Peter Xu wrote:
> On Sun, Apr 02, 2023 at 05:47:45PM +0000, Lukas Straub wrote:
> > There has never been a test for migration with compress enabled.
> >
> > Add a suitable test, testing with compress-wait-thread = false
> > too.
> >
> > iterations = 2 is intentional, so it also tests that no invalid
> > thread state is left over from the previous iteration.
> >
> > Signed-off-by: Lukas Straub
>
> Overall looks good to me:
>
> Reviewed-by: Peter Xu
>
> A few nitpicks below.
>
> > ---
> > tests/qtest/migration-test.c | 67
> > 1 file changed, 67 insertions(+)
> >
> > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> > index 3b615b0da9..dbcab2e8ae 100644
> > --- a/tests/qtest/migration-test.c
> > +++ b/tests/qtest/migration-test.c
> >
> > [...]
> >
> > static void migrate_ensure_non_converge(QTestState *who)
> > {
> > /* Can't converge with 1ms downtime + 3 mbs bandwidth limit */
> > @@ -1524,6 +1559,36 @@ static void test_precopy_unix_xbzrle(void)
> > test_precopy_common();
> > }
> >
> > +static void *
> > +test_migrate_compress_start(QTestState *from,
> > + QTestState *to)
> > +{
> > +migrate_set_parameter_int(from, "compress-level", 9);
> > +migrate_set_parameter_int(from, "compress-threads", 1);
> > +migrate_set_parameter_bool(from, "compress-wait-thread", false);
>
> May worth trying both true/false (can split into two tests)?
Maybe, I just wasn't sure with your CI resources being tight, whether
I should add more tests. I think this test gives the most "bang for the
buck".
> > +migrate_set_parameter_int(to, "decompress-threads", 1);
>
> Why not set both compress/decompress threads to something >1 to check arace
> conditions between the threads?
I just wanted to make sure that it won't get too fast so it really
sends some pages uncompressed. But I guess this can be fixed when
splitting it into 2 tests.
> > +
> > +migrate_set_capability(from, "compress", true);
> > +migrate_set_capability(to, "compress", true);
> > +
> > +return NULL;
> > +}
> > +
> > +static void test_precopy_unix_compress(void)
> > +{
> > +g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
> > +MigrateCommon args = {
> > +.connect_uri = uri,
> > +.listen_uri = uri,
> > +
>
> Empty line.
>
> > +.start_hook = test_migrate_compress_start,
> > +
>
> Empty line.
>
> > +.iterations = 2,
>
> Maybe move the comment in commit message over here?
Good idea, will fix in the next version.
Regards,
Lukas Straub
> > +};
> > +
> > +test_precopy_common();
> > +}
> > +
> > static void test_precopy_tcp_plain(void)
> > {
> > MigrateCommon args = {
> > @@ -2515,6 +2580,8 @@ int main(int argc, char **argv)
> > qtest_add_func("/migration/bad_dest", test_baddest);
> > qtest_add_func("/migration/precopy/unix/plain",
> > test_precopy_unix_plain);
> > qtest_add_func("/migration/precopy/unix/xbzrle",
> > test_precopy_unix_xbzrle);
> > +qtest_add_func("/migration/precopy/unix/compress",
> > + test_precopy_unix_compress);
> > #ifdef CONFIG_GNUTLS
> > qtest_add_func("/migration/precopy/unix/tls/psk",
> > test_precopy_unix_tls_psk);
> > --
> > 2.30.2
> >
>
>
>
--
pgpH1sswvwjmD.pgp
Description: OpenPGP digital signature
Re: [PATCH 01/14] ram.c: Let the compress threads return a CompressResult enum
On Mon, 3 Apr 2023 09:25:41 +0200
Philippe Mathieu-Daudé wrote:
> On 2/4/23 19:56, Lukas Straub wrote:
> > This will be used in the next commits to move save_page_header()
> > out of compress code.
> >
> > Signed-off-by: Lukas Straub
> > ---
> > migration/ram.c | 34 ++
> > 1 file changed, 22 insertions(+), 12 deletions(-)
> >
> > diff --git a/migration/ram.c b/migration/ram.c
> > index 9d1817ab7b..ca561e62bd 100644
> > --- a/migration/ram.c
> > +++ b/migration/ram.c
> > @@ -493,10 +493,17 @@ MigrationOps *migration_ops;
> >
> > CompressionStats compression_counters;
> >
> > +enum CompressResult {
> > +RES_NONE = 0,
>
> What about RES_INVALID?
I think RES_NONE is more accurate, because having no result is a common
case. The submit side first handles the result from a previous
compression and then submits the new compression request. And for
example, when submitting the very first request to the thread there
won't be a previous result. Or when submitting after the threads where
flushed.
I just opted to return RES_NONE on error, because it seems more correct.
> > +RES_ZEROPAGE = 1,
> > +RES_COMPRESS = 2
> > +};
>
>
> > -static bool do_compress_ram_page(QEMUFile *f, z_stream *stream, RAMBlock
> > *block,
> > - ram_addr_t offset, uint8_t *source_buf)
> > +static CompressResult do_compress_ram_page(QEMUFile *f, z_stream *stream,
> > + RAMBlock *block, ram_addr_t
> > offset,
> > + uint8_t *source_buf)
> > {
>
>
> > if (ret < 0) {
> > qemu_file_set_error(migrate_get_current()->to_dst_file, ret);
> > error_report("compressed data failed!");
> > +return RES_NONE;
> > }
> > -return false;
> > +return RES_COMPRESS;
> > }
>
--
pgp0dlTxgRk13.pgp
Description: OpenPGP digital signature
[PATCH 14/14] migration: Initialize and cleanup decompression in migration.c
This fixes compress with colo.
Signed-off-by: Lukas Straub
---
migration/migration.c | 9 +
migration/ram.c | 5 -
2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index ae2025d9d8..cbdc10b840 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -26,6 +26,7 @@
#include "sysemu/cpu-throttle.h"
#include "rdma.h"
#include "ram.h"
+#include "ram-compress.h"
#include "migration/global_state.h"
#include "migration/misc.h"
#include "migration.h"
@@ -316,6 +317,7 @@ void migration_incoming_state_destroy(void)
struct MigrationIncomingState *mis = migration_incoming_get_current();
multifd_load_cleanup();
+compress_threads_load_cleanup();
if (mis->to_src_file) {
/* Tell source that we are done */
@@ -598,6 +600,12 @@ process_incoming_migration_co(void *opaque)
Error *local_err = NULL;
assert(mis->from_src_file);
+
+if (compress_threads_load_setup(mis->from_src_file)) {
+error_report("Failed to setup decompress threads");
+goto fail;
+}
+
mis->migration_incoming_co = qemu_coroutine_self();
mis->largest_page_size = qemu_ram_pagesize_largest();
postcopy_state_set(POSTCOPY_INCOMING_NONE);
@@ -663,6 +671,7 @@ fail:
qemu_fclose(mis->from_src_file);
multifd_load_cleanup();
+compress_threads_load_cleanup();
exit(EXIT_FAILURE);
}
diff --git a/migration/ram.c b/migration/ram.c
index 9072d70f7c..e9a295fab9 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3559,10 +3559,6 @@ void colo_release_ram_cache(void)
*/
static int ram_load_setup(QEMUFile *f, void *opaque)
{
-if (compress_threads_load_setup(f)) {
-return -1;
-}
-
xbzrle_load_setup();
ramblock_recv_map_init();
@@ -3578,7 +3574,6 @@ static int ram_load_cleanup(void *opaque)
}
xbzrle_load_cleanup();
-compress_threads_load_cleanup();
RAMBLOCK_FOREACH_NOT_IGNORED(rb) {
g_free(rb->receivedmap);
--
2.30.2
pgpqSeCtt0bsc.pgp
Description: OpenPGP digital signature
[PATCH 10/14] ram.c: Move core decompression code into its own file
No functional changes intended.
Signed-off-by: Lukas Straub
---
migration/ram-compress.c | 203 ++
migration/ram-compress.h | 5 +
migration/ram.c | 204 ---
3 files changed, 208 insertions(+), 204 deletions(-)
diff --git a/migration/ram-compress.c b/migration/ram-compress.c
index 77902a1d65..f75b8c3079 100644
--- a/migration/ram-compress.c
+++ b/migration/ram-compress.c
@@ -47,6 +47,24 @@ static QemuThread *compress_threads;
static QemuMutex comp_done_lock;
static QemuCond comp_done_cond;
+struct DecompressParam {
+bool done;
+bool quit;
+QemuMutex mutex;
+QemuCond cond;
+void *des;
+uint8_t *compbuf;
+int len;
+z_stream stream;
+};
+typedef struct DecompressParam DecompressParam;
+
+static QEMUFile *decomp_file;
+static DecompressParam *decomp_param;
+static QemuThread *decompress_threads;
+static QemuMutex decomp_done_lock;
+static QemuCond decomp_done_cond;
+
static CompressResult do_compress_ram_page(QEMUFile *f, z_stream *stream,
RAMBlock *block, ram_addr_t offset,
uint8_t *source_buf);
@@ -271,3 +289,188 @@ retry:
return pages;
}
+
+/* return the size after decompression, or negative value on error */
+static int
+qemu_uncompress_data(z_stream *stream, uint8_t *dest, size_t dest_len,
+ const uint8_t *source, size_t source_len)
+{
+int err;
+
+err = inflateReset(stream);
+if (err != Z_OK) {
+return -1;
+}
+
+stream->avail_in = source_len;
+stream->next_in = (uint8_t *)source;
+stream->avail_out = dest_len;
+stream->next_out = dest;
+
+err = inflate(stream, Z_NO_FLUSH);
+if (err != Z_STREAM_END) {
+return -1;
+}
+
+return stream->total_out;
+}
+
+static void *do_data_decompress(void *opaque)
+{
+DecompressParam *param = opaque;
+unsigned long pagesize;
+uint8_t *des;
+int len, ret;
+
+qemu_mutex_lock(>mutex);
+while (!param->quit) {
+if (param->des) {
+des = param->des;
+len = param->len;
+param->des = 0;
+qemu_mutex_unlock(>mutex);
+
+pagesize = TARGET_PAGE_SIZE;
+
+ret = qemu_uncompress_data(>stream, des, pagesize,
+ param->compbuf, len);
+if (ret < 0 && migrate_get_current()->decompress_error_check) {
+error_report("decompress data failed");
+qemu_file_set_error(decomp_file, ret);
+}
+
+qemu_mutex_lock(_done_lock);
+param->done = true;
+qemu_cond_signal(_done_cond);
+qemu_mutex_unlock(_done_lock);
+
+qemu_mutex_lock(>mutex);
+} else {
+qemu_cond_wait(>cond, >mutex);
+}
+}
+qemu_mutex_unlock(>mutex);
+
+return NULL;
+}
+
+int wait_for_decompress_done(void)
+{
+int idx, thread_count;
+
+if (!migrate_use_compression()) {
+return 0;
+}
+
+thread_count = migrate_decompress_threads();
+qemu_mutex_lock(_done_lock);
+for (idx = 0; idx < thread_count; idx++) {
+while (!decomp_param[idx].done) {
+qemu_cond_wait(_done_cond, _done_lock);
+}
+}
+qemu_mutex_unlock(_done_lock);
+return qemu_file_get_error(decomp_file);
+}
+
+void compress_threads_load_cleanup(void)
+{
+int i, thread_count;
+
+if (!migrate_use_compression()) {
+return;
+}
+thread_count = migrate_decompress_threads();
+for (i = 0; i < thread_count; i++) {
+/*
+ * we use it as a indicator which shows if the thread is
+ * properly init'd or not
+ */
+if (!decomp_param[i].compbuf) {
+break;
+}
+
+qemu_mutex_lock(_param[i].mutex);
+decomp_param[i].quit = true;
+qemu_cond_signal(_param[i].cond);
+qemu_mutex_unlock(_param[i].mutex);
+}
+for (i = 0; i < thread_count; i++) {
+if (!decomp_param[i].compbuf) {
+break;
+}
+
+qemu_thread_join(decompress_threads + i);
+qemu_mutex_destroy(_param[i].mutex);
+qemu_cond_destroy(_param[i].cond);
+inflateEnd(_param[i].stream);
+g_free(decomp_param[i].compbuf);
+decomp_param[i].compbuf = NULL;
+}
+g_free(decompress_threads);
+g_free(decomp_param);
+decompress_threads = NULL;
+decomp_param = NULL;
+decomp_file = NULL;
+}
+
+int compress_threads_load_setup(QEMUFile *f)
+{
+int i, thread_count;
+
+if (!migrate_use_compression()) {
+return 0;
+}
+
+thread_count = migrate_decompress_threads();
+decompress_threads = g_new0(QemuThread, thread_count);
+decomp_param = g_new0(DecompressParam, thread_count);
+qemu_mutex_i
[PATCH 07/14] ram.c: Introduce whitespace (squash with next patch)
Introduce whitespace to make it easier to reroll the series.
Signed-off-by: Lukas Straub
---
migration/ram.c | 12
1 file changed, 12 insertions(+)
diff --git a/migration/ram.c b/migration/ram.c
index 20428ccf42..475c04a18b 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -602,6 +602,12 @@ static void *do_data_compress(void *opaque)
return NULL;
}
+
+
+/* split */
+
+
+
static void compress_threads_save_cleanup(void)
{
int i, thread_count;
@@ -641,6 +647,12 @@ static void compress_threads_save_cleanup(void)
comp_param = NULL;
}
+
+
+/* split */
+
+
+
static int compress_threads_save_setup(void)
{
int i, thread_count;
--
2.30.2
pgpp20qnt0baL.pgp
Description: OpenPGP digital signature
[PATCH 08/14] ram.c: Move core compression code into its own file
No functional changes intended.
Signed-off-by: Lukas Straub
---
migration/meson.build| 5 +-
migration/ram-compress.c | 273 +++
migration/ram-compress.h | 65 ++
migration/ram.c | 255 +---
4 files changed, 343 insertions(+), 255 deletions(-)
create mode 100644 migration/ram-compress.c
create mode 100644 migration/ram-compress.h
diff --git a/migration/meson.build b/migration/meson.build
index 0d1bb9f96e..262e3c9754 100644
--- a/migration/meson.build
+++ b/migration/meson.build
@@ -36,4 +36,7 @@ endif
softmmu_ss.add(when: zstd, if_true: files('multifd-zstd.c'))
specific_ss.add(when: 'CONFIG_SOFTMMU',
-if_true: files('dirtyrate.c', 'ram.c', 'target.c'))
+if_true: files('dirtyrate.c',
+ 'ram.c',
+ 'ram-compress.c',
+ 'target.c'))
diff --git a/migration/ram-compress.c b/migration/ram-compress.c
new file mode 100644
index 00..77902a1d65
--- /dev/null
+++ b/migration/ram-compress.c
@@ -0,0 +1,273 @@
+/*
+ * QEMU System Emulator
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ * Copyright (c) 2011-2015 Red Hat Inc
+ *
+ * Authors:
+ * Juan Quintela
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/cutils.h"
+
+#include "ram-compress.h"
+
+#include "qemu/error-report.h"
+#include "migration.h"
+#include "io/channel-null.h"
+#include "exec/ram_addr.h"
+
+CompressionStats compression_counters;
+
+static CompressParam *comp_param;
+static QemuThread *compress_threads;
+/* comp_done_cond is used to wake up the migration thread when
+ * one of the compression threads has finished the compression.
+ * comp_done_lock is used to co-work with comp_done_cond.
+ */
+static QemuMutex comp_done_lock;
+static QemuCond comp_done_cond;
+
+static CompressResult do_compress_ram_page(QEMUFile *f, z_stream *stream,
+ RAMBlock *block, ram_addr_t offset,
+ uint8_t *source_buf);
+
+static void *do_data_compress(void *opaque)
+{
+CompressParam *param = opaque;
+RAMBlock *block;
+ram_addr_t offset;
+CompressResult result;
+
+qemu_mutex_lock(>mutex);
+while (!param->quit) {
+if (param->trigger) {
+block = param->block;
+offset = param->offset;
+param->trigger = false;
+qemu_mutex_unlock(>mutex);
+
+result = do_compress_ram_page(param->file, >stream,
+ block, offset, param->originbuf);
+
+qemu_mutex_lock(_done_lock);
+param->done = true;
+param->result = result;
+qemu_cond_signal(_done_cond);
+qemu_mutex_unlock(_done_lock);
+
+qemu_mutex_lock(>mutex);
+} else {
+qemu_cond_wait(>cond, >mutex);
+}
+}
+qemu_mutex_unlock(>mutex);
+
+return NULL;
+}
+
+void compress_threads_save_cleanup(void)
+{
+int i, thread_count;
+
+if (!migrate_use_compression() || !comp_param) {
+return;
+}
+
+thread_count = migrate_compress_threads();
+for (i = 0; i < thread_count; i++) {
+/*
+ * we use it as a indicator which shows if the thread is
+ * properly init'd or not
+ */
+if (!comp_param[i].file) {
+break;
+}
+
+qemu_mutex_lock(_param[i].mutex);
+comp_param[i].quit = true;
+qemu_cond_signal(_param[i].cond);
+qemu_mutex_unlock(_param[i].mutex);
+
+qemu_thread_join(compress_threads + i);
+qemu_mutex_destroy(_param[i].mutex);
+qemu_cond_dest
[PATCH 03/14] ram.c: Reset result after sending queued data
And take the param->mutex lock for the whole section to ensure
thread-safety.
Now, it is explicitly clear if there is no queued data to send.
Before, this was handled by param->file stream being empty and thus
qemu_put_qemu_file() not sending anything.
This will be used in the next commits to move save_page_header()
out of compress code.
Signed-off-by: Lukas Straub
---
migration/ram.c | 32 ++--
1 file changed, 22 insertions(+), 10 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index 3c9fac086d..bef6292ef7 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1519,6 +1519,13 @@ update_compress_thread_counts(const CompressParam
*param, int bytes_xmit)
static bool save_page_use_compression(RAMState *rs);
+static inline void compress_reset_result(CompressParam *param)
+{
+param->result = RES_NONE;
+param->block = NULL;
+param->offset = 0;
+}
+
static void flush_compressed_data(RAMState *rs)
{
MigrationState *ms = migrate_get_current();
@@ -1540,13 +1547,16 @@ static void flush_compressed_data(RAMState *rs)
for (idx = 0; idx < thread_count; idx++) {
qemu_mutex_lock(_param[idx].mutex);
if (!comp_param[idx].quit) {
-len = qemu_put_qemu_file(ms->to_dst_file, comp_param[idx].file);
+CompressParam *param = _param[idx];
+len = qemu_put_qemu_file(ms->to_dst_file, param->file);
+compress_reset_result(param);
+
/*
* it's safe to fetch zero_page without holding comp_done_lock
* as there is no further request submitted to the thread,
* i.e, the thread should be waiting for a request at this point.
*/
-update_compress_thread_counts(_param[idx], len);
+update_compress_thread_counts(param, len);
}
qemu_mutex_unlock(_param[idx].mutex);
}
@@ -1571,15 +1581,17 @@ static int compress_page_with_multi_thread(RAMBlock
*block, ram_addr_t offset)
retry:
for (idx = 0; idx < thread_count; idx++) {
if (comp_param[idx].done) {
-comp_param[idx].done = false;
-bytes_xmit = qemu_put_qemu_file(ms->to_dst_file,
-comp_param[idx].file);
-qemu_mutex_lock(_param[idx].mutex);
-set_compress_params(_param[idx], block, offset);
-qemu_cond_signal(_param[idx].cond);
-qemu_mutex_unlock(_param[idx].mutex);
+CompressParam *param = _param[idx];
+qemu_mutex_lock(>mutex);
+param->done = false;
+bytes_xmit = qemu_put_qemu_file(ms->to_dst_file, param->file);
+compress_reset_result(param);
+set_compress_params(param, block, offset);
+
+update_compress_thread_counts(param, bytes_xmit);
+qemu_cond_signal(>cond);
+qemu_mutex_unlock(>mutex);
pages = 1;
-update_compress_thread_counts(_param[idx], bytes_xmit);
break;
}
}
--
2.30.2
pgpJY5PCn8mgr.pgp
Description: OpenPGP digital signature
[PATCH 11/14] ram compress: Assert that the file buffer matches the result
Before this series, "nothing to send" was handled by the file buffer
being empty. Now it is tracked via param->result.
Assert that the file buffer state matches the result.
Signed-off-by: Lukas Straub
---
migration/qemu-file.c| 11 +++
migration/qemu-file.h| 1 +
migration/ram-compress.c | 5 +
migration/ram.c | 2 ++
4 files changed, 19 insertions(+)
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 102ab3b439..2b3f3f8549 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -887,6 +887,17 @@ int qemu_put_qemu_file(QEMUFile *f_des, QEMUFile *f_src)
return len;
}
+/*
+ * Check if the writable buffer is empty
+ */
+
+bool qemu_file_buffer_empty(QEMUFile *file)
+{
+assert(qemu_file_is_writable(file));
+
+return !file->iovcnt;
+}
+
/*
* Get a string whose length is determined by a single preceding byte
* A preallocated 256 byte buffer must be passed in.
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index 9d0155a2a1..15e5f189f0 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -113,6 +113,7 @@ size_t qemu_get_buffer_in_place(QEMUFile *f, uint8_t **buf,
size_t size);
ssize_t qemu_put_compression_data(QEMUFile *f, z_stream *stream,
const uint8_t *p, size_t size);
int qemu_put_qemu_file(QEMUFile *f_des, QEMUFile *f_src);
+bool qemu_file_buffer_empty(QEMUFile *file);
/*
* Note that you can only peek continuous bytes from where the current pointer
diff --git a/migration/ram-compress.c b/migration/ram-compress.c
index f75b8c3079..b75a9d2b9a 100644
--- a/migration/ram-compress.c
+++ b/migration/ram-compress.c
@@ -193,6 +193,8 @@ static CompressResult do_compress_ram_page(QEMUFile *f,
z_stream *stream,
uint8_t *p = block->host + offset;
int ret;
+assert(qemu_file_buffer_empty(f));
+
if (buffer_is_zero(p, TARGET_PAGE_SIZE)) {
return RES_ZEROPAGE;
}
@@ -207,6 +209,7 @@ static CompressResult do_compress_ram_page(QEMUFile *f,
z_stream *stream,
if (ret < 0) {
qemu_file_set_error(migrate_get_current()->to_dst_file, ret);
error_report("compressed data failed!");
+qemu_fflush(f);
return RES_NONE;
}
return RES_COMPRESS;
@@ -238,6 +241,7 @@ void flush_compressed_data(int
(send_queued_data(CompressParam *)))
if (!comp_param[idx].quit) {
CompressParam *param = _param[idx];
send_queued_data(param);
+assert(qemu_file_buffer_empty(param->file));
compress_reset_result(param);
}
qemu_mutex_unlock(_param[idx].mutex);
@@ -267,6 +271,7 @@ retry:
qemu_mutex_lock(>mutex);
param->done = false;
send_queued_data(param);
+assert(qemu_file_buffer_empty(param->file));
compress_reset_result(param);
set_compress_params(param, block, offset);
diff --git a/migration/ram.c b/migration/ram.c
index 5a2486bf89..7a8f540737 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1333,11 +1333,13 @@ static int send_queued_data(CompressParam *param)
assert(block == pss->last_sent_block);
if (param->result == RES_ZEROPAGE) {
+assert(qemu_file_buffer_empty(param->file));
len += save_page_header(pss, file, block, offset | RAM_SAVE_FLAG_ZERO);
qemu_put_byte(file, 0);
len += 1;
ram_release_page(block->idstr, offset);
} else if (param->result == RES_COMPRESS) {
+assert(!qemu_file_buffer_empty(param->file));
len += save_page_header(pss, file, block,
offset | RAM_SAVE_FLAG_COMPRESS_PAGE);
len += qemu_put_qemu_file(file, param->file);
--
2.30.2
pgpAVrgY2WIE4.pgp
Description: OpenPGP digital signature
[PATCH 09/14] ram.c: Remove whitespace (squash with previous patch)
Signed-off-by: Lukas Straub --- migration/ram.c | 18 -- 1 file changed, 18 deletions(-) diff --git a/migration/ram.c b/migration/ram.c index 114901241e..31b9b0b9ec 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -530,24 +530,6 @@ static bool pss_overlap(PageSearchStatus *pss1, PageSearchStatus *pss2) (pss1->host_page_start == pss2->host_page_start); } - - - - -/* split */ - - - - - - - -/* split */ - - - - - /** * save_page_header: write page header to wire * -- 2.30.2 pgpLVKcFG27Tm.pgp Description: OpenPGP digital signature
[PATCH 04/14] ram.c: Do not call save_page_header() from compress threads
save_page_header() accesses several global variables, so calling it
from multiple threads is pretty ugly.
Instead, call save_page_header() before writing out the compressed
data from the compress buffer to the migration stream.
This also makes the core compress code more independend from ram.c.
Signed-off-by: Lukas Straub
---
migration/ram.c | 44 +++-
1 file changed, 35 insertions(+), 9 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index bef6292ef7..7ab008145b 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1476,17 +1476,13 @@ static CompressResult do_compress_ram_page(QEMUFile *f,
z_stream *stream,
RAMBlock *block, ram_addr_t offset,
uint8_t *source_buf)
{
-RAMState *rs = ram_state;
-PageSearchStatus *pss = >pss[RAM_CHANNEL_PRECOPY];
uint8_t *p = block->host + offset;
int ret;
-if (save_zero_page_to_file(pss, f, block, offset)) {
+if (buffer_is_zero(p, TARGET_PAGE_SIZE)) {
return RES_ZEROPAGE;
}
-save_page_header(pss, f, block, offset | RAM_SAVE_FLAG_COMPRESS_PAGE);
-
/*
* copy it to a internal buffer to avoid it being modified by VM
* so that we can catch up the error during compression and
@@ -1526,9 +1522,40 @@ static inline void compress_reset_result(CompressParam
*param)
param->offset = 0;
}
-static void flush_compressed_data(RAMState *rs)
+static int send_queued_data(CompressParam *param)
{
+PageSearchStatus *pss = _state->pss[RAM_CHANNEL_PRECOPY];
MigrationState *ms = migrate_get_current();
+QEMUFile *file = ms->to_dst_file;
+int len = 0;
+
+RAMBlock *block = param->block;
+ram_addr_t offset = param->offset;
+
+if (param->result == RES_NONE) {
+return 0;
+}
+
+assert(block == pss->last_sent_block);
+
+if (param->result == RES_ZEROPAGE) {
+len += save_page_header(pss, file, block, offset | RAM_SAVE_FLAG_ZERO);
+qemu_put_byte(file, 0);
+len += 1;
+ram_release_page(block->idstr, offset);
+} else if (param->result == RES_COMPRESS) {
+len += save_page_header(pss, file, block,
+offset | RAM_SAVE_FLAG_COMPRESS_PAGE);
+len += qemu_put_qemu_file(file, param->file);
+} else {
+abort();
+}
+
+return len;
+}
+
+static void flush_compressed_data(RAMState *rs)
+{
int idx, len, thread_count;
if (!save_page_use_compression(rs)) {
@@ -1548,7 +1575,7 @@ static void flush_compressed_data(RAMState *rs)
qemu_mutex_lock(_param[idx].mutex);
if (!comp_param[idx].quit) {
CompressParam *param = _param[idx];
-len = qemu_put_qemu_file(ms->to_dst_file, param->file);
+len = send_queued_data(param);
compress_reset_result(param);
/*
@@ -1574,7 +1601,6 @@ static int compress_page_with_multi_thread(RAMBlock
*block, ram_addr_t offset)
{
int idx, thread_count, bytes_xmit = -1, pages = -1;
bool wait = migrate_compress_wait_thread();
-MigrationState *ms = migrate_get_current();
thread_count = migrate_compress_threads();
qemu_mutex_lock(_done_lock);
@@ -1584,7 +1610,7 @@ retry:
CompressParam *param = _param[idx];
qemu_mutex_lock(>mutex);
param->done = false;
-bytes_xmit = qemu_put_qemu_file(ms->to_dst_file, param->file);
+bytes_xmit = send_queued_data(param);
compress_reset_result(param);
set_compress_params(param, block, offset);
--
2.30.2
pgpJHCbcBHf1x.pgp
Description: OpenPGP digital signature
[PATCH 06/14] ram.c: Remove last ram.c dependency from the core compress code
Make compression interfaces take send_queued_data() as an argument.
Remove save_page_use_compression() from flush_compressed_data().
This removes the last ram.c dependency from the core compress code.
Signed-off-by: Lukas Straub
---
migration/ram.c | 27 +--
1 file changed, 17 insertions(+), 10 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index f55eb0e587..20428ccf42 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1556,13 +1556,10 @@ static int send_queued_data(CompressParam *param)
return len;
}
-static void flush_compressed_data(RAMState *rs)
+static void flush_compressed_data(int (send_queued_data(CompressParam *)))
{
int idx, thread_count;
-if (!save_page_use_compression(rs)) {
-return;
-}
thread_count = migrate_compress_threads();
qemu_mutex_lock(_done_lock);
@@ -1584,6 +1581,15 @@ static void flush_compressed_data(RAMState *rs)
}
}
+static void ram_flush_compressed_data(RAMState *rs)
+{
+if (!save_page_use_compression(rs)) {
+return;
+}
+
+flush_compressed_data(send_queued_data);
+}
+
static inline void set_compress_params(CompressParam *param, RAMBlock *block,
ram_addr_t offset)
{
@@ -1592,7 +1598,8 @@ static inline void set_compress_params(CompressParam
*param, RAMBlock *block,
param->trigger = true;
}
-static int compress_page_with_multi_thread(RAMBlock *block, ram_addr_t offset)
+static int compress_page_with_multi_thread(RAMBlock *block, ram_addr_t offset,
+int (send_queued_data(CompressParam *)))
{
int idx, thread_count, pages = -1;
bool wait = migrate_compress_wait_thread();
@@ -1673,7 +1680,7 @@ static int find_dirty_block(RAMState *rs,
PageSearchStatus *pss)
* Also If xbzrle is on, stop using the data compression at this
* point. In theory, xbzrle can do better than compression.
*/
-flush_compressed_data(rs);
+ram_flush_compressed_data(rs);
/* Hit the end of the list */
pss->block = QLIST_FIRST_RCU(_list.blocks);
@@ -2363,11 +2370,11 @@ static bool save_compress_page(RAMState *rs,
PageSearchStatus *pss,
* much CPU resource.
*/
if (block != pss->last_sent_block) {
-flush_compressed_data(rs);
+ram_flush_compressed_data(rs);
return false;
}
-if (compress_page_with_multi_thread(block, offset) > 0) {
+if (compress_page_with_multi_thread(block, offset, send_queued_data) > 0) {
return true;
}
@@ -3419,7 +3426,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
* page is sent in one chunk.
*/
if (migrate_postcopy_ram()) {
-flush_compressed_data(rs);
+ram_flush_compressed_data(rs);
}
/*
@@ -3512,7 +3519,7 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
}
qemu_mutex_unlock(>bitmap_mutex);
-flush_compressed_data(rs);
+ram_flush_compressed_data(rs);
ram_control_after_iterate(f, RAM_CONTROL_FINISH);
}
--
2.30.2
pgpSx0VD6dJGq.pgp
Description: OpenPGP digital signature
[PATCH 00/14] migration/ram.c: Refactor compress code
This series refactors the ram compress code. It first removes ram.c dependencies from the core compress code, then moves it out to its own file. Finally, on the migration destination side the initialisation and cleanup of compress threads is moved out of ram.c to migration.c. This allows using COLO with compress enabled. This series is based on the following series: https://lore.kernel.org/qemu-devel/af76761aa6978071c5b8e9b872b697db465a5520.1680457631.git.lukasstra...@web.de/T/#t Lukas Straub (14): ram.c: Let the compress threads return a CompressResult enum ram.c: Dont change param->block in the compress thread ram.c: Reset result after sending queued data ram.c: Do not call save_page_header() from compress threads ram.c: Call update_compress_thread_counts from compress_send_queued_data ram.c: Remove last ram.c dependency from the core compress code ram.c: Introduce whitespace (squash with next patch) ram.c: Move core compression code into its own file ram.c: Remove whitespace (squash with previous patch) ram.c: Move core decompression code into its own file ram compress: Assert that the file buffer matches the result ram.c: Remove unused include after moving out code ram-compress.c: Make target independent migration: Initialize and cleanup decompression in migration.c migration/meson.build| 5 +- migration/migration.c| 9 + migration/qemu-file.c| 11 + migration/qemu-file.h| 1 + migration/ram-compress.c | 483 ++ migration/ram-compress.h | 70 ++ migration/ram.c | 490 +++ 7 files changed, 615 insertions(+), 454 deletions(-) create mode 100644 migration/ram-compress.c create mode 100644 migration/ram-compress.h -- 2.30.2 pgpb6iASV2Rl1.pgp Description: OpenPGP digital signature
