-
Notifications
You must be signed in to change notification settings - Fork 164
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.
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
Upgraded Xen-tools from 4.15 to 4.19.0 #4133
Conversation
fails with
|
@OhmSpectator Please rename the patches folder from pkg/xen-tools/patches-4.19.0-rc4-v3 to patches-4.19.0-rc4 . It should work now, I just made changed in my master branch. |
888892e
to
e1d536d
Compare
pkg/xen-tools/patches-4.19.0-rc4-v3/17-enable-qemu-vhost-vsock.patch
Outdated
Show resolved
Hide resolved
+ | ||
if (!vfio_pci_is(vdev, PCI_VENDOR_ID_INTEL, PCI_ANY_ID) || | ||
- !vfio_is_vga(vdev) || nr != 4 || | ||
- &vdev->pdev != pci_find_device(pci_device_root_bus(&vdev->pdev), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering what was the original reason of this patch and if we still need it.... @rucoder , do you know if we still need to revert these changes from Xen?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rene As of now, I don't see any issue with my updated revert vfio patch so keeping the updated one.
pkg/xen-tools/patches-4.19.0-rc4-v3/x86_64/05-xen-spoofing.patch
Outdated
Show resolved
Hide resolved
pkg/xen-tools/patches-4.19.0-rc4-v3/11-char-socket-revert.patch
Outdated
Show resolved
Hide resolved
@roja-zededa , just a reminder for the final version, don't forget to let just one |
@rene @OhmSpectator @rucoder Done with the suggested changes, please let me know if I need to change anything else for the approval. |
@roja-zededa there are some unit test failures (due to different syntax in the kvm file??), and yetus/blanks complaints in https://github.com/lf-edge/eve/pull/4133/checks |
@roja-zededa please test it carefully. Especually PCI pass-through part. |
@roja-zededa and I would wait for stable release and not merge RC4 |
The stable version is out, able to port changes, no issues so far. |
01ada0d
to
82f4966
Compare
Resolved the yetus/blanks issue. The unit test failure with hypervisor is due to removal of [realtime] parameter, the kvm_test.go file tests need modifications (ie. remove [realtime]). Still debugging the unit test failures caused due to pci. |
31fdaea
to
c365539
Compare
@OhmSpectator @rene @rucoder Made the necessary changes, please let me know what else I need to do for this pull request to be merged @christoph-zededa Removed the [realtime] parameter from kvm.go as the new Qemu doesn't support it, the unit tests need to be changed in the future for this PR to be passing all the hypervisor test cases. (As of now I put [realtime] back) |
I apologize I did not recognize the question intonation: so what is the question? Do you need to change tests if they are failing? Answer is: yes, please :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unit tests fix is required
Yey, thanks for the fix in the test! Could you please polish the commit messages as well? |
2fc95e6
to
af2a10c
Compare
@roja-zededa reopen the PR, please =) |
@roja-zededa, ah... I see... You create a PR from your local master. In this case, part of our GitHub actions may fail... Let's ignore them for now. But the next time, please, create a PR from a dedicated branch, not from your master. |
# disable golang as it does not play well together with musl (stderr is defined as FILE* const and fails to compile) | ||
RUN ./configure --prefix=/usr --disable-xen --disable-golang --disable-qemu-traditional --disable-docs --enable-9pfs \ | ||
--with-system-ovmf=/usr/lib/xen/boot/ovmf.bin --disable-stubdom | ||
--with-system-ovmf=/usr/lib/xen/boot/ovmf.bin --disable-stubdom \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add the extra spaces back, increases the readability. same for EXTRA_QEMUU_CONFIGURE_ARGS
.
Able to add ninja through apk in xen-tools/Docker and xen/Docker (1) Changed the seabios and Xen version to 1.16.3 and 4.19.0 respectively (2) Added 12-remove-vanillaqemu4.19-cpupinning.patch which removes new qemu_thread_set_affinity implementation (QEMU 8.0.4), also retained Nikolay CPU Pinning PatchNo.15 (3) Warning treated as error flag is available by default so removed 12-disable-Werror-to-build-under-gcc-11.2.patch (4) NetdevTapOptions doesn't have has_br member so changed 10-bridge-helper-support.patch (5) Bydefault vhost-vsock and vhost-scsi is enabled so removing the corresponding enable flags from xen-tools/Dockerfile (6) Removed 11-char-socket-revert.patch as it's unnecessary (7) Removed [realtime] option from kvm.go and replaced it with [overcommit], hypervisor.go unit test need to be changed to reflect [overcommit] (8) Revert__Revert__vfio_pci-quirks_c__Disable_stolen_memory_for_igd_VFIO__.patch looked super messy, so cleaned it (9) Replaced [realtime] with [overcommit] in kvm_test.go for the unit test case to pass (10) Signed-off-by: Roja Eswaran <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4133 +/- ##
=======================================
Coverage 19.69% 19.69%
=======================================
Files 8 8
Lines 2600 2600
=======================================
Hits 512 512
Misses 1985 1985
Partials 103 103 ☔ View full report in Codecov by Sentry. |
WARNING: STALE PR. Please move to #4186
Due to failing Request Code Owners Review / auto_request_review (pull_request_target) moved this PR to #4186
Porting Xen-tools from 4.15 to 4.19.0 (QEMU Version 8.0.4).
Changes
Apart from the above major changes, the index of other existing patches might have been changed.