On Mon, 2009-07-27 at 11:04 +0200, Daniel Veillard wrote:
On Wed, Jul 22, 2009 at 10:57:36PM +0100, Mark McLoughlin wrote:
In order to hotplug a network/bridge backed NIC, we need to first create the tap file descriptor, add the tap interface to the bridge and then pass the file descriptor to the qemu process using the 'getfd' monitor command.
Once the tapfd has been accepted, we create the network backend using host_net_add, supplying the name assigned to the tapfd. If this fails, we need to close the tapfd in qemu using the 'closefd' monitor command.
* src/qemu_driver.c: add support for tapfd based hotplug in qemudDomainAttachNetDevice() [...] + if (tapfd != -1) { + if (virAsprintf(&tapfd_name, "fd-%s", net->hostnet_name) < 0) + goto no_memory; + + if (virAsprintf(&tapfd_close, "closefd %s", tapfd_name) < 0) + goto no_memory;
[...]
+try_tapfd_close: + VIR_FREE(reply); + + if (tapfd_close) { + if (qemudMonitorCommand(vm, tapfd_close, &reply) < 0) + VIR_WARN(_("Failed to close tapfd with '%s'\n"), tapfd_close); + else + VIR_DEBUG("%s: closefd: %s\n", vm->def->name, reply); + } + goto cleanup;
This makes sense except I didn't expect tapfd_close which seems used only in an error path to be allocated on the main branch (assuming a tapfd).
Right, it's the same idea as the way we pre-allocate the "host_net_remove" monitor command - by allocating upfront, you avoid the potential for OOM failure in the error path. e.g. if the pci_add monitor command fails because of OOM, we're unlikely to be then able to format the closefd monitor command to clean up after ourselves. cheers, Mark.