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

[Osier Yang <jyang@xxxxxxxxxx>]

ä 2011å02æ17æ 23:36, Osier Yang åé:
> ä 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.

It's already fixed as part of 766de43, so no pushing.

Regards
Osier

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list