Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
poddm commented on PR #11071:
URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-4354450527
@rg9975 -
Should MultipathSCSIAdapterBase.java or FiberChannelAdapter.java implement
`AsyncPhysicalDiskConnectorDecorator`?
this check in KVMStoragePoolManagerwould would never be true:
```java
if (adaptor instanceof AsyncPhysicalDiskConnectorDecorator) {
```
Java requires the interface to be declared on the class (or a superclass)
for instanceof to match. Without it, the entire async disk connection path is
dead code.
--
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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
DaanHoogland commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3804729773 @rg9975 , from an lab env attempt: ``` 2026-01-27 10:35:12,197 ERROR [c.c.s.StorageManagerImpl] (qtp1390913202-16:[ctx-e7c7f6e4, ctx-d42028a4, ctx-087c6d5f]) (logid:d3f57cd1) Failed to register system VM template for hypervisor: KVM amd64 com.cloud.utils .exception.CloudRuntimeException: Checksum failed for local template file at com.cloud.upgrade.SystemVmTemplateRegistration.validateTemplateFileForHypervisorAndArch(SystemVmTemplateRegistration.java:782) at com.cloud.upgrade.SystemVmTemplateRegistration.validateAndRegisterTemplate(SystemVmTemplateRegistration.java:788) at com.cloud.storage.StorageManagerImpl.registerSystemVmTemplateForHypervisorArch(StorageManagerImpl.java:3632) at com.cloud.storage.StorageManagerImpl$4.doInTransactionWithoutResult(StorageManagerImpl.java:3662) at com.cloud.utils.db.TransactionCallbackNoReturn.doInTransaction(TransactionCallbackNoReturn.java:25) at com.cloud.utils.db.Transaction$2.doInTransaction(Transaction.java:48) at com.cloud.utils.db.Transaction.execute(Transaction.java:38) at com.cloud.utils.db.Transaction.execute(Transaction.java:45) at com.cloud.storage.StorageManagerImpl.registerSystemVmTemplateOnFirstNfsStore(StorageManagerImpl.java:3642) at com.cloud.storage.StorageManagerImpl.discoverImageStore(StorageManagerImpl.java:3593) ``` -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
DaanHoogland commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3804215565 @rg9975 sorry, systemvms don’t come up in a lab env with this PR -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
blueorangutan commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3804010464 [SF] Trillian Build Failed (tid-15290) -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
blueorangutan commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3803694416 @DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
DaanHoogland commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3803686664 @blueorangutan test -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
blueorangutan commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3694676712 [SF] Trillian Build Failed (tid-15073) -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
blueorangutan commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3694635237 @DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
DaanHoogland commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3694634048 @blueorangutan test -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
blueorangutan commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3693297665 Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 16169 -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
DaanHoogland commented on code in PR #11071:
URL: https://github.com/apache/cloudstack/pull/11071#discussion_r2646146627
##
engine/schema/pom.xml:
##
@@ -24,7 +24,7 @@
org.apache.cloudstack
cloud-engine
-4.20.3.0-SNAPSHOT
+4.23.0.0-SNAPSHOT
Review Comment:
actually I think more needs to change. In my theory the line that was the
problem was replacing the ${project.basedir} so just
8e9316a5e551a93e72fe820234a45caad413a278 needed reverting.
--
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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
blueorangutan commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3690358609 Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 16159 -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
DaanHoogland commented on code in PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#discussion_r2646142856 ## engine/schema/pom.xml: ## @@ -24,7 +24,7 @@ org.apache.cloudstack cloud-engine -4.20.3.0-SNAPSHOT +4.23.0.0-SNAPSHOT Review Comment: ```suggestion 4.20.3.0-SNAPSHOT ``` -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
DaanHoogland commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3690157841 I have no idea @rg9975 , but my first thought went to the engine/schema/pom.xml (line 111) `Cannot find @MSCONF@/db.properties` , does it get copied properly? It is not an intermitted issue. At least two of the prior runs showed the same. -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
rg9975 commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3690143764 @DaanHoogland nothing in this PR changed the files/scripts related to the output you posted. I don't find that output in any of the workflow job output, can you point to where that is? -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
DaanHoogland commented on PR #11071:
URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3687190914
@rg9975 , the deploy fails:
```
10:03:25 fatal: [pr11071-t15047-kvm-ol8-mgmt2]: FAILED! => {"changed": true,
"cmd": "/usr/bin/cloudstack-setup-databases 'cloud’:’**'@'10.0.33.124' ",
"delta": "0:00:00.114740", "end": "2025-12-23 09:03:25.297505", "failed": true,
"rc": 1, "start": "2025-12-23 09:03:25.182765", "stderr": "\n\nWe apologize for
below
error:\n***\nCannot
find
@MSCONF@/db.properties\n***\nPlease
run:\n\ncloudstack-setup-databases -h\n\nfor full help", "stderr_lines":
["", "", "We apologize for below error:",
"***", "Cannot find
@MSCONF@/db.properties",
"***", "Please
run:", "", "cloudstack-setup-databases -h", "", "for full help"], "stdout":
"Mysql user name:cloud
[ \u001b[92mOK\u001b[0m ]\nMy
sql user password:**
[ \u001b[92mOK\u001b[0m ]\nMysql server ip:10.0.33.124
[ \u001b[92mOK\u001b[0m ]\nMysql server port:3306
[ \u001b[92mOK\u001b[0m
]\nChecking Cloud database files ...
", "stdout_lines": ["Mysql user name:cloud
[ \u001b[92mOK\u001b[0m ]", "Mysql user password:**
[ \u001b[92mOK\u001b[0m
]", "Mysql server ip:10.0.33.124
[ \u001b[92mOK\u001b[0m ]", "Mysql server port:3306
[ \u001b[92mOK\u001b[0m ]", "Checking Cloud
database files ... "]}
```
--
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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
blueorangutan commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3685816383 [SF] Trillian Build Failed (tid-15047) -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
blueorangutan commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3685594708 [SF] Trillian Build Failed (tid-15046) -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
blueorangutan commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3685529153 @DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
DaanHoogland commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3685516755 @blueorangutan test -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
blueorangutan commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3684222998 Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 16132 -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
blueorangutan commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3683918913 @DaanHoogland 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. -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
DaanHoogland commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3683911627 @blueorangutan package -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
github-actions[bot] commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3681486363 This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
blueorangutan commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3536855276 [SF] Trillian Build Failed (tid-14841) -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
blueorangutan commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3536694776 [SF] Trillian Build Failed (tid-14840) -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
blueorangutan commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3536675570 @DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
DaanHoogland commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3536674547 @blueorangutan test -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
blueorangutan commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3534734767 Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15773 -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
blueorangutan commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3534477459 @DaanHoogland 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. -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
DaanHoogland commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3534470880 @blueorangutan package -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
rg9975 commented on code in PR #11071:
URL: https://github.com/apache/cloudstack/pull/11071#discussion_r2528345475
##
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/MultipathSCSIAdapterBase.java:
##
@@ -164,9 +178,10 @@ private KVMPhysicalDisk getPhysicalDisk(AddressInfo
address, KVMStoragePool pool
// validate we have a connection, if not we need to connect first.
if (!isConnected(address.getPath())) {
-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.");
+} else {
+LOGGER.debug("Physical disk " + address.getPath() + " is
connected, proceeding to get its size.");
+
Review Comment:
Updates in upcomming commit. method should return null when connection is
not yet made instead of proceeding to the sizing logic. also changed log to a
warn instead of debug since this would be an unexpected condition.
--
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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
rg9975 commented on code in PR #11071:
URL: https://github.com/apache/cloudstack/pull/11071#discussion_r2528336913
##
engine/schema/pom.xml:
##
@@ -106,7 +108,7 @@
templateList.add("systemvmtemplate-${csVersion}.${patch}-x86_64-xen")
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")
Review Comment:
Ok I've updated to get rid of the relative path and the up/down traversal.
project.basedir will resolve to absolute path of the module no matter where the
user or IDE is running the maven command from.
--
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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
blueorangutan commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3533253617 Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15772 -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
blueorangutan commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3532976335 @DaanHoogland 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. -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
DaanHoogland commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3532968483 @blueorangutan package -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
rg9975 commented on code in PR #11071:
URL: https://github.com/apache/cloudstack/pull/11071#discussion_r2527567489
##
plugins/user-authenticators/ldap/pom.xml:
##
@@ -114,11 +115,10 @@
${groovy.version}
test
-
-
+
cglib
cglib-nodep
-test
+${cs.cglib.version}
Review Comment:
This was intentional to resolve artifact resolution errors when code is
loaded in Visual Code.
--
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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
Copilot commented on code in PR #11071:
URL: https://github.com/apache/cloudstack/pull/11071#discussion_r2526782545
##
server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java:
##
@@ -1952,6 +1953,19 @@ public void updateVmDetails(Map
deployParams, Map 6) { printf("%s:%s ", $NF, $(NF-1));} }' | sed -e 's/0x/3/g')
+fi
- if [ ! -z "${lingering_devs}" ]; then
- for dev in ${lingering_devs}; do
- LSSCSI_WWID=$(echo $dev | awk -F: '{print $2}' | sed -e 's/0x/3/g')
- FOUND_OUI=$(echo ${LSSCSI_WWID} | cut -c3-8)
- if [ "${INCOMING_OUI}" != "${FOUND_OUI}" ]; then
- continue;
- fi
- dev=$(echo $dev | awk -F: '{ print $1}')
- logger -t "CS_SCSI_VOL_FIND" "${WWID} processing identified a lingering
device ${dev} from previous lun use, attempting to clean up"
- MP_WWID=$(multipath -l ${dev} | head -1 | awk '{print $1}')
- MP_WWID=${MP_WWID:1} # strip first character (3) off
- # don't do this if the WWID passed in matches the WWID from multipath
- if [ ! -z "${MP_WWID}" ] && [ "${MP_WWID}" != "${WWID}" ]; then
- # run full removal again so all devices and multimap are cleared
- $(dirname $0)/disconnectVolume.sh ${MP_WWID}
- # we don't have a multimap but we may still have some stranded devices
to clean up
- elif [ "${LSSCSI_WWID}" != "${WWID}" ]; then
- echo "1" > /sys/block/$(echo ${dev} | awk -F'/' '{print
$NF}')/device/delete
- fi
- done
- sleep 3
- fi
+// wait for the device path to show up
Review Comment:
Invalid comment syntax. The comment uses `//` (double slash) syntax on line
46 of a bash script, which should use `#` instead. This will cause a syntax
error.
```suggestion
# wait for the device path to show up
```
##
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/MultipathSCSIAdapterBase.java:
##
@@ -164,9 +178,10 @@ private KVMPhysicalDisk getPhysicalDisk(AddressInfo
address, KVMStoragePool pool
// validate we have a connection, if not we need to connect first.
if (!isConnected(address.getPath())) {
-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.");
+} else {
+LOGGER.debug("Physical disk " + address.getPath() + " is
connected, proceeding to get its size.");
+
Review Comment:
Unreachable code after break statement. Line 184 has code after a closing
brace that would have been unreachable in the original structure. The logic
flow appears broken - the `else` block contains only a debug statement with no
actual connection logic.
```suggestion
LOGGER.debug("Physical disk " + address.getPath() + " is not
connected, attempting to connectPhysicalDisk.");
connectPhysicalDisk(address.getPath());
} else {
LOGGER.debug("Physical disk " + address.getPath() + " is
connected, proceeding to get its size.");
```
##
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/MultipathSCSIAdapterBase.java:
##
@@ -98,6 +102,16 @@ public abstract class MultipathSCSIAdapterBase implements
StorageAdaptor {
throw new Error("Unable to find the connectVolume.sh script");
}
+startConnectScript =
Script.findScript(STORAGE_SCRIPTS_DIR.getFinalValue(), startConnectScript);
+if (startConnectScript == null) {
+throw new Error("Unable to find the startConnectScript.sh script");
+}
+
+finishConnectScript =
Script.findScript(STORAGE_SCRIPTS_DIR.getFinalValue(), finishConnectScript);
+if (finishConnectScript == null) {
+throw new Error("Unable to find the finishConnectScript.sh
script");
Review Comment:
Error message references wrong script name. The error message on line 112
says "Unable to find the finishConnectScript.sh script" but should reference
"finishConnectVolume.sh" based on the script filename being checked.
```suggestion
throw new Error("Unable to find the finishConnectVolume.sh
script");
```
##
scripts/storage/multipath/startConnectVolume.sh:
##
@@ -0,0 +1,101 @@
+#!/usr/bin/env bash
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+#
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
vishesh92 commented on code in PR #11071:
URL: https://github.com/apache/cloudstack/pull/11071#discussion_r2526734792
##
scripts/storage/multipath/connectVolume.sh:
##
@@ -29,103 +29,40 @@ WWID=${2:?"WWID required"}
WWID=$(echo $WWID | tr '[:upper:]' '[:lower:]')
-systemctl is-active multipathd || systemctl restart multipathd || {
- echo "$(date): Multipathd is NOT running and cannot be started. This must
be corrected before this host can access this storage volume."
- logger -t "CS_SCSI_VOL_FIND" "${WWID} cannot be mapped to this host because
multipathd is not currently running and cannot be started"
+START_CONNECT=$(dirname $0)/startConnect.sh
+if [ -x "${START_CONNECT}" ]; then
+ echo "$(date): Starting connect process for ${WWID} on lun ${LUN}"
+ ${START_CONNECT} ${LUN} ${WWID}
+ if [ $? -ne 0 ]; then
+ echo "$(date): Failed to start connect process for ${WWID} on lun ${LUN}"
+ logger -t "CS_SCSI_VOL_FIND" "${WWID} failed to start connect process on
lun ${LUN}"
+ exit 1
+ fi
+else
+ echo "$(date): Unable to find startConnect.sh script!"
exit 1
-}
-
-echo "$(date): Looking for ${WWID} on lun ${LUN}"
-
-# get vendor OUI. we will only delete a device on the designated lun if it
matches the
-# incoming WWN OUI value. This is because multiple storage arrays may be
mapped to the
-# host on different fiber channel hosts with the same LUN
-INCOMING_OUI=$(echo ${WWID} | cut -c2-7)
-echo "$(date): Incoming OUI: ${INCOMING_OUI}"
-
-# first we need to check if any stray references are left from a previous use
of this lun
-for fchost in $(ls /sys/class/fc_host | sed -e 's/host//g'); do
- lingering_devs=$(lsscsi -w "${fchost}:*:*:${LUN}" | grep /dev | awk '{if
(NF > 6) { printf("%s:%s ", $NF, $(NF-1));} }' | sed -e 's/0x/3/g')
+fi
- if [ ! -z "${lingering_devs}" ]; then
- for dev in ${lingering_devs}; do
- LSSCSI_WWID=$(echo $dev | awk -F: '{print $2}' | sed -e 's/0x/3/g')
- FOUND_OUI=$(echo ${LSSCSI_WWID} | cut -c3-8)
- if [ "${INCOMING_OUI}" != "${FOUND_OUI}" ]; then
- continue;
- fi
- dev=$(echo $dev | awk -F: '{ print $1}')
- logger -t "CS_SCSI_VOL_FIND" "${WWID} processing identified a lingering
device ${dev} from previous lun use, attempting to clean up"
- MP_WWID=$(multipath -l ${dev} | head -1 | awk '{print $1}')
- MP_WWID=${MP_WWID:1} # strip first character (3) off
- # don't do this if the WWID passed in matches the WWID from multipath
- if [ ! -z "${MP_WWID}" ] && [ "${MP_WWID}" != "${WWID}" ]; then
- # run full removal again so all devices and multimap are cleared
- $(dirname $0)/disconnectVolume.sh ${MP_WWID}
- # we don't have a multimap but we may still have some stranded devices
to clean up
- elif [ "${LSSCSI_WWID}" != "${WWID}" ]; then
- echo "1" > /sys/block/$(echo ${dev} | awk -F'/' '{print
$NF}')/device/delete
- fi
- done
- sleep 3
- fi
+// wait for the device path to show up
Review Comment:
```suggestion
# wait for the device path to show up
```
##
plugins/storage/volume/adaptive/src/main/java/org/apache/cloudstack/storage/datastore/driver/AdaptiveDataStoreDriverImpl.java:
##
@@ -840,55 +840,95 @@ public boolean canHostAccessStoragePool(Host host,
StoragePool pool) {
}
void persistVolumeOrTemplateData(StoragePoolVO storagePool, Map storagePoolDetails,
-DataObject dataObject, ProviderVolume volume, Map
connIdMap) {
+DataObject dataObject, ProviderVolume volume, Map
connIdMap, Long size) {
if (dataObject.getType() == DataObjectType.VOLUME) {
-persistVolumeData(storagePool, storagePoolDetails, dataObject,
volume, connIdMap);
+persistVolumeData(storagePool, storagePoolDetails, dataObject,
volume, connIdMap, size);
} else if (dataObject.getType() == DataObjectType.TEMPLATE) {
-persistTemplateData(storagePool, storagePoolDetails, dataObject,
volume, connIdMap);
+persistTemplateData(storagePool, storagePoolDetails, dataObject,
volume, connIdMap, size);
}
}
void persistVolumeData(StoragePoolVO storagePool, Map
details, DataObject dataObject,
-ProviderVolume managedVolume, Map connIdMap) {
+ ProviderVolume managedVolume, Map connIdMap, Long
size) {
+
+// Get the volume by dataObject id
VolumeVO volumeVO = _volumeDao.findById(dataObject.getId());
+long volumeId = volumeVO.getId();
+// Generate path for volume and details
String finalPath = generatePathInfo(managedVolume, connIdMap);
-volumeVO.setPath(finalPath);
-volumeVO.setFormat(ImageFormat.RAW);
-volumeVO.setPoolId(storagePool.getId());
-volumeVO.setExternalUuid(managedVolume.getExternalUuid());
-volumeVO.setDisplay(true);
-volumeVO.setDisplayVolume(true);
-_volumeDao.u
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
DaanHoogland commented on code in PR #11071:
URL: https://github.com/apache/cloudstack/pull/11071#discussion_r2526704978
##
engine/schema/pom.xml:
##
@@ -106,7 +108,7 @@
templateList.add("systemvmtemplate-${csVersion}.${patch}-x86_64-xen")
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")
Review Comment:
isn’t there another way to get to the root of the root-pom instead of the
root of the subproject?
--
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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
DaanHoogland closed pull request #11071: various fixes for fiberchannel and autoscale prefix option URL: https://github.com/apache/cloudstack/pull/11071 -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
DaanHoogland commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3443430153 @vishesh92 , can you have a look, please? -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
rg9975 commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3443408263 Yes, this still needs to be merged, I don't see any outstanding issues unless someone is concerned with the feedback on the last 2 items. -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
DaanHoogland commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3442855971 @rg9975 , are you still working on this? -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
blueorangutan commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3266565629 Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 14901 -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
vishesh92 commented on code in PR #11071:
URL: https://github.com/apache/cloudstack/pull/11071#discussion_r2332337136
##
engine/schema/pom.xml:
##
@@ -106,7 +108,7 @@
templateList.add("systemvmtemplate-${csVersion}.${patch}-x86_64-xen")
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")
Review Comment:
This doesn't look right.
--
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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
rg9975 commented on code in PR #11071:
URL: https://github.com/apache/cloudstack/pull/11071#discussion_r232568
##
engine/schema/pom.xml:
##
@@ -106,7 +108,7 @@
templateList.add("systemvmtemplate-${csVersion}.${patch}-x86_64-xen")
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")
Review Comment:
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.
--
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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
rg9975 commented on code in PR #11071:
URL: https://github.com/apache/cloudstack/pull/11071#discussion_r210326
##
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/MultipathSCSIAdapterBase.java:
##
@@ -164,9 +178,10 @@ private KVMPhysicalDisk getPhysicalDisk(AddressInfo
address, KVMStoragePool pool
// validate we have a connection, if not we need to connect first.
if (!isConnected(address.getPath())) {
-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.");
Review Comment:
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.
--
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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
DaanHoogland closed pull request #11071: various fixes for fiberchannel and autoscale prefix option URL: https://github.com/apache/cloudstack/pull/11071 -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
DaanHoogland commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3269272845 @blueorangutan test -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
vishesh92 commented on code in PR #11071:
URL: https://github.com/apache/cloudstack/pull/11071#discussion_r2332366820
##
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/MultipathSCSIAdapterBase.java:
##
@@ -164,9 +178,10 @@ private KVMPhysicalDisk getPhysicalDisk(AddressInfo
address, KVMStoragePool pool
// validate we have a connection, if not we need to connect first.
if (!isConnected(address.getPath())) {
-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.");
Review Comment:
Do we need to make a call to the `connectPhysicalDisk` method?
--
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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
vishesh92 commented on code in PR #11071:
URL: https://github.com/apache/cloudstack/pull/11071#discussion_r2332355218
##
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java:
##
@@ -187,17 +207,79 @@ public boolean
connectPhysicalDisksViaVmSpec(VirtualMachineTO vmSpec, boolean is
KVMStoragePool pool = getStoragePool(store.getPoolType(),
store.getUuid());
StorageAdaptor adaptor = getStorageAdaptor(pool.getType());
-result = adaptor.connectPhysicalDisk(vol.getPath(), pool,
disk.getDetails(), isVMMigrate);
+if (adaptor instanceof AsyncPhysicalDiskConnectorDecorator) {
+// If the adaptor supports async disk connection, we can start
the connection
+// and return immediately, allowing the connection to complete
in the background.
+result = ((AsyncPhysicalDiskConnectorDecorator)
adaptor).startConnectPhysicalDisk(vol.getPath(), pool, disk.getDetails());
+if (!result) {
+logger.error("Failed to start connecting disks via vm spec
for vm: " + vmName + " volume:" + vol.toString());
+return false;
+}
-if (!result) {
-logger.error("Failed to connect disks via vm spec for vm: " +
vmName + " volume:" + vol.toString());
-return result;
+// add disk to list of disks to check later
+connectingDisks.add(new ConnectingDiskInfo(vol, adaptor, pool,
disk.getDetails()));
+} else {
+result = adaptor.connectPhysicalDisk(vol.getPath(), pool,
disk.getDetails(), isVMMigrate);
+
+if (!result) {
+logger.error("Failed to connect disks via vm spec for vm:
" + vmName + " volume:" + vol.toString());
+return result;
+}
+}
+}
+
+// if we have any connecting disks to check, wait for them to connect
or timeout
+if (!connectingDisks.isEmpty()) {
+for (ConnectingDiskInfo connectingDisk : connectingDisks) {
+StorageAdaptor adaptor = connectingDisk.adapter;
+KVMStoragePool pool = connectingDisk.pool;
+VolumeObjectTO volume = connectingDisk.volume;
+Map details = connectingDisk.details;
+long diskWaitTimeMillis = getDiskWaitTimeMillis(details);
+
+// wait for the disk to connect
+long startTime = System.currentTimeMillis();
+while (System.currentTimeMillis() - startTime <
diskWaitTimeMillis) {
+if (((AsyncPhysicalDiskConnectorDecorator)
adaptor).isConnected(volume.getPath(), pool, details)) {
+logger.debug(String.format("Disk %s connected
successfully for VM %s", volume.getPath(), vmName));
+break;
+}
+
+sleep(1000); // wait for 1 second before checking again
+}
}
}
return result;
}
+private long getDiskWaitTimeMillis(Map details) {
Review Comment:
```suggestion
private long getDiskWaitTimeMillis(Map details) {
```
--
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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
blueorangutan commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3269346202 [SF] Trillian Build Failed (tid-14249) -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
blueorangutan commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3269280101 @DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
blueorangutan commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3252882963 Packaging result [SF]: ✖️ el8 ✖️ el9 ✔️ debian ✖️ suse15. SL-JID 14845 -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
blueorangutan commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3252520383 @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. -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
vishesh92 commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3252512439 @blueorangutan package -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
shwstppr commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3248998026 @rg9975 can you please resolve the conflicts? -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
github-actions[bot] commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3132442440 This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
rp- commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-3032246850 I ran the Linstor integration tests against it and had not problems, so green light from me, from that perspective. -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
codecov[bot] commented on PR #11071: URL: https://github.com/apache/cloudstack/pull/11071#issuecomment-2995390444 ## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/11071?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report All modified and coverable lines are covered by tests :white_check_mark: > Project coverage is 4.00%. Comparing base [(`5790091`)](https://app.codecov.io/gh/apache/cloudstack/commit/5790091b25de4a170e12f1558c12dcd4424a25c7?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`e55d016`)](https://app.codecov.io/gh/apache/cloudstack/commit/e55d0164186841d41a8577eca78cad0e19b31d12?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache). > :exclamation: There is a different number of reports uploaded between BASE (5790091) and HEAD (e55d016). Click for more details. > > HEAD has 1 upload less than BASE > >| Flag | BASE (5790091) | HEAD (e55d016) | >|--|--|--| >|unittests|1|0| > Additional details and impacted files ```diff @@ Coverage Diff @@ ## 4.20 #11071 +/- ## = - Coverage 16.15%4.00% -12.16% = Files 5657 402 -5255 Lines49789832608 -465290 Branches 60374 5789-54585 = - Hits 80441 1305-79136 + Misses 40849631154 -377342 + Partials 8961 149 -8812 ``` | [Flag](https://app.codecov.io/gh/apache/cloudstack/pull/11071/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | |---|---|---| | [uitests](https://app.codecov.io/gh/apache/cloudstack/pull/11071/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `4.00% <ø> (ø)` | | | [unittests](https://app.codecov.io/gh/apache/cloudstack/pull/11071/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more. [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/cloudstack/pull/11071?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache). :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache). :rocket: New features to boost your workflow: - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems. - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges. -- 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]
Re: [PR] various fixes for fiberchannel and autoscale prefix option [cloudstack]
Copilot commented on code in PR #11071:
URL: https://github.com/apache/cloudstack/pull/11071#discussion_r2161408514
##
scripts/storage/multipath/finishConnectVolume.sh:
##
@@ -0,0 +1,79 @@
+#!/usr/bin/env bash
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+#
+#
+# Given a lun # and a WWID for a volume provisioned externally, find the volume
+# through the SCSI bus and make sure its visable via multipath
Review Comment:
Correct the spelling 'visable' to 'visible' in the comment.
```suggestion
# through the SCSI bus and make sure its visible via multipath
```
##
scripts/storage/multipath/connectVolume.sh:
##
@@ -29,103 +29,40 @@ WWID=${2:?"WWID required"}
WWID=$(echo $WWID | tr '[:upper:]' '[:lower:]')
-systemctl is-active multipathd || systemctl restart multipathd || {
- echo "$(date): Multipathd is NOT running and cannot be started. This must
be corrected before this host can access this storage volume."
- logger -t "CS_SCSI_VOL_FIND" "${WWID} cannot be mapped to this host because
multipathd is not currently running and cannot be started"
+START_CONNECT=$(dirname $0)/startConnect.sh
+if [ -x "${START_CONNECT}" ]; then
+ echo "$(date): Starting connect process for ${WWID} on lun ${LUN}"
+ ${START_CONNECT} ${LUN} ${WWID}
+ if [ $? -ne 0 ]; then
+ echo "$(date): Failed to start connect process for ${WWID} on lun ${LUN}"
+ logger -t "CS_SCSI_VOL_FIND" "${WWID} failed to start connect process on
lun ${LUN}"
+ exit 1
+ fi
+else
+ echo "$(date): Unable to find startConnect.sh script!"
exit 1
-}
-
-echo "$(date): Looking for ${WWID} on lun ${LUN}"
-
-# get vendor OUI. we will only delete a device on the designated lun if it
matches the
-# incoming WWN OUI value. This is because multiple storage arrays may be
mapped to the
-# host on different fiber channel hosts with the same LUN
-INCOMING_OUI=$(echo ${WWID} | cut -c2-7)
-echo "$(date): Incoming OUI: ${INCOMING_OUI}"
-
-# first we need to check if any stray references are left from a previous use
of this lun
-for fchost in $(ls /sys/class/fc_host | sed -e 's/host//g'); do
- lingering_devs=$(lsscsi -w "${fchost}:*:*:${LUN}" | grep /dev | awk '{if
(NF > 6) { printf("%s:%s ", $NF, $(NF-1));} }' | sed -e 's/0x/3/g')
+fi
- if [ ! -z "${lingering_devs}" ]; then
- for dev in ${lingering_devs}; do
- LSSCSI_WWID=$(echo $dev | awk -F: '{print $2}' | sed -e 's/0x/3/g')
- FOUND_OUI=$(echo ${LSSCSI_WWID} | cut -c3-8)
- if [ "${INCOMING_OUI}" != "${FOUND_OUI}" ]; then
- continue;
- fi
- dev=$(echo $dev | awk -F: '{ print $1}')
- logger -t "CS_SCSI_VOL_FIND" "${WWID} processing identified a lingering
device ${dev} from previous lun use, attempting to clean up"
- MP_WWID=$(multipath -l ${dev} | head -1 | awk '{print $1}')
- MP_WWID=${MP_WWID:1} # strip first character (3) off
- # don't do this if the WWID passed in matches the WWID from multipath
- if [ ! -z "${MP_WWID}" ] && [ "${MP_WWID}" != "${WWID}" ]; then
- # run full removal again so all devices and multimap are cleared
- $(dirname $0)/disconnectVolume.sh ${MP_WWID}
- # we don't have a multimap but we may still have some stranded devices
to clean up
- elif [ "${LSSCSI_WWID}" != "${WWID}" ]; then
- echo "1" > /sys/block/$(echo ${dev} | awk -F'/' '{print
$NF}')/device/delete
- fi
- done
- sleep 3
- fi
+// wait for the device path to show up
+while [ ! -e /dev/mapper/3${WWID} ]; do
+ echo "$(date): Waiting for /dev/mapper/3${WWID} to appear"
+ sleep
Review Comment:
The 'sleep' command is missing an argument; consider adding a sleep duration
(e.g., 'sleep 1') to avoid a busy loop.
```suggestion
sleep 1
```
##
scripts/storage/multipath/startConnectVolume.sh:
##
@@ -0,0 +1,101 @@
+#!/usr/bin/env bash
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses
