| Message ID | 20190823101534.465-3-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix for the xen-bus driver | expand |
> -----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
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);
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> --- 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(-)