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@xxxxxxxxxxxxxxxxxxxxxx>
>>> ---
>>> 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().
Regards,
Jim
> Is it correct
> (harmless) to call virReportError when no error occurred (function
> returns >=0)? Maybe libvirt should check for duplicated devices at
> higher level (-> for all drivers in the same place)?
>
>
>>> + if (virDomainNetFindIdx(vm->def, net) != -1) {
>>> + virReportError(VIR_ERR_OPERATION_FAILED,
>>> + _("device matching mac address %s already exists"),
>>> + virMacAddrFormat(&net->mac, mac));
>>> + return -1;
>>> + }
>>> +
>>> if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
>>> /* This is really a "smart hostdev", so it should be attached
>>> * as a hostdev (the hostdev code will reach over into the
>>> @@ -2879,6 +2888,7 @@ libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev)
>>> virDomainHostdevDefPtr hostdev;
>>> virDomainHostdevDefPtr found;
>>> virDomainHostdevSubsysPCIPtr pcisrc;
>>> + char mac[VIR_MAC_STRING_BUFLEN];
>>>
>>> switch (dev->type) {
>>> case VIR_DOMAIN_DEVICE_DISK:
>>> @@ -2896,6 +2906,12 @@ libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev)
>>>
>>> case VIR_DOMAIN_DEVICE_NET:
>>> net = dev->data.net;
>>> + if (virDomainNetFindIdx(vmdef, net) >= 0) {
>>> + virReportError(VIR_ERR_INVALID_ARG,
>>> + _("network device with mac %s already exists."),
>>> + virMacAddrFormat(&net->mac, mac));
>>> + return -1;
>>> + }
>>>
>>>
>> Ah, like you've done here :-).
>>
>>
>>> if (virDomainNetInsert(vmdef, net))
>>> return -1;
>>> dev->data.net = NULL;
>>> @@ -3060,8 +3076,18 @@ libxlDomainDetachNetDevice(libxlDriverPrivatePtr driver,
>>> char mac[VIR_MAC_STRING_BUFLEN];
>>> int ret = -1;
>>>
>>> - if ((detachidx = virDomainNetFindIdx(vm->def, net)) < 0)
>>> + if ((detachidx = virDomainNetFindIdx(vm->def, net)) < 0) {
>>> + if (detachidx == -2) {
>>> + virReportError(VIR_ERR_OPERATION_FAILED,
>>> + _("multiple devices matching mac address %s found"),
>>> + virMacAddrFormat(&net->mac, mac));
>>> + } else {
>>> + virReportError(VIR_ERR_OPERATION_FAILED,
>>> + _("network device %s not found"),
>>> + virMacAddrFormat(&net->mac, mac));
>>> + }
>>>
>>>
>> virDomainNetFindIdx() handles the error reporting now.
>>
>>
>>> return -1;
>>> + }
>>>
>>> detach = vm->def->nets[detachidx];
>>>
>>> @@ -3136,6 +3162,7 @@ libxlDomainDetachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev)
>>> virDomainHostdevDefPtr hostdev, det_hostdev;
>>> virDomainNetDefPtr net;
>>> int idx;
>>> + char mac[VIR_MAC_STRING_BUFLEN];
>>>
>>> switch (dev->type) {
>>> case VIR_DOMAIN_DEVICE_DISK:
>>> @@ -3150,8 +3177,18 @@ libxlDomainDetachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev)
>>>
>>> case VIR_DOMAIN_DEVICE_NET:
>>> net = dev->data.net;
>>> - if ((idx = virDomainNetFindIdx(vmdef, net)) < 0)
>>> + if ((idx = virDomainNetFindIdx(vmdef, net)) < 0) {
>>> + if (idx == -2) {
>>> + virReportError(VIR_ERR_OPERATION_FAILED,
>>> + _("multiple devices matching mac address %s found"),
>>> + virMacAddrFormat(&dev->data.net->mac, mac));
>>> + } else {
>>> + virReportError(VIR_ERR_OPERATION_FAILED,
>>> + _("network device %s not found"),
>>> + virMacAddrFormat(&dev->data.net->mac, mac));
>>> + }
>>>
>>>
>> Same here.
>>
>> Regards,
>> Jim
>>
>>
>
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list