Bug 1439132

Ugh... Not sure exactly what happened, but I easily saw the problem the 'pc98' and 'dvh' types were being left to be "validated" by parted rather than blkid, but for some reason the code missed an 'else' condition when doing a check.  As for the 'bsd' - even stranger. I know I tried every type with the adjusted algorithm, but it seems even though the code can create the 'bsd' table via parted, the recognition of that using blkid is not succeeding - I'll just add it to the list of partitions that parted will check. Finally, I found during some extra matrix testing that if a device is formatted with 'gpt' and then an attempt is made to format using 'mac', the format fails. However, by clearing out '2048' bytes from the beginning of the device that issue goes away - so I've added that too (essentially "reused" the code from bz 1430679).

I have posted patches upstream to resolve the issues:

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

After a partially successful review of the series, I pushed separated pieces of patch 3 of the series - a patch dealing with the missed "else" and the one adding "bsd" to the list of format types to not try to decode using blkid:

commit f2a123203105b4fe010aea866e87f8bcf5f31193
Author: John Ferlan <jferlan>
Date:   Fri Apr 7 07:02:40 2017 -0400

    disk: Resolve issues with disk partition build/start checks
    
...
   
    Commit id 'a48c674fb' added a check for format types "dvh" and "pc98"
    to use the parted print processing instead of using blkid processing
    in order to validate the label on the disk was what is expected for
    disk pool startup. However, commit id 'a4cb4a74f' really messed things
    up by missing an else condition causing PARTEDFindLabel to always
    return DIFFERENT.

$ git describe f2a123203105b4fe010aea866e87f8bcf5f31193
v3.2.0-81-gf2a1232


commit 98f424d5038b362d1b62549930d0b9253106bdca
Author: John Ferlan <jferlan>
Date:   Fri Apr 7 09:38:44 2017 -0400

    disk: Force usage of parted when checking disk format for "bsd"

...    
    
    Add "bsd" to the list of format types to not checked during blkid
    processing even though it supposedly knows the format - for some
    (now unknown) reason it's returning partition table not found. So
    let's just let PARTED handle "bsd" too.
    
$ git describe 98f424d5038b362d1b62549930d0b9253106bdca
v3.2.0-82-g98f424d

I sent a new series to handle review feedback regarding the gpt and mac issue, see here:

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

The new series will wipe 1M from both the head and tail of the device since as it turns out the GPT table writes information on both ends of the device.

With a slight adjustment to the clearing algorithm from review, this series is now pushed upstream:

$ git describe 3c4f2e3fb725054921f855a229afc7daca7119ae
v3.2.0-239-g3c4f2e3
$ git show
commit 3c4f2e3fb725054921f855a229afc7daca7119ae
Author: John Ferlan <jferlan>
Date:   Fri Apr 7 09:36:05 2017 -0400

    disk: Use virStorageBackendZeroPartitionTable
    
...
    
    During 'matrix' testing of all possible combinations I found that if
    device is formated with "gpt" first, then an attempt is made to format
    using "mac", a startup will fail.
    
    Deeper analysis by Peter Krempa indicates that the "mac" table fits
    into the first block on the disk. Since the GPT disklabel is stored
    at LBA address 1 it is not overwritten at all. Thus it's apparent that
    the (blkid) detection tool then prefers GPT over a older disklabel.
    
    The GPT disklabel has also a secondary copy at the last LBA of the disk.
    
    So, follow the same logic as the logical pool in clearing a 1MB swath
    at the beginning and end of the device to avoid potential issues with
    larger sector sizes for the device.
    
    Also fixed a minor formatting nit in virStorageBackendDeviceIsEmpty call.

Versions:
libvirt-3.2.0-4.el7.x86_64
qemu-kvm-rhev-2.9.0-3.el7.x86_64
kernel-3.10.0-664.el7.x86_64

Step to verify:
the script is as follows:
# cat action.sh 
virsh pool-define pool.xml
virsh pool-build disk-pool --overwrite
virsh pool-start disk-pool
parted /dev/sdb p |grep -i partition
virsh pool-destroy disk-pool
virsh pool-undefine disk-pool
mkfs.ext4 /dev/sdb

the pool.xml is as follows:
# cat pool.xml 
<pool type='disk'>
  <name>disk-pool</name>
  <capacity unit='bytes'>0</capacity>
  <allocation unit='bytes'>0</allocation>
  <available unit='bytes'>0</available>
  <source>
    <device path='/dev/sdb'/>
    <format type='**'/>
  </source>
  <target>
    <path>/dev</path>
  </target>
</pool>

1:when the value in "<format type='**'/>" is dvh
	# sh action.sh 
	Pool disk-pool defined from pool.xml

	Pool disk-pool built

	Pool disk-pool started

	Partition Table: dvh
	Pool disk-pool destroyed

	Pool disk-pool has been undefined

2:when the value in "<format type='**'/>" is bsd
	# sh action.sh 
	Pool disk-pool defined from pool.xml

	Pool disk-pool built

	Pool disk-pool started

	Partition Table: bsd
	Pool disk-pool destroyed

	Pool disk-pool has been undefined

3:when the value in "<format type='**'/>" is pc98
	# sh action.sh 
	Pool disk-pool defined from pool.xml

	Pool disk-pool built

	Pool disk-pool started

	Partition Table: pc98
	Pool disk-pool destroyed

	Pool disk-pool has been undefined

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-2017:1846

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-2017:1846