From 102f4604582f5dec5c8656cd4e2a5eef41dc98ed Mon Sep 17 00:00:00 2001 From: Alena Prokharchyk Date: Mon, 28 Nov 2011 11:21:32 -0800 Subject: [PATCH] Fixed updateConfiguration - updateHostDetails used to swallow the exceptions --- .../cloud/api/commands/CreateAccountCmd.java | 8 ++-- api/src/com/cloud/user/AccountService.java | 2 +- .../ConfigurationManagerImpl.java | 44 +++++++++---------- .../com/cloud/user/AccountManagerImpl.java | 2 +- .../cloud/user/MockAccountManagerImpl.java | 2 +- 5 files changed, 28 insertions(+), 30 deletions(-) diff --git a/api/src/com/cloud/api/commands/CreateAccountCmd.java b/api/src/com/cloud/api/commands/CreateAccountCmd.java index 7d7d94ff8a3..583f91e725b 100755 --- a/api/src/com/cloud/api/commands/CreateAccountCmd.java +++ b/api/src/com/cloud/api/commands/CreateAccountCmd.java @@ -79,7 +79,7 @@ public class CreateAccountCmd extends BaseCmd { private String networkDomain; @Parameter(name = ApiConstants.ACCOUNT_DETAILS, type = CommandType.MAP, description = "details for account used to store specific parameters") - private Map details; + private Map details; ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -124,13 +124,13 @@ public class CreateAccountCmd extends BaseCmd { return networkDomain; } - public Map getDetails() { + public Map getDetails() { if (details == null || details.isEmpty()) { return null; } - Collection paramsCollection = details.values(); - Map params = (Map) (paramsCollection.toArray())[0]; + Collection paramsCollection = details.values(); + Map params = (Map) (paramsCollection.toArray())[0]; return params; } diff --git a/api/src/com/cloud/user/AccountService.java b/api/src/com/cloud/user/AccountService.java index 98e37a02fdd..1029f80ee85 100755 --- a/api/src/com/cloud/user/AccountService.java +++ b/api/src/com/cloud/user/AccountService.java @@ -45,7 +45,7 @@ public interface AccountService { * * @return the user if created successfully, null otherwise */ - UserAccount createUserAccount(String userName, String password, String firstName, String lastName, String email, String timezone, String accountName, short accountType, Long domainId, String networkDomain, Map details); + UserAccount createUserAccount(String userName, String password, String firstName, String lastName, String email, String timezone, String accountName, short accountType, Long domainId, String networkDomain, Map details); /** * Deletes a user by userId diff --git a/server/src/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/com/cloud/configuration/ConfigurationManagerImpl.java index 037a0f6ed2d..51c192af086 100755 --- a/server/src/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/com/cloud/configuration/ConfigurationManagerImpl.java @@ -322,74 +322,71 @@ public class ConfigurationManagerImpl implements ConfigurationManager, Configura throw new InvalidParameterValueException(validationMsg); } + //Execute all updates in a single transaction + Transaction txn = Transaction.currentTxn(); + txn.start(); + if (!_configDao.update(name, value)) { s_logger.error("Failed to update configuration option, name: " + name + ", value:" + value); throw new CloudRuntimeException("Failed to update configuration value. Please contact Cloud Support."); } - if (Config.XenGuestNetwork.key().equals(name)) { + + + PreparedStatement pstmt = null; + if (Config.XenGuestNetwork.key().equalsIgnoreCase(name)) { String sql = "update host_details set value=? where name=?"; - Transaction txn = Transaction.currentTxn(); - PreparedStatement pstmt = null; try { pstmt = txn.prepareAutoCloseStatement(sql); pstmt.setString(1, DBEncryptionUtil.encrypt(value)); pstmt.setString(2, "guest.network.device"); pstmt.executeUpdate(); - } catch (SQLException e) { } catch (Throwable e) { + throw new CloudRuntimeException("Failed to update guest.network.device in host_details due to exception ", e); } - } else if (Config.XenPrivateNetwork.key().equals(name)) { + } else if (Config.XenPrivateNetwork.key().equalsIgnoreCase(name)) { String sql = "update host_details set value=? where name=?"; - Transaction txn = Transaction.currentTxn(); - PreparedStatement pstmt = null; try { pstmt = txn.prepareAutoCloseStatement(sql); pstmt.setString(1, DBEncryptionUtil.encrypt(value)); pstmt.setString(2, "private.network.device"); pstmt.executeUpdate(); - } catch (SQLException e) { } catch (Throwable e) { + throw new CloudRuntimeException("Failed to update private.network.device in host_details due to exception ", e); } - } else if (Config.XenPublicNetwork.key().equals(name)) { + } else if (Config.XenPublicNetwork.key().equalsIgnoreCase(name)) { String sql = "update host_details set value=? where name=?"; - Transaction txn = Transaction.currentTxn(); - PreparedStatement pstmt = null; try { pstmt = txn.prepareAutoCloseStatement(sql); pstmt.setString(1, DBEncryptionUtil.encrypt(value)); pstmt.setString(2, "public.network.device"); pstmt.executeUpdate(); - } catch (SQLException e) { } catch (Throwable e) { + throw new CloudRuntimeException("Failed to update public.network.device in host_details due to exception ", e); } - } else if (Config.XenStorageNetwork1.key().equals(name)) { + } else if (Config.XenStorageNetwork1.key().equalsIgnoreCase(name)) { String sql = "update host_details set value=? where name=?"; - Transaction txn = Transaction.currentTxn(); - PreparedStatement pstmt = null; try { pstmt = txn.prepareAutoCloseStatement(sql); pstmt.setString(1, DBEncryptionUtil.encrypt(value)); pstmt.setString(2, "storage.network.device1"); pstmt.executeUpdate(); - } catch (SQLException e) { } catch (Throwable e) { + throw new CloudRuntimeException("Failed to update storage.network.device1 in host_details due to exception ", e); } } else if (Config.XenStorageNetwork2.key().equals(name)) { String sql = "update host_details set value=? where name=?"; - Transaction txn = Transaction.currentTxn(); - PreparedStatement pstmt = null; try { pstmt = txn.prepareAutoCloseStatement(sql); pstmt.setString(1, DBEncryptionUtil.encrypt(value)); pstmt.setString(2, "storage.network.device2"); pstmt.executeUpdate(); - } catch (SQLException e) { } catch (Throwable e) { + throw new CloudRuntimeException("Failed to update storage.network.device2 in host_details due to exception ", e); } } else if (Config.SystemVMUseLocalStorage.key().equalsIgnoreCase(name)) { if (s_logger.isDebugEnabled()) { @@ -400,7 +397,7 @@ public class ConfigurationManagerImpl implements ConfigurationManager, Configura if (serviceOffering != null) { serviceOffering.setUseLocalStorage(useLocalStorage); if (!_serviceOfferingDao.update(serviceOffering.getId(), serviceOffering)) { - s_logger.error("Failed to update ConsoleProxy offering's use_local_storage option to value:" + useLocalStorage); + throw new CloudRuntimeException("Failed to update ConsoleProxy offering's use_local_storage option to value:" + useLocalStorage); } } @@ -408,7 +405,7 @@ public class ConfigurationManagerImpl implements ConfigurationManager, Configura if (serviceOffering != null) { serviceOffering.setUseLocalStorage(useLocalStorage); if (!_serviceOfferingDao.update(serviceOffering.getId(), serviceOffering)) { - s_logger.error("Failed to update SoftwareRouter offering's use_local_storage option to value:" + useLocalStorage); + throw new CloudRuntimeException("Failed to update SoftwareRouter offering's use_local_storage option to value:" + useLocalStorage); } } @@ -416,11 +413,12 @@ public class ConfigurationManagerImpl implements ConfigurationManager, Configura if (serviceOffering != null) { serviceOffering.setUseLocalStorage(useLocalStorage); if (!_serviceOfferingDao.update(serviceOffering.getId(), serviceOffering)) { - s_logger.error("Failed to update SecondaryStorage offering's use_local_storage option to value:" + useLocalStorage); + throw new CloudRuntimeException("Failed to update SecondaryStorage offering's use_local_storage option to value:" + useLocalStorage); } } } - + + txn.commit(); } @Override diff --git a/server/src/com/cloud/user/AccountManagerImpl.java b/server/src/com/cloud/user/AccountManagerImpl.java index 54d3eb1a757..5bec7b7393d 100755 --- a/server/src/com/cloud/user/AccountManagerImpl.java +++ b/server/src/com/cloud/user/AccountManagerImpl.java @@ -605,7 +605,7 @@ public class AccountManagerImpl implements AccountManager, AccountService, Manag @Override @DB @ActionEvent(eventType = EventTypes.EVENT_ACCOUNT_CREATE, eventDescription = "creating Account") - public UserAccount createUserAccount(String userName, String password, String firstName, String lastName, String email, String timezone, String accountName, short accountType, Long domainId, String networkDomain, Map details) { + public UserAccount createUserAccount(String userName, String password, String firstName, String lastName, String email, String timezone, String accountName, short accountType, Long domainId, String networkDomain, Map details) { if (accountName == null) { accountName = userName; diff --git a/server/test/com/cloud/user/MockAccountManagerImpl.java b/server/test/com/cloud/user/MockAccountManagerImpl.java index 9d64c46f5e9..19bf997eb56 100644 --- a/server/test/com/cloud/user/MockAccountManagerImpl.java +++ b/server/test/com/cloud/user/MockAccountManagerImpl.java @@ -265,7 +265,7 @@ public class MockAccountManagerImpl implements Manager, AccountManager { public UserAccount createUserAccount(String userName, String password, String firstName, String lastName, String email, String timezone, String accountName, short accountType, Long domainId, - String networkDomain, Map details) { + String networkDomain, Map details) { // TODO Auto-generated method stub return null; }