Re: [Libvirt-cim] [PATCH] Fix _diskpool_is_member() to return correct pool
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(a)linux.vnet.ibm.com