On 04/11/2017 12:57 AM, Paolo Bonzini wrote: > > > On 11/04/2017 00:05, John Snow wrote: >> >> >> On 04/08/2017 05:52 AM, Paolo Bonzini wrote: >>> >>> >>> On 08/04/2017 08:03, John Snow wrote: >>>> Looks clean, though it may be useful to do a few more things; >>>> >>>> - Demarcate what you think is the monitor API in this file >>> >>> It's already there: >>> >>> +/* >>> + * API for block job drivers and the block layer. >>> + */ >>> + >>> >>> where everything before is for the monitor. >>> >> >> I meant explicitly, with a comment at the top explaining the demarcation. > > Oh, sure. > >>>> - Organize blockjob.h to match to serve as a useful reference. >>> >>> Hmm, yes. > > As it turns out, no headers are necessary---but yours was a very good > remark still, because a couple mistakes in this series stood out when > checking. > > The "API for block job drivers and the block layer" is already in > blockjob_int.h while the rest is in blockjob.h. This is nice because it > provides some validation of the concept behind the patch, and also of > the locking policy I chose for the rest of the work. > > But, there are two exceptions. Both of them are introduced by this > series and they shouldn't be: > > - blockjob_pause/resume_all should have its declaration in block_int.h, > so I've fixed patch 4 accordingly > > - blockjob_create is in blockjob_int.h, but this patch should move it to > the second part of the file, too. > > Thanks, > > Paolo >
"You were wrong, but so was I, thanks for being wrong in a helpful way." No problem. --js
