Re: [PATCH vhost v1 2/4] virtio: vring_create_virtqueue: pass struct instead of multi parameters

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 8 Mar 2024 12:19:21 +0200 (EET), =?utf-8?q?Ilpo_J=C3=A4rvinen?= <ilpo.jarvinen@xxxxxxxxxxxxxxx> wrote:
> On Wed, 6 Mar 2024, Xuan Zhuo wrote:
>
> > Now, we pass multi parameters to vring_create_virtqueue. These parameters
> > may from transport or from driver.
> >
> > vring_create_virtqueue is called by many places.
> > Every time, we try to add a new parameter, that is difficult.
> >
> > If parameters from the driver, that should directly be passed to vring.
> > Then the vring can access the config from driver directly.
> >
> > If parameters from the transport, we squish the parameters to a
> > structure. That will be helpful to add new parameter.
> >
> > Because the virtio_uml.c changes the name, so change the "names" inside
> > the virtio_vq_config from "const char *const *names" to
> > "const char **names".
> >
> > Signed-off-by: Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx>
> > Acked-by: Johannes Berg <johannes@xxxxxxxxxxxxxxxx>
>
> > @@ -60,38 +61,25 @@ struct virtio_device;
> >  struct virtqueue;
> >  struct device;
> >
> > +struct vq_transport_config {
> > +	unsigned int num;
> > +	unsigned int vring_align;
> > +	bool weak_barriers;
> > +	bool may_reduce_num;
> > +	bool (*notify)(struct virtqueue *vq);
> > +	struct device *dma_dev;
> > +};
>
> kerneldoc is missing from this struct too.
>
> It would be generally helpful if you are proactive when somebody comments
> your series by checking if there are similar cases within your series,
> instead of waiting them to be pointed out for you specificly.

Sorry. I missed it.

Will fix in next version.

Thanks


>
> --
>  i.
>




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux