[libvirt] backingStore info adding late breaks virt-aa-helper
Hi, there is a behavioral change I try to track down that affects virt-aa-helper. TL;DR: - it seems backingStore info gets added "later" in recent versions which causes issues in virt-aa-helper Details: For a guest containing a qcow2 disk like this: <disk type='file' device='disk'> <driver name='qemu' type='qcow2'/> <source file='/var/lib/uvtool/libvirt/images/kvmguest-artful-normal.qcow'/> <target dev='vda' bus='virtio'/> </disk> And said qcow disk having a backing file: $ qemu-img info /var/lib/uvtool/libvirt/images/kvmguest-artful-normal.qcow image: /var/lib/uvtool/libvirt/images/kvmguest-artful-normal.qcow [...] backing file: /var/lib/uvtool/libvirt/images/x-uvt-b64-Y29tLnVidW50dS5jbG91ZC5kYWlseTpzZXJ2ZXI6MTcuMTA6cHBjNjRlbCAyMDE3MDcxMw== Now when instantiating the guest this gets the backingStore info added like: <backingStore type='file' index='1'> <format type='qcow2'/> <source file='/var/lib/uvtool/libvirt/images/x-uvt-b64-Y29tLnVidW50dS5jbG91ZC5kYWlseTpzZXJ2ZXI6MTcuMTA6cHBjNjRlbCAyMDE3MDcxMw=='/> <backingStore/> </backingStore> But this now seems to come in "too late" for virt-aa-helper. That tool is reading the guest definition to create custom rules for that guest that opens up the apparmor profile. And in relation to the devices the following in src/security/virt-aa-helper.c is the important part: Loops over disks and in those "down" the chain of backing stores: 929 for (i = 0; i < ctl->def->ndisks; i++) { [...] 947 if (virDomainDiskDefForeachPath(disk, true, add_file_path, &buf) < 0) If you pass virt-aa-helper as in libvirt 3.5 a full snippet with backingStore info it behaves the same as back in 2.5 emmitting a rule for the backing store. But when starting a guest on libvirt 3.5 this does no more work, so it seems that on instantiating the guest Past (2.5) 1. add backingStore info to guest representation 2. virt-aa-helper parses guest representation and creates rules 3. guest starts fine changed to now (3.5): 1. virt-aa-helper parses guest representation and creates rules 2. add backingStore info to guest representation 3. guest fails to start as the apparmor rule to allow it access to its backing file is missing. I've verified that recent libvirt properly adds the backingStore eventually (by disabling the apparmor profile and then starting the guest). Once fully started the live xml representation has the backing store info added. But as outlined above, at the point virt-aa-helper runs now the necessary backingStore data seems to be missing. I couldn't find the related change or a way to fix it so far, so any hints are welcome. -- Christian Ehrhardt Software Engineer, Ubuntu Server Canonical Ltd
Hi, I was mislead by my former assumption on the lifecycle. As virt-aa-helper gets his xml passed into stdin. I captured that and found that in both cases it had the same content. Below steps to reproduce based on that: Test -Xml: <domain type='kvm' id='1'> <name>kvmguest-artful-normal-a2</name> <uuid>f4239a92-f933-4bd3-b9fb-b9c260a7dc65</uuid> <memory unit='KiB'>524288</memory> <vcpu placement='static'>1</vcpu> <os> <type arch='ppc64le' machine='pseries-zesty'>hvm</type> <boot dev='hd'/> </os> <devices> <emulator>/usr/bin/kvm</emulator> <disk type='file' device='disk'> <driver name='qemu' type='qcow2'/> <source file='/var/lib/uvtool/libvirt/images/kvmguest-artful-normal-a2.qcow'/> <backingStore/> <target dev='vda' bus='virtio'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </disk> </devices> <seclabel type='dynamic' model='apparmor' relabel='yes'> <label>libvirt-f4239a92-f933-4bd3-b9fb-b9c260a7dc65</label> <imagelabel>libvirt-f4239a92-f933-4bd3-b9fb-b9c260a7dc65</imagelabel> </seclabel> </domain> File: qemu-img info /var/lib/uvtool/libvirt/images/kvmguest-artful-normal-a2.qcow image: /var/lib/uvtool/libvirt/images/kvmguest-artful-normal-a2.qcow file format: qcow2 virtual size: 8.0G (8589934592 bytes) disk size: 200M cluster_size: 65536 backing file: /var/lib/uvtool/libvirt/images/x-uvt-b64-Y29tLnVidW50dS5jbG91ZC5kYWlseTpzZXJ2ZXI6MTcuMTA6cHBjNjRlbCAyMDE3MDcxNg== backing file format: qcow2 Format specific information: compat: 0.10 refcount bits: 1 To be sure I undefined the guest to not have "a different source" or information. Generate a new profile: $ /usr/lib/libvirt/virt-aa-helper --create --dryrun --uuid 'libvirt-f4239a92-f933-4bd3-b9fb-b9c260a7dc65' < test-virt-aa-helper.xml In 3.5 this is no more having the line: /var/lib/uvtool/libvirt/images/x-uvt-b64-Y29tLnVidW50dS5jbG91ZC5kYWlseTpzZXJ2ZXI6MTcuMTA6cHBjNjRlbCAyMDE3MDcxNg== With that it seems way more "debuggable" and I'll do so, but any hint of a known related change is still welcome.
On said example: Libvirt 2.5: Breakpoint 1, 0x00003fffb7c77ba8 in virDomainDiskDefForeachPath (disk=0x200ab490, ignoreOpenFailure=true, iter=0x20011dc0 <add_file_path>, opaque=0x3fffffffef70) at ../../../src/conf/domain_conf.c:24851 $1 = (virStorageSourcePtr) 0x200ab630 (gdb) p disk->src->path $2 = 0x200a9ff0 "/var/lib/uvtool/libvirt/images/kvmguest-artful-normal-a2.qcow" (gdb) p disk->src->backingStore $3 = (virStorageSourcePtr) 0x200ab720 (gdb) p disk->src->backingStore->path $4 = 0x200a9e30 "/var/lib/uvtool/libvirt/images/x-uvt-b64-Y29tLnVidW50dS5jbG91ZC5kYWlseTpzZXJ2ZXI6MTcuMTA6cHBjNjRlbCAyMDE3MDcxNg==" Libvirt 3.5: Breakpoint 1, 0x00003fffb7c6a3a8 in virDomainDiskDefForeachPath (disk=0x2008cb50, ignoreOpenFailure=true, iter=0x2000f5e0 <add_file_path>, opaque=0x3fffffffef70) at ../../../src/conf/domain_conf.c:25984 25984 ../../../src/conf/domain_conf.c: No such file or directory. (gdb) p disk->src->path $2 = 0x20085800 "/var/lib/uvtool/libvirt/images/kvmguest-artful-normal-a2.qcow" (gdb) p disk->src->backingStore $3 = (virStorageSourcePtr) 0x0 So it is the parsing of the XML into objects I have to track down. Maybe it is even some Ubuntu Delta that no more correctly matches. Will run on build from upstream master as well before next report.
On Mon, Jul 17, 2017 at 8:17 PM, Christian Ehrhardt < christian.ehrhardt@canonical.com> wrote:
So it is the parsing of the XML into objects I have to track down. Maybe it is even some Ubuntu Delta that no more correctly matches. Will run on build from upstream master as well before next report.
Ok, I was able to confirm with latest upstream master which rules out Ubuntu apparmor delta as the source of the issue. But I can build and run to check so worst case I can bisect it now. -- Christian Ehrhardt Software Engineer, Ubuntu Server Canonical Ltd
On Mon, Jul 17, 2017 at 8:40 PM, Christian Ehrhardt < christian.ehrhardt@canonical.com> wrote:
On Mon, Jul 17, 2017 at 8:17 PM, Christian Ehrhardt < christian.ehrhardt@canonical.com> wrote:
So it is the parsing of the XML into objects I have to track down. Maybe it is even some Ubuntu Delta that no more correctly matches. Will run on build from upstream master as well before next report.
Ok, I was able to confirm with latest upstream master which rules out Ubuntu apparmor delta as the source of the issue. But I can build and run to check so worst case I can bisect it now.
This becomes a monologue :-/ Bisect found the breaking change to be in: f813fe810fc3f0ce839f450e662e0173d40a5ce1 is the first bad commit commit f813fe810fc3f0ce839f450e662e0173d40a5ce1 Author: Peter Krempa <pkrempa@redhat.com> Date: Fri Jan 13 16:50:11 2017 +0100 storage: backend: Refactor registration of the backend drivers Add APIs that allow to dynamically register driver backends so that the list of available drivers does not need to be known during compile time. This will allow us to modularize the storage driver on runtime. I'll try to debug in more detail how/where it broke it, but since this is a rather huge refactor and not a small change this might take a while. Any hints still welcome.
On Tue, Jul 18, 2017 at 08:57:36 +0200, Christian Ehrhardt wrote:
On Mon, Jul 17, 2017 at 8:40 PM, Christian Ehrhardt < christian.ehrhardt@canonical.com> wrote:
On Mon, Jul 17, 2017 at 8:17 PM, Christian Ehrhardt < christian.ehrhardt@canonical.com> wrote:
So it is the parsing of the XML into objects I have to track down. Maybe it is even some Ubuntu Delta that no more correctly matches. Will run on build from upstream master as well before next report.
Ok, I was able to confirm with latest upstream master which rules out Ubuntu apparmor delta as the source of the issue. But I can build and run to check so worst case I can bisect it now.
This becomes a monologue :-/ Bisect found the breaking change to be in:
f813fe810fc3f0ce839f450e662e0173d40a5ce1 is the first bad commit commit f813fe810fc3f0ce839f450e662e0173d40a5ce1 Author: Peter Krempa <pkrempa@redhat.com> Date: Fri Jan 13 16:50:11 2017 +0100
storage: backend: Refactor registration of the backend drivers
Add APIs that allow to dynamically register driver backends so that the list of available drivers does not need to be known during compile time.
This will allow us to modularize the storage driver on runtime.
I'll try to debug in more detail how/where it broke it, but since this is a rather huge refactor and not a small change this might take a while. Any hints still welcome.
Hmmm, in that case probably the registration of the storage driver APIs in the aa-helper does not take place properly, and thus the detection of the backing chain will be wrong. I'll have a look
participants (2)
-
Christian Ehrhardt -
Peter Krempa