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

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 :)

> @@ -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.