* Peter Xu (pet...@redhat.com) wrote:
> Hey, Dave!
Hey!
> On Wed, Jun 05, 2024 at 12:31:56AM +0000, Dr. David Alan Gilbert wrote:
> > * Michael Galaxy (mgal...@akamai.com) wrote:
> > > One thing to keep in mind here (despite me not having any hardware to
> > > t
provide good support and work together.
> > > > >
> > > > We are happy to provide the necessary review and maintenance work for
> > > > RDMA
> > > > if the community needs it.
> > > >
> > > > CC'ing Chuan Zheng.
14..fc5f5c57ad 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -398,19 +398,6 @@ SRST
>If called with option off, the emulation returns to normal mode.
> ERST
>
> -{
> -.name = "singlestep",
> -.args_type = "option:s?&quo
d usage of QMP features, or
> triaging bug reports that include examples of usage, and in both
> cases it is pretty relevant to make unstable features stand out to
> the human via the x- prefix IMHO.
Using x- for events makes sense to me; the semantics of events can be
quite su
er the ifdef includes the
SRST/ERST doc section; some ifdef do and some don't; and thus
it depends whether or not you want the command documented
even though it's compiled out.
I think it's probably OK, but maybe worth reconsidering:
Acked-by: Dr. David Alan Gilbert
> SRST
>
ULL, NULL, opaque);
> > +aio_set_fd_handler(ctx, rioc->rdmaout->recv_comp_channel->fd,
> > io_read,
> > + io_write, NULL, NULL, opaque);
> > +aio_set_fd_handler(ctx, rioc->rdmaout->send_comp_channel->fd,
> >
"current migration capabilities");
> return false;
> }
> -migrate_set_block_enabled(true, &local_err);
> -if (local_err) {
> +if (!migrate_cap_set(MIGRATION_CAPABILITY_BLOCK, true, &local_err)) {
> error_propagate(errp, local_err);
> return false;
> }
> --
> 2.39.2
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
he top-level directory.
> + *
> + * Contributions after 2012-01-13 are licensed under the terms of the
> + * GNU GPL, version 2 or (at your option) any later version.
I don't think any of these functions were written by Anthony, and I
think they're all after 2012 aren't they?
* Markus Armbruster (arm...@redhat.com) wrote:
> "Dr. David Alan Gilbert" writes:
>
> > * Markus Armbruster (arm...@redhat.com) wrote:
> >> Clean up includes so that osdep.h is included first and headers
> >> which it implies are not included manually.
&
gt; +++ b/hw/remote/machine.c
> @@ -22,7 +22,6 @@
> #include "hw/remote/iohub.h"
> #include "hw/remote/iommu.h"
> #include "hw/qdev-core.h"
> -#include "hw/remote/iommu.h"
> #include "hw/remote/vfio-user-obj.h"
> #include "
e/qemu/userfaultfd.h
> +++ b/include/qemu/userfaultfd.h
> @@ -13,7 +13,6 @@
> #ifndef USERFAULTFD_H
> #define USERFAULTFD_H
>
> -#include "qemu/osdep.h"
> #include "exec/hwaddr.h"
> #include
>
> --
> 2.39.0
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
{VALGRIND_LOGFILE}" $GDB "$QEMU_PROG" $QEMU_OPTIONS "$@" )
>
>
> Looks like this test does not work if the main machine
> of the selected QEMU binary does not support migration?
Why doesn't it support migration?
> Should we remove this test from the "auto" group?
>
> Anyway, QEMU should also not trigger an assertion, so this
> sounds like another bug?
Yeh; that's a weird failure.
(Alpha page size seems to be 8k from what I can tell; which should be
fine, if you're running on an x86 host)
Dave
> Thomas
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
* Daniel P. Berrangé (berra...@redhat.com) wrote:
> Signed-off-by: Daniel P. Berrangé
Yes, although I'm a little surprised this hasn't thrown up any warnings.
Reviewed-by: Dr. David Alan Gilbert
> ---
> tools/virtiofsd/fuse_log.c | 1 +
> tools/virtiofs
) | value);
> -qemu_file_set_error(migrate_get_current()->to_dst_file, ret);
> +if (migrate_get_current()->to_dst_file) {
> +qemu_file_set_error(migrate_get_current()->to_dst_file, ret);
> +}
> }
> vbasedev->migration->vm_running = running;
> trace_vfio_vmstate_change(vbasedev->name, running, RunState_str(state),
> --
> 2.26.3
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
* Avihai Horon (avih...@nvidia.com) wrote:
>
> On 23/11/2022 20:59, Dr. David Alan Gilbert wrote:
> > External email: Use caution opening links or attachments
> >
> >
> > * Avihai Horon (avih...@nvidia.com) wrote:
> >
> >
> >
> >
t;
>
> # migration.c
> vfio_migration_probe(const char *name) " (%s)"
> +vfio_migration_set_state(const char *name, const char *state) " (%s) state
> %s"
> vfio_migration_v1_set_state(const char *name, uint32_t state) " (%s) state
> %d"
>
ot; into "postcopy" just because in the
> > logic of migration_iteration_run() we don't actually distinguish
> > "compat" and "post". The logic only depends on "total" and "pre".
> >
> > So, if we want to combine "compat" into "post", we should redefine
> > "post" in the comment in include/migration/register.h, something like
> > this:
> >
> > - res_precopy is for data which MUST be migrated in precopy
> > phase or in stopped state, in other words - before target
> > vm start
> >
> > - res_postcopy is for all data except for declared in res_precopy.
> > res_postcopy data CAN be migrated in postcopy, i.e. after target
> > vm start.
> >
> >
> You are right, the definition of res_postcopy should be changed.
>
> Yet, I am not sure if this patch really makes things more clear/simple.
> Juan, what do you think?
>
> Thanks!
> > > } else {
> > > - *res_precopy_only += remaining_size;
> > > + *res_precopy += remaining_size;
> > > }
> > > }
> > >
> >
> >
> > --
> > Best regards,
> > Vladimir
> >
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
* Juan Quintela (quint...@redhat.com) wrote:
> Signed-off-by: Juan Quintela
Reviewed-by: Dr. David Alan Gilbert
> ---
> migration/migration.c | 24
> 1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/migration/migration.c b/mig
= vbasedev->migration;
> @@ -511,7 +511,7 @@ static int vfio_save_iterate(QEMUFile *f, void *opaque)
> }
>
> /*
> - * Reset pending_bytes as .save_live_pending is not called during savevm
> or
> + * Reset pending_bytes as .state_pending_* is not called during sav
> The only "device" that implements different functions for _estimate()
> and _exact() is ram.
>
> Signed-off-by: Juan Quintela
Yeh I think that's OK; I'm a little worried whether you end up calling
the two functions in migration_iteration_run a lot, but still
R
* Juan Quintela (quint...@redhat.com) wrote:
> So remove it everywhere.
>
> Signed-off-by: Juan Quintela
Reviewed-by: Dr. David Alan Gilbert
> ---
> include/migration/register.h | 6 ++
> migration/savevm.h | 2 +-
> hw/s390x/s390-stattrib.c
;"
> migrate_fd_error(const char *error_desc) "error=%s"
> migrate_fd_cancel(void) ""
> migrate_handle_rp_req_pages(const char *rbname, size_t start, size_t len)
> "in %s at 0x%zx len 0x%zx"
> -migrate_pending(uint64_t size, uint64_t max, uint64_t pre, uint64_t compat,
> uint64_t post) "pending size %" PRIu64 " max %" PRIu64 " (pre = %" PRIu64 "
> compat=%" PRIu64 " post=%" PRIu64 ")"
> +migrate_pending(uint64_t size, uint64_t max, uint64_t pre, uint64_t post)
> "pending size %" PRIu64 " max %" PRIu64 " (pre = %" PRIu64 " post=%" PRIu64
> ")"
> migrate_send_rp_message(int msg_type, uint16_t len) "%d: len %d"
> migrate_send_rp_recv_bitmap(char *name, int64_t size) "block '%s' size
> 0x%"PRIi64
> migration_completion_file_err(void) ""
> --
> 2.37.2
>
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
ds on behalf of the main loop, which means pool->head could be
> > modified (iothread calls thread_pool_submit_aio) while being read by the
> > main loop (another worker thread schedules thread_pool_completion_bh).
> >
> > What are the performance implications? I mean, if the aiocontext lock in
> > the bh is actually useful and the bh really has to wait to take it,
> > being taken in much more places throughout the block layer won't be
> > better than extending the poll->lock I guess.
>
> thread_pool_submit_aio() is missing documentation on how it is supposed
> to be called.
>
> Taking pool->lock is conservative and fine in the short-term.
>
> In the longer term we need to clarify how thread_pool_submit_aio() is
> supposed to be used and remove locking to protect pool->head if
> possible.
>
> A bunch of the event loop APIs are thread-safe (aio_set_fd_handler(),
> qemu_schedule_bh(), etc) so it's somewhat natural to make
> thread_pool_submit_aio() thread-safe too. However, it would be nice to
> avoid synchronization and existing callers mostly call it from the same
> event loop thread that runs the BH and we can avoid locking in that
> case.
>
> Stefan
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
r inadvertent wraparound is
> possible.
>
> Fixes: 93676c88 ("nbd: Don't send oversize strings", v4.2.0)
> Reported-by: Dr. David Alan Gilbert
> Signed-off-by: Eric Blake
Reviewed-by: Dr. David Alan Gilbert
> ---
>
> v2: update subject line and commit mess
SIZE. Still, it doesn't hurt to be more explicit in
> how we write our assertion that we are aware that no wraparound is
> possible.
>
> Fixes: 93676c88 ("nbd: Don't send oversize strings", v4.2.0)
> Reported-by: Dr. David Alan Gilbert
> Signed-off-by: Eric Blake
80
> > #9 blk_co_pwritev (blk=0x5595f91db8c0, offset=,
> > bytes=, qiov=qiov@entry=0x5595f9030d58, flags=flags@entry=0)
> > at ../block/block-backend.c:1391
> > #10 0x5595f8328a59 in mirror_read_complete (ret=0, op=0x5595f9030d50)
> > at ../block/mirror.c:260
> > #11 mirror_co_read (opaque=0x5595f9030d50) at ../block/mirror.c:400
> > #12 0x5595f843a39b in coroutine_trampoline (i0=,
> > i1=) at ../util/coroutine-ucontext.c:177
> > #13 0x7f33948b8d40 in ?? () from /lib/x86_64-linux-gnu/libc.so.6
> > #14 0x7f324a3ea6e0 in ?? ()
> > #15 0x in ?? ()
>
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
-git a/tests/unit/test-image-locking.c b/tests/unit/test-image-locking.c
> index ba057bd66c..795c602ff6 100644
> --- a/tests/unit/test-image-locking.c
> +++ b/tests/unit/test-image-locking.c
> @@ -76,7 +76,7 @@ static void check_locked_bytes(int fd, uint64_t perm_locks,
> static void test_image_locking_basic(void)
> {
> BlockBackend *blk1, *blk2, *blk3;
> -char img_path[] = "/tmp/qtest.XX";
> +char *img_path = g_strdup_printf("%s/qtest.XX", g_get_tmp_dir());
> uint64_t perm, shared_perm;
>
> int fd = mkstemp(img_path);
> @@ -112,12 +112,13 @@ static void test_image_locking_basic(void)
> blk_unref(blk3);
> close(fd);
> unlink(img_path);
> +g_free(img_path);
> }
>
> static void test_set_perm_abort(void)
> {
> BlockBackend *blk1, *blk2;
> -char img_path[] = "/tmp/qtest.XX";
> +char *img_path = g_strdup_printf("%s/qtest.XX", g_get_tmp_dir());
> uint64_t perm, shared_perm;
> int r;
> int fd = mkstemp(img_path);
> @@ -140,6 +141,7 @@ static void test_set_perm_abort(void)
> check_locked_bytes(fd, perm, ~shared_perm);
> blk_unref(blk1);
> blk_unref(blk2);
> +g_free(img_path);
> }
>
> int main(int argc, char **argv)
> diff --git a/tests/unit/test-qga.c b/tests/unit/test-qga.c
> index a05a4628ed..b73d231cd5 100644
> --- a/tests/unit/test-qga.c
> +++ b/tests/unit/test-qga.c
> @@ -59,7 +59,7 @@ fixture_setup(TestFixture *fixture, gconstpointer data,
> gchar **envp)
>
> fixture->loop = g_main_loop_new(NULL, FALSE);
>
> -fixture->test_dir = g_strdup("/tmp/qgatest.XX");
> +fixture->test_dir = g_strdup_printf("%s/qgatest.XX",
> g_get_tmp_dir());
> g_assert_nonnull(g_mkdtemp(fixture->test_dir));
>
> path = g_build_filename(fixture->test_dir, "sock", NULL);
> diff --git a/tests/vhost-user-bridge.c b/tests/vhost-user-bridge.c
> index 9b1dab2f28..0da6a6157f 100644
> --- a/tests/vhost-user-bridge.c
> +++ b/tests/vhost-user-bridge.c
> @@ -631,7 +631,7 @@ static void *notifier_thread(void *arg)
> static void
> vubr_host_notifier_setup(VubrDev *dev)
> {
> -char template[] = "/tmp/vubr-XX";
> +char *template = g_strdup_printf("%s/vubr-XX", g_get_tmp_dir());
> pthread_t thread;
> size_t length;
> void *addr;
> @@ -640,6 +640,7 @@ vubr_host_notifier_setup(VubrDev *dev)
> length = qemu_real_host_page_size() * VHOST_USER_BRIDGE_MAX_QUEUES;
>
> fd = mkstemp(template);
> +g_free(template);
> if (fd < 0) {
> vubr_die("mkstemp()");
> }
> --
> 2.34.1
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
**errp)
> }
> }
>
> -ret = migrate_caps_check(cap_list, head, errp);
> +migrate_caps_check(cap_list, head, errp);
>
> /* It works with head == NULL */
> qapi_free_MigrationCapabilityStatusList(head);
> -
> -return ret;
> }
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
> migration: Define BLK_MIG_BLOCK_SIZE as unsigned long long
>
> Ping? This series got reviewed but doesn't seem to have
> made it into a migration pullreq yet.
Queued
> thanks
> -- PMM
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
* Peter Maydell (peter.mayd...@linaro.org) wrote:
> On Thu, 21 Jul 2022 at 13:07, Dr. David Alan Gilbert
> wrote:
> >
> > * Peter Maydell (peter.mayd...@linaro.org) wrote:
> > > When we use BLK_MIG_BLOCK_SIZE in expressions like
> > > block_mig_state.
to bdrv_create_dirty_bitmap that
takes a uint32_t ?
Dave
> #define BDRV_SECTORS_PER_DIRTY_CHUNK (BLK_MIG_BLOCK_SIZE >> BDRV_SECTOR_BITS)
>
> #define BLK_MIG_FLAG_DEVICE_BLOCK 0x01
> --
> 2.25.1
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
to return is within range.
>
> Resolves: Coverity CID 1487239, 1487254
> Signed-off-by: Peter Maydell
Reviewed-by: Dr. David Alan Gilbert
> ---
> migration/migration.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/migration/migration.c b/migration/migration.c
* Daniel P. Berrangé (berra...@redhat.com) wrote:
> This directly implements the get_buffer logic using QIOChannel APIs.
>
> Reviewed-by: Dr. David Alan Gilbert
> Signed-off-by: Daniel P. Berrangé
Coverity is pointing out a fun deadcode path from this:
> diff --git a/migratio
07863
> https://gitlab.com/qemu-project/qemu/-/jobs/2622407862
> https://gitlab.com/qemu-project/qemu/-/jobs/2622407860
> https://gitlab.com/qemu-project/qemu/-/jobs/2622407811
>
> ../io/channel-socket.c:589:9: error: implicit declaration of function
> 'g_assert_unreachable' is invalid in C99
> [-Werror,-Wimplicit-function-declaration]
> g_assert_unreachable();
> ^
Again, non Linux; and should be g_assert_not_reached
I'll fix this up.
Dave
>
> r~
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
ile the former is specifically for accounting in thue rate
> limit calculations. The new name give better guidance on its usage.
>
> Signed-off-by: Daniel P. Berrangé
Reviewed-by: Dr. David Alan Gilbert
> ---
> migration/multifd.c | 4 ++--
> migration/qemu-file.c
From: "Dr. David Alan Gilbert"
'namespace' is misspelled in a bunch of places.
Signed-off-by: Dr. David Alan Gilbert
---
hw/9pfs/9p-xattr-user.c | 8
hw/acpi/nvdimm.c| 2 +-
hw/nvme/ctrl.c | 2 +-
3 files changed, 6 insertions(+), 6 deletions(-)
From: "Dr. David Alan Gilbert"
Inspired by Julia Lawall's fixing of Linux
kernel comments, I looked at qemu, although I did it manually.
Signed-off-by: Dr. David Alan Gilbert
---
hw/intc/openpic.c| 2 +-
hw/net/imx_fec.c | 2 +-
hw
From: "Dr. David Alan Gilbert"
I've sent the 3 char set last month, but have updated
it a little; I cleaned up a comment style that was already
broken so checkpatch is happy.
The 'namesapce' is a new patch; it's amazing how many places
make the same typo!
D
* Daniel P. Berrangé (berra...@redhat.com) wrote:
> This directly implements the get_return_path logic using QIOChannel APIs.
>
> Signed-off-by: Daniel P. Berrangé
Reviewed-by: Dr. David Alan Gilbert
> ---
> migration/qemu-file-channel.c | 16
> mig
probably belongs in an earlier patch.
Other than that,
Reviewed-by: Dr. David Alan Gilbert
> ---
> migration/channel.c | 4 +--
> migration/colo.c | 5 ++--
> migration/meson.build | 1 -
> migration/migration.c | 7 ++---
it(ioc, G_IO_OUT);
> -}
> - continue;
I wondered where that code went, but it turns out it's already copied
into qio_channel_writev_full_all, so:
Reviewed-by: Dr. David Alan Gilbert
> -}
> -if (len < 0) {
> -done = -EIO;
> -
* Daniel P. Berrangé (berra...@redhat.com) wrote:
> Signed-off-by: Daniel P. Berrangé
Reviewed-by: Dr. David Alan Gilbert
> ---
> migration/qemu-file.h | 4
> 1 file changed, 4 deletions(-)
>
> diff --git a/migration/qemu-file.h b/migration/qemu-file.h
> index 0
* Daniel P. Berrangé (berra...@redhat.com) wrote:
> This directly implements the close logic using QIOChannel APIs.
>
> Signed-off-by: Daniel P. Berrangé
Reviewed-by: Dr. David Alan Gilbert
> ---
> migration/qemu-file-channel.c | 12
> migration/qemu-f
NOSYS;
> }
> - ret = f->ops->shut_down(f->ioc, true, true, NULL);
> +
> +if (qio_channel_shutdown(f->ioc, QIO_CHANNEL_SHUTDOWN_BOTH, NULL) < 0) {
> +ret = -EIO;
> +}
OK, so this is following the code you're flattening; so:
Revi
(f->ioc, f->buf + pending, f->total_transferred,
> - IO_BUF_SIZE - pending, &local_error);
> +do {
> +len = qio_channel_read(f->ioc,
Yes, I think that's OK - not that 'len' is an int where 'ret'
was a ssize_t;
-off-by: Daniel P. Berrangé
Reviewed-by: Dr. David Alan Gilbert
> ---
> migration/qemu-file-channel.c | 4 ++--
> migration/qemu-file.c | 18 --
> migration/qemu-file.h | 3 ++-
> 3 files changed, 20 insertions(+), 5 deletions(-)
>
> diff -
* Daniel P. Berrangé (berra...@redhat.com) wrote:
> This directly implements the set_blocking logic using QIOChannel APIs.
>
> Signed-off-by: Daniel P. Berrangé
Reviewed-by: Dr. David Alan Gilbert
> ---
> migration/qemu-file-channel.c | 14 --
> migration/qemu-f
* Daniel P. Berrangé (berra...@redhat.com) wrote:
> On Thu, Jun 09, 2022 at 05:12:41PM +0100, Dr. David Alan Gilbert wrote:
> > * Daniel P. Berrangé (berra...@redhat.com) wrote:
> > > This directly implements the shutdown logic using QIOChannel APIs.
> > >
> > &g
* Get the ioc object for the file, without incrementing
> + * the reference count.
> + *
> + * Returns: the ioc object
> */
> QIOChannel *qemu_file_get_ioc(QEMUFile *file)
> {
> - return file->has_ioc ? QIO_CHANNEL(file->opaque) : NULL;
> +return file->ioc;
> }
> diff --git a/migration/qemu-file.h b/migration/qemu-file.h
> index 6310071f90..0458b1d3b6 100644
> --- a/migration/qemu-file.h
> +++ b/migration/qemu-file.h
> @@ -118,7 +118,7 @@ typedef struct QEMUFileHooks {
> QEMURamSaveFunc *save_page;
> } QEMUFileHooks;
>
> -QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops, bool has_ioc);
> +QEMUFile *qemu_fopen_ops(QIOChannel *ioc, const QEMUFileOps *ops);
> void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks);
> int qemu_get_fd(QEMUFile *f);
> int qemu_fclose(QEMUFile *f);
> --
> 2.36.1
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
* Daniel P. Berrangé (berra...@redhat.com) wrote:
> The only user of the hooks is RDMA which provides a QIOChannel backed
> impl of QEMUFile. It can thus use the qemu_file_get_ioc() method.
>
> Signed-off-by: Daniel P. Berrangé
Reviewed-by: Dr. David Alan Gilbert
> ---
&g
* Daniel P. Berrangé (berra...@redhat.com) wrote:
> With this change, all QEMUFile usage is backed by QIOChannel at
> last.
>
> Signed-off-by: Daniel P. Berrangé
Reviewed-by: Dr. David Alan Gilbert
> ---
> migration/savevm.c | 42 --
ingle fd.
(But now doesn't make any sense any more with these changes either)
Dave
> With regards,
> Daniel
> --
> |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
> What this method is actually used for is to report on the number of
> bytes that have been transferred out of band from the main I/O methods.
> This new name better reflects this purpose.
>
> Signed-off-by: Daniel P. Berrangé
Reviewed-by: Dr. David Alan Gilbert
bytes processed. This switches to a new name that
> reflects the intended usage.
>
> Signed-off-by: Daniel P. Berrangé
Mostly,
Reviewed-by: Dr. David Alan Gilbert
Two nits below:
> ---
> migration/block.c | 10 +-
> migration/migration.c | 2 +-
> migration/qem
ng of data and applies to data queued, which need not have
> been transferred on the wire yet if a flush hasn't taken place.
>
> Signed-off-by: Daniel P. Berrangé
Reviewed-by: Dr. David Alan Gilbert
> ---
> migration/qemu-file.c | 30 +++---
>
tion(QEMUFile *f, size_t size)
> {
> -f->pos += size;
> +f->total_transferred += size;
> }
>
> /** Closes the file
> @@ -658,7 +659,7 @@ int qemu_get_byte(QEMUFile *f)
>
> int64_t qemu_ftell_fast(QEMUFile *f)
> {
> -int64_t ret = f->pos;
> +
* Daniel P. Berrangé (berra...@redhat.com) wrote:
> On Thu, Jun 09, 2022 at 10:51:27AM +0100, Dr. David Alan Gilbert wrote:
> > * Daniel P. Berrangé (berra...@redhat.com) wrote:
> > > This makes the field name align with the newly introduced method
> > > names in the
e
> has been unreachable since RDMA support was first introduced
> 9 years ago.
>
> Signed-off-by: Daniel P. Berrangé
Ah good; less rdma code!
Reviewed-by: Dr. David Alan Gilbert
> ---
> migration/rdma.c | 120 +--
> 1 file cha
* Daniel P. Berrangé (berra...@redhat.com) wrote:
> On Mon, May 16, 2022 at 12:30:15PM +0100, Dr. David Alan Gilbert wrote:
> > * Daniel P. Berrangé (berra...@redhat.com) wrote:
> > > On Mon, May 16, 2022 at 12:13:16PM +0100, Dr. David Alan Gilbert wrote:
> > &
* Daniel P. Berrangé (berra...@redhat.com) wrote:
> On Mon, May 16, 2022 at 12:13:16PM +0100, Dr. David Alan Gilbert wrote:
> > * Leonardo Bras (leob...@redhat.com) wrote:
> > > A build error happens in alpine CI when linux/errqueue.h is included
> > > in io/channel-soc
* Leonardo Bras (leob...@redhat.com) wrote:
> A build error happens in alpine CI when linux/errqueue.h is included
> in io/channel-socket.c, due to redefining of 'struct __kernel_timespec':
OK, looks to be same mechanism as other meson tests.
Reviewed-by: Dr. David Alan Gilbert
& defined(SO_ZEROCOPY))
> > +#define QEMU_MSG_ZEROCOPY
> > +#endif
> > +#endif
> >
> > #define SOCKET_MAX_FDS 16
> >
> > +
>
> This line can be dropped when merge.
Done
> > SocketAddress *
> > qio_channel_socket_get_local_address(Q
t; migration/rdma.c| 1 +
> migration/socket.c | 12 ++-
> monitor/hmp-cmds.c | 6 ++
> scsi/pr-manager-helper.c | 2 +-
> tests/unit/test-io-channel-socket.c | 1 +
> 23 files changed, 379 insertions(+), 44
gt; > +break;
> > +}
> > +
>
> Extra but useless newline.. but not worth a repost. Looks good here:
I removed that.
> Reviewed-by: Peter Xu
>
> Thanks,
>
> > +} else {
> > +/* Send header using the same writev call */
> > +p->iov[0].iov_len = p->packet_len;
> > +p->iov[0].iov_base = p->packet;
> > +}
> >
> > ret = qio_channel_writev_all(p->c, p->iov, p->iovs_num,
> > &local_err);
> > --
> > 2.36.0
> >
>
> --
> Peter Xu
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
lude/hw/virtio/vhost-vsock-common.h
> index d8b565b4da..076b7ab779 100644
> --- a/include/hw/virtio/vhost-vsock-common.h
> +++ b/include/hw/virtio/vhost-vsock-common.h
> @@ -44,7 +44,7 @@ int vhost_vsock_common_start(VirtIODevice *vdev);
> void vhost_vsock_common_stop(VirtIODevice *vdev);
> int vhost_vsock_common_pre_save(void *opaque);
> int vhost_vsock_common_post_load(void *opaque, int version_id);
> -void vhost_vsock_common_realize(VirtIODevice *vdev, const char *name);
> +void vhost_vsock_common_realize(VirtIODevice *vdev);
> void vhost_vsock_common_unrealize(VirtIODevice *vdev);
> uint64_t vhost_vsock_common_get_features(VirtIODevice *vdev, uint64_t
> features,
> Error **errp);
> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
> index 2179b75703..afff9e158e 100644
> --- a/include/hw/virtio/virtio-gpu.h
> +++ b/include/hw/virtio/virtio-gpu.h
> @@ -22,6 +22,7 @@
> #include "sysemu/vhost-user-backend.h"
>
> #include "standard-headers/linux/virtio_gpu.h"
> +#include "standard-headers/linux/virtio_ids.h"
> #include "qom/object.h"
>
> #define TYPE_VIRTIO_GPU_BASE "virtio-gpu-base"
> @@ -37,8 +38,6 @@ OBJECT_DECLARE_SIMPLE_TYPE(VirtIOGPUGL, VIRTIO_GPU_GL)
> #define TYPE_VHOST_USER_GPU "vhost-user-gpu"
> OBJECT_DECLARE_SIMPLE_TYPE(VhostUserGPU, VHOST_USER_GPU)
>
> -#define VIRTIO_ID_GPU 16
> -
> struct virtio_gpu_simple_resource {
> uint32_t resource_id;
> uint32_t width;
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index b31c4507f5..5d774684b1 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -165,8 +165,8 @@ struct VirtioDeviceClass {
> void virtio_instance_init_common(Object *proxy_obj, void *data,
> size_t vdev_size, const char *vdev_name);
>
> -void virtio_init(VirtIODevice *vdev, const char *name,
> - uint16_t device_id, size_t config_size);
> +void virtio_init(VirtIODevice *vdev, uint16_t device_id, size_t config_size);
> +
> void virtio_cleanup(VirtIODevice *vdev);
>
> void virtio_error(VirtIODevice *vdev, const char *fmt, ...) G_GNUC_PRINTF(2,
> 3);
> --
> 2.35.1
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
ot specified,
> wait for tray to open and try again.
>
> This situation is could be resolved 'blockdev-open-tray' by passing
> flag 'force' inside. Thus is seems reasonable to add the same
> capability for 'blockdev-change-medium' too.
For HMP:
A
oexistence of Rust and C code within a
> subsystem
>
> - whether maintainers would be on board with adopting a completely different
> language, and who in the community has enough Rust experience to shepherd us
> through the learning experience
>
> The first two questions have answers in the other message if s/Rust/C++/,
> and as to the last I think we're already further in the discussion.
>
> Thanks,
>
> Paolo
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
t
> start, ram_addr_t len)
> }
>
> struct RAMSrcPageRequest *new_entry =
> - g_malloc0(sizeof(struct RAMSrcPageRequest));
> +g_new0(struct RAMSrcPageRequest, 1);
> new_entry->rb = ramblock;
> new_entry->offset = start;
> new_entry->len = len;
For migration:
Acked-by: Dr. David Alan Gilbert
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
there's no need for the extra level; however
you could do it all in one HMP command I think:
info virtio -- list all available virtio devices
info virtio path -- Display status of a given virtio device
info virtio path queue -- Display status of a given virtio queue
info virtio -h path queue -- Display status of a given vhost queue
info virtio -e path queue [index] -- Display element of a given virtio
queue
It wouldn't need to change the qmp commands underneath unless it made
sense.
Dave
> Jonah
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
c0ce4 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py
> @@ -254,9 +254,11 @@ def doc_type(self):
>
> def check(self, schema):
> QAPISchemaEntity.check(self, schema)
> -if 'deprecated' in [f.name for f in self.features]:
> -raise QAPISemError(
> -self.info, "feature 'deprecated' is not supported for types")
> +for feat in self.features:
> +if feat.is_special():
> +raise QAPISemError(
> +self.info,
> +f"feature '{feat.name}' is not supported for types")
>
> def describe(self):
> assert self.meta
> @@ -726,7 +728,7 @@ class QAPISchemaFeature(QAPISchemaMember):
> role = 'feature'
>
> def is_special(self):
> -return self.name in ('deprecated')
> +return self.name in ('deprecated', 'unstable')
>
>
> class QAPISchemaObjectTypeMember(QAPISchemaMember):
> --
> 2.31.1
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
of the old drawbacks,
> but adds a new one: While using x- makes it very obvious for a human
> user that this is an unstable feature, a feature flag in the schema will
> almost certainly go unnoticed in manual use.
Agreed, I'd keep the x- as well.
Having said that, the x- represents a few different things (that we
don't currently distinguish):
- experimental
- for internal use
- for debugging/human use
Dave
> Kevin
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
t;);
> -goto out;
> +return;
> }
>
> /* Make a guess for the block size, we'll fix it when the guest sends.
> @@ -2661,9 +2659,6 @@ static void scsi_block_realize(SCSIDevice *dev, Error
> **errp)
>
> scsi_realize(&s->qdev, errp);
> scsi_generic_read_device_inquiry(&s->qdev);
> -
> -out:
> -aio_context_release(ctx);
> }
>
> typedef struct SCSIBlockReq {
> --
> 2.31.1
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
* Michael S. Tsirkin (m...@redhat.com) wrote:
> On Wed, Oct 06, 2021 at 02:29:37PM +0100, Dr. David Alan Gilbert wrote:
> > * Michael S. Tsirkin (m...@redhat.com) wrote:
> > > On Wed, Oct 06, 2021 at 09:43:52AM +0100, Dr. David Alan Gilbert wrote:
> > > > * Micha
* Michael S. Tsirkin (m...@redhat.com) wrote:
> On Wed, Oct 06, 2021 at 09:43:52AM +0100, Dr. David Alan Gilbert wrote:
> > * Michael S. Tsirkin (m...@redhat.com) wrote:
> > > On Wed, Oct 06, 2021 at 09:28:50AM +0100, Dr. David Alan Gilbert wrote:
> > > > To me it fee
* Michael S. Tsirkin (m...@redhat.com) wrote:
> On Wed, Oct 06, 2021 at 09:28:50AM +0100, Dr. David Alan Gilbert wrote:
> > To me it feels the same as the distinction between vhost-kernel and qemu
> > backended virtio that we get in net and others - in principal it'
* Michael S. Tsirkin (m...@redhat.com) wrote:
> On Wed, Oct 06, 2021 at 09:09:30AM +0100, Dr. David Alan Gilbert wrote:
> > * Michael S. Tsirkin (m...@redhat.com) wrote:
> > > On Tue, Oct 05, 2021 at 12:10:08PM -0400, Eduardo Habkost wrote:
> > > > On Tue, Oct 05
* Michael S. Tsirkin (m...@redhat.com) wrote:
> On Tue, Oct 05, 2021 at 12:10:08PM -0400, Eduardo Habkost wrote:
> > On Tue, Oct 05, 2021 at 03:01:05PM +0100, Dr. David Alan Gilbert wrote:
> > > * Michael S. Tsirkin (m...@redhat.com) wrote:
> > > > On Tue, Oct 05, 2
class
> > properties.
> >
> > Thanks,
> > Roman.
>
> So the question is how to make vmsd depend on machine type.
> CC Eduardo who poked at this kind of compat stuff recently,
> paolo who looked at qom things most recently and dgilbert
> for advice on migration.
I don't think I've seen anyone change vmsd name dependent on machine
type; making fields appear/disappear is easy - that just ends up as a
property on the device that's checked; I guess if that property is
global (rather than per instance) then you can check it in
vhost_user_blk_class_init and swing the dc->vmsd pointer?
Dave
> --
> MST
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
n missing it
from the start.
Reviewed-by: Dr. David Alan Gilbert
> ---
> migration/migration.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 041b8451a6..215d5281f2 100644
> --- a/migration/migration.c
>
* Peter Xu (pet...@redhat.com) wrote:
> 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
* 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 of after
> > > > each
> > > > iteration? would that be enough?
&
t; __zerocopy_sg_from_iter() -> iov_iter_get_pages().
>
> It happens probably here:
>
> E.g
>
> __ip_append_data()
> msg_zerocopy_realloc()
> mm_account_pinned_pages()
When do they get uncounted? i.e. is it just the data that's in flight
that's marked as pinned?
Dave
> Thanks
>
> >
> > 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
> >
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
to
> assume the worst case possible, given the process ulimits.
We already have the same problem for RDMA;
(Although it has some options for doing rolling locking in chunks
and recently there's code for use with new cards that don't need
locking).
I think the thing here is to offer zerocopy as an option; then people
can decide on the costs etc.
Dave
>
> > > Overall the memory locking needs look like a significant constraint that
> > > will affect ability to use this feature.
> > >
> >
> > I Agree, there is a lot to take in account.
> > In any way, those constraints could be checked at the same function as
> > the setsockopt() right?
>
> QEMU could possibly check its ulimits to see if it is possible, but it
> would be safer to require a migration capability to be explicitly set
> by the mgmt app to opt-in to zerocopy.
>
> > (Setting up configs to improve the chance of zerocopy would probably only
> > happen at/before qemu starting, right?)
>
> Usually, but you can change ulimits on the fly for a process. I'm just
> not sure of semantics if you reduce limits and existing usage exceeds
> the reduced value.
>
> Regards,
> Daniel
> --
> |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
res on kernel
> > side
> > and whether they can be enabled together (MSG_ZEROCOPY, mptcp, kTLS).
> >
> > From the safe side we may want to only enable one of them until we prove
> > they'll work together I guess..
>
> MPTCP is good when we're network limited f
ough, would ensure
> > > the semantics similar to current semantics, reducing risk of unexpected
> > > problems.
> > >
> >
> > What if we do the 'flush()' before we start post-copy, instead of after each
> > iteration? would that be enough?
>
> Possibly, yes. This really need David G's input since he understands
> the code in way more detail than me.
Hmm I'm not entirely sure why we have the sync after each iteration;
the case I can think of is if we're doing async sending, we could have
two versions of the same page in flight (one from each iteration) -
you'd want those to get there in the right order.
Dave
>
> Regards,
> Daniel
> --
> |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
* Gerd Hoffmann (kra...@redhat.com) wrote:
> On Thu, Jun 24, 2021 at 04:01:25PM +0100, Dr. David Alan Gilbert wrote:
> > * Gerd Hoffmann (kra...@redhat.com) wrote:
> > > This patch series adds support for module meta-data. Today this is
> > > eit
stem.rst | 17 +++
> docs/devel/index.rst| 1 +
> docs/devel/modules.rst | 5 +
> docs/devel/qom.rst | 8 ++
> hmp-commands-info.hx| 3 -
> hw/usb/meson.build | 10 +-
> meson.build | 82
* Gerd Hoffmann (kra...@redhat.com) wrote:
> One more little step towards modular tcg ...
>
> Signed-off-by: Gerd Hoffmann
Acked-by: Dr. David Alan Gilbert
> ---
> accel/tcg/hmp.c | 29 +
> monitor/misc.c| 18 -
* Gerd Hoffmann (kra...@redhat.com) wrote:
> Allow commands having a NULL cmd pointer, add a function to set the
> pointer later. Use case: allow modules implement hmp commands.
>
> Signed-off-by: Gerd Hoffmann
So this is OK, so
Acked-by: Dr. David Alan Gilbert
however, I can
* Daniel P. Berrangé (berra...@redhat.com) wrote:
> ping: anyone willing to review this
>
Reviewed-by: Dr. David Alan Gilbert
> On Wed, May 05, 2021 at 11:37:00AM +0100, Daniel P. Berrangé wrote:
> > The GDateTime APIs provided by GLib avoid portability pitfalls, such
> &
* Jonah Palmer (jonah.pal...@oracle.com) wrote:
>
> On 3/24/21 2:31 PM, Dr. David Alan Gilbert wrote:
> > * Jonah Palmer (jonah.pal...@oracle.com) wrote:
> > > From: Laurent Vivier
> > >
> >
> >
> > > --- /dev/nu
M);
>
> -if (!load_snapshot(name, NULL, false, NULL, &err) && saved_vm_running) {
> +if (load_snapshot(name, NULL, false, NULL, &err) && saved_vm_running) {
> vm_start();
> }
> hmp_handle_error(mon, err);
> --
> 2.30.2
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
;m reading htat right, that gets you it restarting it if
load_snapshot returns true, and it returns true if the load_snapshot
worked; i.e. if we were running and we succesfully load a snasphot
then we carry on running.
Other than the commit message, it makes sense tome:
Reviewed-by: Dr. David A
* Dr. David Alan Gilbert (dgilb...@redhat.com) wrote:
> * Dr. David Alan Gilbert (dgilb...@redhat.com) wrote:
> > * Daniel P. Berrangé (berra...@redhat.com) wrote:
> > > The GDateTime APIs provided by GLib avoid portability pitfalls, such
> > > as some platforms where &#
* Dr. David Alan Gilbert (dgilb...@redhat.com) wrote:
> * Daniel P. Berrangé (berra...@redhat.com) wrote:
> > The GDateTime APIs provided by GLib avoid portability pitfalls, such
> > as some platforms where 'struct timeval.tv_sec' field is still 'long'
> >
ateTime
> often results in simpler code too.
>
> Signed-off-by: Daniel P. Berrangé
Reviewed-by: Dr. David Alan Gilbert
> ---
> tools/virtiofsd/passthrough_ll.c | 25 -
> 1 file changed, 4 insertions(+), 21 deletions(-)
>
> diff --git a/tools/v
ateTime
> often results in simpler code too.
>
> Signed-off-by: Daniel P. Berrangé
Reviewed-by: Dr. David Alan Gilbert
> ---
> migration/savevm.c | 13 +
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/migration/savevm.c b/migration/s
, Dec 2019), dropped in 6.0 (commit
> > f862ddbb1, just weeks ago). pc-1.3 was a bit over seven years old when
> > we released 5.0. pc-2.4 will be six years old by the time we release
> > 6.1. Fair game?
>
> As a data-point, libvirt will be dropping support for (release, not the machine type) in the upcomming release. I'm not sure
> whether that justifies more deprecation though.
What qemu features will you then be relying on?
Dave
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
or debugging.
>
> Signed-off-by: Daniel P. Berrangé
I'd have had to admit to not having thought that would fail; the fact
we're debugging something where it does, suggests it's a good idea!
Reviewed-by: Dr. David Alan Gilbert
> ---
> block/file-posix.c | 2 ++
> bloc
* Daniel P. Berrangé (berra...@redhat.com) wrote:
> Signed-off-by: Daniel P. Berrangé
Reviewed-by: Dr. David Alan Gilbert
> ---
> block/file-posix.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/block/file-posix.c b/block/file-posix.c
> index 6aafeda44f..2538
1 - 100 of 513 matches
Mail list logo
