Roy Golan has uploaded a new change for review. Change subject: core: log exception when OVF is corrupted ......................................................................
core: log exception when OVF is corrupted log to error the exception when failing to read the OVF * move the responsibility for logging error deeper * throw only 1 checked exception and with the obvious type * simplify exception stucture * add debug in case one wants to peek to OVF Change-Id: Ibc25b6544a698df013fc6d6b2677d798408289a6 Bug-Url: https://bugzilla.redhat.com/1039468 Signed-off-by: Roy Golan <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetVmFromConfigurationQuery.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InstallerMessages.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/OvfHelper.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/GetTemplatesFromExportDomainQuery.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/GetVmsFromExportDomainQuery.java M backend/manager/modules/compat/src/main/java/org/ovirt/engine/core/compat/backendcompat/XmlDocument.java M backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfManager.java M backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfParser.java M backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfReaderException.java 9 files changed, 78 insertions(+), 57 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/62/26562/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetVmFromConfigurationQuery.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetVmFromConfigurationQuery.java index ebb9a5b..4715054 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetVmFromConfigurationQuery.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetVmFromConfigurationQuery.java @@ -2,6 +2,7 @@ import org.ovirt.engine.core.common.businessentities.ConfigurationType; import org.ovirt.engine.core.common.queries.GetVmFromConfigurationQueryParameters; +import org.ovirt.engine.core.utils.ovf.OvfReaderException; public class GetVmFromConfigurationQuery<P extends GetVmFromConfigurationQueryParameters> extends QueriesCommandBase<P> { @@ -15,7 +16,7 @@ try { getQueryReturnValue().setReturnValue(ovfHelper.readVmFromOvf(getParameters().getVmConfiguration())); getQueryReturnValue().setSucceeded(true); - } catch (Exception e) { + } catch (OvfReaderException e) { log.debug("failed to parse a given ovf configuration: \n" + getParameters().getVmConfiguration(), e); getQueryReturnValue().setExceptionString("failed to parse a given ovf configuration " + e.getMessage()); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InstallerMessages.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InstallerMessages.java index dacdcb1..b3daf8f 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InstallerMessages.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InstallerMessages.java @@ -85,8 +85,12 @@ private boolean _internalPostOldXmlFormat(String message) { boolean error = false; - XmlDocument doc = new XmlDocument(); - doc.LoadXml(message); + XmlDocument doc = null; + try { + doc = new XmlDocument(message); + } catch (Exception e) { + throw new RuntimeException(e); + } XmlNode node = doc.childNodes[0]; if (node != null) { StringBuilder sb = new StringBuilder(); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/OvfHelper.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/OvfHelper.java index 0e96c62..b08088e 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/OvfHelper.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/OvfHelper.java @@ -107,7 +107,7 @@ return DbFacade.getInstance(); } - public boolean isOvfTemplate(String ovfstring) { + public boolean isOvfTemplate(String ovfstring) throws OvfReaderException { return ovfManager.IsOvfTemplate(ovfstring); } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/GetTemplatesFromExportDomainQuery.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/GetTemplatesFromExportDomainQuery.java index 65c66dc..02b16ff 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/GetTemplatesFromExportDomainQuery.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/GetTemplatesFromExportDomainQuery.java @@ -42,17 +42,18 @@ templates.put(template, diskImages); } } catch (OvfReaderException ex) { - AuditLogableBase logable = new AuditLogableBase(); - logable.addCustomValue("Template", ex.getName()); - AuditLogDirector.log(logable, AuditLogType.IMPORTEXPORT_FAILED_TO_IMPORT_TEMPLATE); - } catch (RuntimeException ex) { - AuditLogableBase logable = new AuditLogableBase(); - logable.addCustomValue("Template", "[Unknown name]"); - AuditLogDirector.log(logable, AuditLogType.IMPORTEXPORT_FAILED_TO_IMPORT_TEMPLATE); + auditLogOvfLoadError(ex.getName()); } - } return templates; } + + private void auditLogOvfLoadError(String machineName) { + AuditLogableBase logable = new AuditLogableBase(); + logable.addCustomValue("Template", machineName); + AuditLogDirector.log(logable, AuditLogType.IMPORTEXPORT_FAILED_TO_IMPORT_TEMPLATE); + + } + } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/GetVmsFromExportDomainQuery.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/GetVmsFromExportDomainQuery.java index 8ccdfc9..313c1d5 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/GetVmsFromExportDomainQuery.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/GetVmsFromExportDomainQuery.java @@ -1,5 +1,8 @@ package org.ovirt.engine.core.bll.storage; +import java.util.ArrayList; +import java.util.List; + import org.ovirt.engine.core.bll.OvfHelper; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.businessentities.VM; @@ -7,9 +10,6 @@ import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogDirector; import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogableBase; import org.ovirt.engine.core.utils.ovf.OvfReaderException; - -import java.util.ArrayList; -import java.util.List; public class GetVmsFromExportDomainQuery<P extends GetAllFromExportDomainQueryParameters> extends GetAllFromExportDomainQuery<List<VM>, P> { @@ -29,16 +29,16 @@ vms.add(ovfHelper.readVmFromOvf(ovf)); } } catch (OvfReaderException ex) { - AuditLogableBase logable = new AuditLogableBase(); - logable.addCustomValue("ImportedVmName", ex.getName()); - AuditLogDirector.log(logable, AuditLogType.IMPORTEXPORT_FAILED_TO_IMPORT_VM); - } catch (RuntimeException ex) { - AuditLogableBase logable = new AuditLogableBase(); - logable.addCustomValue("ImportedVmName", "[Unknown name]"); - AuditLogDirector.log(logable, AuditLogType.IMPORTEXPORT_FAILED_TO_IMPORT_VM); + auditLogOvfLoadError(ex.getName()); } } return vms; } + + private void auditLogOvfLoadError(String machineName) { + AuditLogableBase logable = new AuditLogableBase(); + logable.addCustomValue("ImportedVmName", machineName); + AuditLogDirector.log(logable, AuditLogType.IMPORTEXPORT_FAILED_TO_IMPORT_VM); + } } diff --git a/backend/manager/modules/compat/src/main/java/org/ovirt/engine/core/compat/backendcompat/XmlDocument.java b/backend/manager/modules/compat/src/main/java/org/ovirt/engine/core/compat/backendcompat/XmlDocument.java index 71e1371..461e5dc 100644 --- a/backend/manager/modules/compat/src/main/java/org/ovirt/engine/core/compat/backendcompat/XmlDocument.java +++ b/backend/manager/modules/compat/src/main/java/org/ovirt/engine/core/compat/backendcompat/XmlDocument.java @@ -21,25 +21,28 @@ private Document doc; - public void LoadXml(String ovfstring) { - try { - // load doc - DocumentBuilderFactory fact = DocumentBuilderFactory.newInstance(); - fact.setNamespaceAware(true); - DocumentBuilder builder = fact.newDocumentBuilder(); - doc = builder.parse(new InputSource(new StringReader(ovfstring))); + public XmlDocument() { + } - // initialize all the child nodes - NodeList list = doc.getElementsByTagName("*"); - childNodes = new XmlNode[list.getLength()]; - for (int i = 0; i < list.getLength(); i++) { - childNodes[i] = new XmlNode(list.item(i)); - } + public XmlDocument(String xml) throws Exception { + LoadXml(xml); + } - outerXml = ovfstring; - } catch (Exception e) { - throw new RuntimeException(e.getMessage(), e.getCause()); + private void LoadXml(String ovfstring) throws Exception { + // load doc + DocumentBuilderFactory fact = DocumentBuilderFactory.newInstance(); + fact.setNamespaceAware(true); + DocumentBuilder builder = fact.newDocumentBuilder(); + doc = builder.parse(new InputSource(new StringReader(ovfstring))); + + // initialize all the child nodes + NodeList list = doc.getElementsByTagName("*"); + childNodes = new XmlNode[list.getLength()]; + for (int i = 0; i < list.getLength(); i++) { + childNodes[i] = new XmlNode(list.item(i)); } + + outerXml = ovfstring; } public XmlNode SelectSingleNode(String string) { diff --git a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfManager.java b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfManager.java index d0a4e4a..1cb9325 100644 --- a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfManager.java +++ b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfManager.java @@ -10,8 +10,12 @@ import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.compat.Version; import org.ovirt.engine.core.compat.backendcompat.XmlDocument; +import org.ovirt.engine.core.utils.log.Log; +import org.ovirt.engine.core.utils.log.LogFactory; public class OvfManager { + + private Log log = LogFactory.getLog(OvfManager.class); public String ExportVm(VM vm, ArrayList<DiskImage> images, Version version) { OvfWriter ovf = new OvfVmWriter(vm, images, version); @@ -32,17 +36,14 @@ ArrayList<DiskImage> images, ArrayList<VmNetworkInterface> interfaces) throws OvfReaderException { - XmlDocument document = new XmlDocument(); - document.LoadXml(ovfstring); - OvfReader ovf = null; try { - ovf = new OvfVmReader(document, vm, images, interfaces); + ovf = new OvfVmReader(new XmlDocument(ovfstring), vm, images, interfaces); BuildOvf(ovf); } catch (Exception ex) { - String name = (ovf == null) ? OvfVmReader.EmptyName : ovf.getName(); - throw new OvfReaderException("Error parsing OVF:\r\n\r\n" + ovfstring, ex, name); + logOvfLoadError(ex.getMessage(), ovfstring); + throw new OvfReaderException( ex, ovf != null ? ovf.getName() : null); } Guid id = vm.getStaticData().getId(); for (VmNetworkInterface iface : interfaces) { @@ -53,20 +54,23 @@ public void ImportTemplate(String ovfstring, VmTemplate vmTemplate, ArrayList<DiskImage> images, ArrayList<VmNetworkInterface> interfaces) throws OvfReaderException { - XmlDocument document = new XmlDocument(); - document.LoadXml(ovfstring); OvfReader ovf = null; try { - ovf = new OvfTemplateReader(document, vmTemplate, images, interfaces); + ovf = new OvfTemplateReader(new XmlDocument(ovfstring), vmTemplate, images, interfaces); BuildOvf(ovf); } catch (Exception ex) { - String name = (ovf == null) ? OvfVmReader.EmptyName : ovf.getName(); - throw new OvfReaderException("Error parsing OVF:\r\n\r\n" + ovfstring, ex, name); + logOvfLoadError(ex.getMessage(), ovfstring); + throw new OvfReaderException(ex, ovf != null ? ovf.getName() : null); } } - public boolean IsOvfTemplate(String ovfstring) { + private void logOvfLoadError(String message, String ovfstring) { + log.errorFormat("Error parsing OVF due to {0}", message); + log.debugFormat("Error parsing OVF {0}\n", ovfstring); + } + + public boolean IsOvfTemplate(String ovfstring) throws OvfReaderException { return new OvfParser(ovfstring).IsTemplate(); } diff --git a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfParser.java b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfParser.java index aeb24de..e9f5f87 100644 --- a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfParser.java +++ b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfParser.java @@ -23,10 +23,13 @@ protected XmlDocument _document; protected XmlNamespaceManager _xmlNS; - public OvfParser(String ovfstring) { - _document = new XmlDocument(); - _document.LoadXml(ovfstring); - + public OvfParser(String ovfstring) throws OvfReaderException { + try { + _document = new XmlDocument(ovfstring); + } catch (Exception e) { + log.errorFormat("Failed Parsing OVF due to {0} ", e.getMessage()); + throw new OvfReaderException(e); + } _xmlNS = new XmlNamespaceManager(); } diff --git a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfReaderException.java b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfReaderException.java index 448aa99..dbaae0e 100644 --- a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfReaderException.java +++ b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfReaderException.java @@ -4,9 +4,14 @@ public class OvfReaderException extends Exception { private String name; - public OvfReaderException(String message, Exception ex, String name) { - super(message, ex); - this.name = name; + public OvfReaderException(Exception ex, String name) { + super(ex.getMessage(), ex); + this.name = name == null ? OvfReader.EmptyName : name; + } + + public OvfReaderException(Exception e) { + super(e.getMessage(), e); + this.name = OvfReader.EmptyName; } public String getName() { -- To view, visit http://gerrit.ovirt.org/26562 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ibc25b6544a698df013fc6d6b2677d798408289a6 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.4 Gerrit-Owner: Roy Golan <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
