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

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



On 27.04.22 10:27, Roger Pau Monné wrote:
On Wed, Apr 27, 2022 at 11:20:04AM +0300, Oleksandr wrote:

On 25.04.22 16:12, Roger Pau Monné wrote:


Hello Roger


On Sat, Apr 23, 2022 at 10:39:14AM +0300, Oleksandr wrote:
On 22.04.22 12:41, Roger Pau Monné wrote:


Hello Roger

On Fri, Apr 08, 2022 at 09:21:04PM +0300, Oleksandr Tyshchenko wrote:
From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
--- a/docs/man/xl-disk-configuration.5.pod.in
+++ b/docs/man/xl-disk-configuration.5.pod.in
@@ -232,7 +232,7 @@ Specifies the backend implementation to use
    =item Supported values
-phy, qdisk
+phy, qdisk, other
    =item Mandatory
@@ -244,11 +244,13 @@ Automatically determine which backend to use.
    =back
-This does not affect the guest's view of the device.  It controls
-which software implementation of the Xen backend driver is used.
+It controls which software implementation of the backend driver is used.
+Depending on the "protocol" option this may affect the guest's view
+of the device.
    Not all backend drivers support all combinations of other options.
-For example, "phy" does not support formats other than "raw".
+For example, "phy" and "other" do not support formats other than "raw" and
+"other" does not support protocols other than "virtio-mmio".
    Normally this option should not be specified, in which case libxl will
    automatically determine the most suitable backend.
@@ -344,8 +346,35 @@ can be used to disable "hole punching" for file based 
backends which
    were intentionally created non-sparse to avoid fragmentation of the
    file.
+=item B<protocol>=I<PROTOCOL>
+
+=over 4
+
+=item Description
+
+Specifies the communication protocol to use for the chosen "backendtype" option
+
+=item Supported values
+
+xen, virtio-mmio
   From a user PoV, I think it would be better to just select xen or
virtio here, but not the underlying configuration mechanism used to
expose the devices to the guest.
I got your point.



We would likely need to add a different option to select mmio or pci
then, but that should be set by default based on architecture/guest
type.  For example on x86 it should default to pci, while on Arm I
guess it will depend on whether the guest has PCI or not?

In any case, I think we should offer an option that's selecting
between xen or virtio protocol, and the way to expose the
configuration of the device shouldn't need to be explicitly selected
by the user.

ok, for now I will use "xen and virtio" values for the "protocol" option,
then internally toolstack will assume that "virtio" really means
"virtio-mmio".

When there is a need to expand that support to "virtio-pci", we will see how
to deal with it from the configuration PoV, probably like you suggested
above by adding another option (e.g. "transport") with default values based
on the architecture/guest type.
I think this likely also wants to be a separate field in libxl_device_disk,
which could be left empty and libxl will attempt to set a default.
For example have the following in libxl_types.idl:

libxl_device_protocol = Enumeration("device_protocol", [
      (0, "UNKNOWN"),
      (1, "XEN"),
      (2, "VIRTIO"),
      ])

libxl_device_configuration = Enumeration("device_configuration", [
      (0, "UNKNOWN"),
      (1, "XENBUS"),
      (2, "MMIO"),
      ])

libxl_device_disk = Struct("device_disk", [
      ("protocol", libxl_device_protocol),
      ("configuration", libxl_device_configuration),
      ])

I don't like libxl_device_configuration much, I think is too generic,
but I can't think of anything better.  Maybe others can provide better
names.


Here [1] we had a discussion regarding user configuration options.
"protocol" cannot be used as it will lead to the confusion (at least with
Xen PV block device which already has "protocol" frontend's entry in
Xenstore).

Preliminary, we had agreed on the following names:
1. specification: xen or virtio
2. transport: mmio or pci

Please tell me, are you OK with them?

Yes, that's fine. My main point is that libxl_device_disk should
contain both fields, so a 3rd party toolstack can set 'specification =
virtio' and let Xen decide the transport to use.

I'm dubious whether we want to have a xenbus transport for xen
specification to use, but I guess it's fine to say that for
specification == xen transport is ignored.

At least for now, I guess. It might be a thing in case someone decides
to add argo support.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature


 


Rackspace

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