On 07/11/2011 06:50 AM, Daniel P. Berrange wrote:
>>
>> - if (virAsprintf(&ret, "%s/%s.save", driver->saveDir, vm->def->name) < 0) {
>> + if (virAsprintf(&ret, "%s/%s.save", driver->saveDir, uuidstr) < 0) {
>> virReportOOMError();
>> return(NULL);
>> }
>
> NACK, this is just papering over the problem IMHO, resulting in orphaned
> state files being left around forever. We should have deleted the state
> file when the guest was undefined.
I concur with the NACK. In fact, since we have the ability to
independently clean up the managed state files via
virDomainManagedSaveRemove, I think that it might even make sense to
have a flag:
virDomainUndefineFlags(,0) - fail if virDomainHasManagedSaveImage
returns true
virDomainUndefineFlags(,VIR_DOMAIN_UNDEFINE_ALL) - force deletion of
managed save file as well (that is, a combination of
virDomainManagedSaveRemove while ignoring failures, then virDomainUndefine)
By doing that, we then guarantee that undefine will fail unless the
managed save files are also cleaned up, at which point keeping the
managed save files around by name is reasonable (name is easier than
uuid for anyone perusing the managed state directory).
Also, is it possible to rename a domain by using virDomainDefine while
reusing the same UUID but a different name of an already defined VM? If
so, then that needs to take care of renaming any associated files, such
as existing managed save state. In fact, it might be worth providing a
convenience API of virDomainRename that makes it easier to rename
existing domains.
To some degree, I have the same issue at hand with my proposed API for
storage volume snapshot management - cleanup really does need a flag to
state whether snapshots are cleaned up or else the snapshot delete fails
because snapshots still exist.
--
Eric Blake eblake@xxxxxxxxxx +1-801-349-2682
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