On 10/11/2012 05:28 PM, Daniel Erez wrote:


----- Original Message -----
From: "Ravi Nori" <[email protected]>
To: "Daniel Erez" <[email protected]>
Cc: [email protected]
Sent: Thursday, October 11, 2012 3:21:05 PM
Subject: Re: [Engine-devel] "wipe after delete" Matching defaults from GUI to 
Backend

On 10/10/2012 01:17 AM, Daniel Erez wrote:

----- Original Message -----
From: "Ravi Nori" <[email protected]>
To: [email protected]
Sent: Tuesday, October 9, 2012 6:40:56 PM
Subject: [Engine-devel] "wipe after delete" Matching defaults from
GUI to  Backend

I am working on the bug where the defaults values in the GUI for
disk
actions don't correspond to the default values in the API/Backend.
For
example the "wipe after delete" flag defaults to true in the GUI
but
false in the backend code. Default values are not sent from the
frontend
to the backend. Please refer to the bugzilla link

https://bugzilla.redhat.com/show_bug.cgi?id=845466

After discussing with Michael as to how to approach fixing this,
we
think the best way to do it would be to fix it in the Backend
class
businessentities.Disk by replacing primitives with reference types
and
forcing business logic in the class to set the default values to
match
the default values in the client.

I would appreciate some feedback on this and also on what the
default
values are for Disk operations on the GUI.
GUI behavior is as follows:
For File storage domains - WipeAfterDelete check-box is
unchangeable and unchecked (false).
For Block storage domains - WipeAfterDelete check-box is changeable
and
the default value is according to 'SANWipeAfterDelete'
configuration value.

I think you should simply utilize the same configuration value in
REST or backend when creating a new disk.

Thanks

Ravi
_______________________________________________
Engine-devel mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-devel

Thanks for the reply.

Here is my current implementation

I modified businessentities.BaseDisk class to change wipeAfterDelete
to
wrapper class (reference class) for boolean

      private Boolean wipeAfterDelete;

I also added default values for File and block storages
      private boolean wipeAfterDeleteFileStorageDomain = false;
      private boolean wipeAfterDeleteBlockStorageDomain =
Config.<Boolean>GetValue(ConfigValues.SANWipeAfterDelete);

The storage type determines which of the two default values is
returned
by BaseDisk when wipeAfterDelete is not set (is null)
      StorageType storageType = StorageType.UNKNOWN;

So isWipeAfterDeleteMethodLooks like this

      public boolean isWipeAfterDelete() {
          if (wipeAfterDelete == null) {
              return getDefaultIsWipeAfterDelete();
          }
          return wipeAfterDelete;
      }

      private boolean getDefaultIsWipeAfterDelete() {
          if(storageType.equals(StorageType.UNKNOWN)) {
              return false;
          }
          switch (storageType) {
              case ISCSI:
              case FCP:
                  return wipeAfterDeleteBlockStorageDomain;
              default:
                  return wipeAfterDeleteFileStorageDomain;
          }
      }

There is a new method isWipeAfterDeleteSet that will be used by
commands
to set the storage type to get the default wipeAfterDeleteFlag if
wipeAfterDelete was not set.

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

Now in bll.AddDiskCommand I modified executeVmCommand to add the
following

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

This approach minimizes the impact of changes. So all commands
interested in wipeAfterDelete flag based on storage type if
wipeAfterDelete has not been set need to set the storageType before
calling isWipeAfterDelete.

Please let me know if this is the solution conflicts with something
you
have been working on. All the existing code will work as it has been
working, if wipeAfterDelete has not been set and storageType has not
been set the method isWipeAfterDelete return false (like it was
earlier)



Calling 'Config.<?> GetValue' in a business entity could be problematic in the 
UI since
only the backend can invoke it (business entities are common for the backend 
and the UI
which means that both may instantiate/manipulate them).
We use GWT RPC for serializing/deserializing these business entities,
thus we usually can't reference non plain java objects (or use some 
tweaks/tricks if applicable).
Try to check the UI behavior after the new changes and let me know...
[as an alternative, you can probably get 'SANWipeAfterDelete' config value from 
REST code (or backend)].

clients (UI/API) can only call the ConfigurationValues enum, and should use the query for it (gui has this already of course, just look it up)
_______________________________________________
Engine-devel mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-devel

Reply via email to