On Mon, Aug 08, 2022 at 05:41:45AM -0400, Emanuele Giuseppe Esposito wrote: > The way the dataplane stages at startup and stop are monitored is unnecessary > complicated. In virtio-scsi we have dataplane_started, dataplane_starting and > dataplane_stopping in VirtIOSCSI. > In virtio-blk we have dataplene_started in VirtIOBlock, and starting and > stopping > in VirtIOBlockDataPlane. > > Just replace all these flags with an atomic enum. > > Based-on: 20220803162824.948023-1-stefa...@redhat.com
As mentioned on IRC, I don't think it's useful to combine these fields into a state machine because they serve different purposes (starting/stopping prevents re-entrancy, started/stopped tracks whether dataplane is enabled, and fenced tracks whether dataplane is broken). Combining them all makes it harder to refactor those separate concerns. In the future it would be nice to refactor away all this state and have stateless IOThread support (I haven't figured out whether that's possible yet). I'm not against merging this, but I don't think using an enum is an improvement - it's just different. Regarding thread-safety, we need to guarantee that the state stored by a QEMU thread is visible to the IOThread. This patch series doesn't really do this: relaxed loads/stores aren't guaranteed to be visible to the other thread. I comment on this in the patch: there needs to be at least a comment or maybe something extra, like a memory barrier or stronger atomic operation, to make the change to atomics worthwhile.
signature.asc
Description: PGP signature
