Fix two aarch64 specific issues #1755
Conversation
The code change passes -machine option explicitly to the command so that it works for aarch64 guests. Signed-off-by: Huan Xiong <[email protected]>
Commit f0eb8a56e3a508d13ff08adfdf14b9045246a862 changes DevContainer's constructor API. So update tests accordingly. Signed-off-by: Huan Xiong <[email protected]>
add_serial() function doesn't work for aarch64 guest because isa-serial device is compiled into qemu-system-aarch64 (it perhaps shouldn't be by default). As the serial device PL011 on aarch64 is not a pluggable device, the change fixes the issue by using "-serial" option for aarch64 guest. Signed-off-by: Huan Xiong <[email protected]>
|
The 'make check' log is at: https://paste.fedoraproject.org/paste/Dlm9T5rETQ-~zOsTrZfUrA/ None of the issues are introduced by my code change. |
|
@luckyh Done. Thanks. |
| @@ -1473,7 +1474,9 @@ def sort_key(dev): | |||
| cmd += "numactl -m %s " % n | |||
|
|
|||
| # Start constructing devices representation | |||
| devices = qcontainer.DevContainer(qemu_binary, self.name, | |||
| # Machine_type parameter is defined in machines.cfg | |||
|
@balamuruhans Thanks for your review. I thought I should capitalize the first letter in the comment, though it may be confusing. BTW, how should I update the code? The contribution guide document on avocado website says that I should create a new one and close this one, is that true? Also @ldoktor has a fix for the same issue in #1684. If his PR is integrated, I'll close this one. |
| @@ -457,7 +457,8 @@ def add_qmp_monitor(devices, monitor_name, filename): | |||
| def add_serial(devices, name, filename): | |||
| if (not devices.has_option("chardev") or | |||
| not any(devices.has_device(dev) | |||
| for dev in ("isa-serial", "sclpconsole", "spapr-vty"))): | |||
| for dev in ("isa-serial", "sclpconsole", "spapr-vty")) or | |||
| 'aarch64' in params.get('vm_arch_name', arch.ARCH)): | |||
There was a problem hiding this comment.
This is addressed in 3d8084a which is older (2 months) with additional fixes related to s390x so it'd be probably useful to use that, what do you think?
There was a problem hiding this comment.
No problem. I'll remove it.
| @@ -825,7 +826,7 @@ def test_qdev_functional(self): | |||
|
|
|||
| def test_qdev_hotplug(self): | |||
| """ Test the hotplug/unplug functionality """ | |||
| qdev = self.create_qdev('vm1', False, True) | |||
| qdev = self.create_qdev('vm1', 'pc', False, True) | |||
There was a problem hiding this comment.
Tests have to be updated together with the actual change, otherwise you'll get failures after the first commit...
There was a problem hiding this comment.
OK, will combine them.
There was a problem hiding this comment.
Hello @rayx, in general I do prefer explicit -machine, but I'd suggest replacing the self.execute_qemu with this hardcoded -machine so we have the same behavior everywhere (compare to here where we only get this for device_help). Basically I'd suggest replacing the self.execute_qemu with simple return results_stdout_52lts(process.run("%s -machine %s %s 2>&1" % (self.__qemu_binary, self.machine_type, options), ...)) and then using it everywhere where necessary. What do you think?
|
@ldoktor It makes sense. A quick search in the code shows that execute_qemu() is called in three places. I'll take a closer look at the scope of the change. One issue is that I have only tried virtio console tests on aarch64 and doesn't have a good baseline to verify the change. But I'll give it a try. Will let you know if I have questions. |
|
Basically with #1684 arm should work well (and others), this would be just an optimization/cleanup. |
|
See #1922 also. |
The PR fixes two issues I hit while running virtio-console tests on aarch64.
With the fix about three quarters of virtio console tests pass on aarch64. The rest ones fail due to separate issues.
For details please look at commit messages.