Re: [libvirt] [PATCHv4 4/4] qemu: Enable multiqueue network

On 05/21/2013 10:18 AM, Michal Privoznik wrote: s/nothing else/nothing besides/ (or "nothing other than") You probably want to check for queues > 1 rather than != 0. (I would *almost* suggest logging this error anytime tapfdSize came back from the subordinate function(s) smaller than net->driver.virtio.queues; that way would wouldn't have to have top change stuff about non-support in multiple places when macvtap gets the support later. However, if you did the error reporting that way, you wouldn't be able to give as many specifics about the reasoning.) > + } > + > if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK || > actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) { > - if (VIR_ALLOC(tapfd) < 0 || VIR_ALLOC(tapfdName) < 0) { > + tapfdSize = net->driver.virtio.queues; > + if (!tapfdSize) > + tapfdSize = 1; > + > + if (VIR_ALLOC_N(tapfd, tapfdSize) < 0 || > + VIR_ALLOC_N(tapfdName, tapfdSize) < 0) { > virReportOOMError(); > goto cleanup; > } > > - tapfdSize = 1; > if (qemuNetworkIfaceConnect(def, conn, driver, net, > qemuCaps, tapfd, &tapfdSize) < 0) > goto cleanup; > @@ -6532,11 +6546,15 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, > actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { > /* Attempt to use vhost-net mode for these types of > network device */ > - if (VIR_ALLOC(vhostfd) < 0 || VIR_ALLOC(vhostfdName)) { > + vhostfdSize = net->driver.virtio.queues; > + if (!vhostfdSize) > + vhostfdSize = 1; > + > + if (VIR_ALLOC_N(vhostfd, vhostfdSize) < 0 || > + VIR_ALLOC_N(vhostfdName, vhostfdSize)) { > virReportOOMError(); > goto cleanup; > } > - vhostfdSize = 1; > > if (qemuOpenVhostNet(def, net, qemuCaps, vhostfd, &vhostfdSize) < 0) > goto cleanup; > @@ -6600,15 +6618,17 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, > cleanup: > if (ret < 0) > virDomainConfNWFilterTeardown(net); > - for (i = 0; i < tapfdSize; i++) { > + for (i = 0; tapfd && i < tapfdSize; i++) { > if (ret < 0) > VIR_FORCE_CLOSE(tapfd[i]); > - VIR_FREE(tapfdName[i]); > + if (tapfdName) > + VIR_FREE(tapfdName[i]); > } > - for (i = 0; i < vhostfdSize; i++) { > + for (i = 0; vhostfd && i < vhostfdSize; i++) { > if (ret < 0) > VIR_FORCE_CLOSE(vhostfd[i]); > - VIR_FREE(vhostfdName[i]); > + if (vhostfdName) > + VIR_FREE(vhostfdName[i]); > } > VIR_FREE(tapfd); > VIR_FREE(vhostfd); > diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c > index 7e50592..e7d200f 100644 > --- a/src/qemu/qemu_hotplug.c > +++ b/src/qemu/qemu_hotplug.c > @@ -740,13 +740,26 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, > goto cleanup; > } > > + /* Currently nothing else then TAP devices supports multiqueue. */ > + } > + > if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE || > actualType == VIR_DOMAIN_NET_TYPE_NETWORK) { > - if (VIR_ALLOC(tapfd) < 0 || VIR_ALLOC(vhostfd) < 0) { > + tapfdSize = vhostfdSize = net->driver.virtio.queues; > + if (!tapfdSize) > + tapfdSize = vhostfdSize = 1; > + if (VIR_ALLOC_N(tapfd, tapfdSize) < 0 || > + VIR_ALLOC_N(vhostfd, vhostfdSize) < 0) { > virReportOOMError(); > goto cleanup; > } > - tapfdSize = vhostfdSize = 1; > if (qemuNetworkIfaceConnect(vm->def, conn, driver, net, > priv->qemuCaps, tapfd, &tapfdSize) < 0) > goto cleanup; > @@ -754,11 +767,11 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, > if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, vhostfd, &vhostfdSize) < 0) > goto cleanup; > } else if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { > + tapfdSize = vhostfdSize = 1; > if (VIR_ALLOC(tapfd) < 0 || VIR_ALLOC(vhostfd) < 0) { > virReportOOMError(); > goto cleanup; > } > - tapfdSize = vhostfdSize = 1; > if ((tapfd[0] = qemuPhysIfaceConnect(vm->def, driver, net, > priv->qemuCaps, > VIR_NETDEV_VPORT_PROFILE_OP_CREATE)) < 0) > @@ -767,11 +780,11 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, > if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, vhostfd, &vhostfdSize) < 0) > goto cleanup; > } else if (actualType == VIR_DOMAIN_NET_TYPE_ETHERNET) { > - if (VIR_ALLOC(vhostfd) < 0) { > - virReportOOMError(); > - goto cleanup; > - } > vhostfdSize = 1; > + if (VIR_ALLOC(vhostfd) < 0) { > + virReportOOMError(); > + goto cleanup; > + } > if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, vhostfd, &vhostfdSize) < 0) > goto cleanup; > } > @@ -961,15 +974,17 @@ cleanup: > > VIR_FREE(nicstr); > VIR_FREE(netstr); > - for (i = 0; i < tapfdSize; i++) { > + for (i = 0; tapfd && i < tapfdSize; i++) { > VIR_FORCE_CLOSE(tapfd[i]); > - VIR_FREE(tapfdName[i]); > + if (tapfdName) > + VIR_FREE(tapfdName[i]); > } > VIR_FREE(tapfd); > VIR_FREE(tapfdName); > - for (i = 0; i < vhostfdSize; i++) { > + for (i = 0; vhostfd && i < vhostfdSize; i++) { > VIR_FORCE_CLOSE(vhostfd[i]); > - VIR_FREE(vhostfdName[i]); > + if (vhostfdName) > + VIR_FREE(vhostfdName[i]); > } > VIR_FREE(vhostfd); > VIR_FREE(vhostfdName); ACK with those two small fixes.