On Thu, Feb 19, 2015 at 06:13:01PM -0700, Jim Fehlig wrote:
Marek Marczykowski-Górecki wrote:
On Thu, Feb 19, 2015 at 03:58:30PM -0700, Jim Fehlig wrote:
Marek Marczykowski-Górecki wrote:
On Thu, Feb 19, 2015 at 03:10:13PM -0700, Jim Fehlig wrote:
Jim Fehlig wrote:
Marek Marczykowski-Górecki wrote:
> On Thu, Feb 19, 2015 at 01:58:02PM -0700, Jim Fehlig wrote: > >> Marek Marczykowski-Górecki wrote: >> >>> On Thu, Feb 19, 2015 at 11:43:15AM -0700, Jim Fehlig wrote: >>> >>>> Marek Marczykowski-Górecki wrote: >>>> >>>>> It will not be possible to detach such device later. Also improve >>>>> logging in such cases. >>>>> >>>>> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> >>>>> --- >>>>> src/libxl/libxl_driver.c | 41 +++++++++++++++++++++++++++++++++++++++-- >>>>> 1 file changed, 39 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c >>>>> index ce3a99b..005cc96 100644 >>>>> --- a/src/libxl/libxl_driver.c >>>>> +++ b/src/libxl/libxl_driver.c >>>>> @@ -2787,6 +2787,7 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver, >>>>> int actualType; >>>>> libxl_device_nic nic; >>>>> int ret = -1; >>>>> + char mac[VIR_MAC_STRING_BUFLEN]; >>>>> >>>>> /* preallocate new slot for device */ >>>>> if (VIR_REALLOC_N(vm->def->nets, vm->def->nnets + 1) < 0) >>>>> @@ -2801,6 +2802,14 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver, >>>>> >>>>> actualType = virDomainNetGetActualType(net); >>>>> >>>>> + /* -2 means "multiple matches" so then fail also */ >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> >>>> No longer true after commit 2fbae1b2. I think you just want to check if >>>> virDomainNetFindIdx() >= 0, meaning the def already contains a net >>>> device with the same mac address. >>>> >>>> >>>> >>>> >>>> >>> But here the error is when the device *is* found, so the opposite case >>> than already reported as an error by virDomainNetFindIdx. >>> >>> >>> >>> >> If you find an idx >= 0, then the domain def already contains a net >> device with the same mac address, right? In that case, you report an >> error and return failure from libxlDomainAttachNetDevice(). >> >> >> >> > Right, but if I do not find one (idx == -1), I will proceed with > (possibly successful) adding the device, while the error was already > reported by virDomainNetFindIdx. > > > > Ah, right :-/.
Another option: introduce virDomainHasNet() to detect if the domain def already contains the net device.
A better option would be to fix this in libxl, for the benefit of other libxl apps.
Actually libxl has no problem with duplicated mac addresses, its libvirt that makes problem.
Yeah, it appears duplicate mac addresses are only valid if on different PCI devices.
Is that true for libvirt generally (all drivers)?
Back to virDomainHasNet? :-) Or checking for the duplicate directly in libxlDomainAttachNetDevice()?
What do you mean by "directly"? This is exactly what my patch did (until virDomainNetFindIdx stopped reporting duplicates).
I mean coding up the search for an existing mac directly in libxlDomainAttachNetDevice(). E.g.
I see, IMO its better with (reusable...) virDomainHasNet.
static int libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver, libxlDomainObjPrivatePtr priv, virDomainObjPtr vm, virDomainNetDefPtr net) { int actualType; libxl_device_nic nic; int ret = -1; size_t i; virDomainDefPtr def = vm->def;
for (i = 0; i < def->nnets; i++) { if (virMacAddrCmp(&def->nets[i]->mac, &net->mac) == 0) error; } ... }
Regards, Jim
-- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing?