On 10/29/2014 09:33 AM, Ján Tomko wrote:
On 10/29/2014 01:49 PM, John Ferlan wrote:
<...snip...>
I think I'm missing your point. The finding of duplicate sources for storage pools and disallowing them to be defined (or created) has been around since commit id '5a1f27287".
I'm not sure what you meant by your first sentence - perhaps an example would help me especially in the accepted, but invalid condition.
Perhaps 'invalid' wasn't the best choice - the definition itself is valid, but it doesn't refer to an existing device so the pool cannot be started up.
It's possible to define a pool using parentaddr (even on a host with no SCSI, as the address is only used on pool startup, not definition): <adapter type='scsi_host'> <parentaddr unique_id='1'> <address domain='0x0000' bus='0x00' slot='0x1f' function='0x0'/> </parentaddr> </adapter>
After that, defining a pool with scsi_host source fails. Both via name: <adapter type='scsi_host' name='scsi_host13'/> and via parenaddr: <adapter type='scsi_host'> <parentaddr unique_id='1'> <address domain='0x0000' bus='0x00' slot='0x10' function='0x0'/> </parentaddr> </adapter> because of the first, already accepted definition: error: Failed to define pool from scsi2.xml error: XML error: Failed to find scsi_host using PCI '0000:00:1f.0' and unique_id='1'
OK - so the issue you are bringing up resolves around an "incorrect" definition of a pool that will fail on startup and why would restrict someone from creating a second, but perhaps correct definition rather than perhaps fixing their first definition. As an interesting corollary how is this any different than the following example using a DIR pool with a bad target : # cat x1.xml <pool type='dir'> <name>x1</name> <source> </source> <target> <path>/avr/lib/libvirt/images</path> <permissions> <mode>0755</mode> <owner>-1</owner> <group>-1</group> </permissions> </target> </pool> # cat x2.xml <pool type='dir'> <name>x2</name> <source> </source> <target> <path>/var/lib/libvirt/images</path> <permissions> <mode>0755</mode> <owner>-1</owner> <group>-1</group> </permissions> </target> </pool> # virsh pool-define x1.xml Pool x1 defined from x1.xml # virsh pool-define x2.xml error: Failed to define pool from x2.xml error: operation failed: Storage source conflict with pool: 'x1' # virsh pool-start x1 error: Failed to start pool x1 error: cannot open path '/avr/lib/libvirt/images': No such file or directory While I see your point, I guess I would expect someone to edit their current incorrect definition rather than trying to create a new one and use that instead. I think this code follows the spirit of what other code does and enforces only 1 target per pool at definition time regardless of whether that target is valid.That's a different problem which is always a bit controversial - allowing bogus definitions to exist. John