Re: [Qemu-devel] [PATCH v5 01/21] docs: incremental backup documentation

[John Snow <jsnow@redhat.com>]

On 04/17/2015 11:06 AM, Eric Blake wrote:
> On 04/08/2015 04:19 PM, John Snow wrote:
>> Reviewed-by: Max Reitz <mreitz@redhat.com>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>   docs/bitmaps.md | 311 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 311 insertions(+)
>>   create mode 100644 docs/bitmaps.md
>>
>> diff --git a/docs/bitmaps.md b/docs/bitmaps.md
>> new file mode 100644
>> index 0000000..ad8c33b
>> --- /dev/null
>> +++ b/docs/bitmaps.md
>> @@ -0,0 +1,311 @@
>> +# Dirty Bitmaps and Incremental Backup
>> +
>
> Still might be nice to list explicit copyright/license instead of
> relying on implicit top-level GPLv2+, but I won't insist.
>

I think I would rather not clutter up the document itself, if that 
remains suitable. I don't mind those declarations in source code, but 
for a document like this, it seems weird to have it in the preamble.

I can attach a license to the footer, if that's suitable?

>
>> +### Deletion
>> +
>> +* Can be performed on a disabled bitmap, but not a frozen one.
>
> Do you still have a notion of disabled bitmaps?  Earlier, in '## Bitmap
> Modes', you only document 'frozen' (as opposed to the default unnamed
> state).
>

We do internally. It's not likely to come up from a user's perspective, 
but we do intend to disable the bitmap during e.g. migration, boot, etc.

I did pull the "disabled" bit out because it's not a necessary detail yet.

I'll tidy this up and reintroduce the language alongside the patch that 
may expose the user to witnessing a disabled bitmap.

>
>> +
>> +## Transactions (Not yet implemented)
>
> I'm assuming that "[PATCH v2 00/11] block: incremental backup
> transactions" is incomplete, because it forgot to clean this up as part
> of adding transaction support.
>

Fixed in my local copy, yes.

>
>> +
>> +5. Retry the command after fixing the underlaying problem,
>
> s/underlaying/underlying/
>

:(