|
| 1 | +# ABLESTACK VM Hangctl Migration Protection Design |
| 2 | + |
| 3 | +## Background |
| 4 | + |
| 5 | +`ablestack_vm_hangctl` currently has a migration protection path, but it is not |
| 6 | +strong enough for long-running live migrations. |
| 7 | + |
| 8 | +The current flow has these weaknesses: |
| 9 | + |
| 10 | +- A VM is treated as migrating only when `virsh domstate --reason` contains |
| 11 | + `migration`. |
| 12 | +- `virsh domjobinfo` is used for backup/job detection, but active migration jobs |
| 13 | + are not classified separately before the generic backup path. |
| 14 | +- `HANGCTL_MIGRATION_PROGRESS_CHECK_SEC` is defined but not used for migration |
| 15 | + zombie decisions. |
| 16 | +- Migration progress is parsed from `Data processed` or `Memory processed` |
| 17 | + without unit normalization, so values such as `47.235 MiB` are not reliable |
| 18 | + inputs for shell integer arithmetic. |
| 19 | +- A long-running live migration can therefore reach the confirmed action path |
| 20 | + and be destroyed even when the migration is still active. |
| 21 | + |
| 22 | +The protection policy must be conservative: an active migration should be |
| 23 | +protected unless hangctl has clear evidence that the migration job is present |
| 24 | +but has made no progress for the configured grace period. |
| 25 | + |
| 26 | +## Goals |
| 27 | + |
| 28 | +- Detect active live migration from both `domstate --reason` and `domjobinfo`. |
| 29 | +- Use `HANGCTL_MIGRATION_PROGRESS_CHECK_SEC` as the no-progress grace window. |
| 30 | +- Keep `HANGCTL_MIGRATION_CONFIRM_WINDOW_SEC` as the wider confirmation window |
| 31 | + before destructive action is allowed. |
| 32 | +- Treat progress parsing failures as protected active migration when a |
| 33 | + migration job is visible. |
| 34 | +- Avoid classifying migration jobs as generic backup/block jobs. |
| 35 | +- Add smoke tests that do not require a real libvirt migration. |
| 36 | + |
| 37 | +## Non-Goals |
| 38 | + |
| 39 | +- Do not change the default destructive action behavior for non-migration hangs. |
| 40 | +- Do not replace hangctl's existing evidence, dump, storage guard, or libvirtd |
| 41 | + health-check behavior. |
| 42 | +- Do not require new binary dependencies. |
| 43 | + |
| 44 | +## Proposed Decision Flow |
| 45 | + |
| 46 | +For each VM scan: |
| 47 | + |
| 48 | +1. Collect QMP status, blockstats, `domstate --reason`, and `domjobinfo`. |
| 49 | +2. Classify `domjobinfo` before choosing the confirmation window. |
| 50 | +3. If active migration is detected: |
| 51 | + - Parse a progress metric in bytes. |
| 52 | + - If the metric increased, update `migration_last_progress_ts` and protect. |
| 53 | + - If the metric cannot be parsed, protect. |
| 54 | + - If the metric did not increase, compare the current time with |
| 55 | + `migration_last_progress_ts`. |
| 56 | + - Confirm `migration_zombie_no_progress` only when both of these are true: |
| 57 | + - `last_progress_age_sec >= HANGCTL_MIGRATION_PROGRESS_CHECK_SEC` |
| 58 | + - `duration_sec >= HANGCTL_MIGRATION_CONFIRM_WINDOW_SEC` |
| 59 | +4. If active migration is not detected, continue through the existing hangctl |
| 60 | + non-migration decision logic. |
| 61 | + |
| 62 | +This keeps short no-progress windows safe while still allowing hangctl to act on |
| 63 | +real zombie migration jobs after the configured confirmation period. |
| 64 | + |
| 65 | +## Migration Classification |
| 66 | + |
| 67 | +Add a helper in `lib/hangctl/detect.sh`: |
| 68 | + |
| 69 | +```bash |
| 70 | +hangctl_classify_domjobinfo <domjobinfo_text> <out_job_type> <out_operation> <out_is_migration> <out_is_backup> |
| 71 | +``` |
| 72 | + |
| 73 | +`out_is_migration=1` when any of these are true: |
| 74 | + |
| 75 | +- `domstate --reason` contains `migration`. |
| 76 | +- `domjobinfo` contains an `Operation:` value with `migration`. |
| 77 | +- `domjobinfo` contains migration-specific progress fields such as |
| 78 | + `Memory processed`, `Memory remaining`, or `Memory total`. |
| 79 | + |
| 80 | +`out_is_backup=1` should be reserved for non-migration active jobs. A non-`None` |
| 81 | +job must not automatically become backup if it is classified as migration. |
| 82 | + |
| 83 | +## Progress Metric |
| 84 | + |
| 85 | +Add a size parser in `lib/hangctl/detect.sh`: |
| 86 | + |
| 87 | +```bash |
| 88 | +hangctl_parse_size_to_bytes <value> <unit> |
| 89 | +``` |
| 90 | + |
| 91 | +Supported examples: |
| 92 | + |
| 93 | +- `1024 bytes` |
| 94 | +- `47.235 MiB` |
| 95 | +- `2.1 GiB` |
| 96 | +- `1 TB` |
| 97 | + |
| 98 | +The preferred progress metric order is: |
| 99 | + |
| 100 | +1. `Data processed` |
| 101 | +2. `Memory processed` |
| 102 | +3. `Memory remaining` as an inverse metric, where a decrease means progress |
| 103 | + |
| 104 | +If no metric can be parsed from an active migration job, the result must be |
| 105 | +`protect_unknown_progress`, not `confirmed`. |
| 106 | + |
| 107 | +## State Cache Format |
| 108 | + |
| 109 | +Replace the single numeric `<vm>.state.migrate` payload with key-value fields. |
| 110 | +The existing file path can remain the same to limit blast radius. |
| 111 | + |
| 112 | +```text |
| 113 | +migration_metric_bytes=123456789 |
| 114 | +migration_metric_kind=data_processed |
| 115 | +migration_last_progress_ts=1780000000 |
| 116 | +migration_last_seen_ts=1780000030 |
| 117 | +migration_job_type=Unbounded |
| 118 | +migration_operation=Migration Out |
| 119 | +``` |
| 120 | + |
| 121 | +Required helpers in `lib/hangctl/state_cache.sh`: |
| 122 | + |
| 123 | +```bash |
| 124 | +hangctl_state_get_migration_kv <vm> <key> |
| 125 | +hangctl_state_set_migration_kv_all <vm> key=value ... |
| 126 | +hangctl_state_reset_migration <vm> |
| 127 | +``` |
| 128 | + |
| 129 | +Backward compatibility: |
| 130 | + |
| 131 | +- If the old file contains only a number, treat it as the previous |
| 132 | + `migration_metric_bytes`. |
| 133 | +- Rewrite the file in key-value format on the next active migration scan. |
| 134 | + |
| 135 | +## Zombie Evaluation API |
| 136 | + |
| 137 | +Replace the current boolean-only migration zombie function with a richer helper: |
| 138 | + |
| 139 | +```bash |
| 140 | +hangctl_probe_migration_progress_evaluate \ |
| 141 | + <vm> \ |
| 142 | + <domjobinfo_text> \ |
| 143 | + <duration_sec> \ |
| 144 | + <out_status_var> \ |
| 145 | + <out_detail_var> |
| 146 | +``` |
| 147 | + |
| 148 | +Status values: |
| 149 | + |
| 150 | +- `progressing` |
| 151 | +- `no_progress_within_grace` |
| 152 | +- `zombie_no_progress` |
| 153 | +- `protect_unknown_progress` |
| 154 | +- `not_migration` |
| 155 | + |
| 156 | +The caller maps these statuses as follows: |
| 157 | + |
| 158 | +- `progressing`, `no_progress_within_grace`, `protect_unknown_progress`: |
| 159 | + return without action. |
| 160 | +- `zombie_no_progress`: continue to confirmed decision with |
| 161 | + `migration_zombie_no_progress`. |
| 162 | +- `not_migration`: continue existing non-migration logic. |
| 163 | + |
| 164 | +## Logging |
| 165 | + |
| 166 | +Add structured details to the existing JSONL events. |
| 167 | + |
| 168 | +Progressing: |
| 169 | + |
| 170 | +```text |
| 171 | +event=vm.migration_check |
| 172 | +status=progressing |
| 173 | +metric_kind=data_processed |
| 174 | +metric_bytes=123456789 |
| 175 | +delta_bytes=10485760 |
| 176 | +last_progress_age_sec=0 |
| 177 | +note=protecting_active_migration |
| 178 | +``` |
| 179 | + |
| 180 | +No progress but still protected: |
| 181 | + |
| 182 | +```text |
| 183 | +event=vm.migration_check |
| 184 | +status=no_progress_within_grace |
| 185 | +metric_kind=data_processed |
| 186 | +metric_bytes=123456789 |
| 187 | +last_progress_age_sec=180 |
| 188 | +progress_window_sec=300 |
| 189 | +note=protecting_active_migration |
| 190 | +``` |
| 191 | + |
| 192 | +Confirmed zombie: |
| 193 | + |
| 194 | +```text |
| 195 | +event=vm.decision |
| 196 | +final=confirmed |
| 197 | +reason=migration_zombie_no_progress |
| 198 | +last_progress_age_sec=3900 |
| 199 | +progress_window_sec=300 |
| 200 | +confirm_window_sec=3600 |
| 201 | +``` |
| 202 | + |
| 203 | +Unknown progress: |
| 204 | + |
| 205 | +```text |
| 206 | +event=vm.migration_check |
| 207 | +status=protect_unknown_progress |
| 208 | +reason=metric_parse_failed |
| 209 | +note=protecting_active_migration |
| 210 | +``` |
| 211 | + |
| 212 | +## Test Plan |
| 213 | + |
| 214 | +Add a shell smoke test under `tests/`, for example: |
| 215 | + |
| 216 | +```text |
| 217 | +tests/hangctl_migration_protection_smoke.sh |
| 218 | +``` |
| 219 | + |
| 220 | +Suggested cases: |
| 221 | + |
| 222 | +- `domstate="paused (in-migration)"` and progress increases: no action. |
| 223 | +- `domstate="running"` with `domjobinfo Operation: Migration Out`: no action. |
| 224 | +- Active migration with unparsable progress: no action. |
| 225 | +- Active migration with no progress for less than |
| 226 | + `HANGCTL_MIGRATION_PROGRESS_CHECK_SEC`: no action. |
| 227 | +- Active migration with no progress beyond |
| 228 | + `HANGCTL_MIGRATION_PROGRESS_CHECK_SEC` but before |
| 229 | + `HANGCTL_MIGRATION_CONFIRM_WINDOW_SEC`: no action. |
| 230 | +- Active migration with no progress beyond both windows: |
| 231 | + `migration_zombie_no_progress`. |
| 232 | +- Non-migration active backup/block job: keep existing backup policy. |
| 233 | + |
| 234 | +Run: |
| 235 | + |
| 236 | +```bash |
| 237 | +bash -n bin/ablestack_vm_hangctl.sh lib/hangctl/*.sh |
| 238 | +bash tests/hangctl_migration_protection_smoke.sh |
| 239 | +``` |
0 commit comments