Add PCI passthrough support for cloud-hypervisor platform

[smit-gardhariya]

Add PCI passthrough support for cloud-hypervisor platform
Add parameter to pass options while loading module using modprobe

[anirudhrb]

What warning does this suppress and why?

[smit-gardhariya]

Because of lot of if/else in this function, it will throw too complex function error which is C901 for flak8. There is no better way to handle this as those if/else are needed.

[pupacha]

In that case, we can refactor the new code to a new function for example _configure_node_passthrough_devices and call this fn from _config_node

[smit-gardhariya]

Still one if condition we have to add like below snippet, and this would cause the same error of C901

# Set device passthrough params for node context
        if node_runbook.device_passthrough:
            self._configure_device_passthrough(self, node_runbook, node_context)

[pupacha]

Does it still throw the same error if we move the condition to _configure_device_passthrough itself ?

[smit-gardhariya]

Ideally, we should skip the redundant call to function. Anyway, i made the change please check

[anirudhrb]

There is no better way to handle this as those if/else are needed

If we make the function smaller by extracting bits out into functions, this error will go away, right?

[smit-gardhariya]

There is no better way to handle this as those if/else are needed

If we make the function smaller by extracting bits out into functions, this error will go away, right?

Even if i would put one if condition, it was coming. I wanted to call fn only when needed and avoid redundant call even when device pass through is not required.

I removed that condition also now. Please check the latest push

[pupacha]

Thanks for this change. Looks like we got rid of the flake8 error now.

[smit-gardhariya]

Yes that was because of IF we added. Now we removed it and calling fn every time

It's in reviewing by others.

[pupacha]

In that case, we can refactor the new code to a new function for example _configure_node_passthrough_devices and call this fn from _config_node

[anirudhrb]

Could you please post a sample of how a runbook with passthrough configuration would look?

[smit-gardhariya]

Could you please post a sample of how a runbook with passthrough configuration would look?

requirement: core_count: $(guest_vcpu) memory_mb: $(guest_memory) cloud-hypervisor: disk_img: $(qcow2) disk_img_format: "qcow2" firmware: $(firmware_path) cloud_init: extra_user_data: $(extra_user_data) device_passthrough: - managed: "yes" device_type: "pci" host_domain: "0000" host_bus: "19" host_slot: "00" host_function: "0" - managed: "yes" device_type: "pci" host_domain: "0000" host_bus: "19" host_slot: "00" host_function: "1"

This would remain same for QEMU/CLH. As of now we are focusing on CLH only. QEMU might require some additional change.

[pupacha]

Thanks for addressing all the comments, Smit.

I have another comment in general about the approach.

As I understand it, device_passthrough is under node requirements. While this approach works with deploying of a single node, this fails with cases where multiple nodes with passthrough is needed. For example, all network tests will create multiple nodes. And by the current code logic, the deployment of second will fail since all pci devices provided in runbook would be assigned to first node itself.

PCI devices are associated with the host node. And in our context, host node is synonymous to platform. So, we should associate the host pci devices to platform than to node requirement.

Here is what I'm thinking in the form of a sample schema. Introduce 'device_pools' at platform level and define all the host devices there. Pools can be of different types like 'pci_net', 'pci_gpu' etc. All the devices that we want to be used in provisioning guests need to be defined here. And for a node/VM to use a passthrough device, the pool type and devices count has to be mentioned in the node requirement. Platform needs to handle assigning the devices accordingly from the available devices of the pool. Platform needs to maintain the state of the devices (which vm a device is assigned to or not, etc).

platform:
  - type: cloud-hypervisor
    cloud-hypervisor:
      network_boot_timeout: $(boot_timeout)
      hosts:
        - address: $(host_address)
          username: $(host_username)
          private_key_file: $(admin_private_key_file)
      capture_libvirt_debug_logs: true
      device_pools:
        - type: "pci_net"
           devices:
             - domain: "1"
                bus: "1"
                slot: "1"
                function: "1"
             - domain: "2"
                bus: "2"
                slot: "2"
                function: "2"
        - type: "pci_gpu"
           devices:
             - domain: "3"
                bus: "3"
                slot: "3"
                function: "3"
             - domain: "4"
                bus: "4"
                slot: "4"
                function: "4"
    requirement:
      core_count: $(guest_vcpu)
      memory_mb: $(guest_memory)
      cloud-hypervisor:
        disk_img: $(qcow2)
        disk_img_format: "qcow2"
        cloud_init:
          extra_user_data: $(extra_user_data)
        device_passthrough:
          - host_device_pool: "pci_net" 
            managed: "yes"
            count: 2

I haven't put too much thought into schema. What I listed above is the rough idea. I've also discussed this with @anirudhrb . We can brainstorm more on this offline and finalize the schema.