Bug 1363586

Summary: "pool-build --no-overwrite" still mkfs a partition even if the partition has existing filesystem
Product: Red Hat Enterprise Linux 7 Reporter: yisun
Component: libvirtAssignee: John Ferlan <jferlan>
Status: CLOSED ERRATA QA Contact: Pei Zhang <pzhang>
Severity: medium Docs Contact:
Priority: medium    
Version: 7.3CC: dyuan, jferlan, rbalakri, xuzhang, yanyang, ydu, yisun
Target Milestone: rc   
Target Release: ---   
Hardware: x86_64   
OS: Linux   
Whiteboard:
Fixed In Version: libvirt-3.0.0-1.el7 Doc Type: Bug Fix
Doc Text:
Cause: Inconsistent usage of OVERWRITE flags for storage backends Consequence: It was possible to overwrite data on a file system backend if the XML format found in a build operation differed from the format on the target volume Fix: Additional checks were added to help ensure when attempting to build a file system that the target device doesn't already have something Result: Attempting to build a file system pool will properly fail and issue an error message
Story Points: ---
Clone Of: Environment:
Last Closed: 2017-08-01 17:11:42 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description yisun 2016-08-03 06:36:27 UTC
Description:
"pool-build --no-overwrite" still mkfs a partition even if the partition has existing filesystem

How reproducible
100%

Version:
libvirt-2.0.0-3.el7.x86_64

Steps:
1. # cat pool.fs
<pool type='fs'>
  <name>fs</name>
  <source>
    <device path='/dev/sdc1'/>
    <format type='xfs'/>
  </source>
  <target>
    <path>/fspool</path>
  </target>
</pool>


2. # virsh pool-define pool.fs
Pool fs defined from pool.fs

3. Let's use pool-build with "overwrite" to format that partition
# virsh pool-build --overwrite fs
Pool fs built


4. with parted print, we can see sdc1 now has xfs format.
# parted /dev/sdc p
Model: IET VIRTUAL-DISK (scsi)
Disk /dev/sdc: 42.9GB
Sector size (logical/physical): 512B/512B
Partition Table: msdos
Disk Flags:

Number  Start  End    Size   Type     File system  Flags
 1      512B   100MB  100MB  primary  xfs


5. Edit the pool and change the format from xfs to ext4
# virsh pool-edit fs
<pool type='fs'>
  <name>fs</name>
  ...
  <source>
    <device path='/dev/sdc1'/>
*****    <format type='ext4'/>  *******
  </source>
  ...
</pool>

6. Execute pool-build with --no-overwrite to that pool
# virsh pool-build fs --no-overwrite
Pool fs built

7. check the /dev/sdc1's format now, it's changed to "ext4"
# parted /dev/sdc p
Model: IET VIRTUAL-DISK (scsi)
Disk /dev/sdc: 42.9GB
Sector size (logical/physical): 512B/512B
Partition Table: msdos
Disk Flags:

Number  Start  End    Size   Type     File system  Flags
 1      512B   100MB  100MB  primary  ext4

Actual result:
When a file system existing on sdc1, and we use pool-build with --no-overwrite, sdc1 still being formatted.

Expected result:
If a filesystem existing on a partition, "pool-build --no-overwrite" should report an error and not format that partition.

Additional info:
In virsh manual for "pool-build", there is:
"If --no-overwrite is specified, it probes to determine if a filesystem already exists on the target device, returning an error if exists, or using mkfs to format the target device if not."

Comment 1 John Ferlan 2016-11-15 23:48:56 UTC
Posted a couple of patches to resolve:

http://www.redhat.com/archives/libvir-list/2016-November/msg00784.html

Comment 2 John Ferlan 2016-12-15 21:46:17 UTC
Updated patch found in patch 2/11 of the series:

http://www.redhat.com/archives/libvir-list/2016-December/msg00683.html

direct link is:

http://www.redhat.com/archives/libvir-list/2016-December/msg00685.html


This ends up being a bit more tricky due to how the 'overwrite' feature was implemented for file system storage backends.  Details are in the patch.

Comment 3 John Ferlan 2017-01-10 14:12:14 UTC
Patch to resolve now pushed upstream:

$ git describe f23d4bbce39b8f7d2228078947ce507ccc993ef0
v2.5.0-360-gf23d4bb
$

Comment 5 Pei Zhang 2017-03-07 09:10:43 UTC
Verified version :
libvirt-3.1.0-1.el7.x86_64
qemu-kvm-rhev-2.8.0-5.el7.x86_64

Steps:
scenario 1: 
pool-build with --no-overwrite will report an error if the device has a different fs type on the device.

1. Prepare a xml of fspool as following :
# cat fspool.xml 
<pool type='fs'>
  <name>fs</name>
  <capacity unit='bytes'>0</capacity>
  <allocation unit='bytes'>0</allocation>
  <available unit='bytes'>0</available>
  <source>
    <device path='/dev/sdb'/>
    <format type='xfs'/>   
  </source>
  <target>
    <path>/var/lib/libvirt/images/fspool</path>
  </target>
</pool>

2. Define fs pool
# virsh pool-define fspool.xml 
Pool fs defined from fspool.xml

3. Build fs pool with --overwrite
# virsh pool-build fs --overwrite
Pool fs built

4. check file system on device :

# parted /dev/sdb
GNU Parted 3.1
Using /dev/sdb
......
Disk Flags: 

Number  Start  End     Size    File system  Flags
 1      0.00B  4008MB  4008MB  xfs

5. edit fs pool, change format type to ext4
# virsh pool-edit fs
Pool fs XML configuration edited.

6. rebuild the fs pool again, with --no-overwrite, it should report an error.
# virsh pool-build fs --no-overwrite
error: Failed to build pool fs
error: Storage pool already built: Format of device '/dev/sdb' does not match the expected format 'ext4', forced overwrite is necessary

7. based on the error message, build with overwrite.
# virsh pool-build fs --overwrite
Pool fs built

# parted /dev/sdb p
Model:  USB DISK 2.0 (scsi)
Disk /dev/sdb: 4008MB
......
Disk Flags: 

Number  Start  End     Size    File system  Flags
 1      0.00B  4008MB  4008MB  ext4

 
Scenario 2: pool-build with --no-overwrite will report an error when fs type existing on the device even it matches the type configured in xml.

1. check fs on device.
# parted /dev/sdb p
Model:  USB DISK 2.0 (scsi)
Disk /dev/sdb: 4008MB
......
Disk Flags: 

Number  Start  End     Size    File system  Flags
 1      0.00B  4008MB  4008MB  ext4

2. define a fs pool with format type is 'ext4', and build it with --no-overwrite

# virsh pool-build fs --no-overwrite
error: Failed to build pool fs
error: Storage pool already built: Device '/dev/sdb' already formatted using 'ext4'

scenario 3: 
If no filesystem already exists on device, mkfs to format the device as configured in xml.

1. check fs on the device, no fs on it.
# parted /dev/sdc2 p
Error: /dev/sdc2: unrecognised disk label
Model: Unknown (unknown)                                                  
Disk /dev/sdc2: 8590MB
Sector size (logical/physical): 512B/512B
Partition Table: unknown
Disk Flags: 

2. define a fs pool as following
# virsh pool-dumpxml fs
<pool type='fs'>
  <name>fs</name>
  <uuid>ac3b9ac3-9807-4875-9844-a9e232d48e03</uuid>
  <capacity unit='bytes'>0</capacity>
  <allocation unit='bytes'>0</allocation>
  <available unit='bytes'>0</available>
  <source>
    <device path='/dev/sdc2'/>
    <format type='ext4'/>
  </source>
  <target>
    <path>/var/lib/libvirt/images/fspool</path>
    ......
  </target>
</pool>

2. build pool with --no-overwrite, it should build successfully.
# virsh pool-build fs --no-overwrite
Pool fs built

# parted /dev/sdc2 p
Model: Unknown (unknown)
Disk /dev/sdc2: 8590MB
Sector size (logical/physical): 512B/512B
Partition Table: loop
Disk Flags: 

Number  Start  End     Size    File system  Flags
 1      0.00B  8590MB  8590MB  ext4

scenario 4: check man page for modified description  
#man virsh 
find pool-build cmd and check the updated description for file system pool.

As above, move this bug to verified.

Comment 6 errata-xmlrpc 2017-08-01 17:11:42 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-2017:1846

Comment 7 errata-xmlrpc 2017-08-01 23:53:19 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-2017:1846