Doron Fediuck has posted comments on this change.

Change subject: Add an option to create a watchdog device.
......................................................................


Patch Set 9: I would prefer that you didn't submit this

(1 inline comment)

ShaoHe,

Thanks for sorting out the path issue, and for submitting this patch. 

There are other issues I just noticed we need to verify:

1. Default action should not be reset, as this may be harmful. We should 
default to none. So unless the user specified he wants to reset, no harm will 
be caused.

2. Just to make sure I fully understand, by default a VM will not have a 
Watchdog device, unless given by the engine. Is this correct? We need to ensure 
users will not get new devices unless they specifically ask for it.

....................................................
File vdsm/libvirtvm.py
Line 1174:         """
Line 1175:         m = self.createXmlElem(self.type, None, ['address'])
Line 1176:         m.setAttribute('model', self.specParams['model'])
Line 1177:         if 'action' in self.specParams:
Line 1178:             m.setAttribute('action', self.specParams['action'])
Please set default action to None.
We do not want see VMs reseting just because the device was accidentally
 attached.

I expect the users of the watchdog device to specify it explicitly as well as 
the desired action, as this has harmful implications.
Line 1179:         return m
Line 1180: 
Line 1181: 
Line 1182: class RedirDevice(LibvirtVmDevice):


--
To view, visit http://gerrit.ovirt.org/7535
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2b7970a9050ab0279fe03371b9a77692fba30af8
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: ShaoHe Feng <[email protected]>
Gerrit-Reviewer: Adam Litke <[email protected]>
Gerrit-Reviewer: Bing Bu Cao <[email protected]>
Gerrit-Reviewer: Doron Fediuck <[email protected]>
Gerrit-Reviewer: Mark Wu <[email protected]>
Gerrit-Reviewer: Michael Burns <[email protected]>
Gerrit-Reviewer: Ryan Harper <[email protected]>
Gerrit-Reviewer: ShaoHe Feng <[email protected]>
Gerrit-Reviewer: Shu Ming <[email protected]>
Gerrit-Reviewer: Xu He Jie <[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

Reply via email to