Closed luis4a0 closed 5 months ago
Attention: Patch coverage is 97.72727%
with 1 lines
in your changes are missing coverage. Please review.
Project coverage is 88.69%. Comparing base (
8c7a226
) to head (dcc4232
).:exclamation: Current head dcc4232 differs from pull request most recent head 1dc0a54. Consider uploading reports for the commit 1dc0a54 to get more accurate results
Files | Patch % | Lines |
---|---|---|
src/platform/backends/qemu/qemu_snapshot.cpp | 80.00% | 1 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Hey @luis4a0, not sure if it affects this PR, but here is one more thing to consider: how do you plan to handle existing snapshots? For instance, suppose I do this today:
multipass launch --network eth0 -n foo
multipass launch -n bar
multipass snapshot -n s1 foo
multipass snapshot -n s1 bar
After we release bridge addition, I then do
multipass restore foo.s1
multipass restore bar.s1
How will you ensure that foo
will have eth0
, but not bar
?
@ricab, right. This needs to be considered, but in another PR. I'd say we can merge this one and start testing and addressing this other issue. Thanks!
Hey @ricab, thanks for the review! The case you mention is exactly the one I am considering for the next PR. To solve that, two things should be done:
run_at_boot
,configure_new_interfaces
.
The second point is still obscure to me, it should be ran at start but it is not. The proof it's not ran is that the backend tries to launch QEMU with an empty MAC address, what means that the interface was not configured.
This PR adds the
extra_interfaces
data structure to the snapshots infrastructure. This is needed to snapshot instance with interfaces added after creation.