Skip to content
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

virttest/utils_test: fixes to virtio serial file transfer #1903

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

virttest/utils_test: fixes to virtio serial file transfer #1903

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
7 changes: 3 additions & 4 deletions shared/deps/serial/VirtIoChannel_guest_send_receive.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def __init__(self, device_name):
self.ack_format = "3s"
self.ack_msg = b"ACK"
self.hi_format = "2s"
self.hi_msg = "HI"
self.hi_msg = b"HI"
if self.is_windows:
vport_name = '\\\\.\\Global\\' + device_name
from windows_support import WinBufferedReadFile
Expand Down Expand Up @@ -85,7 +85,7 @@ def shake_hand(self, size=0, action="receive"):
self.send(self.hi_msg)
txt = self.receive(hi_msg_len)
out = struct.unpack(self.hi_format, txt)[0]
if out != "HI":
if out != b"HI":
raise ShakeHandError("Fail to get HI from guest.")
size_s = struct.pack("q", size)
self.send(size_s)
Expand All @@ -101,7 +101,6 @@ def shake_hand(self, size=0, action="receive"):
raise ShakeHandError("Fail to get HI from guest.")
self.send(txt)
size = self.receive(8)
print("xxxx size = %s" % size)
if size:
size = struct.unpack("q", size)[0]
txt = struct.pack(self.ack_format, self.ack_msg)
Expand Down Expand Up @@ -180,7 +179,7 @@ def receive(device, filename, p_size=1024):
txt = vio.receive(p_size)
md5_value.update(txt)
file_no.write(txt)
recv_size += p_size
recv_size += len(txt)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice catch :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Btw you should split the patches. Good hint to splitting patches is when you need to use "also" in either description or comment or, as in your case, you need to itemize the changes. They look fairly simple to split (let me suggest my favorited "git cola" ;-) for that)

finally:
file_no.close()
if vio:
Expand Down
6 changes: 3 additions & 3 deletions shared/deps/serial/serial_host_send_receive.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,19 +98,19 @@ def shake_hand(connect, size=0, action="receive"):
connect.send(hi_str)
txt = connect.recv(hi_str_len)
hi_str = struct.unpack("2s", txt)[0]
if hi_str != "HI":
if hi_str != b"HI":
raise ShakeHandError("Fail to get HI from guest.")
size_str = struct.pack("q", size)
connect.send(size_str)
txt = connect.recv(3)
ack_str = struct.unpack("3s", txt)[0]
if ack_str != "ACK":
if ack_str != b"ACK":
raise ShakeHandError("Guest did not ACK the file size message.")
return size
elif action == "receive":
txt = connect.recv(hi_str_len)
hi_str = struct.unpack("2s", txt)[0]
if hi_str != "HI":
if hi_str != b"HI":
raise ShakeHandError("Fail to get HI from guest.")
connect.send(hi_str)
size = connect.recv(8)
Expand Down
23 changes: 19 additions & 4 deletions virttest/utils_test/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,12 @@ def find_python(session, try_binaries=('python3', 'python2', 'python')):
if session.cmd_status("which %s" % python) == 0:
return python

def find_host_python(try_binaries=('python3', 'python2', 'python')):
for python in try_binaries:
out = process.run("which %s" % python, shell=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

You need ignore_status=True here, otherwise it will raise error on the host without python3.

Copy link
Contributor

Choose a reason for hiding this comment

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

or simply use process.system

if out.exit_status == 0:
return python

Copy link
Contributor

@yanan-fu yanan-fu Jan 10, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Even use this method, we can improve find_python to make it work with both host and guest, according to the value of session .

if session:
    s = session.cmd_status(***)
else:
    s = process.run(***).exist_status

Copy link
Contributor

Choose a reason for hiding this comment

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

How about simply using session.cmd_status("%s --version" % python), that should work for linux as well as windows 😉

Copy link
Contributor

Choose a reason for hiding this comment

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

Great, it is available for both linux and windows :)


@error_context.context_aware
def get_image_version(qemu_image):
Expand Down Expand Up @@ -742,6 +748,7 @@ def transfer_data(session, host_cmd, guest_cmd, n_time, timeout,

host_data_file = os.path.join(dir_name,
"tmp-%s" % utils_misc.generate_random_string(8))
session.cmd("mkdir -p %s" % tmp_dir)
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, this resolves the failure for file_sender = guest

guest_data_file = os.path.join(tmp_dir,
"tmp-%s" % utils_misc.generate_random_string(8))

Expand Down Expand Up @@ -775,14 +782,22 @@ def transfer_data(session, host_cmd, guest_cmd, n_time, timeout,
host_script = params.get("host_script", "serial_host_send_receive.py")
host_script = os.path.join(data_dir.get_root_dir(), "shared", "deps",
"serial", host_script)
host_cmd = "python %s -s %s -f %s -a %s" % (host_script, host_device,
host_data_file, action)

host_python = find_host_python()
Copy link
Contributor

Choose a reason for hiding this comment

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

An alternative approach is using an inline shell statement $(command -v python3 python | head -1), but this assumes we are running in bash.

if host_python is None:
logging.error("Python not found on host.")
host_cmd = "%s %s -s %s -f %s -a %s" % (host_python, host_script,
host_device, host_data_file, action)
guest_script = params.get("guest_script",
"VirtIoChannel_guest_send_receive.py")
guest_script = os.path.join(guest_path, guest_script)

guest_cmd = "python %s -d %s -f %s -a %s" % (guest_script, port_name,
guest_data_file, guest_action)
guest_python = find_python(session)
Copy link
Contributor

Choose a reason for hiding this comment

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

We also have windows guests, so that would be a problem..

if guest_python is None:
logging.error("Python not found on guest.")
guest_cmd = "%s %s -d %s -f %s -a %s" % (guest_python, guest_script,
port_name, guest_data_file,
guest_action)
n_time = int(params.get("repeat_times", 1))
txt += " for %s times" % n_time
try:
Expand Down