[PATCH 1/1] domain_conf.c: skip checking ZPCI address is incomplete if not present

[Shalini Chellathurai Saroja]

Hi Daniel,

Thank you for letting me know:-)

Warm Regards

Shalini C S

On 6/30/20 7:44 PM, Daniel Henrique Barboza wrote:
> Hi Shalini,
>
>
> On 6/30/20 12:52 PM, Shalini Chellathurai Saroja wrote:
>> Hi Daniel,
>>
>> Sorry for the incorrect code. Thank you for identifying and fixing it:-)
>
> Glad I could help :)
>
>>
>> I would like to know, how did you identify the error?,
>
> It was by chance. I rebased the code to master before posting a
> series and started libvirtd to do a final test.
>
> I noticed the errors messages right after starting the daemon. What
> I did then was verifying which commit added the message and moved it
> inside the 'if' block, avoiding checking for zPCI address coherence
> regardless of the device having a zPCI address or not.
>
>
>> Is it possible to check for internal errors with the help of unit tests?
>
> Yes, but this was a peculiar case because the condition in which the 
> error
> message was being thrown wasn't causing the function to error out 
> (i.e. returning
> -1):
>
>         if (virZPCIDeviceAddressIsIncomplete(&info->addr.pci.zpci)) {
>             virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                            _("Missing uid or fid attribute of zPCI 
> address"));
>
>         }
>
> All existing tests were passing, but the message was being displayed 
> in the log.
>
> This is the kind of situation that, unless you design a test 
> specifically to catch
> it, most likely will be noticed when someone runs libvirtd.
>
>
> Thanks,
>
>
> DHB
>
>
>> Your answer would help me able to identify any incorrect code before 
>> sending the patch.
>>
>> Warm Regards
>> Shalini C S
>>
>>
>> On 6/26/20 11:49 PM, Daniel Henrique Barboza wrote:
>>> Commit 076591009ad1 ("conf: fix zPCI address auto-generation on
>>> s390") is doing a check for virZPCIDeviceAddressIsIncomplete()
>>> prior to checking if the device has a ZPCI address at all. This
>>> results in errors like these when starting Libvirt:
>>>
>>> error : virDomainDeviceInfoFormat:7527 : internal error:
>>> Missing uid or fid attribute of zPCI address
>>>
>>> Fix it by moving virZPCIDeviceAddressIsIncomplete() after the
>>> check done by virZPCIDeviceAddressIsPresent().
>>>
>>> Fixes: 076591009ad11ec108521b52a4945d0f895fa160
>>> CC: Bjoern Walk <bwalk at linux.ibm.com>
>>> CC: Boris Fiuczynski <fiuczy at linux.ibm.com>
>>> CC: Shalini Chellathurai Saroja <shalini at linux.ibm.com>
>>> CC: Andrea Bolognani <abologna at redhat.com>
>>> Signed-off-by: Daniel Henrique Barboza <danielhb413 at gmail.com>
>>> ---
>>>   src/conf/domain_conf.c | 8 ++++----
>>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>>> index 31ba78b950..33f177b16f 100644
>>> --- a/src/conf/domain_conf.c
>>> +++ b/src/conf/domain_conf.c
>>> @@ -7522,11 +7522,11 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
>>> virTristateSwitchTypeToString(info->addr.pci.multi));
>>>           }
>>> -        if (virZPCIDeviceAddressIsIncomplete(&info->addr.pci.zpci)) {
>>> -            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>>> -                           _("Missing uid or fid attribute of zPCI 
>>> address"));
>>> -        }
>>>           if (virZPCIDeviceAddressIsPresent(&info->addr.pci.zpci)) {
>>> +            if 
>>> (virZPCIDeviceAddressIsIncomplete(&info->addr.pci.zpci))
>>> +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>>> +                               _("Missing uid or fid attribute of 
>>> zPCI address"));
>>> +
>>>               virBufferAsprintf(&childBuf,
>>>                                 "<zpci uid='0x%.4x' fid='0x%.8x'/>\n",
>>> info->addr.pci.zpci.uid.value,