core: set sequence_number when creating new snapshots

Set the sequenceNumber and nextSequenceNumber properties where appropriate, to be passed on to the database or vdsm upon creation. Signed-off-by: Benny Zlotnik <bzlotnik@redhat.com> Bug-Url: https://bugzilla.redhat.com/977778 Change-Id: I42cb4943f21601ad05aee8d241f410834ae92c97
oVirt · Feb 23, 2022 · 92824ff · 92824ff
1 parent d1d8f66
commit 92824ff
Show file tree

Hide file tree

Showing 15 changed files with 90 additions and 17 deletions.
diff --git a/.../modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/CreateSnapshotCommand.java b/.../modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/CreateSnapshotCommand.java
@@ -112,13 +112,24 @@ protected boolean performImageVdsmOperation() {
         // storage team request
         newDiskImage.setVolumeType(VolumeType.Sparse);
         newDiskImage.setVolumeFormat(VolumeFormat.COW);
+
+        // If we are creating a thin VM it makes sense to use the default sequence for the new
+        // image as the template image is not "really" part of the disk, but merely a symlink,
+        // so having a different sequence number doesn't provide much benefit
+        int nextSequenceNumber = 1;
+        if (getActionType() != ActionType.CreateSnapshotFromTemplate) {
+            nextSequenceNumber = imageDao.getMaxSequenceNumber(getImageGroupId()) + 1;
+        }
+
+        newDiskImage.getImage().setSequenceNumber(nextSequenceNumber);
+
         try {
             Guid taskId = persistAsyncTaskPlaceHolder(getParameters().getParentCommand());
 
             VDSReturnValue vdsReturnValue =
                     runVdsCommand(
                             VDSCommandType.CreateVolume,
-                            getCreateVDSCommandParameters());
+                            getCreateVDSCommandParameters(nextSequenceNumber));
 
             if (vdsReturnValue != null && vdsReturnValue.getSucceeded()) {
                 getParameters().setVdsmTaskIds(new ArrayList<>());
@@ -147,7 +158,7 @@ protected boolean performImageVdsmOperation() {
         return false;
     }
 
-    private CreateVolumeVDSCommandParameters getCreateVDSCommandParameters() {
+    private CreateVolumeVDSCommandParameters getCreateVDSCommandParameters(int nextSequenceNumber) {
         CreateVolumeVDSCommandParameters parameters = new CreateVolumeVDSCommandParameters(getStoragePoolId(),
                 getDestinationStorageDomainId(),
                 getImageGroupId(),
@@ -159,7 +170,8 @@ private CreateVolumeVDSCommandParameters getCreateVDSCommandParameters() {
                 getDestinationImageId(),
                 "",
                 getStoragePool().getCompatibilityVersion(),
-                getDiskImage().getContentType());
+                getDiskImage().getContentType(),
+                nextSequenceNumber);
         if (getParameters().getInitialSizeInBytes() != null) {
             parameters.setImageInitialSizeInBytes(getParameters().getInitialSizeInBytes());
         }

diff --git a/...rc/main/java/org/ovirt/engine/core/bll/storage/disk/image/AddImageFromScratchCommand.java b/...rc/main/java/org/ovirt/engine/core/bll/storage/disk/image/AddImageFromScratchCommand.java
@@ -78,6 +78,10 @@ protected void executeCommand() {
         newDiskImage.setContentType(getParameters().getDiskInfo().getContentType());
         newDiskImage.setBackup(getParameters().getDiskInfo().getBackup());
 
+        // We can assume this command is used only when adding the first image, setting the first sequence
+        // number to 1.
+        newDiskImage.getImage().setSequenceNumber(1);
+
         TransactionSupport.executeInNewTransaction(() -> {
             if (!getParameters().isShouldRemainIllegalOnFailedExecution()) {
                 addDiskImageToDb(newDiskImage, getCompensationContext(), Boolean.TRUE);
@@ -91,18 +95,18 @@ protected void executeCommand() {
         if (getParameters().isShouldRemainIllegalOnFailedExecution()) {
             getReturnValue().setActionReturnValue(newDiskImage);
         }
-        processImageInIrs();
+        processImageInIrs(newDiskImage.getImage().getSequenceNumber());
         getReturnValue().setActionReturnValue(newDiskImage);
         setSucceeded(true);
     }
 
-    protected boolean processImageInIrs() {
+    protected boolean processImageInIrs(int sequenceNumber) {
         if (getParameters().getDiskInfo().getDiskStorageType() == DiskStorageType.KUBERNETES) {
             return true;
         }
         Guid taskId = persistAsyncTaskPlaceHolder(getParameters().getParentCommand());
         VDSReturnValue vdsReturnValue = runVdsCommand(VDSCommandType.CreateVolume,
-                getCreateVolumeVDSCommandParameters());
+                getCreateVolumeVDSCommandParameters(sequenceNumber));
         if (vdsReturnValue.getSucceeded()) {
             getParameters().setVdsmTaskIds(new ArrayList<>());
             getParameters().getVdsmTaskIds().add(
@@ -131,7 +135,7 @@ private Long getInitialSize() {
         return initialSize;
     }
 
-    private CreateVolumeVDSCommandParameters getCreateVolumeVDSCommandParameters() {
+    private CreateVolumeVDSCommandParameters getCreateVolumeVDSCommandParameters(int sequenceNumber) {
         CreateVolumeVDSCommandParameters parameters =
                 new CreateVolumeVDSCommandParameters(
                         getParameters().getStoragePoolId(),
@@ -145,7 +149,8 @@ private CreateVolumeVDSCommandParameters getCreateVolumeVDSCommandParameters() {
                         getDestinationImageId(),
                         imagesHandler.getJsonDiskDescription(getParameters().getDiskInfo()),
                         getStoragePool().getCompatibilityVersion(),
-                        getParameters().getDiskInfo().getContentType()
+                        getParameters().getDiskInfo().getContentType(),
+                        sequenceNumber
                 );
 
         // The initial size of a backup scratch disk shouldn't be overridden.

diff --git a/.../org/ovirt/engine/core/bll/storage/disk/image/CloneImageGroupVolumesStructureCommand.java b/.../org/ovirt/engine/core/bll/storage/disk/image/CloneImageGroupVolumesStructureCommand.java
@@ -174,7 +174,8 @@ private void createImage(DiskImage image, int imageIndex) {
                 image.getVolumeType(),
                 getParameters().getDescription(),
                 image.getSize(),
-                initialSize);
+                initialSize,
+                innerImage.getSequenceNumber());
 
         parameters.setEndProcedure(EndProcedure.COMMAND_MANAGED);
         parameters.setParentCommand(getActionType());

diff --git a/...main/java/org/ovirt/engine/core/bll/storage/disk/image/CopyImageGroupWithDataCommand.java b/...main/java/org/ovirt/engine/core/bll/storage/disk/image/CopyImageGroupWithDataCommand.java
@@ -147,7 +147,10 @@ private void createVolume() {
                 getDiskImage().getSize(),
                 determineTotalImageInitialSize(getDiskImage(),
                         getParameters().getDestinationFormat(),
-                        getParameters().getSrcDomain()));
+                        getParameters().getSrcDomain()),
+                // We can use 1 because this is only used when collapsing, no point using the leaf's sequence
+                // number
+                1);
         parameters.setDiskAlias(getParameters().getDiskAlias());
         parameters.setJobWeight(getParameters().getOperationsJobWeight().get(CopyStage.DEST_CREATION.name()));
         parameters.setParentCommand(getActionType());

diff --git a/.../main/java/org/ovirt/engine/core/bll/storage/disk/image/CreateVolumeContainerCommand.java b/.../main/java/org/ovirt/engine/core/bll/storage/disk/image/CreateVolumeContainerCommand.java
@@ -70,7 +70,8 @@ private CreateVolumeVDSCommandParameters getCreateVDSCommandParameters() {
                         getParameters().getImageId(),
                         imagesHandler.getJsonDiskDescription(diskImageDescription),
                         getStoragePool().getCompatibilityVersion(),
-                        DiskContentType.DATA);
+                        DiskContentType.DATA,
+                        getParameters().getSequenceNumber());
         parameters.setLegal(getParameters().isLegal());
         if (getType() != VolumeType.Preallocated && getStorageDomain().getStorageType().isBlockDomain()) {
             parameters.setImageInitialSizeInBytes(Optional.ofNullable(getParameters().getInitialSize()).orElse(0L));

diff --git a/...main/java/org/ovirt/engine/core/common/action/CreateVolumeContainerCommandParameters.java b/...main/java/org/ovirt/engine/core/common/action/CreateVolumeContainerCommandParameters.java
@@ -13,13 +13,15 @@ public class CreateVolumeContainerCommandParameters extends StorageJobCommandPar
     private Long initialSize;
     private boolean legal = true;
 
+    private Integer sequenceNumber;
+
     public CreateVolumeContainerCommandParameters() {
     }
 
     public CreateVolumeContainerCommandParameters(Guid storagePoolId, Guid storageDomainId, Guid srcImageGroupId,
                                                   Guid srcImageId, Guid imageGroupId, Guid imageId,
                                                   VolumeFormat volumeFormat, VolumeType volumeType, String description, long size,
-                                                  Long initialSize) {
+                                                  Long initialSize, Integer sequenceNumber) {
         setStoragePoolId(storagePoolId);
         setStorageDomainId(storageDomainId);
         setImageGroupID(imageGroupId);
@@ -31,6 +33,7 @@ public CreateVolumeContainerCommandParameters(Guid storagePoolId, Guid storageDo
         this.volumeFormat = volumeFormat;
         this.volumeType = volumeType;
         this.initialSize = initialSize;
+        this.sequenceNumber = sequenceNumber;
         fillEntityInfo(imageId);
     }
 
@@ -89,4 +92,12 @@ public boolean isLegal() {
     public void setLegal(boolean legal) {
         this.legal = legal;
     }
+
+    public Integer getSequenceNumber() {
+        return sequenceNumber;
+    }
+
+    public void setSequenceNumber(Integer sequenceNumber) {
+        this.sequenceNumber = sequenceNumber;
+    }
 }
diff --git a/...common/src/main/java/org/ovirt/engine/core/common/businessentities/storage/DiskImage.java b/...common/src/main/java/org/ovirt/engine/core/common/businessentities/storage/DiskImage.java
@@ -115,6 +115,7 @@ protected DiskImage(DiskImage diskImage) {
         setLastModified(new Date());
         setImageStatus(ImageStatus.LOCKED);
         setDiskProfileId(diskImage.getDiskProfileId());
+        getImage().setSequenceNumber(diskImage.getImage().getSequenceNumber());
     }
 
     public Guid getImageId() {

diff --git a/.../main/java/org/ovirt/engine/core/common/vdscommands/CreateVolumeVDSCommandParameters.java b/.../main/java/org/ovirt/engine/core/common/vdscommands/CreateVolumeVDSCommandParameters.java
@@ -16,6 +16,7 @@ public class CreateVolumeVDSCommandParameters extends StoragePoolDomainAndGroupI
     private long imageInitialSizeInBytes;
     private boolean shouldAddBitmaps;
     private boolean legal = true;
+    private Integer sequenceNumber;
 
     // Initialize with Guid.Empty for creating a new image.
     private Guid _imageId;
@@ -32,7 +33,8 @@ public CreateVolumeVDSCommandParameters(Guid storagePoolId,
             Guid newImageId,
             String newImageDescription,
             Version compatibilityVersion,
-            DiskContentType diskContentType) {
+            DiskContentType diskContentType,
+            Integer sequenceNumber) {
         super(storagePoolId, storageDomainId, imageGroupId);
         _imageSizeInBytes = imageSizeInBytes;
         _imageType = imageType;
@@ -43,6 +45,7 @@ public CreateVolumeVDSCommandParameters(Guid storagePoolId,
         setDiskContentType(diskContentType);
         _imageId = imageId;
         setSourceImageGroupId(sourceImageGroupId);
+        this.sequenceNumber = sequenceNumber;
     }
 
     public CreateVolumeVDSCommandParameters() {
@@ -143,6 +146,14 @@ public void setLegal(boolean legal) {
         this.legal = legal;
     }
 
+    public Integer getSequenceNumber() {
+        return sequenceNumber;
+    }
+
+    public void setSequenceNumber(Integer sequenceNumber) {
+        this.sequenceNumber = sequenceNumber;
+    }
+
     @Override
     protected ToStringBuilder appendAttributes(ToStringBuilder tsb) {
         return super.appendAttributes(tsb)
@@ -155,6 +166,7 @@ protected ToStringBuilder appendAttributes(ToStringBuilder tsb) {
                 .append("imageId", getImageId())
                 .append("sourceImageGroupId", getSourceImageGroupId())
                 .append("shouldAddBitmaps", shouldAddBitmaps())
-                .append("legal", isLegal());
+                .append("legal", isLegal())
+                .append("sequenceNumber", getSequenceNumber());
     }
 }
diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/ImageDao.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/ImageDao.java
@@ -13,4 +13,6 @@ public interface ImageDao extends GenericDao<Image, Guid>, StatusAwareDao<Guid,
     public void updateImageVmSnapshotId(Guid id, Guid vmSnapshotId);
 
     public void updateImageSize(Guid id, long size);
+
+    public Integer getMaxSequenceNumber(Guid imageGroupId);
 }
diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/ImageDaoImpl.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/ImageDaoImpl.java
@@ -49,6 +49,15 @@ public void updateImageSize(Guid id, long size) {
         getCallsHandler().executeModification("UpdateImageSize", parameterSource);
     }
 
+    @Override
+    public Integer getMaxSequenceNumber(Guid imageGroupId) {
+        MapSqlParameterSource parameterSource = getCustomMapSqlParameterSource()
+                .addValue("image_group_id", imageGroupId);
+        RowMapper<Integer> mapper = (rs, rowNum) -> rs.getInt(1);
+
+        return getCallsHandler().executeRead("GetMaxSequenceNumber", mapper, parameterSource);
+    }
+
     @Override
     protected MapSqlParameterSource createFullParametersMapper(Image entity) {
         return createIdParameterMapper(entity.getId())

diff --git a/...roker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/CreateVolumeVDSCommand.java b/...roker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/CreateVolumeVDSCommand.java
@@ -41,7 +41,8 @@ protected void executeIrsBrokerCommand() {
                 getParameters().getImageId().toString(),
                 imageInitSize,
                 getParameters().shouldAddBitmaps(),
-                getParameters().isLegal());
+                getParameters().isLegal(),
+                getParameters().getSequenceNumber());
 
         proceedProxyReturnValue();
         Guid taskID = new Guid(uuidReturn.uuid);

diff --git a/...modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IIrsServer.java b/...modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IIrsServer.java
@@ -13,7 +13,7 @@ public interface IIrsServer {
 
     OneUuidReturn createVolume(String sdUUID, String spUUID, String imgGUID, String size, int volFormat,
             int volType, String diskType, String volUUID, String descr, String srcImgGUID, String srcVolUUID,
-            String initialSize, boolean shouldAddBitmaps, boolean legal);
+            String initialSize, boolean shouldAddBitmaps, boolean legal, int sequenceNumber);
 
     OneUuidReturn copyImage(String sdUUID, String spUUID, String vmGUID, String srcImgGUID, String srcVolUUID,
             String dstImgGUID, String dstVolUUID, String descr, String dstSdUUID, int volType, int volFormat,

diff --git a/...es/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/jsonrpc/JsonRpcIIrsServer.java b/...es/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/jsonrpc/JsonRpcIIrsServer.java
@@ -50,7 +50,8 @@ public OneUuidReturn createVolume(String sdUUID,
             String srcVolUUID,
             String initialSize,
             boolean shouldAddBitmaps,
-            boolean legal) {
+            boolean legal,
+            int sequenceNumber) {
         JsonRpcRequest request =
                 new RequestBuilder("Volume.create").withParameter("volumeID", volUUID)
                         .withParameter("storagepoolID", spUUID)
@@ -63,6 +64,7 @@ public OneUuidReturn createVolume(String sdUUID,
                         .withParameter("desc", descr)
                         .withParameter("srcImgUUID", srcImgGUID)
                         .withParameter("srcVolUUID", srcVolUUID)
+                        .withParameter("sequence", sequenceNumber)
                         .withOptionalParameter("initialSize", initialSize)
                         .withOptionalParameter("addBitmaps", shouldAddBitmaps)
                         .withOptionalParameter("legal", legal)

diff --git a/...oker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/GetVolumeInfoVDSCommand.java b/...oker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/GetVolumeInfoVDSCommand.java
@@ -105,6 +105,10 @@ public DiskImage buildImageEntity(Map<String, Object> struct) {
             if (struct.containsKey("generation")) {
                 newImage.getImage().setGeneration(Integer.valueOf(struct.get("generation").toString()));
             }
+
+            if (struct.containsKey("sequence")) {
+                newImage.getImage().setSequenceNumber(Integer.valueOf(struct.get("sequence").toString()));
+            }
         } catch (RuntimeException ex) {
             log.error("Failed building DiskImage: {}", ex.getMessage());
             printReturnValue();

diff --git a/packaging/dbscripts/images_sp.sql b/packaging/dbscripts/images_sp.sql
@@ -195,3 +195,12 @@ BEGIN
     ORDER BY repo_image_name;
 END;$PROCEDURE$
 LANGUAGE plpgsql;
+
+
+CREATE OR REPLACE FUNCTION GetMaxSequenceNumber (v_image_group_id UUID)
+RETURNS int AS $FUNCTION$
+    SELECT MAX(sequence_number)
+    FROM images
+    WHERE image_group_id = v_image_group_id;
+$FUNCTION$
+LANGUAGE sql;