On 03/21/2016 02:11 AM, Chunyan Liu wrote:
When AttachNetDevice failed, should call networkReleaseActualDevice to release actual device, and if actual device is hostdev, should remove the hostdev from vm->def->hostdevs.
Signed-off-by: Chunyan Liu <cyliu@suse.com> --- src/libxl/libxl_driver.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 87ec5a5..05ebe29 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -3122,16 +3122,18 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver, int ret = -1; char mac[VIR_MAC_STRING_BUFLEN];
+ libxl_device_nic_init(&nic); + /* preallocate new slot for device */ if (VIR_REALLOC_N(vm->def->nets, vm->def->nnets + 1) < 0) - goto out; + goto cleanup;
/* If appropriate, grab a physical device from the configured * network's pool of devices, or resolve bridge device name * to the one defined in the network definition. */ if (networkAllocateActualDevice(vm->def, net) < 0) - goto out; + goto cleanup;
actualType = virDomainNetGetActualType(net);
@@ -3139,7 +3141,7 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver, virReportError(VIR_ERR_INVALID_ARG, _("network device with mac %s already exists"), virMacAddrFormat(&net->mac, mac)); - return -1; + goto cleanup; }
if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { @@ -3150,10 +3152,9 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver, */ ret = libxlDomainAttachHostDevice(driver, vm, virDomainNetGetActualHostdev(net)); - goto out; + goto cleanup; }
- libxl_device_nic_init(&nic); if (libxlMakeNic(vm->def, net, &nic) < 0) goto cleanup;
@@ -3167,9 +3168,12 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver,
cleanup: libxl_device_nic_dispose(&nic); - out: - if (!ret) + if (!ret) { vm->def->nets[vm->def->nnets++] = net; + } else { + virDomainNetRemoveHostdev(vm->def, net); + networkReleaseActualDevice(vm->def, net); + }
Nice cleanup. But I think it can go a bit further by assigning the new net to nets right after a successful libxl_device_nic_add(), and only handle failure cleanup here. I've squashed in the change below. Regards, Jim diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 05ebe29..f6417ea 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -3164,13 +3164,12 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver, goto cleanup; } + vm->def->nets[vm->def->nnets++] = net; ret = 0; cleanup: libxl_device_nic_dispose(&nic); - if (!ret) { - vm->def->nets[vm->def->nnets++] = net; - } else { + if (ret) { virDomainNetRemoveHostdev(vm->def, net); networkReleaseActualDevice(vm->def, net); }