libvirt uses password-secret on old style drive_add syntax

<disk type="network" device="disk">
  <driver name="qemu" type="raw" cache="none"/>
  <source protocol="rbd" name="cinder-ceph/volume-8ab7473e-c34c-421a-b6d9-b18a9fa8788a">
    <host name="10.5.26.60" port="6789"/>
    <host name="10.5.26.61" port="6789"/>
    <host name="10.5.26.62" port="6789"/>
  </source>
  <auth username="nova-compute">
    <secret type="ceph" uuid="514c9fca-8cbe-11e2-9c52-3bc8c7819472"/>
  </auth>
  <target bus="virtio" dev="vdb"/>
  <serial>8ab7473e-c34c-421a-b6d9-b18a9fa8788a</serial>
</disk>

$ virsh secret-list
 UUID Usage
--------------------------------------------------------------------------------
 514c9fca-8cbe-11e2-9c52-3bc8c7819472 ceph client.nova-compute secret

Mar 13 12:46:41 juju-1bfad1-yakkety-testing-15 kernel: [252869.417593] audit: type=1400 audit(1489409201.168:404): apparmor="DENIED" operation="open" profile="libvirt-f5de470e-3805-4a10-8ad5-cc5be06a728a" name="/proc/15381/task/25246/comm" pid=15381 comm="qemu-system-x86" requested_mask="wr" denied_mask="wr" fsuid=112 ouid=112

Might be related to bug 1615550

That apparmor is a red herring here.
It is for a feature called "debug-threads".
https://www.redhat.com/archives/libvir-list/2016-March/msg00428.html
In later releases we fixed that permission to work properly, but it doesn't hurt other than some noise - so no SRU.
But your issue should certainly be something else.

I assume it constructs some "drive_add" command like
"drive_add 0 if=none,file=/tmp/test.img,format=raw,id=drive-virtio-disk1" but for the rbd device.

There are examples here: https://wiki.ubuntu.com/QemuDiskHotplug#A.27drive_add.27_example

If you can track down the command it executes we could take a deeper look via.
$ virsh qemu-monitor-command --hmp <domain> '<command> [...]'

Syntax would be like:
device_add driver[,prop=value][,...] -- add device, like -device on the command line

Something like this maybe (still incomplete):
virsh qemu-monitor-command --hmp zesty-test-log 'device_add virtio-blk-device,file=/tmp/test.img,format=raw,id=drive-virtio-disk1'

Sorry this is far from human friendly as it is not meant for humans.
But if you can intercept what your tooling is submitting we could eliminate all of the stack from the equation and take a look.

The following at least seems close to your error message, try to look around from there if you can:
$ virsh qemu-monitor-command --hmp zesty-test-log 'device_add virtio-blk-pci,drive=drive-virtio-disk1,id=mydisk1'
Property 'virtio-blk-device.drive' can't find value 'drive-virtio-disk1'

Of course I don't have anything like that device - so for me that is fine.

This is not bad: https://wiki.ubuntu.com/QemuDiskHotplug#A.27drive_add.27_example

An example would be:
#1 create disk
$ sudo qemu-img create -f qcow2 /var/lib/libvirt/images/test.img 1G
$ sudo chown libvirt-qemu:kvm /var/lib/libvirt/images/test.img
#2 add to qemu
# you might need to either teach your apparmor about it, or disable it
$ virsh qemu-monitor-command --hmp zesty-test-log 'drive_add 0 if=none,file=/tmp/test.img,format=raw,id=disk1'
#3 add it to the guest
$ virsh qemu-monitor-command --hmp zesty-test-log 'device_add virtio-blk-pci,drive=disk1,id=myvirtio1'

#1 in your case certainly is rbd, #2/#3 commands is what you need to find and debug/analyze.
A failing #3 like in your error might indicate a failing #2 (drive_add failed, so it can't be found).

You can check the proper state if your device is there after #2 with
$ virsh qemu-monitor-command --hmp zesty-test-log 'info block'

Trying to translate that - I do see this when trying to boot from a volume:

qemu-system-x86_64: -drive file=rbd:cinder-ceph/volume-d70c7674-e330-46dc-9e4a-9cbd0c5831e4:id=nova-compute:auth_supported=cephx\;none:mon_host=10.5.26.60\:6789\;10.5.26.61\:6789\;10.5.26.62\:6789,password-secret=virtio-disk0-secret0,format=raw,if=none,id=drive-virtio-disk0,serial=d70c7674-e330-46dc-9e4a-9cbd0c5831e4,cache=none: error connecting

OK so maybe:

virsh qemu-monitor-command --hmp instance-0000001a 'drive_add 1 file=rbd:cinder-ceph/volume-d70c7674-e330-46dc-9e4a-9cbd0c5831e4:id=nova-compute:auth_supported=cephx\;none:mon_host=10.5.26.60\:6789\;10.5.26.61\:6789\;10.5.26.62\:6789,password-secret=virtio-disk0-secret1,format=raw,if=none,id=drive-virtio-disk1,serial=d70c7674-e330-46dc-9e4a-9cbd0c5831e4,cache=none'

Hi,
that makes sense.
Due to the fail to connect on the drive_add (which matches the -drive) the "drive" ID was not populated.
So later when something wants to refer to that ID it gets that issue.
BTW - is your error on boot or later on hotplug?

The converted command looks reasonable to me, at least from just reading it.
Did you try
$ virsh qemu-monitor-command --hmp zesty-test-log 'info block'
on the guest booted that way if it had the drive with the id drive-virtio-disk1?

Hmm I need to setup a rbd setup.
Lets hope there is something simple for me ...
You are driving this from openstack/ceph right?

I wanted to see the fail of the initial drive_add and I was able to create:
virsh qemu-monitor-command --hmp instance-0000001a 'drive_add 1 file=rbd:cinder-ceph/volume-8ab7473e-c34c-421a-b6d9-b18a9fa8788a:id=nova-compute:auth_supported=cephx\;none:mon_host=10.5.26.60\:6789\;10.5.26.61\:6789\;10.5.26.62\:6789,password-secret=virtio-disk0-secret1,format=raw,if=none,id=drive-virtio-disk1,serial=8ab7473e-c34c-421a-b6d9-b18a9fa8788a,cache=none'

That is even closer to the root cause and shows:
Block format 'raw' does not support the option 'password-secret'

From there I found that this is an issue with old/new syntax and it needs an attribute prefix on the old syntax used.
Fix: password-secret -> file.password-secret

Then it will complain about lacking the secret, but that is ok I haven't added one in my manual flow.

After that was known I was also able to find:
- https://www.redhat.com/archives/libvir-list/2016-August/msg00783.html
- https://libvirt.org/git/?p=libvirt.git;a=commit;h=d53d465083edeb64cc7b78249c030734c0d91c6b

Fix is in >=2.2 change that started to fail in 1.3.5.
Given that we have:
- xenial 1.3.1-1ubuntu10.8
- yakkety 2.1.0-1ubuntu9
- zesty 2.5.0-3ubuntu3

We only need an SRU for Yakkety.

Testable in https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/2573
Please let me know if you can confirm that this would fix the issue in your setup.

Tested OK:

======
Totals
======
Ran: 103 tests in 1212.0809 sec.
 - Passed: 97
 - Skipped: 6
 - Expected Fail: 0
 - Unexpected Success: 0
 - Failed: 0
Sum of execute time for each test: 656.5238 sec.

that includes volume attach and boot from volume tests.

Thanks James,
also all autopkgtests completed just fine (https://bileto.ubuntu.com/excuses/2573/yakkety.html)
As well as further tests on the server completing without any issue.

That said adding SRU template and pushing to the unapproved queue.

This was still pending in unapproved while another SRU was fully prepared.
I checked with SRU Team and so we rejected the current upload to bundle it with the next one.

This is now as 2.1.0-1ubuntu9.3 in unapproved queue for yakkety.

Due to merging that up with the next SRU I have to ask you James to test that on yakkety-proposed now. Please ack me a confirmation here to be sure on this issue being fixed for you.

I tried to get me a ceph with conjure-up but had no system surviving the cpu/mem needs.
Waiting for JamesPage on that.

Checked by James see comment #69 in bug 1665698

FYI - The referenced comment is the Tempest test that James uses and it contains a ceph block attach which would trigger the bug. Executing it against the proposed version is "verification-done" for it although it was forgotten to link this bug due to our merge of this and the next SRU upload.

I was updating the testcase description to make it more clear what James is testing to verify.

Uploaded a no change rebuild as version 2.1.0-1ubuntu9.4 to help properly linking this bug as well.

Hello James, or anyone else affected,

Accepted libvirt into yakkety-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/libvirt/2.1.0-1ubuntu9.4 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

The automated proposed regression check on qemu/libvirt passed and confirmed that there should be no regression.

@James - you said you kicked off another Tempest run on this to check the issue itself, can you confirm and mark v-done once you checked the results?

@Christian

LGTM (tempest smoke against yakkety-proposed pocket).

08:54:08 ======
08:54:08 Totals
08:54:08 ======
08:54:08 Ran: 103 tests in 949.0000 sec.
08:54:08 - Passed: 97
08:54:08 - Skipped: 6
08:54:08 - Expected Fail: 0
08:54:08 - Unexpected Success: 0
08:54:08 - Failed: 0
08:54:08 Sum of execute time for each test: 550.2997 sec.

What's the qemu Yakkety task for please, and where is it committed?

This bug was fixed in the package libvirt - 2.1.0-1ubuntu9.4

---------------
libvirt (2.1.0-1ubuntu9.4) yakkety; urgency=medium

  * no change rebuild to properly pick up all the changes since the last
    release to yakkety-updates.

libvirt (2.1.0-1ubuntu9.3) yakkety; urgency=medium

  * d/p/ubuntu/qemu-Allow-empty-script-path-to-interface.patch: in the past
    <script path=''/> meant a no-op, but libvirt now fails - fix to match
    the old behavior (LP: #1665698).

libvirt (2.1.0-1ubuntu9.2) yakkety; urgency=medium

  * d/p/ubuntu/fix-command-generation-for-rbd.patch: fix the generation
    of the command used to add disks when secrets are provided like when
    using rbd (LP: #1672367).

 -- Christian Ehrhardt <email address hidden> Fri, 31 Mar 2017 07:48:25 +0200

The verification of the Stable Release Update for libvirt has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

Thanks Steve to make me aware of the open re-triage of the qemu portion.
But while initially we were unsure there is no fix needed for Qemu for this bug here.
It was all about libvirt passing the command syntax correct.
The other qemu references int his bug were due to James testing this via openstack deployments but had nothing to do with a qemu fix needed.
That said killing the yakkety bug task to let status match reality.