Masayuki Sunou wrote:
Hi Dan
I noticed that I misunderstood your suggestion.
--> I heard the point that was not good of this patch which Daniel
Veillard thought from Saori Fukuta.
Thanks Daniel and Saori.
I misunderstood that the point that is missing a call of virGetDomain()
exists before this patch is applied.
But your indication is that the point that is missing a call of
virGetDomain() emerges after this patch is applied.
Therefore, I remake this patch.
This patch changes as follows.
- the argument of virFreeDomain() doesn't change.
- virFreeDomainInternal() is added as the function that is called
only from hash.c.
Hello Masayuki,I think you're misunderstanding the original problem. It looks like src/virsh.c is missing (many) calls to virDomainFree.
If we go back to your original email:
1. Start virsh in interactive mode. 2. Execute domuuid to the domain 3. Execute undefine to the domain which executed domuuid in 2. 4. Create the domain whose name is same as the domain that executed undefine. 5. Execute domuuid for the new domain
Now compare this to what src/virsh.c is doing for each of these calls:2. In cmdDomuuid, we get a new virDomain object. This is allocated in src/hash.c. dom->uses == 1
3. In cmdUndefine, we do another virDomainsLookupBy* function. This returns the same domain object as step (2), but now dom->uses == 2.
4. In cmdDefine, we call virDomainDefineXML. Since this has the same name (see src/hash.c: __virGetDomain), it returns the same domain object as step (2) & (3), but now dom->uses == 3.
etc.So I think we can see the problem: Only some of the command functions in src/virsh.c are freeing the domain object correctly. Some of them do it correctly (cmdResume), others not at all (cmdDomuuid), and no doubt there are some which only get it partially right because they don't consider error cases properly.
I'm sure naysayers will be glad to know that the mlvirsh (OCaml virsh) which I wrote over the weekend also didn't get this right, although the solution there was simpler: just invoke the garbage collector manually between each interactive command so that unreachable domains are freed. (A finaliser was already in place to call virDomainFree, but with garbage collectors you can never be sure when finalisers will run. What we need are deterministic finalizers linked to object scope but no one's quite worked out how to implement that efficiently).
Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature