various fixes for fiberchannel and autoscale prefix option #11071

rg9975 · 2025-06-23T02:13:50Z

Description

This PR fixes various issues in the fiberchannel/adaptive storage provider, including adding an optional interface for handling connecting disks in a 3 step process to improve performance on VMs with a large number of disks to connect. One additional change is included to provide an option to disable the fixed string prefix added to autoscale VMs to allow for longer generated names. A couple of small annotations were added to help builds when running in an m2e-compatible IDE.

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)
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
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Code has been tested and used in live environment with appropriate storage appliances.

How did you try to break this feature and the system with this change?

Code has been tested and used in live environment with appropriate storage appliances.

codecov · 2025-06-23T08:09:41Z

Codecov Report

❌ Patch coverage is 1.52091% with 259 lines in your changes missing coverage. Please review.
✅ Project coverage is 16.17%. Comparing base (f24b175) to head (017a332).
⚠️ Report is 29 commits behind head on 4.20.

Files with missing lines	Patch %	Lines
...pervisor/kvm/storage/MultipathSCSIAdapterBase.java	0.00%	133 Missing ⚠️
.../datastore/driver/AdaptiveDataStoreDriverImpl.java	0.00%	55 Missing ⚠️
.../hypervisor/kvm/storage/KVMStoragePoolManager.java	0.00%	54 Missing ⚠️
...ud/hypervisor/kvm/storage/KVMStorageProcessor.java	0.00%	11 Missing ⚠️
...ava/com/cloud/network/as/AutoScaleManagerImpl.java	33.33%	5 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               4.20   #11071      +/-   ##
============================================
- Coverage     16.17%   16.17%   -0.01%     
- Complexity    13284    13296      +12     
============================================
  Files          5656     5656              
  Lines        498015   498264     +249     
  Branches      60406    60446      +40     
============================================
+ Hits          80538    80570      +32     
- Misses       408514   408729     +215     
- Partials       8963     8965       +2

Flag	Coverage Δ
uitests	`4.00% <ø> (-0.01%)`	⬇️
unittests	`17.02% <1.52%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot

Pull Request Overview

This PR fixes several issues and enhances functionality in both autoscale VM hostname generation and storage connectivity, including improvements for handling disk connections asynchronously and build configuration refinements.

Introduces an optional configuration for including a hostname prefix for autoscale VMs.
Implements asynchronous disk connection handling and improves error messaging and logging across storage adapter scripts.
Adds several m2e IDE build annotations and fixes path issues in Maven POMs.

Reviewed Changes

Copilot reviewed 15 out of 16 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java	Adds hostname generation methods with and without prefix for autoscale VMs.
server/src/main/java/com/cloud/network/as/AutoScaleManager.java	Introduces a new config key to enable/disable the autoscale hostname prefix.
scripts/storage/multipath/startConnectVolume.sh	Implements a shell script to connect volumes, with minor spelling issues in comments.
scripts/storage/multipath/finishConnectVolume.sh	Provides a finishing connect script with similar spelling issues as startConnectVolume.sh.
scripts/storage/multipath/disconnectVolume.sh	Updates background execution behavior for disk disconnect operations.
scripts/storage/multipath/connectVolume.sh	Refactors volume connect logic to use dedicated start/finish connect scripts and a waiting loop.
pom.xml, plugins/*/pom.xml, engine/schema/pom.xml	Updates Maven POM configurations and ensures m2e IDE compatibility.
plugins/storage/volume/adaptive/src/main/java/org/apache/cloudstack/storage/datastore/driver/AdaptiveDataStoreDriverImpl.java	Modifies persistence method signatures and logging for volume operations.
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/MultipathSCSIAdapterBase.java	Adjusts script timeout values and error messages, and adds new script properties.
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java, KVMStoragePoolManager.java, AsyncPhysicalDiskConnectorDecorator.java	Improves exception handling and implements support for asynchronous physical disk connections.

scripts/storage/multipath/startConnectVolume.sh

scripts/storage/multipath/finishConnectVolume.sh

scripts/storage/multipath/connectVolume.sh

...rc/main/java/org/apache/cloudstack/storage/datastore/driver/AdaptiveDataStoreDriverImpl.java

...hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/MultipathSCSIAdapterBase.java

Co-authored-by: Copilot <[email protected]>

…stack/storage/datastore/driver/AdaptiveDataStoreDriverImpl.java Co-authored-by: Copilot <[email protected]>

…/storage/MultipathSCSIAdapterBase.java Co-authored-by: Copilot <[email protected]>

rp- · 2025-07-03T13:20:29Z

I ran the Linstor integration tests against it and had not problems, so green light from me, from that perspective.

rp-

lgtm

github-actions · 2025-07-29T13:02:23Z

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

shwstppr · 2025-09-03T12:14:25Z

@rg9975 can you please resolve the conflicts?

vishesh92 · 2025-09-04T08:35:42Z

@blueorangutan package

blueorangutan · 2025-09-04T08:38:04Z

@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2025-09-04T09:54:53Z

Packaging result [SF]: ✖️ el8 ✖️ el9 ✔️ debian ✖️ suse15. SL-JID 14845

blueorangutan · 2025-09-08T14:20:13Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 14901

DaanHoogland · 2025-09-09T07:22:10Z

@blueorangutan test

blueorangutan · 2025-09-09T07:24:04Z

@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

blueorangutan · 2025-09-09T07:42:38Z

[SF] Trillian Build Failed (tid-14249)

vishesh92 · 2025-09-09T07:43:36Z

engine/schema/pom.xml

                                    templateList.add("systemvmtemplate-${csVersion}.${patch}-x86_64-ovm")
                                    templateList.add("systemvmtemplate-${csVersion}.${patch}-x86_64-hyperv")
-                                    File file = new File("./engine/schema/dist/systemvm-templates/md5sum.txt")
+                                    File file = new File("${basedir}/../../engine/schema/dist/systemvm-templates/md5sum.txt")


This doesn't look right.

Well, it technically will work, although it could have been expressed:

File file = new File("${basedir}/dist/systemvm-templates/md5sum.txt")

The primary motivation was to eliminate the relative File path that seems to cause problems in Visual Studio Code during compilation.

...ns/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java

vishesh92 · 2025-09-09T07:54:17Z

...hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/MultipathSCSIAdapterBase.java

-            if (!connectPhysicalDisk(address, pool, null)) {
-                throw new CloudRuntimeException("Unable to connect to volume " + address.getPath());
-            }
+            LOGGER.debug("Physical disk " + address.getPath() + " is not connected, a request to connectPhysicalDisk must be made before it can be used.");


Do we need to make a call to the connectPhysicalDisk method?

This is intentional in the change. The getPhysicalDisk operation expects that a previous call to connectPhysicalDisk has already been made. If it has not been made, an error is thrown. This allows for asynchronous connections to be processed by the storage provider to improve startup of virtual machines with a large # of attached disks rather than a connection being created as side effect of a getPhysicalDisk call.

…/storage/KVMStoragePoolManager.java Co-authored-by: Vishesh <[email protected]>

various fixes for fiberchannel and autoscale prefix option

e55d016

boring-cyborg bot added component:build component:kvm component:LDAP component:networking component:primary-storage component:storage labels Jun 23, 2025

DaanHoogland requested review from rp-, vishesh92 and Copilot and removed request for rp- June 23, 2025 11:37

Copilot AI reviewed Jun 23, 2025

View reviewed changes

Rene Glover and others added 6 commits June 23, 2025 10:18

Update scripts/storage/multipath/startConnectVolume.sh

1bf169b

Co-authored-by: Copilot <[email protected]>

Update scripts/storage/multipath/finishConnectVolume.sh

7be9beb

Co-authored-by: Copilot <[email protected]>

Update scripts/storage/multipath/connectVolume.sh

6353507

Co-authored-by: Copilot <[email protected]>

Update plugins/storage/volume/adaptive/src/main/java/org/apache/cloud…

2a80c96

…stack/storage/datastore/driver/AdaptiveDataStoreDriverImpl.java Co-authored-by: Copilot <[email protected]>

Update plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm…

f83481e

…/storage/MultipathSCSIAdapterBase.java Co-authored-by: Copilot <[email protected]>

end of line resizeVolume.sh

6a97d69

rp- approved these changes Jul 3, 2025

View reviewed changes

DaanHoogland removed component:build component:LDAP labels Jul 4, 2025

DaanHoogland added this to the 4.20.2 milestone Jul 4, 2025

github-actions bot added the status:has-conflicts label Jul 29, 2025

Merge branch '4.20' into 4.20-fiberchannel-patches

86eff0e

boring-cyborg bot added the component:build label Sep 3, 2025

boring-cyborg bot added the component:LDAP label Sep 3, 2025

github-actions bot removed the status:has-conflicts label Sep 3, 2025

DaanHoogland closed this Sep 9, 2025

DaanHoogland reopened this Sep 9, 2025

vishesh92 reviewed Sep 9, 2025

View reviewed changes

...ns/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java Outdated Show resolved Hide resolved

vishesh92 reviewed Sep 9, 2025

View reviewed changes

Update plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm…

017a332

…/storage/KVMStoragePoolManager.java Co-authored-by: Vishesh <[email protected]>

weizhouapache modified the milestones: 4.20.2, 4.20.3 Sep 10, 2025

various fixes for fiberchannel and autoscale prefix option #11071

Are you sure you want to change the base?

various fixes for fiberchannel and autoscale prefix option #11071

Uh oh!

Conversation

rg9975 commented Jun 23, 2025

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

Uh oh!

codecov bot commented Jun 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rp- commented Jul 3, 2025

Uh oh!

rp- left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jul 29, 2025

Uh oh!

shwstppr commented Sep 3, 2025

Uh oh!

vishesh92 commented Sep 4, 2025

Uh oh!

blueorangutan commented Sep 4, 2025

Uh oh!

blueorangutan commented Sep 4, 2025

Uh oh!

blueorangutan commented Sep 8, 2025

Uh oh!

DaanHoogland commented Sep 9, 2025

Uh oh!

blueorangutan commented Sep 9, 2025

Uh oh!

blueorangutan commented Sep 9, 2025

Uh oh!

vishesh92 Sep 9, 2025

Choose a reason for hiding this comment

Uh oh!

rg9975 Sep 9, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vishesh92 Sep 9, 2025

Choose a reason for hiding this comment

Uh oh!

rg9975 Sep 9, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Jun 23, 2025 •

edited

Loading