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@xxxxxxxxxx> > --- > 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; > } > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list