Closed luis4a0 closed 2 months ago
Attention: Patch coverage is 28.12500%
with 23 lines
in your changes are missing coverage. Please review.
Project coverage is 88.66%. Comparing base (
2f4ac45
) to head (04a9ddb
).
Files | Patch % | Lines |
---|---|---|
...backends/qemu/linux/qemu_platform_detail_linux.cpp | 26.92% | 19 Missing :warning: |
...orm/backends/qemu/qemu_virtual_machine_factory.cpp | 0.00% | 4 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Hey @sharder996! Thanks for the review! I addressed all your comments, plus rebasing on the fix for get
. Now the code should look better, so it's ready for another round of review. Thanks!
Hey @sharder996! I addressed your two points. this is ready for another round of review. Thanks!
@sharder996 I addressed the point I didn't address in the last round, so this is ready again for review. Thanks!
@luis4a0 Got my PC up and dual-booting with Ubuntu so I did some more functional testing. There are a couple of things that I noticed that seem a bit odd:
If I take a snapshot of an instance before and after setting local.foo.bridged=true
, then restore the first snapshot (before adding the bridge) and then restore the second snapshot (after adding the bridge), when I start the instance the bridge does not get initialized. Only after restarting the instance does the bridge come up.
Also, since I recently moved I am on a different network and had to create a new bridge for that network. As such, I have an extra bridge in multipass networks
which was for the old network and does nothing. Deleting the bridge with ip link
will make it go away, but when I restart the Multipass daemon it comes back.
Hey @sharder996, good to hear you have a working Linux again! Thanks for the new review, I'll answer your two points:
extra_interfaces
the bridged interface entry. This will be trivial to do when we implement removing bridges (in the future, not on this PR).
Thanks!
This PR implements the bridging feature on the QEMU backend on Linux.