CP-54207: Move VBD_attach outside of VM migrate downtime #6489
Conversation
| @@ -187,7 +187,7 @@ module VmExtra = struct | |||
| ; pv_drivers_detected: bool [@default false] | |||
| ; xen_platform: (int * int) option (* (device_id, revision) for QEMU *) | |||
| ; platformdata: (string * string) list [@default []] | |||
| ; attached_vdis: (Vbd.id * attached_vdi) list [@default []] | |||
| ; attached_vdis: (string * attached_vdi) list [@default []] | |||
There was a problem hiding this comment.
I'm not sure if we would need to do anything special when updating to this?
There was a problem hiding this comment.
I'm not sure I follow the explanation of the commit, this was an ID of a Virtual Block Device (VBD), not a VM ID. Are you sure this is the right thing to do?
There was a problem hiding this comment.
AIUI, a Vbd.id is actually a tuple (string * string). The first is the vm id and the second is the vbd id.
We're using the vmextra DB (indexed by VM id) and store the (Vbd.id, vdi) in attached_vdis after VBD_attach and retrieve this value to use in VBD_activate. Temporary VM ids are used during migration but currently this doesn't matter as we do activate immediately after attach, so the VM ids are the same.
In this PR, we are doing attach earlier, so the VM id it's using at this point is something like xxxx-00000001, so when we later try to retrieve this value for the VBD_attach, the VM id has changed to xxxx-xxxxxxxxx so we can't find it.
In VM_rename, the VM id key xxxx-00000001 is correctly updated to xxxx-xxxxxxxxx. However, if we're using Vbd.id in attached_vdis, this isn't updated so we still fail to find it as the key is (xxxx-00000001, <vbd_id>) rather than (xxxx-xxxxxxxxx, <vbd_id>). As vmextra is already indexed by VM, using the full vbd.id is unnecessary, so we can avoid this whole issue by not using the VM id part of vbd.id.
snwoods
force-pushed
the
private/stevenwo/CP-54207
branch
from
8c82f1c to
72e919a
Compare
|
From my manual testing, the early VBD_attach looks to be working as expected for most of our SR backends, everything functions the same except the attach happens outside of the VM downtime so the downtime is reduced. However, for smapiv1 local storage (LVM and EXT), during a cross-host live migration the attach is becoming a no-op (not calling out to the smapi script and taking ~5ms rather than several hundred milliseconds). I am currently investigating why this happens but it is not necessarily a bad thing if everything is working correctly as it means the overall downtime is reduced. It is likely the smapiv1 state machine determining that the attach is not needed, but I am looking into why this is happening when attaching early and not when the attach happens later. I am also looking at enabling/disabling this early attach based on a new SM feature so that we can e.g. turn it off specifically for LVM/EXT if that is having problems. This is really the only clean way of doing this as in this section of the code we don't know whether v1 or v3 is involved (and indeed a VM could have some v1 disks and some v3). |
The VM id part of Vbd.id is unnecessary in the attached_vdis key as the DB is already indexed by the VM id. This also prevents problems when the VM is renamed. Signed-off-by: Steven Woods <[email protected]>
snwoods
force-pushed
the
private/stevenwo/CP-54207
branch
2 times, most recently
from
82af736 to
8ac5636
Compare
VBDs can be attached to multiple VMs, so now that VBD_plug has been split into VBD_attach and VBD_activate, the attach can happen outside of the VM migrate downtime. This doesn't change the overall duration of the migration but can reduce the downtime by several seconds. This new functionality is dependent on two flags: firstly, xenopsd_vbd_plug_unplug_legacy must be false so that the VBD_attach and VBD_activate are separate atoms. This is off by default. Then there is another flag can_attach_early which is currently true iff the VBD's SM has required_api_version >= 3.0 Signed-off-by: Steven Woods <[email protected]>
snwoods
force-pushed
the
private/stevenwo/CP-54207
branch
from
8ac5636 to
5351b0b
Compare
|
I think given the amount of testing and the discussions we've had within toolstack and with the storage team, this is well-understood enough that we can change this from a draft to a full PR. The can_attach_early flag will be true for SMAPIv3 SRs but it still won't early attach until the |
VBDs can be attached to multiple VMs, so now that VBD_plug has been split into VBD_attach and VBD_activate, the attach can happen outside of the VM migrate downtime. This doesn't change the overall duration of the migration but does reduce the downtime by several seconds.
Opening as a draft PR to get some eyes on it early as it will need extensive testing.