In case !require_match (disk snapshots or snapshot with external memory)
setting disk->snapshot if it is not set in snapshot definition is
straigtforward. First check default value for the disk from domain definition
(disk_snapshot) and use it if it is set, second if it is not set then use
default value provided as function argument. But require_match case is
trickier. For some reason it reverts this logic except for the case when
disk_snapshot is none. This logic added in commit [1]. AFAIU this is done
because mixing internal and external disks snapshots is not supported (and
still is). But then it is not clear why in case of disks snapshots
(!require_match) the logic is not the same as for internal snapshots because
mixing is not supported in both cases. Also it is seems very surprising that
for some snapshots domain disks settings are respected and for others are not.
Making exception for none seems complicating things further. AFAIU this
exception intention was to disable snapshots for readonly disks because for
these disks disk_snapshot is set to none on parsing stage.
I suggest to use same logic for require_match as for !require_match. This makes
things graspable in respect to settings priorities coming from different places.
And this breaks clients that set disks_snapshot to external and then makes
internal snapshots. I hope this was not used by anyone. There are no other
changes.
This function is used on snapshot redefine too. I guess users are not
supposed to remove snapshot attributes for disks from previously
dumped snapshot xmls.
[1] f9670b - snapshot: improve disk align checking
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@xxxxxxxxxxxxx>
---
src/conf/snapshot_conf.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index bd125dc..7d5367f 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -582,9 +582,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
disk_snapshot = def->dom->disks[idx]->snapshot;
if (!disk->snapshot) {
- if (disk_snapshot &&
- (!require_match ||
- disk_snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE))
+ if (disk_snapshot)
disk->snapshot = disk_snapshot;
else
disk->snapshot = default_snapshot;
--
1.8.3.1
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
