autotest / virt-test

Linux Virtualization Tests
Other
97 stars 140 forks source link

Enable USB type device passthrough using add_hostdev within virt test #2276

Closed bssrikanth closed 9 years ago

bssrikanth commented 9 years ago

Currently hostdev addition supported adding only pci type device for passthrough. This commit has added capability to within virt test framework to generate a hostdev xml section for USB device passthrough. Guest VM xml add hostdev will now be able accept USB specific parameters and add respective xml clause to guest VM accordingly.

Signed-off-by: Srikanth Aithal sraithal@linux.vnet.ibm.com

sathnaga commented 9 years ago

ACKed.

bssrikanth commented 9 years ago

Please let me know any review comments which would help in getting this merged. Thank you.

will-Do commented 9 years ago

I don't think you need define these new classes for USB device, if there's some Attributes for USB not exist, just define them in 'SourceAddress' will be enough.

bssrikanth commented 9 years ago

@will-Do

Yes, agree with your comments and I am using same 'SourceAddress' class for USB device type as well. I am defining the sub clauses required for USB device type in same class which is 'SourceAddress'. I am giving more details below:

In case of 'pci' type devices only <address> sub-clause was enough inside the source clause [which the existing code in hostdev.py supported] sample hostdev clause for PCI device which was generated by hostdev.py:

<hostdev managed="yes" mode="subsystem" type="pci">
    <source>
        <address bus="3" domain="3" function="0" slot="0" />
    </source>
</hostdev>

In case of 'usb' type devices we had to generate sub-clauses for 'vendor_id', 'product_id' and 'address' [this address sub-clause is different than the one which PCI device would use as shown in samples above and below]

sample hostdev clause for USB device, which is currently generated by the modified hostdev.py:

<hostdev managed="yes" mode="subsystem" type="usb">
    <source>
        <vendor id="0x04b3" />
        <product id="0x4011" />
        <address bus="0x002" device="0x007" />
    </source>
</hostdev>

So I had used same parent class 'SourceAddress' and had added new subclasses for generating the required sub-clauses for USB device which are as below:

UntypedusbAddress: this generates 'address' sub-clause for usb devices. same address clause which was used for default 'pci' type cannot be used here. [ex: ```<address bus="0x002" device="0x007" />```]

UntypedVendor: this generates vendor id subclause [ex: <vendor id="0x04b3" />]

UntypedProduct: this generates product id sub-clause [ex: <product id="0x4011" />]

Please let me know if this clarifies your concern.

Thank you,

Srikanth Aithal

bssrikanth commented 9 years ago

@will-Do please let me know if the previous explanation falls in-line with your expectation. Thanks much for your time in reviewing my patch.

bssrikanth commented 9 years ago

@will-Do sorry for asking again. Please let me know if you are convinced by my explanation. Thanks much for your time in advance.

will-Do commented 9 years ago

@bssrikanth My apologies, I'm working on another project these days, so not reply in time. I will review this PR this week and give a comment then.

lmr commented 9 years ago

Hi @bssrikanth. As you might know, we're moving virt-test to avocado-vt, so I please ask you to consider re-implementing this functionality in the later. The new repo is:

https://github.com/avocado-framework/avocado-vt

Please keep in mind the following guidelines when creating the new PR:

1) Most of the time you can pick the patches of your original PR and merge them on a new branch with minimal changes. 3) Don't use autotest APIs, since we're moving away from using them. Please review the code and change things like utils.system -> process.system, utils.run -> process.run, so on and so forth.

Thanks, and I'm sorry for any inconveniences we might have caused your team.