[libvirt] [PATCH 06/15] Forward Mode 'Hostdev' qemu driver implementation

[Shradha Shah]

On 08/14/2012 07:44 AM, Laine Stump wrote:
> On 08/10/2012 12:24 PM, Shradha Shah wrote:
> 
> Some explanation is needed in the commit log of what this is being done
> here. A cut-paste of the comment in the code would be a good start (any
> anyway, that comment can be changed since it's talking about "when there
> is a network with <forward mode='hostdev'>", but that "when" is "now" :-)
> 
> 
>> Signed-off-by: Shradha Shah <sshah at solarflare.com>
>> ---
>>  src/qemu/qemu_command.c |   27 +++++++++++++++++++++++++++
>>  1 files changed, 27 insertions(+), 0 deletions(-)
>>
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index 6f6c6cd..bb66364 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -24,6 +24,7 @@
>>  #include <config.h>
>>  
>>  #include "qemu_command.h"
>> +#include "qemu_hostdev.h"
>>  #include "qemu_capabilities.h"
>>  #include "qemu_bridge_filter.h"
>>  #include "cpu/cpu.h"
>> @@ -5221,12 +5222,38 @@ qemuBuildCommandLine(virConnectPtr conn,
>>  
>>              actualType = virDomainNetGetActualType(net);
>>              if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
>> +                virDomainHostdevDefPtr hostdev = virDomainNetGetActualHostdev(net);
>> +                virDomainHostdevDefPtr found;
>>                  /* type='hostdev' interfaces are handled in codepath
>>                   * for standard hostdev (NB: when there is a network
>>                   * with <forward mode='hostdev', there will need to be
>>                   * code here that adds the newly minted hostdev to the
>>                   * hostdevs array).
>>                   */
>> +                if (qemuAssignDeviceHostdevAlias(def,
>> +                                                 hostdev,
> 
> Combine the above two lines.
> 
>> +                                                 (def->nhostdevs-1)) < 0) {
>> +                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +                                    _("Could not assign alias to Net Hostdev"));
>> +                    goto error;
>> +                }
>> +                
>> +                if (virDomainHostdevFind(def,
>> +                                         hostdev,
>> +                                         &found) < 0) {
> 
> 
> If the device is found already on the list, you should log an error and
> fail.

The device will be found on the list when using interface type=hostdev.
If I log an error and fail wouldn't that mean that interface type=hostdev will always fail
at this point?

> 
> 
>> +                    if (virDomainHostdevInsert(def, 
>> +                                               hostdev) < 0) {
>> +                        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +                                        _("Hostdev not inserted into the array"));
>> +                        goto error;
>> +                    }
>> +                    if (qemuPrepareHostdevPCIDevices(driver, def->name, def->uuid, 
>> +                                                     &hostdev, 1) < 0) {
>> +                        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +                                        _("Prepare Hostdev PCI Devices failed"));
>> +                        goto error;
> 
> It took me awhile to follow that trail, but I do finally understand that
> this is necessary (because qemuPrepareHostDevices has already been
> called by the time we get to here and are building the qemu commandline).
> 
>> +                    } 
>> +                }
>>                  continue;
>>              }
>>  
> 
> ACK with a better commit log message, fixing the comment in the code,
> and logging an error if the device is found already on the hostdev list.
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list