[v2] Some additional cleanups to mdev support

[libvirt PATCH v2 02/12] nodedev: avoid use of VIR_ERR_NO_* errors internally

Posted by Jonathon Jongsma 4 years, 10 months ago

These errors are demoted to debug statements[1] since they're only
intended to be used as return values for public APIs.  This makes it
difficult to debug the problem when something goes wrong since no error
message is logged. Switch instead to VIR_ERR_INTERNAL_ERROR so that the
error is logged as expected.

[1] See the implementation of daemonErrorLogFilter() for details:
https://gitlab.com/libvirt/libvirt/-/blob/e2f82a3704f680fbb37a733476d870c19232c23e/src/remote/remote_daemon.c#L89

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
---
 src/node_device/node_device_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
index 4cf4e4214f..1d1eaa9561 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -709,7 +709,7 @@ nodeDeviceGetMdevctlDefineStartCommand(virNodeDeviceDef *def,
     g_autofree char *parent_addr = nodeDeviceFindAddressByName(def->parent);
 
     if (!parent_addr) {
-        virReportError(VIR_ERR_NO_NODE_DEVICE,
+        virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("unable to find parent device '%s'"), def->parent);
         return NULL;
     }
-- 
2.26.3

Re: [libvirt PATCH v2 02/12] nodedev: avoid use of VIR_ERR_NO_* errors internally

Posted by Laine Stump 4 years, 10 months ago

On 4/13/21 4:39 PM, Jonathon Jongsma wrote:
> These errors are demoted to debug statements[1] since they're only
> intended to be used as return values for public APIs.  This makes it
> difficult to debug the problem when something goes wrong since no error
> message is logged. Switch instead to VIR_ERR_INTERNAL_ERROR so that the
> error is logged as expected.
> 
> [1] See the implementation of daemonErrorLogFilter() for details:
> https://gitlab.com/libvirt/libvirt/-/blob/e2f82a3704f680fbb37a733476d870c19232c23e/src/remote/remote_daemon.c#L89

Huh. TIL.

> 
> Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>

Reviewed-by: Laine Stump <laine@redhat.com>

> ---
>   src/node_device/node_device_driver.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
> index 4cf4e4214f..1d1eaa9561 100644
> --- a/src/node_device/node_device_driver.c
> +++ b/src/node_device/node_device_driver.c
> @@ -709,7 +709,7 @@ nodeDeviceGetMdevctlDefineStartCommand(virNodeDeviceDef *def,
>       g_autofree char *parent_addr = nodeDeviceFindAddressByName(def->parent);
>   
>       if (!parent_addr) {
> -        virReportError(VIR_ERR_NO_NODE_DEVICE,
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
>                          _("unable to find parent device '%s'"), def->parent);
>           return NULL;
>       }
>

Re: [libvirt PATCH v2 02/12] nodedev: avoid use of VIR_ERR_NO_* errors internally

Posted by Erik Skultety 4 years, 10 months ago

On Tue, Apr 13, 2021 at 03:39:38PM -0500, Jonathon Jongsma wrote:
> These errors are demoted to debug statements[1] since they're only
> intended to be used as return values for public APIs.  This makes it
> difficult to debug the problem when something goes wrong since no error
> message is logged. Switch instead to VIR_ERR_INTERNAL_ERROR so that the
> error is logged as expected.
> 
> [1] See the implementation of daemonErrorLogFilter() for details:
> https://gitlab.com/libvirt/libvirt/-/blob/e2f82a3704f680fbb37a733476d870c19232c23e/src/remote/remote_daemon.c#L89
> 
> Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
> ---
Reviewed-by: Erik Skultety <eskultet@redhat.com>