Skip to content

Commit 8487ea4

Browse files
committed
fix: VM disks list would be wrong after a snap revert
The list of VDIs of a VM would be changed following a snapshot revert. It would result in not having the correct list when destroying a VM and leaving VDIs behind. This fix make the `Snapshot.revert()` call refresh the VDI list of the VM object it was created from. Signed-off-by: Damien Thenot <[email protected]>
1 parent bada08c commit 8487ea4

File tree

2 files changed

+28
-11
lines changed

2 files changed

+28
-11
lines changed

lib/snapshot.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,18 @@
22

33
from lib.basevm import BaseVM
44

5+
from typing import TYPE_CHECKING
6+
7+
if TYPE_CHECKING:
8+
from lib.host import Host
9+
from lib.vm import VM
10+
11+
512
class Snapshot(BaseVM):
13+
def __init__(self, uuid: str, host: "Host", vm: "VM"):
14+
self.basevm = vm
15+
super(Snapshot, self).__init__(uuid, host)
16+
617
def _disk_list(self):
718
return self.host.xe('snapshot-disk-list', {'uuid': self.uuid, 'vbd-params': ''},
819
minimal=True)
@@ -21,3 +32,4 @@ def exists(self):
2132
def revert(self):
2233
logging.info("Revert to snapshot %s", self.uuid)
2334
self.host.xe('snapshot-revert', {'uuid': self.uuid})
35+
self.basevm.create_vdis_list() # We reset the base VM object VDIs list because it changed following the revert

lib/vm.py

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,7 @@ def __init__(self, uuid: str, host: 'Host'):
3434
self.previous_host = None # previous host when migrated or being migrated
3535
self.is_windows = self.param_get('platform', 'device_id', accept_unknown_key=True) == '0002'
3636
self.is_uefi = self.param_get('HVM-boot-params', 'firmware', accept_unknown_key=True) == 'uefi'
37-
try:
38-
self.vdis = [VDI(vdi_uuid, host=host) for vdi_uuid in self.vdi_uuids()]
39-
except commands.SSHCommandFailed as e:
40-
# Doesn't work with Dom0 since `vm-disk-list` doesn't work on it so we create empty list
41-
if e.stdout == "Error: No matching VMs found":
42-
logging.info("Couldn't get disks list. We are Dom0. Continuing...")
43-
self.vdis = []
44-
else:
45-
raise
37+
self.create_vdis_list()
4638

4739
def power_state(self) -> str:
4840
return self.param_get('power-state')
@@ -289,13 +281,14 @@ def snapshot(self, ignore_vdis=None):
289281
args = {'uuid': self.uuid, 'new-name-label': 'Snapshot of %s' % self.uuid}
290282
if ignore_vdis:
291283
args['ignore-vdi-uuids'] = ','.join(ignore_vdis)
292-
return Snapshot(self.host.xe('vm-snapshot', args), self.host)
284+
snap_uuid = self.host.xe('vm-snapshot', args)
285+
return Snapshot(snap_uuid, self.host, self)
293286

294287
def checkpoint(self) -> Snapshot:
295288
logging.info("Checkpoint VM")
296289
return Snapshot(self.host.xe('vm-checkpoint', {'uuid': self.uuid,
297290
'new-name-label': 'Checkpoint of %s' % self.uuid}),
298-
self.host)
291+
self.host, self)
299292

300293
def connect_vdi(self, vdi: VDI, device: str = "autodetect") -> str:
301294
logging.info(f">> Plugging VDI {vdi.uuid} on VM {self.uuid}")
@@ -338,6 +331,18 @@ def destroy_vdi(self, vdi_uuid: str) -> None:
338331
super().destroy_vdi(vdi_uuid)
339332
break
340333

334+
def create_vdis_list(self) -> None:
335+
""" Used to redo the VDIs list of the VM when reverting a snapshot. """
336+
try:
337+
self.vdis = [VDI(vdi_uuid, host=self.host) for vdi_uuid in self.vdi_uuids()]
338+
except commands.SSHCommandFailed as e:
339+
# Doesn't work with Dom0 since `vm-disk-list` doesn't work on it so we create empty list
340+
if e.stdout == "Error: No matching VMs found":
341+
logging.info("Couldn't get disks list. We are Dom0. Continuing...")
342+
self.vdis = []
343+
else:
344+
raise
345+
341346
def vifs(self):
342347
_vifs = []
343348
for vif_uuid in safe_split(self.host.xe('vif-list', {'vm-uuid': self.uuid}, minimal=True)):

0 commit comments

Comments
 (0)