src/conf/domain_conf.c | 67 ++++++++++++++++-------------------------- 1 file changed, 25 insertions(+), 42 deletions(-)
This is v2 from:
https://listman.redhat.com/archives/libvir-list/2021-May/msg00481.html
I have reworked the code a bit to have only one error report
instead of multiple ones with different combinations of possible
matching items. Suggested by Laine.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1942367
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
---
src/conf/domain_conf.c | 67 ++++++++++++++++--------------------------
1 file changed, 25 insertions(+), 42 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b3ed3a9c5a..9631e40815 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15645,6 +15645,12 @@ virDomainNetFindIdx(virDomainDef *def, virDomainNetDef *net)
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI);
bool CCWAddrSpecified = virDomainDeviceAddressIsValid(&net->info,
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW);
+ g_autofree char *addr = NULL;
+ const char *macAddr = _("(<null>)");
+ const char *alias = _("(<null>)");
+
+ if (MACAddrSpecified)
+ macAddr = virMacAddrFormat(&net->mac, mac);
for (i = 0; i < def->nnets; i++) {
if (MACAddrSpecified &&
@@ -15676,7 +15682,7 @@ virDomainNetFindIdx(virDomainDef *def, virDomainNetDef *net)
if (MACAddrSpecified) {
virReportError(VIR_ERR_OPERATION_FAILED,
_("multiple devices matching MAC address %s found"),
- virMacAddrFormat(&net->mac, mac));
+ macAddr);
} else {
virReportError(VIR_ERR_OPERATION_FAILED, "%s",
_("multiple matching devices found"));
@@ -15688,47 +15694,24 @@ virDomainNetFindIdx(virDomainDef *def, virDomainNetDef *net)
matchidx = i;
}
- if (matchidx < 0) {
- if (MACAddrSpecified && PCIAddrSpecified) {
- virReportError(VIR_ERR_DEVICE_MISSING,
- _("no device matching MAC address %s found on "
- VIR_PCI_DEVICE_ADDRESS_FMT),
- virMacAddrFormat(&net->mac, mac),
- 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 "
- VIR_CCW_DEVICE_ADDRESS_FMT),
- virMacAddrFormat(&net->mac, mac),
- 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),
- 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),
- 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));
- } else {
- virReportError(VIR_ERR_DEVICE_MISSING, "%s",
- _("no matching device found"));
- }
- }
- return matchidx;
+ if (matchidx >= 0)
+ return matchidx;
+
+ if (net->info.alias)
+ alias = net->info.alias;
+
+ if (CCWAddrSpecified)
+ addr = virDomainCCWAddressAsString(&net->info.addr.ccw);
+ else if (PCIAddrSpecified)
+ addr = virPCIDeviceAddressAsString(&net->info.addr.pci);
+ else
+ addr = g_strdup(_("(<null>)"));
+
+ virReportError(VIR_ERR_DEVICE_MISSING,
+ _("no device found at address '%s' matching MAC address"
+ " '%s' and alias '%s'"),
+ addr, macAddr, alias);
+ return -1;
}
bool
--
2.31.1
On 5/20/21 4:13 PM, Kristina Hanicova wrote: > This is v2 from: > https://listman.redhat.com/archives/libvir-list/2021-May/msg00481.html This is something we usually put [1] > > I have reworked the code a bit to have only one error report > instead of multiple ones with different combinations of possible > matching items. Suggested by Laine. > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1942367 > > Signed-off-by: Kristina Hanicova <khanicov@redhat.com> > --- 1: here. Right after these three dashes ^^^ and before diffstat. Everything that's above the dashes is a commit message, and everything that's after diff stat is patch, leaving this place for comments. BTW: this is also the place where git-notes would format notes (were there any). > src/conf/domain_conf.c | 67 ++++++++++++++++-------------------------- > 1 file changed, 25 insertions(+), 42 deletions(-) The patch looks good to me. I'll let Laine to express himself too since he was the one who suggested this approach. Michal
On 5/20/21 10:13 AM, Kristina Hanicova wrote:
> This is v2 from:
> https://listman.redhat.com/archives/libvir-list/2021-May/msg00481.html
>
> I have reworked the code a bit to have only one error report
> instead of multiple ones with different combinations of possible
> matching items. Suggested by Laine.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1942367
>
> Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Looks okay to me.
Reviewed-by: Laine Stump <laine@redhat.com>
I'll push it shortly.
> ---
> src/conf/domain_conf.c | 67 ++++++++++++++++--------------------------
> 1 file changed, 25 insertions(+), 42 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index b3ed3a9c5a..9631e40815 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -15645,6 +15645,12 @@ virDomainNetFindIdx(virDomainDef *def, virDomainNetDef *net)
> VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI);
> bool CCWAddrSpecified = virDomainDeviceAddressIsValid(&net->info,
> VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW);
> + g_autofree char *addr = NULL;
> + const char *macAddr = _("(<null>)");
> + const char *alias = _("(<null>)");
> +
> + if (MACAddrSpecified)
> + macAddr = virMacAddrFormat(&net->mac, mac);
>
> for (i = 0; i < def->nnets; i++) {
> if (MACAddrSpecified &&
> @@ -15676,7 +15682,7 @@ virDomainNetFindIdx(virDomainDef *def, virDomainNetDef *net)
> if (MACAddrSpecified) {
> virReportError(VIR_ERR_OPERATION_FAILED,
> _("multiple devices matching MAC address %s found"),
> - virMacAddrFormat(&net->mac, mac));
> + macAddr);
> } else {
> virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> _("multiple matching devices found"));
> @@ -15688,47 +15694,24 @@ virDomainNetFindIdx(virDomainDef *def, virDomainNetDef *net)
> matchidx = i;
> }
>
> - if (matchidx < 0) {
> - if (MACAddrSpecified && PCIAddrSpecified) {
> - virReportError(VIR_ERR_DEVICE_MISSING,
> - _("no device matching MAC address %s found on "
> - VIR_PCI_DEVICE_ADDRESS_FMT),
> - virMacAddrFormat(&net->mac, mac),
> - 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 "
> - VIR_CCW_DEVICE_ADDRESS_FMT),
> - virMacAddrFormat(&net->mac, mac),
> - 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),
> - 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),
> - 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));
> - } else {
> - virReportError(VIR_ERR_DEVICE_MISSING, "%s",
> - _("no matching device found"));
> - }
> - }
> - return matchidx;
> + if (matchidx >= 0)
> + return matchidx;
> +
> + if (net->info.alias)
> + alias = net->info.alias;
> +
> + if (CCWAddrSpecified)
> + addr = virDomainCCWAddressAsString(&net->info.addr.ccw);
> + else if (PCIAddrSpecified)
> + addr = virPCIDeviceAddressAsString(&net->info.addr.pci);
> + else
> + addr = g_strdup(_("(<null>)"));
> +
> + virReportError(VIR_ERR_DEVICE_MISSING,
> + _("no device found at address '%s' matching MAC address"
> + " '%s' and alias '%s'"),
> + addr, macAddr, alias);
> + return -1;
> }
>
> bool
>
© 2016 - 2024 Red Hat, Inc.