All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleksandr <olekstysh@gmail.com>
To: George Dunlap <George.Dunlap@citrix.com>,
	Anthony Perard <anthony.perard@citrix.com>,
	Juergen Gross <jgross@suse.com>
Cc: xen-devel <xen-devel@lists.xenproject.org>,
	Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>,
	Wei Liu <wl@xen.org>, Nick Rosbrook <rosbrookn@gmail.com>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Julien Grall <julien@xen.org>,
	Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>,
	Bertrand Marquis <bertrand.marquis@arm.com>
Subject: Re: [PATCH V10 1/3] libxl: Add support for Virtio disk configuration
Date: Thu, 30 Jun 2022 15:18:23 +0300	[thread overview]
Message-ID: <36d4c786-9fb7-4b30-1a4d-171f92cc84d7@gmail.com> (raw)
In-Reply-To: <02648046-7781-61e5-de93-77342e4d6c16@gmail.com>


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@gmail.com> 
>>> wrote:
>>>
>>> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>>>
>>> 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@epam.com>
>> 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?



>
>
>
>>
>> FWIW the Golang changes look fine.


Thanks.


>>
>>   -George
>>
-- 
Regards,

Oleksandr Tyshchenko



  reply	other threads:[~2022-06-30 12:18 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-13 18:05 [PATCH V10 0/3] Virtio support for toolstack on Arm (Was "IOREQ feature (+ virtio-mmio) on Arm") Oleksandr Tyshchenko
2022-06-13 18:05 ` [PATCH V10 1/3] libxl: Add support for Virtio disk configuration Oleksandr Tyshchenko
2022-06-15 14:23   ` Anthony PERARD
2022-06-15 16:32     ` Oleksandr
2022-06-17 14:29       ` Anthony PERARD
2022-06-17 15:34         ` Oleksandr
2022-06-17 16:14           ` Oleksandr Tyshchenko
2022-06-23 10:24             ` [PATCH V10.1 " Anthony PERARD
2022-06-23 13:09               ` Oleksandr Tyshchenko
2022-06-24 12:59             ` [PATCH V10 " George Dunlap
2022-06-25 14:32               ` Oleksandr
2022-06-30 12:18                 ` Oleksandr [this message]
2022-06-30 12:24                   ` Juergen Gross
2022-06-30 18:34                     ` Oleksandr
2022-07-13  0:01                   ` George Dunlap
2022-07-13  8:03                     ` Oleksandr Tyshchenko
2022-07-15 15:24                       ` Anthony PERARD
2022-07-15 16:01                         ` Oleksandr Tyshchenko
2022-06-13 18:05 ` [PATCH V10 2/3] libxl: Introduce basic virtio-mmio support on Arm Oleksandr Tyshchenko
2022-06-13 18:05 ` [PATCH V10 3/3] libxl/arm: Create specific IOMMU node to be referred by virtio-mmio device Oleksandr Tyshchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=36d4c786-9fb7-4b30-1a4d-171f92cc84d7@gmail.com \
    --to=olekstysh@gmail.com \
    --cc=George.Dunlap@citrix.com \
    --cc=Volodymyr_Babchuk@epam.com \
    --cc=anthony.perard@citrix.com \
    --cc=bertrand.marquis@arm.com \
    --cc=jgross@suse.com \
    --cc=julien@xen.org \
    --cc=oleksandr_tyshchenko@epam.com \
    --cc=rosbrookn@gmail.com \
    --cc=sstabellini@kernel.org \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.