On 08/16/2016 11:41 AM, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1366108
So far, the hotplug of vhost-user "worked" by pure chance. Well, qemu would error on our commands, but nevertheless. Now that we have everything prepare, We should support hotplugging og
prepared, we ... of
vhost-user. Firstly, we need to plug in chardev (through which qemu talks to OpenVSwitch), then netdev and only after that we can plug in the virtio-net-pci device.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_hotplug.c | 49 +++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 45 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index feb1f44..0bcb411 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -933,6 +933,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); virDomainCCWAddressSetPtr ccwaddrs = NULL; size_t i; + char *charDevAlias = NULL;
/* preallocate new slot for device */ if (VIR_REALLOC_N(vm->def->nets, vm->def->nnets + 1) < 0) @@ -954,11 +955,11 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, case VIR_DOMAIN_NET_TYPE_DIRECT: case VIR_DOMAIN_NET_TYPE_BRIDGE: case VIR_DOMAIN_NET_TYPE_HOSTDEV: + case VIR_DOMAIN_NET_TYPE_VHOSTUSER: /* These types are supported. */ break;
case VIR_DOMAIN_NET_TYPE_USER: - case VIR_DOMAIN_NET_TYPE_VHOSTUSER: case VIR_DOMAIN_NET_TYPE_SERVER: case VIR_DOMAIN_NET_TYPE_CLIENT: case VIR_DOMAIN_NET_TYPE_MCAST: @@ -1148,6 +1149,26 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, goto cleanup; }
+ if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER) { + if (!qemuDomainSupportsNetdev(vm->def, priv->qemuCaps, net)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Netdev support unavailable"));
"%s", on previous line "vhost-user hot-plug not support by this version of qemu"
+ goto cleanup; + } + + if (virAsprintf(&charDevAlias, "char%s", net->info.alias) < 0) + goto cleanup; + + qemuDomainObjEnterMonitor(driver, vm); + if (qemuMonitorAttachCharDev(priv->mon, charDevAlias, net->data.vhostuser) < 0) { + ignore_value(qemuDomainObjExitMonitor(driver, vm)); + virDomainAuditNet(vm, NULL, net, "attach", false); + goto cleanup; + } + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; + } + qemuDomainObjEnterMonitor(driver, vm); if (qemuDomainSupportsNetdev(vm->def, priv->qemuCaps, net)) { if (qemuMonitorAddNetdev(priv->mon, netstr, @@ -1268,6 +1289,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, } VIR_FREE(vhostfd); VIR_FREE(vhostfdName); + VIR_FREE(charDevAlias); virObjectUnref(cfg); virDomainCCWAddressSetFree(ccwaddrs);
@@ -1283,6 +1305,9 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, if (virAsprintf(&netdev_name, "host%s", net->info.alias) < 0) goto cleanup; qemuDomainObjEnterMonitor(driver, vm); + if (charDevAlias && + qemuMonitorDetachCharDev(priv->mon, charDevAlias) < 0) + VIR_WARN("Failed to remove associated chardev %s", charDevAlias); if (qemuMonitorRemoveNetdev(priv->mon, netdev_name) < 0) VIR_WARN("Failed to remove network backend for netdev %s", netdev_name); @@ -3309,10 +3334,12 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver, virNetDevVPortProfilePtr vport; virObjectEventPtr event; char *hostnet_name = NULL; + char *charDevAlias = NULL; size_t i; int ret = -1; + int actualType = virDomainNetGetActualType(net);
- if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_HOSTDEV) { + if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { /* this function handles all hostdev and netdev cleanup */ ret = qemuDomainRemoveHostDevice(driver, vm, virDomainNetGetActualHostdev(net)); @@ -3322,9 +3349,11 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver, VIR_DEBUG("Removing network interface %s from domain %p %s", net->info.alias, vm, vm->def->name);
- if (virAsprintf(&hostnet_name, "host%s", net->info.alias) < 0) + if (virAsprintf(&hostnet_name, "host%s", net->info.alias) < 0 || + virAsprintf(&charDevAlias, "char%s", net->info.alias) < 0)
Part of me wonders if this should be || (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER && virAsprintf(&hostnet_name, "host%s", net->info.alias) < 0) Since it's only necessary for vhost-user, but it does seem excessive. Then again no more excessive than allocating something we don't use. IDC either way, but I'd be remiss if I didn't point it out.
goto cleanup;
+ qemuDomainObjEnterMonitor(driver, vm); if (qemuDomainSupportsNetdev(vm->def, priv->qemuCaps, net)) { if (qemuMonitorRemoveNetdev(priv->mon, hostnet_name) < 0) { @@ -3347,6 +3376,17 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver, goto cleanup; } } + + if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER) { + /* vhostuser has a chardev too */ + if (qemuMonitorDetachCharDev(priv->mon, charDevAlias) < 0) { + /* well, this is a messy situation. Guest visible PCI device has + * been removed, netdev too but chardev not. The best seems to be + * to just ignore the error and carry on. + */ + } + } +
The attach order is: 1. Build 'netstr' 2. If vhost-user, attach char dev 3. Add netdev/hostdev using netstr But your detach order is 1. Remove netdev/hostdev 2. Remove vhost-user chardev See anything wrong with that dependency-wise? Look at the failure code for Attach - it'll remove CharDev before Netdev So the question is - if you did this in the right order, then would that messy situation not exist? John
if (qemuDomainObjExitMonitor(driver, vm) < 0) goto cleanup;
@@ -3371,7 +3411,7 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver, &net->mac)); }
- if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_DIRECT) { + if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { ignore_value(virNetDevMacVLanDeleteWithVPortProfile( net->ifname, &net->mac, virDomainNetGetActualDirectDev(net), @@ -3397,6 +3437,7 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver,
cleanup: virObjectUnref(cfg); + VIR_FREE(charDevAlias); VIR_FREE(hostnet_name); return ret; }