<div dir="ltr"><div>Hello Cole, one issue is found:</div><div>The qcow2 data file XTTRs is not cleaned on external snapshot when -blockdev is not enabled</div><div><br></div><div>Versions:</div><div>libvirt v5.8.0-134-g9d03e9adf1</div><div>qemu-kvm-4.1.0-13.module+el8.1.0+4313+ef76ec61.x86_64</div><div><br></div><div>Steps:</div><div>1. Convert a OS image to qcow2&qcow2 data file:</div><div># qemu-img convert -O qcow2 -o data_file=/var/lib/libvirt/images/pc-data.raw,data_file_raw=on /var/lib/libvirt/images/pc.qcow2 /var/lib/libvirt/images/pc-data.qcow2</div><div><br></div><div>2. Build and start libvirt source, start libvirt daemon:</div><div># make clean && CC=/usr/lib64/ccache/cc ./autogen.sh&&./configure --without-libssh --build=x86_64-redhat-linux-gnu --host=x86_64-redhat-linux-gnu --program-prefix= --disable-dependency-tracking --prefix=/usr --exec-prefix=/usr --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc --datadir=/usr/share --includedir=/usr/include --libdir=/usr/lib64 --libexecdir=/usr/libexec --localstatedir=/var --sharedstatedir=/var/lib --mandir=/usr/share/man --infodir=/usr/share/info --with-qemu --without-openvz --without-lxc --without-vbox --without-libxl --with-sasl --with-polkit --with-libvirtd --without-phyp --with-esx --without-hyperv --without-vmware --without-xenapi --without-vz --without-bhyve --with-interface --with-network --with-storage-fs --with-storage-lvm --with-storage-iscsi --with-storage-iscsi-direct --with-storage-scsi --with-storage-disk --with-storage-mpath --with-storage-rbd --without-storage-sheepdog --with-storage-gluster --without-storage-zfs --without-storage-vstorage --with-numactl --with-numad --with-capng --without-fuse --with-netcf --with-selinux --with-selinux-mount=/sys/fs/selinux --without-apparmor --without-hal --with-udev --with-yajl --with-sanlock --with-libpcap --with-macvtap --with-audit --with-dtrace --with-driver-modules --with-firewalld --with-firewalld-zone --without-wireshark-dissector --without-pm-utils --with-nss-plugin '--with-packager=Unknown, 2019-08-19-12:13:01, <a href="http://lab.rhel8.me">lab.rhel8.me</a>' --with-packager-version=1.el8 --with-qemu-user=qemu --with-qemu-group=qemu --with-tls-priority=@LIBVIRT,SYSTEM --enable-werror --enable-expensive-tests --with-init-script=systemd --without-login-shell && make -j8</div><div># LD_PRELOAD="$(find src -name '*.so.*'|tr '\n' ' ')" src/.libs/virtlogd</div><div># LD_PRELOAD="$(find src -name '*.so.*'|tr '\n' ' ')" LIBVIRT_DEBUG=3 LIBVIRT_LOG_FILTERS="1:util 1:qemu 1:security" LIBVIRT_LOG_OUTPUTS="1:file:/tmp/libvirt_daemon.log" src/.libs/libvirtd</div><div><br></div><div>3. Define and start an VM with the qcow2&qcow2 data file. Note that the -blockdev is not enabled<br></div><div># virsh define pc-data.xml</div><div># virsh start pc-data</div><div><br></div><div>4. Create snapshot and check the data file XATTRs:</div><div># virsh snapshot-create-as pc-data s1 --no-metadata --disk-only</div><div># getfattr -m - -d /var/lib/libvirt/images/pc-data.raw <br>getfattr: Removing leading '/' from absolute path names<br># file: var/lib/libvirt/images/pc-data.raw<br>security.selinux="unconfined_u:object_r:svirt_image_t:s0:c775,c1011"<br>trusted.libvirt.security.dac="+107:+107"<br>trusted.libvirt.security.ref_dac="1"<br>trusted.libvirt.security.ref_selinux="1"<br>trusted.libvirt.security.selinux="unconfined_u:object_r:svirt_image_t:s0:c284,c367"<br>trusted.libvirt.security.timestamp_dac="1563328069"<br>trusted.libvirt.security.timestamp_selinux="1563328069"</div><div><br></div><div>Shutdown the VM. The XATTRs of data file is not changed.<br></div><div>It is not expected. The XTTRs should not contain *.libvirt.*</div><div><br></div><div>Issue is not reproduced with -blockdev enabled:</div><div><domain type='kvm' xmlns:qemu='<a href="http://libvirt.org/schemas/domain/qemu/1.0">http://libvirt.org/schemas/domain/qemu/1.0</a>'><br>...<br> <qemu:capabilities><br> <qemu:add capability='blockdev'/><br> <qemu:del capability='drive'/><br> </qemu:capabilities><br></domain></div><div><br></div><div>See the libvirt daemon log and vm xml in attachment.<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Oct 8, 2019 at 5:49 AM Cole Robinson <<a href="mailto:crobinso@redhat.com">crobinso@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">This series is the first steps to teaching libvirt about qcow2<br>
data_file support, aka external data files or qcow2 external metadata.<br>
<br>
A bit about the feature: it was added in qemu 4.0. It essentially<br>
creates a two part image file: a qcow2 layer that just tracks the<br>
image metadata, and a separate data file which is stores the VM<br>
disk contents. AFAICT the driving use case is to keep a fully coherent<br>
raw disk image on disk, and only use qcow2 as an intermediate metadata<br>
layer when necessary, for things like incremental backup support.<br>
<br>
The original qemu patch posting is here:<br>
<a href="https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07496.html" rel="noreferrer" target="_blank">https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07496.html</a><br>
<br>
For testing, you can create a new qcow2+raw data_file image from an<br>
existing image, like:<br>
<br>
qemu-img convert -O qcow2 \<br>
-o data_file=NEW.raw,data_file_raw=yes<br>
EXISTING.raw NEW.qcow2<br>
<br>
The goal of this series is to teach libvirt enough about this case<br>
so that we can correctly relabel the data_file on VM startup/shutdown.<br>
The main functional changes are<br>
<br>
* Teach storagefile how to parse out data_file from the qcow2 header<br>
* Store the raw string as virStorageSource->externalDataStoreRaw<br>
* Track that as its out virStorageSource in externalDataStore<br>
* dac/selinux relabel externalDataStore as needed<br>
<br>
>From libvirt's perspective, externalDataStore is conceptually pretty<br>
close to a backingStore, but the main difference is its read/write<br>
permissions should match its parent image, rather than being readonly<br>
like backingStore.<br>
<br>
This series has only been tested on top of the -blockdev enablement<br>
series, but I don't think it actually interacts with that work at<br>
the moment.<br>
<br>
<br>
Future work:<br>
* Exposing this in the runtime XML. We need to figure out an XML<br>
schema. It will reuse virStorageSource obviously, but the main<br>
thing to figure out is probably 1) what the top element name<br>
should be ('dataFile' maybe?), 2) where it sits in the XML<br>
hierarchy (under <disk> or under <source> I guess)<br>
<br>
* Exposing this on the qemu -blockdev command line. Similar to how<br>
in the blockdev world we are explicitly putting the disk backing<br>
chain on the command line, we can do that for data_file too. Then<br>
like persistent <backingStore> XML the user will have the power<br>
to overwrite the data_file location for an individual VM run.<br>
<br>
* Figure out how we expect ovirt/rhev to be using this at runtime.<br>
Possibly taking a running VM using a raw image, doing blockdev-*<br>
magic to pivot it to qcow2+raw data_file, so it can initiate<br>
incremental backup on top of a previously raw only VM?<br>
<br>
<br>
Known issues:<br>
* In the qemu driver, the qcow2 image metadata is only parsed<br>
in -blockdev world if no <backingStore> is specified in the<br>
persistent XML. So basically if there's a <backingStore> listed,<br>
we never parse the qcow2 header and detect the presence of<br>
data_file. Fixable I'm sure but I didn't look into it much yet.<br>
<br>
Most of this is cleanups and refactorings to simplify the actual<br>
functional changes.<br>
<br>
Cole Robinson (30):<br>
storagefile: Make GetMetadataInternal static<br>
storagefile: qcow1: Check for BACKING_STORE_OK<br>
storagefile: qcow1: Fix check for empty backing file<br>
storagefile: qcow1: Let qcowXGetBackingStore fill in format<br>
storagefile: Check version to determine if qcow2 or not<br>
storagefile: Drop now unused isQCow2 argument<br>
storagefile: Use qcowXGetBackingStore directly<br>
storagefile: Push 'start' into qcow2GetBackingStoreFormat<br>
storagefile: Push extension_end calc to qcow2GetBackingStoreFormat<br>
storagefile: Rename qcow2GetBackingStoreFormat<br>
storagefile: Rename qcow2GetExtensions 'format' argument<br>
storagefile: Fix backing format \0 check<br>
storagefile: Add externalDataStoreRaw member<br>
storagefile: Parse qcow2 external data file<br>
storagefile: Fill in meta->externalDataStoreRaw<br>
storagefile: Don't access backingStoreRaw directly in<br>
FromBackingRelative<br>
storagefile: Split out virStorageSourceNewFromChild<br>
storagefile: Add externalDataStore member<br>
storagefile: Fill in meta->externalDataStore<br>
security: dac: Drop !parent handling in SetImageLabelInternal<br>
security: dac: Add is_toplevel to SetImageLabelInternal<br>
security: dac: Restore image label for externalDataStore<br>
security: dac: break out SetImageLabelRelative<br>
security: dac: Label externalDataStore<br>
security: selinux: Simplify SetImageLabelInternal<br>
security: selinux: Drop !parent handling in SetImageLabelInternal<br>
security: selinux: Add is_toplevel to SetImageLabelInternal<br>
security: selinux: Restore image label for externalDataStore<br>
security: selinux: break out SetImageLabelRelative<br>
security: selinux: Label externalDataStore<br>
<br>
src/libvirt_private.syms | 1 -<br>
src/security/security_dac.c | 63 +++++--<br>
src/security/security_selinux.c | 97 +++++++----<br>
src/util/virstoragefile.c | 281 ++++++++++++++++++++------------<br>
src/util/virstoragefile.h | 11 +-<br>
5 files changed, 290 insertions(+), 163 deletions(-)<br>
<br>
-- <br>
2.23.0<br>
<br>
--<br>
libvir-list mailing list<br>
<a href="mailto:libvir-list@redhat.com" target="_blank">libvir-list@redhat.com</a><br>
<a href="https://www.redhat.com/mailman/listinfo/libvir-list" rel="noreferrer" target="_blank">https://www.redhat.com/mailman/listinfo/libvir-list</a><br>
</blockquote></div><br clear="all"><br>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr">Best regards,</div><div dir="ltr">-----------------------------------<br></div><div dir="ltr">Han Han<br>Quality Engineer<br>Redhat.<br><br>Email: <a href="mailto:hhan@redhat.com" target="_blank">hhan@redhat.com</a><br>Phone: +861065339333<br></div></div></div></div></div></div>