Conversation
…nger sends
The KVM agent still registered wrappers for a set of commands that date
back to the pre-4.2 storage model. None of these commands is created or
subclassed anywhere in the management server any more, so the wrappers
could never be reached:
AttachIsoCommand, BackupSnapshotCommand, CheckConsoleProxyLoadCommand,
WatchConsoleProxyLoadCommand, CheckStorageAvailabilityCommand,
CreateCommand, CreatePrivateTemplateFromSnapshotCommand,
CreatePrivateTemplateFromVolumeCommand, CreateVolumeFromSnapshotCommand,
ManageSnapshotCommand, PrimaryStorageDownloadCommand,
RebootRouterCommand, UpgradeSnapshotCommand
Along with the wrappers and their shared console proxy base class, this
removes what only they used:
- the createtmplt.sh and createvm.sh lookups in
LibvirtComputingResource. The storage processor has its own
createtmplt.sh lookup; createvm.sh was looked up at startup but never
executed.
- the buildTemplateLocation, buildQCOW2Processor and
retrieveBashScriptPath helpers in LibvirtUtilitiesHelper and the
BASH_SCRIPT_PATH constant.
- fields that were declared but never read: heartBeatPath,
hostOsVersion, javaTempDir, and an unused local in
getMacAddressToNicNumPair.
- the 27 unit tests that exercised the removed wrappers.
The Command classes themselves stay in core because the XenServer,
VMware and simulator resources still reference them.
Nothing in the tree references these scripts any more. Nine of them have no callers at all: create_private_template.sh, createvm.sh, delvm.sh, get_domr_kernel.sh, get_iqn.sh, importmpl.sh, listvmdisk.sh, listvmdisksize.sh, managevolume.sh createvm.sh was still located at agent startup until the previous commit, but was never executed. The other three are stale copies of scripts that live in scripts/storage/secondary. The secondary storage code resolves them from that directory only, so the qcow2 copies were never run: createvolume.sh, listvmtmplt.sh, listvolume.sh createtmplt.sh, managesnapshot.sh and resizevolume.sh stay, the KVM agent still uses them.
- 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 itself never refers to it.
- 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 left in place.
- LibvirtNetworkDef: builder for libvirt <network> XML, never used.
- KVMGuestOsMapper, KVMStorageResource, StorageAdaptorInfo,
KVMVirtualDisk: no references anywhere in the tree.
14 tasks
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## main #14236 +/- ##
=============================================
- Coverage 19.91% 3.71% -16.21%
=============================================
Files 6373 487 -5886
Lines 577230 41992 -535238
Branches 70696 7942 -62754
=============================================
- Hits 114974 1558 -113416
+ Misses 449690 40208 -409482
+ Partials 12566 226 -12340
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.shandcreatevm.shlookups inLibvirtComputingResource(the storage processor has its owncreatetmplt.shlookup,createvm.shwas located at startup but never executed), three helpers inLibvirtUtilitiesHelper, a handful of fields that were declared but never read, and the 27 unit tests that exercised the wrappers. TheCommandclasses themselves stay incorein 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 inscripts/storage/secondary; the secondary storage code resolves them from that directory only.createtmplt.sh,managesnapshot.shandresizevolume.shremain, 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 inlibvirt.vif.driverinagent.properties, and the Big Switch plugin never refers to it. Anyone still pointingagent.propertiesat 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 typeManagedNFS, 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
scriptsdirectory wholesale.Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
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.driverproperty) 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 teston 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.