On Wed, 2023-10-18 at 10:52 +0200, Kevin Wolf wrote: > Am 16.10.2023 um 17:19 hat David Woodhouse geschrieben: > > From: David Woodhouse <dwmw@xxxxxxxxxxxx> > > > > There's no need to force the user to assign a vdev. We can automatically > > assign one, starting at xvda and searching until we find the first disk > > name that's unused. > > > > This means we can now allow '-drive if=xen,file=xxx' to work without an > > explicit separate -driver argument, just like if=virtio. > > > > Signed-off-by: David Woodhouse <dwmw@xxxxxxxxxxxx> > > Actually, how does this play together with xen_config_dev_blk()? This > looks like it tried to implement a very similar thing (which is IF_XEN > even already existed). Ah yes, thanks for spotting that! I hadn't been looking at the xenfv > Are we now trying to attach each if=xen disk twice in the 'xenpv' > machine? Or if something prevents this, is it dead code. I suspect we end up creating them twice (and probably thus failing to lock the backing file). The old Xen PV device drivers in Qemu, before Paul's XenDevice conversion, were purely reactive. If they see nodes in XenStore describing a backend which they should implement, they'd do so. The code in xen_config_dev_blk() is *creating* those nodes for the frontend (guest) and backend (host/qemu) to see, which prompts the xen-block and xen-net drivers into action. In the new XenDevice model, we can just instantiate a device (e.g. qdev_new(TYPE_XEN_DISK_DEVICE) and its realize() method will create the corresponding XenStore nodes (with some help from the generic XenBus code for the common ones). The new XenDevice/XenBus model will *also* react to nodes which it discovers in XenStore, and instantiate the corresponding devices. So I suspect what'll happen is xen_config_dev_blk() will create the nodes starting at xvda (int vdev = 202 * 256 + 16 * disk->unit), and later my new code will create a new set starting from xvdb or wherever the next free one is. > I think in both cases, we would want to delete that function and remove > the loop that calls it in xen_init_pv()? Yes, I think so. The xen_config_dev_blk() one can just die, as can xen_config_dev_console() which isn't called from anywhere anyway. The vkbd/vfb ones can stay until/unless those drivers are converted to the new XenDevice model. And xen_config_dev_nic() probably just needs to loop doing the same as I did in pc_init_nic() in https://lore.kernel.org/qemu-devel/20231017182545.97973-5-dwmw2@xxxxxxxxxxxxx/T/#u + if (xen_bus && (!nd->model || g_str_equal(model, "xen-net-device"))) { + DeviceState *dev = qdev_new("xen-net-device"); + qdev_set_nic_properties(dev, nd); + qdev_realize_and_unref(dev, xen_bus, &error_fatal); ... but this just reinforces what I said there about "if qmp_device_add() can find the damn bus and do this right, why do we have to litter it through platform code?"
Attachment:
smime.p7s
Description: S/MIME cryptographic signature
