6:02 p.m.
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(a)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;
}