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

2022-07-28 Thread Oleksandr Tyshchenko
чт, 28 июл. 2022 г., 21:28 Julien Grall :

> Hi Oleksandr,
>


Hello Julien

(sorry for the possible format issues)


> On 16/07/2022 17:37, Oleksandr Tyshchenko wrote:
> > Changes V10.1 -> V11:
> > - Anthony already gave his Reviewed-by, I dropped it due to the
> changes
> > - George already gave his Acked-by, I dropped it due to the changes
> > - s/other/standalone for the backendtype
>
> As discussed with Andrew on IRC, the golang changes are autogenerated.
> So I have kept George's acked-by and committed the series.
>


Thank you very much!


> Cheers,
>
> --
> Julien Grall
>


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

2022-07-28 Thread Julien Grall

Hi Oleksandr,

On 16/07/2022 17:37, Oleksandr Tyshchenko wrote:

Changes V10.1 -> V11:
- Anthony already gave his Reviewed-by, I dropped it due to the changes
- George already gave his Acked-by, I dropped it due to the changes
- s/other/standalone for the backendtype


As discussed with Andrew on IRC, the golang changes are autogenerated. 
So I have kept George's acked-by and committed the series.


Cheers,

--
Julien Grall



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

2022-07-20 Thread Anthony PERARD
On Sat, Jul 16, 2022 at 07:37:45PM +0300, Oleksandr Tyshchenko wrote:
> From: Oleksandr Tyshchenko 
> 
> 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_STANDALONE) 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=standalone, 
> 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 

Reviewed-by: Anthony PERARD 

Thanks,

-- 
Anthony PERARD



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

2022-07-20 Thread Oleksandr Tyshchenko

On 20.07.22 06:29, Jiamei Xie wrote:
> Hi Oleksandr,

Hello Jiamei


>
> We have tested it on arm64 with " disk = [ 'phy:/usr/share/guests/disk.img0, 
> xvda1, backendtype=standalone, specification=virtio']". It works ok.
>
> Tested-by: Jiamei Xie 
> Tested-by: Henry Wang 


Thank you for the testing!


>
> Best wishes
> Jiamei Xie
>
>
>> -Original Message-
>> From: Xen-devel  On Behalf Of
>> Oleksandr Tyshchenko
>> Sent: Sunday, July 17, 2022 12:38 AM
>> To: xen-devel@lists.xenproject.org
>> Cc: Oleksandr Tyshchenko ; Wei Liu
>> ; Anthony PERARD ; George
>> Dunlap ; Nick Rosbrook
>> ; Juergen Gross ; Stefano
>> Stabellini ; Julien Grall ; Volodymyr
>> Babchuk ; Bertrand Marquis
>> 
>> Subject: [PATCH V11.1 1/3] libxl: Add support for Virtio disk configuration
>>
>> From: Oleksandr Tyshchenko 
>>
>> 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_STANDALONE) 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=standalone,
>> 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 
>> ---
>> Changes RFC -> V1:
>> - no changes
>>
>> Changes V1 -> V2:
>> - rebase according to the new location of libxl_virtio_disk.c
>>
>> Changes V2 -> V3:
>> - no changes
>>
>> Changes V3 -> V4:
>> - rebase according to the new argument for DEFINE_DEVICE_TYPE_STRUCT
>>
>> Changes V4 -> V5:
>> - split the changes, change the order of the patches
>> - update patch description
>> - don't introduce new "vdisk" configuration option with own parsing 
>> logic,
>>   re-use Xen PV block "disk" parsing/configuration logic for the 
>> virtio-disk
>> - introduce "virtio" flag and document it's usage
>> - add LIBXL_HAVE_DEVICE_DISK_VIRTIO
>> - update libxlu_disk_l.[ch]
>> - drop num_disks variable/MAX_VIRTIO_DISKS
>> - drop Wei's T-b
>>
>> Changes V5 -> V6:
>

RE: [PATCH V11.1 1/3] libxl: Add support for Virtio disk configuration

2022-07-19 Thread Jiamei Xie
Hi Oleksandr,

We have tested it on arm64 with " disk = [ 'phy:/usr/share/guests/disk.img0, 
xvda1, backendtype=standalone, specification=virtio']". It works ok.

Tested-by: Jiamei Xie 
Tested-by: Henry Wang 

Best wishes
Jiamei Xie


> -Original Message-
> From: Xen-devel  On Behalf Of
> Oleksandr Tyshchenko
> Sent: Sunday, July 17, 2022 12:38 AM
> To: xen-devel@lists.xenproject.org
> Cc: Oleksandr Tyshchenko ; Wei Liu
> ; Anthony PERARD ; George
> Dunlap ; Nick Rosbrook
> ; Juergen Gross ; Stefano
> Stabellini ; Julien Grall ; Volodymyr
> Babchuk ; Bertrand Marquis
> 
> Subject: [PATCH V11.1 1/3] libxl: Add support for Virtio disk configuration
> 
> From: Oleksandr Tyshchenko 
> 
> 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_STANDALONE) 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=standalone,
> 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 
> ---
> Changes RFC -> V1:
>- no changes
> 
> Changes V1 -> V2:
>- rebase according to the new location of libxl_virtio_disk.c
> 
> Changes V2 -> V3:
>- no changes
> 
> Changes V3 -> V4:
>- rebase according to the new argument for DEFINE_DEVICE_TYPE_STRUCT
> 
> Changes V4 -> V5:
>- split the changes, change the order of the patches
>- update patch description
>- don't introduce new "vdisk" configuration option with own parsing logic,
>  re-use Xen PV block "disk" parsing/configuration logic for the 
> virtio-disk
>- introduce "virtio" flag and document it's usage
>- add LIBXL_HAVE_DEVICE_DISK_VIRTIO
>- update libxlu_disk_l.[ch]
>- drop num_disks variable/MAX_VIRTIO_DISKS
>- drop Wei's T-b
> 
> Changes V5 -> V6:
>- rebase on current staging
>- use "%"PRIu64 instead of %lu for disk->base in device_disk_add()
>- update *.gen.go files
> 
> Changes V6 -> V7:
>- rebase on current staging
>- update *.gen.go files and libxlu_disk_l.[ch] files
>- update patch description
>- rework significantly to support more flexible configuration
>  and have more generic basic implementation for being able to extend
>  that for other use-cases (virtio-pci, qemu, etc).
> 
> Chan

[PATCH V11.1 1/3] libxl: Add support for Virtio disk configuration

2022-07-16 Thread Oleksandr Tyshchenko
From: Oleksandr Tyshchenko 

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_STANDALONE) 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=standalone, 
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 
---
Changes RFC -> V1:
   - no changes

Changes V1 -> V2:
   - rebase according to the new location of libxl_virtio_disk.c

Changes V2 -> V3:
   - no changes

Changes V3 -> V4:
   - rebase according to the new argument for DEFINE_DEVICE_TYPE_STRUCT

Changes V4 -> V5:
   - split the changes, change the order of the patches
   - update patch description
   - don't introduce new "vdisk" configuration option with own parsing logic,
 re-use Xen PV block "disk" parsing/configuration logic for the virtio-disk
   - introduce "virtio" flag and document it's usage
   - add LIBXL_HAVE_DEVICE_DISK_VIRTIO
   - update libxlu_disk_l.[ch]
   - drop num_disks variable/MAX_VIRTIO_DISKS
   - drop Wei's T-b

Changes V5 -> V6:
   - rebase on current staging
   - use "%"PRIu64 instead of %lu for disk->base in device_disk_add()
   - update *.gen.go files

Changes V6 -> V7:
   - rebase on current staging
   - update *.gen.go files and libxlu_disk_l.[ch] files
   - update patch description
   - rework significantly to support more flexible configuration
 and have more generic basic implementation for being able to extend
 that for other use-cases (virtio-pci, qemu, etc).

Changes V7 -> V8:
   - update *.gen.go files and libxlu_disk_l.[ch] files
   - update patch description and comments in the code
   - use "specification" config option instead of "protocol"
   - update libxl_types.idl and code according to new fields
 in libxl_device_disk

Changes V8 -> V9:
   - update (and harden) checks in libxl__device_disk_setdefault(),
 return error in case of incorrect settings of specification
 and transport
   - remove both asserts in device_disk_add()
   - update virtio related code in libxl__disk_from_xenstore(),
 do not fail if specification node is absent, replace
 open-coded checks of fetched specification and transport by
 libxl_disk_specification_from_string() and 
libxl_disk_transport_from_string()
 respectively
   - s/libxl_device_disk_get_path/libxl__device_disk_get_path
   - add a comment for virtio-mmio parameters in struct libxl_device_disk

Changes V9 -> V10:
   - s/ERROR_FAIL/ERROR_INVAL in both places in libxl__device_disk_setdefault()
   - rework libxl__device_disk_get_path()

Changes V10 -> V10.1:
   - fix small coding style issue in libxl__device_disk_get_path()
   - drop specification check in main_blockattach() and add
 required check in libxl__device_disk_setdefault()
   - update specification check in main_blockdetach()

Changes V10.1 -> V11:
   - Anthony