Re: [Qemu-devel] [PATCH 1/5] RFC: Efficient VM backup for qemu (v1)

[Kevin Wolf]

Am 27.11.2012 08:15, schrieb Dietmar Maurer:
>>> 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?

To which BDS? If it is the BDS that is being backed up, the problem is
that you could only have one implementation per BDS, i.e. you couldn't
use backup and copy on read or I/O throttling or whatever at the same time.

Kevin