Change in vdsm[ovirt-3.6]: vm: fix misleading XML log

fromani at redhat.com fromani at redhat.com
Wed Nov 25 08:49:59 UTC 2015


Hello Dan Kenigsberg,

I'd like you to do a code review.  Please visit

    https://gerrit.ovirt.org/49139

to review the following change.

Change subject: vm: fix misleading XML log
......................................................................

vm: fix misleading XML log

When VDSM is asked to create a new VM, among the other things,
it builds the domain XML according to the configuration sent
by client (aka Engine) and run the before_vm_start hook.

This is done also on the dehibernating (aka migration from file)
path, for no apparent good reason.

This is wasteful and useless at best, and most likely misleading,
because VDSM logs the XML it produced, which is *not* what is
gonna be used in the dehibernation path.

This patch fixes this issue loggging the XML which is actually
about to be fed to libvirt, not the useless one regenerated
by VDSM.

Backport-To: 3.6
Change-Id: I50f5b8a51830c8ce2a098e4ac11dd1bd6a89aac3
Related-To: https://bugzilla.redhat.com/1190210
Related-To: https://bugzilla.redhat.com/1234197
Signed-off-by: Francesco Romani <fromani at redhat.com>
Reviewed-on: https://gerrit.ovirt.org/44843
Reviewed-by: Dan Kenigsberg <danken at redhat.com>
Continuous-Integration: Dan Kenigsberg <danken at redhat.com>
---
M vdsm/virt/vm.py
1 file changed, 16 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/39/49139/1

diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index b75215c..01d205b 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -1836,13 +1836,6 @@
                 self.log.debug('Detaching device %s from the host.' % dev_name)
                 dev.detach()
 
-            if initDomain:
-                domxml = hooks.before_vm_start(self._buildDomainXML(),
-                                               self.conf)
-                # TODO: this is debug information. For 3.6.x we still need to
-                # see the XML even with 'info' as default level.
-                self.log.info(domxml)
-
         if self.recovering:
             self._dom = virdomain.Notifying(
                 self._connection.lookupByUUIDString(self.id),
@@ -1850,6 +1843,13 @@
         elif 'migrationDest' in self.conf:
             pass  # self._dom will be disconnected until migration ends.
         elif 'restoreState' in self.conf:
+            # TODO: for unknown historical reasons, we call this hook also
+            # on this flow. Issues:
+            # - we will also call the more specific before_vm_dehibernate
+            # - we feed the hook with wrong XML
+            # - we ignore the output of the hook
+            hooks.before_vm_start(self._buildDomainXML(), self.conf)
+
             fromSnapshot = self.conf.get('restoreFromSnapshot', False)
             srcDomXML = self.conf.pop('_srcDomXML')
             if fromSnapshot:
@@ -1857,6 +1857,10 @@
                 srcDomXML = self._correctGraphicsConfiguration(srcDomXML)
             hooks.before_vm_dehibernate(srcDomXML, self.conf,
                                         {'FROM_SNAPSHOT': str(fromSnapshot)})
+
+            # TODO: this is debug information. For 3.6.x we still need to
+            # see the XML even with 'info' as default level.
+            self.log.info(srcDomXML)
 
             fname = self.cif.prepareVolumePath(self.conf['restoreState'])
             try:
@@ -1871,6 +1875,11 @@
                 self._connection.lookupByUUIDString(self.id),
                 self._timeoutExperienced)
         else:
+            domxml = hooks.before_vm_start(self._buildDomainXML(), self.conf)
+            # TODO: this is debug information. For 3.6.x we still need to
+            # see the XML even with 'info' as default level.
+            self.log.info(domxml)
+
             flags = libvirt.VIR_DOMAIN_NONE
             with self._confLock:
                 if 'launchPaused' in self.conf:


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I50f5b8a51830c8ce2a098e4ac11dd1bd6a89aac3
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>


More information about the vdsm-patches mailing list