On 6/6/25 11:16, Boris Fiuczynski wrote:
> Let's add the nodedev name to improve the error message for the user.
>
> Signed-off-by: Boris Fiuczynski <[email protected]>
> ---
> src/node_device/node_device_driver.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/src/node_device/node_device_driver.c
> b/src/node_device/node_device_driver.c
> index 2c9e749495..de103d1967 100644
> --- a/src/node_device/node_device_driver.c
> +++ b/src/node_device/node_device_driver.c
> @@ -927,6 +927,7 @@ nodeDeviceGetMdevctlModifySupportCheck(void)
>
> static int
> virMdevctlModify(virNodeDeviceDef *def,
> + const char *def_name,
> bool defined,
> bool live)
> {
> @@ -942,8 +943,9 @@ virMdevctlModify(virNodeDeviceDef *def,
>
> if (nodeDeviceGetMdevctlModifySupportCheck() < 0) {
> VIR_WARN("Installed mdevctl version does not support modify with
> options jsonfile, defined and live");
> - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> - _("Unable to modify mediated device: modify
> unsupported"));
> + virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
> + _("Unable to modify mediated device '%1$s': modify
> unsupported"),
> + def_name);
> return -1;
> }
>
> @@ -952,8 +954,8 @@ virMdevctlModify(virNodeDeviceDef *def,
>
> if (status != 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("Unable to modify mediated device: %1$s"),
> - MDEVCTL_ERROR(errmsg));
> + _("Unable to modify mediated device '%1$s': %2$s"),
> + def_name, MDEVCTL_ERROR(errmsg));
> return -1;
> }
>
> @@ -1608,8 +1610,9 @@ nodeDeviceDefineXML(virConnect *conn,
> * nodeDeviceDefValidateUpdate() is not required as uuid and
> * parent are matching if def was found and changing the type in
> * the persistent config is allowed. */
> - VIR_DEBUG("Update node device '%s' with mdevctl", def->name);
> - modify_failed = (virMdevctlModify(def, true, false) < 0);
Nit pick, I'm putting an empty line in between these two lines ^^ to
visually separate variable declaration and code blocks.
> + virNodeDeviceDef *olddef = virNodeDeviceObjGetDef(persistent_obj);
> + VIR_DEBUG("Update node device '%s' with mdevctl", olddef->name);
> + modify_failed = (virMdevctlModify(def, olddef->name, true, false) <
> 0);
> virNodeDeviceObjEndAPI(&persistent_obj);
> if (modify_failed)
> return NULL;
> @@ -2357,6 +2360,7 @@ nodeDeviceUpdate(virNodeDevice *device,
> /* Update now. */
> VIR_DEBUG("Update node device '%s' with mdevctl", def->name);
> if (virMdevctlModify(new_def,
> + def->name,
> (flags & VIR_NODE_DEVICE_UPDATE_AFFECT_CONFIG),
> (flags & VIR_NODE_DEVICE_UPDATE_AFFECT_LIVE)) <
> 0) {
> goto cleanup;
Reviewed-by: Michal Privoznik <[email protected]>
And merged.
Michal
