ShaoHe Feng has posted comments on this change. Change subject: Add shutdown based on qemu-ga(qemu guest agent) in vdsm ......................................................................
Patch Set 3: Vinzenz: you can have a look at this patch: http://gerrit.ovirt.org/#/c/5640/ Gal Hammer's comment: "I think that the code that handle the qga and qga's shutdown should only be in libvirtvm. I'm not sure if we still support running a vm without libvirt (Dan?) but since the code call libvirt functions it should not be in the vm.Vm class." so I'm not sure if we will run a vm without libvirt. So I refactor this code as duty chain pattern. It is flexible for new child class. The VM is the parent class and the libvirtvm is children class. in order to run vm with other hyper manager not libvirt which means maybe another child class, some attributions of parent class should be Override. Barak Azulay's comment: "The logic around the shutdown should be something like this, if qga.isResponsive(): .... send shutdown to libvirt ... elif guestAgent.isResponsive(): ... do rhev seaquence elif acpi ... .... " so the guestAgent is in VM class, and qga is in libvirtVm class. and acpi should be in both parent class and child class. The children class should override it. -- To view, visit http://gerrit.ovirt.org/9655 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I86977c1b717d63de21ba4818c6b66e43976d65de Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: ShaoHe Feng <[email protected]> Gerrit-Reviewer: Barak Azulay <[email protected]> Gerrit-Reviewer: Bing Bu Cao <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: ShaoHe Feng <[email protected]> Gerrit-Reviewer: Vinzenz Feenstra <[email protected]> Gerrit-Reviewer: Yaniv Kaul <[email protected]> Gerrit-Reviewer: Zhou Zheng Sheng <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
