Skip to content

Linstor fix migration while node offline #8610

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
yadvr merged 3 commits into from
Feb 8, 2024
Merged

Linstor fix migration while node offline #8610

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6377e9d
linstor: Add util method getBestErrorMessage from main
rp- Feb 5, 2024
f176e7d
linstor: failed remove of allow-two-primaries is no fatal error
rp- Dec 27, 2023
1f6954c
linstor: Fix failure if a Linstor node is down while migrating
rp- Feb 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 19 additions & 9 deletions .../volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,27 +269,35 @@ public boolean connectPhysicalDisk(String volumePath, KVMStoragePool pool, Map<S
}

final DevelopersApi api = getLinstorAPI(pool);
String rscName;
try
{
final String rscName = getLinstorRscName(volumePath);
rscName = getLinstorRscName(volumePath);

ResourceMakeAvailable rma = new ResourceMakeAvailable();
ApiCallRcList answers = api.resourceMakeAvailableOnNode(rscName, localNodeName, rma);
checkLinstorAnswersThrow(answers);

} catch (ApiException apiEx) {
s_logger.error(apiEx);
throw new CloudRuntimeException(apiEx.getBestMessage(), apiEx);
}

try
{
// allow 2 primaries for live migration, should be removed by disconnect on the other end
ResourceDefinitionModify rdm = new ResourceDefinitionModify();
Properties props = new Properties();
props.put("DrbdOptions/Net/allow-two-primaries", "yes");
rdm.setOverrideProps(props);
answers = api.resourceDefinitionModify(rscName, rdm);
ApiCallRcList answers = api.resourceDefinitionModify(rscName, rdm);
if (answers.hasError()) {
s_logger.error("Unable to set 'allow-two-primaries' on " + rscName);
throw new CloudRuntimeException(answers.get(0).getMessage());
// do not fail here as adding allow-two-primaries property is only a problem while live migrating
}
} catch (ApiException apiEx) {
s_logger.error(apiEx);
throw new CloudRuntimeException(apiEx.getBestMessage(), apiEx);
// do not fail here as adding allow-two-primaries property is only a problem while live migrating
}
return true;
}
Expand Down Expand Up @@ -353,19 +361,21 @@ public boolean disconnectPhysicalDiskByPath(String localPath)
ApiCallRcList answers = api.resourceDefinitionModify(rsc.get().getName(), rdm);
if (answers.hasError())
{
s_logger.error("Failed to remove 'allow-two-primaries' on " + rsc.get().getName());
throw new CloudRuntimeException(answers.get(0).getMessage());
s_logger.error(
String.format("Failed to remove 'allow-two-primaries' on %s: %s",
rsc.get().getName(), LinstorUtil.getBestErrorMessage(answers)));
// do not fail here as removing allow-two-primaries property isn't fatal
}

return true;
}
s_logger.warn("Linstor: Couldn't find resource for this path: " + localPath);
} catch (ApiException apiEx) {
s_logger.error(apiEx);
throw new CloudRuntimeException(apiEx.getBestMessage(), apiEx);
s_logger.error(apiEx.getBestMessage());
// do not fail here as removing allow-two-primaries property isn't fatal
}
}
return false;
Copy link
Copy Markdown
Contributor

@sureshanaparti sureshanaparti Feb 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rp- any case returns false (on error / exception above ?)

Copy link
Copy Markdown
Contributor Author

@rp- rp- Feb 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIS every upper caller is ignoring the return value anyway
But we don't need to do any disconnect, it is just a bit of a tidy up to remove the property and nothing fatal if it fails.

return true;
}

@Override
Expand Down
11 changes: 11 additions & 0 deletions ...olume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/util/LinstorUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
import com.linbit.linstor.api.ApiException;
import com.linbit.linstor.api.Configuration;
import com.linbit.linstor.api.DevelopersApi;
import com.linbit.linstor.api.model.ApiCallRc;
import com.linbit.linstor.api.model.ApiCallRcList;
import com.linbit.linstor.api.model.ProviderKind;
import com.linbit.linstor.api.model.ResourceGroup;
import com.linbit.linstor.api.model.StoragePool;
Expand Down Expand Up @@ -47,6 +49,15 @@ public static DevelopersApi getLinstorAPI(String linstorUrl) {
return new DevelopersApi(client);
}

public static String getBestErrorMessage(ApiCallRcList answers) {
return answers != null && !answers.isEmpty() ?
answers.stream()
.filter(ApiCallRc::isError)
.findFirst()
.map(ApiCallRc::getMessage)
.orElse((answers.get(0)).getMessage()) : null;
}

public static long getCapacityBytes(String linstorUrl, String rscGroupName) {
DevelopersApi linstorApi = getLinstorAPI(linstorUrl);
try {
Expand Down