Bug 1664324 - Failed to attach scsi controller with option "--config --live" when index attribute isn't specified in XML
Summary: Failed to attach scsi controller with option "--config --live" when index att...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 8
Classification: Red Hat
Component: libvirt
Version: 8.0
Hardware: x86_64
OS: Linux
high
medium
Target Milestone: rc
: 8.3
Assignee: Laine Stump
QA Contact: Virtualization Bugs
URL:
Whiteboard:
: 1647304 (view as bug list)
Depends On: 1559867
Blocks: 1699081 1776265 1846368 1344899
TreeView+ depends on / blocked
 
Reported: 2019-01-08 13:06 UTC by Andrea Bolognani
Modified: 2020-11-04 02:54 UTC (History)
12 users (show)

Fixed In Version: libvirt-6.0.0-17.el8
Doc Type: If docs needed, set a value
Doc Text:
Clone Of: 1344899
Environment:
Last Closed: 2020-11-04 02:53:02 UTC
Type: Bug
Target Upstream Version:


Attachments (Terms of Use)
patch (5.21 KB, patch)
2019-01-16 20:10 UTC, IBM Bug Proxy
no flags Details | Diff
machine.xml used (2.90 KB, application/octet-stream)
2019-01-16 20:20 UTC, IBM Bug Proxy
no flags Details


Links
System ID Private Priority Status Summary Last Updated
Red Hat Product Errata RHSA-2020:4676 0 None None None 2020-11-04 02:53:55 UTC

Description Andrea Bolognani 2019-01-08 13:06:22 UTC
+++ This bug was initially created as a clone of Bug #1344899 +++

Description of problem:
Failed to attach scsi controller with option "--config --live"

Version-Release number of selected component (if applicable):
libvirt-1.3.5-1.el7.x86_64
qemu-kvm-rhev-2.6.0-5.el7.x86_64

How reproducible:
100%

Steps to Reproduce:
1. Start a domxml without scsi controller.
# virsh start avocado-vt-vm1
Domain avocado-vt-vm1 started

2. Hot-plug a scsi controller device without index.
# cat ctl.xml
<controller model="virtio-scsi" type="scsi" />

# virsh attach-device avocado-vt-vm1 ctl.xml --config --live
error: Failed to attach device from con.xml
error: internal error: Cannot parse controller index -1

Actual results:
Failed to attach scsi controller with option "--config --live"

Expected results:
Device attachment should work well.

Infomation:
The command can work with '--config' or '--live' respectively.

--- Additional comment from Laine Stump on 2016-06-12 21:43:33 UTC ---

This *should have been* taken care of by the call to virDomainControllerFindUnusedIndex() that was added to qemuDomainAttachControllerDevice() in the same commit that made specifying the index optional. Apparently there is a bad interaction with this and combining the --live and --config flags. I'll look into it.

--- Additional comment from Laine Stump on 2016-06-22 19:03:21 UTC ---

Patch posted upstream:

https://www.redhat.com/archives/libvir-list/2016-June/msg01579.html

--- Additional comment from Laine Stump on 2016-06-27 14:17:03 UTC ---

Upstream patch caused other problems; this has turned into more of a general problem with preventing --live additions and --config additions from conflicting with each other (details in the email thread replying to the above patch).

--- Additional comment from Jingjing Shao on 2016-08-29 09:03:18 UTC ---

It gets same error when attach scsi controller with option "--persistent" 

libvirt-2.0.0-6.el7.x86_64
# virsh dumpxml vm-jishao | grep scsi -A8
# 
# 
# virsh start vm-jishao
Domain vm-jishao started

# cat controller.xml 
<controller type='scsi' model='virtio-scsi'/>

# virsh attach-device  vm-jishao controller.xml  --persistent
error: Failed to attach device from controller.xml
error: internal error: Cannot parse controller index -1

--- Additional comment from Jingjing Shao on 2016-08-30 02:49:18 UTC ---

With kernel-3.10.0-327.el7.x86_64, qemu-kvm-1.5.3-105.el7.x86_64,libvirt-1.2.17-13.el7.x86_64, I can not reproduce the issue 

1.# virsh dumpxml vm-jishao | grep scsi
  # 
  # 

2.# virsh start vm-jishao
Domain vm-jishao started

3.# cat scsi-controller.xml 
<controller model="virtio-scsi" type="scsi" />

4.# virsh attach-device  vm-jishao  scsi-controller.xml  --config --live
Device attached successfully

# virsh dumpxml vm-jishao | grep scsi -A8
    <controller type='scsi' index='0' model='virtio-scsi'>
      <alias name='scsi0'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
    </controller>


5.# virsh destroy vm-jishao
Domain vm-jishao destroyed

Delete the part of the scsi 
# virsh edit vm-jishao
Domain vm-jishao XML configuration edited.

# virsh start vm-jishao
Domain vm-jishao started

6.# virsh start vm-jishao
Domain vm-jishao started

7.# virsh attach-device  vm-jishao  scsi-controller.xml  --persistent
Device attached successfully

# virsh dumpxml vm-jishao | grep scsi -A8
    <controller type='scsi' index='0' model='virtio-scsi'>
      <alias name='scsi0'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
    </controller>

--- Additional comment from Jingjing Shao on 2016-08-30 02:56:23 UTC ---

(In reply to Jingjing Shao from comment #6)
> With kernel-3.10.0-327.el7.x86_64,
> qemu-kvm-1.5.3-105.el7.x86_64,libvirt-1.2.17-13.el7.x86_64, I can not
> reproduce the issue 

The env is with RHEL7.2 release os + qemu-kvm + libvirt 

> 
> 1.# virsh dumpxml vm-jishao | grep scsi
>   # 
>   # 
> 
> 2.# virsh start vm-jishao
> Domain vm-jishao started
> 
> 3.# cat scsi-controller.xml 
> <controller model="virtio-scsi" type="scsi" />
> 
> 4.# virsh attach-device  vm-jishao  scsi-controller.xml  --config --live
> Device attached successfully
> 
> # virsh dumpxml vm-jishao | grep scsi -A8
>     <controller type='scsi' index='0' model='virtio-scsi'>
>       <alias name='scsi0'/>
>       <address type='pci' domain='0x0000' bus='0x00' slot='0x03'
> function='0x0'/>
>     </controller>
> 
> 
> 5.# virsh destroy vm-jishao
> Domain vm-jishao destroyed
> 
> Delete the part of the scsi 
> # virsh edit vm-jishao
> Domain vm-jishao XML configuration edited.
> 
> # virsh start vm-jishao
> Domain vm-jishao started
> 
> 6.# virsh start vm-jishao
> Domain vm-jishao started
> 
> 7.# virsh attach-device  vm-jishao  scsi-controller.xml  --persistent
> Device attached successfully
> 
> # virsh dumpxml vm-jishao | grep scsi -A8
>     <controller type='scsi' index='0' model='virtio-scsi'>
>       <alias name='scsi0'/>
>       <address type='pci' domain='0x0000' bus='0x00' slot='0x03'
> function='0x0'/>
>     </controller>

--- Additional comment from Laine Stump on 2016-09-07 14:28:07 UTC ---

Although I agree that this is annoying once you're accustomed to not supplying an index, I disagree that this is a test blocker - just specify an index in the <controller> element:

<controller model="virtio-scsi" index='0' type="scsi" />

(or index='1' or '2' or whatever is still unused). Until just a few months ago, libvirt always required manually specifying the index in a <controller> element, so it shouldn't be a problem to continue doing that until this one case is fixed.

--- Additional comment from Jingjing Shao on 2016-09-08 02:39:44 UTC ---

(In reply to Laine Stump from comment #9)
> Although I agree that this is annoying once you're accustomed to not
> supplying an index, I disagree that this is a test blocker - just specify an
> index in the <controller> element:
> 
> <controller model="virtio-scsi" index='0' type="scsi" />

Hi laine, 

I try this issue with "<controller model="virtio-scsi" index='0' type="scsi" />"
and the device attachment works well.

> 
> (or index='1' or '2' or whatever is still unused). Until just a few months
> ago, libvirt always required manually specifying the index in a <controller>
> element, so it shouldn't be a problem to continue doing that until this one
> case is fixed.


The operation without supplying an index is not recommended,right?

But I see "Since 1.3.5 the index is optional; if not specified, it will be auto-assigned to be the lowest unused index for the given controller type" in the libvirt.org. http://libvirt.org/formatdomain.html#elementsControllers 

Does it meam we should support this operation ?

--- Additional comment from Laine Stump on 2016-09-08 13:48:03 UTC ---

(In reply to Jingjing Shao from comment #10)

> 
> The operation without supplying an index is not recommended,right?

Currently the only time omitting the index doesn't work properly is the specific case reported in this BZ - if you try to add a controller with --live --config. And since that is the only type of controller that can be hotplugged, it's only an issue for scsi controllers. For all other controller types (and even for scsi controllers if you do the hotplug (--live) without --config, or --config without --live, it works properly.


> 
> But I see "Since 1.3.5 the index is optional; if not specified, it will be
> auto-assigned to be the lowest unused index for the given controller type"
> in the libvirt.org. http://libvirt.org/formatdomain.html#elementsControllers 
> 
> Does it meam we should support this operation ?

Yes. That's why I didn't close the BZ. It is a bug and needs to be fixed. I just don't think it qualifies as a blocker, so we should remove the blocker flag. I didn't want to do that myself though, since I'm not the person who added it.

--- Additional comment from yangyang on 2016-09-09 05:04:49 UTC ---

Hi Laine,

Is there time to fix it in RHEL7.3. If no, can we move it to RHEL7.4

--- Additional comment from Laine Stump on 2016-09-09 14:58:04 UTC ---

Reading through the thread that started from my proposed patch back in Comment 3: https://www.redhat.com/archives/libvir-list/2016-June/msg01579.html I can say that *definitely* this can't be fixed in any reasonable manner for 7.3. (and if anybody else looks at this BZ in the future, I would recommend they read *all* the child messages of that one).

Additionally, I just now noticed that this is marked as a regression. It definitely it *not* a regression. The ability to define controllers without an explicit user-determined index is a very new thing. In all previous releases of libvirt used by RHEL, any controller definition required  index to be specified.

So, I'm removing the Regression keyword, "[regression]" from the summary, and moving this to 7.4

--- Additional comment from Jingjing Shao on 2016-09-12 03:06:41 UTC ---

(In reply to Laine Stump from comment #13)
> Reading through the thread that started from my proposed patch back in
> Comment 3:
> https://www.redhat.com/archives/libvir-list/2016-June/msg01579.html I can
> say that *definitely* this can't be fixed in any reasonable manner for 7.3.
> (and if anybody else looks at this BZ in the future, I would recommend they
> read *all* the child messages of that one).
> 
> Additionally, I just now noticed that this is marked as a regression. It
> definitely it *not* a regression. The ability to define controllers without
> an explicit user-determined index is a very new thing. In all previous
> releases of libvirt used by RHEL, any controller definition required  index
> to be specified.

Hi laine, 

Would you please check the comment6 ? I mark it as a regression for that I test this issue on the env with RHEL7.2 release os + qemu-kvm + libvirt.
The device attachment can work well.

So I can not quite understand it, can you give me a more detailed explanation about it?  Thank you in advance


> 
> So, I'm removing the Regression keyword, "[regression]" from the summary,
> and moving this to 7.4

--- Additional comment from Laine Stump on 2016-09-12 13:44:20 UTC ---

Okay, I can see how this xml:

  <controller model="virtio-scsi" type='scsi' />

would have been accepted by libvirt and worked prior to the "auto-generate unspecified controller indexes" feature being added (which happened in 1.3.5) - if no index was given, the index would have remained at its initial value, which was 0. So the next time it was parsed, it would format as "index='0'".

However, the fact that it worked was just an accident, and it would only work for index='0'. The libvirt documentation has always stated this:

  "Each controller has [...], and a mandatory attribute index ..."

Also, the XML grammer (which isn't referenced when defining a domain) specifically required an index.

I'm now undecided whether this should be regression or not, since the behavior does change, but the behavior that has changed was previously documented as illegal...

Comment 1 Andrea Bolognani 2019-01-08 13:08:57 UTC
*** Bug 1647304 has been marked as a duplicate of this bug. ***

Comment 2 IBM Bug Proxy 2019-01-16 13:30:41 UTC
------- Comment From leonardo@ibm.com 2019-01-16 08:27 EDT-------
Hello,

This bug seems to be fixed since v4.7.0, I am trying to find the exact commit that fixed this.

By fixed, I mean it does not return errors anymore and controllers are added both in --live and in --config.

After a few tests, it seems the solution's added controllers can have different indexes in --live and --config if some controller is not added in both previously.

Comment 3 IBM Bug Proxy 2019-01-16 20:10:25 UTC
------- Comment From leonardo@ibm.com 2019-01-16 15:06 EDT-------
> This bug seems to be fixed since v4.7.0, I am trying to find the exact
> commit that fixed this.

The commit is :
"qemu: Use the correct vm def on cold attach "(55ce65646348884656fd7bf3f109ebf8f7603494)
(The patch is attached).

So far, I could cherry-pick this patch over v4.5.0 on git and it seems to work fine. I haven't tested it deeply,  though.

---
1: machine.xml used
(attached)

2: scsi.xml used
<controller type='scsi' model='virtio-scsi' />

3: Procedure / output

$ virsh define machine.xml
Domain leo-vm1 defined from machine.xml

$ virsh start leo-vm1
Domain leo-vm1 started

$ ~/libvirt/run ~/libvirt/tools/virsh attach-device leo-vm1 scsi.xml --persistent

$ virsh dumpxml leo-vm1 |grep scsi

$ virsh destroy leo-vm1
Domain leo-vm1 destroyed

$ virsh dumpxml leo-vm1 |grep scsi

$ virsh start leo-vm1
Domain leo-vm1 started

$ virsh attach-device leo-vm1 scsi.xml --live

$ virsh dumpxml leo-vm1 |grep scsi
<controller type='scsi' index='0' model='virtio-scsi'>
<controller type='scsi' index='1' model='virtio-scsi'>
<alias name='scsi1'/>

$ virsh attach-device leo-vm1 scsi.xml --config --live
Device attached successfully

$ virsh dumpxml leo-vm1 |grep scsi
<controller type='scsi' index='0' model='virtio-scsi'>
<alias name='scsi0'/>
<controller type='scsi' index='1' model='virtio-scsi'>
<alias name='scsi1'/>
<controller type='scsi' index='2' model='virtio-scsi'>
<alias name='scsi2'/>

/virsh destroy leo-vm1
Domain leo-vm1 destroyed

$ virsh dumpxml leo-vm1 |grep scsi
<controller type='scsi' index='0' model='virtio-scsi'>
<controller type='scsi' index='1' model='virtio-scsi'>

Comment 4 IBM Bug Proxy 2019-01-16 20:10:28 UTC
Created attachment 1521111 [details]
patch

Comment 5 IBM Bug Proxy 2019-01-16 20:20:23 UTC
Created attachment 1521113 [details]
machine.xml used

Comment 6 IBM Bug Proxy 2019-02-07 16:20:55 UTC
------- Comment From leonardo@ibm.com 2019-02-07 11:16 EDT-------
Hello Laine,

I compiled the patch on top of RHEL8-Snapshot5 libvirt and generated the rpms for testing:
(Here are all libvirt rpm outputs, just in case)

ftp://testcase.software.ibm.com/fromibm/linux/libvirt-rpms-rhel8ss5-172958.tar.gz
- Please use user=anonymous, passwd=anonymous if asked
- Make sure to download it soon, as the link will be available for 3 business days.

Could you please test it, just to make sure the bug is gone?

Thank you!

Comment 7 Laine Stump 2019-02-19 22:31:09 UTC
Yes, that patch fixes this specific problem (although it doesn't solve it in the general case, i.e. when multiple controllers are hotplugged, with different combinations of --live and --config).

Comment 9 IBM Bug Proxy 2019-09-02 19:11:12 UTC
------- Comment From bssrikanth@in.ibm.com 2019-08-30 05:29 EDT-------
This issue seems to be fixed in latest beta having kernel `4.18.0-128.el8.ppc64le` , qemu-kvm-4.1.0-0.el8.patchwork201907251253.ppc64le and libvirt-5.5.0-1.module.

Comment 10 Laine Stump 2019-10-07 15:29:16 UTC
(In reply to IBM Bug Proxy from comment #9)
> ------- Comment From bssrikanth@in.ibm.com 2019-08-30 05:29 EDT-------
> This issue seems to be fixed in latest beta having kernel
> `4.18.0-128.el8.ppc64le` ,
> qemu-kvm-4.1.0-0.el8.patchwork201907251253.ppc64le and
> libvirt-5.5.0-1.module.

Yes, the patch that fixes this specific issue has been in upstream libvirt since 5.0.0. See Comment 7 though.

Comment 13 jiyan 2020-06-11 09:36:25 UTC
Verified this bug with libvirt-6.0.0-17.module+el8.3.0+6423+e4cb6418.x86_64.

Version:
libvirt-6.0.0-17.module+el8.3.0+6423+e4cb6418.x86_64
kernel-4.18.0-213.el8.x86_64
qemu-kvm-4.2.0-19.module+el8.3.0+6473+93e27135.x86_64

Steps:
1: Prepare a shutdown VM without scsi controller, then start VM 
# virsh domstate test83 
shut off

# virsh dumpxml test83 --inactive | grep "scsi"
No output

# virsh start test83 
Domain test83 started

# virsh console test83 
Connected to domain test83
Escape character is ^]

Red Hat Enterprise Linux 8.3 Beta (Ootpa)
Kernel 4.18.0-211.el8.x86_64 on an x86_64

localhost login: root
Password: 
Last login: Thu Jun 11 16:20:09 on ttyS0
[root@localhost ~]# 

2: Attach scsi controller without index by "virsh attach-device --config --live"
# cat con.xml 
<controller model="virtio-scsi" type="scsi" />

# virsh attach-device test83 con.xml --config --live 
Device attached successfully

# virsh dumpxml test83 --inactive | grep "scsi" -A2
    <controller type='scsi' index='0' model='virtio-scsi'>
      <address type='pci' domain='0x0000' bus='0x07' slot='0x00' function='0x0'/>
    </controller>

# virsh dumpxml test83 | grep "scsi" -A2
    <controller type='scsi' index='0' model='virtio-scsi'>
      <alias name='scsi0'/>
      <address type='pci' domain='0x0000' bus='0x07' slot='0x00' function='0x0'/>
    </controller>

3: Restart VM and then attach scsi controller without index by "virsh attach-device --persistent"
# virsh destroy test83 ; virsh start test83
Domain test83 destroyed

Domain test83 started

# virsh console test83 
Connected to domain test83
Escape character is ^]

Red Hat Enterprise Linux 8.3 Beta (Ootpa)
Kernel 4.18.0-211.el8.x86_64 on an x86_64

localhost login: root
Password: 
Last login: Thu Jun 11 17:17:06 on ttyS0
[root@localhost ~]# 

# virsh dumpxml test83 --inactive | grep "scsi" -A2
    <controller type='scsi' index='0' model='virtio-scsi'>
      <address type='pci' domain='0x0000' bus='0x07' slot='0x00' function='0x0'/>
    </controller>
    <controller type='scsi' index='1' model='virtio-scsi'>
      <address type='pci' domain='0x0000' bus='0x08' slot='0x00' function='0x0'/>
    </controller>

# virsh dumpxml test83 | grep "scsi" -A2
    <controller type='scsi' index='0' model='virtio-scsi'>
      <alias name='scsi0'/>
      <address type='pci' domain='0x0000' bus='0x07' slot='0x00' function='0x0'/>
    </controller>
    <controller type='scsi' index='1' model='virtio-scsi'>
      <alias name='scsi1'/>
      <address type='pci' domain='0x0000' bus='0x08' slot='0x00' function='0x0'/>
    </controller>

All test results are asexpected, move this bug to be verified.

Comment 17 errata-xmlrpc 2020-11-04 02:53:02 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (Moderate: virt:rhel and virt-devel:rhel security, bug fix, and enhancement update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHSA-2020:4676


Note You need to log in before you can comment on or make changes to this bug.