Skip to content

Commit

Permalink
VM: Fix live migration (#14696)
Browse files Browse the repository at this point in the history
PR #14539 broke live migration in
commit
d734721

This is because when live migrating a VM, the VM's `Start()` function is
called on the target side (so that live migration can occur) before the
Location of the instance is updated in the DB. This in turn calls
`statusCode` during the start process.

The change to the `statusCode` function meant that this returned an
error status because the instance was in theory non-local, and that
prevented the qemu process from being started.

This PR reverts the `statusCode` error change and moves that logic into
the Render function.

Fixes #14679
  • Loading branch information
tomponline authored Dec 19, 2024
2 parents 7893937 + b056e08 commit 2b27096
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 58 deletions.
59 changes: 30 additions & 29 deletions lxd/instance/drivers/driver_lxc.go
Original file line number Diff line number Diff line change
Expand Up @@ -3254,21 +3254,23 @@ func (d *lxc) Render(options ...func(response any) error) (state any, etag any,
etag := []any{d.expiryDate}

snapState := api.InstanceSnapshot{
CreatedAt: d.creationDate,
Name: strings.SplitN(d.name, "/", 2)[1],
Architecture: architectureName,
Profiles: profileNames,
Config: d.localConfig,
ExpandedConfig: d.expandedConfig,
Devices: d.localDevices.CloneNative(),
ExpandedDevices: d.expandedDevices.CloneNative(),
CreatedAt: d.creationDate,
LastUsedAt: d.lastUsedDate,
Name: strings.SplitN(d.name, "/", 2)[1],
ExpiresAt: d.expiryDate,
Ephemeral: d.ephemeral,
Stateful: d.stateful,
Size: -1, // Default to uninitialised/error state (0 means no CoW usage).
}

snapState.Architecture = architectureName
snapState.Config = d.localConfig
snapState.Devices = d.localDevices.CloneNative()
snapState.Ephemeral = d.ephemeral
snapState.Profiles = profileNames
snapState.ExpiresAt = d.expiryDate
// Default to uninitialised/error state (0 means no CoW usage).
// The size can then be populated optionally via the options argument.
Size: -1,
}

for _, option := range options {
err := option(&snapState)
Expand All @@ -3283,27 +3285,31 @@ func (d *lxc) Render(options ...func(response any) error) (state any, etag any,
// Prepare the ETag
etag = []any{d.architecture, d.localConfig, d.localDevices, d.ephemeral, d.profiles}

statusCode := d.statusCode()
instState := api.Instance{
Name: d.name,
Description: d.description,
Architecture: architectureName,
Profiles: profileNames,
Config: d.localConfig,
ExpandedConfig: d.expandedConfig,
Devices: d.LocalDevices().CloneNative(),
ExpandedDevices: d.expandedDevices.CloneNative(),
Name: d.name,
Status: statusCode.String(),
StatusCode: statusCode,
CreatedAt: d.creationDate,
LastUsedAt: d.lastUsedDate,
Ephemeral: d.ephemeral,
Stateful: d.stateful,
Project: d.project.Name,
Location: d.node,
Type: d.Type().String(),
StatusCode: api.Error, // Default to error status for remote instances that are unreachable.
}

// If instance is local then request status.
if d.state.ServerName == d.Location() {
instState.StatusCode = d.statusCode()
}

instState.Description = d.description
instState.Architecture = architectureName
instState.Config = d.localConfig
instState.CreatedAt = d.creationDate
instState.Devices = d.localDevices.CloneNative()
instState.Ephemeral = d.ephemeral
instState.LastUsedAt = d.lastUsedDate
instState.Profiles = profileNames
instState.Stateful = d.stateful
instState.Project = d.project.Name
instState.Status = instState.StatusCode.String()

for _, option := range options {
err := option(&instState)
Expand Down Expand Up @@ -8151,11 +8157,6 @@ func (d *lxc) NextIdmap() (*idmap.IdmapSet, error) {

// statusCode returns instance status code.
func (d *lxc) statusCode() api.StatusCode {
// If instance is running on a remote cluster member, we cannot determine instance state.
if d.state.ServerName != d.Location() {
return api.Error
}

// Shortcut to avoid spamming liblxc during ongoing operations.
operationStatus := d.operationStatusCode()
if operationStatus != nil {
Expand Down
59 changes: 30 additions & 29 deletions lxd/instance/drivers/driver_qemu.go
Original file line number Diff line number Diff line change
Expand Up @@ -7777,21 +7777,23 @@ func (d *qemu) Render(options ...func(response any) error) (state any, etag any,
etag := []any{d.expiryDate}

snapState := api.InstanceSnapshot{
CreatedAt: d.creationDate,
Name: strings.SplitN(d.name, "/", 2)[1],
Architecture: d.architectureName,
Profiles: profileNames,
Config: d.localConfig,
ExpandedConfig: d.expandedConfig,
Devices: d.localDevices.CloneNative(),
ExpandedDevices: d.expandedDevices.CloneNative(),
CreatedAt: d.creationDate,
LastUsedAt: d.lastUsedDate,
Name: strings.SplitN(d.name, "/", 2)[1],
ExpiresAt: d.expiryDate,
Ephemeral: d.ephemeral,
Stateful: d.stateful,
Size: -1, // Default to uninitialised/error state (0 means no CoW usage).
}

snapState.Architecture = d.architectureName
snapState.Config = d.localConfig
snapState.Devices = d.localDevices.CloneNative()
snapState.Ephemeral = d.ephemeral
snapState.Profiles = profileNames
snapState.ExpiresAt = d.expiryDate
// Default to uninitialised/error state (0 means no CoW usage).
// The size can then be populated optionally via the options argument.
Size: -1,
}

for _, option := range options {
err := option(&snapState)
Expand All @@ -7805,28 +7807,32 @@ func (d *qemu) Render(options ...func(response any) error) (state any, etag any,

// Prepare the ETag
etag = []any{d.architecture, d.localConfig, d.localDevices, d.ephemeral, d.profiles}
statusCode := d.statusCode()

instState := api.Instance{
Name: d.name,
Description: d.description,
Architecture: d.architectureName,
Profiles: profileNames,
Config: d.localConfig,
ExpandedConfig: d.expandedConfig,
Devices: d.localDevices.CloneNative(),
ExpandedDevices: d.expandedDevices.CloneNative(),
Name: d.name,
Status: statusCode.String(),
StatusCode: statusCode,
CreatedAt: d.creationDate,
LastUsedAt: d.lastUsedDate,
Ephemeral: d.ephemeral,
Stateful: d.stateful,
Project: d.project.Name,
Location: d.node,
Type: d.Type().String(),
StatusCode: api.Error, // Default to error status for remote instances that are unreachable.
}

// If instance is local then request status.
if d.state.ServerName == d.Location() {
instState.StatusCode = d.statusCode()
}

instState.Description = d.description
instState.Architecture = d.architectureName
instState.Config = d.localConfig
instState.CreatedAt = d.creationDate
instState.Devices = d.localDevices.CloneNative()
instState.Ephemeral = d.ephemeral
instState.LastUsedAt = d.lastUsedDate
instState.Profiles = profileNames
instState.Stateful = d.stateful
instState.Project = d.project.Name
instState.Status = instState.StatusCode.String()

for _, option := range options {
err := option(&instState)
Expand Down Expand Up @@ -8250,11 +8256,6 @@ func (d *qemu) InitPID() int {
}

func (d *qemu) statusCode() api.StatusCode {
// If instance is running on a remote cluster member, we cannot determine instance state.
if d.state.ServerName != d.Location() {
return api.Error
}

// Shortcut to avoid spamming QMP during ongoing operations.
operationStatus := d.operationStatusCode()
if operationStatus != nil {
Expand Down

0 comments on commit 2b27096

Please sign in to comment.