Skip to content

add case for migration with ethernet interface #6625

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
+280 −6
Draft

add case for migration with ethernet interface #6625

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions libvirt/tests/cfg/virtual_network/migrate/migrate_with_ethernet_interface.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
- virtual_network.migrate.migrate_with_ethernet_interface:
type = migrate_with_ethernet_interface
start_vm = "no"
take_regular_screendumps = no
storage_type = 'nfs'
setup_local_nfs = 'yes'
disk_type = "file"
disk_source_protocol = "netfs"
nfs_mount_dir = "/var/lib/libvirt/migrate"
nfs_mount_options = "soft,timeo=50,retrans=3"
virsh_migrate_dest_state = running
virsh_migrate_options = "--live --p2p --verbose"
outside_ip = "www.google.com"
migration_setup = "yes"
iface_model = "virtio"
ssh_remote_auth = True
unprivileged_user = "${test_user}"
mnt_path_name = ${nfs_mount_dir}
nfs_server_ip = "${migrate_source_host}"
migrate_desturi_port = "22"
migrate_desturi_type = "ssh"
virsh_migrate_desturi = "qemu+ssh://%s@${migrate_dest_host}/session"
vm_ping_outside = "pass"
check_network_accessibility_after_mig = "yes"

# Ethernet interface specific settings
bridge_name = "br0"
tap_name = "mytap0"
tap_mtu = "9000"
# Ethernet interface configuration with managed=no
iface_attrs = {'type_name': 'ethernet', 'target': {'dev': '${tap_name}', 'managed': 'no'}, 'model': '${iface_model}', 'mtu': {'size': '9000'}}
variants:
- precopy_migration:
migration_type = "precopy"
virsh_migrate_extra = ""
migrate_vm_back = "yes"
- postcopy_migration:
migration_type = "postcopy"
virsh_migrate_extra = "--postcopy --timeout 10 --timeout-postcopy --bandwidth 1"
migrate_vm_back = "yes"
- cancel_migration:
migration_type = "cancel"
cancel_migration = "yes"
status_error = "yes"
err_msg = "operation aborted: migration out job: canceled by client|operation aborted: job 'migration out' canceled by client"
virsh_migrate_extra = "--bandwidth 10"
action_during_mig = '[{"func": "virsh.domjobabort", "after_event": "iteration: '1'", "func_param": "'%s' % params.get('migrate_main_vm')"}]'

variants:
- unprivileged_user:

204 changes: 204 additions & 0 deletions libvirt/tests/src/virtual_network/migrate/migrate_with_ethernet_interface.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,204 @@
# ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
#
# Copyright Redhat
#
# SPDX-License-Identifier: GPL-2.0
# Author: Nannan Li<[email protected]>
#
# ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
import time

import aexpect.remote
from avocado.utils import process

from virttest import virsh
from virttest import utils_net
from virttest.libvirt_xml import vm_xml
from virttest.utils_libvirt import libvirt_vmxml
from virttest.utils_libvirt import libvirt_unprivileged

from provider.migration import base_steps
from provider.virtual_network import network_base


def run(test, params, env):
"""
Test migration with ethernet type interface managed=no

This case tests migration of vm with ethernet type interface to and back,
checking that network functions work well after migration.

Test matrix:
- precopy migration
- postcopy migration
- cancel migration

Setup:
1. Prepare migration environment
2. Prepare pre-created tap device and connect it to linux bridge on both hosts
3. Start VM with ethernet type interface pointing to the tap device

:param test: test object
:param params: Dictionary with the test parameters
:param env: Dictionary with test environment.
"""
def create_unpr_user_and_vm(user_name, user_passwd, main_vm_name, remote_session):
"""
Create unprivileged user and setup VM for that user

:param user_name: unprivileged user name
:param user_passwd: unprivileged user password
:param main_vm_name: main vm name in env
:param remote_session: remote machine session
:return: VM object for unprivileged user
"""
test.log.info(f"Creating unprivileged user for local and remote: {user_name}")
try:
add_user, add_pwd = (f'useradd -m {user_name}',
f'echo "{user_name}:{user_passwd}" | chpasswd')
process.run(add_user, shell=True)
process.run(add_pwd, shell=True)
if remote_session:
remote_session.cmd(add_user)
remote_session.cmd(add_pwd)

except Exception as e:
test.log.warn(f"User {user_name} may already exist: {e}")

# Get original VM XML from root user
root_vmxml = vm_xml.VMXML.new_from_inactive_dumpxml(main_vm_name)

# Prepare VM XML for unprivileged user
test.log.info(f"Preparing VM XML for unprivileged user: {user_name}")
unpr_vmxml = network_base.prepare_vmxml_for_unprivileged_user(user_name, root_vmxml)

# Remove existing interfaces and add ethernet interface
unpr_vmxml.del_device('interface', by_tag=True)
libvirt_vmxml.modify_vm_device(unpr_vmxml, 'interface', iface_attrs)

# Remove DAC security label for unprivileged user
unpr_vmxml.del_seclabel(by_attr=[('model', 'dac')])

# Define VM for unprivileged user
test.log.info(f"Defining VM for unprivileged user: {user_name}")
network_base.define_vm_for_unprivileged_user(user_name, unpr_vmxml)

# Get unprivileged VM object
unpr_vm_args = {
'username': params.get('username'),
'password': params.get('password'),
}
unpr_vm = libvirt_unprivileged.get_unprivileged_vm(
unpr_vmxml.vm_name, user_name, user_passwd, **unpr_vm_args)

test.log.info(f"Successfully created unprivileged VM: {unpr_vmxml.vm_name}")
return unpr_vm

def setup_test():
"""
Setup test environment for migration with ethernet interface
"""
test.log.info("Setting up test environment")
remote_host_iface = utils_net.get_default_gateway(
iface_name=True, session=remote_session, force_dhcp=True, json=True)
params.update({"remote_host_iface": remote_host_iface})

utils_net.create_linux_bridge_tmux(bridge_name, host_iface)
network_base.create_tap(tap_name, bridge_name, unpr_user, tap_mtu=tap_mtu)

utils_net.create_linux_bridge_tmux(bridge_name, remote_host_iface, session=remote_session)
network_base.create_tap(tap_name, bridge_name, unpr_user,
session=remote_session, tap_mtu=tap_mtu)

Comment on lines +109 to +112
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Ensure unprivileged user exists on the destination host before remote TAP ops.

create_tap on dest uses “user {unpr_user}”; if the user doesn’t exist remotely, tap creation fails.

If missing, add a guarded creation step before create_tap:

remote_session.cmd(f'getent passwd {unpr_user} || (useradd -m {unpr_user} && echo "{unpr_user}:{unpr_passwd}" | chpasswd)', timeout=60)

🏁 Script executed:

#!/bin/bash
# Verify/create remote unprivileged user prior to remote TAP ops
remote_ip="${server_ip}"
user="${unpr_user}"
pass="${unpr_passwd}"

# Check presence
ssh -o StrictHostKeyChecking=no "${server_user}@${remote_ip}" "getent passwd ${user} >/dev/null || echo MISSING"

Length of output: 41

I need to examine the codebase to verify whether the concern about the unprivileged user is valid.


---

🏁 Script executed:

```shell
#!/bin/bash
# Get full context around lines 102-105 in the test file
cat libvirt/tests/src/virtual_network/migrate/migrate_with_ethernet_interface.py | head -130 | tail -40

Length of output: 1853

🏁 Script executed:

#!/bin/bash
# Find where unpr_user is defined/initialized
rg -n "unpr_user" libvirt/tests/src/virtual_network/migrate/migrate_with_ethernet_interface.py -B3 -A1

Length of output: 1434

🏁 Script executed:

#!/bin/bash
# Find the create_tap function definition
rg -n "def create_tap" --type=py -B2 -A8

Length of output: 849

🏁 Script executed:

#!/bin/bash
# Search for user creation patterns in migration/network test files
rg -n "useradd|adduser|create.*user" --type=py libvirt/tests/src/virtual_network/migrate/ -i

Length of output: 1041

I need to examine the create_unpr_user_and_vm function and check whether it creates the user on both source and destination hosts.


---

🏁 Script executed:

```shell
#!/bin/bash
# Get full create_unpr_user_and_vm function body
sed -n '43,85p' libvirt/tests/src/virtual_network/migrate/migrate_with_ethernet_interface.py

Length of output: 1921

🏁 Script executed:

#!/bin/bash
# Check for any remote user creation or setup around the remote operations
sed -n '170,110p' libvirt/tests/src/virtual_network/migrate/migrate_with_ethernet_interface.py

Length of output: 0

🏁 Script executed:

#!/bin/bash
# Search for where remote_session is set up and if user creation happens there
rg -n "remote_session" libvirt/tests/src/virtual_network/migrate/migrate_with_ethernet_interface.py -B2 -A2

Length of output: 1427

🏁 Script executed:

#!/bin/bash
# Look at the full setup_test function to see what happens before remote create_tap
sed -n '80,110p' libvirt/tests/src/virtual_network/migrate/migrate_with_ethernet_interface.py

Length of output: 1308

I need to verify what the create_tap function does with the user parameter on remote hosts.


---

🏁 Script executed:

```shell
#!/bin/bash
# Get full create_tap implementation
sed -n '152,185p' provider/virtual_network/network_base.py

Length of output: 1115

🏁 Script executed:

#!/bin/bash
# Search for how the user parameter is used in create_tap
rg -n "def create_tap" -A30 provider/virtual_network/network_base.py

Length of output: 1142

Create unprivileged user on destination host before remote TAP creation.

The create_tap function executes ip tuntap add mode tap user {user} which requires the user to exist on the target system. Currently, the unprivileged user is created locally (line 177 in create_unpr_user_and_vm), but the remote create_tap call at lines 103-104 will fail because the user doesn't exist on the destination host.

Add user creation on the remote host before line 103:

remote_session.cmd(f'getent passwd {unpr_user} || (useradd -m {unpr_user} && echo "{unpr_user}:{unpr_passwd}" | chpasswd)', timeout=60)
🤖 Prompt for AI Agents 
libvirt/tests/src/virtual_network/migrate/migrate_with_ethernet_interface.py
around lines 102-105: the remote create_tap call expects the unprivileged user
to exist on the destination host but the user is only created locally; before
calling create_tap on the remote session, run a remote_session.cmd that ensures
the unprivileged user exists on the remote host (use getent to check and, if
missing, create the user with a home and set its password), then proceed to call
network_base.create_tap on the remote session.

def run_test():
"""
Run the main test: migration and verification
"""
test.log.info("TEST_STEP: Migrating VM to target host")
migration_obj.setup_connection()
virsh_ins.start(unpr_vm_obj.name, debug=True, ignore_status=False)
# test.log.debug("------------------------------------------")
# virsh_ins.dom_list(" --all", debug=True)
# time.sleep(420)

migration_obj.run_migration()

if not cancel_migration:
test.log.info("TEST_STEP: Checking VM network connectivity on target host")
if unpr_vm_obj.serial_console is not None:
unpr_vm_obj.cleanup_serial_console()
unpr_vm_obj.create_serial_console()
vm_session_after_mig = unpr_vm_obj.wait_for_serial_login(timeout=240)
vm_session_after_mig.cmd("dhclient -r; dhclient", timeout=120)

test.log.info("TEST_STEP: Testing guest ping to outside")
ips = {'outside_ip': outside_ip}
network_base.ping_check(params, ips, vm_session_after_mig)

if migrate_vm_back:
test.log.info("TEST_STEP: Migrating VM back to source host")
migration_obj.run_migration_back()


def teardown_test():
"""
Cleanup test environment
"""
test.log.info("Cleaning up test environment")
network_base.delete_tap(tap_name)
network_base.delete_tap(tap_name, session=remote_session)

utils_net.delete_linux_bridge_tmux(bridge_name, iface_name=host_iface)
utils_net.delete_linux_bridge_tmux(
bridge_name, iface_name=params.get("remote_host_iface"),
session=remote_session)

bk_xml.sync()
virsh.destroy(unpr_vm_obj.name, unprivileged_user=unpr_user, debug=True)
virsh.undefine(unpr_vm_obj.name, options='--nvram',
unprivileged_user=unpr_user, debug=True)
migration_obj.cleanup_connection()
if remote_session:
remote_session.close()

# Parse test parameters
server_ip = params["server_ip"] = params.get("migrate_dest_host")
server_user = params.get("server_user")
server_pwd = params.get("server_pwd") or params.get("migrate_dest_pwd")
outside_ip = params.get("outside_ip")
bridge_name = params.get("bridge_name", "br0")
tap_name = params.get("tap_name", "mytap0")
tap_mtu = params.get("tap_mtu")
cancel_migration = params.get_boolean("cancel_migration")
migrate_vm_back = params.get_boolean("migrate_vm_back", True)
iface_attrs = eval(params.get("iface_attrs", "{}"))
host_iface = params.get('host_iface')
host_iface = host_iface if host_iface else utils_net.get_default_gateway(
iface_name=True, force_dhcp=True, json=True)

vm_name = params.get('main_vm')
unpr_user = params.get('test_user', 'test')
unpr_passwd = params.get('test_passwd', 'test')
dest_uri = params.get("virsh_migrate_desturi") % unpr_user
params.update({"virsh_migrate_desturi": dest_uri})
connect_uri = f'qemu+ssh://{unpr_user}@localhost/session'

vmxml = vm_xml.VMXML.new_from_inactive_dumpxml(vm_name)
bk_xml = vmxml.copy()

remote_session = aexpect.remote.remote_login(
"ssh", server_ip, "22", server_user, server_pwd, r'[$#%]')

# virsh_ins = virsh.Virsh(uri=connect_uri)
# params["connect_uri"] = connect_uri
# params.update({"customized_uri": connect_uri})
# unpr_vm_obj = create_unpr_user_and_vm(unpr_user, unpr_passwd, vm_name, remote_session)
# unpr_vm_obj.connect_uri = connect_uri
# migration_obj = base_steps.MigrationBase(test, unpr_vm_obj, params)

try:
setup_test()
run_test()
finally:
teardown_test()

31 changes: 25 additions & 6 deletions provider/virtual_network/network_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,20 +149,29 @@ def _ping():
raise exceptions.TestFail('.'.join(failures))


def create_tap(tap_name, bridge_name, user, flag=''):
def create_tap(tap_name, bridge_name, user, flag='', session=None, tap_mtu=''):
"""
Create tap device

:param tap_name: name of tap device
:param bridge_name: bridge to connect to
:param user: user with access
:param flag: additional flags for tap creation
:param session: session object for remote execution, None for local
:param tap_mtu: tap mtu setting value
"""
# Create tap device with ip command
tap_cmd = f'ip tuntap add mode tap user {user} group {user} name ' \
f'{tap_name} {flag};ip link set {tap_name} up;' \
f'ip link set {tap_name} master {bridge_name}'
# Execute command as root
process.run(tap_cmd, shell=True, verbose=True)
if session:
session.cmd(tap_cmd, timeout=60)
else:
process.run(tap_cmd, shell=True, verbose=True)

if tap_mtu:
set_tap_mtu(tap_name, tap_mtu, session)

Comment on lines +152 to 175
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Do not chain with ';' — it masks failures; add timeouts and sanitize inputs.

Semicolon-chained tap_cmd can succeed even if earlier subcommands fail. Also, shell=True and unquoted vars risk injection. Use '&&', quote inputs, and add a local timeout for parity with remote.

Apply:

@@
-    tap_cmd = f'ip tuntap add mode tap user {user} group {user} name ' \
-              f'{tap_name} {flag};ip link set {tap_name} up;' \
-              f'ip link set {tap_name} master {bridge_name}'
+    tap_cmd = ' && '.join([
+        f"ip tuntap add mode tap user {user} group {user} name {tap_name} {flag}".strip(),
+        f"ip link set {tap_name} up",
+        f"ip link set {tap_name} master {bridge_name}",
+    ])
@@
-    if session:
-        session.cmd(tap_cmd, timeout=60)
-    else:
-        process.run(tap_cmd, shell=True, verbose=True)
+    if session:
+        session.cmd(tap_cmd, timeout=60)
+    else:
+        process.run(tap_cmd, shell=True, verbose=True, timeout=60)

Optionally, restrict allowed characters in tap_name/bridge_name/flag or use shlex.quote for stronger hardening. Based on static analysis hints.

🧰 Tools
🪛 Ruff (0.14.1)

171-171: Function call with shell=True parameter identified, security issue

(S604)


def create_macvtap(macvtap_name, iface, user):
Expand Down Expand Up @@ -193,23 +202,33 @@ def create_macvtap(macvtap_name, iface, user):
return mac_addr


def set_tap_mtu(tap, mtu_size):
def set_tap_mtu(tap, mtu_size, session=None):
"""
Set mtu for tap/macvtap device

:param tap: tap/macvtap device name
:param mtu_size: mtu size to set
:param session: session object for remote execution, None for local
"""
process.run(f'ip link set dev {tap} mtu {mtu_size}')
mtu_cmd = f'ip link set dev {tap} mtu {mtu_size}'
if session:
session.cmd(mtu_cmd, timeout=30)
else:
process.run(mtu_cmd)

Comment on lines +205 to 218
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Local process.run lacks shell=True/timeout; align with remote path.

Without shell=True and a timeout, local MTU set can behave inconsistently and hang.

-    mtu_cmd = f'ip link set dev {tap} mtu {mtu_size}'
-    if session:
-        session.cmd(mtu_cmd, timeout=30)
-    else:
-        process.run(mtu_cmd)
+    mtu_cmd = f'ip link set dev {tap} mtu {mtu_size}'
+    if session:
+        session.cmd(mtu_cmd, timeout=30)
+    else:
+        process.run(mtu_cmd, shell=True, timeout=30, verbose=True)
🤖 Prompt for AI Agents 
In provider/virtual_network/network_base.py around lines 205 to 218, the local
branch calling process.run(mtu_cmd) does not pass shell=True and a timeout like
the remote session path, which can cause hangs and inconsistent behavior; update
the local call to use process.run(mtu_cmd, shell=True, timeout=30) (and
propagate or handle exceptions similarly to the session path) so both remote and
local code paths behave consistently.


def delete_tap(tap_name):
def delete_tap(tap_name, session=None):
"""
Delete tap/macvtap device with given name

:param tap_name: name of tap/macvtap device
:param session: session object for remote execution, None for local
"""
process.run(f'ip l del {tap_name}', shell=True, ignore_status=True)
del_cmd = f'ip l del {tap_name}'
if session:
session.cmd(del_cmd)
else:
process.run(del_cmd, shell=True, ignore_status=True)


def prepare_vmxml_for_unprivileged_user(up_user, root_vmxml):
Expand Down
Loading