Bug 845466

wipe after delete in gui default to true based on type of storage iirc.
need to decide if default behavior should be the same by backend doing it, rather than client (problem is the need to expose it to the UI, which means UI needs to rely on backend logic to display the field)

I'm not sure API should not require user to be more specific on such details.

Maybe a good solution which can prevent mistakes such as this one , or bootable field is to set those fields to be mandatory . 
 

(In reply to comment #1)
> wipe after delete in gui default to true based on type of storage iirc.
> need to decide if default behavior should be the same by backend doing it,
> rather than client (problem is the need to expose it to the UI, which means
> UI needs to rely on backend logic to display the field)
> 
> I'm not sure API should not require user to be more specific on such details.

(In reply to comment #1)
> wipe after delete in gui default to true based on type of storage iirc.
> need to decide if default behavior should be the same by backend doing it,
> rather than client (problem is the need to expose it to the UI, which means
> UI needs to rely on backend logic to display the field)
> 
> I'm not sure API should not require user to be more specific on such details.

According to bug 845463 we think it's wrong thus blocking in the GUI.
If we think it's totally wrong then we should block at the BE as well.

Block can be - default is True, but ignore this for FS based file system 
               default is according to the storage type, and return error if trying to override for FS types.

(In reply to comment #3)
> (In reply to comment #1)
> > wipe after delete in gui default to true based on type of storage iirc.
> > need to decide if default behavior should be the same by backend doing it,
> > rather than client (problem is the need to expose it to the UI, which means
> > UI needs to rely on backend logic to display the field)
> > 
> > I'm not sure API should not require user to be more specific on such details.
> 
> According to bug 845463 we think it's wrong thus blocking in the GUI.
> If we think it's totally wrong then we should block at the BE as well.
> 
> Block can be - default is True, but ignore this for FS based file system 
>                default is according to the storage type, and return error if
> trying to override for FS types.

the question is not 'what', but 'where',
+1 for keeping defaults in the BE, UI can duplicate/reuse BE logic/rule for displaying purposes, other client should pass NULL and let BE set appropriate
value.

The change in the BaseDisk class is changing the type of wipeAfterDelete flag ro refercne type Boolean and the addition of a new method

     public boolean isWipeAfterDeleteSet() {
         return wipeAfterDelete != null;
     }

and change in isWipeAfterDelete method

     public boolean isWipeAfterDelete() {
         if (isWipeAfterDeleteSet()) {
             return wipeAfterDelete;
         }
         return false;
     }

The backend handles the logic when wipeAfterDelete flag is not set.
So in AddDiskCommand executeVmCommand method we check if the flag is
set, otherwise we get the storage type and get the default value using the
new Utility class WipeAfterDeleteUtils

        if(!getParameters().getDiskInfo().isWipeAfterDeleteSet()) {
             StorageType storageType =
             getStorageDomain().getstorage_type();
getParameters().getDiskInfo().setWipeAfterDelete(WipeAfterDeleteUtils.getDefaultWipeAfterDeleteFlag(storageType));
         }

and the Utility class is a very simple class that extracts the
default value based on the storagetype

public class WipeAfterDeleteUtils {

     static Boolean wipeAfterDeleteBlockStorageDomain;

     static boolean wipeAfterDeleteFileStorageDomain = false;

     public static synchronized boolean
getDefaultWipeAfterDeleteFlag(final StorageType storageType) {
         if(storageType.isBlockDomain()) {
             if(wipeAfterDeleteBlockStorageDomain == null) {
                     wipeAfterDeleteBlockStorageDomain =
Config.<Boolean>GetValue(ConfigValues.SANWipeAfterDelete);
             }
             return wipeAfterDeleteBlockStorageDomain;
         }
         if(storageType.isFileDomain()) {
             return wipeAfterDeleteFileStorageDomain;
         }
         return false;
     }
}

Link http://gerrit.ovirt.org/#/c/8628/

Commit hash : 4bc5a2565fb9d62daeeb6cbc15570f2995759898

Correct Commit Hash : 5824c91db5d83540d941d220187bc7f7c32d015e

what to test here (notes for QE)?

For File Storage Domains

1. Create a VM on a File Storage Domain (NFS) 
2. Configure a Virtual Disk
   Wipe After Delete check box should be disabled
3. After the virtual disk has been created, the database table "base_disks" should have a new row with wipe_after_delete flag set to FALSE


For Block Storage Domains

1. Create a VM on a Block Storage Domain (SAN) 
2. Configure a Virtual Disk
   Wipe After Delete check box should be enabled and can be changed by the user
3. After the virtual disk has been created, the database table base_disks should have a new row with wipe_after_delete flag set to TRUE if the user checked the check box.
4. If the user did not check the wipe after delete box the value in the database should correspond to the value for 'SANWipeAfterDelete' flag in the database table "vdc_options"

Verified rhevm-3.1.0-22.el6ev.noarch