On 10/02/2014 11:17 AM, Shanzhi Yu wrote: > When create external system checkpoint snapshot, snapshot file for disks > should not be same with snapshot file for memory This is a case of user stupidity. We can't always protect the user from shooting themselves in the foot. If it is easy, it may be worth adding, but I'm not convinced it is easy. A more foolproof (but still not bulletproof) approach would be to track the memspec via the lease manager, since the lease manager already avoids collisions even across situations that aren't easy to spot by mere string equality. But that has it's own set of design questions (to date, the lease manager covers only disk images, not saved state images). > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1148932 > Signed-off-by: Shanzhi Yu <shyu@xxxxxxxxxx> > --- > src/conf/snapshot_conf.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > } else if (memoryFile) { > def->memory = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL; > + if (diskFile != NULL && STREQ(memoryFile,diskFile)){ STREQ_NULLABLE is a little more compact. However, this check is very weak - it does not detect things like 'a/b' and 'a//b' being the same file. I also think it is better to detect this via stat() at the point later on in the process where we sanitize the XML, rather than just here at parse time (where a user can leave the disk destination blank and rely on later code supplying a sane default, which would miss a collision with the memspec also using what would be the generated disk name). Thus, I think that if we are going to do this at all, it should be done in virDomainSnapshotAlignDisks(). -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list