|WARNING| [RFC] bus/vmbus: make driver interface internal

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/170863

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-10-08

# DPDK Patch Review

## Summary
This patch makes the vmbus bus API internal by renaming headers and marking functions with `__rte_internal`. The changes are largely mechanical and follow the deprecation notice, but there are a few issues to address.

---

## Errors

**1. Export symbol macro name mismatch with function name**

In `drivers/bus/vmbus/vmbus_channel.c` at line 462, the export macro uses a different name than the function:

```c
RTE_EXPORT_INTERNAL_SYMBOL(rte_vmbus_chan_dump)
void rte_vmbus_chan_dump(FILE *f, const struct vmbus_channel *chan)
```

The function name is `rte_vmbus_chan_dump`, but the macro should match exactly. According to the guidelines, the export macro takes the symbol name as the first parameter. Verify this is correct for your DPDK version's macro definition. If the macro expects just the base name without the `rte_` prefix, this would be wrong. If it expects the full name, this is correct but should be verified against other usages in the file.

Looking at other instances in the same file (lines 68, 101, 119, etc.), they all follow the pattern `RTE_EXPORT_INTERNAL_SYMBOL(rte_vmbus_<func>)` where `<func>` matches the function name after `rte_vmbus_`. This appears consistent. However, ensure the symbol name passed to the macro exactly matches the actual function identifier.

---

## Warnings

**1. Missing `__rte_internal` annotation on `rte_vmbus_chan_dump` function declaration**

In `drivers/bus/vmbus/vmbus_api.h` at line 335, the function declaration has `__rte_internal`:

```c
__rte_internal
void rte_vmbus_chan_dump(FILE *f, const struct vmbus_channel *chan);
```

This is correct for an internal API function. No issue here.

**2. Header guard name change not documented in release notes**

The header guard in `vmbus_api.h` changed from `_VMBUS_H_` to `VMBUS_API_H` (line 6). While this is an internal header now, the release notes should mention that the header is no longer installed, which is already covered. The guard name change is an implementation detail.

**3. Release notes could be more specific about impact**

The release notes entry states:

```
* bus/vmbus: Made the vmbus bus API internal.
  The ``rte_bus_vmbus.h`` and ``rte_vmbus_reg.h`` headers are no longer installed;
  their contents moved to the driver only headers
  ``vmbus_api.h`` and ``vmbus_reg.h``.
  All ``rte_vmbus_*`` functions are now internal to DPDK.
```

This is clear and appropriate. It states the headers are no longer installed and functions are now internal. No change needed.

---

## Info

**1. Consistent naming of internal headers**

The renamed headers (`vmbus_api.h`, `vmbus_reg.h`) drop the `rte_` prefix, which is consistent with them being driver-internal headers. However, `bus_vmbus_driver.h` retains the `bus_` prefix. This inconsistency is pre-existing and not introduced by this patch, so it's acceptable.

**2. All export macros changed from `RTE_EXPORT_SYMBOL` to `RTE_EXPORT_INTERNAL_SYMBOL`**

This is correct for the stated goal of making the API internal. The pattern is applied consistently across all affected functions in:
- `drivers/bus/vmbus/linux/vmbus_bus.c`
- `drivers/bus/vmbus/vmbus_channel.c`
- `drivers/bus/vmbus/vmbus_common.c`

**3. `rte_vmbus_chan_dump` gets export annotation**

The commit message notes:

> rte_vmbus_chan_dump() had no export annotation at all, so it was declared in the public header but never present in the shared library. It is now exported as internal like the rest.

This is correct. The function previously had no `RTE_EXPORT_SYMBOL` in the `.c` file (line 459 in `vmbus_channel.c`), so it was not exported despite being declared in the header. Adding `RTE_EXPORT_INTERNAL_SYMBOL` makes it available to DPDK components.

---

## Verification Checklist

- [x] No forbidden tokens introduced
- [x] API tags (`__rte_internal`) placed correctly (alone on line before return type)
- [x] Export macros added to `.c` files for all public-to-internal functions
- [x] Headers moved from `headers` to `driver_sdk_headers` in `meson.build`
- [x] Include paths updated consistently (`rte_bus_vmbus.h` - `vmbus_api.h`, `rte_vmbus_reg.h` - `vmbus_reg.h`)
- [x] Release notes updated to document the change
- [x] Deprecation notice removed
- [x] No new usage of deprecated APIs
- [x] Code and documentation updated atomically

---

## Conclusion

The patch is well-structured and follows the announced deprecation plan. The mechanical changes are consistent. The main point to verify is that the export macro usage matches your DPDK version's expected format (symbol name with or without prefix). Assuming the macro usage is correct per the DPDK version, this patch is ready.

No correctness bugs identified. The changes are process and API boundary adjustments, not logic changes.