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>
  </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.



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>.



"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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to