Change in vdsm[master]: vm: Move saveState() out of constructor

fromani at redhat.com fromani at redhat.com
Mon Feb 29 16:38:57 UTC 2016


Francesco Romani has posted comments on this change.

Change subject: vm: Move saveState() out of constructor
......................................................................


Patch Set 3: Code-Review-1

-1 for visibility, please read.
This change looks correct, but devil's in the details.
Could you please add a detailed explanation about how you verified this patch?
I find easy to believe that things works OK in the happy path, but I'm concerned about the error path.
In the (albeit very unlikely) case Vdsm crashes while a Vm is running the Vm class constructor, 
of course after where saveState() was what would it happen?
- before this patch: the Vm would have been recovered from file, and would have been recreated by
  the recovery flow. This means that Vdsm should automatically try to start it again, unless I'm mistaken
  (I didn't actually run the modified code)
- after this patch: this would not happen, so the Vm will just disappear from the system. This will happen
  because libvirt is not yet into play and, the recovery file is not yet created.
  Can Engine cope with this case? Can we do better than this?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb38fdc06bf1a7b881cd64d2af45a93b16c935ea
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Betak <mbetak at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Betak <mbetak at redhat.com>
Gerrit-Reviewer: Milan Zamazal <mzamazal at redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: No


More information about the vdsm-patches mailing list