Kernel structure looks slightly different than what virpcimock creates.
This did not use to be a problem, because we are testing KVM device
assignment even though majority of systems we run on (if not all of
them) use VFIO assignment.

In order to switch our test suite (mainly virhostdevtest and virpcitest)
to test VFIO assignment, the virpcimock must be fixed. Firstly, it needs
to create symlinks under /sys/kernel/iommu_groups/... directories (patch
13/18) so that virhostdev module can iterate over them. Secondly, it
needs to create 'driver_override' file (which exists since
kernel-3.16.0) so that the virtual environment the mock creates matches
real up to date systems (patch 03/18).

Funny thing is, that enhancing the mock uncovered a bug we had (fix is
in 15/18) and also one latent bug (14/18).


As usual, these patches can be found on my github too:

  https://github.com/zippy2/libvirt/tree/virpcimock

and just for the fun of it, here's the latest travis build of that
branch:

  https://travis-ci.org/zippy2/libvirt/builds/571752953


Michal Prívozník (18):
  virpcimock: Move actions checking one level up
  Revert "virpcitest: Test virPCIDeviceDetach failure"
  virpcimock: Create driver_override file in device dirs
  virpcimock: Drop needless typecast
  virpcimock: Use VIR_AUTOFREE()
  virpcimock: Eliminate use of @fakesysfspcidir
  virpcimock: Rename @fakesysfspcidir
  virpcimock: Create devices/ and drivers/ under /sys/bus/pci upfront
  virpcimock: Introduce and use pci_device_get_path()
  virpcimock: Introduce and use pci_driver_get_path()
  virpcimock: Store PCI address as ints not string
  virpcimock: Create PCI devices under /sys/devices/pci*
  virpcimock: Create symlink in /sys/kernel/iommu_groups/N/devices dir
  virhostdev: Check driver name too in virHostdevIsPCINodeDeviceUsed()
  virhostdev: Unify virHostdevPreparePCIDevices behaviour for KVM and
    VFIO cases
  qemuxml2argvtest: Switch to modern vfio backend
  virhostdevtest: Use modern VFIO
  virpcitest: Use modern VFIO

 src/util/virhostdev.c                         |  26 +-
 .../hostdev-pci-address-device.args           |   2 +-
 .../qemuxml2argvdata/hostdev-pci-address.args |   2 +-
 .../net-hostdev-bootorder.args                |   3 +-
 .../net-hostdev-multidomain.args              |   2 +-
 tests/qemuxml2argvdata/net-hostdev.args       |   2 +-
 tests/qemuxml2argvdata/pci-rom.args           |   4 +-
 tests/qemuxml2argvtest.c                      |  14 +-
 tests/virhostdevtest.c                        |   4 +-
 tests/virpcimock.c                            | 394 ++++++++++++------
 tests/virpcitest.c                            |  48 +--
 11 files changed, 304 insertions(+), 197 deletions(-)

-- 
2.21.0

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

I've run make check with each individual patch, and everything
seems fine in my environment.

For all patches:

Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>


I'll see if I can drop some code reviews later on.


Thanks,


DHB

On 8/14/19 8:57 AM, Michal Privoznik wrote:
> Kernel structure looks slightly different than what virpcimock creates.
> This did not use to be a problem, because we are testing KVM device
> assignment even though majority of systems we run on (if not all of
> them) use VFIO assignment.
>
> In order to switch our test suite (mainly virhostdevtest and virpcitest)
> to test VFIO assignment, the virpcimock must be fixed. Firstly, it needs
> to create symlinks under /sys/kernel/iommu_groups/... directories (patch
> 13/18) so that virhostdev module can iterate over them. Secondly, it
> needs to create 'driver_override' file (which exists since
> kernel-3.16.0) so that the virtual environment the mock creates matches
> real up to date systems (patch 03/18).
>
> Funny thing is, that enhancing the mock uncovered a bug we had (fix is
> in 15/18) and also one latent bug (14/18).
>
>
> As usual, these patches can be found on my github too:
>
>    https://github.com/zippy2/libvirt/tree/virpcimock
>
> and just for the fun of it, here's the latest travis build of that
> branch:
>
>    https://travis-ci.org/zippy2/libvirt/builds/571752953
>
>
> Michal Prívozník (18):
>    virpcimock: Move actions checking one level up
>    Revert "virpcitest: Test virPCIDeviceDetach failure"
>    virpcimock: Create driver_override file in device dirs
>    virpcimock: Drop needless typecast
>    virpcimock: Use VIR_AUTOFREE()
>    virpcimock: Eliminate use of @fakesysfspcidir
>    virpcimock: Rename @fakesysfspcidir
>    virpcimock: Create devices/ and drivers/ under /sys/bus/pci upfront
>    virpcimock: Introduce and use pci_device_get_path()
>    virpcimock: Introduce and use pci_driver_get_path()
>    virpcimock: Store PCI address as ints not string
>    virpcimock: Create PCI devices under /sys/devices/pci*
>    virpcimock: Create symlink in /sys/kernel/iommu_groups/N/devices dir
>    virhostdev: Check driver name too in virHostdevIsPCINodeDeviceUsed()
>    virhostdev: Unify virHostdevPreparePCIDevices behaviour for KVM and
>      VFIO cases
>    qemuxml2argvtest: Switch to modern vfio backend
>    virhostdevtest: Use modern VFIO
>    virpcitest: Use modern VFIO
>
>   src/util/virhostdev.c                         |  26 +-
>   .../hostdev-pci-address-device.args           |   2 +-
>   .../qemuxml2argvdata/hostdev-pci-address.args |   2 +-
>   .../net-hostdev-bootorder.args                |   3 +-
>   .../net-hostdev-multidomain.args              |   2 +-
>   tests/qemuxml2argvdata/net-hostdev.args       |   2 +-
>   tests/qemuxml2argvdata/pci-rom.args           |   4 +-
>   tests/qemuxml2argvtest.c                      |  14 +-
>   tests/virhostdevtest.c                        |   4 +-
>   tests/virpcimock.c                            | 394 ++++++++++++------
>   tests/virpcitest.c                            |  48 +--
>   11 files changed, 304 insertions(+), 197 deletions(-)
>

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

On 8/14/19 5:14 PM, Daniel Henrique Barboza wrote:
> I've run make check with each individual patch, and everything
> seems fine in my environment.
> 
> For all patches:
> 
> Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> 
> 
> I'll see if I can drop some code reviews later on.

That'd be great because we are lacking reviewers (just like other 
projects) and these are very specific.

Thanks,
Michal

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

On 8/16/19 5:59 AM, Michal Privoznik wrote:
> On 8/14/19 5:14 PM, Daniel Henrique Barboza wrote:
>> I've run make check with each individual patch, and everything
>> seems fine in my environment.
>>
>> For all patches:
>>
>> Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>>
>>
>> I'll see if I can drop some code reviews later on.
>
> That'd be great because we are lacking reviewers (just like other 
> projects) and these are very specific.

Just dropped reviews in all of them. A few comments in a couple of patches
that you can amend before pushing if you think it's worth it (not worth
another spin, in my opinion).

I couldn't help but wonder: can't we just remove the KVM stub (pci-assign)
support from the code base entirely? QEMU dropped it in 2.11. The kernel
dropped it in 4.12 (mid of 2017). Not sure if there is any existing, running
guests relying on pci-assign these days that can't move to vfio-pci.

If there are still users for pci-assign, perhaps a deprecation notice is in
order (a warning when launching the VM?). Then users can't complain
that the support was removed and they were thrown under the bus.


(... I mean, they will still complain about it, but at least we warned 
about it)



Thanks,


DHB





>
> Thanks,
> Michal

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

On 8/16/19 11:25 PM, Daniel Henrique Barboza wrote:
> 
> 
> On 8/16/19 5:59 AM, Michal Privoznik wrote:
>> On 8/14/19 5:14 PM, Daniel Henrique Barboza wrote:
>>> I've run make check with each individual patch, and everything
>>> seems fine in my environment.
>>>
>>> For all patches:
>>>
>>> Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>>>
>>>
>>> I'll see if I can drop some code reviews later on.
>>
>> That'd be great because we are lacking reviewers (just like other
>> projects) and these are very specific.
> 
> Just dropped reviews in all of them. A few comments in a couple of patches
> that you can amend before pushing if you think it's worth it (not worth
> another spin, in my opinion).
> 
> I couldn't help but wonder: can't we just remove the KVM stub (pci-assign)
> support from the code base entirely? QEMU dropped it in 2.11. The kernel
> dropped it in 4.12 (mid of 2017). Not sure if there is any existing,
> running
> guests relying on pci-assign these days that can't move to vfio-pci.
> 
> If there are still users for pci-assign, perhaps a deprecation notice is in
> order (a warning when launching the VM?). Then users can't complain
> that the support was removed and they were thrown under the bus.
> 
> 
> (... I mean, they will still complain about it, but at least we warned
> about it)
> 

That is the direction we will eventually go. I've discussed this couple
of times (and in fact I have patches ready) and it was always agreed
upon. But only yesterday I've merged a patch that fixes a bug in the way
we reset PCI devices for KVM assignment, which sparked a discussion:

  https://www.redhat.com/archives/libvir-list/2019-August/msg00671.html

But as soon as we get green light from guys I will post patches.

Michal

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