Failed to attach scsi contr. with opt. "--config --live" when index attribute isn't specified in XML

[bugproxy]

[Impact]

 * Libvirt has a concept of a live and a persistent config.
   The code was so far workign on a copy due to that the handling of
   device attach that caused some issues as some functions e.g.
   the scsi attach here - rely on knowing data from the other live
   content.

 * This was fixed upstream to use the correct definition in those cases
   and by effectively splitting the definitions used for checks/code in
   CONFIG/LIVE cases.

[Test Case]

 * Simplified, but complete test steps (working on a fresh Bionic x86 host):
$ sudo apt install uvtool-libvirt
$ time uvt-simplestreams-libvirt --verbose sync --source http://cloud-images.ubuntu.com/daily arch=amd64 label=daily release=bionic
$ ssh-keygen
$ uvt-kvm create --password ubuntu b-testattach arch=amd64 release=bionic label=daily
$ virsh dumpxml b-testattach | grep controller.*scsi
# will show no output - ok
$ echo "<controller type='scsi' model='virtio-scsi' />" > controller_scsi.xml
# check persistent and live config
$ virsh attach-device b-testattach controller_scsi.xml --persistent
error: Failed to attach device from controller_scsi.xml
error: internal error: Cannot parse controller index -1
$ virsh attach-device b-testattach controller_scsi.xml --live --config
error: Failed to attach device from controller_scsi.xml
error: internal error: Cannot parse controller index -1

With the fix those attaches should work just fine.

[Regression Potential]

 * The changes are local to attaching devices, no other functionality
   should be affected. If anything then e.g. due to backporting we could
   have introduced issues there - I'll take a look at testing the case and
   some other attach/detach in general to be sure.

[Other Info]

 * n/a

---

== Comment: #0 - SRIKANTH AITHAL - 2019-03-28 05:10:54 ==
---Problem Description---
Failed to attach scsi contr. with opt. "--config --live" when index attribute isn't specified in XML

---Steps to Reproduce---
 1. Use below guest:
# virsh list
 Id Name State
----------------------------------------------------
 11 srik_ubuntu running

2.Use below xml for controller attach
# cat /home/srikanth/controller_scsi.xml
<controller type='scsi' model='virtio-scsi' />

3. Check for any previous controllers attached:
# virsh dumpxml srik_ubuntu | grep controller.*scsi

4.Issue with persistent hot-attach:
# virsh attach-device srik_ubuntu /home/***/controller_scsi.xml --persistent
error: Failed to attach device from /home/***/controller_scsi.xml
error: internal error: Cannot parse controller index -1

5.Issue with persistent live-config:
# virsh attach-device srik_ubuntu /home/***/controller_scsi.xml --live --config
error: Failed to attach device from /home/***/controller_scsi.xml
error: internal error: Cannot parse controller index -1

6. Where as index is getting auto generated for live attach, as well as config attach
# virsh attach-device srik_ubuntu /home/***/controller_scsi.xml --live
Device attached successfully
# virsh dumpxml test_vm1 | grep controller.*scsi
    <controller type='scsi' index='0' model='virtio-scsi'>

Expected result:

From Libvirt >=1.3.5 all attach varients of hot controller attach should generate the controller index automatically if not given explicitly.

Machine Type = witherspoon

---uname output---
Linux ltcgen6 4.15.0-1017.19-bz175922-ibm-gt #bz175922 SMP Thu Mar 21 09:34:09 CDT 2019 ppc64le ppc64le ppc64le GNU/Linux

---Debugger---
A debugger is not configured

Userspace tool common name: libvirt-bin 4.0.0-1ubuntu8.6

The userspace tool has the following bit modes: 64

Userspace rpm: libvirt-bin

[bugproxy (bugproxy)]

Default Comment by Bridge

[Frank Heimes (fheimes)]

Marking as "invalid" as this issue was not found with the generic Ubuntu kernel. Please raise a support case against the appropriate project through the support portal.

[bugproxy (bugproxy)]

------- Comment From <email address hidden> 2019-04-08 10:21 EDT-------
(In reply to comment #10)
> Marking as "invalid" as this issue was not found with the generic Ubuntu
> kernel. Please raise a support case against the appropriate project through
> the support portal.

This bug also reproduces on ubuntu bionic stock kernel (4.15.0-46-generic) on ppc64el.

I have been dealing with this bug in the last days. It is a bug on libvirt, and it was fixed on 4.6.

I am backporting commit 55ce6564, which seems to fix this issue.
The preview patch is attached.

I only need to do some testing before verifying there is no other change needed.

[bugproxy (bugproxy)]

------- Comment on attachment From <email address hidden> 2019-04-08 10:27 EDT-------

This patch can be applied on top of https://git.launchpad.net/ubuntu/+source/libvirt
branch origin/ubuntu/bionic-devel

[Frank Heimes (fheimes)]

Ok, as you managed to reproduce it also on the stock bionic kernel (4.15.0-46-generic) we will handle it based on the enablement efforts, too and I'll reopen this ticket again (setting it to Triaged).

[Christian Ehrhardt  (paelzer)]

Simplified, but complete test steps (working on a fresh Bionic x86 host):
$ sudo apt install uvtool-libvirt
$ time uvt-simplestreams-libvirt --verbose sync --source http://cloud-images.ubuntu.com/daily arch=amd64 label=daily release=bionic
$ ssh-keygen
$ uvt-kvm create --password ubuntu b-testattach arch=amd64 release=bionic label=daily
$ virsh dumpxml b-testattach | grep controller.*scsi
# will show no output - ok
$ echo "<controller type='scsi' model='virtio-scsi' />" > controller_scsi.xml
# check persistent and live config
$ virsh attach-device b-testattach controller_scsi.xml --persistent
error: Failed to attach device from controller_scsi.xml
error: internal error: Cannot parse controller index -1
$ virsh attach-device b-testattach controller_scsi.xml --live --config
error: Failed to attach device from controller_scsi.xml
error: internal error: Cannot parse controller index -1

With the fix those attaches should work just fine.

[Christian Ehrhardt  (paelzer)]

@Leonardo - thanks for the backport, but this patch won't help.
The reason is that it applied to
 https://git.launchpad.net/ubuntu/+source/libvirt/log/?h=ubuntu/bionic-devel
as-is, but this is before all the patches in
 https://git.launchpad.net/ubuntu/+source/libvirt/tree/debian/patches?h=ubuntu/bionic-devel
are applied.
Among other things this applies:
 https://git.launchpad.net/ubuntu/+source/libvirt/tree/debian/patches/stable/0027-Pass-oldDev-to-virDomainDefCompatibleDevice-on-devic.patch?h=ubuntu/bionic-devel

Due to that I can't take the patch as-is but need to backport it on my own.
Somewhat noisy but it should work.

[bugproxy (bugproxy)]

------- Comment From <email address hidden> 2019-04-10 09:50 EDT-------
> @Leonardo - thanks for the backport, but this patch won't help.
> The reason is that it applied to
> https://git.launchpad.net/ubuntu/+source/libvirt/log/?h=ubuntu/bionic-devel
> as-is, but this is before all the patches in
> https://git.launchpad.net/ubuntu/+source/libvirt/tree/debian/
> patches?h=ubuntu/bionic-devel
> are applied.
> Among other things this applies:
> https://git.launchpad.net/ubuntu/+source/libvirt/tree/debian/patches/stable/
> 0027-Pass-oldDev-to-virDomainDefCompatibleDevice-on-devic.patch?h=ubuntu/
> bionic-devel
>
> Due to that I can't take the patch as-is but need to backport it on my own.
> Somewhat noisy but it should work.

Sorry, my mistake.
I have updated the patch to be applied over https://git.launchpad.net/ubuntu/+source/libvirt/log/?h=applied/ubuntu/bionic-devel.
Please take a look.

[bugproxy (bugproxy)]

------- Comment on attachment From <email address hidden> 2019-04-10 09:52 EDT-------

I have already applied it over the applied/ubuntu-bionic-devel tree, created the packages for ppc64el, and tested myself.
It seems to be working.

[Christian Ehrhardt  (paelzer)]

I'm testing the PPA at [1] myself, please feel free to do so as well if you want.

[1]: https://launchpad.net/~paelzer/+archive/ubuntu/bug-1823676-libvirt-scsi-attach

[Christian Ehrhardt  (paelzer)]

FYI - some tests completed, but others failed on the weekend (for other reasons).
I'm rerunning them, in case you tested the PPA yourself and have any negative feedback so far let me know.

[Christian Ehrhardt  (paelzer)]

Regression testing completed, all tests passed

The most interesting bits are on disk/and other hotplug which worked fine as well:
...
8.0.0 (13:24:57): Check further disk types on bionic
  8.1.0 (13:24:57): LVM disks
    8.1.1 (13:24:57): prepare lvm disk
    8.1.2 (13:25:22): prepare lvm volumes
    8.1.3 (13:25:28): prep 2nd KVM guest bionic-2nd-lvm on bionic-kvm for bionic
    8.1.4 (13:27:32): attach-device disk-lvm.xml => Pass
    8.1.5 (13:27:34): Check if extra disk is present on bionic-2nd-lvm => Pass
    8.1.6 (13:27:38): detach-device disk-lvm.xml => Pass
    8.1.7 (13:27:39): Check if extra disk is not-present on bionic-2nd-lvm => Pass
    8.1.8 (13:27:42): start guest with static extra disk
    8.1.9 (13:28:02): start 2nd lvl guest bionic-2nd-lvm on bionic-kvm => Pass
    8.1.10 (13:28:05): does guest with extra static disk boot up => Pass
    8.1.11 (13:28:46): Check if extra disk is present on bionic-2nd-lvm => Pass
  8.2.0 (13:29:04): ZFS disks
    8.2.1 (13:29:04): prepare ZFS disk
    8.2.2 (13:29:22): prep 2nd KVM guest bionic-2nd-zfs on bionic-kvm for bionic
    8.2.3 (13:30:19): attach-device disk-zfs.xml => Pass
    8.2.4 (13:30:21): Check if extra disk is present on bionic-2nd-zfs => Pass
    8.2.5 (13:30:25): detach-device disk-zfs.xml => Pass
    8.2.6 (13:30:27): Check if extra disk is not-present on bionic-2nd-zfs => Pass
    8.2.7 (13:30:29): start guest with static extra disk
    8.2.8 (13:30:49): start 2nd lvl guest bionic-2nd-zfs on bionic-kvm => Pass
    8.2.9 (13:30:52): does guest with extra static disk boot up => Pass
    8.2.10 (13:31:33): Check if extra disk is present on bionic-2nd-zfs => Pass

9.0.0 (13:31:47): Check non HW depending hotplug on bionic
    9.0.1 (13:31:47): prep 2nd KVM guest bionic-2nd-hotplug on bionic-kvm for bionic
  9.1.0 (13:32:42): hotplug rng
    9.1.1 (13:32:45): attach-device hotplug-rng.xml => Pass
    9.1.2 (13:32:47): detach-device hotplug-rng.xml => Pass
  9.2.0 (13:32:55): hotplug input
    9.2.1 (13:32:58): attach-device hotplug-input.xml => Pass
    9.2.2 (13:33:01): detach-device hotplug-input.xml => Pass
  9.3.0 (13:33:03): hotplug memory
    9.3.1 (13:33:21): start guest with numa nodes for mem hotplug
    9.3.2 (13:33:23): start 2nd lvl guest bionic-2nd-hotplug on bionic-kvm => Pass
    9.3.3 (13:33:25): does guest with more-max-mem boot up => Pass
    9.3.4 (13:34:09): attach-device hotplug-mem64.xml => Pass
    9.3.5 (13:34:12): detach-device hotplug-mem64.xml => Pass
...

Preparing an SRU upload for this

[Christian Ehrhardt  (paelzer)]

Uploaded to the SRU -unapproved queue [1].
Waiting on the SRU team now.

[1]: https://launchpad.net/ubuntu/bionic/+queue?queue_state=1

[Brian Murray (brian-murray)]

Hello bugproxy, or anyone else affected,

Accepted libvirt into bionic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/libvirt/4.0.0-1ubuntu8.9 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-bionic to verification-done-bionic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-bionic. In either case, without details of your testing we will not be able to proceed.

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

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

[Christian Ehrhardt  (paelzer)]

Recreated setup, pre-fix the error triggers:
root@b:~# virsh attach-device b-testattach controller_scsi.xml --persistent
error: Failed to attach device from controller_scsi.xml
error: internal error: Cannot parse controller index -1

root@b:~# virsh attach-device b-testattach controller_scsi.xml --live --config
error: Failed to attach device from controller_scsi.xml
error: internal error: Cannot parse controller index -1

Installing from proposed:
root@b:~# apt update
Hit:1 http://archive.ubuntu.com/ubuntu bionic InRelease
Hit:2 http://security.ubuntu.com/ubuntu bionic-security InRelease
Hit:3 http://archive.ubuntu.com/ubuntu bionic-updates InRelease
Hit:4 http://archive.ubuntu.com/ubuntu bionic-backports InRelease
Hit:5 http://archive.ubuntu.com/ubuntu bionic-proposed InRelease
Reading package lists... Done
Building dependency tree
Reading state information... Done
7 packages can be upgraded. Run 'apt list --upgradable' to see them.
root@b:~# apt upgrade
Reading package lists... Done
Building dependency tree
Reading state information... Done
Calculating upgrade... Done
The following packages will be upgraded:
  libvirt-clients libvirt-daemon libvirt-daemon-driver-storage-rbd libvirt-daemon-system libvirt0 netplan.io nplan
7 upgraded, 0 newly installed, 0 to remove and 0 not upgraded.
Need to get 62.3 kB/4177 kB of archives.
After this operation, 12.3 kB of additional disk space will be used.
Do you want to continue? [Y/n] Y
Get:1 http://archive.ubuntu.com/ubuntu bionic-proposed/main amd64 netplan.io amd64 0.96-0ubuntu0.18.04.4 [60.5 kB]
Get:2 http://archive.ubuntu.com/ubuntu bionic-proposed/main amd64 nplan all 0.96-0ubuntu0.18.04.4 [1792 B]
Fetched 62.3 kB in 0s (613 kB/s)
Preconfiguring packages ...
(Reading database ... 52646 files and directories currently installed.)
Preparing to unpack .../0-libvirt-daemon-system_4.0.0-1ubuntu8.9_amd64.deb ...
Unpacking libvirt-daemon-system (4.0.0-1ubuntu8.9) over (4.0.0-1ubuntu8.8) ...
Preparing to unpack .../1-libvirt-clients_4.0.0-1ubuntu8.9_amd64.deb ...
Unpacking libvirt-clients (4.0.0-1ubuntu8.9) over (4.0.0-1ubuntu8.8) ...
Preparing to unpack .../2-libvirt-daemon-driver-storage-rbd_4.0.0-1ubuntu8.9_amd64.deb ...
Unpacking libvirt-daemon-driver-storage-rbd (4.0.0-1ubuntu8.9) over (4.0.0-1ubuntu8.8) ...
Preparing to unpack .../3-libvirt-daemon_4.0.0-1ubuntu8.9_amd64.deb ...
Unpacking libvirt-daemon (4.0.0-1ubuntu8.9) over (4.0.0-1ubuntu8.8) ...
Preparing to unpack .../4-libvirt0_4.0.0-1ubuntu8.9_amd64.deb ...
Unpacking libvirt0:amd64 (4.0.0-1ubuntu8.9) over (4.0.0-1ubuntu8.8) ...
Preparing to unpack .../5-netplan.io_0.96-0ubuntu0.18.04.4_amd64.deb ...
Unpacking netplan.io (0.96-0ubuntu0.18.04.4) over (0.96-0ubuntu0.18.04.3) ...
Preparing to unpack .../6-nplan_0.96-0ubuntu0.18.04.4_all.deb ...
Unpacking nplan (0.96-0ubuntu0.18.04.4) over (0.96-0ubuntu0.18.04.3) ...
Processing triggers for ureadahead (0.100.0-21) ...
Setting up libvirt0:amd64 (4.0.0-1ubuntu8.9) ...
Setting up netplan.io (0.96-0ubuntu0.18.04.4) ...
Setting up libvirt-daemon (4.0.0-1ubuntu8.9) ...
Processing triggers for libc-bin (2.27-3ubuntu1) ...
Setting up nplan (0.96-0ubuntu0.18.04.4) ...
Processing triggers for sy...

[Launchpad Janitor (janitor)]

This bug was fixed in the package libvirt - 4.0.0-1ubuntu8.9

---------------
libvirt (4.0.0-1ubuntu8.9) bionic; urgency=medium

  * d/p/ubuntu/lp-1823676-Use-the-correct-vm-def-on-cold-attach.patch:
    fix issues attaching scsi adapters without explicit index (LP: #1823676)

 -- Christian Ehrhardt <email address hidden> Wed, 10 Apr 2019 15:14:09 +0200

[Brian Murray (brian-murray)]

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.