[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 05/10] snapshot: Add support for specifying snapshot disk backing type



On 01/10/14 00:30, Eric Blake wrote:
> On 01/09/2014 09:15 AM, Peter Krempa wrote:
>> Add support for specifying various types when doing snapshots. This will
>> later allow to do snapshots on network backed volumes. Disks of type
>> 'volume' are not supported by snapshots (yet).
>> ---
>>  docs/formatsnapshot.html.in     | 15 ++++++++
>>  docs/schemas/domainsnapshot.rng | 76 +++++++++++++++++++++++++++++++++++------
>>  src/conf/snapshot_conf.c        | 25 +++++++++++---
>>  src/conf/snapshot_conf.h        | 15 ++++----
>>  src/qemu/qemu_driver.c          | 59 +++++++++++++++++++++-----------
>>  5 files changed, 149 insertions(+), 41 deletions(-)
> 
> Hopefully some tests are added later in the series to exercise the new
> RNG schema...
> 
>>
>> diff --git a/docs/formatsnapshot.html.in b/docs/formatsnapshot.html.in
>> index 76689cb..c2cd18c 100644
>> --- a/docs/formatsnapshot.html.in
>> +++ b/docs/formatsnapshot.html.in
>> @@ -170,6 +170,21 @@
>>              snapshots, the original file name becomes the read-only
>>              snapshot, and the new file name contains the read-write
>>              delta of all disk changes since the snapshot.
>> +
>> +            <span class="since">Since 1.2.2</span> the <code>disk</code> element
> 
> Ah, here you admit the post-release nature of the patch :)

Yeah, I wrote those docs after it was clear it won't make 1.2.2 but I
forgot to update the previous patch when I was posting it.

> 
>> @@ -616,6 +626,9 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf,
>>
>>      if (type < 0)
>>          type = VIR_DOMAIN_DISK_TYPE_FILE;
>> +    else
>> +        virBufferAsprintf(buf, " type='%s'",
>> +                          virDomainDiskTypeToString(type));
> 
> Should we always output the type, or is this a case where for
> back-compat, if the user omitted type, it's okay for us to omit it too?
>  Users already have to be prepared for more xml on output than what they
> put on input, and being explicit about type='file' even when the user
> didn't specify it may help us down the road.
> 

I certainly can change it to output that element always and assume
type='file' if it's not present. I wasn't sure about backcompat and thus
I went for extra state for the missing element.

I'll change it to this approach in the next version.

Peter


Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]