Re: [PULL 00/24] Net Patches

[Eugenio Perez Martin]

On Wed, Jul 20, 2022 at 5:40 AM Jason Wang <jasowang@redhat.com> wrote:
>
> On Wed, Jul 20, 2022 at 12:40 AM Peter Maydell <peter.maydell@linaro.org> 
> wrote:
> >
> > On Tue, 19 Jul 2022 at 14:17, Jason Wang <jasowang@redhat.com> wrote:
> > >
> > > The following changes since commit 
> > > f9d9fff72eed03acde97ea2d66104748dc474b2e:
> > >
> > >   Merge tag 'qemu-sparc-20220718' of https://github.com/mcayland/qemu 
> > > into staging (2022-07-19 09:57:13 +0100)
> > >
> > > are available in the git repository at:
> > >
> > >   https://github.com/jasowang/qemu.git tags/net-pull-request
> > >
> > > for you to fetch changes up to f8a9fd7b7ab6601b76e253bbcbfe952f8c1887ec:
> > >
> > >   net/colo.c: fix segmentation fault when packet is not parsed correctly 
> > > (2022-07-19 21:05:20 +0800)
> > >
> > > ----------------------------------------------------------------
> > >
> > > ----------------------------------------------------------------
> >
> > Fails to build, many platforms:
> >
> > eg
> > https://gitlab.com/qemu-project/qemu/-/jobs/2742242194
> >
> > libcommon.fa.p/net_vhost-vdpa.c.o: In function `vhost_vdpa_cvq_unmap_buf':
> > /builds/qemu-project/qemu/build/../net/vhost-vdpa.c:234: undefined
> > reference to `vhost_iova_tree_find_iova'
> > /builds/qemu-project/qemu/build/../net/vhost-vdpa.c:242: undefined
> > reference to `vhost_vdpa_dma_unmap'
> > /builds/qemu-project/qemu/build/../net/vhost-vdpa.c:247: undefined
> > reference to `vhost_iova_tree_remove'
> > libcommon.fa.p/net_vhost-vdpa.c.o: In function `vhost_vdpa_cleanup':
> > /builds/qemu-project/qemu/build/../net/vhost-vdpa.c:163: undefined
> > reference to `vhost_iova_tree_delete'
> > libcommon.fa.p/net_vhost-vdpa.c.o: In function `vhost_vdpa_cvq_map_buf':
> > /builds/qemu-project/qemu/build/../net/vhost-vdpa.c:285: undefined
> > reference to `vhost_iova_tree_map_alloc'
> > /builds/qemu-project/qemu/build/../net/vhost-vdpa.c:291: undefined
> > reference to `vhost_vdpa_dma_map'
> > /builds/qemu-project/qemu/build/../net/vhost-vdpa.c:300: undefined
> > reference to `vhost_iova_tree_remove'
> > libcommon.fa.p/net_vhost-vdpa.c.o: In function
> > `vhost_vdpa_net_handle_ctrl_avail':
> > /builds/qemu-project/qemu/build/../net/vhost-vdpa.c:445: undefined
> > reference to `vhost_svq_push_elem'
> > /builds/qemu-project/qemu/build/../net/vhost-vdpa.c:408: undefined
> > reference to `vhost_svq_add'
> > /builds/qemu-project/qemu/build/../net/vhost-vdpa.c:422: undefined
> > reference to `vhost_svq_poll'
> > /builds/qemu-project/qemu/build/../net/vhost-vdpa.c:434: undefined
> > reference to `virtio_net_handle_ctrl_iov'
> > libcommon.fa.p/net_vhost-vdpa.c.o: In function `net_init_vhost_vdpa':
> > /builds/qemu-project/qemu/build/../net/vhost-vdpa.c:611: undefined
> > reference to `vhost_iova_tree_new'
> > libcommon.fa.p/net_vhost-vdpa.c.o: In function
> > `glib_autoptr_cleanup_VhostIOVATree':
> > /builds/qemu-project/qemu/hw/virtio/vhost-iova-tree.h:20: undefined
> > reference to `vhost_iova_tree_delete'
> > collect2: error: ld returned 1 exit status
> > [2436/4108] Compiling C object
> > libqemu-s390x-softmmu.fa.p/meson-generated_.._qapi_qapi-introspect.c.o
> >
> >
> >
> > Presumably the conditions in the various meson.build files are
> > out of sync about when to build the net/vhost-vdpa.c code vs
> > the code that's implementing the functions it's trying to call.
> >
> > Specifically, the functions being called will only be present
> > if the target architecture has CONFIG_VIRTIO, which isn't
> > guaranteed, but we try to link the vhost-vdpa code in anyway.
>
> Right, this is probably because vhost-vdpa start to use virtio loigc (cvq).
>
> Eugenio, please fix this and I will send a new version of the pull request.
>

Is the right solution to build vhost-vdpa.c only if CONFIG_VIRTIO_NET
is defined?

It would make it equal as vhost_net_user in net/meson.buikd:
if have_vhost_net_user
  softmmu_ss.add(when: 'CONFIG_VIRTIO_NET', if_true:
files('vhost-user.c'), if_false: files('vhost-user-stub.c'))
  softmmu_ss.add(when: 'CONFIG_ALL', if_true: files('vhost-user-stub.c'))
endif

vs

if have_vhost_net_vdpa
  softmmu_ss.add(files('vhost-vdpa.c'))
endif

Or that would be considered as a regression? The other solution would
be to add vhost-shadow-virtqueue-stub.c and make these functions
return -ENOTSUP and similar.

Thanks!