Daniel P. Berrangé <berra...@redhat.com> wrote: > On Thu, Feb 02, 2023 at 02:39:05PM +0100, Juan Quintela wrote: >> Daniel P. Berrangé <berra...@redhat.com> wrote: >> > On Thu, Feb 02, 2023 at 01:51:28PM +0100, Juan Quintela wrote: >> >> Daniel P. Berrangé <berra...@redhat.com> wrote: >> >> > On Thu, Feb 02, 2023 at 01:22:12PM +0100, Juan Quintela wrote: >> >> >> "manish.mishra" <manish.mis...@nutanix.com> wrote: >> >> >> > MSG_PEEK peeks at the channel, The data is treated as unread and >> >> >> > the next read shall still return this data. This support is >> >> >> > currently added only for socket class. Extra parameter 'flags' >> >> >> > is added to io_readv calls to pass extra read flags like MSG_PEEK. >> >> >> > >> >> >> > Reviewed-by: Peter Xu <pet...@redhat.com> >> >> >> > Reviewed-by: Daniel P. Berrange <berra...@redhat.com> >> >> >> > Suggested-by: Daniel P. Berrange <berra...@redhat.com> >> >> >> > Signed-off-by: manish.mishra <manish.mis...@nutanix.com> >> >> >> >> >> >> >> >> >> This change breaks RDMA migration. >> >> >> >> >> >> FAILED: libcommon.fa.p/migration_rdma.c.o >> >> >> cc -m64 -mcx16 -Ilibcommon.fa.p -I/usr/include/pixman-1 >> >> >> -I/usr/include/libpng16 -I/usr/include/spice-server >> >> >> -I/usr/include/spice-1 -I/usr/include/cacard -I/usr/include/glib-2.0 >> >> >> -I/usr/lib64/glib-2.0/include -I/usr/include/sysprof-4 >> >> >> -I/usr/include/nss3 -I/usr/include/nspr4 -I/usr/include/PCSC >> >> >> -I/usr/include/p11-kit-1 -I/usr/include/libusb-1.0 -I/usr/include/SDL2 >> >> >> -I/usr/include/libmount -I/usr/include/blkid >> >> >> -I/usr/include/gio-unix-2.0 -I/usr/include/slirp >> >> >> -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g >> >> >> -isystem /mnt/code/qemu/full/linux-headers -isystem linux-headers >> >> >> -iquote . -iquote /mnt/code/qemu/full -iquote >> >> >> /mnt/code/qemu/full/include -iquote /mnt/code/qemu/full/tcg/i386 >> >> >> -pthread -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE >> >> >> -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing >> >> >> -fno-common -fwrapv -Wundef -Wwrite-strings -Wmissing-prototypes >> >> >> -Wstrict-prototypes -Wredundant-decls -Wold-style-declaration >> >> >> -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k >> >> >> -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs >> >> >> -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 >> >> >> -Wmissing-format-attribute -Wno-missing-include-dirs >> >> >> -Wno-shift-negative-value -Wno-psabi -fstack-protector-strong -fPIE >> >> >> -D_REENTRANT -Wno-undef -DSTRUCT_IOVEC_DEFINED -MD -MQ >> >> >> libcommon.fa.p/migration_rdma.c.o -MF >> >> >> libcommon.fa.p/migration_rdma.c.o.d -o >> >> >> libcommon.fa.p/migration_rdma.c.o -c >> >> >> ../../../../mnt/code/qemu/full/migration/rdma.c >> >> >> ../../../../mnt/code/qemu/full/migration/rdma.c: In function >> >> >> ‘qio_channel_rdma_class_init’: >> >> >> ../../../../mnt/code/qemu/full/migration/rdma.c:4020:25: error: >> >> >> assignment to ‘ssize_t (*)(QIOChannel *, const struct iovec *, size_t, >> >> >> int **, size_t *, int, Error **)’ {aka ‘long int (*)(QIOChannel *, >> >> >> const struct iovec *, long unsigned int, int **, long unsigned int *, >> >> >> int, Error **)’} from incompatible pointer type ‘ssize_t >> >> >> (*)(QIOChannel *, const struct iovec *, size_t, int **, size_t *, >> >> >> Error **)’ {aka ‘long int (*)(QIOChannel *, const struct iovec *, long >> >> >> unsigned int, int **, long unsigned int *, Error **)’} >> >> >> [-Werror=incompatible-pointer-types] >> >> >> 4020 | ioc_klass->io_readv = qio_channel_rdma_readv; >> >> >> | ^ >> >> >> cc1: all warnings being treated as errors >> >> >> >> >> >> And I don't really know how to fix it, because the problem is that rdma >> >> >> don't use qio_channel_readv_full() at all. >> >> > >> >> > Likely qio_channel_rdma_readv just adds the 'int flags' param added. >> >> > It doesn't need to actually do anything with the flags as they are >> >> > checked before >> >> >> >> I can do that. That would fix the compilation issue. >> >> >> >> But will rdma work? Because it fakes a qio channel, so what is going to >> >> implement the MSG_PEEK functionality for it? It don't end calling >> >> recv() at all. >> > >> > It is no problem - the qio_channel_readv method changes in this patch >> > add: >> > >> > + if ((flags & QIO_CHANNEL_READ_FLAG_MSG_PEEK) && >> > + !qio_channel_has_feature(ioc, QIO_CHANNEL_FEATURE_READ_MSG_PEEK)) >> > { >> > + error_setg_errno(errp, EINVAL, >> > + "Channel does not support peek read"); >> > + return -1; >> > + } >> > >> > >> > so it is impossible for qio_channel_rdma_readv to be invoked with >> > flags having MSG_PEEK set, thus RDMA can ignore the whole concept. >> >> And as we require MSG_PEEK to do migration, we have lost RDMA migration >> in the process. >> >> The following patch on the series use this functionality to read the >> beggining of the streams in the channels. > > It guards that usage of MSG_PEEK with > > if (... && qio_channel_has_feature(ioc, > QIO_CHANNEL_FEATURE_READ_MSG_PEEK)) {
You win. They are back. Thanks very much for the explanation.