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@linux.vnet.ibm.com