Commit id '36555364' removed the setting of the driver->privileged,
which the udevProcessPCI would need in order to read the PCI device
configs.

Signed-off-by: John Ferlan <jferlan@redhat.com>
---

 Not quite sure after seeing other issues how I missed this during
 review of the series to change the udev nodedev code to use a thread.

 I tripped across this while "investigating" a recurring issue I'm
 having with the udev code in an avocado nwfilter test where during
 a libvirtd restart the udev initialization "hangs" and cannot be killed
 resulting in a <defunct> process and the only recovery is reboot. Still
 trying to hack through that, but figured this should go into 3.10 at
 least. So far only 3.9 would be affected, but only to not get PCI
 device details.

 src/node_device/node_device_udev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index 35df13b153..1e1b71742b 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1933,6 +1933,8 @@ nodeStateInitialize(bool privileged,
         return -1;
     }
 
+    driver->privileged = privileged;
+
     if (!(driver->devs = virNodeDeviceObjListNew()) ||
         !(priv = udevEventDataNew()))
         goto cleanup;
-- 
2.13.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Thu, Nov 23, 2017 at 07:31:46AM -0500, John Ferlan wrote:
> Commit id '36555364' removed the setting of the driver->privileged,
> which the udevProcessPCI would need in order to read the PCI device
> configs.
>
> Signed-off-by: John Ferlan <jferlan@redhat.com>
> ---
>
>  Not quite sure after seeing other issues how I missed this during
>  review of the series to change the udev nodedev code to use a thread.

Not sure why I removed in the first place.

>
>  I tripped across this while "investigating" a recurring issue I'm
>  having with the udev code in an avocado nwfilter test where during
>  a libvirtd restart the udev initialization "hangs" and cannot be killed

Is ^this one related to the async thread as well? Because it would just add
more fuel to the centos6 + upstream libvirt fire.

>  resulting in a <defunct> process and the only recovery is reboot. Still
>  trying to hack through that, but figured this should go into 3.10 at
>  least. So far only 3.9 would be affected, but only to not get PCI
>  device details.
>
>  src/node_device/node_device_udev.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
> index 35df13b153..1e1b71742b 100644
> --- a/src/node_device/node_device_udev.c
> +++ b/src/node_device/node_device_udev.c
> @@ -1933,6 +1933,8 @@ nodeStateInitialize(bool privileged,
>          return -1;
>      }
>
> +    driver->privileged = privileged;
> +
>      if (!(driver->devs = virNodeDeviceObjListNew()) ||
>          !(priv = udevEventDataNew()))
>          goto cleanup;
> --

Reviewed-by: Erik Skultety <eskultet@redhat.com>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 11/23/2017 09:12 AM, Erik Skultety wrote:
> On Thu, Nov 23, 2017 at 07:31:46AM -0500, John Ferlan wrote:
>> Commit id '36555364' removed the setting of the driver->privileged,
>> which the udevProcessPCI would need in order to read the PCI device
>> configs.
>>
>> Signed-off-by: John Ferlan <jferlan@redhat.com>
>> ---
>>
>>  Not quite sure after seeing other issues how I missed this during
>>  review of the series to change the udev nodedev code to use a thread.
> 
> Not sure why I removed in the first place.
> 
>>
>>  I tripped across this while "investigating" a recurring issue I'm
>>  having with the udev code in an avocado nwfilter test where during
>>  a libvirtd restart the udev initialization "hangs" and cannot be killed
> 
> Is ^this one related to the async thread as well? Because it would just add
> more fuel to the centos6 + upstream libvirt fire.
> 

Sadly no...  The call to udevPCITranslateInit (e.g. pci_system_init)
never returns while processing some PCI device. It's not very
reproducible, but it seems to be some interaction with systemd usage of
an lspci, but it's hard to prove and debug as it only happens running
"some" avocado test "at the right time". I could run 8 times with no
failure, but on the 9th it fails. Other times it happens on the 2nd run.
Never on the first one (figures, eh!?).

As for the upstream issue - my thought there is very simplistic and
hopeful that it'd pass muster.... Just modify the build or install
dependencies to check for a minimum systemd version and be done with it!
 Although to be honest I haven't put that much thought to it ;-).
They're taking upstream libvirt and expecting that it'd work with some
older systemd - I dunno if I'd expect that.  The question becomes what
version of libvirt does the Centos6 distribution provide... If then you
as a consumer decide to build/run further version upstream code - you
should also be expected to update whatever requirement that code has.

John

>>  resulting in a <defunct> process and the only recovery is reboot. Still
>>  trying to hack through that, but figured this should go into 3.10 at
>>  least. So far only 3.9 would be affected, but only to not get PCI
>>  device details.
>>
>>  src/node_device/node_device_udev.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
>> index 35df13b153..1e1b71742b 100644
>> --- a/src/node_device/node_device_udev.c
>> +++ b/src/node_device/node_device_udev.c
>> @@ -1933,6 +1933,8 @@ nodeStateInitialize(bool privileged,
>>          return -1;
>>      }
>>
>> +    driver->privileged = privileged;
>> +
>>      if (!(driver->devs = virNodeDeviceObjListNew()) ||
>>          !(priv = udevEventDataNew()))
>>          goto cleanup;
>> --
> 
> Reviewed-by: Erik Skultety <eskultet@redhat.com>
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list