[Bug 1726804] Re: rules for images on attach-device not containing lock permission

[Corey Bryant]

OpenStack regression testing passed successfully on pike-proposed:

xenial-pike-proposed with stable charms:

Ran: 102 tests in 1588.4030 sec.
 - Passed: 93
 - Skipped: 9
 - Expected Fail: 0
 - Unexpected Success: 0
 - Failed: 0
Sum of execute time for each test: 687.2787 sec.

xenial-pike-proposed with dev charms:

Ran: 102 tests in 1403.7715 sec.
 - Passed: 93
 - Skipped: 9
 - Expected Fail: 0
 - Unexpected Success: 0
 - Failed: 0
Sum of execute time for each test: 649.5511 sec.

** Changed in: cloud-archive/queens
       Status: Fix Committed => Fix Released

** Tags removed: verification-pike-needed
** Tags added: verification-pike-done

-- 
You received this bug notification because you are a member of Ubuntu
OpenStack, which is subscribed to Ubuntu Cloud Archive.
https://bugs.launchpad.net/bugs/1726804

Title:
  rules for images on attach-device not containing lock permission

Status in Ubuntu Cloud Archive:
  Fix Released
Status in Ubuntu Cloud Archive pike series:
  Fix Released
Status in Ubuntu Cloud Archive queens series:
  Fix Released
Status in libvirt package in Ubuntu:
  Fix Released
Status in libvirt source package in Artful:
  Fix Released

Bug description:
  [Impact]

   * Qemu 2.10 started to lock image files to ensure no data corruption 
     occurs. Unfurtunately that isn't covered by the apparmor rules we had 
     for images so far - it need to add "k" permission.

   * This was spotted and done in Artful, but the tests for the hot-add of
     disks were hidden behind some other known not-too-bad issues. So by 
     fixing those tests I realized that hot-add of disks is currently broken
     in Artful.

  [Test Case]

  # Get a very minimal Testguest that keeps running to attach something
  $ qemu-img create /tmp/A.img 1M
  cat <<EOF > testguest.xml
  <domain type='kvm'>
          <name>testguest</name>
          <uuid>deadbeef-dead-beef-dead-beefdeadbeef</uuid>
          <memory unit='KiB'>1024</memory>
          <vcpu placement='static'>1</vcpu>
          <os>
                  <type arch='x86_64' machine='pc-i440fx-zesty'>hvm</type>
                  <boot dev='hd'/>
          </os>
          <features>
                  <acpi/>
                  <apic/>
                  <pae/>
          </features>
          <devices>
                  <emulator>/usr/bin/kvm-spice</emulator>
                  <disk type='file' device='disk'>
                          <driver name='qemu'/>
                          <source file='/tmp/A.img'/>
                          <target dev='vda'/>
                  </disk>
          </devices>
          <seclabel type='dynamic' model='apparmor' relabel='yes'/>
  </domain>
  EOF
  $ virsh define testguest.xml
  $ virsh start testguest

  # Prepare Disk
  $ qemu-img create /tmp/F.img 1M
  $ cat <<EOF >diskF.xml
  <disk type='file'>
    <driver name='qemu'/>
    <source file='/tmp/F.img'/>
    <target dev='sdc'/>
  </disk>
  EOF

  # Then attach:
  $ virsh attach-device testguest diskF.xml

  * This should work, but fails without the fix as:
     error: internal error: unable to execute QEMU command 'device_add': 
  Property 'scsi-hd.drive' can't find value 'drive-scsi0-0-0-1'
    With a related apparmor denial:
     apparmor="DENIED" operation="file_lock" profile="libvirt-7d781722-69b7-8801-fe96-caf37b7a8969" name="/tmp/tmpKzZQR0/device_disk.img" pid=17582 comm="qemu" requested_mask="k" denied_mask="k" fsuid=0 ouid=0

  * With the fix the file is rwk and works to be attached

  [Regression Potential]

   * This is only adding apparmor lock permissions to files added after 
     start. Thereby the only thing that comes to mind is if now things are 
     locked that were not before, and thereby cause issues. But OTOH no one 
     but qemu should lock the image files in use - and if someone else does 
     he now correctly sees qemu holding the lock. Seems safe to me.

  [Other Info]
   
   * This is an release/upgrade-regression which should be fixed 
     asap. I already wrote and submitted a fix to upstream, but given that 
     this can break a lot of use cases we ahve to fix fast and reroll in 
     case upstream decides to modify.

  ---

  
  On something like:
   $ virsh attach-device <guest> <xml>

  The rule rendered is:
  "/tmp/B.img" rw,

  This is missing the k flag needed on qemu >=2.10.

  This applies to block and file definitions:
  <disk type='block'>
    <driver name='qemu'/>
    <source dev='/tmp/B.img'/>
    <target dev='sdb'/>
  </disk>
  <disk type='file'>
    <driver name='qemu'/>
    <source file='/tmp/F.img'/>
    <target dev='sdc'/>
  </disk>

  Both are rendered correctly as:
  "/tmp/F.img" rwk,
  If being part of the domain xml instead of being a hot-add.

To manage notifications about this bug go to:
https://bugs.launchpad.net/cloud-archive/+bug/1726804/+subscriptions