[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