Re: [libvirt] [PATCH] qemu: Remove redundant error reporting codes

于 2011年02月17日 19:15, Daniel P. Berrange 写道: > On Thu, Feb 17, 2011 at 05:30:23PM +0800, Osier Yang wrote: >> As virDomainDefParseString already reported the error if it >> fails, and the redundant error reports codes will override >> error reported by virDomainDefParseString with some unclear >> messages, removed them. >> >> * src/qemu/qemu_driver.c >> --- >> src/qemu/qemu_driver.c | 33 +++++++++++++-------------------- >> 1 files changed, 13 insertions(+), 20 deletions(-) >> >> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c >> index ab664a0..fd8e401 100644 >> --- a/src/qemu/qemu_driver.c >> +++ b/src/qemu/qemu_driver.c >> @@ -3600,8 +3600,10 @@ static virDomainPtr >> qemudDomainCreate(virConnectPtr conn, const char *xml, >> virCheckFlags(VIR_DOMAIN_START_PAUSED, NULL); >> >> qemuDriverLock(driver); >> - if (!(def = virDomainDefParseString(driver->caps, xml, >> - VIR_DOMAIN_XML_INACTIVE))) >> + >> + def = virDomainDefParseString(driver->caps, xml, >> VIR_DOMAIN_XML_INACTIVE); >> + if (!def) >> + /* virDomainDefParseString reports the error. */ >> goto cleanup; >> > > This is a needless change that increases line length. > >> if (virSecurityManagerVerify(driver->securityManager, def)< 0) >> @@ -5746,12 +5748,9 @@ qemudDomainSaveImageOpen(struct qemud_driver >> *driver, >> } >> >> /* Create a domain from this XML */ >> - if (!(def = virDomainDefParseString(driver->caps, xml, >> - VIR_DOMAIN_XML_INACTIVE))) { >> - qemuReportError(VIR_ERR_OPERATION_FAILED, >> - "%s", _("failed to parse XML")); >> + def = virDomainDefParseString(driver->caps, xml, >> VIR_DOMAIN_XML_INACTIVE); >> + if (!def) >> goto error; >> - } >> >> VIR_FREE(xml); >> >> @@ -6412,8 +6411,9 @@ static virDomainPtr >> qemudDomainDefine(virConnectPtr conn, const char *xml) { >> int dupVM; >> >> qemuDriverLock(driver); >> - if (!(def = virDomainDefParseString(driver->caps, xml, >> - VIR_DOMAIN_XML_INACTIVE))) >> + >> + def = virDomainDefParseString(driver->caps, xml, >> VIR_DOMAIN_XML_INACTIVE); >> + if (!def) >> goto cleanup; >> > > So is this chunk. > >> if (virSecurityManagerVerify(driver->securityManager, def)< 0) >> @@ -8046,13 +8046,9 @@ qemudDomainMigratePrepareTunnel(virConnectPtr >> dconn, >> } >> >> /* Parse the domain XML. */ >> - if (!(def = virDomainDefParseString(driver->caps, dom_xml, >> - VIR_DOMAIN_XML_INACTIVE))) { >> - qemuReportError(VIR_ERR_OPERATION_FAILED, >> - "%s", _("failed to parse XML, libvirt version may be " >> - "different between source and destination host")); >> + def = virDomainDefParseString(driver->caps, dom_xml, >> VIR_DOMAIN_XML_INACTIVE); >> + if (!def) >> goto cleanup; >> - } >> >> if (!qemuDomainIsMigratable(def)) >> goto cleanup; >> @@ -8320,12 +8316,9 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn, >> VIR_DEBUG("Generated uri_out=%s", *uri_out); >> >> /* Parse the domain XML. */ >> - if (!(def = virDomainDefParseString(driver->caps, dom_xml, >> - VIR_DOMAIN_XML_INACTIVE))) { >> - qemuReportError(VIR_ERR_OPERATION_FAILED, >> - "%s", _("failed to parse XML")); >> + def = virDomainDefParseString(driver->caps, dom_xml, >> VIR_DOMAIN_XML_INACTIVE); >> + if (!def) >> goto cleanup; >> - } > > > These other chunks are fine, but it'd be preferrable to keep the existing > line formatting if (!(def = ....)) Okay, will push with these updates, thanks for the reviewing.