Note: This bug is displayed in read-only format because the product is no longer active in Red Hat Bugzilla.

Bug 1122510

Summary:	Align "wipe after delete" option for file disks between GUI and REST
Product:	[Retired] oVirt	Reporter:	Ori Gofen <ogofen>
Component:	ovirt-engine-core	Assignee:	Idan Shaby <ishaby>
Status:	CLOSED CURRENTRELEASE	QA Contact:	Ori Gofen <ogofen>
Severity:	medium	Docs Contact:
Priority:	unspecified
Version:	3.5	CC:	acanan, amureini, bugs, ecohen, gklein, iheim, juan.hernandez, ogofen, rbalakri, scohen, yeylon
Target Milestone:	---	Flags:	scohen: needinfo+ scohen: needinfo+
Target Release:	3.5.0
Hardware:	Unspecified
OS:	Unspecified
Whiteboard:	storage
Fixed In Version:	ovirt-3.5.0_rc2	Doc Type:	Bug Fix
Doc Text:	Cause: Irrelevant Consequence: Up until now, it was possible to change the property "Wipe After Delete" for file disks only via the REST API. Fix: Enabled to do that also via the GUI. Result: The "Wipe After Delete" check box in the "Add Virtual Disk" and "Edit Virtual Disk" dialogs is now editable. Note: on file-based storage domain (NSF, POSIX, GlusterFS and local FS) disks will not be wiped in any case - this is a logical setting so that a disk can be moved between file and block storage and retain this property.	Story Points:	---
Clone Of:
Clones:	1127781 (view as bug list)		Environment:
Last Closed:	2014-10-17 12:36:22 UTC	Type:	Bug
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	Storage	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:
Bug Depends On:
Bug Blocks:	1070823

Description Ori Gofen 2014-07-23 12:07:17 UTC

Description of problem:
Modifying "wipe after delete" flag is available via rest-api while oVirt docs state this flag should be not available for file domain(UI greys this option as expected).

engine=# SELECT vm_names,storage_type,disk_id,storage_name,wipe_after_delete from images_storage_domain_view ;
 vm_names | storage_type |               disk_id         | storage_name | wipe_after_delete 
----------+--------------+--------------------------------------+--------------+-------------------
 vm_test  |            3 | e1a05b5b-5781-4bda-9cdf-c165359aa8a1 | ISCSI     | f
 vm_test  |            1 | 64feaf6f-1333-4517-9f75-a0038bb55cee | nfs       | t
 vm_test  |            3 | 8a422c1e-7cb4-4dba-b94e-44272c89bf40 | ISCSI     | f
 

Version-Release number of selected component (if applicable):
beta.1 && av10.3

How reproducible:
100%

Steps to Reproduce:
SomeNfsDisk.set_wipe_after_delete(True)
SomeNfsDisk.update()

Actual results:
operation successful

Expected results:
operation should be disabled

Additional info:

Comment 1 Juan Hernández 2014-07-23 12:53:30 UTC

The RESTAPI doesn't contain logic to check the type of the the storage domain, it just passes the data given by the caller to the backend. If this should be ignored or cause an error then it should be done in the backend.

Comment 2 Allon Mureinik 2014-07-23 14:18:19 UTC

(In reply to Ori from comment #0)
> Description of problem:
> Modifying "wipe after delete" flag is available via rest-api while oVirt
> docs state this flag should be not available for file domain(UI greys this
> option as expected).

Actually, I'm not quite sure this is the way we'd like to do. Since 3.4 we allow moving disks from block storage to file storage, so perhaps we'd like to keep this parameter as a "logical" parameter and only act upon in case the underlying storage makes sense (see bug 1097820).

However, I agree the two interfaces should be aligned - either both GUI and REST should allow it, or they should both block it.

Sean - your two cents?

Comment 3 Sean Cohen 2014-07-24 10:31:44 UTC

(In reply to Allon Mureinik from comment #2)
> (In reply to Ori from comment #0)
> > Description of problem:
> > Modifying "wipe after delete" flag is available via rest-api while oVirt
> > docs state this flag should be not available for file domain(UI greys this
> > option as expected).
> 
> Actually, I'm not quite sure this is the way we'd like to do. Since 3.4 we
> allow moving disks from block storage to file storage, so perhaps we'd like
> to keep this parameter as a "logical" parameter and only act upon in case
> the underlying storage makes sense (see bug 1097820).
> 
IMHO if the disk is moved from a block domain the flag should be set by the user

> However, I agree the two interfaces should be aligned - either both GUI and
> REST should allow it, or they should both block it.
> 
> Sean - your two cents?

Indeed the two interfaces should be aligned, to keep it simple they should both block it.
Sean

Comment 4 Allon Mureinik 2014-07-27 09:04:24 UTC

Sean, after giving the issue some more thought, I think the better approach would be to just allow setting it in the UI and change the field name to "treat securely" or something less asinine to the same effect. 

What happens when you move a disk with WAD=true from a block domain to a file domain?
- Blocking the operation would consist of bad UX
- Clearing the WAD flag would be inconsistent with other move operations, which do not modify other properties. Additionally, consider the flow of moving a disk block1>file->block. You'd end up with a disk with different properties although you theoretically moved it back to where it came from.
- Simply allowing it will have all the problems of the current situation, with the additional inconsistency of why can I get a WAD disk on a file domain by moving it but not straight-forward by creating it.

My suggestion:
- Allow it in the GUI
- Change the property to address the functionality and not the implementation (e.g., "secure deleting")
- Add a warning when moving from a file domain to a block domain.

Sean - ack/nack

Comment 5 Sean Cohen 2014-07-29 05:41:01 UTC

(In reply to Allon Mureinik from comment #4)
> Sean, after giving the issue some more thought, I think the better approach
> would be to just allow setting it in the UI and change the field name to
> "treat securely" or something less asinine to the same effect. 
> 
> What happens when you move a disk with WAD=true from a block domain to a
> file domain?
> - Blocking the operation would consist of bad UX
> - Clearing the WAD flag would be inconsistent with other move operations,
> which do not modify other properties. Additionally, consider the flow of
> moving a disk block1>file->block. You'd end up with a disk with different
> properties although you theoretically moved it back to where it came from.

Indeed, this raises another security consistency consideration...

> - Simply allowing it will have all the problems of the current situation,
> with the additional inconsistency of why can I get a WAD disk on a file
> domain by moving it but not straight-forward by creating it.
> 
> My suggestion:
> - Allow it in the GUI
> - Change the property to address the functionality and not the
> implementation (e.g., "secure deleting")
> - Add a warning when moving from a file domain to a block domain.
+1 on the warning,
Ack on "secure deleting" property change.
Sean

Comment 6 Idan Shaby 2014-08-07 14:34:49 UTC

I added a clone of this bug for the warning part - bug 1127781.
This bug will deal with enabling "wipe after delete" for file domains.

Comment 7 Ori Gofen 2014-09-14 15:13:17 UTC

reproduced on rhevm vt3.1

Comment 8 Allon Mureinik 2014-09-15 06:51:51 UTC

Ori, this worked fine on our dev envs - please add exact steps to reproduce and the output of "rpm -qa | grep ovirt".
Thanks!

Comment 9 Allon Mureinik 2014-09-15 07:48:53 UTC

To summarize the expected result:

1. Via GUI/REST you can always specify Wipe After Delete.
2. On File Domains, this property is not acted upon (as per bug 1097820).
3. Looking forward - we should have a warning when MOVING a disk WITHOUT Wipe After Delete for a file domain (as per bug 1127781).

The verification of THIS bug should only check (1).

Comment 10 Ori Gofen 2014-09-15 07:56:47 UTC

agreed,hence,the steps to verify this bug are now changed and will be as follows: 

1. check via UI,rest-api that "wipe_after_delete" check flag is enabled and not
greyed out on every creation pop up window.

2. deleting those disks (on File) sends PostZero=False to vdsm

Comment 11 Ori Gofen 2014-09-15 09:01:12 UTC

verified on vt3.1

Comment 12 Sandro Bonazzola 2014-10-17 12:36:22 UTC

oVirt 3.5 has been released and should include the fix for this issue.

Comment 13 Allon Mureinik 2015-06-07 08:45:33 UTC

*** Bug 1223697 has been marked as a duplicate of this bug. ***