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