[libvirt] [PATCH 1/2] conf: nodedev: Don't refresh host caps in testdriver

Cole Robinson crobinso at redhat.com
Fri Mar 2 21:36:47 UTC 2018


On 03/02/2018 04:02 PM, John Ferlan wrote:
> 
> 
> On 02/23/2018 06:16 PM, Cole Robinson wrote:
>> Add a 'testdriver' bool that we set for test_driver.c nodedevs
>> which will skip accessing host resources via virNodeDeviceUpdateCaps
>>
>> Signed-off-by: Cole Robinson <crobinso at redhat.com>
>> ---
>>  src/conf/node_device_conf.c | 3 +++
>>  src/conf/node_device_conf.h | 1 +
>>  src/test/test_driver.c      | 2 ++
>>  3 files changed, 6 insertions(+)
>>
>> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
>> index fd8f4e4a9..90c940f11 100644
>> --- a/src/conf/node_device_conf.c
>> +++ b/src/conf/node_device_conf.c
>> @@ -2425,6 +2425,9 @@ virNodeDeviceUpdateCaps(virNodeDeviceDefPtr def)
>>  {
>>      virNodeDevCapsDefPtr cap = def->caps;
>>  
>> +    if (def->testdriver)
>> +        return 0;
>> +
>>      while (cap) {
>>          switch (cap->data.type) {
>>          case VIR_NODE_DEV_CAP_SCSI_HOST:
>> diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
>> index 685ae3034..665f766e2 100644
>> --- a/src/conf/node_device_conf.h
>> +++ b/src/conf/node_device_conf.h
>> @@ -316,6 +316,7 @@ struct _virNodeDeviceDef {
>>      char *driver;                       /* optional driver name */
>>      char *devnode;                      /* /dev path */
>>      char **devlinks;                    /* /dev links */
>> +    bool testdriver;                    /* if true, skip host checks */
> 
> Not sure this should be in virNodeDeviceDef... I think it should be in
> virNodeDeviceObj. Yes, a bit more work, but I think cleaner. You'd need
> to create an accessor function in order to set the flag from
> test_driver. Then avoid calling virNodeDeviceUpdateCaps only from
> virNodeDeviceMatch if the flag is set.
> 
> Also instead of "testdriver", how about "skipUpdateCaps" since the
> purpose of this is to skip calling virNodeDeviceUpdateCaps?
> 
> At least that hides that this currently is only for the test driver.
> Perhaps in the future there could be some other reason to not want to
> update the caps for some specific definition after perhaps it's "known"
> or "determined" that a specific update had occurred.

Okay, thanks for review. How about finding a way to remove UpdateCaps
from generic conf.c implementations instead? Seems wrong that something
functions needed to implement ListAllDevices touch host resources, I
don't think other objects work like that

Thanks,
Cole




More information about the libvir-list mailing list