qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: Leonardo Bras Soares Passos <leobras@redhat.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>,
	qemu-devel <qemu-devel@nongnu.org>,
	"Juan Quintela" <quintela@redhat.com>,
	"Peter Xu" <peterx@redhat.com>,
	"Stefan Hajnoczi" <stefanha@redhat.com>
Subject: Re: [PULL 00/16] migration queue
Date: Mon, 16 May 2022 09:18:59 +0100	[thread overview]
Message-ID: <YoII85MMsmXvyECW@work-vm> (raw)
In-Reply-To: <CAJ6HWG5wiod4gJJi2-bMkLPECiow4bC-ux-szWL-0=p3edpTvg@mail.gmail.com>

* Leonardo Bras Soares Passos (leobras@redhat.com) wrote:
> On Wed, May 11, 2022 at 5:55 AM Dr. David Alan Gilbert
> <dgilbert@redhat.com> wrote:
> >
> > * Leonardo Bras Soares Passos (leobras@redhat.com) wrote:
> > > From a previous thread:
> > >
> > > On Thu, Apr 28, 2022 at 1:20 PM Dr. David Alan Gilbert
> > > <dgilbert@redhat.com> wrote:
> > > >
> > > > Leo:
> > > >   Unfortunately this is failing a couple of CI tests; the MSG_ZEROCOPY
> > > > one I guess is the simpler one; I think Stefanha managed to find the
> > > > liburing fix for the __kernel_timespec case, but that looks like a bit
> > > > more fun!
> > > >
> > > > Dave
> > >
> > > I thought Stefanha had fixed this bug, and we were just waiting for a
> > > new alpine rootfs/image with that fixed.
> > > Is that correct?
> > >
> > > On Tue, May 10, 2022 at 7:43 AM Dr. David Alan Gilbert
> > > <dgilbert@redhat.com> wrote:
> > > >
> > > > * Daniel P. Berrangé (berrange@redhat.com) wrote:
> > > > > On Tue, May 10, 2022 at 10:58:30AM +0100, Dr. David Alan Gilbert wrote:
> > > [...]
> > > > >
> > > > > Yuk. That very much looks like a bug in liburing itself to me.
> > > > >
> > > > >
> > > > > We've exposed the latent bug by including linux/errqueue.h
> > > >
> > > > Yes, I think there was a thread after the 1st pull where Leo identified
> > > > the patch that fixed it; but it's not in that image.
> > >
> > > I only fixed the MSG_ZEROCOPY missing define bug, as I got that
> > > Stefanha had already fixed the issue in liburing/alpine.
> > >
> > > questions:
> > > - Has Stefanha really fixed that, and we are just waiting for a new
> > > image, or have I got that wrong?
> > > - How should I proceed with that?
> > >
> > > - If we proceed with fixing this up in alpine, will that require this
> > > patchset to be on pause until it's fixed there?
> >
> > It needs to pass in CI; so yes.
> >
> > > - If so, is there any suggestion on how to fix that in qemu code?
> > > (this header is needed because of SO_EE_* defines)
> >
> > I've not actually looked at the detail of the failure; but yes I think
> > we need a qemu workaround here.
> >
> > If there's no simple fix, then adding a test to meson.build to
> > conditionally disable liburing might be best; like the test code for
> > libcap_ng I guess (search in meson.build for libcap_ng.found  at around
> > line 540.
> 
> Hello Dave,
> 
> I solved this issue by doing this:
> 
> +linux_io_uring_test = '''
> +  #include <liburing.h>
> +  #include <linux/errqueue.h>
> +
> +  int main(void) { return 0; }'''
> +
>  linux_io_uring = not_found
>  if not get_option('linux_io_uring').auto() or have_block
>    linux_io_uring = dependency('liburing', version: '>=0.3',
>                                required: get_option('linux_io_uring'),
>                                method: 'pkg-config', kwargs: static_kwargs)
> +  if not cc.links(linux_io_uring_test)
> +    linux_io_uring = not_found
> +  endif
>  endif
> +
> 
> Seems to work fine in CI, and now Alpine does not fail anymore.
> (See pipeline https://gitlab.com/LeoBras/qemu/-/pipelines/538123933
> for reference)
> 
> I am not sure if this is the right thing to do, but I will be sending
> it as a full new patchset (v13), with the first patch being the one
> with the above change and the rest just carrying the recommended
> fixes.

Thanks! That looks promising.  I'll cook a new pull.

> I was also thinking I could instead send the single "fix" patch, and
> recommend adding it before my v12. If that is the correct approach for
> this case, please let me know so I can improve in the future. (I am
> trying to figure out what is simpler/best for maintainers)

Either way would be fine; the full series is probably better.

Dave

> Best regards,
> Leo
> 
> 
> 
> 
> 
> 
> 
> >
> > Dave
> >
> > > Thank you all!
> > >
> > > Best regards,
> > > Leo
> > >
> > > >
> > > > 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 / dgilbert@redhat.com / Manchester, UK
> > > >
> > >
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> >
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



  reply	other threads:[~2022-05-16  8:38 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-10  8:33 [PULL 00/16] migration queue Dr. David Alan Gilbert (git)
2022-05-10  8:33 ` [PULL 01/16] tests: fix encoding of IP addresses in x509 certs Dr. David Alan Gilbert (git)
2022-05-10  8:33 ` [PULL 02/16] tests: add more helper macros for creating TLS " Dr. David Alan Gilbert (git)
2022-05-10  8:33 ` [PULL 03/16] tests: add migration tests of TLS with PSK credentials Dr. David Alan Gilbert (git)
2022-05-10  8:33 ` [PULL 04/16] tests: add migration tests of TLS with x509 credentials Dr. David Alan Gilbert (git)
2022-05-10  8:33 ` [PULL 05/16] tests: convert XBZRLE migration test to use common helper Dr. David Alan Gilbert (git)
2022-05-10  8:33 ` [PULL 06/16] tests: convert multifd migration tests " Dr. David Alan Gilbert (git)
2022-05-10  8:33 ` [PULL 07/16] tests: add multifd migration tests of TLS with PSK credentials Dr. David Alan Gilbert (git)
2022-05-10  8:33 ` [PULL 08/16] tests: add multifd migration tests of TLS with x509 credentials Dr. David Alan Gilbert (git)
2022-05-10  8:33 ` [PULL 09/16] tests: ensure migration status isn't reported as failed Dr. David Alan Gilbert (git)
2022-05-10  8:33 ` [PULL 10/16] QIOChannel: Add flags on io_writev and introduce io_flush callback Dr. David Alan Gilbert (git)
2022-05-10  8:33 ` [PULL 11/16] QIOChannelSocket: Implement io_writev zero copy flag & io_flush for CONFIG_LINUX Dr. David Alan Gilbert (git)
2022-05-10  8:33 ` [PULL 12/16] migration: Add zero-copy-send parameter for QMP/HMP for Linux Dr. David Alan Gilbert (git)
2022-05-10  8:33 ` [PULL 13/16] migration: Add migrate_use_tls() helper Dr. David Alan Gilbert (git)
2022-05-10  8:33 ` [PULL 14/16] multifd: multifd_send_sync_main now returns negative on error Dr. David Alan Gilbert (git)
2022-05-10  8:33 ` [PULL 15/16] multifd: Send header packet without flags if zero-copy-send is enabled Dr. David Alan Gilbert (git)
2022-05-10  8:33 ` [PULL 16/16] multifd: Implement zero copy write in multifd migration (multifd-zero-copy) Dr. David Alan Gilbert (git)
2022-05-10  9:58 ` [PULL 00/16] migration queue Dr. David Alan Gilbert
2022-05-10 10:19   ` Daniel P. Berrangé
2022-05-10 10:43     ` Dr. David Alan Gilbert
2022-05-11  3:40       ` Leonardo Bras Soares Passos
2022-05-11  8:55         ` Dr. David Alan Gilbert
2022-05-13  6:28           ` Leonardo Bras Soares Passos
2022-05-16  8:18             ` Dr. David Alan Gilbert [this message]
2022-05-16  8:51         ` Stefan Hajnoczi
2022-05-16  9:40           ` Daniel P. Berrangé
2022-05-16 10:09             ` Daniel P. Berrangé
2022-05-16 15:30               ` Stefan Hajnoczi
  -- strict thread matches above, loose matches on Subject: below --
2022-05-09 15:02 Dr. David Alan Gilbert (git)
2020-10-26 16:19 Dr. David Alan Gilbert (git)
2020-10-26 16:39 ` no-reply
2020-10-26 16:52   ` Dr. David Alan Gilbert
2020-10-27 11:28 ` Peter Maydell
2020-10-31 16:12 ` Christian Schoenebeck
2020-10-31 17:26   ` Peter Maydell
2020-10-31 17:46     ` Peter Xu
2020-10-31 19:10       ` Christian Schoenebeck
2020-11-01 10:17         ` Christian Schoenebeck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YoII85MMsmXvyECW@work-vm \
    --to=dgilbert@redhat.com \
    --cc=berrange@redhat.com \
    --cc=leobras@redhat.com \
    --cc=peterx@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).