On Wed, Jul 10, 2024 at 3:19 PM Stefano Garzarella <sgarzare@xxxxxxxxxx> wrote: > > On Wed, Jul 10, 2024 at 11:08:48AM GMT, Jason Wang wrote: > >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. > > Sorry, I was not clear, I try to rephrase: > vdpa_sim_blk already supports a module parameter called `shared_backend` > introduced mainly to test live migration on the same host. When that > parameter is on, all the created devices share the same backend and so > we can easily do migration from one to another. > > With that parameter on or off, the device is always 128 MB, but now > that's a problem for testing, because it looks like XFS requires at > least 300 MB: https://issues.redhat.com/browse/RHEL-45951 > > That's why I sent this patch. > > When `shared_backend` is off (default), using the provisioning > parameters seems feasible to me, but when it's on, how do I deal with > it? > > Being a simulator, we can maybe make it so that only the first device > can change the size for example, or that all devices control the size, > but then we would have to handle the size change at runtime, which I > think is feasible, but it requires some work to send a notification of > configuration change, etc. Can we mandate the size parameter to be exactly the same as the first vDPA block simulator? > > > > >> > >> > > >> >> > >> >> 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. > > Yeah, maybe it's overkill and I don't have much time these days :-( > > I think the easiest way is to merge this patch, but I understand that a > module parameter is not very beautiful > > I'll try to see if I can implement provisioning parameters for > vdpa_sim_blk. Allowing capacity to be set only to the first device if > `shared_backend` is on. > > WDYT? Something like this. When there's no block simulator, allow an arbitrary capacity. When there is one, fail the creation when the capacity doesn't match. (when 'shared_backend' is on). Thanks > > Thanks, > Stefano >