Re: [libvirt] [PATCH] qemuOpenVhostNet: Decrease vhostfdSize on open failure
On 05/24/2013 08:50 AM, Michal Privoznik wrote:
Currently, if there's an error opening /dev/vhost-net (e.g.
because
it doesn't exist) but it's not required we proceed with vhostfd array
filled with -1 and vhostfdSize unchanged. Later, when constructing
the qemu command line only non-negative items within vhostfd array
are taken into account. This means, vhostfdSize may be greater than
the actual count of non-negative items in vhostfd array. This results
in improper command line arguments being generated, e.g.:
-netdev tap,fd=21,id=hostnet0,vhost=on,vhostfd=(null)
---
src/qemu/qemu_command.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 434f5a7..d969f88 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -486,6 +486,8 @@ qemuOpenVhostNet(virDomainDefPtr def,
"but is unavailable"));
goto error;
}
+ i--;
+ (*vhostfdSize)--;
This will still go back through the loop again and try to open another.
I would instead just set vhostfdSize = i (in case there were any
successful opens) and break out of the loop.
And again you'll need to decide what is an error and what gets just a
warning - if someone asks for 3 queues and gets none, that should lead
to continuing without any error or even warning. But if they request 2
and only get one, is that an error, or do we warn and continue? or just
silently continue? (I don't have the answer, I'm just asking the
question :-)
}
}
virDomainAuditNetDevice(def, net, "/dev/vhost-net", *vhostfdSize);
@@ -6560,12 +6562,10 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
}
for (i = 0; i < vhostfdSize; i++) {
- if (vhostfd[i] >= 0) {
- virCommandTransferFD(cmd, vhostfd[i]);
- if (virAsprintf(&vhostfdName[i], "%d", vhostfd[i]) < 0) {
- virReportOOMError();
- goto cleanup;
- }
+ virCommandTransferFD(cmd, vhostfd[i]);
+ if (virAsprintf(&vhostfdName[i], "%d", vhostfd[i]) < 0) {
+ virReportOOMError();
+ goto cleanup;
}
}