Re: [Xen-devel] [PATCH v2 2/2] xen-bus: Avoid rewriting identical values to xenstore

[Paul Durrant <Paul.Durrant@citrix.com>]

> -----Original Message-----
> From: Anthony PERARD <anthony.perard@citrix.com>
> Sent: 23 August 2019 11:16
> To: qemu-devel@nongnu.org
> Cc: Anthony Perard <anthony.perard@citrix.com>; Stefano Stabellini <sstabellini@kernel.org>; Paul
> Durrant <Paul.Durrant@citrix.com>; xen-devel@lists.xenproject.org
> Subject: [PATCH v2 2/2] xen-bus: Avoid rewriting identical values to xenstore
> 
> When QEMU receives a xenstore watch event suggesting that the "state"
> of the frontend changed, it records this in its own state but it also
> re-write the value back into xenstore even so there were no change.
> This triggers an unnecessary xenstore watch event which QEMU will
> process again (and maybe the frontend as well). Also QEMU could
> potentially write an already old value.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

Reviewed-by: Paul Durrant <paul.durrant@citrix.com>

> ---
> 
> Notes:
>     v2:
>     - fix coding style
>     - only change frontend_set_state() and use 'publish' instead of 'export'.
> 
>  hw/xen/xen-bus.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/xen/xen-bus.c b/hw/xen/xen-bus.c
> index 62c127b926..a04478ad4f 100644
> --- a/hw/xen/xen-bus.c
> +++ b/hw/xen/xen-bus.c
> @@ -698,7 +698,8 @@ int xen_device_frontend_scanf(XenDevice *xendev, const char *key,
>  }
> 
>  static void xen_device_frontend_set_state(XenDevice *xendev,
> -                                          enum xenbus_state state)
> +                                          enum xenbus_state state,
> +                                          bool publish)
>  {
>      const char *type = object_get_typename(OBJECT(xendev));
> 
> @@ -710,7 +711,9 @@ static void xen_device_frontend_set_state(XenDevice *xendev,
>                                      xs_strstate(state));
> 
>      xendev->frontend_state = state;
> -    xen_device_frontend_printf(xendev, "state", "%u", state);
> +    if (publish) {
> +        xen_device_frontend_printf(xendev, "state", "%u", state);
> +    }
>  }
> 
>  static void xen_device_frontend_changed(void *opaque)
> @@ -726,7 +729,7 @@ static void xen_device_frontend_changed(void *opaque)
>          state = XenbusStateUnknown;
>      }
> 
> -    xen_device_frontend_set_state(xendev, state);
> +    xen_device_frontend_set_state(xendev, state, false);
> 
>      if (state == XenbusStateInitialising &&
>          xendev->backend_state == XenbusStateClosed &&
> @@ -1169,7 +1172,7 @@ static void xen_device_realize(DeviceState *dev, Error **errp)
>      xen_device_frontend_printf(xendev, "backend-id", "%u",
>                                 xenbus->backend_id);
> 
> -    xen_device_frontend_set_state(xendev, XenbusStateInitialising);
> +    xen_device_frontend_set_state(xendev, XenbusStateInitialising, true);
> 
>      xendev->exit.notify = xen_device_exit;
>      qemu_add_exit_notifier(&xendev->exit);
> --
> Anthony PERARD


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel