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
