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

Christian Ehrhardt christian.ehrhardt at canonical.com
Tue Oct 8 05:52:32 UTC 2019


On Mon, Oct 7, 2019 at 11:49 PM 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

Hi Cole,
it seems the changes to dac/selinux follow a common pattern, in the
past those changes then mostly applied to security-apparmor as well.
Are you going to add patches for that security backend as well before
this is final or do you expect the apparmor users Debian/Suse/Ubuntu
to do so later on?

Furthermore for the static XML->Apparmor part it will most likely need
an extension to [1] as well.
Fortunately as you said it is very similar to backingDevices which
means it should be rather easy to add this.

[1]: https://libvirt.org/git/?p=libvirt.git;a=blob;f=src/security/virt-aa-helper.c;h=5853ad985fe17c91af3f1dc39d179f22a1dca5b7;hb=HEAD#l980



>  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



--
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd




More information about the libvir-list mailing list