On Tue, May 18, 2021 at 12:14:17PM +0200, Emanuele Giuseppe Esposito wrote: > On 18/05/2021 12:00, Vladimir Sementsov-Ogievskiy wrote: > > 18.05.2021 12:40, Emanuele Giuseppe Esposito wrote: > > > Progressmeter is protected by the AioContext mutex, which > > > is taken by the block jobs and their caller (like blockdev). > > > > > > We would like to remove the dependency of block layer code on the > > > AioContext mutex, since most drivers and the core I/O code are already > > > not relying on it. > > > > > > Create a new C file to implement the ProgressMeter API, but keep the > > > struct as public, to avoid forcing allocation on the heap. > > > > > > Also add a mutex to be able to provide an accurate snapshot of the > > > progress values to the caller. > > > > > > Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com> > > > > patch changed a lot, so you can't keep Stefan's r-b. r-b should be kept > > if patch is unchanged. > > Sorry, my bad. Will remove it, if we keep these changes (see below).
I took a look again: Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com> Regarding hiding the struct, in a library with a public API the authors need to be very careful about exposing struct fields in order to prevent ABI breakage or applications making use of internal fields. However, in QEMU we're less strict since we have full control over the codebase (including internal API consumers). If other factors outweigh the need for strict encapsulation, then you can put the struct definition in the header file. Grepping for "private" in QEMU shows lots of examples and there are probably many more without an explicit "private" comment. Code reviewers can reject patches that touch private struct fields or patches can be submitted to remove existing access to private fields, so we don't have the limitations that library authors have. Stefan
signature.asc
Description: PGP signature
