wido opened a new pull request, #14236:
URL: https://github.com/apache/cloudstack/pull/14236

   ### Description
   
   This PR removes code from the KVM agent plugin that nothing calls any more. 
It is the first of three cleanup PRs; the other two build on this one or are 
independent of it.
   
   **Command wrappers for commands the management server no longer sends** (13 
wrappers plus the shared console proxy base class). These belong to the pre-4.2 
storage model. Nothing in the management server creates or subclasses these 
commands, so the wrappers could never be reached:
   
   AttachIsoCommand, BackupSnapshotCommand, CheckConsoleProxyLoadCommand, 
WatchConsoleProxyLoadCommand, CheckStorageAvailabilityCommand, CreateCommand, 
CreatePrivateTemplateFromSnapshotCommand, 
CreatePrivateTemplateFromVolumeCommand, CreateVolumeFromSnapshotCommand, 
ManageSnapshotCommand, PrimaryStorageDownloadCommand, RebootRouterCommand, 
UpgradeSnapshotCommand.
   
   With them go the things only they used: the `createtmplt.sh` and 
`createvm.sh` lookups in `LibvirtComputingResource` (the storage processor has 
its own `createtmplt.sh` lookup, `createvm.sh` was located at startup but never 
executed), three helpers in `LibvirtUtilitiesHelper`, a handful of fields that 
were declared but never read, and the 27 unit tests that exercised the 
wrappers. The `Command` classes themselves stay in `core` in this PR because 
XenServer, VMware and the simulator still reference them; that is handled in a 
follow-up PR.
   
   **Unused scripts in `scripts/storage/qcow2`** (12 files). Nine have no 
callers anywhere in the tree. Three (`createvolume.sh`, `listvmtmplt.sh`, 
`listvolume.sh`) are stale copies of scripts in `scripts/storage/secondary`; 
the secondary storage code resolves them from that directory only. 
`createtmplt.sh`, `managesnapshot.sh` and `resizevolume.sh` remain, the agent 
still uses them.
   
   **Classes with no references anywhere** (7 files):
   
   - `IvsVifDriver`: VIF driver for Big Switch IVS. It could only be activated 
by naming it in `libvirt.vif.driver` in `agent.properties`, and the Big Switch 
plugin never refers to it. Anyone still pointing `agent.properties` at it will 
get a class-not-found at agent start, so this deserves a release note.
   - `ManagedNfsStorageAdaptor`: picked up by the storage adaptor reflection 
scan, but the management server never creates a pool of type `ManagedNFS`, so 
the adaptor was never selected. The enum value is removed in the follow-up PR.
   - `LibvirtNetworkDef`, `KVMGuestOsMapper`, `KVMStorageResource`, 
`StorageAdaptorInfo`, `KVMVirtualDisk`.
   
   No functional behaviour changes for any supported configuration. Packaging 
needs no changes: both the RPM spec and the Debian rules copy the `scripts` 
directory wholesale.
   
   ### Types of changes
   
   - [ ] Breaking change (fix or feature that would cause existing 
functionality to change)
   - [ ] New feature (non-breaking change which adds functionality)
   - [ ] Bug fix (non-breaking change which fixes an issue)
   - [ ] Enhancement (improves an existing feature and functionality)
   - [x] Cleanup (Code refactoring and cleanup, that may add test cases)
   - [ ] Build/CI
   - [ ] Test (unit or integration test code)
   
   ### Feature/Enhancement Scale or Bug Severity
   
   #### Feature/Enhancement Scale
   
   - [ ] Major
   - [x] Minor
   
   #### Bug Severity
   
   - [ ] BLOCKER
   - [ ] Critical
   - [ ] Major
   - [ ] Minor
   - [ ] Trivial
   
   ### Screenshots (if appropriate):
   
   ### How Has This Been Tested?
   
   Every removed class, method and script was checked for references across the 
whole tree (Java main and test sources, Spring XML, properties, Python and 
shell), including reflection-based loading (the storage adaptor scan, the 
`libvirt.vif.driver` property) and libvirt callback interfaces.
   
   The KVM plugin compiles with JDK 17 and its unit tests pass (862 tests, 0 
failures). A full-tree `mvn -T 1C -Dsimulator test` on the combined cleanup 
branches passes: 147 modules, 13,188 tests, 0 failures.
   
   #### How did you try to break this feature and the system with this change?
   
   Searched for every removed symbol across all file types, not only Java, so 
that reflection strings, Spring bean definitions and script names would show 
up. Classes that are loaded by reflection but do have a real pool type behind 
them (`FiberChannelAdapter`) and libvirt callback methods 
(`LibvirtDomainListener.onLifecycleChange`) were identified as false positives 
and kept.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to