Bug 1631733

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.

(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.