core: deletion unused image links

Problem:
When creating a disk on a block storage domain (for example, FibreChannel), links to the disk are created on all active hosts that belong to the StoragePool of the disk being created. Further, when deleting a created disk, links to the disk are deleted only on the host with the SPM role, while incorrect links to the deleted disk remain on the other hosts.

Solution:
Added an API method on VDSM to remove unused links. When deleting a disk, the command to delete unused links is invoked for all active hosts that are associated with the StoragePool of the disk being deleted.

Signed-off-by: Evgeniy Kononov <[email protected]>

Author: ekononov-dev

backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/disk/image/RemoveImageCommand.java

@@ -16,12 +16,15 @@
 import org.ovirt.engine.core.bll.NonTransactiveCommandAttribute;
 import org.ovirt.engine.core.bll.context.CommandContext;
 import org.ovirt.engine.core.bll.storage.domain.PostDeleteActionHandler;
+import org.ovirt.engine.core.bll.storage.utils.VdsCommandsHelper;
 import org.ovirt.engine.core.common.AuditLogType;
 import org.ovirt.engine.core.common.VdcObjectType;
 import org.ovirt.engine.core.common.action.ActionType;
 import org.ovirt.engine.core.common.action.RemoveImageParameters;
 import org.ovirt.engine.core.common.asynctasks.AsyncTaskType;
 import org.ovirt.engine.core.common.businessentities.Snapshot;
+import org.ovirt.engine.core.common.businessentities.VDS;
+import org.ovirt.engine.core.common.businessentities.VDSStatus;
 import org.ovirt.engine.core.common.businessentities.VM;
 import org.ovirt.engine.core.common.businessentities.VmDeviceId;
 import org.ovirt.engine.core.common.businessentities.storage.DiskImage;
@@ -30,6 +33,7 @@
 import org.ovirt.engine.core.common.errors.EngineError;
 import org.ovirt.engine.core.common.errors.EngineException;
 import org.ovirt.engine.core.common.vdscommands.DeleteImageGroupVDSCommandParameters;
+import org.ovirt.engine.core.common.vdscommands.DeleteImageUnusedLinksVDSCommandParameters;
 import org.ovirt.engine.core.common.vdscommands.GetImagesListVDSCommandParameters;
 import org.ovirt.engine.core.common.vdscommands.VDSCommandType;
 import org.ovirt.engine.core.common.vdscommands.VDSReturnValue;
@@ -42,6 +46,7 @@
 import org.ovirt.engine.core.dao.ImageDao;
 import org.ovirt.engine.core.dao.ImageStorageDomainMapDao;
 import org.ovirt.engine.core.dao.SnapshotDao;
+import org.ovirt.engine.core.dao.VdsDao;
 import org.ovirt.engine.core.dao.VmDao;
 import org.ovirt.engine.core.dao.VmDeviceDao;
 import org.ovirt.engine.core.utils.ovf.OvfManager;
@@ -85,6 +90,10 @@ public class RemoveImageCommand<T extends RemoveImageParameters> extends BaseIma
     @Inject
     private VmDao vmDao;
 
+    @Inject
+    private VdsCommandsHelper vdsCommandsHelper;
+    @Inject
+    private VdsDao vdsDao;
 
     public RemoveImageCommand(T parameters, CommandContext cmdContext) {
         super(parameters, cmdContext);
@@ -335,11 +344,48 @@ protected VDSReturnValue performDeleteImageVdsmOperation() {
                     ImageStatus.ILLEGAL,
                     getCompensationContext());
         }
-        return runVdsCommand(VDSCommandType.DeleteImageGroup,
+        final VDSReturnValue vdsReturnValue = runVdsCommand(VDSCommandType.DeleteImageGroup,
                 postDeleteActionHandler.fixParameters(
                         new DeleteImageGroupVDSCommandParameters(getDiskImage().getStoragePoolId(),
                                 getStorageDomainId(), getDiskImage().getId(),
                                 getDiskImage().isWipeAfterDelete(), getStorageDomain().getDiscardAfterDelete(),
                                 getParameters().isForceDelete())));
+
+        if (vdsReturnValue.getSucceeded()) {
+            performDeleteImageUnusedLinks();
+        }
+
+        return vdsReturnValue;
+    }
+
+    protected void performDeleteImageUnusedLinks() {
+        List<Guid> vdsForExecute = vdsDao.getAllForStoragePoolAndStatus(getDiskImage().getStoragePoolId(), VDSStatus.Up)
+                .stream()
+                .map(VDS::getId)
+                .collect(Collectors.toList());
+
+        log.info("Deleting unused image links on hosts: [{}].", vdsForExecute);
+
+        for (Guid vdsId : vdsForExecute) {
+            try {
+                log.info("Start deleting unused image links on host '{}'. Image '{}', storage domain '{}', "
+                                + "storage pool '{}'",
+                        vdsId, getDiskImage().getId(), getStorageDomainId(), getDiskImage().getStoragePoolId());
+
+                vdsCommandsHelper.runVdsCommandWithoutFailover(
+                        VDSCommandType.DeleteImageUnusedLinks,
+                        new DeleteImageUnusedLinksVDSCommandParameters(vdsId, getStorageDomainId(),
+                                getDiskImage().getStoragePoolId(), getDiskImage().getId()),
+                        getDiskImage().getStoragePoolId(), null);
+
+                log.info("Unused image links were successfully deleted on host '{}'. Image '{}', storage domain '{}', "
+                                + "storage pool '{}'",
+                        vdsId, getDiskImage().getId(), getStorageDomainId(), getDiskImage().getStoragePoolId());
+            } catch (EngineException e) {
+                log.error("Error while deleting unused image links on host '{}'. Image '{}', storage domain '{}', "
+                                + "storage pool '{}', exception = {}",
+                        vdsId, getDiskImage().getId(), getStorageDomainId(), getDiskImage().getStoragePoolId(), e);
+            }
+        }
     }
 }

backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/DeleteImageUnusedLinksVDSCommandParameters.java

@@ -0,0 +1,64 @@
+package org.ovirt.engine.core.common.vdscommands;
+
+import org.ovirt.engine.core.common.utils.ToStringBuilder;
+import org.ovirt.engine.core.compat.Guid;
+
+public class DeleteImageUnusedLinksVDSCommandParameters extends VdsIdVDSCommandParametersBase {
+
+    /**
+     * ID storage domain.
+     */
+    private Guid sdUUID;
+
+    /**
+     * ID storage pool.
+     */
+    private Guid spUUID;
+
+    /**
+     * ID image.
+     */
+    private Guid imgUUID;
+
+    public DeleteImageUnusedLinksVDSCommandParameters() {
+    }
+
+    public DeleteImageUnusedLinksVDSCommandParameters(Guid vdsId, Guid sdUUID, Guid spUUID, Guid imgUUID) {
+        super(vdsId);
+        this.sdUUID = sdUUID;
+        this.spUUID = spUUID;
+        this.imgUUID = imgUUID;
+    }
+
+    public Guid getSdUUID() {
+        return sdUUID;
+    }
+
+    public void setSdUUID(Guid sdUUID) {
+        this.sdUUID = sdUUID;
+    }
+
+    public Guid getSpUUID() {
+        return spUUID;
+    }
+
+    public void setSpUUID(Guid spUUID) {
+        this.spUUID = spUUID;
+    }
+
+    public Guid getImgUUID() {
+        return imgUUID;
+    }
+
+    public void setImgUUID(Guid imgUUID) {
+        this.imgUUID = imgUUID;
+    }
+
+    @Override
+    protected ToStringBuilder appendAttributes(ToStringBuilder tsb) {
+        return super.appendAttributes(tsb)
+                .append("sdUUID", getSdUUID())
+                .append("spUUID", getSpUUID())
+                .append("imgUUID", getImgUUID());
+    }
+}

backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/VDSCommandType.java

@@ -90,6 +90,7 @@ public enum VDSCommandType {
     GetStoragePoolInfo("org.ovirt.engine.core.vdsbroker.irsbroker"),
     DestroyStoragePool("org.ovirt.engine.core.vdsbroker.irsbroker"),
     DeleteImageGroup("org.ovirt.engine.core.vdsbroker.irsbroker"),
+    DeleteImageUnusedLinks("org.ovirt.engine.core.vdsbroker.vdsbroker"),
     MoveImageGroup("org.ovirt.engine.core.vdsbroker.irsbroker"),
     CloneImageGroupStructure("org.ovirt.engine.core.vdsbroker.irsbroker"),
     SyncImageGroupData("org.ovirt.engine.core.vdsbroker.irsbroker"),

backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/jsonrpc/JsonRpcVdsServer.java

@@ -2695,4 +2695,16 @@ public ScreenshotInfoReturn createScreenshot(String vmId) {
         Map<String, Object> response = new FutureMap(this.client, request).withIgnoreResponseKey();
         return new ScreenshotInfoReturn(response);
     }
+
+    @Override
+    public StatusOnlyReturn deleteImageUnusedLinks(String sdUUID, String spUUID, String imgUUID) {
+        JsonRpcRequest request =
+                new RequestBuilder("Image.deleteUnusedLinks")
+                        .withParameter("storagedomainID", sdUUID)
+                        .withParameter("storagepoolID", spUUID)
+                        .withParameter("imageID", imgUUID)
+                        .build();
+        Map<String, Object> response = new FutureMap(this.client, request);
+        return new StatusOnlyReturn(response);
+    }
 }

backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/DeleteImageUnusedLinksVDSCommand.java

@@ -0,0 +1,39 @@
+package org.ovirt.engine.core.vdsbroker.vdsbroker;
+
+import org.ovirt.engine.core.common.vdscommands.DeleteImageUnusedLinksVDSCommandParameters;
+
+public class DeleteImageUnusedLinksVDSCommand<P extends DeleteImageUnusedLinksVDSCommandParameters>
+        extends VdsBrokerCommand<P> {
+
+    protected StatusOnlyReturn statusReturn;
+
+    public DeleteImageUnusedLinksVDSCommand(P parameters) {
+        super(parameters);
+    }
+
+    @Override
+    protected Status getReturnStatus() {
+        return statusReturn.status;
+    }
+
+    @Override
+    protected Object getReturnValueFromBroker() {
+        return statusReturn;
+    }
+
+    @Override
+    public Object getReturnValue() {
+        return statusReturn;
+    }
+
+    @Override
+    protected void executeVdsBrokerCommand() {
+        DeleteImageUnusedLinksVDSCommandParameters params = getParameters();
+        statusReturn = getBroker().deleteImageUnusedLinks(
+                params.getSdUUID().toString(),
+                params.getSpUUID().toString(),
+                params.getImgUUID().toString());
+
+        proceedProxyReturnValue();
+    }
+}

backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/IVdsServer.java

@@ -610,4 +610,5 @@ StatusOnlyReturn glusterVolumeResetBrickCommitForce(String volumeName,
 
     ScreenshotInfoReturn createScreenshot(String vmId);
 
+    StatusOnlyReturn deleteImageUnusedLinks(String sdUUID, String spUUID, String imgUUID);
 }

backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/NullVdsServer.java

@@ -1109,4 +1109,9 @@ public GlusterVolumeGlobalOptionsInfoReturn glusterVolumeGlobalOptionsGet() {
     public ScreenshotInfoReturn createScreenshot(String vmId) {
         return null;
     }
+
+    @Override
+    public StatusOnlyReturn deleteImageUnusedLinks(String sdUUID, String spUUID, String imgUUID) {
+        return null;
+    }
 }