SMB server and client tools by SRIKKANTH · Pull Request #4182 · microsoft/lisa

SRIKKANTH · 2026-01-01T16:27:20Z

These tools are useful in validating samba file share with Linux OS and CIFS module in kernel.

kanchansenlaskar · 2026-01-01T17:16:51Z

lisa/tools/smb.py

+
+    def _check_exists(self) -> bool:
+        # Check if mount.cifs exists
+        result = self.node.execute("which mount.cifs", sudo=True)


Can we create a small tool for which and use that here?

added command name so that "_check_exists()" can do the checking

kanchansenlaskar · 2026-01-01T17:16:56Z

lisa/tools/smb.py

+        self.node.execute(f"mkdir -p {share_path}", sudo=True)
+        self.node.execute(f"chmod 777 {share_path}", sudo=True)


Can we use mkdir and chmod tools here?

kanchansenlaskar · 2026-01-01T17:33:15Z

lisa/tools/smb.py

+        mount_options = [f"vers={smb_version}", "file_mode=0777", "dir_mode=0777"]
+
+        if username and password:
+            mount_options.extend([f"username={username}", f"password={password}"])


Are we ok to pass password here? or can we make use of some more secured approach here?
e.g. ~/.smbcredentials containing:
username=youruser
password=yourpassword
domain=MYDOMAIN

Then we secure it with something like: chmod 600 ~/.smbcredentials

and then mount it the way you want:
sudo mount -t cifs //server/share /mnt/point
-o credentials=/home/lisa/.smbcredentials

Removed username and password support as its not really needed for cifs testcase.

adityagesh · 2026-01-02T05:32:23Z

lisa/tools/smb.py

+
+        # Write SMB configuration
+        echo = self.node.tools[Echo]
+        echo.write_to_file(smb_config, PurePosixPath("/etc/samba/smb.conf"), sudo=True)


Can we have the config file path as a class variable?

Direct path used here as Its same across all the distros.

My concern is that the same file may be referenced in the future in a different method.

adityagesh · 2026-01-02T05:35:57Z

lisa/tools/smb.py

+        if not service.is_service_running(self._smb_service):
+            raise LisaException(f"Failed to start SMB server ({self._smb_service})")
+        if not service.is_service_running(self._nmb_service):
+            raise LisaException(f"Failed to start NMB server ({self._nmb_service})")


Isn't restart_service a sync operation? Doesn't restart_service fail it service doesn't start running? Is a separate check required?

SMB server and client tools These tools are useful in validating samba file share with Linux OS and CIFS module in kernel.

Copilot

Pull request overview

This PR adds SMB (Server Message Block) server and client tools to enable validation of samba file shares with Linux OS and the CIFS kernel module. The implementation follows patterns established by similar tools in the codebase like NFSServer and NFSClient.

Key changes:

Introduces SmbServer and SmbClient tool classes with distribution-specific installation and service management
Adds CIFS to the FileSystem enum to support SMB/CIFS mounting operations
Exports the new tools through the main tools package

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File	Description
lisa/tools/smb.py	New file implementing SmbServer for hosting SMB shares and SmbClient for mounting them, with support for Ubuntu, Debian, Alpine, CBLMariner, CoreOs, Fedora, Oracle, Redhat, and Suse distributions
lisa/tools/mkfs.py	Adds 'cifs' to the FileSystem enum to enable CIFS filesystem type in mount operations
lisa/tools/init.py	Exports SmbClient and SmbServer classes for use by test suites

Test Suggestions (Required per Guideline 1000000):

Based on the SMB server and client tools implementation, the following testing guidance is recommended:

Key Test Cases:
Since this PR introduces new tools without corresponding test cases, integration tests should be created to validate SMB functionality. Suggested test method names (to be created):

verify_smb_share_creation
verify_smb_client_mount
verify_smb_file_operations

Impacted LISA Features:
This change does not directly impact existing LISA features but enables validation of SMB/CIFS capabilities which may be used by features such as:

Storage-related features
Network file sharing capabilities

Tested Azure Marketplace Images:
Test with representative images from major Linux distributions that support SMB:

canonical 0001-com-ubuntu-server-jammy 22_04-lts-gen2 latest
debian debian-12 12-gen2 latest
microsoftcblmariner azure-linux-3 azure-linux-3-gen2 latest
redhat rhel 9_5 latest
suse sles-15-sp6 gen2 latest

lisa/tools/smb.py

Copilot · 2026-01-04T05:34:21Z

lisa/tools/smb.py

+        # Check if samba services exist
+        service = self.node.tools[Service]
+        return service.check_service_exists(
+            self._smb_service
+        ) and service.check_service_exists(self._nmb_service)


The _check_exists method should verify that the samba service binaries exist on the system, not check if the service exists in systemd/service manager. Looking at similar tools in the codebase (like NFSServer), _check_exists typically checks if the installed packages exist rather than if services are registered. Consider checking for the installed packages or binary commands instead.

Suggested change

# Check if samba services exist

service = self.node.tools[Service]

return service.check_service_exists(

self._smb_service

) and service.check_service_exists(self._nmb_service)

# Verify that the Samba server binaries exist on the system.

# This follows the pattern used by other tools (e.g., NFSServer),

# where _check_exists validates the presence of required commands

# rather than relying on service-manager registration.

result = self.node.execute(

"command -v smbd && command -v nmbd",

sudo=True,

shell=True,

)

return result.exit_code == 0

Copilot · 2026-01-04T05:34:22Z

lisa/tools/smb.py

+    def start(self) -> None:
+        """Start SMB services."""
+        service = self.node.tools[Service]
+        service.restart_service(self._smb_service)
+        service.restart_service(self._nmb_service)
+        # stop firewall to allow SMB traffic
+        self.node.tools[Firewall].stop()
+
+    def stop(self) -> None:
+        """Stop SMB services."""
+        service = self.node.tools[Service]
+        service.stop_service(self._smb_service)
+        service.stop_service(self._nmb_service)
+
+    def is_running(self) -> bool:
+        """Check if SMB services are running."""
+        service = self.node.tools[Service]
+        return service.is_service_running(
+            self._smb_service
+        ) and service.is_service_running(self._nmb_service)
+
+    def remove_share(self, share_path: str) -> None:
+        """Remove a SMB share and its directory."""
+        self.node.tools[Rm].remove_directory(share_path, sudo=True)


The start, stop, is_running, and remove_share methods lack docstrings. Public methods should have docstrings explaining their purpose and behavior.

Copilot · 2026-01-04T05:34:22Z

lisa/tools/smb.py

+        """Unmount SMB share."""
+        self.node.tools[Mount].umount(point=mount_point, disk_name="", erase=False)
+
+    def is_mounted(self, mount_point: str) -> bool:
+        """Check if mount point exists and is mounted."""
+        return self.node.tools[Mount].check_mount_point_exist(mount_point)
+
+    def cleanup_mount_point(self, mount_point: str) -> None:
+        """Remove mount point directory."""


The unmount_share, is_mounted, and cleanup_mount_point methods lack docstrings. Public methods should have docstrings explaining their purpose, parameters, and behavior.

Suggested change

"""Unmount SMB share."""

self.node.tools[Mount].umount(point=mount_point, disk_name="", erase=False)

def is_mounted(self, mount_point: str) -> bool:

"""Check if mount point exists and is mounted."""

return self.node.tools[Mount].check_mount_point_exist(mount_point)

def cleanup_mount_point(self, mount_point: str) -> None:

"""Remove mount point directory."""

"""Unmount an SMB share from the specified mount point.

This method detaches the SMB share from the filesystem but does not

remove the mount point directory or erase any data on the underlying

storage.

:param mount_point: The path where the SMB share is currently mounted.

"""

self.node.tools[Mount].umount(point=mount_point, disk_name="", erase=False)

def is_mounted(self, mount_point: str) -> bool:

"""Check whether the given mount point is currently mounted.

:param mount_point: The filesystem path to check.

:return: ``True`` if the mount point exists and has an active mount,

otherwise ``False``.

"""

return self.node.tools[Mount].check_mount_point_exist(mount_point)

def cleanup_mount_point(self, mount_point: str) -> None:

"""Remove the directory used as the mount point.

This should typically be called after :meth:`unmount_share` to clean up

the now-unused mount point directory.

:param mount_point: The filesystem path of the mount point directory

to remove.

"""

lisa/tools/smb.py

adityagesh · 2026-01-05T03:58:59Z

Where is the code/case that is using these tools?

SRIKKANTH · 2026-01-05T04:06:51Z

Where is the code/case that is using these tools?

#4184

adityagesh · 2026-01-05T04:16:58Z

Where is the code/case that is using these tools?

#4184

next time, I would recommend keeping both the code in the same PR so it's easier to review

SRIKKANTH · 2026-01-05T08:44:05Z

Closing this PR as the tools are merged into PR #4184

SRIKKANTH requested a review from LiliDeng as a code owner January 1, 2026 16:27

kanchansenlaskar reviewed Jan 1, 2026

View reviewed changes

SRIKKANTH force-pushed the smyakam/smb_tool/01_01_2026 branch from 55f69cd to 2c95b32 Compare January 1, 2026 19:35

adityagesh reviewed Jan 2, 2026

View reviewed changes

SRIKKANTH force-pushed the smyakam/smb_tool/01_01_2026 branch 2 times, most recently from 8b5b8c9 to 88855ae Compare January 2, 2026 14:44

SMB server and client tools

9dae422

SMB server and client tools These tools are useful in validating samba file share with Linux OS and CIFS module in kernel.

SRIKKANTH force-pushed the smyakam/smb_tool/01_01_2026 branch from 06c830d to 9dae422 Compare January 2, 2026 14:48

SRIKKANTH mentioned this pull request Jan 2, 2026

New testcase 'verify_smb_linux' #4184

Open

LiliDeng requested review from adityagesh, Copilot and kanchansenlaskar January 4, 2026 05:25

Copilot started reviewing on behalf of LiliDeng January 4, 2026 05:31 View session

Copilot AI reviewed Jan 4, 2026

View reviewed changes

SRIKKANTH closed this Jan 5, 2026

		self.node.execute(f"mkdir -p {share_path}", sudo=True)
		self.node.execute(f"chmod 777 {share_path}", sudo=True)

-        # Check if samba services exist
-        service = self.node.tools[Service]
-        return service.check_service_exists(
-            self._smb_service
-        ) and service.check_service_exists(self._nmb_service)
+        # Verify that the Samba server binaries exist on the system.
+        # This follows the pattern used by other tools (e.g., NFSServer),
+        # where _check_exists validates the presence of required commands
+        # rather than relying on service-manager registration.
+        result = self.node.execute(
+            "command -v smbd && command -v nmbd",
+            sudo=True,
+            shell=True,
+        )
+        return result.exit_code == 0

-        """Unmount SMB share."""
-        self.node.tools[Mount].umount(point=mount_point, disk_name="", erase=False)
-    def is_mounted(self, mount_point: str) -> bool:
-        """Check if mount point exists and is mounted."""
-        return self.node.tools[Mount].check_mount_point_exist(mount_point)
-    def cleanup_mount_point(self, mount_point: str) -> None:
-        """Remove mount point directory."""
+        """Unmount an SMB share from the specified mount point.
+        This method detaches the SMB share from the filesystem but does not
+        remove the mount point directory or erase any data on the underlying
+        storage.
+        :param mount_point: The path where the SMB share is currently mounted.
+        """
+        self.node.tools[Mount].umount(point=mount_point, disk_name="", erase=False)
+    def is_mounted(self, mount_point: str) -> bool:
+        """Check whether the given mount point is currently mounted.
+        :param mount_point: The filesystem path to check.
+        :return: ``True`` if the mount point exists and has an active mount,
+            otherwise ``False``.
+        """
+        return self.node.tools[Mount].check_mount_point_exist(mount_point)
+    def cleanup_mount_point(self, mount_point: str) -> None:
+        """Remove the directory used as the mount point.
+        This should typically be called after :meth:`unmount_share` to clean up
+        the now-unused mount point directory.
+        :param mount_point: The filesystem path of the mount point directory
+            to remove.
+        """

Conversation

SRIKKANTH commented Jan 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kanchansenlaskar Jan 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Jan 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

adityagesh commented Jan 5, 2026

Uh oh!

SRIKKANTH commented Jan 5, 2026

Uh oh!

adityagesh commented Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SRIKKANTH commented Jan 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

SRIKKANTH commented Jan 1, 2026 •

edited

Loading

kanchansenlaskar Jan 1, 2026 •

edited

Loading

adityagesh commented Jan 5, 2026 •

edited

Loading