From 3e92a631558cecc63f968a0af58b0c788a039fce Mon Sep 17 00:00:00 2001 From: Nicolas Vazquez Date: Mon, 31 Jan 2022 03:51:13 -0300 Subject: [PATCH 1/7] [XenServer/XCP-ng] Pass the image store NFS version on storage commands (#5886) * Add NFS version to mount command * Remove extra line * Extend NFS version to mount secondary storage * Unused import * Refactor NFS version to be granular * Make use of the ConfigKey on the NFS version setting value --- .../CopyToSecondaryStorageCommand.java | 8 ++- .../image/NfsImageStoreDriverImpl.java | 22 +------- .../resource/CitrixResourceBase.java | 19 ++++--- .../resource/XenServerStorageProcessor.java | 18 +++--- .../Xenserver625StorageProcessor.java | 55 +++++++++++-------- .../hypervisor/xenserver/cloud-plugin-storage | 4 ++ scripts/vm/hypervisor/xenserver/vmopsSnapshot | 13 +++-- .../diagnostics/DiagnosticsServiceImpl.java | 4 +- 8 files changed, 79 insertions(+), 64 deletions(-) diff --git a/core/src/main/java/org/apache/cloudstack/diagnostics/CopyToSecondaryStorageCommand.java b/core/src/main/java/org/apache/cloudstack/diagnostics/CopyToSecondaryStorageCommand.java index 8e76aad580f..5cd4991c287 100644 --- a/core/src/main/java/org/apache/cloudstack/diagnostics/CopyToSecondaryStorageCommand.java +++ b/core/src/main/java/org/apache/cloudstack/diagnostics/CopyToSecondaryStorageCommand.java @@ -22,11 +22,13 @@ public class CopyToSecondaryStorageCommand extends StorageSubSystemCommand { private String secondaryStorageUrl; private String systemVmIp; private String fileName; + private String nfsVersion; - public CopyToSecondaryStorageCommand(String secondaryStorageUrl, String systemVmIp, String fileName) { + public CopyToSecondaryStorageCommand(String secondaryStorageUrl, String systemVmIp, String fileName, String nfsVersion) { this.secondaryStorageUrl = secondaryStorageUrl; this.systemVmIp = systemVmIp; this.fileName = fileName; + this.nfsVersion = nfsVersion; } public String getSecondaryStorageUrl() { @@ -41,6 +43,10 @@ public class CopyToSecondaryStorageCommand extends StorageSubSystemCommand { return fileName; } + public String getNfsVersion() { + return nfsVersion; + } + @Override public boolean executeInSequence() { return false; diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/image/NfsImageStoreDriverImpl.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/image/NfsImageStoreDriverImpl.java index aea792211dd..6e5c979f111 100755 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/image/NfsImageStoreDriverImpl.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/image/NfsImageStoreDriverImpl.java @@ -18,31 +18,15 @@ */ package org.apache.cloudstack.storage.image; -import java.util.Map; - -import javax.inject.Inject; - -import org.apache.cloudstack.storage.datastore.db.ImageStoreDetailsDao; - import com.cloud.capacity.CapacityManager; public abstract class NfsImageStoreDriverImpl extends BaseImageStoreDriverImpl { - @Inject - ImageStoreDetailsDao _imageStoreDetailsDao; - /** - * Retrieve NFS version to be used for imgStoreId store, if provided in image_store_details table - * @param imgStoreId store id - * @return "secstorage.nfs.version" associated value for imgStoreId in image_store_details table if exists, null if not + * Retrieve the NFS version to be used for the imgStoreId store */ - protected String getNfsVersion(long imgStoreId){ - Map imgStoreDetails = _imageStoreDetailsDao.getDetails(imgStoreId); - String nfsVersionKey = CapacityManager.ImageStoreNFSVersion.key(); - if (imgStoreDetails != null && imgStoreDetails.containsKey(nfsVersionKey)){ - return imgStoreDetails.get(nfsVersionKey); - } - return null; + protected String getNfsVersion(long imgStoreId) { + return CapacityManager.ImageStoreNFSVersion.valueIn(imgStoreId); } } diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java index a9715765bf7..9348bcaab9f 100644 --- a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java +++ b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java @@ -1110,8 +1110,8 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe return cdromVBD; } - protected boolean createSecondaryStorageFolder(final Connection conn, final String remoteMountPath, final String newFolder) { - final String result = callHostPlugin(conn, "vmopsSnapshot", "create_secondary_storage_folder", "remoteMountPath", remoteMountPath, "newFolder", newFolder); + protected boolean createSecondaryStorageFolder(final Connection conn, final String remoteMountPath, final String newFolder, final String nfsVersion) { + final String result = callHostPlugin(conn, "vmopsSnapshot", "create_secondary_storage_folder", "remoteMountPath", remoteMountPath, "newFolder", newFolder, "nfsVersion", nfsVersion); return result != null; } @@ -1482,8 +1482,8 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe return vm; } - protected boolean deleteSecondaryStorageFolder(final Connection conn, final String remoteMountPath, final String folder) { - final String details = callHostPlugin(conn, "vmopsSnapshot", "delete_secondary_storage_folder", "remoteMountPath", remoteMountPath, "folder", folder); + protected boolean deleteSecondaryStorageFolder(final Connection conn, final String remoteMountPath, final String folder, final String nfsVersion) { + final String details = callHostPlugin(conn, "vmopsSnapshot", "delete_secondary_storage_folder", "remoteMountPath", remoteMountPath, "folder", folder, "nfsVersion", nfsVersion); return details != null && details.equals("1"); } @@ -4102,7 +4102,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe } protected boolean postCreatePrivateTemplate(final Connection conn, final String templatePath, final String tmpltFilename, final String templateName, String templateDescription, String checksum, - final long size, final long virtualSize, final long templateId) { + final long size, final long virtualSize, final long templateId, final String nfsVersion) { if (templateDescription == null) { templateDescription = ""; @@ -4113,7 +4113,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe } final String result = callHostPlugin(conn, "vmopsSnapshot", "post_create_private_template", "templatePath", templatePath, "templateFilename", tmpltFilename, "templateName", templateName, - "templateDescription", templateDescription, "checksum", checksum, "size", String.valueOf(size), "virtualSize", String.valueOf(virtualSize), "templateId", String.valueOf(templateId)); + "templateDescription", templateDescription, "checksum", checksum, "size", String.valueOf(size), "virtualSize", String.valueOf(virtualSize), "templateId", String.valueOf(templateId), "nfsVersion", nfsVersion); boolean success = false; if (result != null && !result.isEmpty()) { @@ -5661,6 +5661,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe String secondaryStorageUrl = cmd.getSecondaryStorageUrl(); String vmIP = cmd.getSystemVmIp(); String diagnosticsZipFile = cmd.getFileName(); + String nfsVersion = cmd.getNfsVersion(); String localDir = null; boolean success; @@ -5671,7 +5672,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe URI uri = new URI(secondaryStorageUrl); secondaryStorageMountPath = uri.getHost() + ":" + uri.getPath(); localDir = BASE_MOUNT_POINT_ON_REMOTE + UUID.nameUUIDFromBytes(secondaryStorageMountPath.getBytes()); - String mountPoint = mountNfs(conn, secondaryStorageMountPath, localDir); + String mountPoint = mountNfs(conn, secondaryStorageMountPath, localDir, nfsVersion); if (org.apache.commons.lang.StringUtils.isBlank(mountPoint)) { return new CopyToSecondaryStorageAnswer(cmd, false, "Could not mount secondary storage " + secondaryStorageMountPath + " on host " + localDir); } @@ -5698,11 +5699,11 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe } } - private String mountNfs(Connection conn, String remoteDir, String localDir) { + private String mountNfs(Connection conn, String remoteDir, String localDir, String nfsVersion) { if (localDir == null) { localDir = BASE_MOUNT_POINT_ON_REMOTE + UUID.nameUUIDFromBytes(remoteDir.getBytes()); } - return callHostPlugin(conn, "cloud-plugin-storage", "mountNfsSecondaryStorage", "localDir", localDir, "remoteDir", remoteDir); + return callHostPlugin(conn, "cloud-plugin-storage", "mountNfsSecondaryStorage", "localDir", localDir, "remoteDir", remoteDir, "nfsVersion", nfsVersion); } // Unmount secondary storage from host diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/XenServerStorageProcessor.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/XenServerStorageProcessor.java index a035eac30fb..69f60c5f543 100644 --- a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/XenServerStorageProcessor.java +++ b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/XenServerStorageProcessor.java @@ -916,8 +916,9 @@ public class XenServerStorageProcessor implements StorageProcessor { try { final NfsTO nfsStore = (NfsTO) destStore; final URI uri = new URI(nfsStore.getUrl()); + final String nfsVersion = nfsStore.getNfsVersion(); // Create the volume folder - if (!hypervisorResource.createSecondaryStorageFolder(conn, uri.getHost() + ":" + uri.getPath(), destVolume.getPath())) { + if (!hypervisorResource.createSecondaryStorageFolder(conn, uri.getHost() + ":" + uri.getPath(), destVolume.getPath(), nfsVersion)) { throw new InternalErrorException("Failed to create the volume folder."); } @@ -1179,6 +1180,7 @@ public class XenServerStorageProcessor implements StorageProcessor { secondaryStorageUrl = cacheStore.getUrl(); destPath = destData.getPath(); } + String nfsVersion = cacheStore.getNfsVersion(); final SnapshotObjectTO snapshotTO = (SnapshotObjectTO) srcData; final SnapshotObjectTO snapshotOnImage = (SnapshotObjectTO) destData; @@ -1235,7 +1237,7 @@ public class XenServerStorageProcessor implements StorageProcessor { if (fullbackup) { // the first snapshot is always a full snapshot - if (!hypervisorResource.createSecondaryStorageFolder(conn, secondaryStorageMountPath, folder)) { + if (!hypervisorResource.createSecondaryStorageFolder(conn, secondaryStorageMountPath, folder, nfsVersion)) { details = " Filed to create folder " + folder + " in secondary storage"; s_logger.warn(details); return new CopyCmdAnswer(details); @@ -1349,6 +1351,7 @@ public class XenServerStorageProcessor implements StorageProcessor { final TemplateObjectTO template = (TemplateObjectTO) cmd.getDestTO(); final NfsTO destStore = (NfsTO) cmd.getDestTO().getDataStore(); final int wait = cmd.getWait(); + final String nfsVersion = destStore.getNfsVersion(); final String secondaryStoragePoolURL = destStore.getUrl(); final String volumeUUID = volume.getPath(); @@ -1364,7 +1367,7 @@ public class XenServerStorageProcessor implements StorageProcessor { final URI uri = new URI(secondaryStoragePoolURL); secondaryStorageMountPath = uri.getHost() + ":" + uri.getPath(); installPath = template.getPath(); - if (!hypervisorResource.createSecondaryStorageFolder(conn, secondaryStorageMountPath, installPath)) { + if (!hypervisorResource.createSecondaryStorageFolder(conn, secondaryStorageMountPath, installPath, nfsVersion)) { details = " Filed to create folder " + installPath + " in secondary storage"; s_logger.warn(details); return new CopyCmdAnswer(details); @@ -1391,7 +1394,7 @@ public class XenServerStorageProcessor implements StorageProcessor { final String templatePath = secondaryStorageMountPath + "/" + installPath; result = hypervisorResource.postCreatePrivateTemplate(conn, templatePath, tmpltFilename, tmpltUUID, userSpecifiedName, null, physicalSize, virtualSize, - template.getId()); + template.getId(), nfsVersion); if (!result) { throw new CloudRuntimeException("Could not create the template.properties file on secondary storage dir: " + tmpltURI); } @@ -1411,7 +1414,7 @@ public class XenServerStorageProcessor implements StorageProcessor { hypervisorResource.removeSR(conn, tmpltSR); } if (secondaryStorageMountPath != null) { - hypervisorResource.deleteSecondaryStorageFolder(conn, secondaryStorageMountPath, installPath); + hypervisorResource.deleteSecondaryStorageFolder(conn, secondaryStorageMountPath, installPath, nfsVersion); } details = "Creating template from volume " + volumeUUID + " failed due to " + e.toString(); s_logger.error(details, e); @@ -1465,7 +1468,8 @@ public class XenServerStorageProcessor implements StorageProcessor { final String destNfsPath = destUri.getHost() + ":" + destUri.getPath(); - if (!hypervisorResource.createSecondaryStorageFolder(conn, destNfsPath, destDir)) { + String destNfsVersion = destStore.getNfsVersion(); + if (!hypervisorResource.createSecondaryStorageFolder(conn, destNfsPath, destDir, destNfsVersion)) { final String details = " Failed to create folder " + destDir + " in secondary storage"; s_logger.warn(details); @@ -1500,7 +1504,7 @@ public class XenServerStorageProcessor implements StorageProcessor { templatePath = templatePath.replaceAll("//", "/"); result = hypervisorResource.postCreatePrivateTemplate(conn, templatePath, templateFilename, templateUuid, userSpecifiedTemplateName, null, - physicalSize, virtualSize, templateObjTO.getId()); + physicalSize, virtualSize, templateObjTO.getId(), destNfsVersion); if (!result) { throw new CloudRuntimeException("Could not create the template.properties file on secondary storage dir: " + templateUri); diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/Xenserver625StorageProcessor.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/Xenserver625StorageProcessor.java index 7e2c701e51a..a31b597f89b 100644 --- a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/Xenserver625StorageProcessor.java +++ b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/Xenserver625StorageProcessor.java @@ -73,11 +73,11 @@ public class Xenserver625StorageProcessor extends XenServerStorageProcessor { super(resource); } - private void mountNfs(Connection conn, String remoteDir, String localDir) { + private void mountNfs(Connection conn, String remoteDir, String localDir, String nfsVersion) { if (localDir == null) { localDir = BASE_MOUNT_POINT_ON_REMOTE + UUID.nameUUIDFromBytes(remoteDir.getBytes()); } - String result = hypervisorResource.callHostPluginAsync(conn, "cloud-plugin-storage", "mountNfsSecondaryStorage", 100 * 1000, "localDir", localDir, "remoteDir", remoteDir); + String result = hypervisorResource.callHostPluginAsync(conn, "cloud-plugin-storage", "mountNfsSecondaryStorage", 100 * 1000, "localDir", localDir, "remoteDir", remoteDir, "nfsVersion", nfsVersion); if (StringUtils.isBlank(result)) { String errMsg = "Could not mount secondary storage " + remoteDir + " on host " + localDir; s_logger.warn(errMsg); @@ -244,9 +244,9 @@ public class Xenserver625StorageProcessor extends XenServerStorageProcessor { } } - protected SR createFileSr(Connection conn, String remotePath, String dir) { + protected SR createFileSr(Connection conn, String remotePath, String dir, String nfsVersion) { String localDir = BASE_MOUNT_POINT_ON_REMOTE + UUID.nameUUIDFromBytes(remotePath.getBytes()); - mountNfs(conn, remotePath, localDir); + mountNfs(conn, remotePath, localDir, nfsVersion); return createFileSR(conn, localDir + "/" + dir); } @@ -269,6 +269,7 @@ public class Xenserver625StorageProcessor extends XenServerStorageProcessor { final String storeUrl = srcImageStore.getUrl(); final URI uri = new URI(storeUrl); String volumePath = srcData.getPath(); + String nfsVersion = srcImageStore.getNfsVersion(); volumePath = StringUtils.stripEnd(volumePath, "/"); @@ -282,7 +283,7 @@ public class Xenserver625StorageProcessor extends XenServerStorageProcessor { volumeDirectory = volumePath.substring(0, index); } - srcSr = createFileSr(conn, uri.getHost() + ":" + uri.getPath(), volumeDirectory); + srcSr = createFileSr(conn, uri.getHost() + ":" + uri.getPath(), volumeDirectory, nfsVersion); final Set setVdis = srcSr.getVDIs(conn); @@ -416,7 +417,7 @@ public class Xenserver625StorageProcessor extends XenServerStorageProcessor { } protected String backupSnapshot(final Connection conn, final String primaryStorageSRUuid, final String localMountPoint, final String path, final String secondaryStorageMountPath, - final String snapshotUuid, String prevBackupUuid, final String prevSnapshotUuid, final Boolean isISCSI, int wait) { + final String snapshotUuid, String prevBackupUuid, final String prevSnapshotUuid, final Boolean isISCSI, int wait, String nfsVersion) { boolean filesrcreated = false; // boolean copied = false; @@ -427,7 +428,7 @@ public class Xenserver625StorageProcessor extends XenServerStorageProcessor { final String remoteDir = secondaryStorageMountPath; try { - ssSR = createFileSr(conn, remoteDir, path); + ssSR = createFileSr(conn, remoteDir, path, nfsVersion); filesrcreated = true; final VDI snapshotvdi = VDI.getByUuid(conn, snapshotUuid); @@ -509,6 +510,7 @@ public class Xenserver625StorageProcessor extends XenServerStorageProcessor { secondaryStorageUrl = cacheStore.getUrl(); destPath = destData.getPath(); } + String nfsVersion = cacheStore != null ? cacheStore.getNfsVersion() : null; final SnapshotObjectTO snapshotTO = (SnapshotObjectTO)srcData; final SnapshotObjectTO snapshotOnImage = (SnapshotObjectTO)destData; @@ -569,7 +571,7 @@ public class Xenserver625StorageProcessor extends XenServerStorageProcessor { Task task = null; try { final String localDir = BASE_MOUNT_POINT_ON_REMOTE + UUID.nameUUIDFromBytes(secondaryStorageMountPath.getBytes()); - mountNfs(conn, secondaryStorageMountPath, localDir); + mountNfs(conn, secondaryStorageMountPath, localDir, nfsVersion); final boolean result = makeDirectory(conn, localDir + "/" + folder); if (!result) { details = " Failed to create folder " + folder + " in secondary storage"; @@ -577,7 +579,7 @@ public class Xenserver625StorageProcessor extends XenServerStorageProcessor { return new CopyCmdAnswer(details); } - snapshotSr = createFileSr(conn, secondaryStorageMountPath, folder); + snapshotSr = createFileSr(conn, secondaryStorageMountPath, folder, nfsVersion); task = snapshotVdi.copyAsync(conn, snapshotSr, null, null); // poll every 1 seconds , @@ -649,7 +651,7 @@ public class Xenserver625StorageProcessor extends XenServerStorageProcessor { throw new CloudRuntimeException("S3 upload of snapshots " + snapshotPaUuid + " failed"); } } else { - final String result = backupSnapshot(conn, primaryStorageSRUuid, localMountPoint, folder, secondaryStorageMountPath, snapshotUuid, prevBackupUuid, prevSnapshotUuid, isISCSI, wait); + final String result = backupSnapshot(conn, primaryStorageSRUuid, localMountPoint, folder, secondaryStorageMountPath, snapshotUuid, prevBackupUuid, prevSnapshotUuid, isISCSI, wait, nfsVersion); final String[] tmp = result.split("#"); snapshotBackupUuid = tmp[0]; physicalSize = Long.parseLong(tmp[1]); @@ -695,6 +697,7 @@ public class Xenserver625StorageProcessor extends XenServerStorageProcessor { final String secondaryStoragePoolURL = destStore.getUrl(); final String volumeUUID = volume.getPath(); + final String nfsVersion = destStore.getNfsVersion(); final String userSpecifiedName = template.getName(); @@ -708,7 +711,7 @@ public class Xenserver625StorageProcessor extends XenServerStorageProcessor { final URI uri = new URI(secondaryStoragePoolURL); secondaryStorageMountPath = uri.getHost() + ":" + uri.getPath(); installPath = template.getPath(); - if (!hypervisorResource.createSecondaryStorageFolder(conn, secondaryStorageMountPath, installPath)) { + if (!hypervisorResource.createSecondaryStorageFolder(conn, secondaryStorageMountPath, installPath, nfsVersion)) { details = " Filed to create folder " + installPath + " in secondary storage"; s_logger.warn(details); return new CopyCmdAnswer(details); @@ -716,7 +719,7 @@ public class Xenserver625StorageProcessor extends XenServerStorageProcessor { final VDI vol = getVDIbyUuid(conn, volumeUUID); // create template SR - tmpltSR = createFileSr(conn, uri.getHost() + ":" + uri.getPath(), installPath); + tmpltSR = createFileSr(conn, uri.getHost() + ":" + uri.getPath(), installPath, nfsVersion); // copy volume to template SR task = vol.copyAsync(conn, tmpltSR, null, null); @@ -736,7 +739,7 @@ public class Xenserver625StorageProcessor extends XenServerStorageProcessor { final long physicalSize = tmpltVDI.getPhysicalUtilisation(conn); // create the template.properties file final String templatePath = secondaryStorageMountPath + "/" + installPath; - result = hypervisorResource.postCreatePrivateTemplate(conn, templatePath, tmpltFilename, tmpltUUID, userSpecifiedName, null, physicalSize, virtualSize, template.getId()); + result = hypervisorResource.postCreatePrivateTemplate(conn, templatePath, tmpltFilename, tmpltUUID, userSpecifiedName, null, physicalSize, virtualSize, template.getId(), nfsVersion); if (!result) { throw new CloudRuntimeException("Could not create the template.properties file on secondary storage dir"); } @@ -756,7 +759,7 @@ public class Xenserver625StorageProcessor extends XenServerStorageProcessor { hypervisorResource.removeSR(conn, tmpltSR); } if (secondaryStorageMountPath != null) { - hypervisorResource.deleteSecondaryStorageFolder(conn, secondaryStorageMountPath, installPath); + hypervisorResource.deleteSecondaryStorageFolder(conn, secondaryStorageMountPath, installPath, nfsVersion); } details = "Creating template from volume " + volumeUUID + " failed due to " + e.toString(); s_logger.error(details, e); @@ -805,6 +808,7 @@ public class Xenserver625StorageProcessor extends XenServerStorageProcessor { } final NfsTO nfsImageStore = (NfsTO)imageStore; + final String nfsVersion = nfsImageStore.getNfsVersion(); final String primaryStorageNameLabel = pool.getUuid(); final String secondaryStorageUrl = nfsImageStore.getUrl(); final int wait = cmd.getWait(); @@ -851,7 +855,7 @@ public class Xenserver625StorageProcessor extends XenServerStorageProcessor { final String snapshotUuid = getSnapshotUuid(snapshotInstallPath); final URI uri = new URI(secondaryStorageUrl); - srcSr = createFileSr(conn, uri.getHost() + ":" + uri.getPath(), snapshotDirectory); + srcSr = createFileSr(conn, uri.getHost() + ":" + uri.getPath(), snapshotDirectory, nfsVersion); final String[] parents = snapshot.getParents(); final List snapshotChains = new ArrayList(); @@ -940,14 +944,15 @@ public class Xenserver625StorageProcessor extends XenServerStorageProcessor { Task task = null; try { final NfsTO nfsStore = (NfsTO)destStore; + final String nfsVersion = nfsStore.getNfsVersion(); final URI uri = new URI(nfsStore.getUrl()); // Create the volume folder - if (!hypervisorResource.createSecondaryStorageFolder(conn, uri.getHost() + ":" + uri.getPath(), destVolume.getPath())) { + if (!hypervisorResource.createSecondaryStorageFolder(conn, uri.getHost() + ":" + uri.getPath(), destVolume.getPath(), nfsVersion)) { throw new InternalErrorException("Failed to create the volume folder."); } // Create a SR for the volume UUID folder - secondaryStorage = createFileSr(conn, uri.getHost() + ":" + uri.getPath(), destVolume.getPath()); + secondaryStorage = createFileSr(conn, uri.getHost() + ":" + uri.getPath(), destVolume.getPath(), nfsVersion); // Look up the volume on the source primary storage pool final VDI srcVdi = getVDIbyUuid(conn, srcVolume.getPath()); // Copy the volume to secondary storage @@ -992,6 +997,7 @@ public class Xenserver625StorageProcessor extends XenServerStorageProcessor { if (srcStore instanceof NfsTO) { final NfsTO nfsStore = (NfsTO)srcStore; + final String nfsVersion = nfsStore.getNfsVersion(); final String volumePath = srcVolume.getPath(); int index = volumePath.lastIndexOf("/"); final String volumeDirectory = volumePath.substring(0, index); @@ -1006,7 +1012,7 @@ public class Xenserver625StorageProcessor extends XenServerStorageProcessor { } catch (final Exception e) { return new CopyCmdAnswer(e.toString()); } - final SR srcSr = createFileSr(conn, uri.getHost() + ":" + uri.getPath(), volumeDirectory); + final SR srcSr = createFileSr(conn, uri.getHost() + ":" + uri.getPath(), volumeDirectory, nfsVersion); Task task = null; try { final SR primaryStoragePool = hypervisorResource.getStorageRepository(conn, @@ -1089,12 +1095,14 @@ public class Xenserver625StorageProcessor extends XenServerStorageProcessor { boolean result = false; try { - srcSr = createFileSr(conn, srcUri.getHost() + ":" + srcUri.getPath(), srcDir); + String srcNfsVersion = srcStore.getNfsVersion(); + srcSr = createFileSr(conn, srcUri.getHost() + ":" + srcUri.getPath(), srcDir, srcNfsVersion); final String destNfsPath = destUri.getHost() + ":" + destUri.getPath(); final String localDir = BASE_MOUNT_POINT_ON_REMOTE + UUID.nameUUIDFromBytes(destNfsPath.getBytes()); - mountNfs(conn, destUri.getHost() + ":" + destUri.getPath(), localDir); + String destNfsVersion = destStore.getNfsVersion(); + mountNfs(conn, destUri.getHost() + ":" + destUri.getPath(), localDir, destNfsVersion); makeDirectory(conn, localDir + "/" + destDir); destSr = createFileSR(conn, localDir + "/" + destDir); @@ -1148,7 +1156,7 @@ public class Xenserver625StorageProcessor extends XenServerStorageProcessor { templatePath = templatePath.replaceAll("//", "/"); - result = hypervisorResource.postCreatePrivateTemplate(conn, templatePath, templateFilename, templateUuid, nameLabel, null, physicalSize, virtualSize, destObj.getId()); + result = hypervisorResource.postCreatePrivateTemplate(conn, templatePath, templateFilename, templateUuid, nameLabel, null, physicalSize, virtualSize, destObj.getId(), destNfsVersion); if (!result) { throw new CloudRuntimeException("Could not create the template.properties file on secondary storage dir"); @@ -1236,7 +1244,8 @@ public class Xenserver625StorageProcessor extends XenServerStorageProcessor { final String destNfsPath = destUri.getHost() + ":" + destUri.getPath(); final String localDir = BASE_MOUNT_POINT_ON_REMOTE + UUID.nameUUIDFromBytes(destNfsPath.getBytes()); - mountNfs(conn, destNfsPath, localDir); + String nfsVersion = destStore.getNfsVersion(); + mountNfs(conn, destNfsPath, localDir, nfsVersion); makeDirectory(conn, localDir + "/" + destDir); destSr = createFileSR(conn, localDir + "/" + destDir); @@ -1263,7 +1272,7 @@ public class Xenserver625StorageProcessor extends XenServerStorageProcessor { templatePath = templatePath.replaceAll("//", "/"); - result = hypervisorResource.postCreatePrivateTemplate(conn, templatePath, templateFilename, templateUuid, nameLabel, null, physicalSize, virtualSize, templateObjTO.getId()); + result = hypervisorResource.postCreatePrivateTemplate(conn, templatePath, templateFilename, templateUuid, nameLabel, null, physicalSize, virtualSize, templateObjTO.getId(), nfsVersion); if (!result) { throw new CloudRuntimeException("Could not create the template.properties file on secondary storage dir"); diff --git a/scripts/vm/hypervisor/xenserver/cloud-plugin-storage b/scripts/vm/hypervisor/xenserver/cloud-plugin-storage index 207d4f4800b..bc909474dcb 100644 --- a/scripts/vm/hypervisor/xenserver/cloud-plugin-storage +++ b/scripts/vm/hypervisor/xenserver/cloud-plugin-storage @@ -238,6 +238,8 @@ def umount(localDir): def mountNfsSecondaryStorage(session, args): remoteDir = args['remoteDir'] localDir = args['localDir'] + nfsVersion = args['nfsVersion'] + logging.debug("mountNfsSecondaryStorage with params: " + str(args)) mounted = False f = open("/proc/mounts", 'r') for line in f: @@ -250,6 +252,8 @@ def mountNfsSecondaryStorage(session, args): makedirs(localDir) options = "soft,tcp,timeo=133,retrans=1" + if nfsVersion: + options += ",vers=" + nfsVersion try: cmd = ['mount', '-o', options, remoteDir, localDir] txt = util.pread2(cmd) diff --git a/scripts/vm/hypervisor/xenserver/vmopsSnapshot b/scripts/vm/hypervisor/xenserver/vmopsSnapshot index 6b4693ca62e..b74a8550e05 100755 --- a/scripts/vm/hypervisor/xenserver/vmopsSnapshot +++ b/scripts/vm/hypervisor/xenserver/vmopsSnapshot @@ -68,7 +68,8 @@ def create_secondary_storage_folder(session, args): # Mount the remote resource folder locally remote_mount_path = args["remoteMountPath"] local_mount_path = os.path.join(CLOUD_DIR, util.gen_uuid()) - mount(remote_mount_path, local_mount_path) + nfsVersion = args["nfsVersion"] + mount(remote_mount_path, local_mount_path, nfsVersion) # Create the new folder new_folder = local_mount_path + "/" + args["newFolder"] @@ -104,7 +105,8 @@ def delete_secondary_storage_folder(session, args): # Mount the remote resource folder locally remote_mount_path = args["remoteMountPath"] local_mount_path = os.path.join(CLOUD_DIR, util.gen_uuid()) - mount(remote_mount_path, local_mount_path) + nfsVersion = args["nfsVersion"] + mount(remote_mount_path, local_mount_path, nfsVersion) # Delete the specified folder folder = local_mount_path + "/" + args["folder"] @@ -136,7 +138,8 @@ def post_create_private_template(session, args): # get local template folder templatePath = args["templatePath"] local_mount_path = os.path.join(CLOUD_DIR, util.gen_uuid()) - mount(templatePath, local_mount_path) + nfsVersion = args["nfsVersion"] + mount(templatePath, local_mount_path, nfsVersion) # Retrieve args filename = args["templateFilename"] name = args["templateName"] @@ -307,9 +310,11 @@ def makedirs(path): raise xs_errors.XenError(errMsg) return -def mount(remoteDir, localDir): +def mount(remoteDir, localDir, nfsVersion=None): makedirs(localDir) options = "soft,tcp,timeo=133,retrans=1" + if nfsVersion: + options += ",vers=" + nfsVersion try: cmd = ['mount', '-o', options, remoteDir, localDir] txt = util.pread2(cmd) diff --git a/server/src/main/java/org/apache/cloudstack/diagnostics/DiagnosticsServiceImpl.java b/server/src/main/java/org/apache/cloudstack/diagnostics/DiagnosticsServiceImpl.java index de370b3ae2b..e00d898c1aa 100644 --- a/server/src/main/java/org/apache/cloudstack/diagnostics/DiagnosticsServiceImpl.java +++ b/server/src/main/java/org/apache/cloudstack/diagnostics/DiagnosticsServiceImpl.java @@ -30,6 +30,7 @@ import java.util.regex.Pattern; import javax.inject.Inject; import javax.naming.ConfigurationException; +import com.cloud.capacity.CapacityManager; import org.apache.cloudstack.api.command.admin.diagnostics.GetDiagnosticsDataCmd; import org.apache.cloudstack.api.command.admin.diagnostics.RunDiagnosticsCmd; import org.apache.cloudstack.diagnostics.fileprocessor.DiagnosticsFilesList; @@ -313,7 +314,8 @@ public class DiagnosticsServiceImpl extends ManagerBase implements PluggableServ } private Pair copyToSecondaryStorageNonVMware(final DataStore store, final String vmControlIp, String fileToCopy, Long vmHostId) { - CopyToSecondaryStorageCommand toSecondaryStorageCommand = new CopyToSecondaryStorageCommand(store.getUri(), vmControlIp, fileToCopy); + String nfsVersion = CapacityManager.ImageStoreNFSVersion.valueIn(store.getId()); + CopyToSecondaryStorageCommand toSecondaryStorageCommand = new CopyToSecondaryStorageCommand(store.getUri(), vmControlIp, fileToCopy, nfsVersion); Answer copyToSecondaryAnswer = agentManager.easySend(vmHostId, toSecondaryStorageCommand); Pair copyAnswer; if (copyToSecondaryAnswer != null) { From 5f07e4daaf7fd1a7e2bff4296a62a5acf527dcbb Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Mon, 31 Jan 2022 17:19:31 +0530 Subject: [PATCH 2/7] ui: fix filtering readonly details while VM update (#5887) * ui: fix filtering readonly details while VM update * refactor * error on add Fixes #5724 Signed-off-by: Abhishek Kumar --- ui/src/components/view/DetailSettings.vue | 45 ++++++++++++++--------- 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/ui/src/components/view/DetailSettings.vue b/ui/src/components/view/DetailSettings.vue index 2588c11fb2e..7d143ba52ef 100644 --- a/ui/src/components/view/DetailSettings.vue +++ b/ui/src/components/view/DetailSettings.vue @@ -173,13 +173,6 @@ export default { this.deployasistemplate = json.listtemplatesresponse.template[0].deployasis }) }, - filterOrReadOnlyDetails () { - for (var i = 0; i < this.details.length; i++) { - if (!this.allowEditOfDetail(this.details[i].name)) { - this.details.splice(i, 1) - } - } - }, allowEditOfDetail (name) { if (this.resource.readonlydetails) { if (this.resource.readonlydetails.split(',').map(item => item.trim()).includes(name)) { @@ -211,6 +204,27 @@ export default { (this.resource.domainid === this.$store.getters.userInfo.domainid && this.resource.account === this.$store.getters.userInfo.account) || this.resource.project && this.resource.projectid === this.$store.getters.project.id }, + getDetailsParam (details) { + var params = {} + var filteredDetails = details + if (this.resource.readonlydetails && filteredDetails) { + filteredDetails = [] + var readOnlyDetailNames = this.resource.readonlydetails.split(',').map(item => item.trim()) + for (var detail of this.details) { + if (!readOnlyDetailNames.includes(detail.name)) { + filteredDetails.push(detail) + } + } + } + if (filteredDetails.length === 0) { + params.cleanupdetails = true + } else { + filteredDetails.forEach(function (item, index) { + params['details[0].' + item.name] = item.value + }) + } + return params + }, runApi () { var apiName = '' if (this.resourceType === 'UserVm') { @@ -226,14 +240,8 @@ export default { return } - const params = { id: this.resource.id } - if (this.details.length === 0) { - params.cleanupdetails = true - } else { - this.details.forEach(function (item, index) { - params['details[0].' + item.name] = item.value - }) - } + var params = { id: this.resource.id } + params = Object.assign(params, this.getDetailsParam(this.details)) this.loading = true api(apiName, params).then(json => { var details = {} @@ -259,18 +267,19 @@ export default { this.error = this.$t('message.error.provide.setting') return } + if (!this.allowEditOfDetail(this.newKey)) { + this.error = this.$t('error.unable.to.proceed') + return + } this.error = false this.details.push({ name: this.newKey, value: this.newValue }) - this.filterOrReadOnlyDetails() this.runApi() }, updateDetail (index) { - this.filterOrReadOnlyDetails() this.runApi() }, deleteDetail (index) { this.details.splice(index, 1) - this.filterOrReadOnlyDetails() this.runApi() }, onShowAddDetail () { From c1bba2a308e2df1eefb772b8ffa8a3abc2240d9b Mon Sep 17 00:00:00 2001 From: dahn Date: Mon, 31 Jan 2022 13:29:26 +0100 Subject: [PATCH 3/7] Do not restart VPC tiers with cleanup (#5873) * do not restart VPC tiers with cleanup * no option for cleanup for VPC tiers * Update server/src/main/java/com/cloud/network/NetworkServiceImpl.java * paramNames * remove superfluent parameter Co-authored-by: Daan Hoogland Co-authored-by: Daniel Augusto Veronezi Salvador <38945620+GutoVeronezi@users.noreply.github.com> --- .../com/cloud/network/NetworkServiceImpl.java | 39 +++++++++++-------- ui/src/config/section/network.js | 2 +- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java index de97c2939db..de13cd7b3e4 100644 --- a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java +++ b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java @@ -2000,12 +2000,7 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService, C Account caller = CallContext.current().getCallingAccount(); // Verify network id - NetworkVO network = _networksDao.findById(networkId); - if (network == null) { - // see NetworkVO.java - - throwInvalidIdException("unable to find network with specified id", String.valueOf(networkId), "networkId"); - } + NetworkVO network = getNetworkVO(networkId, "Unable to find a network with the specified ID."); // don't allow to delete system network if (isNetworkSystem(network)) { @@ -2035,10 +2030,20 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService, C @Override @ActionEvent(eventType = EventTypes.EVENT_NETWORK_RESTART, eventDescription = "restarting network", async = true) public boolean restartNetwork(Long networkId, boolean cleanup, boolean makeRedundant, User user) throws ConcurrentOperationException, ResourceUnavailableException, InsufficientCapacityException { + NetworkVO network = getNetworkVO(networkId, "Network with specified id doesn't exist"); + return restartNetwork(network, cleanup, makeRedundant, user); + } + + private NetworkVO getNetworkVO(Long networkId, String errMsgFormat) { NetworkVO network = _networksDao.findById(networkId); if (network == null) { - throwInvalidIdException("Network with specified id doesn't exist", networkId.toString(), "networkId"); + throwInvalidIdException(errMsgFormat, networkId.toString(), "networkId"); } + return network; + } + + @ActionEvent(eventType = EventTypes.EVENT_NETWORK_RESTART, eventDescription = "restarting network", async = true) + public boolean restartNetwork(NetworkVO network, boolean cleanup, boolean makeRedundant, User user) throws ConcurrentOperationException, ResourceUnavailableException, InsufficientCapacityException { // Don't allow to restart network if it's not in Implemented/Setup state if (!(network.getState() == Network.State.Implemented || network.getState() == Network.State.Setup)) { @@ -2064,11 +2069,12 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService, C cleanup = true; } - boolean success = _networkMgr.restartNetwork(networkId, callerAccount, user, cleanup); + long id = network.getId(); + boolean success = _networkMgr.restartNetwork(id, callerAccount, user, cleanup); if (success) { - s_logger.debug("Network id=" + networkId + " is restarted successfully."); + s_logger.debug(String.format("Network id=%d is restarted successfully.",id)); } else { - s_logger.warn("Network id=" + networkId + " failed to restart."); + s_logger.warn(String.format("Network id=%d failed to restart.",id)); } return success; @@ -2078,11 +2084,14 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService, C @ActionEvent(eventType = EventTypes.EVENT_NETWORK_RESTART, eventDescription = "restarting network", async = true) public boolean restartNetwork(RestartNetworkCmd cmd) throws ConcurrentOperationException, ResourceUnavailableException, InsufficientCapacityException { // This method restarts all network elements belonging to the network and re-applies all the rules - Long networkId = cmd.getNetworkId(); + NetworkVO network = getNetworkVO(cmd.getNetworkId(), "Network [%s] to restart was not found."); boolean cleanup = cmd.getCleanup(); + if (network.getVpcId() != null && cleanup) { + throwInvalidIdException("Cannot restart a VPC tier with cleanup, please restart the whole VPC.", network.getUuid(), "network tier"); + } boolean makeRedundant = cmd.getMakeRedundant(); User callerUser = _accountMgr.getActiveUser(CallContext.current().getCallingUserId()); - return restartNetwork(networkId, cleanup, makeRedundant, callerUser); + return restartNetwork(network, cleanup, makeRedundant, callerUser); } @Override @@ -2219,11 +2228,7 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService, C boolean restartNetwork = false; // verify input parameters - final NetworkVO network = _networksDao.findById(networkId); - if (network == null) { - // see NetworkVO.java - throwInvalidIdException("Specified network id doesn't exist in the system", String.valueOf(networkId), "networkId"); - } + final NetworkVO network = getNetworkVO(networkId, "Specified network id doesn't exist in the system"); //perform below validation if the network is vpc network if (network.getVpcId() != null && networkOfferingId != null) { diff --git a/ui/src/config/section/network.js b/ui/src/config/section/network.js index 95c24159bb0..43992e81ced 100644 --- a/ui/src/config/section/network.js +++ b/ui/src/config/section/network.js @@ -99,7 +99,7 @@ export default { label: 'label.restart.network', message: 'message.restart.network', dataView: true, - args: ['cleanup'], + args: (record) => record.vpcid == null ? ['cleanup'] : [], // if it is a tier in a VPC and so it has a vpc do not allow "cleanup show: (record) => record.type !== 'L2', groupAction: true, popup: true, From fde34df560fdc57e601ab414aec8c26cbbdc9a4f Mon Sep 17 00:00:00 2001 From: dahn Date: Mon, 31 Jan 2022 14:35:27 +0100 Subject: [PATCH 4/7] Make sure other than user VMs can have multiple NICs in a network (#5896) * only check user VMs * Update engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java Co-authored-by: Daan Hoogland Co-authored-by: Wei Zhou --- .../main/java/com/cloud/vm/VirtualMachineManagerImpl.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java index 706b665c286..340a3e2e99d 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -4043,7 +4043,10 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac /** * duplicated in {@see UserVmManagerImpl} for a {@see UserVmVO} */ - private void checkIfNetworkExistsForVM(VirtualMachine virtualMachine, Network network) { + private void checkIfNetworkExistsForUserVM(VirtualMachine virtualMachine, Network network) { + if (virtualMachine.getType() != VirtualMachine.Type.User) { + return; // others may have multiple nics in the same network + } List allNics = _nicsDao.listByVmId(virtualMachine.getId()); for (NicVO nic : allNics) { if (nic.getNetworkId() == network.getId()) { @@ -4056,7 +4059,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac InsufficientCapacityException { final CallContext cctx = CallContext.current(); - checkIfNetworkExistsForVM(vm, network); + checkIfNetworkExistsForUserVM(vm, network); s_logger.debug("Adding vm " + vm + " to network " + network + "; requested nic profile " + requested); final VMInstanceVO vmVO = _vmDao.findById(vm.getId()); final ReservationContext context = new ReservationContextImpl(null, null, cctx.getCallingUser(), cctx.getCallingAccount()); From 1b3e7f65b7b0ddf6a8bfbd69ae3783356dfc944f Mon Sep 17 00:00:00 2001 From: sureshanaparti <12028987+sureshanaparti@users.noreply.github.com> Date: Wed, 2 Feb 2022 10:02:00 +0530 Subject: [PATCH 5/7] Update proper destroy status when SSVM is destroyed. (#5908) --- .../secondarystorage/SecondaryStorageManagerImpl.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java b/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java index 99539a29dcb..44e8a114a90 100644 --- a/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java +++ b/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java @@ -1036,12 +1036,10 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar if (host != null) { s_logger.debug(String.format("Removing host entry for secondary storage VM [%s].", vmId)); _hostDao.remove(host.getId()); - _tmplStoreDao.expireDnldUrlsForZone(host.getDataCenterId()); _volumeStoreDao.expireDnldUrlsForZone(host.getDataCenterId()); - return true; } - return false; + return true; } catch (ResourceUnavailableException e) { s_logger.error(String.format("Unable to expunge secondary storage [%s] due to [%s].", ssvm.toString(), e.getMessage()), e); return false; From ddd311c695d1c77b0cca159ab2fd04bd40affa2d Mon Sep 17 00:00:00 2001 From: Nicolas Vazquez Date: Wed, 2 Feb 2022 03:51:47 -0300 Subject: [PATCH 6/7] [XenServer/XCP-ng] Sync the 'platform' setting according to the 'cpu.corespersocket' setting (#5892) * Update platform setting if cpu.corespersocket is set * Refactor conditions * Allow empty map but not null --- .../resource/CitrixResourceBase.java | 16 +++++++-- .../resource/CitrixResourceBaseTest.java | 33 +++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java index 9348bcaab9f..19625a2f09a 100644 --- a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java +++ b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java @@ -226,6 +226,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe protected static final HashMap s_powerStatesTable; public static final String XS_TOOLS_ISO_AFTER_70 = "guest-tools.iso"; + protected static final String PLATFORM_CORES_PER_SOCKET_KEY = "cores-per-socket"; static { s_powerStatesTable = new HashMap(); @@ -1899,13 +1900,25 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe } } + protected void syncPlatformAndCoresPerSocketSettings(String coresPerSocket, Map platform) { + if (org.apache.commons.lang3.StringUtils.isBlank(coresPerSocket) || platform == null) { + return; + } + if (platform.containsKey(PLATFORM_CORES_PER_SOCKET_KEY)) { + s_logger.debug("Updating the cores per socket value from: " + platform.get(PLATFORM_CORES_PER_SOCKET_KEY) + " to " + coresPerSocket); + } + platform.put(PLATFORM_CORES_PER_SOCKET_KEY, coresPerSocket); + } + protected void finalizeVmMetaData(final VM vm, final VM.Record vmr, final Connection conn, final VirtualMachineTO vmSpec) throws Exception { final Map details = vmSpec.getDetails(); if (details != null) { final String platformstring = details.get(VmDetailConstants.PLATFORM); + final String coresPerSocket = details.get(VmDetailConstants.CPU_CORE_PER_SOCKET); if (platformstring != null && !platformstring.isEmpty()) { final Map platform = StringUtils.stringToMap(platformstring); + syncPlatformAndCoresPerSocketSettings(coresPerSocket, platform); vm.setPlatform(conn, platform); } else { final String timeoffset = details.get(VmDetailConstants.TIME_OFFSET); @@ -1914,10 +1927,9 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe platform.put(VmDetailConstants.TIME_OFFSET, timeoffset); vm.setPlatform(conn, platform); } - final String coresPerSocket = details.get(VmDetailConstants.CPU_CORE_PER_SOCKET); if (coresPerSocket != null) { final Map platform = vm.getPlatform(conn); - platform.put("cores-per-socket", coresPerSocket); + syncPlatformAndCoresPerSocketSettings(coresPerSocket, platform); vm.setPlatform(conn, platform); } } diff --git a/plugins/hypervisors/xenserver/src/test/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBaseTest.java b/plugins/hypervisors/xenserver/src/test/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBaseTest.java index b1a89c9da82..e40c7e89cee 100644 --- a/plugins/hypervisors/xenserver/src/test/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBaseTest.java +++ b/plugins/hypervisors/xenserver/src/test/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBaseTest.java @@ -23,6 +23,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import com.cloud.utils.StringUtils; import org.apache.xmlrpc.XmlRpcException; import org.junit.Assert; import org.junit.Before; @@ -51,6 +52,8 @@ import com.xensource.xenapi.PBD; import com.xensource.xenapi.SR; import com.xensource.xenapi.Types.XenAPIException; +import static com.cloud.hypervisor.xenserver.resource.CitrixResourceBase.PLATFORM_CORES_PER_SOCKET_KEY; + @RunWith(PowerMockRunner.class) @PrepareForTest({Host.class, Script.class, SR.class}) public class CitrixResourceBaseTest { @@ -72,6 +75,8 @@ public class CitrixResourceBaseTest { private String hostUuidMock = "hostUuidMock"; + private static final String platformString = "device-model:qemu-upstream-compat;vga:std;videoram:8;apic:true;viridian:false;timeoffset:0;pae:true;acpi:1;hpet:true;secureboot:false;nx:true"; + @Before public void beforeTest() throws XenAPIException, XmlRpcException { citrixResourceBase._host.setUuid(hostUuidMock); @@ -369,4 +374,32 @@ public class CitrixResourceBaseTest { Assert.assertEquals(1, startUpCommandsForLocalStorage.size()); } + + @Test + public void syncPlatformAndCoresPerSocketSettingsEmptyCoresPerSocket() { + String coresPerSocket = null; + Map platform = Mockito.mock(Map.class); + citrixResourceBase.syncPlatformAndCoresPerSocketSettings(coresPerSocket, platform); + Mockito.verify(platform, Mockito.never()).put(Mockito.any(), Mockito.any()); + Mockito.verify(platform, Mockito.never()).remove(Mockito.any()); + } + + @Test + public void syncPlatformAndCoresPerSocketSettingsEmptyCoresPerSocketOnPlatform() { + String coresPerSocket = "2"; + Map platform = StringUtils.stringToMap(platformString); + citrixResourceBase.syncPlatformAndCoresPerSocketSettings(coresPerSocket, platform); + Assert.assertTrue(platform.containsKey(PLATFORM_CORES_PER_SOCKET_KEY)); + Assert.assertEquals(coresPerSocket, platform.get(PLATFORM_CORES_PER_SOCKET_KEY)); + } + + @Test + public void syncPlatformAndCoresPerSocketSettingsUpdateCoresPerSocketOnPlatform() { + String coresPerSocket = "2"; + String platformStr = platformString + "," + PLATFORM_CORES_PER_SOCKET_KEY + ":3"; + Map platform = StringUtils.stringToMap(platformStr); + citrixResourceBase.syncPlatformAndCoresPerSocketSettings(coresPerSocket, platform); + Assert.assertTrue(platform.containsKey(PLATFORM_CORES_PER_SOCKET_KEY)); + Assert.assertEquals(coresPerSocket, platform.get(PLATFORM_CORES_PER_SOCKET_KEY)); + } } From 8adb8df2fe8a3b29f19dc571b65ac33987f9be59 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 2 Feb 2022 16:35:47 +0530 Subject: [PATCH 7/7] server: find suitable disk offering for volume upload (#5852) * server: find suitable disk offering for volume upload Fixes #5696 * fix npe check * fixes, refactor, rename method and handle custom iops * ui: allow offering selection * list only disk offerings * show name * revert error check * use checkaccess Signed-off-by: Abhishek Kumar --- .../cloud/storage/dao/DiskOfferingDao.java | 2 + .../storage/dao/DiskOfferingDaoImpl.java | 13 ++++ .../query/dao/DiskOfferingJoinDaoImpl.java | 14 ++--- .../cloud/storage/VolumeApiServiceImpl.java | 60 ++++++++++++++----- ui/src/views/storage/UploadLocalVolume.vue | 58 ++++++++++++++++-- 5 files changed, 121 insertions(+), 26 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/DiskOfferingDao.java b/engine/schema/src/main/java/com/cloud/storage/dao/DiskOfferingDao.java index 3305752459d..2425e584122 100644 --- a/engine/schema/src/main/java/com/cloud/storage/dao/DiskOfferingDao.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/DiskOfferingDao.java @@ -34,4 +34,6 @@ public interface DiskOfferingDao extends GenericDao { List listAllBySizeAndProvisioningType(long size, Storage.ProvisioningType provisioningType); + List findCustomDiskOfferings(); + } diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/DiskOfferingDaoImpl.java b/engine/schema/src/main/java/com/cloud/storage/dao/DiskOfferingDaoImpl.java index b9fa10c1236..ce52b0ed3dc 100644 --- a/engine/schema/src/main/java/com/cloud/storage/dao/DiskOfferingDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/DiskOfferingDaoImpl.java @@ -29,6 +29,7 @@ import javax.persistence.EntityExistsException; import org.apache.cloudstack.resourcedetail.dao.DiskOfferingDetailsDao; import org.springframework.stereotype.Component; +import com.cloud.offering.DiskOffering; import com.cloud.offering.DiskOffering.Type; import com.cloud.storage.DiskOfferingVO; import com.cloud.storage.Storage; @@ -173,6 +174,18 @@ public class DiskOfferingDaoImpl extends GenericDaoBase im } } + @Override + public List findCustomDiskOfferings() { + SearchBuilder sb = createSearchBuilder(); + sb.and("type", sb.entity().getType(), SearchCriteria.Op.EQ); + sb.and("customized", sb.entity().isCustomized(), SearchCriteria.Op.EQ); + sb.done(); + SearchCriteria sc = sb.create(); + sc.setParameters("customized", true); + sc.setParameters("type", DiskOffering.Type.Disk.toString()); + return listBy(sc); + } + @Override public boolean remove(Long id) { DiskOfferingVO diskOffering = createForUpdate(); diff --git a/server/src/main/java/com/cloud/api/query/dao/DiskOfferingJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/DiskOfferingJoinDaoImpl.java index 685f30169b4..a738aed3020 100644 --- a/server/src/main/java/com/cloud/api/query/dao/DiskOfferingJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/DiskOfferingJoinDaoImpl.java @@ -19,11 +19,8 @@ package com.cloud.api.query.dao; import java.util.List; import java.util.Map; -import com.cloud.api.ApiDBUtils; -import com.cloud.dc.VsphereStoragePolicyVO; -import com.cloud.dc.dao.VsphereStoragePolicyDao; -import com.cloud.server.ResourceTag; -import com.cloud.user.AccountManager; +import javax.inject.Inject; + import org.apache.cloudstack.annotation.AnnotationService; import org.apache.cloudstack.annotation.dao.AnnotationDao; import org.apache.cloudstack.api.ApiConstants; @@ -32,16 +29,19 @@ import org.apache.cloudstack.context.CallContext; import org.apache.log4j.Logger; import org.springframework.stereotype.Component; +import com.cloud.api.ApiDBUtils; import com.cloud.api.query.vo.DiskOfferingJoinVO; +import com.cloud.dc.VsphereStoragePolicyVO; +import com.cloud.dc.dao.VsphereStoragePolicyDao; import com.cloud.offering.DiskOffering; import com.cloud.offering.ServiceOffering; +import com.cloud.server.ResourceTag; +import com.cloud.user.AccountManager; import com.cloud.utils.db.Attribute; import com.cloud.utils.db.GenericDaoBase; import com.cloud.utils.db.SearchBuilder; import com.cloud.utils.db.SearchCriteria; -import javax.inject.Inject; - @Component public class DiskOfferingJoinDaoImpl extends GenericDaoBase implements DiskOfferingJoinDao { public static final Logger s_logger = Logger.getLogger(DiskOfferingJoinDaoImpl.class); diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index d092ad11135..35de68b43e0 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -125,6 +125,7 @@ import com.cloud.host.dao.HostDao; import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.hypervisor.HypervisorCapabilitiesVO; import com.cloud.hypervisor.dao.HypervisorCapabilitiesDao; +import com.cloud.offering.DiskOffering; import com.cloud.org.Grouping; import com.cloud.resource.ResourceState; import com.cloud.serializer.GsonHelper; @@ -315,6 +316,8 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic private static final Set STATES_VOLUME_CANNOT_BE_DESTROYED = new HashSet<>(Arrays.asList(Volume.State.Destroy, Volume.State.Expunging, Volume.State.Expunged, Volume.State.Allocated)); + private static final String CUSTOM_DISK_OFFERING_UNIQUE_NAME = "Cloud.com-Custom"; + protected VolumeApiServiceImpl() { _volStateMachine = Volume.State.getStateMachine(); _gson = GsonHelper.getGsonLogger(); @@ -478,7 +481,6 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic if (!diskOffering.isCustomized()) { throw new InvalidParameterValueException("Please specify a custom sized disk offering."); } - _configMgr.checkDiskOfferingAccess(volumeOwner, diskOffering, zone); } @@ -489,12 +491,41 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic return UUID.randomUUID().toString(); } + private Long getDefaultCustomOfferingId(Account owner, DataCenter zone) { + DiskOfferingVO diskOfferingVO = _diskOfferingDao.findByUniqueName(CUSTOM_DISK_OFFERING_UNIQUE_NAME); + if (diskOfferingVO == null || !DiskOffering.State.Active.equals(diskOfferingVO.getState())) { + return null; + } + try { + _configMgr.checkDiskOfferingAccess(owner, diskOfferingVO, zone); + return diskOfferingVO.getId(); + } catch (PermissionDeniedException ignored) { + } + return null; + } + + private Long getCustomDiskOfferingIdForVolumeUpload(Account owner, DataCenter zone) { + Long offeringId = getDefaultCustomOfferingId(owner, zone); + if (offeringId != null) { + return offeringId; + } + List offerings = _diskOfferingDao.findCustomDiskOfferings(); + for (DiskOfferingVO offering : offerings) { + try { + _configMgr.checkDiskOfferingAccess(owner, offering, zone); + return offering.getId(); + } catch (PermissionDeniedException ignored) {} + } + return null; + } + @DB protected VolumeVO persistVolume(final Account owner, final Long zoneId, final String volumeName, final String url, final String format, final Long diskOfferingId, final Volume.State state) { - return Transaction.execute(new TransactionCallback() { + return Transaction.execute(new TransactionCallbackWithException() { @Override public VolumeVO doInTransaction(TransactionStatus status) { VolumeVO volume = new VolumeVO(volumeName, zoneId, -1, -1, -1, new Long(-1), null, null, Storage.ProvisioningType.THIN, 0, Volume.Type.DATADISK); + DataCenter zone = _dcDao.findById(zoneId); volume.setPoolId(null); volume.setDataCenterId(zoneId); volume.setPodId(null); @@ -504,23 +535,22 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic volume.setAccountId((owner == null) ? Account.ACCOUNT_ID_SYSTEM : owner.getAccountId()); volume.setDomainId((owner == null) ? Domain.ROOT_DOMAIN : owner.getDomainId()); - if (diskOfferingId == null) { - DiskOfferingVO diskOfferingVO = _diskOfferingDao.findByUniqueName("Cloud.com-Custom"); - if (diskOfferingVO != null) { - long defaultDiskOfferingId = diskOfferingVO.getId(); - volume.setDiskOfferingId(defaultDiskOfferingId); + Long volumeDiskOfferingId = diskOfferingId; + if (volumeDiskOfferingId == null) { + volumeDiskOfferingId = getCustomDiskOfferingIdForVolumeUpload(owner, zone); + if (volumeDiskOfferingId == null) { + throw new CloudRuntimeException(String.format("Unable to find custom disk offering in zone: %s for volume upload", zone.getUuid())); } - } else { - volume.setDiskOfferingId(diskOfferingId); + } - DiskOfferingVO diskOfferingVO = _diskOfferingDao.findById(diskOfferingId); + volume.setDiskOfferingId(volumeDiskOfferingId); + DiskOfferingVO diskOfferingVO = _diskOfferingDao.findById(volumeDiskOfferingId); - Boolean isCustomizedIops = diskOfferingVO != null && diskOfferingVO.isCustomizedIops() != null ? diskOfferingVO.isCustomizedIops() : false; + Boolean isCustomizedIops = diskOfferingVO != null && diskOfferingVO.isCustomizedIops() != null ? diskOfferingVO.isCustomizedIops() : false; - if (isCustomizedIops == null || !isCustomizedIops) { - volume.setMinIops(diskOfferingVO.getMinIops()); - volume.setMaxIops(diskOfferingVO.getMaxIops()); - } + if (isCustomizedIops == null || !isCustomizedIops) { + volume.setMinIops(diskOfferingVO.getMinIops()); + volume.setMaxIops(diskOfferingVO.getMaxIops()); } // volume.setSize(size); diff --git a/ui/src/views/storage/UploadLocalVolume.vue b/ui/src/views/storage/UploadLocalVolume.vue index c4f1d569014..0e5df836fcc 100644 --- a/ui/src/views/storage/UploadLocalVolume.vue +++ b/ui/src/views/storage/UploadLocalVolume.vue @@ -69,7 +69,8 @@ optionFilterProp="children" :filterOption="(input, option) => { return option.componentOptions.propsData.label.toLowerCase().indexOf(input.toLowerCase()) >= 0 - }" > + }" + @change="onZoneChange" > @@ -79,6 +80,22 @@ + + + + + {{ opt.name || opt.displaytext }} + + + 0) { - this.zoneSelected = this.zones[0].id + this.onZoneChange(this.zones[0].id) } } }) }, + onZoneChange (zoneId) { + this.zoneSelected = this.zones[0].id + this.fetchDiskOfferings(zoneId) + }, + fetchDiskOfferings (zoneId) { + this.offeringLoading = true + this.offerings = [{ id: -1, name: '' }] + this.form.setFieldsValue({ + diskofferingid: undefined + }) + api('listDiskOfferings', { + zoneid: zoneId, + listall: true + }).then(json => { + for (var offering of json.listdiskofferingsresponse.diskoffering) { + if (offering.iscustomized) { + this.offerings.push(offering) + } + } + }).finally(() => { + this.offeringLoading = false + }) + }, handleRemove (file) { const index = this.fileList.indexOf(file) const newFileList = this.fileList.slice() @@ -188,7 +230,7 @@ export default { } this.loading = true api('getUploadParamsForVolume', params).then(json => { - this.uploadParams = (json.postuploadvolumeresponse && json.postuploadvolumeresponse.getuploadparams) ? json.postuploadvolumeresponse.getuploadparams : '' + this.uploadParams = json.postuploadvolumeresponse?.getuploadparams || '' const { fileList } = this if (this.fileList.length > 1) { this.$notification.error({ @@ -224,12 +266,20 @@ export default { }).catch(e => { this.$notification.error({ message: this.$t('message.upload.failed'), - description: `${this.$t('message.upload.iso.failed.description')} - ${e}`, + description: `${this.$t('message.upload.volume.failed')} - ${e}`, duration: 0 }) }).finally(() => { this.loading = false }) + }).catch(e => { + this.$notification.error({ + message: this.$t('message.upload.failed'), + description: `${this.$t('message.upload.volume.failed')} - ${e?.response?.data?.postuploadvolumeresponse?.errortext || e}`, + duration: 0 + }) + }).finally(() => { + this.loading = false }) }) },