I am currently focusing in kernel development, so I will probably not be
of much help in reviewing general Live Migration changes.
For above reason I am removing my Reviewer status from Migration and RDMA
Migration.
Signed-off-by: Leonardo Bras
---
MAINTAINERS | 2 --
1 file changed, 2
On Mon, Jul 10, 2023 at 3:16 PM Michael S. Tsirkin wrote:
>
> On Mon, Jul 10, 2023 at 02:49:05PM -0300, Leonardo Bras Soares Passos wrote:
> > On Thu, Jul 6, 2023 at 5:00 PM Michael S. Tsirkin wrote:
> > >
> > > On Thu, Jul 06, 2023 at 03:02:07PM -0400, Peter Xu
n Thu, Jul 06, 2023 at 03:07:40PM -0300, Leonardo Bras Soares Passos
> > > > wrote:
> > > > > > I asked the same question, and I still keep confused: whether
> > > > > > there's a
> > > > > > first bad commit? Starting fro
On Thu, Jul 6, 2023 at 3:24 PM Peter Xu wrote:
>
> On Thu, Jul 06, 2023 at 03:07:40PM -0300, Leonardo Bras Soares Passos wrote:
> > > I asked the same question, and I still keep confused: whether there's a
> > > first bad commit? Starting from when it fails?
>
On Thu, Jul 6, 2023 at 3:24 PM Peter Xu wrote:
>
> On Thu, Jul 06, 2023 at 03:07:40PM -0300, Leonardo Bras Soares Passos wrote:
> > > I asked the same question, and I still keep confused: whether there's a
> > > first bad commit? Starting from when it fails?
>
On Thu, Jul 6, 2023 at 11:35 AM Peter Xu wrote:
>
> On Thu, Jul 06, 2023 at 01:55:47AM -0300, Leonardo Bras wrote:
> > When trying to migrate a machine type pc-q35-6.0 or lower, with this
> > cmdline options,
> >
> > -device
> > driver=pcie-root-port,port=1
On Thu, Jul 6, 2023 at 4:37 AM Juan Quintela wrote:
>
> Leonardo Bras wrote:
> > When trying to migrate a machine type pc-q35-6.0 or lower, with this
> > cmdline options,
> >
> > -device
> > driver=pcie-root-port,port=18,chassis=19,id=pcie-root-port18,bus
sk
field.
So, clear PCI_EXP_SLTSTA_PDS bit on cmask, so the fake incompatibility on
get_pci_config_device() does not abort the migration.
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2215819
Signed-off-by: Leonardo Bras
---
hw/pci/pcie.c | 4
1 file changed, 4 insertions(+)
diff
On Wed, Jul 5, 2023 at 3:40 AM Leonardo Bras Soares Passos
wrote:
>
> On Tue, Jul 4, 2023 at 3:43 AM Michael S. Tsirkin wrote:
> >
> > On Tue, Jul 04, 2023 at 03:20:36AM -0300, Leonardo Brás wrote:
> > > Hello Peter and Michael, I have a few updates on this:
> >
On Tue, Jul 4, 2023 at 3:43 AM Michael S. Tsirkin wrote:
>
> On Tue, Jul 04, 2023 at 03:20:36AM -0300, Leonardo Brás wrote:
> > Hello Peter and Michael, I have a few updates on this:
> >
> > On Mon, 2023-07-03 at 02:20 -0300, Leonardo Brás wrote:
> > > He
Hello Peter and Michael, I have a few updates on this:
On Mon, 2023-07-03 at 02:20 -0300, Leonardo Brás wrote:
> Hello Peter and Michael, thanks for reviewing!
>
>
> On Thu, 2023-06-29 at 16:56 -0400, Peter Xu wrote:
> > On Thu, Jun 29, 2023 at 04:06:53PM -0400, Michae
> > > > from commit 17858a1695 ("hw/acpi/ich9: Set ACPI PCI hot-plug as
> > > > > default on
> > > > > Q35").
> > > > >
> > > > > On Thu, Jun 29, 2023 at 06:05:00AM -0300, Leonardo Bras wrote:
> > > > > >
k, so the fake incompatibility on
get_pci_config_device() does not abort the migration.
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2215819
Signed-off-by: Leonardo Bras
---
hw/pci/pcie.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index b8c24cf45
On Fri, May 26, 2023 at 5:24 AM Juan Quintela wrote:
>
> Leonardo Brás wrote:
> > On Mon, 2023-05-15 at 21:57 +0200, Juan Quintela wrote:
> >> In the past, we had to put the in the main thread all the operations
> >> related with sizes due to qemu_file not beeing
On Fri, May 26, 2023 at 5:21 AM Juan Quintela wrote:
>
> Leonardo Brás wrote:
> > On Mon, 2023-05-15 at 21:57 +0200, Juan Quintela wrote:
> >> When we sent a page through QEMUFile hooks (RDMA) there are three
> >> posiblities:
> >> - We are not using RD
On Fri, May 26, 2023 at 5:18 AM Juan Quintela wrote:
>
> Leonardo Brás wrote:
> > On Mon, 2023-05-15 at 21:57 +0200, Juan Quintela wrote:
> >> Since previous commit, we calculate how much data we have send with
> >> migration_transferred_bytes() so no nee
On Fri, May 26, 2023 at 5:17 AM Juan Quintela wrote:
>
> Leonardo Brás wrote:
> > On Mon, 2023-05-15 at 21:57 +0200, Juan Quintela wrote:
> >> Signed-off-by: Juan Quintela
> >> Reviewed-by: Cédric Le Goater
> >> ---
> >> migration/migratio
On Fri, May 26, 2023 at 5:09 AM Juan Quintela wrote:
>
> Leonardo Brás wrote:
> > On Mon, 2023-05-15 at 21:56 +0200, Juan Quintela wrote:
> >> That is the moment we know we have transferred something.
> >>
> >> Signed-off-by: Juan Quint
On Fri, May 26, 2023 at 5:07 AM Juan Quintela wrote:
>
> Leonardo Brás wrote:
> > On Mon, 2023-05-15 at 21:56 +0200, Juan Quintela wrote:
> >> It is a time that needs to be cleaned each time cancel migration.
> >> Once there create migration_time_since() to calcul
On Fri, May 26, 2023 at 5:04 AM Juan Quintela wrote:
>
> Leonardo Brás wrote:
> > On Mon, 2023-05-15 at 21:56 +0200, Juan Quintela wrote:
> >> We forget several places to add to trasferred amount of data. With
> >> this fixes I get:
> >>
> >
if (ret != 0) {
> return -1;
> }
> +stat64_add(&mig_stats.multifd_bytes, size);
> +stat64_add(&mig_stats.transferred, size);
> return 0;
> }
Humm, those are atomic ops, right?
You think we could have 'multifd_bytes' and 'transferred'
use the RDMA
> - * protocol is completely asynchronous. We do not yet know
> - * whether an identified chunk is zero or not because we're
> - * waiting for other pages to potentially be merged with
> - * the current chunk. So, we have to call qemu_update_position()
> - * later on when the actual write occurs.
> - */
> -if (bytes_sent) {
> -*bytes_sent = 1;
> -}
> -
> /*
> * Drain the Completion Queue if possible, but do not block,
> * just poll.
Oh, so this one complements 13/16.
Since it doesn't do imaginary transfers anymore, there is no need to use
bytes_sent pointer to keep track of them anymore.
Other than the pages_sent above that I couldn't understand:
Reviewed-by: Leonardo Bras
er(QEMUFile *f)
> return len;
> }
>
> -void qemu_file_credit_transfer(QEMUFile *f, size_t size)
> -{
> -f->total_transferred += size;
> -}
> -
> /** Closes the file
> *
> * Returns negative error value if any error happened on previous operations
> or
Reviewed-by: Leonardo Bras
; RAMBlock *block,
> }
>
> if (bytes_xmit) {
> -ram_transferred_add(bytes_xmit);
> *pages = 1;
> }
>
Reviewed-by: Leonardo Bras
7;t belong to the current chunk,
> * an actual RDMA write will occur and a new chunk will be formed.
> */
> -ret = qemu_rdma_write(f, rdma, block_offset, offset, size);
> +ret = qemu_rdma_write(rdma, block_offset, offset, size);
> if (ret < 0) {
> error_report("rdma migration: write error! %d", ret);
> goto err;
> @@ -3927,7 +3926,7 @@ static int qemu_rdma_registration_stop(QEMUFile *f,
> CHECK_ERROR_STATE();
>
> qemu_fflush(f);
> -ret = qemu_rdma_drain_cq(f, rdma);
> +ret = qemu_rdma_drain_cq(rdma);
>
> if (ret < 0) {
> goto err;
Reviewed-by: Leonardo Bras
page() which increment zero-pages.
It also calls ram_save_page() which increments normal-pages.
Reviewed-by: Leonardo Bras
postcopy_preempt_enabled(bool value) "%d"
>
> # migration-stats
> -migration_transferred_bytes(uint64_t qemu_file, uint64_t multifd) "qemu_file
> %" PRIu64 " multifd %" PRIu64
> +migration_transferred_bytes(uint64_t qemu_file, uint64_t multifd, uint64_t
> rdma) "qemu_file %" PRIu64 " multifd %" PRIu64 " RDMA %" PRIu64
>
> # channel.c
> migration_set_incoming_channel(void *ioc, const char *ioctype) "ioc=%p
> ioctype=%s"
Reviewed-by: Leonardo Bras
ze_t ram_control_save_page(QEMUFile *f, ram_addr_t
> block_offset,
> if (f->hooks && f->hooks->save_page) {
> int ret = f->hooks->save_page(f, block_offset,
>offset, size, bytes_sent);
> -if (ret != RAM_SAVE_CONTROL_NOT_SUPP) {
> -migration_rate_account(size);
> -}
>
> if (ret != RAM_SAVE_CONTROL_DELAYED &&
> ret != RAM_SAVE_CONTROL_NOT_SUPP) {
I reviewed this one together with 8/16.
It makes sense for me to squash them together, but anyway:
Reviewed-by: Leonardo Bras
nk it would look nicer if squashed with 9/16, though. It would make it more
clear this is being added to replace migration_rate_account() strategy.
What do you think?
Other than that,
Reviewed-by: Leonardo Bras
>
> if (rate_limit_max == RATE_LIMIT_MAX) {
> @@ -58,
quot; multifd %" PRIu64
> +
> # channel.c
> migration_set_incoming_channel(void *ioc, const char *ioctype) "ioc=%p
> ioctype=%s"
> migration_set_outgoing_channel(void *ioc, const char *ioctype, const char
> *hostname, void *err) "ioc=%p ioctype=%s hostname=%s err=%p"
LGTM
Reviewed-by: Leonardo Bras
on_total_bytes(s);
> +current_bytes = migration_transferred_bytes(s->to_dst_file);
> transferred = current_bytes - s->iteration_initial_bytes;
> time_spent = current_time - s->iteration_start_time;
> bandwidth = (double)transferred / time_spent;
LGTM
Reviewed-by: Leonardo Bras
On Mon, 2023-05-15 at 21:56 +0200, Juan Quintela wrote:
> These way we can make them atomic and use this functions from any
> place. I also moved all functions that use rate_limit to
> migration-stats.
>
> Functions got renamed, they are not qemu_file anymore.
>
> qemu_file_rate_limit -> migrati
(f, 1);
> }
>
If we are counting transferred data at fflush, it makes sense to increase rate-
limit accounting at the same place. It may be less granular, but is more
efficient.
FWIW:
Reviewed-by: Leonardo Bras
set(&stats->setup_time, now - since);
> +}
IIUC this calculates a time delta and saves on stats->setup_time, is that right?
It took me some time to understand that, since the function name is
migration_time_since(), which seems more generic.
Would not be more intuitive t
n()) {
> qemu_put_be64(f, RAM_SAVE_FLAG_MULTIFD_FLUSH);
> +size += 8;
sizeof(uint64_t) here is probably too much.
Maybe, it would be nice to have qemu_put_* to return the value, and in this
case:
size += qemu_put_be64(...)
What do you think?
Anyway,
Reviewed-by: Leonardo Br
Suggest "RATE_LIMIT_MAX_NONE".
> >
> > Then even better
> >
> > RATE_LIMIT_DISABLED?
>
> I'd vote for RATE_LIMIT_DISABLED.
Me too.
>
> > RATE_LIMIT_NONE?
> >
> > Using MAX and NONE at the same time looks strange.
>
> Cheers,
>
> C.
>
Reviewed-by: Leonardo Bras
On Wed, May 3, 2023 at 6:49 AM Jonathan Cameron
wrote:
>
> On Tue, 2 May 2023 21:27:02 -0300
> Leonardo Bras wrote:
>
> > Since it's implementation on v8.0.0-rc0, having the PCI_ERR_UNCOR_MASK
> > set for machine types < 8.0 will cause migration to fail if the ta
gt; @@ -113,7 +113,7 @@ int pcie_aer_init(PCIDevice *dev, uint8_t cap_ver,
> > > uint16_t offset,
> > > pci_set_long(dev->w1cmask + offset + PCI_ERR_UNCOR_STATUS,
> > > PCI_ERR_UNC_SUPPORTED);
> > > pci_set_long(dev->config + offse
e type <= 7.2.
Fixes: 010746ae1d ("hw/pci/aer: Implement PCI_ERR_UNCOR_MASK register")
Suggested-by: Michael S. Tsirkin
Signed-off-by: Leonardo Bras
---
include/hw/pci/pci.h | 2 ++
hw/core/machine.c| 1 +
hw/pci/pci.c | 2 ++
hw/pci/pcie_aer.c| 11 +++
4
there is not enough testing/support nor any reported users for
this scenario, we should disable this combination before it may cause any
problems for users.
Suggested-by: Dr. David Alan Gilbert
Signed-off-by: Leonardo Bras
Acked-by: Peter Xu
Reviewed-by: Dr. David Alan Gilbert
---
Changes sinc
On Fri, Apr 21, 2023 at 12:23 PM Peter Xu wrote:
>
> On Wed, Apr 19, 2023 at 06:29:57PM +0200, Juan Quintela wrote:
> > Now that David has stepped down with Migration maintainership,
> > Leonardo and Peter has volunteer to review the migration patches.
> > This wa
r 30, 2023 at 03:20:14PM +0100, Daniel P. Berrangé wrote:
> > > > > On Mon, Mar 27, 2023 at 01:15:18PM -0300, Leonardo Bras wrote:
> > > > > > Since the introduction of multifd, it's possible to perform a
> > > > > > multifd
> > > > >
s not enough testing/support nor any reported users for
this scenario, we should disable this combination before it may cause any
problems for users.
Suggested-by: Dr. David Alan Gilbert
Signed-off-by: Leonardo Bras
---
migration/migration.c | 5 +
1 file changed, 5 insertions(+)
diff --
On Fri, 2023-02-10 at 03:36 -0300, Leonardo Bras wrote:
> Currently running migration_incoming_state_destroy() without first running
> multifd_load_cleanup() will cause a yank error:
>
> qemu-system-x86_64: ../util/yank.c:107: yank_unregister_instance:
> Assertion `QLIST_EMPTY(
Before assigning "p->quit = true" for every multifd channel,
multifd_load_cleanup() will call multifd_recv_terminate_threads() which
already does the same assignment, while protected by a mutex.
So there is no point doing the same assignment again.
Signed-off-by: Leonardo Bras
-
migration
is usually just ran at most a few times, and after it succeeds there is
not much to be done before exiting the process.
Yet still, it should not hurt performance to join all of them.
Signed-off-by: Leonardo Bras
---
migration/multifd.c | 3 ++-
1 file changed, 2 insertions(+), 1
n value to decide on setting autostart = false, which will never
happen.
In order to simplify the codebase, change multifd_load_cleanup() signature
to 'void multifd_load_cleanup(void)', and for every usage remove error
handling or decision made based on return value != 0.
Signed-off-by:
up() to make sure nothing else is received
before dirty_bitmap_mig_before_vm_start().
Signed-off-by: Leonardo Bras
---
migration/multifd.h | 1 +
migration/migration.c | 4 +++-
migration/multifd.c | 7 +++
3 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/migration/multifd.h
Sent with the incorrect credentials. Sorry about the noise.
Will resend them correctly.
On Fri, 2023-02-10 at 03:31 -0300, Leonardo Bras wrote:
> Since it's introduction in commit f986c3d256 ("migration: Create multifd
> migration threads"), multifd_load_cleanup() nev
n value to decide on setting autostart = false, which will never
happen.
In order to simplify the codebase, change multifd_load_cleanup() signature
to 'void multifd_load_cleanup(void)', and for every usage remove error
handling or decision made based on return value != 0.
Signed-off-by:
up() to make sure nothing else is received
before dirty_bitmap_mig_before_vm_start().
Signed-off-by: Leonardo Bras
---
migration/multifd.h | 1 +
migration/migration.c | 4 +++-
migration/multifd.c | 7 +++
3 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/migration/multifd.h
migration
is usually just ran at most a few times, and after it succeeds there is
not much to be done before exiting the process.
Yet still, it should not hurt performance to join all of them.
Signed-off-by: Leonardo Bras
---
migration/multifd.c | 3 ++-
1 file changed, 2 insertions(+), 1
Before assigning "p->quit = true" for every multifd channel,
multifd_load_cleanup() will call multifd_recv_terminate_threads() which
already does the same assignment, while protected by a mutex.
So there is no point doing the same assignment again.
Signed-off-by: Leonardo Bras
-
On Tue, 2022-11-29 at 15:50 -0500, Peter Xu wrote:
> On Tue, Nov 29, 2022 at 05:28:26PM -0300, Leonardo Bras Soares Passos wrote:
> > Hello Peter,
>
> Leo,
>
> >
> > On Thu, Nov 24, 2022 at 1:04 PM Peter Xu wrote:
> > >
> > > On Wed, Nov 0
t; info->has_total_time = true;
> info->total_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME) -
> s->start_time;
> +}
> +
> +if (migrate_show_downtime(s)) {
> +info->has_downtime = true;
> + info->downtime = s->downtime;
> +} else {
> info->has_expected_downtime = true;
> info->expected_downtime = s->expected_downtime;
> }
> --
> 2.37.3
>
FWIW:
Reviewed-by: Leonardo Bras
Hello Peter,
On Thu, Nov 24, 2022 at 1:04 PM Peter Xu wrote:
>
> On Wed, Nov 09, 2022 at 02:56:29AM -0300, Leonardo Bras wrote:
> > diff --git a/migration/savevm.c b/migration/savevm.c
> > index a0cdb714f7..250caff7f4 100644
> > --- a/migration/savevm.c
>
On Thu, Nov 10, 2022 at 10:48 AM Juan Quintela wrote:
>
> Leonardo Bras wrote:
> D> When multifd and postcopy-ram capabilities are enabled, if a
> > migrate-start-postcopy is attempted, the migration will finish sending the
> > memory pages and then crash with the follo
On Wed, Nov 9, 2022 at 10:31 AM Dr. David Alan Gilbert
wrote:
>
> * Leonardo Bras (leob...@redhat.com) wrote:
> > When multifd and postcopy-ram capabilities are enabled, if a
> > migrate-start-postcopy is attempted, the migration will finish sending the
> > memory pages
ead()
before MIGRATION_YANK_INSTANCE is unregistered.
Fixes: b5eea99ec2 ("migration: Add yank feature")
Reported-by: Li Xiaohui
Signed-off-by: Leonardo Bras
---
migration/migration.h | 1 +
migration/migration.c | 18 +-
migration/savevm.c| 2 ++
3 files changed, 16 inse
On Tue, 2022-10-25 at 12:07 +0200, Juan Quintela wrote:
> Daniel P. Berrangé wrote:
> > On Tue, Oct 25, 2022 at 01:47:31AM -0300, Leonardo Bras wrote:
> > > Zero-copy multifd migration sends both the header and the memory pages in
> > > a
> > > single sy
On Tue, 2022-10-25 at 11:51 +0200, Juan Quintela wrote:
> Leonardo Bras wrote:
> > When zero-copy-send is enabled, each loop iteration of the
> > multifd_send_thread will calls for qio_channel_write_*() twice: The first
> > one for sending the header without zero-copy flag a
to happen while the most recent ones
are ongoing, instead of waiting for everything to finish before sending
more.
It all works fine in my tests, but maybe I missed some cornercase.
Please provide any feedback you find fit.
Thank you all!
Best regards,
Leo
Leonardo Bras (4):
migration/multifd
Move flushing code from multifd_send_sync_main() to a new helper, and call
it in multifd_send_sync_main().
Signed-off-by: Leonardo Bras
---
migration/multifd.c | 30 +++---
1 file changed, 19 insertions(+), 11 deletions(-)
diff --git a/migration/multifd.c b/migration
hput while consuming less cpu time.
Signed-off-by: Leonardo Bras
---
migration/multifd.c | 16 +++-
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/migration/multifd.c b/migration/multifd.c
index c5d1f911a4..fe9df460f6 100644
--- a/migration/multifd.c
+++ b/migration
he maximum number of pending writes remaining before the function
returns. Also, implement this change in qio_channel_socket_flush().
Change current calls of qio_channel_flush() so (max_pending == 0), and the
flush-all behavior is maintained.
Signed-off-by: Leonardo Bras
---
include/io/cha
which element of the array will be used as a header.
Suggested-by: Juan Quintela
Signed-off-by: Leonardo Bras
---
migration/multifd.h | 5 -
migration/multifd.c | 52 -
2 files changed, 32 insertions(+), 25 deletions(-)
diff --git a/migration/multif
On Tue, 2022-08-02 at 08:39 +0200, Juan Quintela wrote:
> This implements the zero page dection and handling.
>
> Signed-off-by: Juan Quintela
>
> ---
>
> Add comment for offset (dave)
> Use local variables for offset/block to have shorter lines
> ---
> migration/multifd.h | 5 +
> migrat
On Tue, 2022-08-02 at 08:39 +0200, Juan Quintela wrote:
> This patch adds counters and similar. Logic will be added on the
> following patch.
>
> Signed-off-by: Juan Quintela
>
> ---
>
> Added counters for duplicated/non duplicated pages.
> Removed reviewed by from David.
> Add total_zero_page
_after_iterate(f, RAM_CONTROL_SETUP);
>
> -(*rsp)->ram_save_target_page = ram_save_target_page_legacy;
> +if (migrate_use_multifd() && !migrate_use_main_zero_page()) {
> +(*rsp)->ram_save_target_page = ram_save_target_page_multifd;
> +} else {
> +(*rsp)->ram_save_target_page = ram_save_target_page_legacy;
> +}
> +
> ret = multifd_send_sync_main(f);
> if (ret < 0) {
> return ret;
The rest LGTM.
FWIW:
Reviewed-by: Leonardo Bras
On Fri, Aug 19, 2022 at 8:32 AM Juan Quintela wrote:
>
> Leonardo Brás wrote:
> > On Tue, 2022-08-02 at 08:39 +0200, Juan Quintela wrote:
> >> We do the send_prepare() and the fill of the head packet without the
> >> mutex held. It will help a lot for compressio
On Fri, Aug 19, 2022 at 7:03 AM Juan Quintela wrote:
>
> Leonardo Brás wrote:
> > On Tue, 2022-08-02 at 08:39 +0200, Juan Quintela wrote:
> >> Use of flags with respect to locking was incensistant. For the
> >> sending side:
> >> - it was set to 0
On Fri, Aug 19, 2022 at 6:52 AM Juan Quintela wrote:
>
> Leonardo Brás wrote:
> > On Tue, 2022-08-02 at 08:39 +0200, Juan Quintela wrote:
> >> We are going to create a new function for multifd latest in the series.
> >>
> >> Signed-off-by: Juan Quintela
&
On Thu, Aug 18, 2022 at 5:05 PM Peter Xu wrote:
>
> On Tue, Aug 16, 2022 at 06:12:50AM -0400, Emanuele Giuseppe Esposito wrote:
> > +static void kvm_memory_region_node_add(KVMMemoryListener *kml,
> > + struct kvm_userspace_memory_region
> > *mem)
> > +{
> > +
> if (!migrate_release_ram() || !migration_in_postcopy()) {
> return;
LGTM. FWIW:
Reviewed-by: Leonardo Bras
On Tue, 2022-08-02 at 08:39 +0200, Juan Quintela wrote:
> We have to enable it by default until we introduce the new code.
>
> Signed-off-by: Juan Quintela
>
> ---
>
> Change it to a capability. As capabilities are off by default, have
> to change MULTIFD_ZERO_PAGE to MAIN_ZERO_PAGE, so it is
On Tue, 2022-08-02 at 08:39 +0200, Juan Quintela wrote:
> We do the send_prepare() and the fill of the head packet without the
> mutex held. It will help a lot for compression and later in the
> series for zero pages.
>
> Notice that we can use p->pages without holding p->mutex because
> p->pendi
w to handle the SYNC flag */
> p->flags &= ~MULTIFD_FLAG_SYNC;
> -trace_multifd_recv(p->id, p->packet_num, p->normal_num, flags,
> - p->next_packet_size);
> p->num_packets++;
> p->total_normal_pages += p->normal_num;
> qemu_mutex_unlock(&p->mutex);
> @@ -1128,7 +1131,7 @@ static void *multifd_recv_thread(void *opaque)
> }
> }
>
> -if (flags & MULTIFD_FLAG_SYNC) {
> +if (sync_needed) {
> qemu_sem_post(&multifd_recv_state->sem_sync);
> qemu_sem_wait(&p->sem_sync);
> }
Ok, IIUC this part should have the same behavior as before, but using a bool
instead of an u32.
Other than question [1], LGTM.
FWIW:
Reviewed-by: Leonardo Bras
igrationStats ram_counters;
>
> -static void ram_transferred_add(uint64_t bytes)
> +void ram_transferred_add(uint64_t bytes)
> {
> if (runstate_is_running()) {
> ram_counters.precopy_bytes += bytes;
Other than that, FWIW:
Reviewed-by: Leonardo Bras
Best regards,
Leo
64_t) pages->num) * qemu_target_page_size()
> -+ p->packet_len;
> +transferred = ((uint64_t) pages->num) * p->page_size + p->packet_len;
> qemu_file_acct_rate_limit(f, transferred);
> ram_counters.multifd_bytes += transferred;
> ram_counters.transferred += transferred;
> @@ -939,6 +935,7 @@ int multifd_save_setup(Error **errp)
> /* We need one extra place for the packet header */
> p->iov = g_new0(struct iovec, page_count + 1);
> p->normal = g_new0(ram_addr_t, page_count);
> +p->page_size = qemu_target_page_size();
>
> if (migrate_use_zero_copy_send()) {
> p->write_flags = QIO_CHANNEL_WRITE_FLAG_ZERO_COPY;
> @@ -1186,6 +1183,7 @@ int multifd_load_setup(Error **errp)
> p->name = g_strdup_printf("multifdrecv_%d", i);
> p->iov = g_new0(struct iovec, page_count);
> p->normal = g_new0(ram_addr_t, page_count);
> +p->page_size = qemu_target_page_size();
> }
>
> for (i = 0; i < thread_count; i++) {
IIUC this info should never change after assigned, and is the same on every
multifd channel param.
I wonder if it would be interesting to have a common area for this kind of info,
which could be referenced by every multifd channel parameter.
Or maybe too much trouble?
Anyway, FWIW:
Reviewed-by: Leonardo Bras
Best regards,
Leo
On Tue, 2022-08-02 at 08:39 +0200, Juan Quintela wrote:
> We are going to create a new function for multifd latest in the series.
>
> Signed-off-by: Juan Quintela
> Reviewed-by: Dr. David Alan Gilbert
> Signed-off-by: Juan Quintela
Double Signed-off-by again.
> ---
> migration/ram.c | 13 +++
t; +p->sent_bytes += p->packet_len;;
Double semicolon.
> +p->sent_bytes += p->next_packet_size;
> p->pending_job--;
> qemu_mutex_unlock(&p->mutex);
>
IIUC, it changes how rate-limiting and ram counters perceive
TE_FLAG_ZERO_COPY;
> @@ -1183,6 +1183,7 @@ int multifd_load_setup(Error **errp)
> p->name = g_strdup_printf("multifdrecv_%d", i);
> p->iov = g_new0(struct iovec, page_count);
> p->normal = g_new0(ram_addr_t, page_count);
> +p->page_count = page_count;
> p->page_size = qemu_target_page_size();
> }
>
Same comment as Patch [1/12] here.
FWIW:
Reviewed-by: Leonardo Bras
Best regards,
Leo
c58ffc29 ("QIOChannelSocket: Implement io_writev zero copy flag &
io_flush for CONFIG_LINUX")
Signed-off-by: Leonardo Bras
---
io/channel-socket.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/io/channel-socket.c b/io/channel-socket.c
index e6e7a12ae1..4f6bb6fe37 1
Please include:
Fixes: 69ef1f36b0 ("migration: define 'tls-creds' and 'tls-hostname'
migration parameters")
Fixes: 1d58872a91 ("migration: do not wait for free thread")
Fixes: d2f1d29b95 ("migration: add support for a "tls-authz" migration
Some of params->has_* = true are missing in migration_instance_init, this
causes migrate_params_check() to skip some tests, allowing some
unsupported scenarios.
Fix this by adding all missing params->has_* = true in
migration_instance_init().
Signed-off-by: Leonardo Bras
---
mig
ero-copy-send on migrate_params_check().
3) XBZRLE is also a compression capability, so it makes sense to also add
it to the list of capabilities which are not supported with zero-copy-send.
Fixes: 1abaec9a1b2c ("migration: Change zero_copy_send from migration parameter
to migration
On Tue, Jul 12, 2022 at 7:42 PM Peter Xu wrote:
>
> On Mon, Jul 11, 2022 at 06:11:13PM -0300, Leonardo Bras wrote:
> > Some errors, like the lack of Scatter-Gather support by the network
> > interface(NETIF_F_SG) may cause sendmsg(...,MSG_ZEROCOPY) to fail on using
> > ze
, which checks for errors each of the previous calls to
sendmsg(...,MSG_ZEROCOPY). If all of them failed to use zero-copy, then
increment dirty_sync_missed_zero_copy migration stat to let the user know
about it.
Signed-off-by: Leonardo Bras
Reviewed-by: Daniel P. Berrangé
---
migration/ram.h
Signed-off-by: Leonardo Bras
Acked-by: Markus Armbruster
Reviewed-by: Daniel P. Berrangé
---
qapi/migration.json | 7 ++-
migration/migration.c | 2 ++
monitor/hmp-cmds.c| 5 +
3 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/qapi/migration.json b/qapi
("QIOChannelSocket: Implement io_writev zero copy flag &
io_flush for CONFIG_LINUX")
Signed-off-by: Leonardo Bras
Reviewed-by: Daniel P. Berrangé
Acked-by: Daniel P. Berrangé
Reviewed-by: Juan Quintela
---
io/channel-socket.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
ation release number changed from 7.2 to 7.1
- migration stat renamed from zero-copy-copied to
dirty-sync-missed-zero-copy
- Updated documentation to make it more user-friendly
Changes since v1:
- Idea of using a warning replaced by using a migration stat counter
Leonardo Br
On Thu, Jul 7, 2022 at 7:18 PM Peter Xu wrote:
>
> On Thu, Jul 07, 2022 at 06:14:17PM -0300, Leonardo Brás wrote:
> > Having 'if(queued == sent)' will cause us to falsely return '1' in two buggy
> > cases, while 'if queued == 0) will either skip early o
On Thu, 2022-07-07 at 16:06 -0400, Peter Xu wrote:
> On Thu, Jul 07, 2022 at 04:59:22PM -0300, Leonardo Bras Soares Passos wrote:
> > Hello Peter,
> >
> > On Thu, Jul 7, 2022 at 2:56 PM Peter Xu wrote:
> > >
> > > On Mon, Jul 04, 2022 at 05:23:15PM -
On Thu, 2022-07-07 at 15:56 -0400, Peter Xu wrote:
> On Thu, Jul 07, 2022 at 04:50:47PM -0300, Leonardo Bras Soares Passos wrote:
> > > I also think we should squash
> > > patch 2/3 as patch 3 only started to provide meaningful values.
> >
> > IIRC Previously in
On Thu, 2022-07-07 at 15:52 -0400, Peter Xu wrote:
> On Thu, Jul 07, 2022 at 04:44:21PM -0300, Leonardo Bras Soares Passos wrote:
> > Hello Peter,
> >
> > On Thu, Jul 7, 2022 at 2:47 PM Peter Xu wrote:
> > >
> > > Hi, Leo,
> > >
> > >
Hello Peter,
On Thu, Jul 7, 2022 at 2:56 PM Peter Xu wrote:
>
> On Mon, Jul 04, 2022 at 05:23:15PM -0300, Leonardo Bras wrote:
> > Some errors, like the lack of Scatter-Gather support by the network
> > interface(NETIF_F_SG) may cause sendmsg(...,MSG_ZEROCOPY) to fail on u
Hello Peter,
On Thu, Jul 7, 2022 at 2:54 PM Peter Xu wrote:
>
> On Mon, Jul 04, 2022 at 05:23:14PM -0300, Leonardo Bras wrote:
> > Signed-off-by: Leonardo Bras
> > ---
> > qapi/migration.json | 7 ++-
> > migration/migration.c | 2 ++
> > monitor
Hello Peter,
On Thu, Jul 7, 2022 at 2:47 PM Peter Xu wrote:
>
> Hi, Leo,
>
> On Mon, Jul 04, 2022 at 05:23:13PM -0300, Leonardo Bras wrote:
> > If flush is called when no buffer was sent with MSG_ZEROCOPY, it currently
> > returns 1. This return code should be used only
, which checks for errors each of the previous calls to
sendmsg(...,MSG_ZEROCOPY). If all of them failed to use zero-copy, then
increment dirty_sync_missed_zero_copy migration stat to let the user know
about it.
Signed-off-by: Leonardo Bras
---
migration/ram.h | 2 ++
migration/multifd.c | 2
Signed-off-by: Leonardo Bras
---
qapi/migration.json | 7 ++-
migration/migration.c | 2 ++
monitor/hmp-cmds.c| 4
3 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/qapi/migration.json b/qapi/migration.json
index 7102e474a6..fed08b9b88 100644
--- a/qapi/migration.json
1 - 100 of 481 matches
Mail list logo
