[Libvirt-cim] [PATCH] Fix _diskpool_is_member() to return correct pool

[Kaitlin Rupert]

Dan Smith wrote:
> KR> Verifying the volume exists isn't enough to prove the disk_pool
> KR> struct is the proper pool.  We need to verify the volume is in the
> KR> pool of a given pool struct.
> 
> Hmm, that's bizarre.  Why don't all images show up in all pools then?
> I definitely agree with the second statement, but I find it
> interesting that we haven't seen this until now...

There was a bug in the test case we had.. you only see this issue if you 
have multiple diskpools defined.  Sometimes, you get lucky and the right 
pool is returned.  It depends on the order the pools are looped through.

> 
> KR>  static bool _diskpool_is_member(virConnectPtr conn,
> KR>                                  const struct disk_pool *pool,
> KR> -                                const char *file)
> KR> +                                const char *file,
> KR> +                                virStorageVolPtr vol)
> KR>  {
> 
> This change (well, the dependent one below) means that this code won't
> compile on libvirt < 0.4.0.

Ah, yuck.

> 
> KR> -        virStorageVolPtr vol = NULL;
> KR>          bool result = false;
> KR> +        virStoragePoolPtr pool_vol = NULL;
> KR> +        const char *pool_name = NULL;
> 
> KR> -        vol = virStorageVolLookupByPath(conn, file);
> KR> -        if (vol != NULL)
> KR> -                result = true;
> KR> -
> KR> +        pool_vol = virStoragePoolLookupByVolume(vol);
> KR> +        if (vol != NULL) {
> KR> +                pool_name = virStoragePoolGetName(pool_vol);                
> KR> +                if ((pool_name != NULL) && (STREQC(pool_name, pool->tag)))
> KR> +                        result = true;
> KR> +        }
> 
> Why make the caller do the lookup?  I can't see any reason why the
> virStorageVolLookupByPath() should be moved to the caller, which its
> part of the pool implementation and would work just fine here.
> 

You end up getting the vol each time through the loop - you really only 
need to get it once.

But good call about how it won't compile with libvirt < 0.4.0.  I'll 
re-work it (and test with a version of libvirt < 0.4.0 this time around =).

-- 
Kaitlin Rupert
IBM Linux Technology Center
kaitlin at linux.vnet.ibm.com