Change in vdsm[master]: virt: Replace vm._dom.XMLDesc(0) with domain descriptor usage

fromani at redhat.com fromani at redhat.com
Tue Feb 16 14:16:14 UTC 2016


Francesco Romani has posted comments on this change.

Change subject: virt: Replace vm._dom.XMLDesc(0) with domain descriptor usage
......................................................................


Patch Set 3:

(1 comment)

https://gerrit.ovirt.org/#/c/52882/3/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 4581:                 # when migration completes, see qemuMigrationFinish function).
Line 4582:                 # In this case self._dom is disconnected because the function
Line 4583:                 # _completeIncomingMigration didn't update it yet.
Line 4584:                 try:
Line 4585:                     domxml = self._updateDomainDescriptor()
> I don't like mixing side effect (updating the xml) and returning the curren
I suggested to return the update value from updateDomainDescriptor out of convenience, but actually your approach is the most correct.

Unfortunately, to avoid regression, we will have to call updateDomainDescriptor() each time we access XMLDesc(). We don't have (yet) a clear cut about when it is safe to cache XML and when it is not, so we must be overly cautious.

We could file a RFE on libvirt asking for an event to be fired when XML changes. After all, libvirt knows well and easily when the domain changes.

On a future patch we could start dropping updateDomainDescriptor() after careful per-flow evaluation.
When/if we get this new XML update event, it will be even simpler and easier.
Line 4586:                 except virdomain.NotConnectedError:
Line 4587:                     pass
Line 4588:                 else:
Line 4589:                     hooks.after_vm_pause(domxml, self.conf)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic159cb84049d90cdb2a4279a34145462c8ee6be4
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: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: Milan Zamazal <mzamazal at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list