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

Osier Yang jyang at redhat.com
Fri Feb 18 06:30:01 UTC 2011


于 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




More information about the libvir-list mailing list