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

Re: [PATCH V10 1/3] libxl: Add support for Virtio disk configuration


  • To: George Dunlap <George.Dunlap@xxxxxxxxxx>, Anthony Perard <anthony.perard@xxxxxxxxxx>
  • From: Oleksandr Tyshchenko <Oleksandr_Tyshchenko@xxxxxxxx>
  • Date: Wed, 13 Jul 2022 08:03:17 +0000
  • Accept-language: en-US, ru-RU
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=epam.com; dmarc=pass action=none header.from=epam.com; dkim=pass header.d=epam.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=cyNgUV/5mVr39DRK6kH9RT7sXF7cYf5nsnGaiX9okSE=; b=ckPCt4NRKtK/+104rGxfW3/kYZvI4O6JGFUmRq9Ow+zkWtwChfTBujWoN3jp/5P5+m4hVdIF72/xL5Vw8p8YXGID4jmQ+MRGfHcO5Nlr7MwJExh5R05G2+JGvj6I+tWhvL2S8QEJ8P3dCi4baiT+rdPFIslSq9K6SA/gumsXCsNoIFygKpeQar2KYowBGxtgH1HmcNXnIzqJ9FRccS715FEDA+J3VxiHX4uyZkjW8G6+I79NzsNmsLhh4Hyo3ffCU7wzJTeRpdpkZV9ErC+tVvBYb0d+Hlg94IYI7J9vTTaFBTwUeD8lDHR2pkb2utXexf/48FV6Km+uQRv1RYDUSQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=EjMff4rbKfF4dIIcCZVBMlsUj2th3452paZb3CHHdqfJIdnborxgWM7Z7BOzPuNZV4VudoJwqiHunEBhhAeFzoI+PawdbT/1V0eFNSx+U+GkX1JssOrwRRVA1spp8t/kI5zLnmemOp4i59j5M7Lkp8ZnxmGzM9lDo2h5mCPN9s/lpe3+8zwm3HxabdyFxUukfVQIOUKvyL4BfYvacqFcFKQTNPBVftv6YdkMbMXnZm54kEqujhC/4FiZJwrrgoi85NKngxjfzKf0jkPk/Zgn/RsX/N8DjskLFMqQwnHS4De6pMT5HRR/hfT1yLgqGVQhmIcd0CBbrvcRNbIlrXbMxQ==
  • Cc: Juergen Gross <jgross@xxxxxxxx>, xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Nick Rosbrook <rosbrookn@xxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, Oleksandr <olekstysh@xxxxxxxxx>
  • Delivery-date: Wed, 13 Jul 2022 08:03:40 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHYf1A6pQ9CS25xdkGDIpueSgv3zq1QiHQAgAAkCwCAAwIqgIAAEjKAgAALK4CACsnJAIABrEAAgAe2TICAE6BaAIAAhq0A
  • Thread-topic: [PATCH V10 1/3] libxl: Add support for Virtio disk configuration

On 13.07.22 03:01, George Dunlap wrote:

Hello George, Anthony

>
>
>> On 30 Jun 2022, at 22:18, Oleksandr <olekstysh@xxxxxxxxx> wrote:
>>
>>
>> Dear all.
>>
>>
>> On 25.06.22 17:32, Oleksandr wrote:
>>>
>>> On 24.06.22 15:59, George Dunlap wrote:
>>>
>>> Hello George
>>>
>>>>
>>>>> On 17 Jun 2022, at 17:14, Oleksandr Tyshchenko 
>>>>> <olekstysh@xxxxxxxxx> wrote:
>>>>>
>>>>> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
>>>>>
>>>>> This patch adds basic support for configuring and assisting 
>>>>> virtio-mmio
>>>>> based virtio-disk backend (emulator) 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's point of view:
>>>>> - as the frontend is virtio-blk which is not a Xenbus driver, nothing
>>>>> written to Xenstore are fetched by the frontend currently ("vdev"
>>>>> is not passed to the frontend). But this might need to be revised
>>>>> in future, so frontend data might be written to Xenstore in order to
>>>>> support hotplugging virtio devices or passing the backend domain id
>>>>> on arch where the device-tree is not available.
>>>>> - 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.
>>>>>
>>>>> For the immediate purpose and an ability to extend that support for
>>>>> other use-cases in future (Qemu, virtio-pci, etc) perform the 
>>>>> following
>>>>> actions:
>>>>> - Add new disk backend type (LIBXL_DISK_BACKEND_OTHER) and reflect
>>>>> that in the configuration
>>>>> - Introduce new disk "specification" and "transport" fields to struct
>>>>> libxl_device_disk. Both are written to the Xenstore. The transport
>>>>> field is only used for the specification "virtio" and it assumes
>>>>> only "mmio" value for now.
>>>>> - Introduce new "specification" option with "xen" communication
>>>>> protocol being default value.
>>>>> - Add new device kind (LIBXL__DEVICE_KIND_VIRTIO_DISK) as current
>>>>> one (LIBXL__DEVICE_KIND_VBD) doesn't fit into Virtio disk model
>>>>>
>>>>> An example of domain configuration for Virtio disk:
>>>>> disk = [ 'phy:/dev/mmcblk0p3, xvda1, backendtype=other, 
>>>>> specification=virtio']
>>>>>
>>>>> Nothing has changed for default Xen disk configuration.
>>>>>
>>>>> 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 is not an ideal splitting, but this way we avoid breaking
>>>>> the bisectability.
>>>>>
>>>>> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
>>>> OK, I am *really* sorry for coming in here at the last minute and 
>>>> quibbling about things.
>>>
>>>
>>> no problem
>>>
>>>
>>>> But this introduces a public interface which looks really wrong to 
>>>> me.  I’ve replied to the mail from December where Juergen proposed 
>>>> the “Other” protocol.
>>>>
>>>> Hopefully this will be a simple matter of finding a better name 
>>>> than “other”. (Or you guys convincing me that “other” is really the 
>>>> best name for this value; or even Anthony asserting his right as a 
>>>> maintainer to overrule my objection if he thinks I’m out of line.)
>>>
>>>
>>> I saw your reply to V6 and Juergen's answer. I share Juergen's 
>>> opinion as well as I understand your concern. I think, this is 
>>> exactly the situation when finding a perfect name (obvious, short, 
>>> etc) for the backendtype (in our particular case) is really difficult.
>>>
>>> Personally I tend to leave "other", because there is no better 
>>> alternative from my PoV. Also I might be completely wrong here, but 
>>> I don't think we will have to extend the "backendtype" for 
>>> supporting other possible virtio backend implementations in the 
>>> foreseeable future:
>>>
>>> - when Qemu gains the required support we will choose qdisk: 
>>> backendtype qdisk specification virtio
>>> - for the possible virtio alternative of the blkback we will choose 
>>> phy: backendtype phy specification virtio
>>>
>>> If there will be a need to keep various implementation, we will be 
>>> able to describe that by using "transport" (mmio, pci, xenbus, argo, 
>>> whatever).
>>> Actually this is why we also introduced "specification" and "transport".
>>>
>>> IIRC, there were other (suggested?) names except "other" which are 
>>> "external" and "daemon". If you think that one of them is better 
>>> than "other", I will happy to use it.
>>
>>
>> Could we please make a decision on this?
>>
>> If "other" is not unambiguous, then maybe we could choose "daemon" to 
>> describe arbitrary user-level backends, any thought?
>
> Unfortunately I didn’t have time to dig into this; I’m just going to 
> have to withdraw my objection, and let you & Juergen decide what to 
> call it.

George, thanks for letting me know. Juergen proposed to use "standalone" 
for the new backendtype name which is far more specific. I agree with that.


Anthony, would you be happy with that renaming?



>
> Re the golang changes:
>
> Acked-by: George Dunlap <george.dunlap@xxxxxxxxxx>


Thanks.


-- 
Regards,

Oleksandr Tyshchenko

 


Rackspace

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