Hi Wen,
The patch is excellent. I tested the it and it worked with the back-end
_passthrough_affected_by API properly. I find a few problems.
First problem is that only when mouse points to the "Name" column, it
shows the related devices. It's not obvious enough for the user to
discover it. Maybe we can also show the tooltip in all the columns, or
as long as the mouse points to any place inside the row of the device.
The second problem is that the information it shows is not very
self-explanatory. For example, it shows the following information for my
wlan NIC.
interface: wlp3s0, address: 8c:70:5a:5d:5e:90, link_type: IEEE 802.11
While this is correct, but how this information is related to the device
:"Centrino Advanced-N 6205 [Taylor Peak]" is not clear.
Suppose there are 4 affected devices, maybe we can show it like this
Affected devices:
net_wlp3s0_8c_70_5a_5d_5e_90
interface: wlp3s0, address: 8c:70:5a:5d:5e:90, link_type: IEEE 802.11
device2_name_blah
device3_name_blah
device4_name_blah
detailed information blah
I have some other inline comments, please scroll down and see below.
on 2014/10/24 18:16, Wen Wang wrote:
> From: Wen Wang <wenwang@linux.vnet.ibm.com>
>
> This patch provide the way that shows more specific PCI device
> information by hanging the mouse on top of device name.
>
> Signed-off-by: Wen Wang <wenwang@linux.vnet.ibm.com>
> ---
> ui/js/src/kimchi.api.js | 14 ++++++++++++++
> ui/js/src/kimchi.guest_edit_main.js | 25 +++++++++++++++++++++++++
> 2 files changed, 39 insertions(+), 0 deletions(-)
>
> diff --git a/ui/js/src/kimchi.api.js b/ui/js/src/kimchi.api.js
> index 8a6e416..aebac56 100644
> --- a/ui/js/src/kimchi.api.js
> +++ b/ui/js/src/kimchi.api.js
> @@ -1129,6 +1129,20 @@ var kimchi = {
> });
> },
>
> + getPCIDeviceDescriptions : function(pcidev, suc, err) {
I think "getPCIDeviceCompanions" may be a better name.
> + kimchi.requestJSON({
> + url : kimchi.url + 'host/devices?_passthrough_affected_by=' + pcidev,
> + type : 'GET',
> + contentType : 'application/json',
> + dataType : 'json',
> + resend : true,
> + success : suc,
> + error : err ? err : function(data) {
> + kimchi.message.error(data.responseJSON.reason);
> + }
> + });
> + },
> +
> getISCSITargets : function(server, port, suc, err) {
> server = encodeURIComponent(server);
> port = port ? '&_server_port='+encodeURIComponent(port) : '';
> diff --git a/ui/js/src/kimchi.guest_edit_main.js b/ui/js/src/kimchi.guest_edit_main.js
> index 030e112..580f648 100644
> --- a/ui/js/src/kimchi.guest_edit_main.js
> +++ b/ui/js/src/kimchi.guest_edit_main.js
> @@ -365,6 +365,9 @@ kimchi.guest_edit_main = function() {
> kimchi.getCapabilities(function(result) {
> var pciEnabled = result.kernel_vfio;
> for(var i=0; i<hostPCIs.length; i++){
> +
> +
> +
> var itemNode = $.parseHTML(kimchi.substitute($('#pci-tmpl').html(),{
> name: hostPCIs[i].name,
> product: hostPCIs[i].product.description,
> @@ -407,6 +410,28 @@ kimchi.guest_edit_main = function() {
> });
> }
> });
> + kimchi.getPCIDeviceDescriptions(hostPCIs[i].name, function(infoData) {
> + var pciTitle = "";
> + if(infoData.length>0) {
> + for(var p=0; p<infoData.length; p++) {
> + if(infoData[p].interface) {
> + pciTitle += "interface: " + infoData[p].interface;
> + pciTitle += ", address: " + infoData[p].address;
> + pciTitle += ", link_type: " + infoData[p].link_type;
> + pciTitle += " ";
> + } else if(infoData[p].block) {
> + pciTitle += "block: " + infoData[p].block;
> + pciTitle += ", drive_type: " + infoData[p].drive_type;
> + pciTitle += ", model: " + infoData[p].model;
> + pciTitle += " ";
> + }
If you notice, each returned infoData[p] should have a ".device_type".
For example,
[
{
"parent":"pci_0000_00_19_0",
"name":"net_em1_00_21_cc_c2_6d_a6",
"device_type":"net",
"address":"00:21:cc:c2:6d:a6",
"interface":"em1",
"path":"/sys/devices/pci0000:00/0000:00:19.0/net/em1",
"link_type":"IEEE 802.3"
}
]
If the 'infoData[p].device_type == "net"', we can sure that it also
contains ".interface"m ".address" and also ".link_type". I think the
it's better to check the "device_type" rather than to check existence of
"infoData[p].interface" or "infoData[p].block". This is because there
might be a totally different device information which also contains
".interface" or ".block".
> +
> + }
> + for(var q=0; q<infoData.length; q++) {
> + pciTitle === "" || $(".name", "#" + infoData[q].parent).attr("title", pciTitle);
> + }
> + }
> + });
> }
> });
> });
>