Change in vdsm[master]: vm: Modify and save state after hotunplugging the disk

Xavi Francisco has uploaded a new change for review.

Change subject: vm: Modify and save state after hotunplugging the disk
......................................................................

vm: Modify and save state after hotunplugging the disk

The rationale behind this patch is to modify the way the hotunplug
process is executed. Now when the hotunplug command is called we first
remove the disk from the internal state and after that the libvirt
hotunplug command is executed.The problem with that approach is
that if VDSM restarts between the modification of the internal
state and the actual unplugging of the disk, the disk never gets
unplugged leaving the system in an inconsistent state.

This patch solves the issue by executing the modification of the
internal state after the actual unplugging has successfully happened.
This state is later saved so if either the disk has been unplugged or
not VDSM is able to recover its state in case of a restart.

Change-Id: I5169fa16591283de33aa82c5730626bfd5d3eaf5
Signed-off-by: Xavi Francisco <xfrancis(a)redhat.com&gt;
---
M vdsm/virt/vm.py
1 file changed, 12 insertions(+), 20 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/87/28187/1

diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index bd670b9..1eb2adc 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -3449,20 +3449,6 @@
         driveXml = drive.getXML().toprettyxml(encoding='utf-8')
         self.log.debug("Hotunplug disk xml: %s", driveXml)
         # Remove found disk from vm's drives list
-        if isVdsmImage(drive):
-            self.sdIds.remove(drive.domainID)
-        self._devices[DISK_DEVICES].remove(drive)
-        # Find and remove disk device from vm's conf
-        diskDev = None
-        for dev in self.conf['devices'][:]:
-            if (dev['type'] == DISK_DEVICES and
-                    dev['path'] == drive.path):
-                with self._confLock:
-                    self.conf['devices'].remove(dev)
-                diskDev = dev
-                break
-
-        self.saveState()
 
         hooks.before_disk_hotunplug(driveXml, self.conf,
                                     params=drive.custom)
@@ -3472,19 +3458,25 @@
             self.log.error("Hotunplug failed", exc_info=True)
             if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN:
                 return errCode['noVM']
-            self._devices[DISK_DEVICES].append(drive)
-            # Restore disk device in vm's conf and _devices
-            if diskDev:
-                with self._confLock:
-                    self.conf['devices'].append(diskDev)
-            self.saveState()
             return {
                 'status': {'code':
errCode['hotunplugDisk']['status']['code'],
                            'message': e.message}}
         else:
+            if isVdsmImage(drive):
+                self.sdIds.remove(drive.domainID)
+            self._devices[DISK_DEVICES].remove(drive)
+            # Find and remove disk device from vm's conf
+            for dev in self.conf['devices'][:]:
+                if (dev['type'] == DISK_DEVICES and
+                        dev['path'] == drive.path):
+                    with self._confLock:
+                        self.conf['devices'].remove(dev)
+                    break
             hooks.after_disk_hotunplug(driveXml, self.conf,
                                        params=drive.custom)
             self._cleanupDrives(drive)
+        finally:
+            self.saveState()
 
         return {'status': doneCode, 'vmList': self.status()}
 


-- 
To view, visit http://gerrit.ovirt.org/28187
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I5169fa16591283de33aa82c5730626bfd5d3eaf5
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Xavi Francisco <xfrancis(a)redhat.com&gt;

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vm: Modify and save state after hotunplugging the disk
......................................................................


Patch Set 1:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8598/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9532/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9386/ : SUCCESS

-- 
To view, visit http://gerrit.ovirt.org/28187
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5169fa16591283de33aa82c5730626bfd5d3eaf5
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Xavi Francisco <xfrancis(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Itamar Heim has posted comments on this change.

Change subject: vm: Modify and save state after hotunplugging the disk
......................................................................


Patch Set 1:

ping

-- 
To view, visit http://gerrit.ovirt.org/28187
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5169fa16591283de33aa82c5730626bfd5d3eaf5
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Xavi Francisco <xfrancis(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: vm: Modify and save state after hotunplugging the disk
......................................................................


Patch Set 1:

Taking this.

-- 
To view, visit http://gerrit.ovirt.org/28187
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5169fa16591283de33aa82c5730626bfd5d3eaf5
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Xavi Francisco <xfrancis(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vm: Modify and save state after hotunplugging the disk
......................................................................


Patch Set 2:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/10338/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/11123/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1340/ : There was an
infra issue, please contact infra(a)ovirt.org

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/11280/ : FAILURE

-- 
To view, visit http://gerrit.ovirt.org/28187
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5169fa16591283de33aa82c5730626bfd5d3eaf5
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Xavi Francisco <xfrancis(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Allon Mureinik]

Allon Mureinik has abandoned this change.

Change subject: vm: Modify and save state after hotunplugging the disk
......................................................................


Abandoned

No longer relevant in light of the recent patches merged.

-- 
To view, visit https://gerrit.ovirt.org/28187
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: abandon
Gerrit-Change-Id: I5169fa16591283de33aa82c5730626bfd5d3eaf5
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Xavi Francisco <xfrancis(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org

automation(a)ovirt.org has posted comments on this change.

Change subject: vm: Modify and save state after hotunplugging the disk
......................................................................


Patch Set 2:

* Update tracker::IGNORE, no Bug-Url found

-- 
To view, visit https://gerrit.ovirt.org/28187
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5169fa16591283de33aa82c5730626bfd5d3eaf5
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Xavi Francisco <xfrancis(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No