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

[Jason Wang]

On Tue, Jul 9, 2024 at 8:41 PM Stefano Garzarella <sgarzare@xxxxxxxxxx> wrote:
>
> On Tue, Jul 09, 2024 at 10:56:16AM GMT, Jason Wang wrote:
> >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.
>
> Yep, but really related. I think we need to handle that case when
> supporting the `capacity` setting.

Ok, so if I was not wrong, the goal is to test migration.

>
> >
> >>
> >> 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).
>
> Then it would almost become a real device, no longer just a simulator.
> It's enough work, though, as you said, but at that point we'd just have
> to specify the backend file to use for the device.
>
> In that case what API would we need to use to allow the user to set the
> backend file?

Yes, I think we can allow some vendor specific provisioning parameters.

But not sure it's an overkill for the use case here. If others are
happy with the shared_backed. I'm fine.

Thanks

>
> Thanks,
> Stefano
>