Re: [PATCH v1 1/7] xen-block: Do not write frontend nodes

[Paul Durrant]

On 10/11/2023 15:42, Volodymyr Babchuk wrote:
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>
> The PV backend running in other than Dom0 domain (non toolstack domain)
> is not allowed to write frontend nodes. The more, the backend does not
> need to do that at all, this is purely toolstack/xl devd business.
>
> I do not know for what reason the backend does that here, this is not really
> needed, probably it is just a leftover and all xen_device_frontend_printf()
> instances should go away completely.

It is not a leftover and it is needed in the case that QEMU is 
instantiating the backend unilaterally... i.e. without the involvement 
of any Xen toolstack.
Agreed that, if QEMU, is running in a deprivileged context, that is not 
an option. The correct way to determined this though is whether the 
device is being created via the QEMU command line or whether is being 
created because XenStore nodes written by a toolstack were discovered.
In the latter case there should be a XenBackendInstance that corresponds 
to the XenDevice whereas in the former case there should not be. For 
example, see xen_backend_try_device_destroy()

  Paul


> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> ---
>   hw/block/xen-block.c | 11 +++++++----
>   hw/xen/xen-bus.c     |  2 +-
>   2 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
> index a07cd7eb5d..dc4d477c22 100644
> --- a/hw/block/xen-block.c
> +++ b/hw/block/xen-block.c
> @@ -221,6 +221,7 @@ static void xen_block_realize(XenDevice *xendev, Error 
> **errp)
>       XenBlockVdev *vdev = &blockdev->props.vdev;
>       BlockConf *conf = &blockdev->props.conf;
>       BlockBackend *blk = conf->blk;
> +    XenBus *xenbus = XEN_BUS(qdev_get_parent_bus(DEVICE(xendev)));
>   
>       if (vdev->type == XEN_BLOCK_VDEV_TYPE_INVALID) {
>           error_setg(errp, "vdev property not set");
> @@ -280,10 +281,12 @@ static void xen_block_realize(XenDevice *xendev, Error 
> **errp)
>   
>       xen_device_backend_printf(xendev, "info", "%u", blockdev->info);
>   
> -    xen_device_frontend_printf(xendev, "virtual-device", "%lu",
> -                               vdev->number);
> -    xen_device_frontend_printf(xendev, "device-type", "%s",
> -                               blockdev->device_type);
> +    if (xenbus->backend_id == 0) {
> +        xen_device_frontend_printf(xendev, "virtual-device", "%lu",
> +                                   vdev->number);
> +        xen_device_frontend_printf(xendev, "device-type", "%s",
> +                                   blockdev->device_type);
> +    }
>   
>       xen_device_backend_printf(xendev, "sector-size", "%u",
>                                 conf->logical_block_size);
> diff --git a/hw/xen/xen-bus.c b/hw/xen/xen-bus.c
> index ece8ec40cd..06d5192aca 100644
> --- a/hw/xen/xen-bus.c
> +++ b/hw/xen/xen-bus.c
> @@ -1048,7 +1048,7 @@ static void xen_device_realize(DeviceState *dev, Error 
> **errp)
>       xen_device_backend_set_online(xendev, true);
>       xen_device_backend_set_state(xendev, XenbusStateInitWait);
>   
> -    if (!xen_device_frontend_exists(xendev)) {
> +    if (!xen_device_frontend_exists(xendev) && xenbus->backend_id == 0) {
>           xen_device_frontend_printf(xendev, "backend", "%s",
>                                      xendev->backend_path);
>           xen_device_frontend_printf(xendev, "backend-id", "%u",