[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/2] xen-bus: Avoid rewriting identical values t
From: |
Anthony PERARD |
Subject: |
Re: [Qemu-devel] [PATCH 2/2] xen-bus: Avoid rewriting identical values to xenstore |
Date: |
Thu, 22 Aug 2019 14:18:02 +0100 |
User-agent: |
Mutt/1.12.1 (2019-06-15) |
On Thu, Aug 22, 2019 at 12:25:44PM +0100, Paul Durrant wrote:
> > From: Anthony PERARD <address@hidden>
> > Sent: 22 August 2019 12:18
> >
> > On Thu, Aug 22, 2019 at 11:36:32AM +0100, Paul Durrant wrote:
> > > But, now I look at the code again without your patch applied I don't
> > > actually see the problem it is
> > trying to fix. The functions xen_device_[back|front]end_set_state return
> > early if the state being set
> > matches the existing state and hence never get to the line where the state
> > is written to xenstore.
> >
> > Let's see:
> > * step 1 (initial states in xenstore and QEMU)
> > xenstore/frontend/state = 4
> > xendev->frontend_state = 4
> > * step 2 (frontend changes state in xenstore)
> > xenstore/frontend/state = 5
> > * step 3 (watch event received by QEMU)
> > xen_device_frontend_changed()
> > state = read(xenstore/frontend/state) (state=5)
> > xen_device_frontend_set_state(state)
> > xendev->frontend_state != state (4!=5)
> > xendev->frontend_state = state
> > xenstore/frontend/state = state
> > * step 4
> > # watch event triggers xen_device_frontend_changed() again but
> > # this time xendev->frontend_state == xenstore/frontend_state
> >
> > This is how QEMU writes to xenstore an identical value.
> >
> > That behavior might be an issue if the frontend changes the value after
> > QEMU have read it but before QEMU writes it again.
>
> Ah, ok, so the problem is actually limited to frontend state because that is
> written by both frontend and backend, so whether QEMU writes an updated
> frontend state to xenstore needs to be controlled. It's only called in two
> places xen_device_frontend_changed() and xen_device_realize(). The write to
> xenstore should be avoided in the former case, but not the latter. So adding
> a 'publish' boolean and using that to determine whether the write to xenstore
> is done seems like the right approach. But I don't think any change is needed
> to xen_device_backend_set_online() or xen_device_backend_set_state(), is it?
I guess it's not that much of a issue for backend_set_*(), the double
write would only happen when the toolstack try to tear down the backend,
so it would happen only once.
Alright, I'll only change frontend_set_state() and use 'publish'.
--
Anthony PERARD
- [Qemu-devel] [PATCH 1/2] xen-bus: Fix backend state transition on device reset, (continued)
- [Qemu-devel] [PATCH 2/2] xen-bus: Avoid rewriting identical values to xenstore, Anthony PERARD, 2019/08/21
- Re: [Qemu-devel] [PATCH 2/2] xen-bus: Avoid rewriting identical values to xenstore, Paul Durrant, 2019/08/21
- Re: [Qemu-devel] [PATCH 2/2] xen-bus: Avoid rewriting identical values to xenstore, Anthony PERARD, 2019/08/22
- Re: [Qemu-devel] [PATCH 2/2] xen-bus: Avoid rewriting identical values to xenstore, Paul Durrant, 2019/08/22
- Re: [Qemu-devel] [PATCH 2/2] xen-bus: Avoid rewriting identical values to xenstore, Anthony PERARD, 2019/08/22
- Re: [Qemu-devel] [PATCH 2/2] xen-bus: Avoid rewriting identical values to xenstore, Paul Durrant, 2019/08/22
- Re: [Qemu-devel] [PATCH 2/2] xen-bus: Avoid rewriting identical values to xenstore,
Anthony PERARD <=
- Re: [Qemu-devel] [PATCH 2/2] xen-bus: Avoid rewriting identical values to xenstore, Paul Durrant, 2019/08/22
Re: [Qemu-devel] [PATCH 0/2] Fix for the xen-bus driver, no-reply, 2019/08/21