Merge pull request #6301 from maramsmurthy/nvme-dynamic-flbas-and-type-fix

PraveenPenguin · web-flow · commit 58cb69a21f83 · 2026-05-18T23:01:14.000+05:30
Nvme dynamic flbas and type fix
diff --git a/avocado/utils/nvme.py b/avocado/utils/nvme.py
@@ -110,7 +110,18 @@ def get_current_ns_ids(controller_name):
     output = process.run(cmd, shell=True, sudo=True, ignore_status=True).stdout_text
     for line in output.splitlines():
         if line.startswith("["):
-            namespaces.append(int(line.split()[1].split("]")[0]) + 1)
+            # Format is: [   0]:0x1 where 0x1 is the namespace ID in hex
+            if ':' in line:
+                ns_id_hex = line.split(':')[-1].strip()
+                # Convert hex string (e.g., '0x1') to integer
+                try:
+                    namespaces.append(int(ns_id_hex, 16))
+                except ValueError:
+                    # If not hex, try decimal
+                    try:
+                        namespaces.append(int(ns_id_hex))
+                    except ValueError:
+                        LOGGER.warning(f"Could not parse namespace ID from: {line}")
     return namespaces
 
 
@@ -280,18 +291,142 @@ def attach_ns(ns_id, controller_name, cont_id):
     """
     attach the namespace_id to specified controller
 
-    :param ns_id: namespace ID
+    :param ns_id: namespace ID (string or int)
     :param controller_name: controller name
     :param cont_id: controller_ID
     """
+    # Ensure ns_id is an integer for consistent comparison
+    ns_id = int(ns_id)
     cmd = f"nvme attach-ns /dev/{controller_name} --namespace-id={ns_id} -controllers={cont_id}"
     if not process.run(cmd, shell=True, ignore_status=True):
         raise NvmeException("namespaces attach command failed")
     ns_rescan(controller_name)
+    # Add delay to allow kernel to update namespace list after rescan
+    time.sleep(2)
     if not is_ns_exists(controller_name, ns_id):
         raise NvmeException("namespaces attached but not listing")
 
 
+def get_supported_lba_formats(controller_name):
+    """
+    Query and return supported LBA formats for the NVMe controller.
+
+    This function attempts to retrieve the LBA Format (LBAF) array from an
+    existing namespace on the controller. The LBAF array is controller-wide,
+    meaning all namespaces on the same controller share the same set of
+    supported formats, though each namespace can select a different format.
+
+    :param controller_name: Name of the controller (e.g., 'nvme0')
+    :return: List of dicts containing format details, each with keys:
+        'index', 'block_size', 'metadata_size', 'relative_performance', 'valid'
+    :rtype: list
+    :raises: NvmeException if unable to query formats
+    """
+    # Try to get formats from an existing namespace
+    namespaces = get_current_ns_list(controller_name)
+    
+    if namespaces:
+        # Query first available namespace for LBAF array
+        namespace = get_namespace_absolute_path(namespaces[0])
+        cmd = f"nvme id-ns {namespace} -o json"
+        try:
+            result = process.run(cmd, shell=True, ignore_status=False, sudo=True)
+            ns_data = json.loads(result.stdout_text)
+            
+            # Extract and parse LBAF array
+            lba_formats = []
+            for idx, lbaf in enumerate(ns_data.get('lbafs', [])):
+                # Check if format is valid (ds > 0 means valid data size)
+                ds = lbaf.get('ds', 0)
+                if ds > 0:
+                    lba_formats.append({
+                        'index': idx,
+                        'block_size': 2 ** ds,  # Convert power-of-2 to actual size
+                        'metadata_size': lbaf.get('ms', 0),
+                        'relative_performance': lbaf.get('rp', 0),
+                        'valid': True
+                    })
+            
+            if lba_formats:
+                LOGGER.debug(f"Found {len(lba_formats)} valid LBA formats for {controller_name}")
+                return lba_formats
+                
+        except (process.CmdError, json.JSONDecodeError, KeyError) as e:
+            LOGGER.warning(f"Failed to query LBA formats from namespace: {e}")
+    
+    # Fallback: If no namespace exists or query failed, return common formats
+    # Most NVMe devices support at least 512B and 4KB formats at indices 0 and 1
+    # This is a safe assumption based on NVMe specification common implementations
+    LOGGER.warning(
+        f"No namespace found on {controller_name}, using common format assumptions. "
+        f"This fallback assumes FLBAS indices 0 and 1 are valid with 512B and 4KB blocks respectively."
+    )
+    return [
+        {'index': 0, 'block_size': 512, 'metadata_size': 0,
+         'relative_performance': 0, 'valid': True},
+        {'index': 1, 'block_size': 4096, 'metadata_size': 0,
+         'relative_performance': 0, 'valid': True}
+    ]
+
+
+def get_optimal_flbas(controller_name):
+    """
+    Determine the optimal FLBAS (Formatted LBA Size) index for namespace creation.
+    
+    FLBAS is a namespace-specific field (bits 3:0) that selects which LBA format
+    from the controller's LBAF array should be used. This function implements
+    an intelligent selection strategy:
+    
+    1. Prefer FLBAS index 0 if it's valid (most common default)
+    2. If index 0 is invalid, select first format with metadata_size=0
+    3. Among formats with no metadata, prefer common block sizes (512B, 4KB)
+    
+    :param controller_name: Name of the controller (e.g., 'nvme0')
+    :return: FLBAS index (0-15) to use for namespace creation
+    :rtype: int
+    :raises: NvmeException if no valid format is found
+    """
+    try:
+        formats = get_supported_lba_formats(controller_name)
+        
+        if not formats:
+            raise NvmeException(f"No valid LBA formats found for {controller_name}")
+        
+        # Strategy 1: Try index 0 first (most common default)
+        for fmt in formats:
+            if fmt['index'] == 0 and fmt['valid']:
+                LOGGER.info(f"Using FLBAS index 0 (block_size={fmt['block_size']}B) "
+                           f"for {controller_name}")
+                return 0
+        
+        # Strategy 2: Find first format with no metadata
+        formats_no_metadata = [f for f in formats if f['metadata_size'] == 0]
+        
+        if formats_no_metadata:
+            # Prefer common block sizes: 512B or 4KB
+            for preferred_size in [512, 4096]:
+                for fmt in formats_no_metadata:
+                    if fmt['block_size'] == preferred_size:
+                        LOGGER.info(f"Using FLBAS index {fmt['index']} "
+                                   f"(block_size={fmt['block_size']}B) for {controller_name}")
+                        return fmt['index']
+            
+            # If no preferred size found, use first format without metadata
+            selected = formats_no_metadata[0]
+            LOGGER.info(f"Using FLBAS index {selected['index']} "
+                       f"(block_size={selected['block_size']}B) for {controller_name}")
+            return selected['index']
+        
+        # Strategy 3: Last resort - use first valid format even with metadata
+        selected = formats[0]
+        LOGGER.warning(f"All formats have metadata. Using FLBAS index {selected['index']} "
+                      f"(block_size={selected['block_size']}B, metadata={selected['metadata_size']}B)")
+        return selected['index']
+        
+    except Exception as e:
+        raise NvmeException(f"Failed to determine optimal FLBAS for {controller_name}: {e}")
+
+
 def create_full_capacity_ns(controller_name, shared_ns=False):
     """
     Creates one namespace with full capacity
@@ -304,19 +439,112 @@ def create_full_capacity_ns(controller_name, shared_ns=False):
     create_one_ns("1", controller_name, ns_size, shared_ns=shared_ns)
 
 
-def create_one_ns(ns_id, controller_name, ns_size, shared_ns=False):
-    """
-    creates a single namespaces with given size and controller_id
-
-    :param ns_id: Namespace ID
-    :param controller_name: name of the controller like nvme0/nvme1 etc..
-    :param ns_size: Size of the namespace that is going to be created
-    """
-    cmd = f"nvme create-ns /dev/{controller_name} --nsze={ns_size} --ncap={ns_size} --flbas=0 --dps=0"
+def create_one_ns(ns_id, controller_name, ns_size, shared_ns=False, flbas=0):
+    """
+    Creates a single namespace with given size and controller_id.
+    
+    This function supports dynamic FLBAS (Formatted LBA Size) selection with
+    intelligent fallback behavior:
+    
+    **FLBAS Selection Modes:**
+    
+    1. **Default Mode (flbas=0)**: Backward compatible behavior
+       - First attempts namespace creation with FLBAS=0
+       - If FLBAS=0 fails, automatically detects optimal FLBAS and retries
+       - Recommended for most use cases
+    
+    2. **Auto-Detect Mode (flbas=-1)**: Skip FLBAS=0 attempt
+       - Immediately detects and uses optimal FLBAS value
+       - Useful for devices known to have non-standard FLBAS configurations
+       - Saves one failed attempt on such devices
+    
+    3. **Explicit Mode (flbas=1-15)**: Use specific FLBAS index
+       - Uses the specified FLBAS value without auto-detection
+       - No retry on failure
+       - For advanced users who know their device's FLBAS requirements
+
+    :param ns_id: Namespace ID (typically 1-based)
+    :param controller_name: Name of the controller like nvme0/nvme1 etc..
+    :param ns_size: Size of the namespace in blocks (based on selected LBA format)
+    :param shared_ns: Whether to create a shared namespace (default: False)
+    :param flbas: FLBAS index to use:
+                  - 0 (default): Try FLBAS=0, auto-retry on failure
+                  - -1: Skip FLBAS=0, immediately use auto-detected optimal value
+                  - 1-15: Use explicit FLBAS index, no auto-detection
+    :raises: NvmeException if namespace creation fails with all attempted FLBAS values
+    
+    :example:
+        # Standard usage (backward compatible)
+        create_one_ns("1", "nvme0", 2097152)
+        
+        # Force auto-detection for non-standard devices
+        create_one_ns("1", "nvme0", 2097152, flbas=-1)
+        
+        # Use explicit FLBAS value
+        create_one_ns("1", "nvme0", 2097152, flbas=2)
+    """
+    # Determine retry strategy based on flbas parameter
+    # Only enable auto-detection fallback for default flbas=0
+    should_retry_with_auto_detect = (flbas == 0)
+    
+    # Handle explicit auto-detection request (flbas=-1)
+    # This skips the FLBAS=0 attempt and goes straight to optimal detection
+    if flbas == -1:
+        try:
+            flbas = get_optimal_flbas(controller_name)
+            LOGGER.info(f"Auto-detected FLBAS={flbas} for namespace creation on {controller_name}")
+        except NvmeException as e:
+            LOGGER.error(f"Failed to auto-detect FLBAS: {e}")
+            raise NvmeException(f"Cannot create namespace: FLBAS auto-detection failed: {e}")
+    
+    # Build and execute namespace creation command
+    cmd = f"nvme create-ns /dev/{controller_name} --nsze={ns_size} --ncap={ns_size} --flbas={flbas} --dps=0"
     if shared_ns:
         cmd = f"{cmd} -m 1"
-    if process.system(cmd, shell=True, ignore_status=True):
-        raise NvmeException(f"namespace create command failed {cmd}")
+    
+    result = process.system(cmd, shell=True, ignore_status=True)
+    
+    # Intelligent retry: If creation failed with default FLBAS=0, try auto-detection
+    # This provides automatic fallback for devices with non-standard FLBAS configurations
+    if result != 0 and should_retry_with_auto_detect:
+        LOGGER.warning(f"Namespace creation failed with FLBAS=0 on {controller_name}")
+        LOGGER.info("Attempting auto-detection of optimal FLBAS value...")
+        
+        try:
+            optimal_flbas = get_optimal_flbas(controller_name)
+            
+            # Only retry if optimal FLBAS is different from what we tried
+            if optimal_flbas != 0:
+                LOGGER.info(f"Retrying with auto-detected FLBAS={optimal_flbas}")
+                cmd = f"nvme create-ns /dev/{controller_name} --nsze={ns_size} --ncap={ns_size} --flbas={optimal_flbas} --dps=0"
+                if shared_ns:
+                    cmd = f"{cmd} -m 1"
+                
+                result = process.system(cmd, shell=True, ignore_status=True)
+                
+                if result == 0:
+                    LOGGER.info(f"Namespace creation succeeded with FLBAS={optimal_flbas}")
+                    flbas = optimal_flbas  # Update for logging
+                else:
+                    raise NvmeException(
+                        f"Namespace creation failed with both FLBAS=0 and auto-detected FLBAS={optimal_flbas}. "
+                        f"Command: {cmd}"
+                    )
+            else:
+                raise NvmeException(
+                    f"Namespace creation failed with FLBAS=0 and auto-detection also suggests FLBAS=0. "
+                    f"Command: {cmd}"
+                )
+        except NvmeException as e:
+            raise NvmeException(f"Namespace creation failed: {e}")
+    elif result != 0:
+        # Failed with user-specified FLBAS (not 0), don't retry
+        raise NvmeException(
+            f"Namespace create command failed with FLBAS={flbas}. "
+            f"Command: {cmd}. Exit code: {result}"
+        )
+    
+    # Attach namespace to controller(s)
     cont_id = get_controller_id(controller_name)
     if shared_ns:
         ctrls = get_alternate_controller_name(controller_name)
diff --git a/avocado/utils/pci.py b/avocado/utils/pci.py
@@ -192,6 +192,10 @@ def get_slot_from_sysfs(full_pci_address):
 
     :return: Removed port related details using re, only returns till
              physical slot of the adapter.
+             Examples::
+
+                 U78CC.001.FZHAK92-P2-C3
+                 U50EE.001.WZS0011-P3-C20-R2
     """
     if not os.path.isfile(f"/sys/bus/pci/devices/{full_pci_address}/devspec"):
         return None
@@ -201,7 +205,7 @@ def get_slot_from_sysfs(full_pci_address):
     if not os.path.isfile(f"/proc/device-tree/{devspec}/ibm,loc-code"):
         return None
     slot = genio.read_file(f"/proc/device-tree/{devspec}/ibm,loc-code")
-    slot_ibm = re.match(r"((\w+)[.])+(\w+)-[PC(\d+)-]*C(\d+)", slot)
+    slot_ibm = re.match(r"((\w+)[.])+(\w+)-[PC(\d+)-]*C(\d+)(?:-R\d+)?", slot)
     if slot_ibm:
         return slot_ibm.group()
     slot_openpower = re.match(r"(\w+)[\s]*(\w+)(\d*)", slot)
