Bug 1465373 - disallow file.logfile for gluster backing images
disallow file.logfile for gluster backing images
Status: ASSIGNED
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: libvirt (Show other bugs)
7.4
x86_64 Linux
unspecified Severity unspecified
: rc
: ---
Assigned To: Peter Krempa
Han Han
: FutureFeature, Reopened
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2017-06-27 06:21 EDT by yanqzhan@redhat.com
Modified: 2017-09-21 06:29 EDT (History)
9 users (show)

See Also:
Fixed In Version:
Doc Type: Enhancement
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2017-06-27 06:40:25 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description yanqzhan@redhat.com 2017-06-27 06:21:51 EDT
Description of problem:
Libvirt should support gluster with "file.logfile" option

Version-Release number of selected component (if applicable):
libvirt-3.2.0-14.el7.x86_64
qemu-kvm-rhev-2.9.0-14.el7.x86_64

How reproducible:
100%

Steps to Reproduce:
1.Create json backing image via inet gluster protocols with "file.logfile" option
# getenforce
Enforcing
# qemu-img create -f qcow2 -b 'json:{"file.driver":"gluster","file.volume":"gluster-vol1","file.path":"R.qcow2","file.logfile":"/tmp/log","file.debug":9,"file.server":[{"type":"inet","host":"10.66.*.*","port":"24007"}]}' /var/lib/libvirt/images/gluster_inet.img
Formatting '/var/lib/libvirt/images/gluster_inet.img', fmt=qcow2 size=104857600 backing_file=json:{"file.driver":"gluster",,"file.volume":"gluster-vol1",,"file.path":"R.qcow2",,"file.logfile":"/tmp/log",,"file.debug":9,,"file.server":[{"type":"inet",,"host":"10.66.*.*",,"port":"24007"}]} encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16

# ll -hZ /tmp/log
-rw-------. root root unconfined_u:object_r:user_tmp_t:s0 /tmp/log

2.Cold-plug the image to guest V:
# virsh attach-disk V  /var/lib/libvirt/images/gluster_inet.img vdb --subdriver qcow2 --config
Disk attached successfully

3.# virsh start V
error: Failed to start domain V
error: internal error: qemu unexpectedly closed the monitor: 2017-06-27T08:46:08.304690Z qemu-kvm: -chardev pty,id=charserial0: char device redirected to /dev/pts/2 (label charserial0)
ERROR: failed to create logfile "/tmp/log" (Permission denied)
2017-06-27T08:46:08.321864Z qemu-kvm: -drive file=/var/lib/libvirt/images/gluster_inet.img,format=qcow2,if=none,id=drive-virtio-disk1: Could not open backing file: Could not open image: Permission denied

4.Disable selinux and change permission for /tmp/log, then restart the guest again:
# setenforce 0
# chown qemu:qemu /tmp/log
# virsh start V
Domain V started


Actual results:
As in step3, when try to start a guest with gluster backing images with "file.logfile" option, guest will fail to start due to permission denied for the log file.

Expected results:
Libvirt should support gluster with "file.logfile" option, need a attribute 'logfile' in xml as the gluster client log location,
e.g.
<disk type='network' device='disk'>
<driver name='qemu' type='qcow2' cache='none'/>
<source protocol='gluster' name='gluster-vol1/V-7.4.qcow2'>
<host name='$gfs_IP' logfile='/tmp/log'/>
</source>
<target dev='vda' bus='virtio'/>
</disk>


Additional info:
Comment 1 Daniel Berrange 2017-06-27 06:28:35 EDT
IMHO this is an example of something we should in fact not support in general. Permanently embedding a log file path inside a disk image is a really terrible idea. A logfile path should be controlled by libvirt at guest startup, using a fixed path determined by libvirt.
Comment 2 Peter Krempa 2017-06-27 06:40:25 EDT
Logging for gluster disks can be enabled via the gluster_debug_level option in qemu.conf. Gluster then logs into the VM log file.

Libvirt should not take the log file from the backing image. It's possible that it might be worth adding that on a per-disk basis, but currently I don't really think it's worth.
Comment 3 Daniel Berrange 2017-06-27 06:43:47 EDT
I wonder if when launching QEMU, libvirt can explicitly set the 'file.logfile' parameter to empty, to forcably overridden any bad settings hardcoded in the disk image ?
Comment 4 Peter Krempa 2017-06-27 06:52:53 EDT
Currently we have instrumentation to only modify the properties of the top level image. QEMU has command line syntax which would allow this, but it's rather unpractical.

I dont't think that it's really worth doing it until we will track the full backing chain and be able to specify image properties individually, since it would require ugly hacky code to address the image in the backing chain while the image is detected automatically by qemu (not passed on the commandline)
Comment 5 Daniel Berrange 2017-06-27 06:57:22 EDT
We do at least extract the full backing chain from images when starting a VM / adding a disk. So perhaps we should explicitly report an error from that code if we see a logfile parameter set for gluster, so we get a clear upfront error message, rather than QEMU failing some time later ?
Comment 6 Peter Krempa 2017-06-27 07:03:14 EDT
Yes, that is possible and pretty trivial to do. Apart from starting the VM there are two cases that will perhaps need special attention. Snapshots with --reuse-external and block-copy with --reuse-external.

I'll repurpose this bug to track that. I think it makes sense to forbid it.

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