[libvirt] [PATCH] Drop virStorageBackendLogicalMatchPoolSource

[John Ferlan]

On 06/17/2016 11:16 AM, Ján Tomko wrote:
> On Thu, Jun 16, 2016 at 07:44:05AM -0400, John Ferlan wrote:
>>
>>
>> On 06/16/2016 06:03 AM, Ján Tomko wrote:
>>> On Wed, Jun 15, 2016 at 06:45:59PM -0400, John Ferlan wrote:
>>>>
>>>>
>>>> On 06/15/2016 01:19 PM, Ján Tomko wrote:
>>>>> Regression introduced by commit 71b803a for [1] that prevents
>>>>> starting up
>>>>> a logical pool created with <source><device path=''></source>
>>>>> after it has been moved to a different physical volume.
>>>>
>>>> Is there a bug for this?  XML examples?
>>>>
>>>> Is an empty source device path string a valid value?  Reading
>>>> http://libvirt.org/formatstorage.html doesn't give me that impression.
>>>
>>> I meant any pool with a <device> specified, not an empty path.
>>
>> But the patch is specifically targeted at removing the _logical pool
>> check.  So the question is, is it "legal" for a logical pool to "start"
>> with an empty string as a source device path? Secondarily I'm curious
>> what's the use case of that?
> 
> I do not know what happens with an empty string.
> <source><device path=''></source> was not an exact snippet,
> just a hint that there is a device path specified.
> 
> If you create a pool like this:
> 
> <pool type='logical'>
>  <name>vgname</name>
>  <source>
>    <device path='/dev/sda4'/>
>    <name>vgname</name>

As I see things, this more or less lets libvirt "know" that "/dev/sda4"
and "vgname" are associated.

>   </source>
>   <target>
>      <path>/dev/vgname</path>
>   </target>
> </pool>
> 
> Then pvmove /dev/sda4 to /dev/sda5, libvirt will refuse that pool
> even though <name>vgname</name> is enough to uniquely identify a storage
> pool.
> 

As an admin you take this step to pvmove your data from /dev/sda4 to
/dev/sda5.

But then you expect libvirt to know that? Do you expect libvirt to
automagically update the XML to /dev/sda5 during 'build' or 'start'?

>>
>>>
>>> The whole point of LVM is abstraction from the lower layers so we
>>> shouldn't ever be checking this.
>>>
>>
>> So it's OK to activate/start a pool where the source device path is
>> incorrect?
> 
> For LVM, the important path is in <source><name>.
> 

But isn't there a 1-to-1 relationship between the VG and the PV?  A PV
cannot be in more than one VG, so if you move your PV, then you have to
update your XML to let libvirt know.


John
>>
>>>>
>>>> "device
>>>>    Provides the source for pools backed by physical devices (pool types
>>>> fs, logical, disk, iscsi, zfs). May be repeated multiple times
>>>> depending
>>>> on backend driver. Contains a required attribute path which is either
>>>> the fully qualified path to the block device node or for iscsi the
>>>> iSCSI
>>>> Qualified Name (IQN). Since 0.4.1"
>>>>
>>>>
>>>>>
>>>>> For logical pools <source><name> contains the name of the volume group
>>>>> and uniquely identifies the VG on the host.
>>>>>
>>>>> This also speeds up startup for pools that do not have any <device>s
>>>>> specified.
>>>>>
>>>>> [1] https://bugzilla.redhat.com/show_bug.cgi?id=1025230
>>>>> ---
>>>>>  src/storage/storage_backend_logical.c | 104
>>>>> +---------------------------------
>>>>>  1 file changed, 2 insertions(+), 102 deletions(-)
>>>>>
>>>>
>>>> This essentially reverts a patch that was used to resolve a bz
>>>> without a
>>>> patch to resolve the issue in the bug.  What's the proposal/patch to
>>>> resolve the issue from the bug?
>>>>
>>>
>>> The issue in the bug is just cosmetic and should not block fixing this
>>> regresion.
>>>
>>
>> How is it just cosmetic?  Let's say we allow starting a pool with an
>> incorrect or invalid source device path. Any attempt to "use" or "list"
>> a volume in the pool would fail.
>>
> 
> The pool works fine, the logical volumes are under the same path in
> /dev/, the only thing that has changed is the underlying PV device.
> 
>> I will agree that it would seem unlikely in a "real world" situation
>> that an admin would create/configure a logical pool using a vgname that
>> is not associated with the provided source device path; however, to me
>> it seems just as unlikely that the source device path is either not
>> provided or is set to ''.
>>
> 
> Not providing the source device path at all is perfectly fine for
> existing logical pools, they are only needed for building new pools.
> 
> Jan