[Malar2703]

No description provided.

[Bischoff]

Eric

Thanks a lot for this PR!

For the next ones, would it be possible to have a PR description? I only had the PR's title, and for the rest I had to guess your intentions.

I also notice no unit tests, no documentation.

Claude, prompted by Eric

I reviewed the current head of the branch (cfbfdb6). Configuring the IP address of a Network Express interface is a useful addition. However, I found a regression and several bugs, so I think the PR needs more work.

tox -e pep8 passes, and the existing unit tests show no new failure, but only because none of them covers the changed paths.

Bugs

1. Regression on FCP

In smtLayer/getVM.py, posOpsList = {...}, ends with a comma, so it becomes a tuple instead of a dict.

Positional operands of all GETVM subfunctions are then ignored, including the existing FCPINFO used by SMTClient.get_fcp_info_by_status(): parsing getvm TESTVM fcpinfo free gives parms = {} instead of {'status': 'free'}.

No unit test covers this path, which explains the issue went undetected.

2. Configuration files removed too broadly

NetworkOPS._create_neth_configuration_files() generates rm -f /etc/sysconfig/network-scripts/ifcfg-{intf_name[:2]}*. For enc1600, this
removes every ifcfg-en* file, including the configuration of the guest's regular NIC.

3. DHCP and optional fields

_create_neth_configuration_files() reads net['cidr'], net['gateway_addr'] and net['ip_addr'] unconditionally, so method dhcp (accepted by the validation) or a network without gateway raises KeyError or concatenates None.

4. Only one network

The loop overwrites cfg_str, so only the last entry of guest_networks is written.

5. pcif parameter

The REST schema does not list pcif as required, but VMHandler.configure_network_interface_ip() reads interface['pcif'], which gives a 500 error when it is missing.

6. PCIF matching

getVM.getNethIntInfo() looks for pcif[-4:] as a substring of the PCI address, which can match the wrong device. Comparing the domain field exactly would be safer.

Design questions

1. The generated file is /etc/sysconfig/network-scripts/ifcfg-*, which only works for RHEL up to 8, not for RHEL 9 and later (NetworkManager), SLES (/etc/sysconfig/network/) or Ubuntu (netplan), while os_version is required but not used.

Which distributions are meant to be supported?

2. zdirect_network_configuration() always reboots the guest, even with active=True. Is this intended? It should at least be documented.

3. The input validation in SDKAPI.guest_configure_network_interface_ip() is a copy/paste of guest_create_network_interface(), including the error messages that still mention guest_create_network_interface. I suggest adapting it. Optionally, factor it into a common helper.

Tests and documentation

1. There are no unit tests for the new API, the network configuration generation and getNethIntInfo().

2. Documentation: nothing in doc/source/restapi.rst, parameters.yaml or zvmsdk/tests/fvt/api_templates/ for POST /guests/{userid}/interface/ip. It would be nice to also document the prerequisites (guest running and reachable through IUCV, PCIF attached), the supported distributions, and the reboot.

3. PR description

That would help reviewers too.