krun: request enabling DHCP client

[slp]

Now that libkrun has an embedded DHCP client, if we're using passt, pass the NET_FLAG_DHCP_CLIENT flag to enable it.

[gemini-code-assist]

Code Review

This pull request updates the krun_add_net_unixstream call in src/libcrun/handlers/krun.c to include the NET_FLAG_DHCP_CLIENT flag. The review feedback identifies a missing NULL check for the dlsym lookup which could lead to a crash, suggests using crun_make_error for consistent error propagation instead of immediate exit, and recommends adding a fallback definition for the new flag to maintain compatibility with older build environments.

[packit-as-a-service]

Ephemeral COPR build failed. @containers/packit-build please check.

[slp]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces support for the NET_FLAG_DHCP_CLIENT flag in libkrun networking configuration, adds a null check for the krun_add_net_unixstream symbol, and improves error handling by returning structured errors instead of exiting. The reviewer suggested implementing a fallback mechanism for the krun_add_net_unixstream call to maintain backward compatibility with older libkrun versions that might return -EINVAL when the new flag is used.

[gemini-code-assist]

To maintain backward compatibility with older versions of libkrun that do not support the NET_FLAG_DHCP_CLIENT flag, consider falling back to calling krun_add_net_unixstream without the flag if it returns -EINVAL. This ensures that networking still works (albeit without the embedded DHCP client) on systems with older library versions.

      ret = krun_add_net_unixstream (ctx_id, NULL, kconf->passt_fds[PASST_FD_PARENT], &mac[0], COMPAT_NET_FEATURES, NET_FLAG_DHCP_CLIENT);
      if (UNLIKELY (ret == -EINVAL))
        ret = krun_add_net_unixstream (ctx_id, NULL, kconf->passt_fds[PASST_FD_PARENT], &mac[0], COMPAT_NET_FEATURES, 0);
      if (UNLIKELY (ret < 0))
        return crun_make_error (err, -ret, "could not set krun net configuration");

[slp]

I think it's better to fail, since enabling passt without the embedded DHCP client can be confusing for users.

[giuseppe]

don't we break existing setups if we don't ignore EINVAL though?

[slp]

I've just applied Gemini's suggestion.

[slp]

/gemini review

[gemini-code-assist]

Code Review

This pull request updates the krun_add_net_unixstream call in src/libcrun/handlers/krun.c to pass NET_FLAG_DHCP_CLIENT instead of 0. The reviewer pointed out that if crun is compiled against an older version of libkrun that does not define NET_FLAG_DHCP_CLIENT, the build will fail. They suggested conditionally defining NET_FLAG_DHCP_CLIENT to 0 if it is not already defined to maintain backward compatibility.

[slp]

/gemini review

[gemini-code-assist]

Code Review

This pull request adds support for building with older libkrun headers by defining NET_FLAG_DHCP_CLIENT if it is missing, and updates krun_add_net_unixstream to use this flag. It also adds validation for the krun_add_net_unixstream symbol lookup and changes the error handling to return a structured error instead of exiting. The review feedback points out that if the runtime version of libkrun is older, passing NET_FLAG_DHCP_CLIENT will return -EINVAL. To prevent regressions, a fallback mechanism should be implemented to retry the call with 0 flags when -EINVAL is encountered.

[slp]

/gemini review

[gemini-code-assist]

Code Review

This pull request improves compatibility with older versions of the libkrun library. It defines NET_FLAG_DHCP_CLIENT if it is not already defined, adds a check to ensure krun_add_net_unixstream is successfully resolved via dlsym, and implements a fallback mechanism that retries without the DHCP client flag if the initial call returns -EINVAL. Additionally, it replaces a hard exit with a structured error return. No review comments were provided, so there is no additional feedback.

[giuseppe]

LGTM