Skip to content

fix: correctly handle NFS metalink templates - mount export dir, compute real file size #13934

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

Closed
waterWang wants to merge 2 commits into from
Closed

fix: correctly handle NFS metalink templates - mount export dir, compute real file size #13934

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3354408
fix: correctly handle NFS metalink template downloads and remote file…
waterWang Aug 20, 2026
3b9a32e
fix: use correct downloader type per URL in MetalinkDirectTemplateDow…
waterWang Aug 20, 2026
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
7 changes: 4 additions & 3 deletions ...src/main/java/org/apache/cloudstack/direct/download/MetalinkDirectTemplateDownloader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,9 @@ public Long getRemoteFileSize(String metalinkUrl, String format) {
if (url.endsWith("torrent")) {
continue;
}
if (downloader.checkUrl(url)) {
return downloader.getRemoteFileSize(url, format);
DirectTemplateDownloader urlDownloader = createDownloaderForMetalinks(url, null, null, null, headers, connectTimeout, soTimeout, null, null);
if (urlDownloader != null && urlDownloader.checkUrl(url)) {
return urlDownloader.getRemoteFileSize(url, format);

@Damans227 Damans227 Oct 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

is this reached on the url check? a metalink link starting with http gets the plain http checker, so the size is still read from the metalink file itself. thats likely why kiran still sees 6580590 TB

}
}
return null;
Expand All @@ -171,4 +172,4 @@ public List<String> getMetalinkChecksums(String metalinkUrl) {
return downloader.getMetalinkChecksums(metalinkUrl);
}

}
}
60 changes: 53 additions & 7 deletions core/src/main/java/org/apache/cloudstack/direct/download/NfsDirectTemplateDownloader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,12 @@
import com.cloud.utils.UriUtils;
import com.cloud.utils.exception.CloudRuntimeException;
import com.cloud.utils.script.Script;
import com.cloud.utils.storage.QCOW2Utils;

import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.List;
Expand All @@ -33,11 +37,13 @@ public class NfsDirectTemplateDownloader extends DirectTemplateDownloaderImpl {

private String srcHost;
private String srcPath;
private String fileName;

private static final String mountCommand = "mount -t nfs %s %s";

/**
* Parse url and set srcHost and srcPath
* Parse NFS URL and split the path into the export directory (mountable) and file name.
* For example, nfs://host/export/templates/file.qcow2 -> srcPath=/export/templates, fileName=file.qcow2
*/
private void parseUrl() {
URI uri = null;
Expand All @@ -47,6 +53,16 @@ private void parseUrl() {
if (uri.getScheme() != null && uri.getScheme().equalsIgnoreCase("nfs")) {
srcHost = uri.getHost();
srcPath = uri.getPath();
if (srcPath != null) {
int lastSlash = srcPath.lastIndexOf('/');
if (lastSlash >= 0) {
fileName = srcPath.substring(lastSlash + 1);
srcPath = srcPath.substring(0, lastSlash);
}
}
if (srcPath == null || srcPath.isEmpty()) {
srcPath = "/";
}
}
} catch (URISyntaxException e) {
throw new CloudRuntimeException("Invalid NFS url " + url + " caused error: " + e.getMessage());
Expand All @@ -66,12 +82,18 @@ public NfsDirectTemplateDownloader(String url, String destPool, Long templateId,
@Override
public Pair<Boolean, String> downloadTemplate() {
String mountSrcUuid = UUID.randomUUID().toString();
String mount = String.format(mountCommand, srcHost + ":" + srcPath, "/mnt/" + mountSrcUuid);
String mountPoint = "/mnt/" + mountSrcUuid;
String mount = String.format(mountCommand, srcHost + ":" + srcPath, mountPoint);
Script.runSimpleBashScript(mount);

@Damans227 Damans227 Oct 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

should this create the mount folder first like the size check does? kirans log shows the mount failing with "mount point does not exist"

String downloadDir = getDestPoolPath() + File.separator + getDirectDownloadTempPath(getTemplateId());

@Damans227 Damans227 Oct 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

what happens when this runs from a metalink? the nfs downloader there is made without the pool or template id, so the copy goes to null/null/template/n/null

setDownloadedFilePath(downloadDir + File.separator + getTemporaryFileName());
Script.runSimpleBashScript("cp /mnt/" + mountSrcUuid + srcPath + " " + getDownloadedFilePath());
Script.runSimpleBashScript("umount /mnt/" + mountSrcUuid);
String destPath = downloadDir + File.separator + getTemporaryFileName();
setDownloadedFilePath(destPath);
if (fileName != null && !fileName.isEmpty()) {
Script.runSimpleBashScript("cp " + mountPoint + "/" + fileName + " " + destPath);
} else {
Script.runSimpleBashScript("cp " + mountPoint + " " + destPath);
}
Script.runSimpleBashScript("umount " + mountPoint);
return new Pair<>(true, getDownloadedFilePath());
}

Expand All @@ -88,7 +110,31 @@ public boolean checkUrl(String url) {

@Override
public Long getRemoteFileSize(String url, String format) {
return null;
String mountSrcUuid = UUID.randomUUID().toString();
String mountPoint = "/mnt/" + mountSrcUuid;
Script.runSimpleBashScript("mkdir -p " + mountPoint);
Script.runSimpleBashScript(String.format(mountCommand, srcHost + ":" + srcPath, mountPoint));
try {
File file = new File(mountPoint + "/" + (fileName != null ? fileName : ""));
if (!file.exists()) {
logger.error(String.format("File not found on NFS mount: %s", file.getAbsolutePath()));
return null;
}
if ("qcow2".equalsIgnoreCase(format) && fileName != null && !fileName.isEmpty()) {
try (InputStream is = new FileInputStream(file)) {
return QCOW2Utils.getVirtualSize(is, false);
} catch (IOException e) {
logger.warn(String.format("Could not read qcow2 virtual size for NFS file %s, falling back to file length: %s", url, e.getMessage()));
}
}
return file.length();
} catch (Exception e) {
logger.error(String.format("Could not get remote file size for NFS URL: %s due to: %s", url, e.getMessage()), e);
return null;
} finally {
Script.runSimpleBashScript("umount -l " + mountPoint + " 2>/dev/null");
Script.runSimpleBashScript("rmdir " + mountPoint + " 2>/dev/null");
}
}

@Override
Expand All @@ -100,4 +146,4 @@ public List<String> getMetalinkUrls(String metalinkUrl) {
public List<String> getMetalinkChecksums(String metalinkUrl) {
return null;
}
}
}
Loading