On 5/18/21 6:07 PM, Laine Stump wrote:
On 5/18/21 5:44 AM, Kristina Hanicova wrote:
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1942367
Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- src/conf/domain_conf.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7044701fac..e21b9fb946 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15781,38 +15781,45 @@ virDomainNetFindIdx(virDomainDef *def, virDomainNetDef *net) if (matchidx < 0) { if (MACAddrSpecified && PCIAddrSpecified) { virReportError(VIR_ERR_DEVICE_MISSING, - _("no device matching MAC address %s found on " + _("no device matching MAC address %s and alias %s found on " VIR_PCI_DEVICE_ADDRESS_FMT), virMacAddrFormat(&net->mac, mac), + NULLSTR(net->info.alias), net->info.addr.pci.domain, net->info.addr.pci.bus, net->info.addr.pci.slot, net->info.addr.pci.function); } else if (MACAddrSpecified && CCWAddrSpecified) { virReportError(VIR_ERR_DEVICE_MISSING, - _("no device matching MAC address %s found on " + _("no device matching MAC address %s and alias %s found on "
These messages will look strange in the (most common) case where alias isn't specified, e.g.:
no device matching MAC address DE:AD:BE:EF:01:10 and alias found on [some CCW address]
On the other hand, the idea of even further exploding this bunch of conditionals to include all combinations is just horrible to think about!
What about instead reworking this to use a single virReportError() that references a few pointers setup beforehand and then substituting (a properly i8n'ized!) "(unspecified)" for each item that hasn't been specified, e.g.:
g_autofree *addr = g_strdup(_("(unspecified)")); const char *mac = _("(unspecified)"); const char *alias = _("(unspecified)");
if (MACAddrSpecified) mac = virMacAddrFormat(&net->mac, mac); if (net->info.alias) alias = net->info.alias
if (CCWAddrSpecified) addr = virCCWAddressAsString(blah); else if (PCIAddrSpecified) addr = virPCIDeviceAddressAsString(blah);
virReportError(blah... _("no device found at address '%s' matching MAC address '%s' and alias '%s'"), addr, mac, alias);
or something like that. It's still not ideal, but avoids the conditional explosion and I think is less confusing than having "alias" followed by nothing.
IIUC, NULLSTR() will expand to "<null>" not an empty string. "unspecified" sounds better. What I worry about is translations: in my native language and it's not a problem to have the error message split as you suggest. But maybe there are some languages where it might be problem? On the other hand - we can go with your suggestion and change this later as soon as we learn it's problematic for translators. Kristina, what's your opinion? Michal