Re: [PATCH] vdpa_sim_blk: add `capacity` module parameter

[Jason Wang]

On Mon, Jul 8, 2024 at 4:15 PM Stefano Garzarella <sgarzare@xxxxxxxxxx> wrote:
>
> Hi Cindy, Jason,
>
> On Mon, Jul 08, 2024 at 03:59:34PM GMT, Jason Wang wrote:
> >On Mon, Jul 8, 2024 at 3:06 PM Cindy Lu <lulu@xxxxxxxxxx> wrote:
> >>
> >> On Fri, 5 Jul 2024 at 20:42, Stefano Garzarella <sgarzare@xxxxxxxxxx> wrote:
> >> >
> >> > On Fri, Jul 05, 2024 at 07:30:51AM GMT, Michael S. Tsirkin wrote:
> >> > >On Fri, Jul 05, 2024 at 01:28:21PM +0200, Stefano Garzarella wrote:
> >> > >> The vDPA block simulator always allocated a 128 MiB ram-disk, but some
> >> > >> filesystems (e.g. XFS) may require larger minimum sizes (see
> >> > >> https://issues.redhat.com/browse/RHEL-45951).
> >> > >>
> >> > >> So to allow us to test these filesystems, let's add a module parameter
> >> > >> to control the size of the simulated virtio-blk devices.
> >> > >> The value is mapped directly to the `capacity` field of the virtio-blk
> >> > >> configuration space, so it must be expressed in sector numbers of 512
> >> > >> bytes.
> >> > >>
> >> > >> The default value (0x40000) is the same as the previous value, so the
> >> > >> behavior without setting `capacity` remains unchanged.
> >> > >>
> >> > >> Before this patch or with this patch without setting `capacity`:
> >> > >>   $ modprobe vdpa-sim-blk
> >> > >>   $ vdpa dev add mgmtdev vdpasim_blk name blk0
> >> > >>   virtio_blk virtio6: 1/0/0 default/read/poll queues
> >> > >>   virtio_blk virtio6: [vdb] 262144 512-byte logical blocks (134 MB/128 MiB)
> >> > >>
> >> > >> After this patch:
> >> > >>   $ modprobe vdpa-sim-blk capacity=614400
> >> > >>   $ vdpa dev add mgmtdev vdpasim_blk name blk0
> >> > >>   virtio_blk virtio6: 1/0/0 default/read/poll queues
> >> > >>   virtio_blk virtio6: [vdb] 614400 512-byte logical blocks (315 MB/300 MiB)
> >> > >>
> >> > >> Signed-off-by: Stefano Garzarella <sgarzare@xxxxxxxxxx>
> >> > >
> >> > >What a hack. Cindy was working on adding control over config
> >> > >space, why can't that be used?
> >> >
> >> > If it can be used easily with virtio-blk device too, it will be great.
> >> > @Cindy do you plan to support that changes for a virtio-blk device too?
> >> >
> >> Hi Stefano
> >> I plan to add support to change the vdpa device's configuration after
> >> it is created.
> >
> >I think for Stefano's case, we can just implement it via provisioning
> >parameters?
>
> Yep, I think we don't need to change it after creation, but specifying
> while creating should be enough.
>
> So, IIUC we can already do it, implementing something similar to
> vdpasim_net_setup_config() to call during vdpasim_blk_dev_add(), right?

Right.

>
> What about when we have `shared_backend` set to true for the
> vdpa_sim_blk.ko? In this case the backend is supposed to be shared
> between all the devices to test live migration.

This seems to be another topic.

>
> Maybe we can just change the size of the shared ramdisk to be reflected
> to all devices.
>
> Suggestions?

Could we specify the path to tmpfs or others during provisioning
instead?  It seems more general (but more work).

>
> @Cindy do you want to work on this for blk as well?
> If you don't have time, I'll look at it when I can allocate some time.
>
> >
> >Thanks

Thanks

> >
> >> In the first step, I want to use the vdpa tool to add
> >> support for changing the MAC address for the network device. the next
> >> step will also add MTU settings etc
> >> here is the link
> >> https://lore.kernel.org/all/20240708064820.88955-1-lulu@xxxxxxxxxx/T/#t
> >>
>
> I'll take a look, thanks for ccing me!
>
> Stefano
>
> >> in the device part, the device needs to implement its function of
> >> int (*dev_set_attr)(struct vdpa_mgmt_dev *mdev, struct vdpa_device *dev,
> >>        const struct vdpa_dev_set_config *config);
> >> the configuration will be passed by struct vdpa_dev_set_config. I'm
> >> not sure if this kind of design is suitable for you? Really thanks and
> >> any comments are welcome
> >> thanks
> >> Cindy
> >>
> >>
> >> > In the mean time, for the simulator I thought that this change was fine.
> >> > It's just used for testing and debugging...
> >> >
> >> > My main question is how to use that when we have `shared_backend` set to
> >> > true, since we use that setting to test for example live migration. In
> >> > that case, how do we handle the size of the shared ramdisk between
> >> > devices?
> >> >
> >> > Thanks,
> >> > Stefano
> >> >
> >>
> >
>