Change in ovirt-engine[master]: webadmin: Updated SPICE cab download fix (#846341)

ecohen at redhat.com ecohen at redhat.com
Thu Aug 9 17:34:12 UTC 2012


Einav Cohen has submitted this change and it was merged.

Change subject: webadmin: Updated SPICE cab download fix (#846341)
......................................................................


webadmin: Updated SPICE cab download fix (#846341)

https://bugzilla.redhat.com/846341

Before this patch, this was the situation:

class Configurator {

   protected abstract Event getSpiceVersionFileFetchedEvent();

   // Configurator c'tor
   public Configurator() {
      ...
      ...
      updateSpiceVersion();
   }

   private void updateSpiceVersion() {
      doSomething(getSpiceVersionFileFetchedEvent());
   }
}

---

class WebAdminConfigurator extends Configurator {

   public Event spiceVersionFileFetchedEvent = [initialized];

   // WebAdminConfigurator c'tor
   public WebAdminConfigurator(EventBus eventBus) {
      super();
      ...
      ...
   }

   @Override
   protected Event getSpiceVersionFileFetchedEvent() {
      return spiceVersionFileFetchedEvent;
   }
}

In the WebAdminConfigurator c'tor, there is a call to "super()", i.e.,
Configurator c'tor, that calls to updateSpiceVersion(), which calls to
getSpiceVersionFileFetchedEvent().
getSpiceVersionFileFetchedEvent() is an abstract method in Configurator.
Its "implementation" (override) in WebAdminConfigurator returns the
spiceVersionFileFetchedEvent member which, theoretically, should be
initialized with the "[initialized]" value.

However, practically, since the above is happenning during the base
class (Configurator) c'tor execution, the WebAdminConfigurator members'
initialization hasn't occurred yet, therefore,
"spiceVersionFileFetchedEvent" is actually null, and not initialized to
"[initialized]" yet.

This prevents the "post-fetch-spice-version-file" event from being
treated by the relevant event handler, hence the actual server-side
SPICE ActiveX cab version is not updated appropriately within
the configurator; so the server-side spice-version value practically
supplied by the Configurator is the default hard-coded value with which
the spiceVersion Configurator member was initialized, i.e. "4.4".

So in case the client has a spice ActiveX with a version greater than
4.4, it won't download the SPICE ActiveX cab from the server, since the
client thinks that the version of the server-side cab is 4.4, i.e.,
less than what is currently installed, so no reason to download and
upgrade the ActiveX.

If the actual version of the server-side cab is greater than 4.4, it can
cause a buggy behavior in case client is installed with a version
smaller than the actual server-side version: Updated server-side cab
should be downloaded to the client since its actual version is larger
than the one installed on the client, but due to this bug - it won't
happen.

E.g.:
- reported (buggy) server-side SPICE version: 4.4
- actual server-side SPICE version: 5.0
- SPICE version installed on the client: 4.8.
- client asks server for the server-side-SPICE version.
- server returns: 4.4
- client compares currently-installed version (4.8) against server-side
  version (4.4) -> 4.4 is smaller than 4.8 -> client concludes that
  there is no need to download server-side cab.

This is incorrect behavior - actual server-side SPICE version is 5.0,
and *should* be downloaded to the client in this case.

In this patch, the call to updateSpiceVersion() is done in the
WebAdminConfigurator (and UserPortalConfigurator) c'tor, instead of
the Configurator c'tor, in a stage in which the
spiceVersionFileFetchedEvent member is already initialized;
Therefore, once the SPICE version txt file is fetched and read, the
version is updated correctly within the configurator, server reports
correct version of server-side cab, hence client downloads updated
server-side cab when it should.

Change-Id: I100e722ed85554451d51e9ddb73f3bb35522684f
Signed-off-by: Einav Cohen <ecohen at redhat.com>
---
M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/Configurator.java
M frontend/webadmin/modules/userportal-gwtp/src/main/java/org/ovirt/engine/ui/userportal/uicommon/UserPortalConfigurator.java
M frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/uicommon/WebAdminConfigurator.java
3 files changed, 7 insertions(+), 4 deletions(-)

Approvals:
  Einav Cohen: Verified; Looks good to me, approved


--
To view, visit http://gerrit.ovirt.org/7065
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I100e722ed85554451d51e9ddb73f3bb35522684f
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Einav Cohen <ecohen at redhat.com>
Gerrit-Reviewer: Daniel Erez <derez at redhat.com>
Gerrit-Reviewer: Einav Cohen <ecohen at redhat.com>



More information about the Engine-commits mailing list