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

[Laine Stump]

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 at 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.


>                                VIR_CCW_DEVICE_ADDRESS_FMT),
>                              virMacAddrFormat(&net->mac, mac),
> +                           NULLSTR(net->info.alias),
>                              net->info.addr.ccw.cssid,
>                              net->info.addr.ccw.ssid,
>                              net->info.addr.ccw.devno);
>           } else if (PCIAddrSpecified) {
>               virReportError(VIR_ERR_DEVICE_MISSING,
> -                           _("no device found on " VIR_PCI_DEVICE_ADDRESS_FMT),
> +                           _("no device matching alias %s  found on "
> +                             VIR_PCI_DEVICE_ADDRESS_FMT),
> +                           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 (CCWAddrSpecified) {
>               virReportError(VIR_ERR_DEVICE_MISSING,
> -                           _("no device found on " VIR_CCW_DEVICE_ADDRESS_FMT),
> +                           _("no device matching alias %s found on "
> +                             VIR_CCW_DEVICE_ADDRESS_FMT),
> +                           NULLSTR(net->info.alias),
>                              net->info.addr.ccw.cssid,
>                              net->info.addr.ccw.ssid,
>                              net->info.addr.ccw.devno);
>           } else if (MACAddrSpecified) {
>               virReportError(VIR_ERR_DEVICE_MISSING,
> -                           _("no device matching MAC address %s found"),
> -                           virMacAddrFormat(&net->mac, mac));
> +                           _("no device matching MAC address %s and alias %s found"),
> +                           virMacAddrFormat(&net->mac, mac),
> +                           NULLSTR(net->info.alias));
>           } else {
>               virReportError(VIR_ERR_DEVICE_MISSING, "%s",
>                              _("no matching device found"));
>