On 10/26/2012 04:54 PM, Eric Blake wrote:
This resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=862515
which describes inconsistencies in dealing with duplicate mac addresses on network devices in a domain.
(at any rate, it resolves *almost* everything, and prints out an informative error message for the one problem that isn't solved, but has a workaround.)
@@ -6158,14 +6158,6 @@ qemuDomainAttachDeviceConfig(qemuCapsPtr caps,
case VIR_DOMAIN_DEVICE_NET: net = dev->data.net; - if (virDomainNetIndexByMac(vmdef, &net->mac) >= 0) { - char macbuf[VIR_MAC_STRING_BUFLEN]; - - virMacAddrFormat(&net->mac, macbuf); - virReportError(VIR_ERR_INVALID_ARG, - _("mac %s already exists"), macbuf); - return -1; - } if (virDomainNetInsert(vmdef, net)) { Are you dropping functionality here? Shouldn't you still be checking
On 10/25/2012 02:31 PM, Laine Stump wrote: that the new NetFindIdx function returns -1?
No, that was intentional, and noted in the log message. It is incorrect to prohibit duplicate mac addresses for persistent (and live) attach. We've never checked for it in the live attach code, and we should check for it here.
- vshError(ctl, _("No found interface whose MAC address is %s"), mac); - goto cleanup; + if (!matchNode) { + vshError(ctl, _("No found interface whose MAC address is %s"), mac); This would read better as 'No interface found', since you are already touching this line.
I changed it to "No interface with MAC address %s was found"
ACK with those nits addressed; I like the overall improvement in error messages.
Pushed. Thanks!