Re: [PATCH] conf: Report alias name of the detached device in error

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(a)redhat.com&gt; > --- >   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.