Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nvproxy: Clean up struct field tags. #11316

Merged
merged 1 commit into from
Dec 30, 2024
Merged

Conversation

copybara-service[bot]
Copy link

@copybara-service copybara-service bot commented Dec 21, 2024

nvproxy: Clean up struct field tags.

Before this change, there were 2 places in which the driver struct names were
defined for nvproxy structs:

  1. As struct field tags. The first field of structs had a tag nvproxy:*. This
    was kind of awkward. Such metadata is usually a struct comment.
  2. In version.go while registering the struct with a name. Not all structs are
    defined in nvproxy (for example simple structs). In such cases, the driver
    struct name is directly assigned while registering struct info.

This change gets rid of (1). Most of the struct tags were nvproxy:"same". Now
driverStructs() always infers the driver struct name using the nvproxy struct
name itself. The few cases where the nvproxy tag was needed, because driver
struct name was lower cased, were handled by defining driverStructWithName()
which allows specifying a different name. Now all driver struct names
definitions are in one place.

Along the way, also made the following fixes:

  • For some reason, many structs defined in pkg/abi/nvgpu/frontend.go had
    camel-cased naming, while all other structs in pkg/abi/nvgpu/ctrl.go and
    pkg/abi/nvgpu/classes.go were named the same as their driver structs. The
    convention in the abi/* packages is to follow the kernel source naming.
    This is against Go sytle guide, but is more readable for gVisor purposes and
    has been a long accepted convention. This also makes the task of removing (1)
    easier. So renamed all such structs as per their driver names.
  • A lot of code in pkg/sentry/devices/nvproxy/version.go was still referring to
    driver struct info as "struct names", even though it was representing more
    than just struct names. It also contains the reflect.Type of the struct which
    is used to compare the nvproxy struct layout to the driver struct layout.

@copybara-service copybara-service bot added the exported Issue was exported automatically label Dec 21, 2024
@copybara-service copybara-service bot force-pushed the test/cl708532738 branch 2 times, most recently from 3baec23 to a84bd9f Compare December 23, 2024 22:17
@copybara-service copybara-service bot force-pushed the test/cl708532738 branch 2 times, most recently from 5cef916 to ee98572 Compare December 30, 2024 09:00
Before this change, there were 2 places in which the driver struct names were
defined for nvproxy structs:
1. As struct field tags. The first field of structs had a tag `nvproxy:*`. This
   was kind of awkward. Such metadata is usually a struct comment.
2. In version.go while registering the struct with a name. Not all structs are
   defined in nvproxy (for example simple structs). In such cases, the driver
   struct name is directly assigned while registering struct info.

This change gets rid of (1). Most of the struct tags were `nvproxy:"same"`. Now
driverStructs() always infers the driver struct name using the nvproxy struct
name itself. The few cases where the nvproxy tag was needed, because driver
struct name was lower cased, were handled by defining driverStructWithName()
which allows specifying a different name. Now all driver struct names
definitions are in one place.

Along the way, also made the following fixes:
- For some reason, many structs defined in pkg/abi/nvgpu/frontend.go had
  camel-cased naming, while all other structs in pkg/abi/nvgpu/ctrl.go and
  pkg/abi/nvgpu/classes.go were named the same as their driver structs. The
  convention in the abi/* packages is to follow the kernel source naming.
  This is against Go sytle guide, but is more readable for gVisor purposes and
  has been a long accepted convention. This also makes the task of removing (1)
  easier. So renamed all such structs as per their driver names.
- A lot of code in pkg/sentry/devices/nvproxy/version.go was still referring to
  driver struct info as "struct names", even though it was representing more
  than just struct names. It also contains the reflect.Type of the struct which
  is used to compare the nvproxy struct layout to the driver struct layout.

PiperOrigin-RevId: 710648105
@copybara-service copybara-service bot merged commit b11efea into master Dec 30, 2024
0 of 2 checks passed
@copybara-service copybara-service bot deleted the test/cl708532738 branch December 30, 2024 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exported Issue was exported automatically
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant