[libvirt] [PATCH] qemuOpenVhostNet: Decrease vhostfdSize on open failure

[Michal Privoznik]

On 24.05.2013 16:42, Laine Stump wrote:
> 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 :-)
> 

Well, MQ is designed to be M:N (where M is the number of TAP devices =
queues, N is the number of vhost-nets). So I think we can continue with
any number of successful openings.

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