In playing with this code for the afternoon, it looks like someone created getStoragePoolbyURI, then halfway through decided that it would be easier to do something else, then created getStoragePoolByUri. I say this because right at the point where the original would create a storage pool, there is a non-existent "addStoragePool" commented out, and it never does create one from the LibVirtStoragePoolDef it puts together. From that point it's pretty much unfinished.
So what I'm doing is to adjust the getStoragePoolByUri just slightly to use a static UUID. This is the code we're already calling so we'll just stop using a random UUID. Then I'll remove the unfinished getStoragePoolbyURI, as well as the getVolumeFromURI (which as edison says, nothing calls), since it depends on it. Hopefully this will reduce confusion going forward. On Wed, Sep 26, 2012 at 3:46 PM, Edison Su <edison...@citrix.com> wrote: > getVolumeFromURI and getVolumeFromURI are dead code, nobody calls them any > more. It's ok be removed. > >> -----Original Message----- >> From: Marcus Sorensen [mailto:shadow...@gmail.com] >> Sent: Wednesday, September 26, 2012 2:22 PM >> To: cloudstack-dev@incubator.apache.org >> Subject: Re: advice: trying to fix secondary/iso nfs storage on KVM >> >> I'd say it's pretty confusing to have two calls with the same name but >> differing upper/lower case. >> >> On Wed, Sep 26, 2012 at 3:21 PM, Marcus Sorensen <shadow...@gmail.com> >> wrote: >> > Yeah, that's what we want. What generates a random one is the >> > existing one we use, getStoragePoolByUri: >> > >> > uuid = UUID.randomUUID().toString(); >> > >> > >> > >> > On Wed, Sep 26, 2012 at 3:19 PM, Edison Su <edison...@citrix.com> >> wrote: >> >> After taking a look at the code carefully... What we really do is: >> don't generate a random uuid, but using >> >> uuid = UUID.nameUUIDFromBytes( >> >> new String(sourceHost + >> sourcePath).getBytes()).toString(); >> >> instead. >> >> >> >>> -----Original Message----- >> >>> From: Edison Su [mailto:edison...@citrix.com] >> >>> Sent: Wednesday, September 26, 2012 2:09 PM >> >>> To: cloudstack-dev@incubator.apache.org >> >>> Subject: RE: advice: trying to fix secondary/iso nfs storage on KVM >> >>> >> >>> Yes, you are right, getStoragePoolbyURI is the right one. >> >>> >> >>> > -----Original Message----- >> >>> > From: Marcus Sorensen [mailto:shadow...@gmail.com] >> >>> > Sent: Wednesday, September 26, 2012 2:03 PM >> >>> > To: cloudstack-dev@incubator.apache.org >> >>> > Subject: Re: advice: trying to fix secondary/iso nfs storage on >> KVM >> >>> > >> >>> > It looks like the aforementioned getStoragePoolbyURI takes care >> of >> >>> > this, that is it takes the host/path, generates a UUID based on >> it, >> >>> > and then searches existing storage pools for it. If none is found, >> it >> >>> > creates a new one, otherwise it uses the existing pool. >> >>> > >> >>> > So would changing KVMStoragePoolManager.java to do >> >>> getStoragePoolbyURI >> >>> > instead of getStoragePoolByUri be enough to fix? >> >>> > >> >>> > On Wed, Sep 26, 2012 at 2:58 PM, Edison Su <edison...@citrix.com> >> >>> wrote: >> >>> > > The latest libvirt(bundled in rhel 6.3) has this limitation, >> you >> >>> > can't create duplicate storage pool with the same host/path, thus >> >>> > breaks some of the logic in the current code. >> >>> > > To fix the issue, we need to list all the available storage >> pools >> >>> on >> >>> > kvm host, check one by one: if any one of storage pools has the >> same >> >>> > attribute(for nfs, it's host and path, for dir storage, it's path) >> as >> >>> > we are looking for, then return. >> >>> > > >> >>> > >> -----Original Message----- >> >>> > >> From: Marcus Sorensen [mailto:shadow...@gmail.com] >> >>> > >> Sent: Wednesday, September 26, 2012 1:32 PM >> >>> > >> To: cloudstack-dev@incubator.apache.org >> >>> > >> Subject: Re: advice: trying to fix secondary/iso nfs storage >> on >> >>> KVM >> >>> > >> >> >>> > >> I also notice that LibvirtComputingResource calls >> >>> > getStoragePoolByURI >> >>> > >> >> >>> > >> getStoragePoolByURI calls getStoragePoolByUri per >> >>> > >> KVMStoragePoolManager.java >> >>> > >> >> >>> > >> in LibvirtStorageAdaptor there exists both getStoragePoolByUri >> and >> >>> > >> getStoragePoolbyURI. The latter looks like it generates a UUID >> >>> based >> >>> > >> on the host/path of the secondary storage, and creates a >> storage >> >>> > pool >> >>> > >> based on it if needed. So I'm wondering if we are just >> calling >> >>> the >> >>> > >> wrong one, although this is called many times in >> >>> > >> LibvirtComputingResource so perhaps we intend both behaviors >> at >> >>> > >> certain points. >> >>> > >> >> >>> > >> To recap: >> >>> > >> >> >>> > >> getStoragePoolByUri will always try to create a new nfs pool >> with >> >>> a >> >>> > >> random UUID >> >>> > >> >> >>> > >> getStoragePoolbyURI attempts to generate a UUID from the >> >>> > >> sourcehost/path and uses that (static UUID) >> >>> > >> >> >>> > >> >> >>> > >> >> >>> > >> On Wed, Sep 26, 2012 at 2:10 PM, Marcus Sorensen >> >>> > <shadow...@gmail.com> >> >>> > >> wrote: >> >>> > >> > I noticed that I can no longer create multiple VMs from ISO. >> >>> After >> >>> > >> the >> >>> > >> > first one, I get an error where the agent is trying to >> create a >> >>> > >> > duplicate storage pool, throwing 'Storage source conflict >> with >> >>> > pool' >> >>> > >> > >> >>> > >> > In looking into it, it seems that with ISOs we call >> >>> > >> > getStoragePoolByURI with the path to the iso >> >>> > >> > >> >>> > >> > This generates a new random UUID and attempts to create a >> >>> storage >> >>> > >> pool >> >>> > >> > with this UUID via createStoragePool >> >>> > >> > >> >>> > >> > createStoragePool first looks to see if the pool passed >> already >> >>> > >> > exists, which it doesn't since we just generated the UUID >> out of >> >>> > thin >> >>> > >> > air. Then it attempts to create a new storage pool, and >> voila, >> >>> we >> >>> > get >> >>> > >> > the storage source conflict because we're trying to create a >> new >> >>> > >> > storage pool. >> >>> > >> > >> >>> > >> > So, I am left with a bunch of questions as to why it was >> done >> >>> this >> >>> > >> > way. Should I find a way to pull the secondary storage's >> real >> >>> UUID, >> >>> > >> > and use that in getStoragePoolByUri/createStoragePool, or >> should >> >>> I >> >>> > >> > continue using a random UUID, but fix createStoragePool to >> look >> >>> > for >> >>> > >> an >> >>> > >> > existing storage source before trying to generate a new nfs >> >>> > storage >> >>> > >> > pool?
