Usual problem with the commit subject.

On 23/09/2024 19:42, jspew...@iol.unh.edu wrote:
+    def set_num_virtual_functions(self, num: int, pf_port: Port) -> bool:

I'd call this set_num_of_virtual_functions, set_virtual_functions_num, or set_number_of_vfs. I think I prefer the last one.

+        """Overrides 
:meth:`~.os_session.OSSession.set_num_virtual_functions`."""
+        sys_bus_path = f"/sys/bus/pci/devices/{pf_port.pci}/sriov_numvfs".replace(":", 
"\\:")
+        curr_num_vfs = int(self.send_command(f"cat {sys_bus_path}").stdout)
+        if num > 0 and curr_num_vfs >= num:
+            self._logger.info(
+                f"{curr_num_vfs} VFs already configured on port 
{pf_port.identifier.pci} on node "
+                f"{pf_port.identifier.node}."
+            )
+            return False
+        elif num > 0 and curr_num_vfs > 0:
+            self._logger.error(
+                f"Not enough VFs configured on port {pf_port.identifier.pci} on 
node "
+                f"{pf_port.identifier.node}. Need {num} but only {curr_num_vfs} are 
configured. "
+                "DTS is unable to modify number of preexisting VFs."
+            )
+            raise InternalError("Failed to create VFs on port.")

This is not an InternalError. That exception represents a bug internal to DTS, it's not related to the node not satisfying requirements. I am guessing this requires a new kind of error.

+        self.send_command(f"echo {num} > {sys_bus_path}", privileged=True, 
verify=True)
+        return True
+
+    def get_pci_addr_of_vfs(self, pf_port: Port) -> list[str]:
+        """Overrides :meth:`~.os_session.OSSession.get_pci_addr_of_vfs`."""
+        sys_bus_path = f"/sys/bus/pci/devices/{pf_port.pci}".replace(":", 
"\\:")
+        curr_num_vfs = int(self.send_command(f"cat 
{sys_bus_path}/sriov_numvfs").stdout)
+        if curr_num_vfs > 0:
+            pci_addrs = self.send_command(
+                'awk -F "PCI_SLOT_NAME=" "/PCI_SLOT_NAME=/ {print \\$2}" '
+                + f"{sys_bus_path}/virtfn*/uevent",

I like the use of awk. Using a TextWrapper may be overkill when we have easier options like this one.


+    def set_num_virtual_functions(self, num: int, pf_port: Port) -> bool:
+        """Update the number of virtual functions (VFs) on a port.

Like Juraj mentioned, this docstring should say that it can be used for creating and removing VFs.

Reply via email to