<html>
  <head>
    <meta content="text/html; charset=ISO-8859-1"
      http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <div class="moz-cite-prefix">Hi Cristian,<br>
      your analyses about the flow and root of the problem are correct.<br>
      However I think you are only seeing the case where the flow just
      works, you did not took in account that<br>
      rollbacks are used in case of errors/crashes/fails in the middle
      of test. For instance:<br>
      <br>
      &nbsp;&nbsp;&nbsp; 1-
      inst.vms_create(params_1)&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
      <br>
      &nbsp;&nbsp;&nbsp; 2-
      inst.vms_create(params_2)&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
      <br>
      &nbsp;&nbsp;&nbsp; 3- vms =
      inst.vms_get_list()&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; <br>
      &nbsp;&nbsp;&nbsp; 4-
      inst.vm_start('kimchi-vm1')&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
      <br>
      &nbsp;&nbsp;&nbsp; 5- info =
      inst.vm_lookup('kimchi-vm1')&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; <br>
      &nbsp;&nbsp;&nbsp; ...<br>
      <br>
      - If the code fails/breaks in (3), then (1) and (2) must be
      'rollbacked'.<br>
      - If the code fails/breaks in (5) , then (1), (2) and (4) must be
      'rollbacked' ... (not really sure if 4 is needed, but the code was
      already there)<br>
      <br>
      For these reasons, I cannot remove the rollback after create the
      vms, otherwise, they will remains in the system.<br>
      <br>
      If the code breaks in (4), while starting the vm (receive an
      exception), how would you remove the vms created above (1)(2) ? I
      asked myself this.<br>
      The option I could think know is to add try/except in the code,
      but, the with/rollback was implemented exactly for this (or to
      avoid this).<br>
      I also tried to split with/rollback, but this does not work.<br>
      <br>
      <br>
      On 02/26/2014 04:52 PM, Cr&iacute;stian Viana wrote:<br>
    </div>
    <blockquote cite="mid:530E45F0.9090603@linux.vnet.ibm.com"
      type="cite">
      <meta content="text/html; charset=ISO-8859-1"
        http-equiv="Content-Type">
      AFAIU, here is what happens in the method "test_vm_update": the
      VMs "kimchi-vm1" and "kimchi-vm2" are created; the commands to <b>delete

        those VMS by their names</b> are deferred in the transaction; <b>the

        VM "kimchi-vm1" is renamed to "&#1087;e&#969;-&#8744;&#1084;"</b>; the command to <b>delete

        "&#1087;e&#969;-&#8744;&#1084;" by its name</b> is deferred in the transaction.<br>
      <br>
      Notice how the VM is deleted twice. Both "kimchi-vm1" and "&#1087;e&#969;-&#8744;&#1084;"
      are the same VMs, with different names in different moments. The
      command "vm_delete" fails for two reasons: 1) the VM "kimchi-vm1"
      does not exist anymore when "vm_delete" is executed
      (NotFoundError); 2) even if it did, you cannot delete a VM twice
      (also NotFoundError).<br>
      So the correct solution would be to not delete the VM twice, and
      keep track of the names they have during the tests. If the VMs are
      renamed, the deferred commands will fail in the future when they
      execute.<br>
      <br>
      The same analysis can be done to work out a better solution for
      the "vm_stop" case. That function "_rollback_wrapper" looks very
      hacky and there should be a way not to need it.<br>
      <br>
      <div class="moz-cite-prefix">Am 26-02-2014 16:05, schrieb Rodrigo
        Trujillo:<br>
      </div>
      <blockquote
cite="mid:1393441518-27878-7-git-send-email-rodrigo.trujillo@linux.vnet.ibm.com"
        type="cite">
        <pre wrap="">With new backend messaging system, when a VM is not found the backend
raises an exception which was breaking Rollback functions. This patch
fixes this problem.

Signed-off-by: Rodrigo Trujillo <a moz-do-not-send="true" class="moz-txt-link-rfc2396E" href="mailto:rodrigo.trujillo@linux.vnet.ibm.com">&lt;rodrigo.trujillo@linux.vnet.ibm.com&gt;</a>
---
 tests/test_model.py | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/tests/test_model.py b/tests/test_model.py
index 74e2424..d48377d 100644
--- a/tests/test_model.py
+++ b/tests/test_model.py
@@ -547,14 +547,17 @@ class ModelTests(unittest.TestCase):
             params_2 = {'name': 'kimchi-vm2', 'template': '/templates/test'}
             inst.vms_create(params_1)
             inst.vms_create(params_2)
-            rollback.prependDefer(inst.vm_delete, 'kimchi-vm1')
-            rollback.prependDefer(inst.vm_delete, 'kimchi-vm2')
+            rollback.prependDefer(self._rollback_wrapper, inst.vm_delete,
+                                  'kimchi-vm1')
+            rollback.prependDefer(self._rollback_wrapper, inst.vm_delete,
+                                  'kimchi-vm2')

             vms = inst.vms_get_list()
             self.assertTrue('kimchi-vm1' in vms)

             inst.vm_start('kimchi-vm1')
-            rollback.prependDefer(inst.vm_stop, 'kimchi-vm1')
+            rollback.prependDefer(self._rollback_wrapper, inst.vm_stop,
+                                  'kimchi-vm1')

             info = inst.vm_lookup('kimchi-vm1')
             self.assertEquals('running', info['state'])
@@ -569,7 +572,8 @@ class ModelTests(unittest.TestCase):
                               'kimchi-vm1', {'name': 'kimchi-vm2'})
             inst.vm_update('kimchi-vm1', params)
             self.assertEquals(info['uuid'], inst.vm_lookup(u'&#1087;e&#969;-&#8744;&#1084;')['uuid'])
-            rollback.prependDefer(inst.vm_delete, u'&#1087;e&#969;-&#8744;&#1084;')
+            rollback.prependDefer(self._rollback_wrapper, inst.vm_delete,
+                                  u'&#1087;e&#969;-&#8744;&#1084;')

     @unittest.skipUnless(utils.running_as_root(), 'Must be run as root')
     def test_network(self):
@@ -758,6 +762,17 @@ class ModelTests(unittest.TestCase):
                           inst.task_lookup(taskid)['message'])
         self.assertEquals('failed', inst.task_lookup(taskid)['status'])

+    # This wrapper function is needed due to the new backend messaging in
+    # vm model. vm_stop and vm_delete raise exception if vm is not found.
+    # These functions are called after vm has been deleted if test finishes
+    # correctly, then NofFoundError exception is raised and rollback breaks
+    def _rollback_wrapper(self, func, vmname):
+        try:
+            func(vmname)
+        except NotFoundError:
+            # VM has been deleted already
+            return
+
     @unittest.skipUnless(utils.running_as_root(), 'Must be run as root')
     def test_delete_running_vm(self):
         inst = model.Model(objstore_loc=self.tmp_store)
@@ -769,10 +784,12 @@ class ModelTests(unittest.TestCase):

             params = {'name': u'k&#299;&#1084;&#1089;h&#299;-&#8744;&#1084;', 'template': u'/templates/test'}
             inst.vms_create(params)
-            rollback.prependDefer(inst.vm_delete, u'k&#299;&#1084;&#1089;h&#299;-&#8744;&#1084;')
+            rollback.prependDefer(self._rollback_wrapper, inst.vm_delete,
+                                  u'k&#299;&#1084;&#1089;h&#299;-&#8744;&#1084;')

             inst.vm_start(u'k&#299;&#1084;&#1089;h&#299;-&#8744;&#1084;')
-            rollback.prependDefer(inst.vm_stop, u'k&#299;&#1084;&#1089;h&#299;-&#8744;&#1084;')
+            rollback.prependDefer(self._rollback_wrapper, inst.vm_stop,
+                                  u'k&#299;&#1084;&#1089;h&#299;-&#8744;&#1084;')

             inst.vm_delete(u'k&#299;&#1084;&#1089;h&#299;-&#8744;&#1084;')

</pre>
      </blockquote>
      <br>
      <br>
      <fieldset class="mimeAttachmentHeader"></fieldset>
      <br>
      <pre wrap="">_______________________________________________
Kimchi-devel mailing list
<a class="moz-txt-link-abbreviated" href="mailto:Kimchi-devel@ovirt.org">Kimchi-devel@ovirt.org</a>
<a class="moz-txt-link-freetext" href="http://lists.ovirt.org/mailman/listinfo/kimchi-devel">http://lists.ovirt.org/mailman/listinfo/kimchi-devel</a>
</pre>
    </blockquote>
    <br>
  </body>
</html>