Re: [Qemu-block] [Qemu-devel] [PATCH 2/8] block: fix unbounded stack for dump_qdict

On Tue, Mar 08, 2016 at 02:47:31PM +0100, Markus Armbruster wrote: > Fam Zheng writes: > > Also I think the double underscore identifiers are considered reserved in C, > > no? > > Correct. C99 7.1.3 Reserved identifiers: All identifiers that begin > with an underscore and

Re: [Qemu-block] [Qemu-devel] [PATCH 2/8] block: fix unbounded stack for dump_qdict

On Tue, Mar 08, 2016 at 01:17:03PM +0100, Paolo Bonzini wrote: > > > On 08/03/2016 09:12, Markus Armbruster wrote: > > I'm afraid this isn't a good idea. It relies on the non-local argument > > that nobody will ever put a key longer than 255 into a qdict that gets > > dumped. That may even be

[Qemu-block] [PATCH 0/2] block/qapi: trivial fixes

One is to use literal printf format when possible. One is to fix an unbounded usage of stack. Peter Xu (2): block/qapi: make two printf() formats literal block/qapi: fix unbounded stack for dump_qdict block/qapi.c | 14 ++ 1 file changed, 6 insertions(+), 8 deletions

[Qemu-block] [PATCH 1/2] block/qapi: make two printf() formats literal

Fix two places to use literal printf format when possible. Signed-off-by: Peter Xu <pet...@redhat.com> --- block/qapi.c | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/block/qapi.c b/block/qapi.c index db2d3fb..c4c2115 100644 --- a/block/qapi.c +++ b/block/

Re: [Qemu-block] [PATCH 1/2] block/qapi: make two printf() formats literal

On Wed, Mar 09, 2016 at 03:14:03PM -0700, Eric Blake wrote: > > +func_fprintf(f, "%*s[%i]:%c", indentation * 4, "", i, > > + composite ? '\n' : ' '); > > [The nerd in me wants to point out that you could avoid the ternary by > writing '"\n "[composite]', but that's too

Re: [Qemu-block] [PATCH 1/2] block/qapi: make two printf() formats literal

On Mon, Mar 21, 2016 at 03:14:48PM -0600, Eric Blake wrote: > On 03/09/2016 06:46 PM, Peter Xu wrote: > > > > Is this a grammar btw? > > Yes, C has an ugly grammar, because [] is just syntactic sugar for > deferencing pointer addition with nicer operator precedence

Re: [Qemu-block] [Qemu-devel] [PATCH 2/8] block: fix unbounded stack for dump_qdict

On Tue, Mar 08, 2016 at 09:12:45AM +0100, Markus Armbruster wrote: > Peter Xu <pet...@redhat.com> writes: > > const char *format = composite ? "%*s%s:\n" : "%*s%s: "; > > Unrelated to your patch: ugh! > > Printf formats should

Re: [Qemu-block] [Qemu-devel] [PATCH] vmdk: Switch to heap arrays for vmdk_write_cid

file->bs, s->desc_offset, desc, DESC_SIZE); > -if (ret < 0) { > -return ret; > -} > > -return 0; > +out: > +g_free(desc); > +g_free(tmp_desc); > +return ret; > } > > static int vmdk_is_cid_valid(BlockDriverState *bs) > -- > 2.4.3 > > Besides the above nit-pick: Reviewed-by: Peter Xu <pet...@redhat.com>

[Qemu-block] [PATCH 0/8] Fix several unbounded stack usage

bonz...@redhat.com> CC: Richard Henderson <r...@twiddle.net> CC: Eduardo Habkost <ehabk...@redhat.com> CC: Gerd Hoffmann <kra...@redhat.com> CC: Juan Quintela <quint...@redhat.com> CC: Amit Shah <amit.s...@redhat.com> CC: Luiz Capitulino <lcapitul...@redh

[Qemu-block] [PATCH 2/8] block: fix unbounded stack for dump_qdict

Suggested-by: Paolo Bonzini <pbonz...@redhat.com> CC: Markus Armbruster <arm...@redhat.com> CC: Kevin Wolf <kw...@redhat.com> CC: qemu-block@nongnu.org Signed-off-by: Peter Xu <pet...@redhat.com> --- block/qapi.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) d

Re: [Qemu-block] [PATCH v2 1/1] qemu/migration: fix the double free problem on from_src_file

-n "${QEMU_NEED_PID}" ]; then > > > +echo $BASHPID > "${QEMU_TEST_DIR}/qemu-${_QEMU_HANDLE}.pid"; > > > +fi; exec "$QEMU_PROG" $QEMU_OPTIONS "$@" ) > > > QEMU X.Y.Z monitor - type 'help' for more information >

Re: [Qemu-block] [PATCH v1 1/1] qemu/migration: fix the double free problem on from_src_file

on/savevm.c > +++ b/migration/savevm.c > @@ -2290,7 +2290,6 @@ int load_snapshot(const char *name, Error **errp) > > aio_context_acquire(aio_context); > ret = qemu_loadvm_state(f); > -qemu_fclose(f); > aio_context_release(aio_context); > > migr

Re: [Qemu-block] [PATCH v1 1/1] qemu/migration: fix the double free problem on from_src_file

On Tue, Jun 06, 2017 at 11:38:05AM +0800, QingFeng Hao wrote: > > > 在 2017/6/6 11:03, Peter Xu 写道: > >On Mon, Jun 05, 2017 at 12:48:51PM +0200, QingFeng Hao wrote: > >>In load_vmstate, mis->from_src_file is freed twice, the first free is by

Re: [Qemu-block] [PATCH v2] iothread: fix epollfd leak in the process of delIOThread

ntext_setup(AioContext *ctx) > #endif > } > > +void aio_context_destroy(AioContext *ctx) > +{ > +#ifdef CONFIG_EPOLL_CREATE1 > +close(ctx->epollfd); Would it be better to call aio_epoll_disable() here? Otherwise it looks good to me. > +#endif > +} > + Regards, -- Peter Xu

Re: [Qemu-block] [PATCH v4] iothread: fix epollfd leak in the process of delIOThread

On Wed, May 16, 2018 at 07:14:53PM +0800, WangJie (Pluto) wrote: > Hi, Peter Xu: > If call aio_epoll_disable() here, aio_epoll_disable() will return > before close ctx->epollfd, > Because the ctx->epoll_enabled is false in the moment. > In the p

Re: [Qemu-block] [PATCH] iothread: fix epollfd leak in the process of delIOThread

; > > iothread->ctx = NULL; > > } > > -- > > 1.8.3.1 > > > > Please add an aio_context_destroy() function in aio-posix.c and call it from > aio_context_finalize(). IOThread code should not touch AioContext internals. I believe Fam means aio_ctx_

Re: [Qemu-block] [PATCH v6 2/2] iothread: let aio_epoll_disable fit to aio_context_destroy

{ > -close(ctx->epollfd); > -} > +aio_epoll_disable(ctx); Hmm... I think this patch should be the first if to split. Anyway, IMHO version 5 is already good enough and has got r-bs, no need to bother reposting a version 7. Maintainer could possibly touch up the commit message if necessary. Thanks, > #endif > } > > -- > 1.8.3.1 > -- Peter Xu

Re: [Qemu-block] raw iotest regressions in 2.12.0-rc0

On Wed, Mar 21, 2018 at 05:58:48PM -0400, John Snow wrote: > ./check -v -raw > Failures: 109 132 136 148 152 183 > > 3fd2457d18edf5736f713dfe1ada9c87a9badab1 is the first bad commit > commit 3fd2457d18edf5736f713dfe1ada9c87a9badab1 > Author: Peter Xu <pet...@redhat.com> &

Re: [Qemu-block] raw iotest regressions in 2.12.0-rc0

On Wed, Mar 21, 2018 at 05:58:48PM -0400, John Snow wrote: > ./check -v -raw > Failures: 109 132 136 148 152 183 > > 3fd2457d18edf5736f713dfe1ada9c87a9badab1 is the first bad commit > commit 3fd2457d18edf5736f713dfe1ada9c87a9badab1 > Author: Peter Xu <pet...@redhat.com> &

Re: [Qemu-block] [PATCH 2/2] virtio-scsi/virtio-blk: Disable poll handlers when stopping vq handler

g IOMMU faults > during kexec for a long time, so any sort of marking a device broken > for a fault should be thoroughly considered, especially when a device > could be assigned to a user who can trivially trigger a fault. > > > 2) translation faliures should definitely not print messages to stderr. > > Yep, easy DoS vector for a malicious guest, or malicious userspace > driver within the guest. Thanks, Note that it's using error_report_once() upstream so it'll only print once for the whole lifecycle of QEMU process, and it's still a tracepoint downstream so no error will be dumped by default. So AFAIU it's not a DoS target for either. I would consider it a good hint for strange bugs since AFAIU DMA error should never exist on well-behaved guests. However I'll be fine too to post a patch to make it an explicit tracepoint again if any of us would still like it to go away. Thanks, -- Peter Xu

Re: [Qemu-block] [Qemu-devel] [PATCH v2 04/11] monitor: Create MonitorHMP with readline state

mentioning that it's only used in main thread so no lock is needed for all the HMP only structs (until someone wants to hammer on HMP again). Thanks, -- Peter Xu

Re: [Qemu-block] [PATCH] cutils: Move size_to_str() from "qemu-common.h" to "qemu/cutils.h"

On Tue, Sep 03, 2019 at 02:57:31PM -0400, John Snow wrote: [...] > Seems proper. It must be an oversight to begin with that we declared it > in qemu-common but defined it in cutils. Porbably true.. > > Reviewed-by: John Snow Reviewed-by: Peter Xu Thanks, -- Peter Xu

Re: [PATCH v2 0/4] apic: Fix migration breakage of >255 vcpus

Hi, John, I don't know seabios well, but I did have a pointer in my previous email on where it faulted. It seems to me that the issue is that SeaBIOS may have got incorrect secs/cyls/heads information (and explicitly setting secs=1,cyls=1,heads=1 on the block device fixes the issue). Thanks, -- Peter Xu

Re: [PATCH v2 0/4] apic: Fix migration breakage of >255 vcpus

On Sat, Oct 19, 2019 at 11:41:53AM +0800, Peter Xu wrote: > On Wed, Oct 16, 2019 at 11:40:01AM -0300, Eduardo Habkost wrote: > > On Wed, Oct 16, 2019 at 10:29:29AM +0800, Peter Xu wrote: > > > v2: > > > - use uint32_t rather than int64_t [Juan] > > > - o

Re: [PATCH v1 3/3] migration: multifd: Enable zerocopy

unless you happen to be running a QEMU config that already > implies locked memory (eg PCI assignment) Yes it would be great to be a migration capability in parallel to multifd. At initial phase if it's easy to be implemented on multi-fd only, we can add a dependency between the caps. In the future we can remove that dependency when the code is ready to go without multifd. Thanks, -- Peter Xu

Re: [PATCH v1 2/3] io: Add zerocopy and errqueue

hat reached my mind.. The qio_channel_writev_full() may not be suitable for async operations, as the name "full" implies synchronous to me. So maybe we can add a new helper for zero copy on the channel? We may also want a new QIOChannelFeature as QIO_CHANNEL_FEATURE_ZEROCOPY, then we fail qio_channel_writv_zerocopy() (or whatever name we come up with) if that bit is not set in qio channel features. Thanks, -- Peter Xu

Re: [PATCH v1 0/3] QIOChannel flags + multifd zerocopy

., what's the configuration of your VM for testing? What's the migration time before/after the patchset applied? What is the network you're using? Thanks, -- Peter Xu

Re: [PATCH v1 3/3] migration: multifd: Enable zerocopy

On Wed, Sep 01, 2021 at 09:53:07AM +0100, Daniel P. Berrangé wrote: > On Tue, Aug 31, 2021 at 04:29:09PM -0400, Peter Xu wrote: > > On Tue, Aug 31, 2021 at 02:16:42PM +0100, Daniel P. Berrangé wrote: > > > On Tue, Aug 31, 2021 at 08:02:39AM -0300, Leonardo Bras w

Re: [PATCH v1 3/3] migration: multifd: Enable zerocopy

are currently > mutually exclusive as they both use the same kernel network hooks > framework. Then we may need to at least figure out whether zerocopy needs to mask out mptcp. -- Peter Xu

Re: [PATCH v1 2/3] io: Add zerocopy and errqueue

On Wed, Sep 01, 2021 at 09:50:56AM +0100, Daniel P. Berrangé wrote: > On Tue, Aug 31, 2021 at 04:27:04PM -0400, Peter Xu wrote: > > On Tue, Aug 31, 2021 at 01:57:33PM +0100, Daniel P. Berrangé wrote: > > > On Tue, Aug 31, 2021 at 08:02:38AM -0300, Leonardo Bras wrote: &g

Re: [PATCH v1 2/3] io: Add zerocopy and errqueue

even valid... :) There can still be two versions depending on when the page is read and feed to the NICs as the page can be changing during the window, but as long as the latter sent page always lands later than the former page on dest node then it looks ok. -- Peter Xu

Re: [PATCH v1 2/3] io: Add zerocopy and errqueue

On Wed, Sep 08, 2021 at 05:13:40PM -0300, Leonardo Bras Soares Passos wrote: > On Tue, Sep 7, 2021 at 1:44 PM Peter Xu wrote: > > > > On Thu, Sep 02, 2021 at 03:59:25AM -0300, Leonardo Bras Soares Passos wrote: > > > I also suggested something like that, but I

Re: [PATCH v1 2/3] io: Add zerocopy and errqueue

at'll guarantee the buffer not changing during sending, however we gain nothing besides the "api cleaness" then.. Thanks, -- Peter Xu

Re: [PATCH v1 2/3] io: Add zerocopy and errqueue

cover when switching to postcopy. We could merge this flush() into the existing per-iteration sync of multi-fd, but it's not strictly necessary, imho. We can see which is easier. The rest looks good to me. Thanks, -- Peter Xu

Re: [PATCH v1 2/3] io: Add zerocopy and errqueue

tect the zero copy capability and use one alternative. Thanks, -- Peter Xu

Re: [PATCH v1 2/3] io: Add zerocopy and errqueue

tion is outstanding at any one point. Ordered delivery is the common case, but not guaranteed. Notifications may arrive out of order on retransmission and socket teardown. So no matter whether we have a flush() per iteration before, it seems we may want one when zero copy enabled? Thanks, -- Peter Xu

Re: [PATCH v1 3/3] migration: multifd: Enable zerocopy

nly with either mlock() (FOLL_MLOCK) or vfio (FOLL_PIN). I don't really think MSG_ZEROCOPY is doing that at all... I'm looking at __zerocopy_sg_from_iter() -> iov_iter_get_pages(). Say, I think there're plenty of iov_iter_get_pages() callers and normal GUPs, I think most of them do no need such accountings. -- Peter Xu

Re: [PATCH v1 3/3] migration: multifd: Enable zerocopy

On Wed, Sep 08, 2021 at 10:59:57AM +0800, Jason Wang wrote: > On Wed, Sep 8, 2021 at 2:32 AM Peter Xu wrote: > > > > On Thu, Sep 02, 2021 at 04:22:55AM -0300, Leonardo Bras Soares Passos wrote: > > > > I don't think it is valid to unconditiona

Re: [PATCH v1 3/3] migration: multifd: Enable zerocopy

On Wed, Sep 08, 2021 at 09:19:27AM +0100, Dr. David Alan Gilbert wrote: > * Jason Wang (jasow...@redhat.com) wrote: > > On Wed, Sep 8, 2021 at 2:32 AM Peter Xu wrote: > > > > > > On Thu, Sep 02, 2021 at 04:22:55AM -0300, Leonardo Bras Soares Passos > >

Re: [PATCH v1 2/3] io: Add zerocopy and errqueue

On Wed, Sep 08, 2021 at 09:30:58AM +0100, Dr. David Alan Gilbert wrote: > * Peter Xu (pet...@redhat.com) wrote: > > On Tue, Sep 07, 2021 at 12:06:15PM +0100, Dr. David Alan Gilbert wrote: > > > > > What if we do the 'flush()' before we start post-copy, instead

Re: [PATCH v2 8/9] hw/dma: Use dma_addr_t type definition when relevant

residual = dma_buf_write(ptr, len, >qsg, attrs); If there's a new version: Maybe also change the return value types of dma_buf_write|read() to dma_addr_t? It'll be changed anyway in the next patch, so not a big deal. The rest patches looks good to me. Thanks. -- Peter Xu

Re: [PATCH v3 00/10] hw/dma: Use dma_addr_t type definition when relevant

a_addr_t (Peter) > - Drop 'propagate MemTxResult' patch (David) > - Added R-b tags Reviewed-by: Peter Xu -- Peter Xu

Re: [PATCH v8 5/5] multifd: Implement zero copy write in multifd migration (multifd-zero-copy)

oes not fall into the user<->kernel switches (which is where the extra overhead of write() syscall, iiuc), but we simply blocked on any of the write()s because the socket write buffer is full... So we could have saved some cpu cycles by merging the calls, but performance-wise we may not get much. Thanks, -- Peter Xu

Re: [PATCH v8 5/5] multifd: Implement zero copy write in multifd migration (multifd-zero-copy)

nd_sync_main(QEMUFile *f) > if (p->quit) { > error_report("%s: channel %d has already quit", __func__, i); > qemu_mutex_unlock(>mutex); > -return; > +return 0; Same question here. > } The rest looks good. Thanks, -- Peter Xu

Re: [PATCH v8 5/5] multifd: Implement zero copy write in multifd migration (multifd-zero-copy)

On Mon, Feb 07, 2022 at 11:49:38PM -0300, Leonardo Bras Soares Passos wrote: > Hello Peter, thanks for reviewing! > > On Mon, Feb 7, 2022 at 11:22 PM Peter Xu wrote: > > > > On Tue, Feb 01, 2022 at 03:29:03AM -0300, Leonardo Bras wrote: > > > -void multifd_send_syn

Re: [PATCH v8 1/5] QIOChannel: Add flags on io_writev and introduce io_flush callback

pted to > receive a flag parameter. That allows shared code between zero copy and > non-zero copy writev, and also an easier implementation on new flags. > > Signed-off-by: Leonardo Bras With Dan's comment addressed on removing the redundant assertion: Reviewed-by: Peter Xu -- Peter Xu

Re: [PATCH v3] hw/dma: Let dma_buf_read() / dma_buf_write() propagate MemTxResult

enough to hold the residual size, and the type is constant > on both 32/64-bit hosts. > > Update the few dma_buf_read() / dma_buf_write() callers to the new > API. > > Reviewed-by: Klaus Jensen > Signed-off-by: Philippe Mathieu-Daudé > Signed-off-by: Philippe Mathieu-Daudé Acked-by: Peter Xu -- Peter Xu

Re: [PATCH v2 21/22] softmmu/memory: Clean up local variable shadowing

moryRegionList *ml; > | ^~ > softmmu/memory.c:3213:32: note: shadowed declaration is here >3213 | MemoryRegionList *new_ml, *ml, *next_ml; > |^~ > > Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Peter Xu -- Peter Xu

Re: [PATCH 1/2] vmstate: Mark VMStateInfo.get/put() coroutine_mixed_fn

the callback definition in VMStateInfo to be explicit about it. > > Signed-off-by: Kevin Wolf Acked-by: Peter Xu -- Peter Xu

Re: [RFC PATCH v2 22/22] softmmu/physmem: Clean up local variable shadowing

ame before/after this > change (and if it didn't do so then it's a bug..) > > > --- > > softmmu/physmem.c | 10 +----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > Reviewed-by: Daniel P. Berrangé Reviewed-by: Peter Xu -- Peter Xu

Re: [PATCH 2/7] migration: Clean up local variable shadowing

else rename variables. > > Signed-off-by: Markus Armbruster Reviewed-by: Peter Xu -- Peter Xu

Re: [PATCH 1/7] migration/rdma: Fix save_page method to fail on polling error

remains zero. > > Broken since day one, and duplicated more recently. > > Fixes: 2da776db4846 (rdma: core logic) > Fixes: b390afd8c50b (migration/rdma: Fix out of order wrid) > Signed-off-by: Markus Armbruster Reviewed-by: Peter Xu -- Peter Xu

Re: [PATCH V2 3/6] cpr: relax blockdev migration blockers

goto fail; > } > diff --git a/block/vvfat.c b/block/vvfat.c > index 266e036..9d050ba 100644 > --- a/block/vvfat.c > +++ b/block/vvfat.c > @@ -1268,7 +1268,7 @@ static int vvfat_open(BlockDriverState *bs, QDict > *options, int flags, > "The vvfat (rw) format used by node '%s' " > "does not support live migration", > bdrv_get_device_or_node_name(bs)); > -ret = migrate_add_blocker(>migration_blocker, errp); > +ret = migrate_add_blocker_normal(>migration_blocker, errp); > if (ret < 0) { > goto fail; > } > -- > 1.8.3.1 > -- Peter Xu

Re: [PATCH V2 4/6] cpr: relax vhost migration blockers

dev_init(struct vhost_dev *hdev, void *opaque, > } > > if (hdev->migration_blocker != NULL) { > -r = migrate_add_blocker(>migration_blocker, errp); > +r = migrate_add_blocker_normal(>migration_blocker, errp); > if (r < 0) { > goto fail_busyloop; > } > -- > 1.8.3.1 > -- Peter Xu

Re: [PATCH v2 5/9] migration: check required subsections are loaded, once

ore.kernel.org/qemu-devel/ZR2P1RbxCfBdYBaQ@x1n/ I still think it is safer to not fail unless justified that we won't hit surprises in the ->needed(). There are a lot so I assume it's non-trivial to justify. We can switch the tracepoint into error_report() in that case, though, as long as it won't fail the migration. Thanks, -- Peter Xu

Re: [PATCH 4/5] RFC: migration: check required subsections are loaded, once

the migration? > + > trace_vmstate_subsection_load_good(vmsd->name); > return 0; > } > -- > 2.41.0 > -- Peter Xu

Re: [PATCH 0/5] RFC: migration: check required entries and sections are loaded

, am I right? It'll be great if they can be acked (or even picked up earlier?) by subsystem maintainers if so, and copy stable if justified proper. Thanks, -- Peter Xu

Re: introducing vrc :)

On Thu, Apr 21, 2022 at 05:04:52PM +0200, Paolo Bonzini wrote: > On 4/20/22 20:12, Peter Xu wrote: > > > a while ago I looked at tools that could be used too build a call graph. > > > The simplest but most effective that I found was a small Perl program > > > (

Re: introducing vrc :)

quot; aka RTL call graph) that used > the GCC dumps to build the graph. Paolo, Do you have any plan to put it into some git repository? Thanks! -- Peter Xu

Re: [PATCH v9 7/7] multifd: Implement zero copy write in multifd migration (multifd-zero-copy)

} > + > socket_send_channel_create(multifd_new_send_channel_async, p); > } > > diff --git a/migration/socket.c b/migration/socket.c > index 3754d8f72c..4fd5e85f50 100644 > --- a/migration/socket.c > +++ b/migration/socket.c > @@ -79,8 +79,9 @@ static void socket_outgoing_migration(QIOTask *task, > > trace_migration_socket_outgoing_connected(data->hostname); > > -if (migrate_use_zero_copy_send()) { > -error_setg(, "Zero copy send not available in migration"); > +if (migrate_use_zero_copy_send() && > +!qio_channel_has_feature(sioc, QIO_CHANNEL_FEATURE_WRITE_ZERO_COPY)) > { > +error_setg(, "Zero copy send feature not detected in host > kernel"); > } > > out: > -- > 2.36.0 > -- Peter Xu

Re: [PATCH v9 5/7] multifd: multifd_send_sync_main now returns negative on error

ll it's callers to make use of this change and possibly fail > earlier. > > (This change is important to next patch on multifd zero copy > implementation, to make it sure an error in zero-copy flush does not go > unnoticed. > > Signed-off-by: Leonardo Bras Reviewed-by: Peter Xu -- Peter Xu

Re: [PATCH v10 6/7] multifd: Send header packet without flags if zero-copy-send is enabled

erocopy */ > +ret = qio_channel_write_all(p->c, (void *)p->packet, > +p->packet_len, _err); > +if (ret != 0) { > +break; > +} > +

Re: [PATCH v10 7/7] multifd: Implement zero copy write in multifd migration (multifd-zero-copy)

change will dirty the page and > cause it to be re-sent on a next iteration anyway. > > A lot of locked memory may be needed in order to use multifd migration > with zero-copy enabled, so disabling the feature should be necessary for > low-privileged users trying to perform multifd

Re: [External] [PATCH v13 3/8] QIOChannelSocket: Implement io_writev zero copy flag & io_flush for CONFIG_LINUX

e migration, > > - Because of this, when an user code wants to add zero copy as a feature, it > > requires a mechanism to disable it, so it can still be accessible to less > > privileged users. > > > > Signed-off-by: Leonardo Bras > > Reviewed-by: Peter Xu > > R

Re: [External] [PATCH v13 3/8] QIOChannelSocket: Implement io_writev zero copy flag & io_flush for CONFIG_LINUX

s > the idea manually keeping track of flush happening? Yes if we can check this up it'll be good enough to me. The trace point could help in some case in the future too to monitor the behavior of kernel MSG_ERRQUEUE but if you don't like it then it's okay. -- Peter Xu

Re: [External] [PATCH v13 3/8] QIOChannelSocket: Implement io_writev zero copy flag & io_flush for CONFIG_LINUX

On Wed, Jun 08, 2022 at 03:14:36PM -0300, Leonardo Bras Soares Passos wrote: > On Wed, Jun 8, 2022 at 8:41 AM Peter Xu wrote: > > > > On Wed, Jun 08, 2022 at 02:37:28AM -0300, Leonardo Bras Soares Passos wrote: > > > (1) is not an option, as the interface currently uses

Re: [External] [PATCH v13 3/8] QIOChannelSocket: Implement io_writev zero copy flag & io_flush for CONFIG_LINUX

On Mon, Jun 13, 2022 at 05:58:44PM -0300, Leonardo Bras Soares Passos wrote: > Hello Peter, > > On Wed, Jun 8, 2022 at 5:23 PM Peter Xu wrote: > [...] > > > In a previous iteration of the patchset, it was made clear that it's > > > desirable to detect when th

Re: [PATCH v11 2/7] QIOChannelSocket: Implement io_writev zero copy flag & io_flush for CONFIG_LINUX

ux.. I don't think it's anything dangerous, but IMHO it's another way of being not clean comparing of using some "#ifdef"s. Comparing to this approach the "use #ifdef" approach is actually slightly more cleaner to me. :) Let's wait for some other inputs. -- Peter Xu

Re: [PATCH v12 2/7] QIOChannelSocket: Implement io_writev zero copy flag & io_flush for CONFIG_LINUX

; #define SOCKET_MAX_FDS 16 > > + This line can be dropped when merge. > SocketAddress * > qio_channel_socket_get_local_address(QIOChannelSocket *ioc, > Error **errp) This does look nicer, imho. :) Thanks! -- Peter Xu

Re: [PATCH v11 2/7] QIOChannelSocket: Implement io_writev zero copy flag & io_flush for CONFIG_LINUX

ly ask for that - we can leave that for later. Thanks, -- Peter Xu

Re: QMP (without OOB) function running in thread different from the main thread as part of aio_poll

at > >> ../softmmu/physmem.c:2641 > >> #28 0x558ed403a857 in flatview_write (fv=0x558ed66d62c0, > >> addr=4291004224, attrs=..., buf=0x7f9029957028, len=1) at > >> ../softmmu/physmem.c:2683 > >> #29 0x558ed403ac07 in address_space_write (as=0x558ed4ca2b20 > >> , addr=4291004224, attrs=..., > >> buf=0x7f9029957028, len=1) at ../softmmu/physmem.c:2779 > >> #30 0x558ed403ac74 in address_space_rw (as=0x558ed4ca2b20 > >> , addr=4291004224, attrs=..., > >> buf=0x7f9029957028, len=1, is_write=true) at > >> ../softmmu/physmem.c:2789 > >> #31 0x558ed40cea88 in kvm_cpu_exec (cpu=0x558ed622a910) at > >> ../accel/kvm/kvm-all.c:2989 > >> #32 0x558ed40d179a in kvm_vcpu_thread_fn (arg=0x558ed622a910) at > >> ../accel/kvm/kvm-accel-ops.c:51 > >> #33 0x558ed42d925f in qemu_thread_start (args=0x558ed5c68c80) at > >> ../util/qemu-thread-posix.c:541 > >> #34 0x7f9028ab7ea7 in start_thread (arg=) at > >> pthread_create.c:477 > >> #35 0x7f9027c18a2f in clone () at > >> ../sysdeps/unix/sysv/linux/x86_64/clone.S:95 Totally unfamiliar with block jobs, but... does it mean that snapshot_*_job_bh()s should just always make sure BQL taken? Thanks, -- Peter Xu

Re: [PULL 00/26] Next patches

ld on anything that isn't Linux: > > In file included from ../migration/postcopy-ram.c:40: > /private/var/folders/76/zy5ktkns50v6gt5g8r0sf6scgn/T/cirrus-ci-build/include/qemu/userfaultfd.h:18:10: > fatal error: 'linux/userfaultfd.h' file not found Oops, my fault. Juan, please feel free to drop patch "util/userfaultfd: Add uffd_open()". I'll respin with the whole set. -- Peter Xu

Re: [PATCH v2 04/20] qemu-file: We only call qemu_file_transferred_* on the sending side

On Tue, Jun 13, 2023 at 06:02:05PM +0200, Juan Quintela wrote: > Peter Xu wrote: > > On Tue, May 30, 2023 at 08:39:25PM +0200, Juan Quintela wrote: > >> Remove the increase in qemu_file_fill_buffer() and add asserts to > >> qemu_file_transferred* functions. > >>

Re: [PATCH v2 06/20] qemu_file: total_transferred is not used anymore

can also propose something else, but IMHO that seems to be a higher priority? And whatever else I haven't noticed. I'll continue reading but I'm sure you know the best on this.. so I'd really rely on you. What do you think? Thanks, -- Peter Xu

Re: [PATCH v2 16/20] migration/rdma: Split qemu_fopen_rdma() into input/output functions

HANNEL_RDMA)); > +rioc->file = qemu_file_new_input(QIO_CHANNEL(rioc)); > +rioc->rdmain = rdma; > +rioc->rdmaout = rdma->return_path; > > -if (qemu_file_mode_is_not_valid(mode)) { We can also drop this function together. If with that: Reviewed-b

Re: [PATCH v2 20/20] qemu-file: Make qemu_file_get_error_obj() static

On Tue, May 30, 2023 at 08:39:41PM +0200, Juan Quintela wrote: > It was not used outside of qemu_file.c anyways. > > Signed-off-by: Juan Quintela Reviewed-by: Peter Xu -- Peter Xu

Re: [PATCH v2 19/20] qemu-file: Simplify qemu_file_shutdown()

On Tue, May 30, 2023 at 08:39:40PM +0200, Juan Quintela wrote: > Signed-off-by: Juan Quintela Reviewed-by: Peter Xu -- Peter Xu

Re: [PATCH v2 18/20] qemu_file: Make qemu_file_is_writable() static

On Tue, May 30, 2023 at 08:39:39PM +0200, Juan Quintela wrote: > It is not used outside of qemu_file, and it shouldn't. > > Signed-off-by: Juan Quintela Reviewed-by: Peter Xu -- Peter Xu

Re: [RFC 4/6] migration: Deprecate -incoming

" set the uri with migrate-incoming.", incoming); I still use uri for all my scripts, alongside with "-global migration.xxx" and it works. Shall we just leave it there? Or is deprecating it helps us in any form? -- Peter Xu

Re: [RFC 4/6] migration: Deprecate -incoming

On Mon, Jun 12, 2023 at 10:51:08PM +0200, Juan Quintela wrote: > Peter Xu wrote: > > On Mon, Jun 12, 2023 at 09:33:42PM +0200, Juan Quintela wrote: > >> Only "defer" is recommended. After setting all migation parameters, > >> start incoming migratio

Re: [PATCH v2 05/20] qemu_file: Use a stat64 for qemu_file_transferred

> g_assert(qemu_file_is_writable(f)); > qemu_fflush(f); > -return f->total_transferred; > +return stat64_get(_stats.qemu_file_transferred); > } > > void qemu_put_be16(QEMUFile *f, unsigned int v) > -- > 2.40.1 > -- Peter Xu

Re: [PATCH v2 04/20] qemu-file: We only call qemu_file_transferred_* on the sending side

e worth also touching the document of QEMUFile::total_transferred to clarify what it accounts? Reviewed-by: Peter Xu Though when I'm looking at the counters (didn't follow every single recent patch on this..), I found that now reading transferred value is actually more expensive - qemu_file_transfe

Re: [RFC 4/6] migration: Deprecate -incoming

channels=16" would > > desugar to > > > > migrate_set_parameter multifd-channels 16 > > migrate_incoming tcp:foo > > > > The only incompatibility is for people who are using "," in an URI, > > which is rare and only an issue for the "exec" protocol. If we worry on breaking anyone, we can apply the keyval parsing only when !exec. Not sure whether it matters a huge lot.. > > Aha, that makes sense. And will allow us to deprecate/remove the > --global migration.* stuff. We may still need a way to set the caps/params for src qemu?.. Thanks, -- Peter Xu

Re: [RFC 4/6] migration: Deprecate -incoming

ore.kernel.org/r/20230622021320.66124-23-quint...@redhat.com -- Peter Xu

Re: [PATCH v2 06/20] qemu_file: total_transferred is not used anymore

On Thu, Jun 22, 2023 at 01:05:49AM +0200, Juan Quintela wrote: > Peter Xu wrote: > > On Tue, May 30, 2023 at 08:39:27PM +0200, Juan Quintela wrote: > >> Signed-off-by: Juan Quintela > >> --- > >> migration/qemu-file.c | 4 > >> 1 file

Re: [RFC 4/6] migration: Deprecate -incoming

should always works with old binaries. - When src is newer it should be able to know what's missing on dest and skip the new bits. - When dst is newer it should all rely on src (which is older) and it should always understand src's language. - All !main channels need to be established later than the handshake - if we're going to do this anyway we probably should do it altogether to make channels named, so each channel used in migration needs to have a common header. Prepare to deprecate the old tricks of channel orderings. Does it look reasonable? Anything to add/remove? Thanks, -- Peter Xu

Re: [RFC 4/6] migration: Deprecate -incoming

On Thu, Jun 22, 2023 at 05:33:29PM +0100, Daniel P. Berrangé wrote: > On Thu, Jun 22, 2023 at 11:54:43AM -0400, Peter Xu wrote: > > I can try to move the todo even higher. Trying to list the initial goals > > here: > > > > - One extra phase of handshake between src/d

Re: [RFC 4/6] migration: Deprecate -incoming

On Fri, Jun 23, 2023 at 08:17:46AM +0100, Daniel P. Berrangé wrote: > On Thu, Jun 22, 2023 at 03:20:01PM -0400, Peter Xu wrote: > > On Thu, Jun 22, 2023 at 05:33:29PM +0100, Daniel P. Berrangé wrote: > > > On Thu, Jun 22, 2023 at 11:54:43AM -0400, Peter Xu wrote: > > > &

Re: [RFC 4/6] migration: Deprecate -incoming

On Fri, Jun 23, 2023 at 09:23:18AM +0100, Daniel P. Berrangé wrote: > On Thu, Jun 22, 2023 at 11:54:43AM -0400, Peter Xu wrote: > > On Thu, Jun 22, 2023 at 10:59:58AM +0100, Daniel P. Berrangé wrote: > > > I've mentioned several times before that the user should never

Re: [PATCH v2 1/5] migration: Use proper indentation for migration.json

On Thu, Jun 22, 2023 at 09:50:15PM +0200, Juan Quintela wrote: > We broke it with dirtyrate limit patches. > > Signed-off-by: Juan Quintela Acked-by: Peter Xu -- Peter Xu

Re: [PATCH v2 5/5] migration: Deprecate old compression method

On Thu, Jun 22, 2023 at 09:50:19PM +0200, Juan Quintela wrote: > Signed-off-by: Juan Quintela Acked-by: Peter Xu -- Peter Xu

Re: [RFC 4/6] migration: Deprecate -incoming

mdline tool that can easily tunnel qmp commands so whoever developers using pure cmdlines can switch to the new way easily. Thanks, -- Peter Xu

Re: [PATCH] migration: for snapshots, hold the BQL during setup callbacks

Help needed on figuring this out (and if there is, IMHO it'll be worthwhile to put that into comment of save_setup() hook). Thanks, -- Peter Xu

Re: [PATCH 0/9] QEMU file cleanups

ing unsigned here. I've reviewed all the rest patches and all look good here. Thanks, -- Peter Xu

Re: [PATCH 0/9] QEMU file cleanups

On Thu, May 04, 2023 at 04:56:46PM +0200, Juan Quintela wrote: > Peter Xu wrote: > > On Thu, May 04, 2023 at 01:38:32PM +0200, Juan Quintela wrote: > >> - convince and review code to see that everything is uint64_t. > > > > One general question to patches regarding t

Re: [PATCH 8/9] qemu-file: Make ram_control_save_page() use accessors for rate_limit

On Thu, May 04, 2023 at 01:38:40PM +0200, Juan Quintela wrote: > Signed-off-by: Juan Quintela Reviewed-by: Peter Xu -- Peter Xu

Re: [PATCH 6/9] qemu-file: remove shutdown member

) in the places that it was tested. > > Signed-off-by: Juan Quintela Nit: I'd squash this with previous. Reviewed-by: Peter Xu -- Peter Xu

Re: [PATCH 9/9] qemu-file: Account for rate_limit usage on qemu_fflush()

ile won't be accounted until flushed. Two limits here: - IO_BUF_SIZE==32K, the real buffer - MAX_IOV_SIZE==64 (I think), the async buffer to put guest page ptrs directly, on x86 it's 64*4K=256K So the impact should be no more than 288KB on x86. Looks still fine.. Reviewed-by: Peter Xu -- Peter Xu

Re: [PATCH 5/9] qemu-file: No need to check for shutdown in qemu_file_rate_limit

On Thu, May 04, 2023 at 01:38:37PM +0200, Juan Quintela wrote: > After calling qemu_file_shutdown() we set the error as -EIO if there > is no another previous error, so no need to check it here. > > Signed-off-by: Juan Quintela Reviewed-by: Peter Xu -- Peter Xu

Re: [PATCH] migration: for snapshots, hold the BQL during setup callbacks

On Wed, May 10, 2023 at 08:31:13AM +0200, Juan Quintela wrote: > Peter Xu wrote: > > Hi > > [Adding Kevin to the party] > > > On Fri, May 05, 2023 at 03:46:52PM +0200, Fiona Ebner wrote: > >> To fix it, ensure that the BQL is held during setup. To avoid changin

  1   2   >