Conversation
None of these has a caller anywhere in the tree, tests included: getHostDistro, isCentosHost, isDirectAttachedNetwork, isSnapshotSupported, post_default_network_rules and getPoolManager. post_default_network_rules shares its name with a function in security_group.py; the script is still invoked through other paths, only the unused Java wrapper for that sub-command goes.
Getters and setters that nothing calls, tests included: getVcpu, getEngine, setBootKernel, setSharable, setAttachDeferred, isAttachDeferred, setHostNetType, getHostNetType, getNetSourceMode, getVirtualPortType, getVirtualPortInterfaceId and isLinkStateUp. With them go the fields that only they touched and that the generated XML never read: _kernel, _initrd, _root, _cmdline, _shareable, _deferAttach, _hostNetType and the HostNicType enum, plus the long unused _ipAddr.
The four NfsStoragePool setters, getHBFile, getHBFolder and runScriptRetry have no callers anywhere in the tree.
Each of these has no caller anywhere in the tree, tests included: LibvirtSecretDef: getEphemeral, getVolumeVolume, getCephName LibvirtStorageVolumeXMLParser: getBackingFileNameIfExists LibvirtStorageAdaptor: storagePoolRefresh KVMStorageProcessor: getDefaultStorageScriptsDir LibvirtMigrateCommandWrapper: hasClvmDestinationDisks MultipathSCSIAdapterBase / FiberChannelAdapter: isStoragePoolTypeSupported QemuImg: supportsSkipZeros KVMHostInfo: getTotalCpus
Running PMD's UnusedPrivateMethod, UnusedPrivateField and
UnusedLocalVariable rules over the module reported these, each verified
by hand against the sources and the tests:
LibvirtComputingResource: field host, field gson, method
executeBashScript, local devNum in configureIpAddresses, local
brname in destroyNetworkRulesForVM
LibvirtConnection: field s_connection
LibvirtStorageAdaptor: field _manageSnapshotPath and its lookup of
managesnapshot.sh, which the adaptor never ran
LibvirtMigrateCommandWrapper: method isClvmBlockDevice
LibvirtCopyToSecondaryStorageWrapper: local success
LibvirtCreateDiskOnlyVMSnapshotCommandWrapper: two unused copies of
the volume list
Left as reported: getPifs and getOvsPifs are called and were flagged in
error, and the write-only fields _crash and _readonly in LibvirtVMDef
are kept because dropping them would silently turn their setters into
no-ops.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #14238 +/- ##
==========================================
Coverage 19.91% 19.91%
- Complexity 20200 20201 +1
==========================================
Files 6373 6373
Lines 577230 577025 -205
Branches 70696 70684 -12
==========================================
- Hits 114974 114942 -32
+ Misses 449690 449524 -166
+ Partials 12566 12559 -7
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 methods, fields and private members from the KVM agent plugin that have no callers anywhere in the tree, tests included. It is independent of the other two cleanup PRs and applies directly to
main.The commits are split per class so that individual pieces can be dropped in review:
LibvirtComputingResource:getHostDistro,isCentosHost,isDirectAttachedNetwork,isSnapshotSupported,post_default_network_rules,getPoolManager. The last one shares its name with a function insecurity_group.py; the script is still invoked through other paths, only the unused Java wrapper for that sub-command goes.LibvirtVMDef: twelve getters and setters, plus the fields that only they touched and that the generated XML never read (_kernel,_initrd,_root,_cmdline,_shareable,_deferAttach,_hostNetTypeand theHostNicTypeenum, and the long unused_ipAddr).KVMHABase: the fourNfsStoragePoolsetters,getHBFile,getHBFolderandrunScriptRetry.LibvirtSecretDef(three getters, the class itself is in use),LibvirtStorageVolumeXMLParser,LibvirtStorageAdaptor,KVMStorageProcessor,LibvirtMigrateCommandWrapper, the two multipath adapters,QemuImgandKVMHostInfo.UnusedPrivateMethod,UnusedPrivateFieldandUnusedLocalVariablerules, each verified by hand. Two PMD findings were false positives (getPifs,getOvsPifsare called) and two write-only fields inLibvirtVMDef(_crash,_readonly) were deliberately kept because dropping them would silently turn their setters into no-ops.Methods that are called only by unit tests were intentionally left alone. Removing them would mean removing test assertions, and two of them are recent CLVM helpers that may belong to in-flight work.
No functional behaviour changes.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
Each candidate was checked with a whole-word search across every file type in the tree, and for same-named declarations elsewhere that could indicate a call through an interface or superclass. Two candidates had hits outside their file and both turned out to be unrelated (a local variable of the same name in
StatsCollector, and the Python function insecurity_group.py).After the removals every touched file was scanned for fields that had lost their last reference and for unused imports. The KVM plugin compiles with JDK 17, Checkstyle is clean and the unit tests pass (892 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?
Ran PMD over the module to cross-check the manual scan, and treated every PMD finding as a claim to verify rather than as ground truth; one finding (
conninKVMStorageProcessor) turned out to have a side-effecting assignment and was kept.