On 05/16/2013 08:49 AM, Michal Privoznik wrote:
> ---
> src/qemu/qemu_command.c | 26 ++++++++++++++++++--------
> src/qemu/qemu_hotplug.c | 27 ++++++++++++++++-----------
> 2 files changed, 34 insertions(+), 19 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 5d64705..7661b13 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -6499,12 +6499,16 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
>
> 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;
> @@ -6526,11 +6530,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;
Hmm. So in the case of macvtap, you have a single macvtap device fd and
multiple vhost-net fds. Is this correct/complete?
> + 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;
> @@ -6594,15 +6602,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 695edc7..bdad4d8 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -737,11 +737,14 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
>
> 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 = 0;
I think you wanted to say ".... = 1;" here, didn't you?
> + 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;
> @@ -749,11 +752,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;
But in this case, for macvtap you hard code to 1 tapfd and 1 vhost-net,
which isn't the same as what you do for the initial startup commandline
(1 tapfd, n vhost-nets).
> 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)
> @@ -762,11 +765,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;
> }
> @@ -956,15 +959,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);
The discrepency between startup and hotplug vhostfdSize needs to be
fixed (and also I'm still curious if macvtap doesn't support multiple
queues, since users of macvtap are likely already the ones who are most
performance-conscious, so most likely to want multiple queues).
Also you need to set to 1 instead of 0 in the hotplug case when the
config says nothing.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list