> > The only solution I came up with is to add before/after hooks in the > > block job. I agree with the criticism, but I think it's general > > enough and at the same time easy enough to implement. > > > > > IMHO, the current implementation is quite simple and easy to maintain. > > > > No, "if (bs->backup_info)" simply doesn't belong in bdrv_co_writev. > > I do not really understand that argument, because the current > COPY_ON_READ implementation also works that way: > > if (bs->copy_on_read) { > flags |= BDRV_REQ_COPY_ON_READ; > } > if (flags & BDRV_REQ_COPY_ON_READ) { > bs->copy_on_read_in_flight++; > } > > if (bs->copy_on_read_in_flight) { > wait_for_overlapping_requests(bs, sector_num, nb_sectors); > } > > tracked_request_begin(&req, bs, sector_num, nb_sectors, false); > > if (flags & BDRV_REQ_COPY_ON_READ) { ... > > Or do you also want to move that to block job hooks?
Just tried to move that code, but copy on read feature is unrelated to block jobs, i.e. one can open a bdrv with BDRV_O_COPY_ON_READ, and that does not create a job. I already suggested to add those hooks to BDS instead - don't you think that would work?