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

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 = ....))