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

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

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|