The zPCI address validation or autogeneration does not work as
expected in the following scenarios
1. uid = 0 and fid = 0
2. uid = 0 and fid > 0
3. uid = 0 and fid not specified
4. uid not specified and fid > 0
5. 2 zPCI devices with uid > 0 and fid not specified.

This is because of the following reasons
1. If uid = 0 or fid = 0 the code assumes that user has not specified
   the corresponding address
2. If either uid or fid is provided, the code assumes that both uid
   and fid addresses are specified by the user.

This patch fixes these issues.
---
v2:
  - Call same function to reserve zPCI address when zPCI address is
    fully/partially/not specified by the user.
  - Redefine structure of zPCI address.
  - Add tests to verify the auto-generated xml(qemuxml2xmltest.c).
  - Separate/merge patches.
  - Minor changes based on review feedback.

v1:
  https://www.redhat.com/archives/libvir-list/2020-April/msg00479.html

Shalini Chellathurai Saroja (5):
  conf: use g_autofree to ensure automatic cleanup
  conf: fix zPCI address auto-generation on s390
  qemu: move ZPCI uid validation into device validation
  tests: qemu: add more tests for ZPCI on S390
  tests: add test with PCI and CCW device

 src/conf/device_conf.c                        | 45 +++++------
 src/conf/domain_addr.c                        | 77 ++++++-------------
 src/conf/domain_conf.c                        | 10 ++-
 src/libvirt_private.syms                      |  4 +-
 src/qemu/qemu_command.c                       |  6 +-
 src/qemu/qemu_hotplug.c                       |  2 +-
 src/qemu/qemu_validate.c                      | 18 ++++-
 src/util/virpci.c                             | 23 ++----
 src/util/virpci.h                             | 15 +++-
 .../hostdev-vfio-zpci-autogenerate-fids.args  | 31 ++++++++
 .../hostdev-vfio-zpci-autogenerate-fids.xml   | 29 +++++++
 .../hostdev-vfio-zpci-autogenerate-uids.args  | 31 ++++++++
 .../hostdev-vfio-zpci-autogenerate-uids.xml   | 29 +++++++
 .../hostdev-vfio-zpci-ccw-memballoon.args     | 28 +++++++
 .../hostdev-vfio-zpci-ccw-memballoon.xml      | 17 ++++
 .../hostdev-vfio-zpci-duplicate.xml           | 30 ++++++++
 ...ostdev-vfio-zpci-invalid-uid-valid-fid.xml | 21 +++++
 .../hostdev-vfio-zpci-set-zero.xml            | 21 +++++
 .../hostdev-vfio-zpci-uid-set-zero.xml        | 20 +++++
 tests/qemuxml2argvtest.c                      | 25 ++++++
 .../hostdev-vfio-zpci-autogenerate-fids.xml   | 43 +++++++++++
 .../hostdev-vfio-zpci-autogenerate-uids.xml   | 43 +++++++++++
 .../hostdev-vfio-zpci-ccw-memballoon.xml      | 32 ++++++++
 tests/qemuxml2xmltest.c                       | 10 +++
 24 files changed, 498 insertions(+), 112 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-autogenerate-fids.args
 create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-autogenerate-fids.xml
 create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-autogenerate-uids.args
 create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-autogenerate-uids.xml
 create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-ccw-memballoon.args
 create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-ccw-memballoon.xml
 create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-duplicate.xml
 create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-invalid-uid-valid-fid.xml
 create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-set-zero.xml
 create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-uid-set-zero.xml
 create mode 100644 tests/qemuxml2xmloutdata/hostdev-vfio-zpci-autogenerate-fids.xml
 create mode 100644 tests/qemuxml2xmloutdata/hostdev-vfio-zpci-autogenerate-uids.xml
 create mode 100644 tests/qemuxml2xmloutdata/hostdev-vfio-zpci-ccw-memballoon.xml

-- 
2.25.4

Ping, in case you missed it.

On 6/18/20 10:25 AM, Shalini Chellathurai Saroja wrote:
> The zPCI address validation or autogeneration does not work as
> expected in the following scenarios
> 1. uid = 0 and fid = 0
> 2. uid = 0 and fid > 0
> 3. uid = 0 and fid not specified
> 4. uid not specified and fid > 0
> 5. 2 zPCI devices with uid > 0 and fid not specified.
>
> This is because of the following reasons
> 1. If uid = 0 or fid = 0 the code assumes that user has not specified
>     the corresponding address
> 2. If either uid or fid is provided, the code assumes that both uid
>     and fid addresses are specified by the user.
>
> This patch fixes these issues.
> ---
> v2:
>    - Call same function to reserve zPCI address when zPCI address is
>      fully/partially/not specified by the user.
>    - Redefine structure of zPCI address.
>    - Add tests to verify the auto-generated xml(qemuxml2xmltest.c).
>    - Separate/merge patches.
>    - Minor changes based on review feedback.
>
> v1:
>    https://www.redhat.com/archives/libvir-list/2020-April/msg00479.html
>
> Shalini Chellathurai Saroja (5):
>    conf: use g_autofree to ensure automatic cleanup
>    conf: fix zPCI address auto-generation on s390
>    qemu: move ZPCI uid validation into device validation
>    tests: qemu: add more tests for ZPCI on S390
>    tests: add test with PCI and CCW device
>
>   src/conf/device_conf.c                        | 45 +++++------
>   src/conf/domain_addr.c                        | 77 ++++++-------------
>   src/conf/domain_conf.c                        | 10 ++-
>   src/libvirt_private.syms                      |  4 +-
>   src/qemu/qemu_command.c                       |  6 +-
>   src/qemu/qemu_hotplug.c                       |  2 +-
>   src/qemu/qemu_validate.c                      | 18 ++++-
>   src/util/virpci.c                             | 23 ++----
>   src/util/virpci.h                             | 15 +++-
>   .../hostdev-vfio-zpci-autogenerate-fids.args  | 31 ++++++++
>   .../hostdev-vfio-zpci-autogenerate-fids.xml   | 29 +++++++
>   .../hostdev-vfio-zpci-autogenerate-uids.args  | 31 ++++++++
>   .../hostdev-vfio-zpci-autogenerate-uids.xml   | 29 +++++++
>   .../hostdev-vfio-zpci-ccw-memballoon.args     | 28 +++++++
>   .../hostdev-vfio-zpci-ccw-memballoon.xml      | 17 ++++
>   .../hostdev-vfio-zpci-duplicate.xml           | 30 ++++++++
>   ...ostdev-vfio-zpci-invalid-uid-valid-fid.xml | 21 +++++
>   .../hostdev-vfio-zpci-set-zero.xml            | 21 +++++
>   .../hostdev-vfio-zpci-uid-set-zero.xml        | 20 +++++
>   tests/qemuxml2argvtest.c                      | 25 ++++++
>   .../hostdev-vfio-zpci-autogenerate-fids.xml   | 43 +++++++++++
>   .../hostdev-vfio-zpci-autogenerate-uids.xml   | 43 +++++++++++
>   .../hostdev-vfio-zpci-ccw-memballoon.xml      | 32 ++++++++
>   tests/qemuxml2xmltest.c                       | 10 +++
>   24 files changed, 498 insertions(+), 112 deletions(-)
>   create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-autogenerate-fids.args
>   create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-autogenerate-fids.xml
>   create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-autogenerate-uids.args
>   create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-autogenerate-uids.xml
>   create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-ccw-memballoon.args
>   create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-ccw-memballoon.xml
>   create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-duplicate.xml
>   create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-invalid-uid-valid-fid.xml
>   create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-set-zero.xml
>   create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-uid-set-zero.xml
>   create mode 100644 tests/qemuxml2xmloutdata/hostdev-vfio-zpci-autogenerate-fids.xml
>   create mode 100644 tests/qemuxml2xmloutdata/hostdev-vfio-zpci-autogenerate-uids.xml
>   create mode 100644 tests/qemuxml2xmloutdata/hostdev-vfio-zpci-ccw-memballoon.xml
>

On Wed, 2020-06-24 at 18:59 +0200, Shalini Chellathurai Saroja wrote:
> Ping, in case you missed it.

Please wait at least a week or so before pinging a series, and
refrain from CCing individual developers when you do - we're all
subscribed to libvir-list.

Anyway I've seen the series and I'm planning to review it, I just
haven't gotten around to it yet :)

-- 
Andrea Bolognani / Red Hat / Virtualization

On Thu, 2020-06-18 at 10:25 +0200, Shalini Chellathurai Saroja wrote:
> Shalini Chellathurai Saroja (5):
>   conf: use g_autofree to ensure automatic cleanup
>   conf: fix zPCI address auto-generation on s390
>   qemu: move ZPCI uid validation into device validation
>   tests: qemu: add more tests for ZPCI on S390
>   tests: add test with PCI and CCW device

Aside from the comments in patch 2/5, everything looks good to me.

The series does, however, not update the release notes (NEWS.rst):
can you please post a 6/5 patch that takes care of that, assuming we
decide not to go with a respin?

Thanks!

-- 
Andrea Bolognani / Red Hat / Virtualization

On 6/25/20 8:01 PM, Andrea Bolognani wrote:
> On Thu, 2020-06-18 at 10:25 +0200, Shalini Chellathurai Saroja wrote:
>> Shalini Chellathurai Saroja (5):
>>    conf: use g_autofree to ensure automatic cleanup
>>    conf: fix zPCI address auto-generation on s390
>>    qemu: move ZPCI uid validation into device validation
>>    tests: qemu: add more tests for ZPCI on S390
>>    tests: add test with PCI and CCW device
> Aside from the comments in patch 2/5, everything looks good to me.
>
> The series does, however, not update the release notes (NEWS.rst):
> can you please post a 6/5 patch that takes care of that, assuming we
> decide not to go with a respin?

Hi Andrea,

Thank you :-). I have attached the patch to update the release notes 
with this email.

>
> Thanks!
>

On Fri, 2020-06-26 at 14:13 +0200, Shalini Chellathurai Saroja wrote:
> On 6/25/20 8:01 PM, Andrea Bolognani wrote:
> > Aside from the comments in patch 2/5, everything looks good to me.
> > 
> > The series does, however, not update the release notes (NEWS.rst):
> > can you please post a 6/5 patch that takes care of that, assuming we
> > decide not to go with a respin?
> 
> Hi Andrea,
> 
> Thank you :-). I have attached the patch to update the release notes 
> with this email.

Thanks. I tweaked the commit message a bit, and I also had to squash
the diff below into patch 2 to make CentOS 7 and macOS happy. The
series is now pushed. Have a nice weekend :)


diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
index 21398e90f2..64a713a5f9 100644
--- a/src/conf/device_conf.c
+++ b/src/conf/device_conf.c
@@ -52,7 +52,7 @@ static int
 virZPCIDeviceAddressParseXML(xmlNodePtr node,
                              virPCIDeviceAddressPtr addr)
 {
-    virZPCIDeviceAddress def = { 0 };
+    virZPCIDeviceAddress def = { .uid = { 0 }, .fid = { 0 } };
     g_autofree char *uid = NULL;
     g_autofree char *fid = NULL;

-- 
Andrea Bolognani / Red Hat / Virtualization