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.

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

Rename ring buffer to com.canonical.lxd #12548

Merged
merged 5 commits into from
Nov 27, 2023

Conversation

MusicDin
Copy link
Member

This PR updates the name of ring buffer from org.linuxcontainers.lxd to com.canonical.lxd.

With this change in place, restart of existing VMs will be required. Otherwise, (old) lxd-agent will write data to the org.linuxcontainers.lxd qemu serial device that is not connected to the new ring buffer from which lxd server will try to read. After VM is restarted, new lxd-agent will be copied from the shared host filesystem to the VM.

Fixes #12149

stgraber and others added 3 commits November 23, 2023 11:28
Signed-off-by: Stéphane Graber <stgraber@stgraber.org>
Signed-off-by: Din Music <din.music@canonical.com>
Signed-off-by: Stéphane Graber <stgraber@stgraber.org>
Signed-off-by: Din Music <din.music@canonical.com>
Signed-off-by: Din Music <din.music@canonical.com>
@MusicDin MusicDin marked this pull request as ready for review November 23, 2023 14:16
@mihalicyn
Copy link
Member

Looks good to me!

And also thanks for adding a great explanatory comments.

mihalicyn
mihalicyn previously approved these changes Nov 23, 2023
@mihalicyn
Copy link
Member

We still need to test this very-very carefully to be 100% that nothing was missed.

simondeziel
simondeziel previously approved these changes Nov 23, 2023
Copy link
Member

@simondeziel simondeziel left a comment

Choose a reason for hiding this comment

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

LGTM with some tiny nitpicks.

lxd/instance/drivers/driver_qemu.go Outdated Show resolved Hide resolved
lxd/instance/drivers/driver_qemu_templates.go Outdated Show resolved Hide resolved
@tomponline
Copy link
Member

@simondeziel @MusicDin would we be able to add some tests to lxd-ci that would check that the agent is working on an older image (e.g. jammy assuming the SRU won't go through) is working with the newer LXD?

And also that the agent is working in an image for a newer version of Ubuntu (once our changes land in lxd-agent-loader package) on an older (5.0/stable) LXD?

tomponline
tomponline previously approved these changes Nov 23, 2023
Copy link
Member

@tomponline tomponline left a comment

Choose a reason for hiding this comment

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

LGTM thanks!

What was the outcome of the test to statefully restore a VM running an older lxd-agent that talks to the older serial device?

…unit, and serial devices

Signed-off-by: Din Music <din.music@canonical.com>
@MusicDin
Copy link
Member Author

MusicDin commented Nov 23, 2023

Steps that I did (Also see the spec where I tried to document this https://discourse.ubuntu.com/t/rename-lxd-ring-buffer-to-com-canonical-lxd/40499):

$ lxc launch ubuntu:22.04 v1 --vm
$ lxc snapshot v1 mysnap --stateful

# >> Upgrade LXD

# This will work, because VM state is not recovered.
$ lxc restore v1 mysnap
$ lxc start v1

# This will NOT work, because old LXD agent is restored as well and it is not able
# to communicate with the LXD server through a common ring buffer.
$ lxc restore v1 mysnap --stateful
Error: Failed restoring state from "/var/snap/lxd/common/lxd/virtual-machines/v1/state": Monitor is disconnected

@tomponline
Copy link
Member

Steps that I did (Also see the spec where I tried to document this https://discourse.ubuntu.com/t/rename-lxd-ring-buffer-to-com-canonical-lxd/40499):

$ lxc launch ubuntu:22.04 v1 --vm
$ lxc snapshot v1 mysnap --stateful

# >> Upgrade LXD

# This will work, because VM state is not recovered.
$ lxc restore v1 mysnap
$ lxc start v1

# This will NOT work, because old LXD agent is restored as well and it is not able
# to communicate with the LXD server through a common ring buffer.
$ lxc restore v1 mysnap --stateful
Error: Failed restoring state from "/var/snap/lxd/common/lxd/virtual-machines/v1/state": Monitor is disconnected

Hrm, are you sure the failure to restore the snapshot was due to the ring buffer? Did you get an error from the qemu logs?

@MusicDin
Copy link
Member Author

Good point, need to verify that

@MusicDin
Copy link
Member Author

Hrm, are you sure the failure to restore the snapshot was due to the ring buffer? Did you get an error from the qemu logs?

You were right Tom, my assumption was wrong. According to @mihalicyn, the issue is related to non-existing state of the new serial device.

Here is the instance log:

qemu-system-x86_64: Failed to load virtio-console:virtio
qemu-system-x86_64: error while loading state for instance 0x0 of device '0000:00:01.0:00.5/virtio-console'
qemu-system-x86_64: load of migration failed: Invalid argument

Signed-off-by: Din Music <din.music@canonical.com>
@tomponline
Copy link
Member

OK thats fine then as we wont need to consider restoring an actively running older agent.

@tomponline tomponline merged commit 2d0d062 into canonical:main Nov 27, 2023
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rename ring buffer to com.canonical.lxd
5 participants