"Wipe after delete" defaults to true in GUI, but false in REST API. They should be the same to avoid issues. There are more instances of this problem, so please think about some way how to keep defaults in sync in all places (e.g. a bug 842344 for bootable disk default).
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