[Libvir] [PATCH] Do check the UUID in __virGetDomain()

Richard W.M. Jones rjones at redhat.com
Mon Jun 25 10:32:00 UTC 2007


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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 3237 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20070625/da5af477/attachment-0001.bin>


More information about the libvir-list mailing list