RHEL Engineering is moving the tracking of its product development work on RHEL 6 through RHEL 9 to Red Hat Jira (issues.redhat.com). If you're a Red Hat customer, please continue to file support cases via the Red Hat customer portal. If you're not, please head to the "RHEL project" in Red Hat Jira and file new tickets here. Individual Bugzilla bugs in the statuses "NEW", "ASSIGNED", and "POST" are being migrated throughout September 2023. Bugs of Red Hat partners with an assigned Engineering Partner Manager (EPM) are migrated in late September as per pre-agreed dates. Bugs against components "kernel", "kernel-rt", and "kpatch" are only migrated if still in "NEW" or "ASSIGNED". If you cannot log in to RH Jira, please consult article #7032570. That failing, please send an e-mail to the RH Jira admins at rh-issues@redhat.com to troubleshoot your issue as a user management inquiry. The email creates a ServiceNow ticket with Red Hat. Individual Bugzilla bugs that are migrated will be moved to status "CLOSED", resolution "MIGRATED", and set with "MigratedToJIRA" in "Keywords". The link to the successor Jira issue will be found under "Links", have a little "two-footprint" icon next to it, and direct you to the "RHEL project" in Red Hat Jira (issue links are of type "https://issues.redhat.com/browse/RHEL-XXXX", where "X" is a digit). This same link will be available in a blue banner at the top of the page informing you that that bug has been migrated.
Bug 1233129 - Libvirt should check if source device is used by a disk pool when defining a logical pool, vice versa
Summary: Libvirt should check if source device is used by a disk pool when defining a ...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: libvirt
Version: 7.2
Hardware: x86_64
OS: Linux
medium
medium
Target Milestone: rc
: ---
Assignee: John Ferlan
QA Contact: Meina Li
URL:
Whiteboard:
Depends On:
Blocks: 1401400
TreeView+ depends on / blocked
 
Reported: 2015-06-18 09:41 UTC by Yang Yang
Modified: 2018-04-10 10:35 UTC (History)
5 users (show)

Fixed In Version: libvirt-3.7.0-1.el7
Doc Type: No Doc Update
Doc Text:
undefined
Clone Of:
Environment:
Last Closed: 2018-04-10 10:33:22 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Product Errata RHEA-2018:0704 0 None None None 2018-04-10 10:35:51 UTC

Description Yang Yang 2015-06-18 09:41:53 UTC
Description of problem:
Currently, a logical pool can be defined/built/start when the source
device has been used by a running disk pool. It will make the disk pool
unavailable, vice versa.

Version-Release number of selected component (if applicable):
libvirt-1.2.16-1.el7.x86_64

How reproducible:
100%

Steps to Reproduce:
1. define/build/start a disk pool using /dev/sdc as source device
# virsh pool-define disk.xml
#virsh pool-build disk1
#virsh pool-start disk1

<pool type='disk'>
  <name>disk1</name>
  <uuid>9146cba7-efb3-4059-b791-f45e70b1ed26</uuid>
  <capacity unit='bytes'>0</capacity>
  <allocation unit='bytes'>0</allocation>
  <available unit='bytes'>0</available>
  <source>
    <device path='/dev/sdc'/>
    <format type='dos'/>
  </source>
  <target>
    <path>/dev</path>
    <permissions>
      <mode>0755</mode>
    </permissions>
  </target>
</pool>

2. define/build/start a logical pool using /dev/sdc as source device
#virsh pool-define logical.xml
#virsh pool-build HostVG
#virsh pool-start HostVG

<pool type='logical'>
  <name>HostVG</name>
  <uuid>469f950e-33de-470d-8e7a-63c285866f95</uuid>
  <capacity unit='bytes'>16018046976</capacity>
  <allocation unit='bytes'>209715200</allocation>
  <available unit='bytes'>15808331776</available>
  <source>
    <device path='/dev/sdc'/>
    <name>HostVG</name>
    <format type='lvm2'/>
  </source>
  <target>
    <path>/dev/HostVG</path>
    <permissions>
      <mode>0755</mode>
    </permissions>
  </target>
</pool>

3. refresh disk pool
# virsh pool-refresh disk1
error: Failed to refresh pool disk1
error: internal error: Child process (/usr/libexec/libvirt_parthelper /dev/sdc) unexpected exit status 2

Actual results:
in step 2, logical pool is defined/built/started successfully

Expected results:
It's better check if the source device is already used by some logical pool and disk pool

Additional info:

Comment 2 Yang Yang 2015-06-19 03:25:38 UTC
John,
In the case of gluster in following function, source will conflict once source dir and source host of any 2 pools are same. It does not take account of source name. So even if source names of 2 pools are different, they are treated to source conflicts. But in fact, they should not conflict.

virStoragePoolSourceFindDuplicate(virConnectPtr conn,
                                  virStoragePoolObjListPtr pools,
                                  virStoragePoolDefPtr def)
......
case VIR_STORAGE_POOL_GLUSTER:
            if (STREQ(pool->def->source.dir, def->source.dir) &&
                virStoragePoolSourceMatchSingleHost(&pool->def->source,
                                                    &def->source))
                matchpool = pool;
            break;

Repro steps
1. prepare 2 gluster volumes in a host
e.g. I have 2 volumes, gluster-vol1 and gluster-vol2 on my gluster server

# gluster volume info
 
Volume Name: gluster-vol1
Type: Distribute
Volume ID: 28c86ac7-eab3-436b-930f-8bfaa8d6559f
Status: Started
Snap Volume: no
Number of Bricks: 1
Transport-type: tcp
Bricks:
Brick1: 10.66.4.164:/br1
Options Reconfigured:
performance.readdir-ahead: on
server.allow-insecure: on
nfs.disable: on
auto-delete: disable
snap-max-soft-limit: 90
snap-max-hard-limit: 256
 
Volume Name: gluster-vol2
Type: Distribute
Volume ID: 7b96a8b7-56d4-4e94-bf4e-4fab7db56988
Status: Started
Snap Volume: no
Number of Bricks: 1
Transport-type: tcp
Bricks:
Brick1: 10.66.4.164:/br2
Options Reconfigured:
server.allow-insecure: on
performance.readdir-ahead: on
auto-delete: disable
snap-max-soft-limit: 90
snap-max-hard-limit: 256

2. define/start a gluster pool using gluster-vol1 as source
#virsh pool-define gluster.xml
#virsh pool-start gluster
# virsh pool-dumpxml gluster
<pool type='gluster'>
  <name>gluster</name>
  <uuid>6a65fea0-6546-41be-98f0-1c4180eadca9</uuid>
  <capacity unit='bytes'>75125227520</capacity>
  <allocation unit='bytes'>70293786624</allocation>
  <available unit='bytes'>4831440896</available>
  <source>
    <host name='10.66.4.164'/>
    <dir path='/'/>
    <name>gluster-vol1</name>
  </source>
</pool>

3. define one more gluster pool using gluster-vol2 as source
# cat gluster-pool.xml 
<pool type="gluster">
        <name>gluster1</name>
        <source>
          <name>gluster-vol2</name>  ----> source name is different from 1st gluster pool, but host and dir are same with 1st gluster pool
          <host name='10.66.4.164'/>
          <dir path='/'/>
        </source>
      </pool>
# virsh pool-define gluster-pool.xml 
error: Failed to define pool from gluster-pool.xml
error: operation failed: Storage source conflict with pool: 'gluster'

Regards 
Yang

Comment 3 Yang Yang 2015-06-19 06:03:41 UTC
Another concern about gluster pool is that, given a gluster volume consists of over 2 bricks, define 1st gluster pool with host-0, then define 2st gluster pool with host-1, both pools have same source name and dir path (IOW, both pools are using same gluster volume as source), the function cannot check thus conflict. Maybe it's difficult to check.

Repro steps
1. prepare a gluster volume consists of 2 bricks

# gluster volume info gluster-vol2
 
Volume Name: gluster-vol2
Type: Distribute
Volume ID: 7b96a8b7-56d4-4e94-bf4e-4fab7db56988
Status: Started
Snap Volume: no
Number of Bricks: 2
Transport-type: tcp
Bricks:
Brick1: 10.66.4.164:/br2
Brick2: 10.66.5.63:/br2
Options Reconfigured:
server.allow-insecure: on
performance.readdir-ahead: on
auto-delete: disable
snap-max-soft-limit: 90
snap-max-hard-limit: 256

2. define 1st gluster pool using Brick1 as source
# virsh pool-define gluster.xml
Pool gluster defined from gluster.xml
# virsh pool-dumpxml gluster
<pool type='gluster'>
  <name>gluster</name>
  <uuid>6a65fea0-6546-41be-98f0-1c4180eadca9</uuid>
  <capacity unit='bytes'>158970347520</capacity>
  <allocation unit='bytes'>107715432448</allocation>
  <available unit='bytes'>51254915072</available>
  <source>
    <host name='10.66.4.164'/>
    <dir path='/'/>
    <name>gluster-vol2</name>
  </source>
</pool>

3. define 2st gluster pool using Brick2 as source
# virsh pool-define gluster-pool.xml 
Pool gluster1 defined from gluster-pool.xml
# virsh pool-dumpxml gluster1
<pool type='gluster'>
  <name>gluster1</name>
  <uuid>1fa4a4c7-0828-436a-a7f4-e5655ab01968</uuid>
  <capacity unit='bytes'>158970347520</capacity>
  <allocation unit='bytes'>107715416064</allocation>
  <available unit='bytes'>51254931456</available>
  <source>
    <host name='10.66.5.63'/>
    <dir path='/'/>
    <name>gluster-vol2</name>
  </source>
</pool>

Comment 4 John Ferlan 2015-06-23 21:09:15 UTC
Mixing disk, logical, and gluster in one bz is to say the least confusing.

With respect to disk/logical and using /dev/sdc - that's perhaps something that can be addressed.  Of course cross pool checking in general isn't address.  I'm sure you could come up with a way to take an iSCSI pool created /dev/sdX and use it for a disk pool (it's what I do), so restricting it is a double edged sword. I'm not as sure this is a "bug" per se - it may be one of those you know your storage configuration and not configure in this manner. If you do, then you're on your own.  I'm going to condnak design on this as the existing duplicate algorithm is heavily reliant on the pool types being the same.  Having to build in knowledge of pool types that "could" use a "similar" path could be a bit more tricky.  Realistically the best fix would be some sort of locking system that would handle these cross pool issues.

As for gluster, the check done in virStoragePoolSourceFindDuplicate is meant to cover some generic cases. I request a different bug on this.  Yes, it is much more difficult to check duplication.  You can go out of your way to make some sort of duplication occur, but is that a "real world" example?

Comment 5 Yang Yang 2015-06-29 02:58:42 UTC
(In reply to John Ferlan from comment #4)
> Mixing disk, logical, and gluster in one bz is to say the least confusing.
> 
> With respect to disk/logical and using /dev/sdc - that's perhaps something
> that can be addressed.  Of course cross pool checking in general isn't
> address.  I'm sure you could come up with a way to take an iSCSI pool
> created /dev/sdX and use it for a disk pool (it's what I do), so restricting
> it is a double edged sword. I'm not as sure this is a "bug" per se - it may
> be one of those you know your storage configuration and not configure in
> this manner. If you do, then you're on your own.  I'm going to condnak
> design on this as the existing duplicate algorithm is heavily reliant on the
> pool types being the same.  Having to build in knowledge of pool types that
> "could" use a "similar" path could be a bit more tricky.  Realistically the
> best fix would be some sort of locking system that would handle these cross
> pool issues.
> 
> As for gluster, the check done in virStoragePoolSourceFindDuplicate is meant
> to cover some generic cases. I request a different bug on this.  Yes, it is
> much more difficult to check duplication.  You can go out of your way to
> make some sort of duplication occur, but is that a "real world" example?

Opened a separate bug 1236438 to track issues for gluster. Yes, it is common example in the real world. As a cluster, a gluster volume often consists of multiple bricks. So users can use any brick as pool source.

Comment 6 John Ferlan 2015-07-14 18:21:36 UTC
Moving to consideration for 7.3

Comment 8 John Ferlan 2016-07-28 14:25:52 UTC
moving to 7.4

Comment 9 John Ferlan 2017-04-05 14:45:53 UTC
Posted some patches upstream that will resolve this:

https://www.redhat.com/archives/libvir-list/2017-April/msg00243.html

Comment 10 John Ferlan 2017-04-13 14:22:55 UTC
Pushed patches upstream:

commit 4143b194ce29867929b05517c178ef9371ebb8b4
Author: John Ferlan <jferlan>
Date:   Wed Apr 5 09:04:54 2017 -0400

    conf: Check for storage conflicts across pool types
    
...
    
    The virStoragePoolObjSourceFindDuplicate logic used by PoolCreateXML
    and PoolDefineXML avoids comparing the new definition against "other"
    pool types. This can cause unexpected corruption if two different pool
    source types used the same source device path. For example, a 'disk'
    pool using source type device=/dev/sdc could be unwittingly overwritten
    by using /dev/sdc for a 'logical' pool which also uses the source
    device path.
    
    So rather than blindly ignoring those checks when def->type !=
    pool->def->type - have the pool->def->type switch logic handle the
    check for which def->type's should be checked.

$ git describe 4143b194ce29867929b05517c178ef9371ebb8b4
v3.2.0-158-g4143b19
$

Comment 13 Meina Li 2017-12-21 02:28:27 UTC
Verified components version:
libvirt-3.9.0-6.el7.x86_64

Verified steps:
1. Define/build/start a logical pool using /dev/sdb as source device.
# virsh pool-dumpxml HostVG
<pool type='logical'>
  <name>HostVG</name>
  <uuid>db456984-d54b-4e6d-a36d-ea939fab00d2</uuid>
  <capacity unit='bytes'>1069547520</capacity>
  <allocation unit='bytes'>314572800</allocation>
  <available unit='bytes'>754974720</available>
  <source>
    <device path='/dev/sdb'/>
    <name>HostVG</name>
    <format type='lvm2'/>
  </source>
  <target>
    <path>/dev/HostVG</path>
  </target>
</pool>

2. Define/create a disk pool using the same /dev/sdb sa source device.
# vim disk-pool.xml
<pool type='disk'>
  <name>disk1</name>
  <uuid>9146cba7-efb3-4059-b791-f45e70b1ed26</uuid>
  <source>
    <device path='/dev/sdb'/>
    <format type='dos'/>
  </source>
  <target>
    <path>/dev</path>
    <permissions>
      <mode>0755</mode>
    </permissions>
  </target>
</pool>

# virsh pool-define disk-pool.xml 
error: Failed to define pool from disk-pool.xml
error: operation failed: Storage source conflict with pool: 'HostVG'
# virsh pool-create disk-pool.xml 
error: Failed to create pool from disk-pool.xml
error: operation failed: Storage source conflict with pool: 'HostVG'

3. Change /dev/sdb to /dev/sdc in disk-pool.xml, define/create disk pool successfully.

4. Opposite test: define/create disk pool firstly, and then define/create logical pool using the same source device with disk pool, the result is expected.

Move it to verified.

Comment 17 errata-xmlrpc 2018-04-10 10:33:22 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHEA-2018:0704


Note You need to log in before you can comment on or make changes to this bug.