* [PATCH v2 01/12] qemu-file: Don't increment qemu_file_transferred at qemu_file_fill_buffer
2023-10-25 9:11 [PATCH v2 00/12] migration: Yet another round of atomic counters Juan Quintela
@ 2023-10-25 9:11 ` Juan Quintela
2023-10-25 13:14 ` Fabiano Rosas
2023-10-25 9:11 ` [PATCH v2 02/12] qemu_file: Use a stat64 for qemu_file_transferred Juan Quintela
` (10 subsequent siblings)
11 siblings, 1 reply; 14+ messages in thread
From: Juan Quintela @ 2023-10-25 9:11 UTC (permalink / raw)
To: qemu-devel
Cc: Stefan Hajnoczi, Fam Zheng, Peter Xu, Hailiang Zhang,
Fabiano Rosas, Li Zhijian, qemu-block, Juan Quintela,
Leonardo Bras
We only call qemu_file_transferred_* on the sending side. Remove the
increment at qemu_file_fill_buffer() and add asserts to
qemu_file_transferred* functions.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration/qemu-file.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 3fb25148d1..6814c562e6 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -337,7 +337,6 @@ static ssize_t coroutine_mixed_fn qemu_fill_buffer(QEMUFile *f)
if (len > 0) {
f->buf_size += len;
- f->total_transferred += len;
} else if (len == 0) {
qemu_file_set_error_obj(f, -EIO, local_error);
} else {
@@ -627,6 +626,8 @@ uint64_t qemu_file_transferred_noflush(QEMUFile *f)
uint64_t ret = f->total_transferred;
int i;
+ g_assert(qemu_file_is_writable(f));
+
for (i = 0; i < f->iovcnt; i++) {
ret += f->iov[i].iov_len;
}
@@ -636,6 +637,7 @@ uint64_t qemu_file_transferred_noflush(QEMUFile *f)
uint64_t qemu_file_transferred(QEMUFile *f)
{
+ g_assert(qemu_file_is_writable(f));
qemu_fflush(f);
return f->total_transferred;
}
--
2.41.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 02/12] qemu_file: Use a stat64 for qemu_file_transferred
2023-10-25 9:11 [PATCH v2 00/12] migration: Yet another round of atomic counters Juan Quintela
2023-10-25 9:11 ` [PATCH v2 01/12] qemu-file: Don't increment qemu_file_transferred at qemu_file_fill_buffer Juan Quintela
@ 2023-10-25 9:11 ` Juan Quintela
2023-10-25 9:11 ` [PATCH v2 03/12] qemu_file: total_transferred is not used anymore Juan Quintela
` (9 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Juan Quintela @ 2023-10-25 9:11 UTC (permalink / raw)
To: qemu-devel
Cc: Stefan Hajnoczi, Fam Zheng, Peter Xu, Hailiang Zhang,
Fabiano Rosas, Li Zhijian, qemu-block, Juan Quintela,
Leonardo Bras
This way we can read it from any thread.
I checked that it gives the same value as the current one. We never
use two qemu_files at the same time.
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration/migration-stats.h | 4 ++++
migration/qemu-file.c | 5 +++--
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/migration/migration-stats.h b/migration/migration-stats.h
index 2358caad63..b7795e7914 100644
--- a/migration/migration-stats.h
+++ b/migration/migration-stats.h
@@ -81,6 +81,10 @@ typedef struct {
* Number of bytes sent during precopy stage.
*/
Stat64 precopy_bytes;
+ /*
+ * Number of bytes transferred with QEMUFile.
+ */
+ Stat64 qemu_file_transferred;
/*
* Amount of transferred data at the start of current cycle.
*/
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 6814c562e6..384985f534 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -283,6 +283,7 @@ void qemu_fflush(QEMUFile *f)
} else {
uint64_t size = iov_size(f->iov, f->iovcnt);
f->total_transferred += size;
+ stat64_add(&mig_stats.qemu_file_transferred, size);
}
qemu_iovec_release_ram(f);
@@ -623,7 +624,7 @@ int coroutine_mixed_fn qemu_get_byte(QEMUFile *f)
uint64_t qemu_file_transferred_noflush(QEMUFile *f)
{
- uint64_t ret = f->total_transferred;
+ uint64_t ret = stat64_get(&mig_stats.qemu_file_transferred);
int i;
g_assert(qemu_file_is_writable(f));
@@ -639,7 +640,7 @@ uint64_t qemu_file_transferred(QEMUFile *f)
{
g_assert(qemu_file_is_writable(f));
qemu_fflush(f);
- return f->total_transferred;
+ return stat64_get(&mig_stats.qemu_file_transferred);
}
void qemu_put_be16(QEMUFile *f, unsigned int v)
--
2.41.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 03/12] qemu_file: total_transferred is not used anymore
2023-10-25 9:11 [PATCH v2 00/12] migration: Yet another round of atomic counters Juan Quintela
2023-10-25 9:11 ` [PATCH v2 01/12] qemu-file: Don't increment qemu_file_transferred at qemu_file_fill_buffer Juan Quintela
2023-10-25 9:11 ` [PATCH v2 02/12] qemu_file: Use a stat64 for qemu_file_transferred Juan Quintela
@ 2023-10-25 9:11 ` Juan Quintela
2023-10-25 9:11 ` [PATCH v2 04/12] migration: Use the number of transferred bytes directly Juan Quintela
` (8 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Juan Quintela @ 2023-10-25 9:11 UTC (permalink / raw)
To: qemu-devel
Cc: Stefan Hajnoczi, Fam Zheng, Peter Xu, Hailiang Zhang,
Fabiano Rosas, Li Zhijian, qemu-block, Juan Quintela,
Leonardo Bras
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration/qemu-file.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 384985f534..641ab703cc 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -41,9 +41,6 @@ struct QEMUFile {
QIOChannel *ioc;
bool is_writable;
- /* The sum of bytes transferred on the wire */
- uint64_t total_transferred;
-
int buf_index;
int buf_size; /* 0 when writing */
uint8_t buf[IO_BUF_SIZE];
@@ -282,7 +279,6 @@ void qemu_fflush(QEMUFile *f)
qemu_file_set_error_obj(f, -EIO, local_error);
} else {
uint64_t size = iov_size(f->iov, f->iovcnt);
- f->total_transferred += size;
stat64_add(&mig_stats.qemu_file_transferred, size);
}
--
2.41.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 04/12] migration: Use the number of transferred bytes directly
2023-10-25 9:11 [PATCH v2 00/12] migration: Yet another round of atomic counters Juan Quintela
` (2 preceding siblings ...)
2023-10-25 9:11 ` [PATCH v2 03/12] qemu_file: total_transferred is not used anymore Juan Quintela
@ 2023-10-25 9:11 ` Juan Quintela
2023-10-25 9:11 ` [PATCH v2 05/12] qemu_file: Remove unused qemu_file_transferred() Juan Quintela
` (7 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Juan Quintela @ 2023-10-25 9:11 UTC (permalink / raw)
To: qemu-devel
Cc: Stefan Hajnoczi, Fam Zheng, Peter Xu, Hailiang Zhang,
Fabiano Rosas, Li Zhijian, qemu-block, Juan Quintela,
Leonardo Bras
We only use migration_transferred_bytes() to calculate the rate_limit,
for that we don't need to flush whatever is on the qemu_file buffer.
Remember that the buffer is really small (normal case is 32K if we use
iov's can be 64 * TARGET_PAGE_SIZE), so this is not relevant to
calculations.
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration/migration-stats.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/migration/migration-stats.c b/migration/migration-stats.c
index 4cc989d975..1d9197b4c3 100644
--- a/migration/migration-stats.c
+++ b/migration/migration-stats.c
@@ -63,7 +63,7 @@ uint64_t migration_transferred_bytes(QEMUFile *f)
{
uint64_t multifd = stat64_get(&mig_stats.multifd_bytes);
uint64_t rdma = stat64_get(&mig_stats.rdma_bytes);
- uint64_t qemu_file = qemu_file_transferred(f);
+ uint64_t qemu_file = stat64_get(&mig_stats.qemu_file_transferred);
trace_migration_transferred_bytes(qemu_file, multifd, rdma);
return qemu_file + multifd + rdma;
--
2.41.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 05/12] qemu_file: Remove unused qemu_file_transferred()
2023-10-25 9:11 [PATCH v2 00/12] migration: Yet another round of atomic counters Juan Quintela
` (3 preceding siblings ...)
2023-10-25 9:11 ` [PATCH v2 04/12] migration: Use the number of transferred bytes directly Juan Quintela
@ 2023-10-25 9:11 ` Juan Quintela
2023-10-25 9:11 ` [PATCH v2 06/12] qemu-file: Remove _noflush from qemu_file_transferred_noflush() Juan Quintela
` (6 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Juan Quintela @ 2023-10-25 9:11 UTC (permalink / raw)
To: qemu-devel
Cc: Stefan Hajnoczi, Fam Zheng, Peter Xu, Hailiang Zhang,
Fabiano Rosas, Li Zhijian, qemu-block, Juan Quintela,
Leonardo Bras
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration/qemu-file.h | 18 ------------------
migration/qemu-file.c | 7 -------
2 files changed, 25 deletions(-)
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index a29c37b0d0..8b71152754 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -33,24 +33,6 @@ QEMUFile *qemu_file_new_input(QIOChannel *ioc);
QEMUFile *qemu_file_new_output(QIOChannel *ioc);
int qemu_fclose(QEMUFile *f);
-/*
- * qemu_file_transferred:
- *
- * Report the total number of bytes transferred with
- * this file.
- *
- * For writable files, any pending buffers will be
- * flushed, so the reported value will be equal to
- * the number of bytes transferred on the wire.
- *
- * For readable files, the reported value will be
- * equal to the number of bytes transferred on the
- * wire.
- *
- * Returns: the total bytes transferred
- */
-uint64_t qemu_file_transferred(QEMUFile *f);
-
/*
* qemu_file_transferred_noflush:
*
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 641ab703cc..efa5f11033 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -632,13 +632,6 @@ uint64_t qemu_file_transferred_noflush(QEMUFile *f)
return ret;
}
-uint64_t qemu_file_transferred(QEMUFile *f)
-{
- g_assert(qemu_file_is_writable(f));
- qemu_fflush(f);
- return stat64_get(&mig_stats.qemu_file_transferred);
-}
-
void qemu_put_be16(QEMUFile *f, unsigned int v)
{
qemu_put_byte(f, v >> 8);
--
2.41.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 06/12] qemu-file: Remove _noflush from qemu_file_transferred_noflush()
2023-10-25 9:11 [PATCH v2 00/12] migration: Yet another round of atomic counters Juan Quintela
` (4 preceding siblings ...)
2023-10-25 9:11 ` [PATCH v2 05/12] qemu_file: Remove unused qemu_file_transferred() Juan Quintela
@ 2023-10-25 9:11 ` Juan Quintela
2023-10-25 9:11 ` [PATCH v2 07/12] migration: migration_transferred_bytes() don't need the QEMUFile Juan Quintela
` (5 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Juan Quintela @ 2023-10-25 9:11 UTC (permalink / raw)
To: qemu-devel
Cc: Stefan Hajnoczi, Fam Zheng, Peter Xu, Hailiang Zhang,
Fabiano Rosas, Li Zhijian, qemu-block, Juan Quintela,
Leonardo Bras
qemu_file_transferred() don't exist anymore, so we can reuse the name.
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
v2: Update the documentation (thanks fabiano)
---
migration/qemu-file.h | 9 ++++-----
migration/block.c | 4 ++--
migration/qemu-file.c | 2 +-
migration/savevm.c | 6 +++---
migration/vmstate.c | 4 ++--
5 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index 8b71152754..1b2f6b8d8f 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -34,15 +34,14 @@ QEMUFile *qemu_file_new_output(QIOChannel *ioc);
int qemu_fclose(QEMUFile *f);
/*
- * qemu_file_transferred_noflush:
+ * qemu_file_transferred:
*
- * As qemu_file_transferred except for writable files, where no flush
- * is performed and the reported amount will include the size of any
- * queued buffers, on top of the amount actually transferred.
+ * No flush is performed and the reported amount will include the size
+ * of any queued buffers, on top of the amount actually transferred.
*
* Returns: the total bytes transferred and queued
*/
-uint64_t qemu_file_transferred_noflush(QEMUFile *f);
+uint64_t qemu_file_transferred(QEMUFile *f);
/*
* put_buffer without copying the buffer.
diff --git a/migration/block.c b/migration/block.c
index b60698d6e2..47f11d0e4f 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -752,7 +752,7 @@ static int block_save_setup(QEMUFile *f, void *opaque)
static int block_save_iterate(QEMUFile *f, void *opaque)
{
int ret;
- uint64_t last_bytes = qemu_file_transferred_noflush(f);
+ uint64_t last_bytes = qemu_file_transferred(f);
trace_migration_block_save("iterate", block_mig_state.submitted,
block_mig_state.transferred);
@@ -804,7 +804,7 @@ static int block_save_iterate(QEMUFile *f, void *opaque)
}
qemu_put_be64(f, BLK_MIG_FLAG_EOS);
- uint64_t delta_bytes = qemu_file_transferred_noflush(f) - last_bytes;
+ uint64_t delta_bytes = qemu_file_transferred(f) - last_bytes;
return (delta_bytes > 0);
}
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index efa5f11033..0158db2a54 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -618,7 +618,7 @@ int coroutine_mixed_fn qemu_get_byte(QEMUFile *f)
return result;
}
-uint64_t qemu_file_transferred_noflush(QEMUFile *f)
+uint64_t qemu_file_transferred(QEMUFile *f)
{
uint64_t ret = stat64_get(&mig_stats.qemu_file_transferred);
int i;
diff --git a/migration/savevm.c b/migration/savevm.c
index 8622f229e5..9c90499609 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -927,9 +927,9 @@ static int vmstate_load(QEMUFile *f, SaveStateEntry *se)
static void vmstate_save_old_style(QEMUFile *f, SaveStateEntry *se,
JSONWriter *vmdesc)
{
- uint64_t old_offset = qemu_file_transferred_noflush(f);
+ uint64_t old_offset = qemu_file_transferred(f);
se->ops->save_state(f, se->opaque);
- uint64_t size = qemu_file_transferred_noflush(f) - old_offset;
+ uint64_t size = qemu_file_transferred(f) - old_offset;
if (vmdesc) {
json_writer_int64(vmdesc, "size", size);
@@ -3053,7 +3053,7 @@ bool save_snapshot(const char *name, bool overwrite, const char *vmstate,
goto the_end;
}
ret = qemu_savevm_state(f, errp);
- vm_state_size = qemu_file_transferred_noflush(f);
+ vm_state_size = qemu_file_transferred(f);
ret2 = qemu_fclose(f);
if (ret < 0) {
goto the_end;
diff --git a/migration/vmstate.c b/migration/vmstate.c
index 1cf9e45b85..16420fa9a3 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -386,7 +386,7 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
void *curr_elem = first_elem + size * i;
vmsd_desc_field_start(vmsd, vmdesc_loop, field, i, n_elems);
- old_offset = qemu_file_transferred_noflush(f);
+ old_offset = qemu_file_transferred(f);
if (field->flags & VMS_ARRAY_OF_POINTER) {
assert(curr_elem);
curr_elem = *(void **)curr_elem;
@@ -416,7 +416,7 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
return ret;
}
- written_bytes = qemu_file_transferred_noflush(f) - old_offset;
+ written_bytes = qemu_file_transferred(f) - old_offset;
vmsd_desc_field_end(vmsd, vmdesc_loop, field, written_bytes, i);
/* Compressed arrays only care about the first element */
--
2.41.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 07/12] migration: migration_transferred_bytes() don't need the QEMUFile
2023-10-25 9:11 [PATCH v2 00/12] migration: Yet another round of atomic counters Juan Quintela
` (5 preceding siblings ...)
2023-10-25 9:11 ` [PATCH v2 06/12] qemu-file: Remove _noflush from qemu_file_transferred_noflush() Juan Quintela
@ 2023-10-25 9:11 ` Juan Quintela
2023-10-25 9:11 ` [PATCH v2 08/12] migration: migration_rate_limit_reset() " Juan Quintela
` (4 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Juan Quintela @ 2023-10-25 9:11 UTC (permalink / raw)
To: qemu-devel
Cc: Stefan Hajnoczi, Fam Zheng, Peter Xu, Hailiang Zhang,
Fabiano Rosas, Li Zhijian, qemu-block, Juan Quintela,
Leonardo Bras, Philippe Mathieu-Daudé
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration/migration-stats.h | 4 +---
migration/migration-stats.c | 6 +++---
migration/migration.c | 6 +++---
3 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/migration/migration-stats.h b/migration/migration-stats.h
index b7795e7914..e3863bf9bb 100644
--- a/migration/migration-stats.h
+++ b/migration/migration-stats.h
@@ -137,11 +137,9 @@ void migration_rate_set(uint64_t new_rate);
/**
* migration_transferred_bytes: Return number of bytes transferred
*
- * @f: QEMUFile used for main migration channel
- *
* Returns how many bytes have we transferred since the beginning of
* the migration. It accounts for bytes sent through any migration
* channel, multifd, qemu_file, rdma, ....
*/
-uint64_t migration_transferred_bytes(QEMUFile *f);
+uint64_t migration_transferred_bytes(void);
#endif
diff --git a/migration/migration-stats.c b/migration/migration-stats.c
index 1d9197b4c3..4ae8c0c722 100644
--- a/migration/migration-stats.c
+++ b/migration/migration-stats.c
@@ -30,7 +30,7 @@ bool migration_rate_exceeded(QEMUFile *f)
}
uint64_t rate_limit_start = stat64_get(&mig_stats.rate_limit_start);
- uint64_t rate_limit_current = migration_transferred_bytes(f);
+ uint64_t rate_limit_current = migration_transferred_bytes();
uint64_t rate_limit_used = rate_limit_current - rate_limit_start;
if (rate_limit_max > 0 && rate_limit_used > rate_limit_max) {
@@ -56,10 +56,10 @@ void migration_rate_set(uint64_t limit)
void migration_rate_reset(QEMUFile *f)
{
- stat64_set(&mig_stats.rate_limit_start, migration_transferred_bytes(f));
+ stat64_set(&mig_stats.rate_limit_start, migration_transferred_bytes());
}
-uint64_t migration_transferred_bytes(QEMUFile *f)
+uint64_t migration_transferred_bytes(void)
{
uint64_t multifd = stat64_get(&mig_stats.multifd_bytes);
uint64_t rdma = stat64_get(&mig_stats.rdma_bytes);
diff --git a/migration/migration.c b/migration/migration.c
index 67547eb6a1..e199d2f50d 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2694,7 +2694,7 @@ static MigThrError migration_detect_error(MigrationState *s)
static void migration_calculate_complete(MigrationState *s)
{
- uint64_t bytes = migration_transferred_bytes(s->to_dst_file);
+ uint64_t bytes = migration_transferred_bytes();
int64_t end_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
int64_t transfer_time;
@@ -2720,7 +2720,7 @@ static void update_iteration_initial_status(MigrationState *s)
* wrong speed calculation.
*/
s->iteration_start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
- s->iteration_initial_bytes = migration_transferred_bytes(s->to_dst_file);
+ s->iteration_initial_bytes = migration_transferred_bytes();
s->iteration_initial_pages = ram_get_total_transferred_pages();
}
@@ -2739,7 +2739,7 @@ static void migration_update_counters(MigrationState *s,
}
switchover_bw = migrate_avail_switchover_bandwidth();
- current_bytes = migration_transferred_bytes(s->to_dst_file);
+ current_bytes = migration_transferred_bytes();
transferred = current_bytes - s->iteration_initial_bytes;
time_spent = current_time - s->iteration_start_time;
bandwidth = (double)transferred / time_spent;
--
2.41.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 08/12] migration: migration_rate_limit_reset() don't need the QEMUFile
2023-10-25 9:11 [PATCH v2 00/12] migration: Yet another round of atomic counters Juan Quintela
` (6 preceding siblings ...)
2023-10-25 9:11 ` [PATCH v2 07/12] migration: migration_transferred_bytes() don't need the QEMUFile Juan Quintela
@ 2023-10-25 9:11 ` Juan Quintela
2023-10-25 9:11 ` [PATCH v2 09/12] qemu-file: Simplify qemu_file_get_error() Juan Quintela
` (3 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Juan Quintela @ 2023-10-25 9:11 UTC (permalink / raw)
To: qemu-devel
Cc: Stefan Hajnoczi, Fam Zheng, Peter Xu, Hailiang Zhang,
Fabiano Rosas, Li Zhijian, qemu-block, Juan Quintela,
Leonardo Bras, Philippe Mathieu-Daudé
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration/migration-stats.h | 4 +---
migration/migration-stats.c | 2 +-
migration/migration.c | 2 +-
3 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/migration/migration-stats.h b/migration/migration-stats.h
index e3863bf9bb..68f3939188 100644
--- a/migration/migration-stats.h
+++ b/migration/migration-stats.h
@@ -120,10 +120,8 @@ uint64_t migration_rate_get(void);
* migration_rate_reset: Reset the rate limit counter.
*
* This is called when we know we start a new transfer cycle.
- *
- * @f: QEMUFile used for main migration channel
*/
-void migration_rate_reset(QEMUFile *f);
+void migration_rate_reset(void);
/**
* migration_rate_set: Set the maximum amount that can be transferred.
diff --git a/migration/migration-stats.c b/migration/migration-stats.c
index 4ae8c0c722..f690b98a03 100644
--- a/migration/migration-stats.c
+++ b/migration/migration-stats.c
@@ -54,7 +54,7 @@ void migration_rate_set(uint64_t limit)
stat64_set(&mig_stats.rate_limit_max, limit / XFER_LIMIT_RATIO);
}
-void migration_rate_reset(QEMUFile *f)
+void migration_rate_reset(void)
{
stat64_set(&mig_stats.rate_limit_start, migration_transferred_bytes());
}
diff --git a/migration/migration.c b/migration/migration.c
index e199d2f50d..bb62244288 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2775,7 +2775,7 @@ static void migration_update_counters(MigrationState *s,
stat64_get(&mig_stats.dirty_bytes_last_sync) / expected_bw_per_ms;
}
- migration_rate_reset(s->to_dst_file);
+ migration_rate_reset();
update_iteration_initial_status(s);
--
2.41.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 09/12] qemu-file: Simplify qemu_file_get_error()
2023-10-25 9:11 [PATCH v2 00/12] migration: Yet another round of atomic counters Juan Quintela
` (7 preceding siblings ...)
2023-10-25 9:11 ` [PATCH v2 08/12] migration: migration_rate_limit_reset() " Juan Quintela
@ 2023-10-25 9:11 ` Juan Quintela
2023-10-25 9:11 ` [PATCH v2 10/12] migration: Use migration_transferred_bytes() Juan Quintela
` (2 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Juan Quintela @ 2023-10-25 9:11 UTC (permalink / raw)
To: qemu-devel
Cc: Stefan Hajnoczi, Fam Zheng, Peter Xu, Hailiang Zhang,
Fabiano Rosas, Li Zhijian, qemu-block, Juan Quintela,
Leonardo Bras
If we pass a NULL error is the same that returning directly the value.
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration/qemu-file.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 0158db2a54..7e738743ce 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -204,7 +204,7 @@ void qemu_file_set_error_obj(QEMUFile *f, int ret, Error *err)
*/
int qemu_file_get_error(QEMUFile *f)
{
- return qemu_file_get_error_obj(f, NULL);
+ return f->last_error;
}
/*
--
2.41.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 10/12] migration: Use migration_transferred_bytes()
2023-10-25 9:11 [PATCH v2 00/12] migration: Yet another round of atomic counters Juan Quintela
` (8 preceding siblings ...)
2023-10-25 9:11 ` [PATCH v2 09/12] qemu-file: Simplify qemu_file_get_error() Juan Quintela
@ 2023-10-25 9:11 ` Juan Quintela
2023-10-25 9:11 ` [PATCH v2 11/12] migration: Remove transferred atomic counter Juan Quintela
2023-10-25 9:11 ` [PATCH v2 12/12] qemu-file: Make qemu_fflush() return errors Juan Quintela
11 siblings, 0 replies; 14+ messages in thread
From: Juan Quintela @ 2023-10-25 9:11 UTC (permalink / raw)
To: qemu-devel
Cc: Stefan Hajnoczi, Fam Zheng, Peter Xu, Hailiang Zhang,
Fabiano Rosas, Li Zhijian, qemu-block, Juan Quintela,
Leonardo Bras
There are only two differnces with the old value:
- the amount of QEMUFile that hasn't yet been flushed. It can be
discussed what is more exact, the new or the old one.
- the amount of transferred bytes that we forgot to account for (the
newer is better, i.e. exact).
Notice that this two values are used to:
a - present to the user
b - calculate the rate_limit
So a few KB here and there is not going to make a difference.
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration/migration.c | 2 +-
migration/ram.c | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index bb62244288..a6cde985a2 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -942,7 +942,7 @@ 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(&mig_stats.transferred);
+ info->ram->transferred = migration_transferred_bytes();
info->ram->total = ram_bytes_total();
info->ram->duplicate = stat64_get(&mig_stats.zero_pages);
/* legacy value. It is not used anymore */
diff --git a/migration/ram.c b/migration/ram.c
index 92769902bb..5ccf70333a 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -564,7 +564,7 @@ void mig_throttle_counter_reset(void)
rs->time_last_bitmap_sync = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
rs->num_dirty_pages_period = 0;
- rs->bytes_xfer_prev = stat64_get(&mig_stats.transferred);
+ rs->bytes_xfer_prev = migration_transferred_bytes();
}
/**
@@ -1030,7 +1030,7 @@ static void migration_trigger_throttle(RAMState *rs)
{
uint64_t threshold = migrate_throttle_trigger_threshold();
uint64_t bytes_xfer_period =
- stat64_get(&mig_stats.transferred) - rs->bytes_xfer_prev;
+ migration_transferred_bytes() - rs->bytes_xfer_prev;
uint64_t bytes_dirty_period = rs->num_dirty_pages_period * TARGET_PAGE_SIZE;
uint64_t bytes_dirty_threshold = bytes_xfer_period * threshold / 100;
@@ -1100,7 +1100,7 @@ static void migration_bitmap_sync(RAMState *rs, bool last_stage)
/* reset period counters */
rs->time_last_bitmap_sync = end_time;
rs->num_dirty_pages_period = 0;
- rs->bytes_xfer_prev = stat64_get(&mig_stats.transferred);
+ rs->bytes_xfer_prev = migration_transferred_bytes();
}
if (migrate_events()) {
uint64_t generation = stat64_get(&mig_stats.dirty_sync_count);
--
2.41.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 11/12] migration: Remove transferred atomic counter
2023-10-25 9:11 [PATCH v2 00/12] migration: Yet another round of atomic counters Juan Quintela
` (9 preceding siblings ...)
2023-10-25 9:11 ` [PATCH v2 10/12] migration: Use migration_transferred_bytes() Juan Quintela
@ 2023-10-25 9:11 ` Juan Quintela
2023-10-25 9:11 ` [PATCH v2 12/12] qemu-file: Make qemu_fflush() return errors Juan Quintela
11 siblings, 0 replies; 14+ messages in thread
From: Juan Quintela @ 2023-10-25 9:11 UTC (permalink / raw)
To: qemu-devel
Cc: Stefan Hajnoczi, Fam Zheng, Peter Xu, Hailiang Zhang,
Fabiano Rosas, Li Zhijian, qemu-block, Juan Quintela,
Leonardo Bras
After last commit, it is a write only variable.
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration/migration-stats.h | 4 ----
migration/multifd.c | 3 ---
migration/ram.c | 1 -
3 files changed, 8 deletions(-)
diff --git a/migration/migration-stats.h b/migration/migration-stats.h
index 68f3939188..05290ade76 100644
--- a/migration/migration-stats.h
+++ b/migration/migration-stats.h
@@ -97,10 +97,6 @@ typedef struct {
* Number of bytes sent through RDMA.
*/
Stat64 rdma_bytes;
- /*
- * Total number of bytes transferred.
- */
- Stat64 transferred;
/*
* Number of pages transferred that were full of zeros.
*/
diff --git a/migration/multifd.c b/migration/multifd.c
index e2a45c667a..ec58c58082 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -188,7 +188,6 @@ static int multifd_send_initial_packet(MultiFDSendParams *p, Error **errp)
return -1;
}
stat64_add(&mig_stats.multifd_bytes, size);
- stat64_add(&mig_stats.transferred, size);
return 0;
}
@@ -733,8 +732,6 @@ static void *multifd_send_thread(void *opaque)
stat64_add(&mig_stats.multifd_bytes,
p->next_packet_size + p->packet_len);
- stat64_add(&mig_stats.transferred,
- p->next_packet_size + p->packet_len);
p->next_packet_size = 0;
qemu_mutex_lock(&p->mutex);
p->pending_job--;
diff --git a/migration/ram.c b/migration/ram.c
index 5ccf70333a..6d2bf50614 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -455,7 +455,6 @@ void ram_transferred_add(uint64_t bytes)
} else {
stat64_add(&mig_stats.downtime_bytes, bytes);
}
- stat64_add(&mig_stats.transferred, bytes);
}
struct MigrationOps {
--
2.41.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 12/12] qemu-file: Make qemu_fflush() return errors
2023-10-25 9:11 [PATCH v2 00/12] migration: Yet another round of atomic counters Juan Quintela
` (10 preceding siblings ...)
2023-10-25 9:11 ` [PATCH v2 11/12] migration: Remove transferred atomic counter Juan Quintela
@ 2023-10-25 9:11 ` Juan Quintela
11 siblings, 0 replies; 14+ messages in thread
From: Juan Quintela @ 2023-10-25 9:11 UTC (permalink / raw)
To: qemu-devel
Cc: Stefan Hajnoczi, Fam Zheng, Peter Xu, Hailiang Zhang,
Fabiano Rosas, Li Zhijian, qemu-block, Juan Quintela,
Leonardo Bras, Philippe Mathieu-Daudé
This let us simplify code of this shape.
qemu_fflush(f);
int ret = qemu_file_get_error(f);
if (ret) {
return ret;
}
into:
int ret = qemu_fflush(f);
if (ret) {
return ret;
}
I updated all callers where there is any error check.
qemu_fclose() don't need to check for f->last_error because
qemu_fflush() returns it at the beggining of the function.
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
In v2: Now that we call always qemu_fflush() for all files, we can
simplify qemu_fclose()
---
migration/qemu-file.h | 2 +-
migration/colo.c | 11 +++--------
migration/migration.c | 7 +------
migration/qemu-file.c | 23 +++++++----------------
migration/ram.c | 22 +++++++---------------
migration/rdma.c | 4 +---
migration/savevm.c | 3 +--
7 files changed, 21 insertions(+), 51 deletions(-)
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index 1b2f6b8d8f..1774116f79 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -71,7 +71,7 @@ void qemu_file_set_error_obj(QEMUFile *f, int ret, Error *err);
void qemu_file_set_error(QEMUFile *f, int ret);
int qemu_file_shutdown(QEMUFile *f);
QEMUFile *qemu_file_get_return_path(QEMUFile *f);
-void qemu_fflush(QEMUFile *f);
+int qemu_fflush(QEMUFile *f);
void qemu_file_set_blocking(QEMUFile *f, bool block);
int qemu_file_get_to_fd(QEMUFile *f, int fd, size_t size);
diff --git a/migration/colo.c b/migration/colo.c
index 72f4f7b37e..4447e34914 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -314,9 +314,7 @@ static void colo_send_message(QEMUFile *f, COLOMessage msg,
return;
}
qemu_put_be32(f, msg);
- qemu_fflush(f);
-
- ret = qemu_file_get_error(f);
+ ret = qemu_fflush(f);
if (ret < 0) {
error_setg_errno(errp, -ret, "Can't send COLO message");
}
@@ -335,9 +333,7 @@ static void colo_send_message_value(QEMUFile *f, COLOMessage msg,
return;
}
qemu_put_be64(f, value);
- qemu_fflush(f);
-
- ret = qemu_file_get_error(f);
+ ret = qemu_fflush(f);
if (ret < 0) {
error_setg_errno(errp, -ret, "Failed to send value for message:%s",
COLOMessage_str(msg));
@@ -483,8 +479,7 @@ static int colo_do_checkpoint_transaction(MigrationState *s,
}
qemu_put_buffer(s->to_dst_file, bioc->data, bioc->usage);
- qemu_fflush(s->to_dst_file);
- ret = qemu_file_get_error(s->to_dst_file);
+ ret = qemu_fflush(s->to_dst_file);
if (ret < 0) {
goto out;
}
diff --git a/migration/migration.c b/migration/migration.c
index a6cde985a2..ce12fca520 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -305,12 +305,7 @@ static int migrate_send_rp_message(MigrationIncomingState *mis,
qemu_put_be16(mis->to_src_file, (unsigned int)message_type);
qemu_put_be16(mis->to_src_file, len);
qemu_put_buffer(mis->to_src_file, data, len);
- qemu_fflush(mis->to_src_file);
-
- /* It's possible that qemu file got error during sending */
- ret = qemu_file_get_error(mis->to_src_file);
-
- return ret;
+ return qemu_fflush(mis->to_src_file);
}
/* Request one page from the source VM at the given start address.
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 7e738743ce..d64500310d 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -262,14 +262,14 @@ static void qemu_iovec_release_ram(QEMUFile *f)
* This will flush all pending data. If data was only partially flushed, it
* will set an error state.
*/
-void qemu_fflush(QEMUFile *f)
+int qemu_fflush(QEMUFile *f)
{
if (!qemu_file_is_writable(f)) {
- return;
+ return f->last_error;
}
- if (qemu_file_get_error(f)) {
- return;
+ if (f->last_error) {
+ return f->last_error;
}
if (f->iovcnt > 0) {
Error *local_error = NULL;
@@ -287,6 +287,7 @@ void qemu_fflush(QEMUFile *f)
f->buf_index = 0;
f->iovcnt = 0;
+ return f->last_error;
}
/*
@@ -353,22 +354,12 @@ static ssize_t coroutine_mixed_fn qemu_fill_buffer(QEMUFile *f)
*/
int qemu_fclose(QEMUFile *f)
{
- int ret, ret2;
- qemu_fflush(f);
- ret = qemu_file_get_error(f);
-
- ret2 = qio_channel_close(f->ioc, NULL);
+ int ret = qemu_fflush(f);
+ int ret2 = qio_channel_close(f->ioc, NULL);
if (ret >= 0) {
ret = ret2;
}
g_clear_pointer(&f->ioc, object_unref);
-
- /* If any error was spotted before closing, we should report it
- * instead of the close() return value.
- */
- if (f->last_error) {
- ret = f->last_error;
- }
error_free(f->last_error_obj);
g_free(f);
trace_qemu_file_fclose();
diff --git a/migration/ram.c b/migration/ram.c
index 6d2bf50614..3a1d9882ce 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -305,17 +305,15 @@ int64_t ramblock_recv_bitmap_send(QEMUFile *file,
qemu_put_be64(file, size);
qemu_put_buffer(file, (const uint8_t *)le_bitmap, size);
+ g_free(le_bitmap);
/*
* Mark as an end, in case the middle part is screwed up due to
* some "mysterious" reason.
*/
qemu_put_be64(file, RAMBLOCK_RECV_BITMAP_ENDING);
- qemu_fflush(file);
-
- g_free(le_bitmap);
-
- if (qemu_file_get_error(file)) {
- return qemu_file_get_error(file);
+ int ret = qemu_fflush(file);
+ if (ret) {
+ return ret;
}
return size + sizeof(size);
@@ -3055,9 +3053,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
}
qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
- qemu_fflush(f);
-
- return 0;
+ return qemu_fflush(f);
}
/**
@@ -3176,10 +3172,8 @@ out:
}
qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
- qemu_fflush(f);
ram_transferred_add(8);
-
- ret = qemu_file_get_error(f);
+ ret = qemu_fflush(f);
}
if (ret < 0) {
return ret;
@@ -3256,9 +3250,7 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
qemu_put_be64(f, RAM_SAVE_FLAG_MULTIFD_FLUSH);
}
qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
- qemu_fflush(f);
-
- return 0;
+ return qemu_fflush(f);
}
static void ram_state_pending_estimate(void *opaque, uint64_t *must_precopy,
diff --git a/migration/rdma.c b/migration/rdma.c
index 2a1852ec7f..0884333190 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -3849,9 +3849,7 @@ int rdma_registration_start(QEMUFile *f, uint64_t flags)
trace_rdma_registration_start(flags);
qemu_put_be64(f, RAM_SAVE_FLAG_HOOK);
- qemu_fflush(f);
-
- return 0;
+ return qemu_fflush(f);
}
/*
diff --git a/migration/savevm.c b/migration/savevm.c
index 9c90499609..b19d625259 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1583,8 +1583,7 @@ int qemu_savevm_state_complete_precopy(QEMUFile *f, bool iterable_only,
}
flush:
- qemu_fflush(f);
- return 0;
+ return qemu_fflush(f);
}
/* Give an estimate of the amount left to be transferred,
--
2.41.0
^ permalink raw reply related [flat|nested] 14+ messages in thread