[libvirt] [PATCH 00/30] storagefile, security: qcow2 data_file support

Han Han hhan at redhat.com
Tue Oct 15 07:56:53 UTC 2019


Hello Cole, one issue is found:
The qcow2 data file XTTRs is not cleaned on external snapshot when
-blockdev is not enabled

Versions:
libvirt v5.8.0-134-g9d03e9adf1
qemu-kvm-4.1.0-13.module+el8.1.0+4313+ef76ec61.x86_64

Steps:
1. Convert a OS image to qcow2&qcow2 data file:
# 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

2. Build and start libvirt source, start libvirt daemon:
# 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,
lab.rhel8.me' --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
# LD_PRELOAD="$(find src -name '*.so.*'|tr '\n' ' ')" src/.libs/virtlogd
# 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

3. Define and start an VM with the qcow2&qcow2 data file. Note that the
-blockdev is not enabled
# virsh define pc-data.xml
# virsh start pc-data

4. Create snapshot and check the data file XATTRs:
# virsh snapshot-create-as pc-data s1 --no-metadata --disk-only
# getfattr -m - -d /var/lib/libvirt/images/pc-data.raw
getfattr: Removing leading '/' from absolute path names
# file: var/lib/libvirt/images/pc-data.raw
security.selinux="unconfined_u:object_r:svirt_image_t:s0:c775,c1011"
trusted.libvirt.security.dac="+107:+107"
trusted.libvirt.security.ref_dac="1"
trusted.libvirt.security.ref_selinux="1"
trusted.libvirt.security.selinux="unconfined_u:object_r:svirt_image_t:s0:c284,c367"
trusted.libvirt.security.timestamp_dac="1563328069"
trusted.libvirt.security.timestamp_selinux="1563328069"

Shutdown the VM. The XATTRs of data file is not changed.
It is not expected. The XTTRs should not contain *.libvirt.*

Issue is not reproduced with -blockdev enabled:
<domain type='kvm' xmlns:qemu='http://libvirt.org/schemas/domain/qemu/1.0'>
...
  <qemu:capabilities>
    <qemu:add capability='blockdev'/>
    <qemu:del capability='drive'/>
  </qemu:capabilities>
</domain>

See the libvirt daemon log and vm xml in attachment.

On Tue, Oct 8, 2019 at 5:49 AM Cole Robinson <crobinso at redhat.com> wrote:

> This series is the first steps to teaching libvirt about qcow2
> data_file support, aka external data files or qcow2 external metadata.
>
> A bit about the feature: it was added in qemu 4.0. It essentially
> creates a two part image file: a qcow2 layer that just tracks the
> image metadata, and a separate data file which is stores the VM
> disk contents. AFAICT the driving use case is to keep a fully coherent
> raw disk image on disk, and only use qcow2 as an intermediate metadata
> layer when necessary, for things like incremental backup support.
>
> The original qemu patch posting is here:
> https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07496.html
>
> For testing, you can create a new qcow2+raw data_file image from an
> existing image, like:
>
>     qemu-img convert -O qcow2 \
>         -o data_file=NEW.raw,data_file_raw=yes
>         EXISTING.raw NEW.qcow2
>
> The goal of this series is to teach libvirt enough about this case
> so that we can correctly relabel the data_file on VM startup/shutdown.
> The main functional changes are
>
>   * Teach storagefile how to parse out data_file from the qcow2 header
>   * Store the raw string as virStorageSource->externalDataStoreRaw
>   * Track that as its out virStorageSource in externalDataStore
>   * dac/selinux relabel externalDataStore as needed
>
> >From libvirt's perspective, externalDataStore is conceptually pretty
> close to a backingStore, but the main difference is its read/write
> permissions should match its parent image, rather than being readonly
> like backingStore.
>
> This series has only been tested on top of the -blockdev enablement
> series, but I don't think it actually interacts with that work at
> the moment.
>
>
> Future work:
>   * Exposing this in the runtime XML. We need to figure out an XML
>     schema. It will reuse virStorageSource obviously, but the main
>     thing to figure out is probably 1) what the top element name
>     should be ('dataFile' maybe?), 2) where it sits in the XML
>     hierarchy (under <disk> or under <source> I guess)
>
>   * Exposing this on the qemu -blockdev command line. Similar to how
>     in the blockdev world we are explicitly putting the disk backing
>     chain on the command line, we can do that for data_file too. Then
>     like persistent <backingStore> XML the user will have the power
>     to overwrite the data_file location for an individual VM run.
>
>   * Figure out how we expect ovirt/rhev to be using this at runtime.
>     Possibly taking a running VM using a raw image, doing blockdev-*
>     magic to pivot it to qcow2+raw data_file, so it can initiate
>     incremental backup on top of a previously raw only VM?
>
>
> Known issues:
>   * In the qemu driver, the qcow2 image metadata is only parsed
>     in -blockdev world if no <backingStore> is specified in the
>     persistent XML. So basically if there's a <backingStore> listed,
>     we never parse the qcow2 header and detect the presence of
>     data_file. Fixable I'm sure but I didn't look into it much yet.
>
> Most of this is cleanups and refactorings to simplify the actual
> functional changes.
>
> Cole Robinson (30):
>   storagefile: Make GetMetadataInternal static
>   storagefile: qcow1: Check for BACKING_STORE_OK
>   storagefile: qcow1: Fix check for empty backing file
>   storagefile: qcow1: Let qcowXGetBackingStore fill in format
>   storagefile: Check version to determine if qcow2 or not
>   storagefile: Drop now unused isQCow2 argument
>   storagefile: Use qcowXGetBackingStore directly
>   storagefile: Push 'start' into qcow2GetBackingStoreFormat
>   storagefile: Push extension_end calc to qcow2GetBackingStoreFormat
>   storagefile: Rename qcow2GetBackingStoreFormat
>   storagefile: Rename qcow2GetExtensions 'format' argument
>   storagefile: Fix backing format \0 check
>   storagefile: Add externalDataStoreRaw member
>   storagefile: Parse qcow2 external data file
>   storagefile: Fill in meta->externalDataStoreRaw
>   storagefile: Don't access backingStoreRaw directly in
>     FromBackingRelative
>   storagefile: Split out virStorageSourceNewFromChild
>   storagefile: Add externalDataStore member
>   storagefile: Fill in meta->externalDataStore
>   security: dac: Drop !parent handling in SetImageLabelInternal
>   security: dac: Add is_toplevel to SetImageLabelInternal
>   security: dac: Restore image label for externalDataStore
>   security: dac: break out SetImageLabelRelative
>   security: dac: Label externalDataStore
>   security: selinux: Simplify SetImageLabelInternal
>   security: selinux: Drop !parent handling in SetImageLabelInternal
>   security: selinux: Add is_toplevel to SetImageLabelInternal
>   security: selinux: Restore image label for externalDataStore
>   security: selinux: break out SetImageLabelRelative
>   security: selinux: Label externalDataStore
>
>  src/libvirt_private.syms        |   1 -
>  src/security/security_dac.c     |  63 +++++--
>  src/security/security_selinux.c |  97 +++++++----
>  src/util/virstoragefile.c       | 281 ++++++++++++++++++++------------
>  src/util/virstoragefile.h       |  11 +-
>  5 files changed, 290 insertions(+), 163 deletions(-)
>
> --
> 2.23.0
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>


-- 
Best regards,
-----------------------------------
Han Han
Quality Engineer
Redhat.

Email: hhan at redhat.com
Phone: +861065339333
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20191015/db7dd82a/attachment-0001.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: data-file-xattr-unclean.tar.gz
Type: application/gzip
Size: 351713 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20191015/db7dd82a/attachment-0001.gz>


More information about the libvir-list mailing list