On 05/14/2013 08:16 PM, Guannan Ren wrote:

"couldn't find interface named" "couldn't find interface with MAC address"

use generic message as follows "couldn't find interface with"

If you were going to do this, having "with" at the end sounds awkward; it would be better to just make it "couldn't find interface '%s'". However, it doesn't make sense to make the messages in the driver itself generic, because they are inside functions that are specific to mac address / name, so it's appropriate for the error message to be specific. I think I like Dan's suggestion (3) the best - in virsh, before doing any lookups just try parsing the string into a mac address (with virMacAddrParse()) - if virMacAddrParse() is successful, you then only need to try lookupbymac (and can report "couldn't find interface with MAC address '%s' if the lookup fails), and if virMacAddrParse fails, you then only need to try lookupbyname. Once you've done that, you shouldn't need any changes to the libvirtd log messages at all.

--- src/interface/interface_backend_netcf.c | 16 ++++++++-------- src/interface/interface_backend_udev.c | 8 ++++---- 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/src/interface/interface_backend_netcf.c b/src/interface/interface_backend_netcf.c index cbba4fd..c6f3f42 100644 --- a/src/interface/interface_backend_netcf.c +++ b/src/interface/interface_backend_netcf.c @@ -104,12 +104,12 @@ static struct netcf_if *interfaceDriverGetNetcfIF(struct netcf *ncf, virInterfac int errcode = ncf_error(ncf, &errmsg, &details); if (errcode != NETCF_NOERROR) { virReportError(netcf_to_vir_err(errcode), - _("couldn't find interface named '%s': %s%s%s"), + _("couldn't find interface with '%s': %s%s%s"), ifinfo->name, errmsg, details ? " - " : "", details ? details : ""); } else { virReportError(VIR_ERR_NO_INTERFACE, - _("couldn't find interface named '%s'"), + _("couldn't find interface with '%s'"), ifinfo->name); } } @@ -334,7 +334,7 @@ netcfConnectListAllInterfaces(virConnectPtr conn, int errcode = ncf_error(driver->netcf, &errmsg, &details); if (errcode != NETCF_NOERROR) { virReportError(netcf_to_vir_err(errcode), - _("couldn't find interface named '%s': %s%s%s"), + _("couldn't find interface with '%s': %s%s%s"), names[i], errmsg, details ? " - " : "", details ? details : ""); goto cleanup; @@ -342,7 +342,7 @@ netcfConnectListAllInterfaces(virConnectPtr conn, /* Ignore the NETCF_NOERROR, as the interface is very likely * deleted by other management apps (e.g. virt-manager). */ - VIR_WARN("couldn't find interface named '%s', might be " + VIR_WARN("couldn't find interface with '%s', might be " "deleted by other process", names[i]); continue; } @@ -421,12 +421,12 @@ static virInterfacePtr netcfInterfaceLookupByName(virConnectPtr conn, int errcode = ncf_error(driver->netcf, &errmsg, &details); if (errcode != NETCF_NOERROR) { virReportError(netcf_to_vir_err(errcode), - _("couldn't find interface named '%s': %s%s%s"), + _("couldn't find interface with '%s': %s%s%s"), name, errmsg, details ? " - " : "", details ? details : ""); } else { virReportError(VIR_ERR_NO_INTERFACE, - _("couldn't find interface named '%s'"), name); + _("couldn't find interface with '%s'"), name); } goto cleanup; } @@ -454,14 +454,14 @@ static virInterfacePtr netcfInterfaceLookupByMACString(virConnectPtr conn, const char *errmsg, *details; int errcode = ncf_error(driver->netcf, &errmsg, &details); virReportError(netcf_to_vir_err(errcode), - _("couldn't find interface with MAC address '%s': %s%s%s"), + _("couldn't find interface with '%s': %s%s%s"), macstr, errmsg, details ? " - " : "", details ? details : ""); goto cleanup; } if (niface == 0) { virReportError(VIR_ERR_NO_INTERFACE, - _("couldn't find interface with MAC address '%s'"), + _("couldn't find interface with '%s'"), macstr); goto cleanup; } diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c index 1fd7d46..e61be52 100644 --- a/src/interface/interface_backend_udev.c +++ b/src/interface/interface_backend_udev.c @@ -420,7 +420,7 @@ udevInterfaceLookupByName(virConnectPtr conn, const char *name) dev = udev_device_new_from_subsystem_sysname(udev, "net", name); if (!dev) { virReportError(VIR_ERR_NO_INTERFACE, - _("couldn't find interface named '%s'"), + _("couldn't find interface with '%s'"), name); goto cleanup; } @@ -467,7 +467,7 @@ udevInterfaceLookupByMACString(virConnectPtr conn, const char *macstr) /* Check that we got something back */ if (!dev_entry) { virReportError(VIR_ERR_NO_INTERFACE, - _("couldn't find interface with MAC address '%s'"), + _("couldn't find interface with '%s'"), macstr); goto cleanup; } @@ -940,7 +940,7 @@ udevGetIfaceDef(struct udev *udev, const char *name) dev = udev_device_new_from_subsystem_sysname(udev, "net", name); if (!dev) { virReportError(VIR_ERR_NO_INTERFACE, - _("couldn't find interface named '%s'"), name); + _("couldn't find interface with '%s'"), name); goto error; }

@@ -1068,7 +1068,7 @@ udevInterfaceIsActive(virInterfacePtr ifinfo) ifinfo->name); if (!dev) { virReportError(VIR_ERR_NO_INTERFACE, - _("couldn't find interface named '%s'"), + _("couldn't find interface with '%s'"), ifinfo->name); status = -1; goto cleanup;