Re: [libvirt] [PATCH 1/3] Revert "storage: For FS pool check for properly formatted target volume"
On 01/12/2017 09:41 AM, Peter Krempa wrote:
On Thu, Jan 12, 2017 at 09:36:01 -0500, John Ferlan wrote:
>
>
> On 01/12/2017 09:24 AM, Peter Krempa wrote:
>> The check does not work properly (crashes) with netfs filesystems and
>> also checking that a device is not empty when attempting to mount a
>> filesystem is not very usefull since the mount will fail anyways.
>>
>> As the code would improve only a very minor corner case I don't really
>> see a reason to have this code at all.
>>
>> This code would also fail if libvirt is compiled without support for
>> blkid and without parted.
>>
>> This reverts commit a11fd69735e6951cda9bf256d8e423696a441aa4.
>> ---
>> src/storage/storage_backend_fs.c | 13 ++-----------
>> 1 file changed, 2 insertions(+), 11 deletions(-)
>>
>
> Instead of reverting why not just fix the issue.
Because I think the whole check is pointless. It's valid only when
mounting a local filesystem, and in that case basically the same check
is done when mounting the filesystem by the kernel.
Here's the difference between the check and removing the check (not
withstanding the no PARTED and no BLKID available)...
Create a pool using ext4, start it - life is happy. Destroy the pool.
Define a pool using xfs. Start it
w/ my change:
# virsh pool-start fs
error: Failed to start pool fs
error: Storage pool already built: Device '/dev/sde' formatted cannot
overwrite using 'xfs', requires build --overwrite
w/o my change (e.g. if the change is reverted):
# virsh pool-start fs
error: Failed to start pool fs
error: internal error: Child process (/usr/bin/mount -t xfs /dev/sde
/home/vm-images/fs) unexpected exit status 32: mount: wrong fs type, bad
option, bad superblock on /dev/sde,
missing codepage or helper program, or other error
In some cases useful info is found in syslog - try
dmesg | tail or so.
----
I personally like the former, but I do understand/agree that the latter
will also work.
John