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

Re: [PATCH V6 1/2] libxl: Add support for Virtio disk configuration


  • To: Oleksandr Tyshchenko <olekstysh@xxxxxxxxx>
  • From: Anthony PERARD <anthony.perard@xxxxxxxxxx>
  • Date: Tue, 14 Dec 2021 16:03:00 +0000
  • Authentication-results: esa3.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none
  • Cc: <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>, Wei Liu <wl@xxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Nick Rosbrook <rosbrookn@xxxxxxxxxxxx>, "Juergen Gross" <jgross@xxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, "Julien Grall" <julien@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>
  • Delivery-date: Tue, 14 Dec 2021 16:03:22 +0000
  • Ironport-data: A9a23:rZz4rKPRaBVDRmrvrR3QkcFynXyQoLVcMsEvi/4bfWQNrUom0mRWm 2NJDzuPa67cZDf0fdwkO4Ww8ENT68eExt8xQAto+SlhQUwRpJueD7x1DKtR0wB+jCHnZBg6h ynLQoCYdKjYdpJYz/uUGuCJQUNUjMlkfZKhTr6UUsxNbVU8En5400s/w7RRbrNA2rBVPSvc4 bsenOWHULOV82Yc3rU8sv/rRLtH5ZweiRtA1rAMTakjUGz2zhH5OKk3N6CpR0YUd6EPdgKMq 0Qv+5nilo/R109F5tpICd8XeGVSKlLZFVDmZna7x8FOK/WNz8A/+v9TCRYSVatYo3LYz9Es9 9kOjJ2XdCkTN/bhvf9GViANRkmSPYUekFPGCX22sMjVxEzaaXr8hf5pCSnaP6VBpLwxWzsXs 6VFdnZdNXhvhMrvqF6/YuBqmsQkKtitJI4Fs2ts5TrYEewnUdbIRKCiCdpwgm9o3pEQR662i 8wxOTwzcCieYSN2C3A8D7Y7u/iJqFvDSmgNwL6SjfVuuDWCpOBr65D2K8bccNGOQcRTn26bq 3jA8mC/BQsVXPSdxiCC6WmEnfLUkGXwX4d6PL+l8v9nhnWDy2pVDwcZPXOxrOOlkEe4V5RaI lYN5ys1haEo8QqgSdyVdyO/pHmIrxsNQe16Gucx6ByO4qfM6gPfDW8BJhZRZdpjuMIoSDgC0 l6Sg8ivFTFpqKeSS3+W6vGTtzzaES8RIGwZeT4fTSMK5tDipMc4iRenZtFnHa2uh9v5Awbs0 iuKpygzgbYUpcMT3qD99lfC6xq2oYPDVAky5QP/V2Oj4ARiaYXjbIutgXDE6d5QIYDfSUOO1 EXogODHsrpIV8vU0nXQHqNdR9lF+sppLhWb0A5uQqYttA2s3EONR4kLzRRlf0JQZ5NslSDSX GffvgZY5Zl2NXSsbLNqb4/ZN/nG3ZQMBvy+CKmKM4MmjoxZMVbeoXowPRL4M3XFyRB0yckC1 YGnndFA5JrwIYBu13KISugUytfHLQhulDqIFfgXI/lKuIdyhUJ5q59ZYDNijchjtctoRTk5F f4FaKNmLD0FD4XDjtH/q9J7ELzzBSFT6WrKg8JWbPWfBQFtBXssDfTcqZt4Jdc0w/4MzrqWo SDtMqO99LYZrSeYQeltQio8AI4DoL4l9S5rVcDSFQjAN4cfjXaHs/5EKspfkUgP/+1/1/9kJ 8TpiO3basmjvg/vomxHBbGk9dQKXE3y2WqmYnv9CBBiLsUIb1GYpbfZkv7HqXBm4tyf7pBl/ dVNF2rzHPI+euiVJJqMNa/0kQru5SN1dSAbdxKgH+S/sX7EqOBCQxEdRNdtSy3VARmclDacy SiMBhIU+bvEr4MvqYGbjqGYtYa5VeB5GxMCTWXc6L+3Mwjc/3aintAcALrZI2iFWTOm4rima MVU0+r4bK8NkmFVvtcuCL1s168/uYfi/ucI0gR+EXzXRF23Ebc8cGKe1MxCu/QVlL9UsAe7Q GyV/dxeNenbMc/pCgdJdgEkcv6CxbcfnTyLtaY5J0Dz5SlW+rubUBoNY0nQ2XIFdLYsadEr2 +YsvsIS+jeTsBtyP4bUlD1Q+kSNMmcED/ctuKYFDdK5kQEs0FxDP8DRU3ek/JGVZtxQGUA2O TvI1rHajrFRy0eeIXo+EX/BgbhUiZgU4U0YyVYDIxKCm8bfh+9x1xpUqGxlQgNQxxRB8uRyJ mk0aBElefTQp29l1JpZQmShOwBdHxnIqEX+xmwAmHDdU0T1BHfGK3cwOLrV8U0Um46GkuO3I F1MJL7ZbAvX
  • Ironport-hdrordr: A9a23:+K0bd6mX85AttVytnd4M7/PIxZDpDfIu3DAbv31ZSRFFG/Fxl6 iV/cjz8SWE7wr5OUtQ/exoV5PtfZqxz/FICMwqTNGftWrdyQ6VxeNZnOjfKlTbckWUnINgPO VbAspD4bXLfCFHZK3BgDVQfexP/OW6
  • Ironport-sdr: XKX/I7FhXgjC+hJsx+R7vVltiq4NG0+XVhJkwfOqvlHQjsORA2Hg4wtEYZHbgYt6SE0VCDjgul XvipK2YVWDZvvCSsw7exfl3v5/P2IMnpYbwVT7jvmrmV6cuEsebP2rBXcS60tYJAmYCm641ZGo 2hx9Z174wHhZW+EjaUpl8/LO+AMd9cnoozqJ+T9tU0dbGqNvQRQLS1T12O415GV3+WfPUQnO9n xca8OjK6JpqBscs+MNm0t3ZXv8tpNUJqN6IQVG4RF2M4ObPHuYiWkrmPINVGm9bhcH3g9Djhs5 3175TgdLPK06lQ8W5cJdfgX/
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Wed, Dec 08, 2021 at 06:59:43PM +0200, Oleksandr Tyshchenko wrote:
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> 
> This patch adds basic support for configuring and assisting virtio-disk
> backend (emualator) which is intended to run out of Qemu and could be
> run in any domain.
> Although the Virtio block device is quite different from traditional
> Xen PV block device (vbd) from the toolstack point of view:
>  - as the frontend is virtio-blk which is not a Xenbus driver, nothing
>    written to Xenstore are fetched by the frontend (the vdev is not
>    passed to the frontend)
>  - the ring-ref/event-channel are not used for the backend<->frontend
>    communication, the proposed IPC for Virtio is IOREQ/DM
> it is still a "block device" and ought to be integrated in existing
> "disk" handling. So, re-use (and adapt) "disk" parsing/configuration
> logic to deal with Virtio devices as well.

How backend are intended to be created? Is there something listening on
xenstore?

You mention QEMU as been the backend, do you intend to have QEMU
listening on xenstore to create a virtio backend? Or maybe it is on the
command line? There is QMP as well, but it's probably a lot more
complicated as I think libxl needs refactoring for that.

> Besides introducing new disk backend type (LIBXL_DISK_BACKEND_VIRTIO)
> introduce new device kind (LIBXL__DEVICE_KIND_VIRTIO_DISK) as current
> one (LIBXL__DEVICE_KIND_VBD) doesn't fit into Virtio disk model.
> 
> In order to inform the toolstack that Virtio disk needs to be used
> extend "disk" configuration by introducing new "virtio" flag.
> An example of domain configuration:
> disk = [ 'backend=DomD, phy:/dev/mmcblk1p3, xvda1, rw, virtio' ]

This new "virtio" flags feels strange. Would having something like
"backendtype=virtio" works?

> Please note, this patch is not enough for virtio-disk to work
> on Xen (Arm), as for every Virtio device (including disk) we need
> to allocate Virtio MMIO params (IRQ and memory region) and pass
> them to the backend, also update Guest device-tree. The subsequent
> patch will add these missing bits. For the current patch,
> the default "irq" and "base" are just written to the Xenstore.

This feels like the patches are in the wrong order. I don't think it is
a good idea to allow to create broken configuration until a follow-up
patch fixes things.

> diff --git a/tools/xl/xl_block.c b/tools/xl/xl_block.c
> index 70eed43..50a4d45 100644
> --- a/tools/xl/xl_block.c
> +++ b/tools/xl/xl_block.c
> @@ -50,6 +50,11 @@ int main_blockattach(int argc, char **argv)
>          return 0;
>      }
>  
> +    if (disk.virtio) {
> +        fprintf(stderr, "block-attach is not supported for Virtio device\n");
> +        return 1;
> +    }

This might not be the right place. libxl might want to prevent hotplug
instead.

Thanks,

-- 
Anthony PERARD



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.