Skip to content

Comments

tao.yang/zstack-ZSV-3564@@2#3

Closed
MatheMatrix wants to merge 1 commit intosync/feature-zsv_4.1.0_ui_performance_newfrom
tao.yang/zstack-ZSV-3564@@2
Closed

tao.yang/zstack-ZSV-3564@@2#3
MatheMatrix wants to merge 1 commit intosync/feature-zsv_4.1.0_ui_performance_newfrom
tao.yang/zstack-ZSV-3564@@2

Conversation

@MatheMatrix
Copy link
Owner

@MatheMatrix MatheMatrix commented Dec 6, 2023

sync/feature-zsv_4.1.0_ui_performance_new

Summary by CodeRabbit

  • 新功能

    • 移除了对特定虚拟交换机类型的支持,改为更灵活的网络实现扩展点。
    • 增加了对不同虚拟交换机和提供者类型的支持,以适应多样化的网络配置需求。
  • 错误修复

    • 修正了网络实现逻辑中的错误,确保网络在各种场景下都能正确实现。
  • 文档

    • 更新了相关文档,以反映对虚拟交换机类型支持的变更。
  • 重构

    • 对网络实现的扩展点进行了重构,以提高代码的可维护性和扩展性。
  • 样式

    • 调整了代码格式,提高了代码的可读性。
  • 测试

    • 增加了新的测试用例,以验证网络实现逻辑的正确性。
  • 杂务

    • 清理了不再使用的代码和资源,优化了代码库的整体结构。
  • 回退

    • 在必要时回退了一些代码更改,以确保功能的稳定性。

Resolves: ZSV-3564

Change-Id: I64796f6a707778747a6272656573786975717967
@coderabbitai
Copy link

coderabbitai bot commented Dec 6, 2023

Walkthrough:
代码库中的网络层(L2)实现方式发生了变化,移除了对虚拟交换机类型(VSwitchType)的支持,并改进了网络实现扩展点的参数和逻辑。这些更改涉及了多个文件,包括接口定义、实现类和相关的网络处理逻辑。此外,还对KVM相关的网络后端进行了调整,以适应这些变化。

Changes:

文件路径 变更摘要
header/.../L2NetworkRealizationExtensionPoint.java 移除了getSupportedVSwitchType方法
network/.../L2NetworkApiInterceptor.java 添加了新的条件逻辑块处理msg.getL2ProviderType()为null的情况
network/.../L2NetworkManager.java 修改了getRealizationExtension方法签名,适应不同的参数组合
network/.../L2NetworkManagerImpl.java 调整了realizationExtsl2ProviderMap的数据结构
network/.../L2NoVlanNetwork.java 更新了handle方法,使用Q类和Tuple替代SimpleQuery
plugin/kvm/.../AbstractKVMConnectExtensionForL2Network.java 引入了数据库查询,使用Q
plugin/kvm/.../KVMConstant.java 添加了新的L2提供者类型常量,移除了L2_PROVIDER_TYPE_LINUX_BRIDGE
plugin/kvm/.../KVMRealizeL2NoVlanNetworkBackend.java 移除了getSupportedVSwitchType方法
plugin/kvm/.../KVMRealizeL2VlanNetworkBackend.java 移除了getSupportedVSwitchType方法,修改了completeNicInformation方法
plugin/sdnController/.../HardwareVxlanNetwork.java 移除了realizeNetwork方法中的VSwitchType声明和使用
plugin/sdnController/.../KVMRealizeHardwareVxlanNetworkBackend.java 移除了getSupportedVSwitchType方法,添加了新的方法和导入
plugin/sdnController/.../KVMRealizeHardwareVxlanPoolNetworkBackend.java 移除了getSupportedVSwitchType方法,修改了getL2NetworkTypeVmNicOn方法
plugin/vxlan/.../VxlanNetwork.java 移除了realizeNetwork方法中的vSwitchType依赖
plugin/vxlan/.../KVMRealizeL2VxlanNetworkBackend.java 移除了getSupportedVSwitchType方法,修改了makeBridgeName方法
plugin/vxlan/.../KVMRealizeL2VxlanNetworkPoolBackend.java 移除了getSupportedVSwitchType方法,添加了getL2NetworkTypeVmNicOn方法
plugin/vxlan/.../VxlanNetworkPool.java 移除了realizeNetwork方法中的vSwitchType依赖
simulator/.../L2NoVlanNetworkRealizer.java 移除了getSupportedVSwitchType方法
simulator/.../L2VlanNetworkRealizer.java 移除了getSupportedVSwitchType方法,更新了realize方法

Poem:

🐰🎉

代码如织,变幻莫测,

虚拟交换,静悄悄退场。

兔子跳跃,庆祝新章,

网络流转,更加畅快。


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • If you reply to a review comment from CodeRabbit, the bot will automatically respond.
  • To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
  • Note: Review comments are made on code diffs or files, not on the PR overview.
  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Note: For conversation with the bot, please use the review comments on code diffs or files.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between e3dc5a3 and 8ed722f.
Files selected for processing (18)
  • header/src/main/java/org/zstack/header/network/l2/L2NetworkRealizationExtensionPoint.java (1 hunks)
  • network/src/main/java/org/zstack/network/l2/L2NetworkApiInterceptor.java (1 hunks)
  • network/src/main/java/org/zstack/network/l2/L2NetworkManager.java (1 hunks)
  • network/src/main/java/org/zstack/network/l2/L2NetworkManagerImpl.java (3 hunks)
  • network/src/main/java/org/zstack/network/l2/L2NoVlanNetwork.java (14 hunks)
  • plugin/kvm/src/main/java/org/zstack/kvm/AbstractKVMConnectExtensionForL2Network.java (6 hunks)
  • plugin/kvm/src/main/java/org/zstack/kvm/KVMConstant.java (1 hunks)
  • plugin/kvm/src/main/java/org/zstack/kvm/KVMRealizeL2NoVlanNetworkBackend.java (4 hunks)
  • plugin/kvm/src/main/java/org/zstack/kvm/KVMRealizeL2VlanNetworkBackend.java (3 hunks)
  • plugin/sdnController/src/main/java/org/zstack/sdnController/hardwareVxlan/HardwareVxlanNetwork.java (1 hunks)
  • plugin/sdnController/src/main/java/org/zstack/sdnController/hardwareVxlan/KVMRealizeHardwareVxlanNetworkBackend.java (5 hunks)
  • plugin/sdnController/src/main/java/org/zstack/sdnController/hardwareVxlan/KVMRealizeHardwareVxlanPoolNetworkBackend.java (1 hunks)
  • plugin/vxlan/src/main/java/org/zstack/network/l2/vxlan/vxlanNetwork/VxlanNetwork.java (1 hunks)
  • plugin/vxlan/src/main/java/org/zstack/network/l2/vxlan/vxlanNetworkPool/KVMRealizeL2VxlanNetworkBackend.java (5 hunks)
  • plugin/vxlan/src/main/java/org/zstack/network/l2/vxlan/vxlanNetworkPool/KVMRealizeL2VxlanNetworkPoolBackend.java (5 hunks)
  • plugin/vxlan/src/main/java/org/zstack/network/l2/vxlan/vxlanNetworkPool/VxlanNetworkPool.java (1 hunks)
  • simulator/simulatorImpl/src/main/java/org/zstack/simulator/L2NoVlanNetworkRealizer.java (2 hunks)
  • simulator/simulatorImpl/src/main/java/org/zstack/simulator/L2VlanNetworkRealizer.java (2 hunks)
Additional comments: 49
header/src/main/java/org/zstack/header/network/l2/L2NetworkRealizationExtensionPoint.java (1)
  • 16-24: 确认getSupportedVSwitchType方法已从L2NetworkRealizationExtensionPoint接口中移除,且相关的代码调整正确无误。
network/src/main/java/org/zstack/network/l2/L2NetworkApiInterceptor.java (1)
  • 81-84: 代码逻辑更改看起来是合理的,它处理了msg.getL2ProviderType()为空且l2.getvSwitchType()不等于L2NetworkConstant.VSWITCH_TYPE_LINUX_BRIDGE的情况。请确保这个逻辑更改与预期的行为一致,并且已经通过了相关的单元测试。
network/src/main/java/org/zstack/network/l2/L2NetworkManager.java (1)
  • 6-15: 代码段显示了L2NetworkManager接口的方法声明已经根据不同的网络和提供者类型进行了修改。这与PR目标和AI生成的摘要相符,表明了对L2网络实现扩展获取逻辑的重定义。请确保所有依赖于这些方法的代码都已更新以适应新的签名。
network/src/main/java/org/zstack/network/l2/L2NetworkManagerImpl.java (3)
  • 56-60: The refactoring of the realizationExts and l2ProviderMap data structures aligns with the overall changes described in the PR summary. Ensure that all dependent code has been updated to work with the new map structures.

  • 501-538: The overloading of the getRealizationExtension method to support different parameter types is consistent with the changes to the data structures. Verify that the logic within these methods correctly handles the new data structures and that all calls to these methods have been updated accordingly.

  • 567-583: The populateExtensions method has been updated to populate the new realizationExts and l2ProviderMap data structures. Ensure that the logic here correctly populates the maps based on the extensions available from the PluginRegistry and that the rest of the system is compatible with these changes.

network/src/main/java/org/zstack/network/l2/L2NoVlanNetwork.java (7)
  • 10-26: 新增了多个导入语句,包括DatabaseFacade, Q, SQL, SQLBatchTuple,这些变化符合代码重构的常规做法。

  • 42-50: 新增了静态方法导入Platform.argerrPlatform.err,这些变化符合代码重构的常规做法。

  • 290-296: 在handle(final PrepareL2NetworkOnHostMsg msg)方法中,使用Q类替换了SimpleQuery类来执行数据库查询。请确保新的查询机制使用正确。

  • 310-326: 在handle(final CheckL2NetworkOnHostMsg msg)方法中,使用TupleQ类进行数据库查询。请确保新的查询机制使用正确。

  • 419-425: 在realizeNetwork方法中,新增了一个参数providerType。请确保这个方法的签名和它的使用在整个代码库中保持一致。

  • 685-695: 在attachL2NetworkToCluster方法中,修改了SQL查询。请验证新的SQL查询语法正确且逻辑合理。

  • 793-800: 在deleteL2Bridge方法中,新增了一个参数clusterUuids。请确保这个方法的签名和它的使用在整个代码库中保持一致。

plugin/kvm/src/main/java/org/zstack/kvm/AbstractKVMConnectExtensionForL2Network.java (6)
  • 6-12: 代码中新增了对org.zstack.core.db.Q的导入,这与PR目标和AI生成的摘要中提到的数据库查询调整一致。

  • 22-28: 新增的TupleTypedQuery的导入语句,以及相关的Java集合类,表明了代码中对数据库查询和数据结构的调整。

  • 80-93: 在run方法中,使用Q类进行数据库查询并填充l2ProviderMap映射,这与AI生成的摘要中提到的逻辑添加和控制流修改一致。

  • 102-108: 在run方法中,对l2NetworksCheckList进行去重和排序处理,这是对控制流的一个优化,以确保网络物理接口的检查不会重复进行。

  • 158-165: 在realize_no_vlan流程中,使用l2ProviderMap来获取L2网络的提供者类型,并传递给L2NetworkRealizationExtensionPoint的实现,这与AI生成的摘要中提到的控制流修改一致。

  • 193-200: 在realize_vlan流程中,同样使用l2ProviderMap来获取L2网络的提供者类型,并传递给L2NetworkRealizationExtensionPoint的实现,这与AI生成的摘要中提到的控制流修改一致。

plugin/kvm/src/main/java/org/zstack/kvm/KVMConstant.java (2)
  • 139-139: 在代码审查中发现了一个不一致的地方:用户提供的摘要中提到 L2_PROVIDER_TYPE_LINUX_BRIDGE 常量被移除了,但在代码块中该常量仍然存在。请确认这是否是预期的更改,或者是否需要更新摘要以反映实际代码的状态。

  • 140-141: 新增的常量 L2_PROVIDER_TYPE_OVS_DPDKL2_PROVIDER_TYPE_MACVLAN 与用户提供的摘要相符,表明支持的 L2 提供者类型有所变化。这些更改看起来是合理的。

plugin/kvm/src/main/java/org/zstack/kvm/KVMRealizeL2NoVlanNetworkBackend.java (6)
  • 288-294: 在代码汇总中提到了getSyncSignature方法进行了重新格式化,但在代码块中看到这个方法是新增的,而不仅仅是格式上的变化。这是代码汇总和代码块之间的不一致之处。

  • 283-294: 代码汇总中没有提到对deleteOvsBridge方法的任何更改,但代码块显示了这个方法内部的重要逻辑更改。这是代码汇总中的疏忽。

  • 17-23: 代码汇总中没有提到对realize方法的任何更改,但代码块显示了这个方法的更改。这是代码汇总中的疏忽。

  • 17-23: 代码汇总中没有提到对check方法的任何更改,但代码块显示了这个方法的更改。这是代码汇总中的疏忽。

  • 17-23: 代码汇总中没有提到对delete方法的任何更改,但代码块显示了这个方法的更改。这是代码汇总中的疏忽。

  • 169-170: 代码汇总中没有提到对completeNicInformation方法的任何更改,但代码块显示了这个方法的更改。这是代码汇总中的疏忽。

plugin/kvm/src/main/java/org/zstack/kvm/KVMRealizeL2VlanNetworkBackend.java (3)
  • 14-17: 采用通配符导入可能会导入不必要的类,但如果项目的代码风格指南允许,这种做法是可以接受的。请确认这符合项目的代码风格指南。

  • 174-176: 移除了getSupportedVSwitchType方法,并将其逻辑合并到getL2NetworkTypeVmNicOn中。请验证这一变更是否影响了现有功能或预期行为。


Verification result:

移除getSupportedVSwitchType方法并更新getL2NetworkTypeVmNicOn方法后,已确认代码库中不存在对已移除方法的调用。

  • 180-189: completeNicInformation方法现在直接使用KVMAgentCommands.NicTO,而不是带包名的全名。这与导入的更改一致,没有引入任何问题。
plugin/sdnController/src/main/java/org/zstack/sdnController/hardwareVxlan/HardwareVxlanNetwork.java (1)
  • 60-66: 在移除VSwitchType的使用后,请确保相关的网络实现逻辑已经适当更新,并且这个改变已经在整个代码库中一致地实施了。此外,如果这个方法是公开导出的,还需要检查是否有任何外部依赖可能会受到影响。
plugin/sdnController/src/main/java/org/zstack/sdnController/hardwareVxlan/KVMRealizeHardwareVxlanNetworkBackend.java (2)
  • 15-15: 在摘要中提到 L2NetworkRealizationExtensionPoint 的导入已被移除,但在代码块中该导入仍然存在。请确认这是否是预期的更改。

  • 175-176: 摘要中提到 getL2NetworkTypeVmNicOn 方法从一个重写的方法变成了一个新方法,但在代码块中它仍然被标记为重写的方法。请验证这是否是预期的更改。

plugin/sdnController/src/main/java/org/zstack/sdnController/hardwareVxlan/KVMRealizeHardwareVxlanPoolNetworkBackend.java (2)
  • 91-97: 在KVMRealizeHardwareVxlanPoolNetworkBackend类中,getL2NetworkTypeVmNicOn方法的实现没有变化,与AI生成的摘要中提到的“修改逻辑以返回不同的值”不符。请确认这是否是预期的行为,或者是否有其他相关的变化未在这个代码块中显示。

  • 91-101: 代码审查没有发现其他问题。请确保移除getSupportedVSwitchType方法和修改getL2NetworkTypeVmNicOn方法的逻辑是符合项目需求的,并且这些变化不会影响到其他依赖这些方法的代码。

plugin/vxlan/src/main/java/org/zstack/network/l2/vxlan/vxlanNetwork/VxlanNetwork.java (1)
  • 220-226: 在 realizeNetwork 方法中移除了 vSwitchType 的依赖,这是一个重要的逻辑变化。请确保这一变化与整个系统的设计和其他部分的实现保持一致,并且不会影响现有的功能。
plugin/vxlan/src/main/java/org/zstack/network/l2/vxlan/vxlanNetworkPool/KVMRealizeL2VxlanNetworkBackend.java (5)
  • 1-5: 代码中的导入语句与摘要中描述的一致,移除了多个特定的导入语句,并添加了一个通配符导入语句。

  • 26-32: 摘要中提到的getSupportedVSwitchType方法已经从类中移除,这与代码中的变化相符。

  • 62-64: makeBridgeName方法的逻辑已经被修改,现在在变量vxlan前面包含了一个空格,这与摘要中的描述相符。

  • 107-114: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [66-114]

realize方法的逻辑已经被修改,包括在方法调用中变量名周围添加了空格。这些修改与摘要中的描述相符。

  • 354-359: 摘要中没有提到getSupportedHypervisorTypegetL2NetworkTypeVmNicOn方法的变化,但是这些方法在代码中存在且没有变化。
plugin/vxlan/src/main/java/org/zstack/network/l2/vxlan/vxlanNetworkPool/KVMRealizeL2VxlanNetworkPoolBackend.java (4)
  • 16-25: 新增的导入语句替换了原有的通配符导入,这有助于提高代码的清晰度和避免潜在的命名冲突。这是一个改进的做法。

  • 32-41: 将java.util.*的通配符导入替换为具体类的导入,这有助于减少编译时间和避免不必要的命名空间污染。

  • 206-217: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [206-237]

run方法中的SQL查询被重构,现在使用了SQL.NewQ.New来构建查询。这种方式更加清晰,并且易于理解和维护。但是,建议验证这些更改是否对查询性能有影响,特别是在大数据集上。

  • 311-316: getSupportedHypervisorType方法被重写以返回HypervisorType.KVM,这与PR的目标一致,即专注于KVM虚拟化类型。同时,添加了getL2NetworkTypeVmNicOn方法,这可能是为了提供更多的网络类型信息。需要确保这些更改与整体架构和预期的使用方式保持一致。
plugin/vxlan/src/main/java/org/zstack/network/l2/vxlan/vxlanNetworkPool/VxlanNetworkPool.java (1)
  • 1092-1098: The changes to the realizeNetwork method appear to align with the PR's objective of refactoring the handling of network realization and removing the dependency on vSwitchType. Ensure that all necessary updates to the method's logic have been made to accommodate the removal of vSwitchType.
simulator/simulatorImpl/src/main/java/org/zstack/simulator/L2NoVlanNetworkRealizer.java (1)
  • 34-35: 代码中添加了 getSupportedHypervisorType 方法的实现,但AI生成的摘要中没有提到这一点。请确认这一变化是否符合预期,并注意任何可能的影响。
simulator/simulatorImpl/src/main/java/org/zstack/simulator/L2VlanNetworkRealizer.java (2)
  • 18-20: 根据提供的代码块,realize方法现在包含了一个数据库查询操作。请确保这个查询是必要的,并且已经过优化,以避免潜在的性能问题。

  • 37-37: 确认getSupportedHypervisorType方法返回的HypervisorType是正确的,并且与系统中支持的虚拟化类型一致。

@MatheMatrix MatheMatrix closed this Dec 7, 2023
MatheMatrix pushed a commit that referenced this pull request Mar 7, 2024
commit #3 config sync framework

DBImpact

Resolves: ZSTAC-63265

Change-Id: I667762726f616e66726e64787771776f6a6b636c
MatheMatrix pushed a commit that referenced this pull request Mar 7, 2024
commit #3 config sync framework

DBImpact

Resolves: ZSTAC-63265

Change-Id: I667762726f616e66726e64787771776f6a6b636c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant