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;
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list