DaanHoogland commented on code in PR #9832:
URL: https://github.com/apache/cloudstack/pull/9832#discussion_r1820458545
##########
plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java:
##########
@@ -268,17 +270,33 @@ private void allow2PrimariesIfInUse(DevelopersApi api,
String rscName) throws Ap
String inUseNode = LinstorUtil.isResourceInUse(api, rscName);
if (inUseNode != null && !inUseNode.equalsIgnoreCase(localNodeName)) {
// allow 2 primaries for live migration, should be removed by
disconnect on the other end
- ResourceConnectionModify rcm = new ResourceConnectionModify();
- Properties props = new Properties();
- props.put("DrbdOptions/Net/allow-two-primaries", "yes");
- props.put("DrbdOptions/Net/protocol", "C");
- rcm.setOverrideProps(props);
- ApiCallRcList answers = api.resourceConnectionModify(rscName,
inUseNode, localNodeName, rcm);
- if (answers.hasError()) {
- s_logger.error(String.format(
- "Unable to set protocol C and 'allow-two-primaries' on
%s/%s/%s",
- inUseNode, localNodeName, rscName));
- // do not fail here as adding allow-two-primaries property is
only a problem while live migrating
+
+ // if non hyperconverged setup, we have to set allow-two-primaries
on the resource-definition
+ // as there is no resource connection between diskless nodes.
+ if (LinstorUtil.areResourcesDiskless(api, rscName,
Arrays.asList(inUseNode, localNodeName))) {
+ ResourceDefinitionModify rdm = new ResourceDefinitionModify();
+ Properties props = new Properties();
+ props.put("DrbdOptions/Net/allow-two-primaries", "yes");
+ props.put("DrbdOptions/Net/protocol", "C");
+ rdm.setOverrideProps(props);
+ ApiCallRcList answers = api.resourceDefinitionModify(rscName,
rdm);
+ if (answers.hasError()) {
+ s_logger.error(String.format("Unable to set protocol C and
'allow-two-primaries' on %s", rscName));
+ // do not fail here as adding allow-two-primaries property
is only a problem while live migrating
+ }
Review Comment:
method?
##########
plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java:
##########
@@ -268,17 +270,33 @@ private void allow2PrimariesIfInUse(DevelopersApi api,
String rscName) throws Ap
String inUseNode = LinstorUtil.isResourceInUse(api, rscName);
if (inUseNode != null && !inUseNode.equalsIgnoreCase(localNodeName)) {
// allow 2 primaries for live migration, should be removed by
disconnect on the other end
- ResourceConnectionModify rcm = new ResourceConnectionModify();
- Properties props = new Properties();
- props.put("DrbdOptions/Net/allow-two-primaries", "yes");
- props.put("DrbdOptions/Net/protocol", "C");
- rcm.setOverrideProps(props);
- ApiCallRcList answers = api.resourceConnectionModify(rscName,
inUseNode, localNodeName, rcm);
- if (answers.hasError()) {
- s_logger.error(String.format(
- "Unable to set protocol C and 'allow-two-primaries' on
%s/%s/%s",
- inUseNode, localNodeName, rscName));
- // do not fail here as adding allow-two-primaries property is
only a problem while live migrating
+
+ // if non hyperconverged setup, we have to set allow-two-primaries
on the resource-definition
+ // as there is no resource connection between diskless nodes.
+ if (LinstorUtil.areResourcesDiskless(api, rscName,
Arrays.asList(inUseNode, localNodeName))) {
+ ResourceDefinitionModify rdm = new ResourceDefinitionModify();
+ Properties props = new Properties();
+ props.put("DrbdOptions/Net/allow-two-primaries", "yes");
+ props.put("DrbdOptions/Net/protocol", "C");
+ rdm.setOverrideProps(props);
+ ApiCallRcList answers = api.resourceDefinitionModify(rscName,
rdm);
+ if (answers.hasError()) {
+ s_logger.error(String.format("Unable to set protocol C and
'allow-two-primaries' on %s", rscName));
+ // do not fail here as adding allow-two-primaries property
is only a problem while live migrating
+ }
+ } else {
+ ResourceConnectionModify rcm = new ResourceConnectionModify();
+ Properties props = new Properties();
+ props.put("DrbdOptions/Net/allow-two-primaries", "yes");
+ props.put("DrbdOptions/Net/protocol", "C");
+ rcm.setOverrideProps(props);
+ ApiCallRcList answers = api.resourceConnectionModify(rscName,
inUseNode, localNodeName, rcm);
+ if (answers.hasError()) {
+ s_logger.error(String.format(
+ "Unable to set protocol C and
'allow-two-primaries' on %s/%s/%s",
+ inUseNode, localNodeName, rscName));
+ // do not fail here as adding allow-two-primaries property
is only a problem while live migrating
+ }
Review Comment:
method?
##########
plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java:
##########
@@ -318,19 +336,34 @@ public boolean connectPhysicalDisk(String volumePath,
KVMStoragePool pool, Map<S
}
private void removeTwoPrimariesRcProps(DevelopersApi api, String
inUseNode, String rscName) throws ApiException {
- ResourceConnectionModify rcm = new ResourceConnectionModify();
List<String> deleteProps = new ArrayList<>();
deleteProps.add("DrbdOptions/Net/allow-two-primaries");
deleteProps.add("DrbdOptions/Net/protocol");
- rcm.deleteProps(deleteProps);
- ApiCallRcList answers = api.resourceConnectionModify(rscName,
localNodeName, inUseNode, rcm);
- if (answers.hasError()) {
- s_logger.error(
- String.format("Failed to remove 'protocol' and
'allow-two-primaries' on %s/%s/%s: %s",
- localNodeName,
- inUseNode,
- rscName,
LinstorUtil.getBestErrorMessage(answers)));
- // do not fail here as removing allow-two-primaries property isn't
fatal
+
+ {
+ ResourceDefinitionModify rdm = new ResourceDefinitionModify();
+ rdm.deleteProps(deleteProps);
+ ApiCallRcList answers = api.resourceDefinitionModify(rscName, rdm);
+ if (answers.hasError()) {
+ s_logger.error(
+ String.format("Failed to remove 'protocol' and
'allow-two-primaries' on %s: %s",
+ rscName,
LinstorUtil.getBestErrorMessage(answers)));
+ // do not fail here as removing allow-two-primaries property
isn't fatal
+ }
+ }
+
+ {
+ ResourceConnectionModify rcm = new ResourceConnectionModify();
+ rcm.deleteProps(deleteProps);
+ ApiCallRcList answers = api.resourceConnectionModify(rscName,
localNodeName, inUseNode, rcm);
+ if (answers.hasError()) {
+ s_logger.error(
+ String.format("Failed to remove 'protocol' and
'allow-two-primaries' on %s/%s/%s: %s",
+ localNodeName,
+ inUseNode,
+ rscName,
LinstorUtil.getBestErrorMessage(answers)));
+ // do not fail here as removing allow-two-primaries property
isn't fatal
+ }
}
Review Comment:
why these blocks? should these be methods?
--
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]
