Re: [PATCH v3 17/18] util: Alter virNodeDevice{Create|Delete}Vport to use nodedev APIs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 03/12/2017 09:20 PM, Laine Stump wrote:
> On 03/10/2017 04:10 PM, John Ferlan wrote:
>> If we have a connection pointer there's no sense walking through the
>> sysfs in order to create/destroy the vHBA. Instead, let's make use of
>> the node device create/destroy API's.
>>
>> Since we don't have to rewrite all the various parent options for
>> the test driver in order to test whether the storage pool creation
>> works as the node device creation has been tested already, let's just
>> use the altered API to test the storage pool paths.
>>
>> Fix a "bug" in the storage pool test driver code which "assumed"
>> testStoragePoolObjSetDefaults should fill in the configFile for
>> both the Define/Create (persistent) and CreateXML (transient) pools
>> by just VIR_FREE() of the pool during CreateXML.  Because the
>> configFile was filled in, during Destroy, the pool wouldn't be
>> free causing a test using the same name pool to fail.
> 
> Without trying to go through this patch, the commit log makes it sound like there are 3 separate things being done. Or am I misinterpreting? Can it maybe be split up so a mindless reviewer doesn't need to do any work to figure out which is the code fixing the bug. If no splitting is possible/useful, then I'll tackle it as-is tomorrow.
> 

What's happening here is using the NodeDevice API's in order to create
the vHBA instead of essentially repeating what the nodedev API does for
create.

In working through the code I discovered the test_driver
pool->configFile "bug" and it seems "over described" it... I can
separate that.

The primary reason for doing this is I didn't want to rewrite all the
parent* options in the test driver in order to test the storage pool XML
options. By using the node device create - I get that for free!

I also figured it'd be "quicker" and more reliable to use what nodedev
driver has 'in memory' rather than walking through the file system that
theoretically udev has already done for us and nodedev has saved away.

I could really take the hard road and create an API that would handle
all those parent* options and do the right thing. That would mean both
nodedevice and storage pool would call that.


John

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux