Re: [Libvir] [PATCH] Do check the UUID in __virGetDomain()
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
Attachments:
- smime.p7s (application/x-pkcs7-signature — 3.2 KB)