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 <bw...@linux.ibm.com>
CC: Boris Fiuczynski <fiu...@linux.ibm.com>
CC: Shalini Chellathurai Saroja <shal...@linux.ibm.com>
CC: Andrea Bolognani <abolo...@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb...@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,


Reply via email to