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

Daniel Henrique Barboza danielhb413 at gmail.com
Tue Jun 30 17:44:50 UTC 2020


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,




More information about the libvir-list mailing list