Don't use the LOG*D macros.  They expect a domid, but "domain" here is
the PCI domain.  Hence it is inappropriate for this use.

Make the write error messages uniform with LOGE.  errno has the
interesting information while rc is just -1.  Drop printing rc and use
LOGE to print errno as text.

The interesting part of a failed write to do_flr is that PCI BDF, so
print that.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
 tools/libs/light/libxl_pci.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tools/libs/light/libxl_pci.c b/tools/libs/light/libxl_pci.c
index f4c4f17545..27d9dbff25 100644
--- a/tools/libs/light/libxl_pci.c
+++ b/tools/libs/light/libxl_pci.c
@@ -1554,27 +1554,27 @@ static int libxl__device_pci_reset(libxl__gc *gc, unsigned int domain, unsigned
         char *buf = GCSPRINTF(PCI_BDF, domain, bus, dev, func);
         rc = write(fd, buf, strlen(buf));
         if (rc < 0)
-            LOGD(ERROR, domain, "write to %s returned %d", reset, rc);
+            LOGE(ERROR, "write '%s' to %s failed", buf, reset);
         close(fd);
         return rc < 0 ? rc : 0;
     }
     if (errno != ENOENT)
-        LOGED(ERROR, domain, "Failed to access pciback path %s", reset);
+        LOGE(ERROR, "Failed to access pciback path %s", reset);
     reset = GCSPRINTF("%s/"PCI_BDF"/reset", SYSFS_PCI_DEV, domain, bus, dev, func);
     fd = open(reset, O_WRONLY);
     if (fd >= 0) {
         rc = write(fd, "1", 1);
         if (rc < 0)
-            LOGED(ERROR, domain, "write to %s returned %d", reset, rc);
+            LOGE(ERROR, "write to %s failed", reset);
         close(fd);
         return rc < 0 ? rc : 0;
     }
     if (errno == ENOENT) {
-        LOGD(ERROR, domain,
-             "The kernel doesn't support reset from sysfs for PCI device "PCI_BDF,
-             domain, bus, dev, func);
+        LOG(ERROR,
+            "The kernel doesn't support reset from sysfs for PCI device "PCI_BDF,
+            domain, bus, dev, func);
     } else {
-        LOGED(ERROR, domain, "Failed to access reset path %s", reset);
+        LOGE(ERROR, "Failed to access reset path %s", reset);
     }
     return -1;
 }
-- 
2.39.2

On 13.03.23 20:57, Jason Andryuk wrote:
> Don't use the LOG*D macros.  They expect a domid, but "domain" here is
> the PCI domain.  Hence it is inappropriate for this use.
> 
> Make the write error messages uniform with LOGE.  errno has the
> interesting information while rc is just -1.  Drop printing rc and use
> LOGE to print errno as text.
> 
> The interesting part of a failed write to do_flr is that PCI BDF, so
> print that.
> 
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

On Tue, Mar 14, 2023 at 04:12:03PM +0100, Juergen Gross wrote:
> On 13.03.23 20:57, Jason Andryuk wrote:
> > Don't use the LOG*D macros.  They expect a domid, but "domain" here is
> > the PCI domain.  Hence it is inappropriate for this use.
> > 
> > Make the write error messages uniform with LOGE.  errno has the
> > interesting information while rc is just -1.  Drop printing rc and use
> > LOGE to print errno as text.
> > 
> > The interesting part of a failed write to do_flr is that PCI BDF, so
> > print that.
> > 
> > Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
> 
> Reviewed-by: Juergen Gross <jgross@suse.com>

Acked-by: Anthony PERARD <anthony.perard@citrix.com>

Thanks,

-- 
Anthony PERARD