Re: [libvirt] [PATCHv2 3/3] qemu: simplify interface fd handling in monitor

With only a single caller to these two monitor commands, I didn't need to wrap a new WithFds version, but just change the command itself. * src/qemu/qemu_monitor.h (qemuMonitorAddNetdev) (qemuMonitorAddHostNetwork): Add parameters. * src/qemu/qemu_monitor.c (qemuMonitorAddNetdev) (qemuMonitorAddHostNetwork): Add support for fd passing. * src/qemu/qemu_hotplug.c (qemuDomainAttachNetDevice): Use it to simplify code. --- v2: new patch src/qemu/qemu_hotplug.c | 57 ++++++--------------------------------------- src/qemu/qemu_monitor.c | 59 +++++++++++++++++++++++++++++++++++++++------- src/qemu/qemu_monitor.h | 8 ++++- 3 files changed, 64 insertions(+), 60 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 87fbb9a..9d7435c 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -611,63 +611,39 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, if (tapfd != -1) { if (virAsprintf(&tapfd_name, "fd-%s", net->info.alias) < 0) goto no_memory; - - qemuDomainObjEnterMonitorWithDriver(driver, vm); - if (qemuMonitorSendFileHandle(priv->mon, tapfd_name, tapfd) < 0) { - qemuDomainObjExitMonitorWithDriver(driver, vm); - goto cleanup; - } - qemuDomainObjExitMonitorWithDriver(driver, vm); - - if (!virDomainObjIsActive(vm)) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("guest unexpectedly quit")); - goto cleanup; - } } if (vhostfd != -1) { if (virAsprintf(&vhostfd_name, "vhostfd-%s", net->info.alias) < 0) goto no_memory; - - qemuDomainObjEnterMonitorWithDriver(driver, vm); - if (qemuMonitorSendFileHandle(priv->mon, vhostfd_name, vhostfd) < 0) { - qemuDomainObjExitMonitorWithDriver(driver, vm); - goto try_tapfd_close; - } - qemuDomainObjExitMonitorWithDriver(driver, vm); - - if (!virDomainObjIsActive(vm)) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("guest unexpectedly quit")); - goto cleanup; - } } if (qemuCapsGet(qemuCaps, QEMU_CAPS_NETDEV) && qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { if (!(netstr = qemuBuildHostNetStr(net, ',', -1, tapfd_name, vhostfd_name))) - goto try_tapfd_close; + goto cleanup; } else { if (!(netstr = qemuBuildHostNetStr(net, ' ', vlan, tapfd_name, vhostfd_name))) - goto try_tapfd_close; + goto cleanup; } qemuDomainObjEnterMonitorWithDriver(driver, vm); if (qemuCapsGet(qemuCaps, QEMU_CAPS_NETDEV) && qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - if (qemuMonitorAddNetdev(priv->mon, netstr) < 0) { + if (qemuMonitorAddNetdev(priv->mon, netstr, tapfd, tapfd_name, + vhostfd, vhostfd_name) < 0) { qemuDomainObjExitMonitorWithDriver(driver, vm); qemuAuditNet(vm, NULL, net, "attach", false); - goto try_tapfd_close; + goto cleanup; } } else { - if (qemuMonitorAddHostNetwork(priv->mon, netstr) < 0) { + if (qemuMonitorAddHostNetwork(priv->mon, netstr, tapfd, tapfd_name, + vhostfd, vhostfd_name) < 0) { qemuDomainObjExitMonitorWithDriver(driver, vm); qemuAuditNet(vm, NULL, net, "attach", false); - goto try_tapfd_close; + goto cleanup; } } qemuDomainObjExitMonitorWithDriver(driver, vm); @@ -765,23 +741,6 @@ try_remove: } goto cleanup; -try_tapfd_close: - if (!virDomainObjIsActive(vm)) - goto cleanup; - - if (tapfd_name || vhostfd_name) { - qemuDomainObjEnterMonitorWithDriver(driver, vm); - if (tapfd_name && - qemuMonitorCloseFileHandle(priv->mon, tapfd_name) < 0) - VIR_WARN("Failed to close tapfd with '%s'", tapfd_name); - if (vhostfd_name && - qemuMonitorCloseFileHandle(priv->mon, vhostfd_name) < 0) - VIR_WARN("Failed to close vhostfd with '%s'", vhostfd_name); - qemuDomainObjExitMonitorWithDriver(driver, vm); - } - - goto cleanup; - no_memory: virReportOOMError(); goto cleanup; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 074b0b2..65bebe3 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1819,11 +1819,15 @@ int qemuMonitorCloseFileHandle(qemuMonitorPtr mon, int qemuMonitorAddHostNetwork(qemuMonitorPtr mon, - const char *netstr) + const char *netstr, + int tapfd, const char *tapfd_name, + int vhostfd, const char *vhostfd_name) { - int ret; - VIR_DEBUG("mon=%p netstr=%s", - mon, netstr); + int ret = -1; + VIR_DEBUG("mon=%p netstr=%s tapfd=%d tapfd_name=%s " + "vhostfd=%d vhostfd_name=%s", + mon, netstr, tapfd, NULLSTR(tapfd_name), + vhostfd, NULLSTR(vhostfd_name)); if (!mon) { qemuReportError(VIR_ERR_INVALID_ARG, "%s", @@ -1831,10 +1835,27 @@ int qemuMonitorAddHostNetwork(qemuMonitorPtr mon, return -1; } + if (tapfd >= 0 && qemuMonitorSendFileHandle(mon, tapfd_name, tapfd) < 0) + return -1; + if (vhostfd >= 0 && + qemuMonitorSendFileHandle(mon, vhostfd_name, vhostfd) < 0) { + vhostfd = -1; + goto cleanup; + } + if (mon->json) ret = qemuMonitorJSONAddHostNetwork(mon, netstr); else ret = qemuMonitorTextAddHostNetwork(mon, netstr); + +cleanup: + if (ret < 0) { + if (tapfd >= 0 && qemuMonitorCloseFileHandle(mon, tapfd_name) < 0) + VIR_WARN("failed to close device handle '%s'", tapfd_name); + if (vhostfd >= 0 && qemuMonitorCloseFileHandle(mon, vhostfd_name) < 0) + VIR_WARN("failed to close device handle '%s'", vhostfd_name); + } + return ret; } @@ -1862,11 +1883,15 @@ int qemuMonitorRemoveHostNetwork(qemuMonitorPtr mon, int qemuMonitorAddNetdev(qemuMonitorPtr mon, - const char *netdevstr) + const char *netdevstr, + int tapfd, const char *tapfd_name, + int vhostfd, const char *vhostfd_name) { - int ret; - VIR_DEBUG("mon=%p netdevstr=%s", - mon, netdevstr); + int ret = -1; + VIR_DEBUG("mon=%p netdevstr=%s tapfd=%d tapfd_name=%s " + "vhostfd=%d vhostfd_name=%s", + mon, netdevstr, tapfd, NULLSTR(tapfd_name), + vhostfd, NULLSTR(vhostfd_name)); if (!mon) { qemuReportError(VIR_ERR_INVALID_ARG, "%s", @@ -1874,14 +1899,30 @@ int qemuMonitorAddNetdev(qemuMonitorPtr mon, return -1; } + if (tapfd >= 0 && qemuMonitorSendFileHandle(mon, tapfd_name, tapfd) < 0) + return -1; + if (vhostfd >= 0 && + qemuMonitorSendFileHandle(mon, vhostfd_name, vhostfd) < 0) { + vhostfd = -1; + goto cleanup; + } + if (mon->json) ret = qemuMonitorJSONAddNetdev(mon, netdevstr); else ret = qemuMonitorTextAddNetdev(mon, netdevstr); + +cleanup: + if (ret < 0) { + if (tapfd >= 0 && qemuMonitorCloseFileHandle(mon, tapfd_name) < 0) + VIR_WARN("failed to close device handle '%s'", tapfd_name); + if (vhostfd >= 0 && qemuMonitorCloseFileHandle(mon, vhostfd_name) < 0) + VIR_WARN("failed to close device handle '%s'", vhostfd_name); + } + return ret; } - int qemuMonitorRemoveNetdev(qemuMonitorPtr mon, const char *alias) { diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index a20ff8e..e933af1 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -352,14 +352,18 @@ int qemuMonitorCloseFileHandle(qemuMonitorPtr mon, * sendable item here */ int qemuMonitorAddHostNetwork(qemuMonitorPtr mon, - const char *netstr); + const char *netstr, + int tapfd, const char *tapfd_name, + int vhostfd, const char *vhostfd_name); int qemuMonitorRemoveHostNetwork(qemuMonitorPtr mon, int vlan, const char *netname); int qemuMonitorAddNetdev(qemuMonitorPtr mon, - const char *netdevstr); + const char *netdevstr, + int tapfd, const char *tapfd_name, + int vhostfd, const char *vhostfd_name); int qemuMonitorRemoveNetdev(qemuMonitorPtr mon, const char *alias);