<div dir="ltr"><div>Hi Cole,</div><div>I merged crobinso/qcow2-data_file branch to 37b565c00. Reserved new capabilities introduced by these to branches to resolve conflicts.<br></div><div>Then build and test as following:</div><div># ./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</div><div><br></div><div>Start libvirtd and virtlogd<br></div><div># LD_PRELOAD="$(find src -name '*.so.*'|tr '\n' ' ')" src/.libs/libvirtd</div><div># LD_PRELOAD="$(find src -name '*.so.*'|tr '\n' ' ')" ./src/virtlogd</div><div><br></div><div>Then try to list all domains:</div><div># virsh list --all</div><div><br></div><div>Libvirtd exits with segment fault:</div><div>[1] 30104 segmentation fault (core dumped) LD_PRELOAD="$(find src -name '*.so.*'|tr '\n' ' ')" src/.libs/libvirtd <br></div><div><br></div><div>Version:</div><div>qemu-4.1<br></div><div><br></div><div>Backtrace:</div><div>(gdb) bt<br>#0 0x00007fbe57a0d1b9 in virDomainVirtioSerialAddrSetAddControllers (def=<optimized out>, def=<optimized out>, addrs=<optimized out>) at conf/domain_addr.c:1656 <br>#1 virDomainVirtioSerialAddrSetCreateFromDomain (def=def@entry=0x7fbde81cc3f0) at conf/domain_addr.c:1753 <br>#2 0x00007fbe0179897e in qemuDomainAssignVirtioSerialAddresses (def=0x7fbde81cc3f0) at qemu/qemu_domain_address.c:3174 <br>#3 qemuDomainAssignAddresses (def=0x7fbde81cc3f0, qemuCaps=0x7fbde81d2210, driver=0x7fbde8126850, obj=0x0, newDomain=<optimized out>) at qemu/qemu_domain_address.c:3174 <br>#4 0x00007fbe57a39e0d in virDomainDefPostParse (def=def@entry=0x7fbde81cc3f0, caps=caps@entry=0x7fbde8154d20, parseFlags=parseFlags@entry=4610, xmlopt=xmlopt@entry=0x7fbde83ce070, <br> parseOpaque=parseOpaque@entry=0x0) at conf/domain_conf.c:5858<br>#5 0x00007fbe57a525c5 in virDomainDefParseNode (xml=<optimized out>, root=0x7fbde83c5ff0, caps=0x7fbde8154d20, xmlopt=0x7fbde83ce070, parseOpaque=0x0, flags=4610) at conf/domain_conf.c:21677 <br>#6 0x00007fbe57a526c8 in virDomainDefParse (xmlStr=xmlStr@entry=0x0, filename=<optimized out>, caps=caps@entry=0x7fbde8154d20, xmlopt=xmlopt@entry=0x7fbde83ce070, parseOpaque=parseOpaque@entry=0x0, <br> flags=flags@entry=4610) at conf/domain_conf.c:21628<br>#7 0x00007fbe57a528f6 in virDomainDefParseFile (filename=<optimized out>, caps=caps@entry=0x7fbde8154d20, xmlopt=xmlopt@entry=0x7fbde83ce070, parseOpaque=parseOpaque@entry=0x0, flags=flags@entry=4610) <br> at conf/domain_conf.c:21653<br>#8 0x00007fbe57a5e16a in virDomainObjListLoadConfig (opaque=0x0, notify=0x0, name=0x7fbde81d7ff3 "pc", autostartDir=0x7fbde8124070 "/etc/libvirt/qemu/autostart", configDir=0x7fbde8124050 "/etc/libvirt/qemu", <br> xmlopt=0x7fbde83ce070, caps=0x7fbde8154d20, doms=0x7fbde8126940) at conf/virdomainobjlist.c:503<br>#9 virDomainObjListLoadAllConfigs (doms=0x7fbde8126940, configDir=0x7fbde8124050 "/etc/libvirt/qemu", autostartDir=0x7fbde8124070 "/etc/libvirt/qemu/autostart", liveStatus=liveStatus@entry=false, <br> caps=0x7fbde8154d20, xmlopt=0x7fbde83ce070, notify=0x0, opaque=0x0) at conf/virdomainobjlist.c:625<br>#10 0x00007fbe017f57e2 in qemuStateInitialize (privileged=true, callback=<optimized out>, opaque=<optimized out>) at qemu/qemu_driver.c:1007 <br>#11 0x00007fbe57b8033d in virStateInitialize (privileged=true, mandatory=mandatory@entry=false, callback=callback@entry=0x55dfb702ecc0 <daemonInhibitCallback>, opaque=opaque@entry=0x55dfb8869d60) <br> at libvirt.c:666<br>#12 0x000055dfb702ed1d in daemonRunStateInit (opaque=0x55dfb8869d60) at remote/remote_daemon.c:846<br>#13 0x00007fbe579f4be2 in virThreadHelper (data=<optimized out>) at util/virthread.c:196<br>#14 0x00007fbe55a322de in start_thread (arg=<optimized out>) at pthread_create.c:486<br>#15 0x00007fbe55763133 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95</div><div><br></div><div>Could you please check this issue?</div><div>The full threads backtrace is 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>