RHEL Engineering is moving the tracking of its product development work on RHEL 6 through RHEL 9 to Red Hat Jira (issues.redhat.com). If you're a Red Hat customer, please continue to file support cases via the Red Hat customer portal. If you're not, please head to the "RHEL project" in Red Hat Jira and file new tickets here. Individual Bugzilla bugs in the statuses "NEW", "ASSIGNED", and "POST" are being migrated throughout September 2023. Bugs of Red Hat partners with an assigned Engineering Partner Manager (EPM) are migrated in late September as per pre-agreed dates. Bugs against components "kernel", "kernel-rt", and "kpatch" are only migrated if still in "NEW" or "ASSIGNED". If you cannot log in to RH Jira, please consult article #7032570. That failing, please send an e-mail to the RH Jira admins at rh-issues@redhat.com to troubleshoot your issue as a user management inquiry. The email creates a ServiceNow ticket with Red Hat. Individual Bugzilla bugs that are migrated will be moved to status "CLOSED", resolution "MIGRATED", and set with "MigratedToJIRA" in "Keywords". The link to the successor Jira issue will be found under "Links", have a little "two-footprint" icon next to it, and direct you to the "RHEL project" in Red Hat Jira (issue links are of type "https://issues.redhat.com/browse/RHEL-XXXX", where "X" is a digit). This same link will be available in a blue banner at the top of the page informing you that that bug has been migrated.
Bug 1631733 - sgio setting doesn't take effect in scsi hostdev
Summary: sgio setting doesn't take effect in scsi hostdev
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: libvirt
Version: 7.6
Hardware: x86_64
OS: Linux
medium
medium
Target Milestone: rc
: ---
Assignee: John Ferlan
QA Contact: yisun
URL:
Whiteboard:
Depends On:
Blocks: 1656360 1656362
TreeView+ depends on / blocked
 
Reported: 2018-09-21 11:49 UTC by yisun
Modified: 2018-12-05 10:38 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
: 1656360 (view as bug list)
Environment:
Last Closed: 2018-12-05 02:17:57 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)

Description yisun 2018-09-21 11:49:25 UTC
Description of problem:
sgio setting doesn't take effect in scsi hostdev

Version-Release number of selected component (if applicable):
libvirt-4.5.0-10.virtcov.el7.x86_64
qemu-kvm-rhev-2.12.0-17.el7.x86_64


How reproducible:
100%

Steps to Reproduce:
1. Having a iscsi based scsi device on host
# lsscsi -g
...
[11:0:0:0]   disk    LIO-ORG  device.logical-  4.0   /dev/sdb   /dev/sg2 

# ll /dev/sg2 
crw-rw----. 1 root root 21, 2 Sep 21 06:44 /dev/sg2

# cat /sys/dev/char/21\:2/device/unpriv_sgio 
0

2. Passthrough the deviec to vm with following xml
    <hostdev mode='subsystem' type='scsi' managed='no' sgio='unfiltered'>
      <source>
        <adapter name='scsi_host11'/>
        <address bus='0' target='0' unit='0'/>
      </source>
      <alias name='hostdev0'/>
      <address type='drive' controller='0' bus='0' target='0' unit='1'/>
    </hostdev>

3. start the vm
# virsh start avocado-vt-vm1
virDomain avocado-vt-vm1 started


4. check the value of /sys/dev/char/21\:2/device/unpriv_sgio
# cat /sys/dev/char/21\:2/device/unpriv_sgio
0
<====== sgio='unfiltered' should set this value to 1, but here is 0

5. use following xml (sgio='filtered'), destroy and start the vm again
    <hostdev mode='subsystem' type='scsi' managed='no' sgio='filtered'>
      <source>
        <adapter name='scsi_host11'/>
        <address bus='0' target='0' unit='0'/>
      </source>
      <alias name='hostdev0'/>
      <address type='drive' controller='0' bus='0' target='0' unit='1'/>
    </hostdev>

# virsh start avocado-vt-vm1
Domain avocado-vt-vm1 started

# cat /sys/dev/char/21\:2/device/unpriv_sgio
0


6. check if scsi3 PR command can be issued in vm
[root@localhost ~]# cat test.sh
#! /bin/sh
sg_persist --no-inquiry -v --out --register-ignore --param-sark 123aaa "$@"
sg_persist --no-inquiry --in -k "$@"
sg_persist --no-inquiry -v --out --reserve --param-rk 123aaa --prout-type 5 "$@"
sg_persist --no-inquiry --in -r "$@"
sg_persist --no-inquiry -v --out --release --param-rk 123aaa --prout-type 5 "$@"
sg_persist --no-inquiry --in -r "$@"
sg_persist --no-inquiry -v --out --register --param-rk 123aaa --prout-type 5 "$@"
sg_persist --no-inquiry --in -k "$@"
[root@localhost ~]# lsblk
NAME          MAJ:MIN RM  SIZE RO TYPE MOUNTPOINT
sda             8:0    0   10G  0 disk 
├─sda1          8:1    0    1G  0 part /boot
└─sda2          8:2    0    9G  0 part 
  ├─rhel-root 253:0    0    8G  0 lvm  /
  └─rhel-swap 253:1    0    1G  0 lvm  [SWAP]
sdb             8:16   0 1000M  0 disk 
[root@localhost ~]# sh test.sh /dev/sdb
    Persistent Reservation Out cmd: 5f 06 00 00 00 00 00 00 18 00 
PR out: command (Register and ignore existing key) successful
  PR generation=0x7, 1 registered reservation key follows:
    0x123aaa
    Persistent Reservation Out cmd: 5f 01 05 00 00 00 00 00 18 00 
PR out: command (Reserve) successful
  PR generation=0x7, Reservation follows:
    Key=0x123aaa
    scope: LU_SCOPE,  type: Write Exclusive, registrants only
    Persistent Reservation Out cmd: 5f 02 05 00 00 00 00 00 18 00 
PR out: command (Release) successful
  PR generation=0x7, there is NO reservation held
    Persistent Reservation Out cmd: 5f 00 05 00 00 00 00 00 18 00 
PR out: command (Register) successful
  PR generation=0x7, there are NO registered reservation keys
<====== all commands issued successfully, but expecting failures.


Actual results:
hostdev's sgio doesn't take effect.


Additional info:
virtual disk xml works fine, when sgio='unfiltered', the unpri_sgio value will be set to 1, and scsi3 pr cmds can be issued. If sgio='filtered', unpriv_sgio will be set to 0 and scsi3 pr commands cannot be issued successfully in vm.

Comment 9 John Ferlan 2018-12-04 18:32:31 UTC
Going back to the problem statement and the dump of the hostdev:

2. Passthrough the deviec to vm with following xml
    <hostdev mode='subsystem' type='scsi' managed='no' sgio='unfiltered'>
      <source>
        <adapter name='scsi_host11'/>
        <address bus='0' target='0' unit='0'/>
      </source>
      <alias name='hostdev0'/>
      <address type='drive' controller='0' bus='0' target='0' unit='1'/>
    </hostdev>

I realized something is missing... There's no <shareable/> element which is a requirement for things to work. If you look at the referenced bz:

https://bugzilla.redhat.com/show_bug.cgi?id=1072736#c12

you'll note that during my development/investigation of the original change, it was noted there that the shareable element had to be provided. Once I do that on the host you provided me, things seem to work.

# ls -al /sys/dev/block/*/device/unpriv_sgio
-rw-r--r--. 1 root root 4096 Dec  4 12:19 /sys/dev/block/8:0/device/unpriv_sgio
-rw-r--r--. 1 root root 4096 Dec  4 12:19 /sys/dev/block/8:16/device/unpriv_sgio
#  cat /sys/dev/block/*/device/unpriv_sgio
0
0
# virsh destroy avocado-vt-vm1
Domain 2 destroyed

(edit the domain to add the <shareable/> to the hostdev)
# virsh edit avocado-vt-vm1
Domain avocado-vt-vm1 XML configuration edited.

# virsh start avocado-vt-vm1
Domain avocado-vt-vm1 started

# virsh dumpxml avocado-vt-vm1 | grep -A8 "\<hostdev mode"
    <hostdev mode='subsystem' type='scsi' managed='no' sgio='unfiltered'>
      <source>
        <adapter name='scsi_host89'/>
        <address bus='0' target='0' unit='0'/>
      </source>
      <shareable/>
      <alias name='hostdev0'/>
      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
    </hostdev>

# cat /sys/dev/block/*/device/unpriv_sgio
0
1
#

So, I think this particular issue can be set as notabug; however, there is an interesting gotcha that I did find in the code. Because the code returns 0 early, if unpriv_sgio was previously set, then someone removes the <shareable/> element from the domain, the sysfs value won't get changed.

...
    } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) {
        hostdev = dev->data.hostdev;

        if (!qemuIsSharedHostdev(hostdev))
            return 0;

        if (!(hostdev_path = qemuGetHostdevPath(hostdev)))
            goto cleanup;

        path = hostdev_path;
    } else {
...

Note the quiet return 0 if not shared.

But that's a "different" bug/issue... Present in existing code and fix-able in a "future" revision.  It's not that big a deal anyway.

I did test that I could fix it though... Using the /home/libvirt-rhel repository that I built with the /home/libvirt-rhel/amend.patch, you can "service libvirtd stop", then "setenforce 0", "/home/libvirt-rhel/run /home/libvirt-rhel/src/libvirtd", and then see that when the <shareable/> is removed from the domain that a previously set value of 1 is returned to 0. And then restore things by quitting the libvirtd, resetting "setenforce enforcing", and restarting libvirt "service libvirtd start".

I've left the system essentially the same way I found it with avocado-vt-vm1 running *without* the <shareable/> set on the hostdev. You can test that at least by adding <shareable/> and restarting the domain that you get what's expected.

Comment 10 yisun 2018-12-05 02:17:57 UTC
(In reply to John Ferlan from comment #9)
> Going back to the problem statement and the dump of the hostdev:
> So, I think this particular issue can be set as notabug; however, there is
> an interesting gotcha that I did find in the code. Because the code returns
> 0 early, if unpriv_sgio was previously set, then someone removes the
> <shareable/> element from the domain, the sysfs value won't get changed.
> 
> ...
>     } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) {
>         hostdev = dev->data.hostdev;
> 
>         if (!qemuIsSharedHostdev(hostdev))
>             return 0;
> 
>         if (!(hostdev_path = qemuGetHostdevPath(hostdev)))
>             goto cleanup;
> 
>         path = hostdev_path;
>     } else {
> ...
> 
> Note the quiet return 0 if not shared.
> 
> But that's a "different" bug/issue... Present in existing code and fix-able
> in a "future" revision.  It's not that big a deal anyway.
> 
> I did test that I could fix it though... Using the /home/libvirt-rhel
> repository that I built with the /home/libvirt-rhel/amend.patch, you can
> "service libvirtd stop", then "setenforce 0", "/home/libvirt-rhel/run
> /home/libvirt-rhel/src/libvirtd", and then see that when the <shareable/> is
> removed from the domain that a previously set value of 1 is returned to 0.
> And then restore things by quitting the libvirtd, resetting "setenforce
> enforcing", and restarting libvirt "service libvirtd start".
> 
> I've left the system essentially the same way I found it with avocado-vt-vm1
> running *without* the <shareable/> set on the hostdev. You can test that at
> least by adding <shareable/> and restarting the domain that you get what's
> expected.

hmm... my bad, didn't add <shareable/> in test code, totally forget about it ...  Thanks for the further investigation. I'm closing this bug and will open a new one for your last comment 9.


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