)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"5a260945a678f3270c2246b8ac08c4bb62364d5c","unresolved":false,"context_lines":[{"line_number":11,"context_line":"used is \u0027i6300esb\u0027, and the action is based"},{"line_number":12,"context_line":"on the \u0027hw_watchdog_action\u0027 extra spec of the flavor,"},{"line_number":13,"context_line":"or the \u0027hw_watchdog_action\u0027 property of the image"},{"line_number":14,"context_line":"(which will override the flavor value if present)."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Implements: blueprint libvirt-watchdog"},{"line_number":17,"context_line":"DocImpact"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"AAAAU3%2F%2Fza0%3D","line":14,"updated":"2014-02-24 18:02:58.000000000","message":"this part is no longer relevant","commit_id":"fa2ea975605e821e25012edd847746a6af9108ab"}],"nova/db/watchdog_actions.py":[{"author":{"_account_id":4573,"name":"yunhong-jiang","email":"yunhong.jiang@intel.com","username":"yunhong-jiang"},"change_message_id":"7934eb8baef2f1bba57854c691c6f1b258b2e755","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"# the values which may be passed to libvirt"},{"line_number":19,"context_line":"RAW_WATCHDOG_ACTIONS \u003d [\u0027poweroff\u0027, \u0027reset\u0027, \u0027pause\u0027, \u0027none\u0027]"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"def is_valid_watchdog_action(val):"},{"line_number":23,"context_line":"    \"\"\"Check if the given value is a valid watchdog device parameter.\"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAUH%2F%2BBzE%3D","line":20,"updated":"2014-01-30 21:06:39.000000000","message":"Why do we need define this in the DB layer? I\u0027d assume it should be in libvirt layer?","commit_id":"693a490e9f59127f273ef1ef6f4f0ecd72d3d636"}],"nova/tests/virt/libvirt/test_libvirt.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"f47febb08b42c5a99327944e1298da9647b5a1db","unresolved":false,"context_lines":[{"line_number":1289,"context_line":"        self.assertIsInstance(cfg.devices[7],"},{"line_number":1290,"context_line":"                              vconfig.LibvirtConfigGuestWatchdog)"},{"line_number":1291,"context_line":""},{"line_number":1292,"context_line":"        self.assertEqual(cfg.devices[7].action, \"none\")"},{"line_number":1293,"context_line":""},{"line_number":1294,"context_line":"    def test_unsupported_video_driver_through_image_meta(self):"},{"line_number":1295,"context_line":"        self.flags(virt_type\u003d\u0027kvm\u0027, group\u003d\u0027libvirt\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAU3%2F%2FzZ4%3D","line":1292,"updated":"2014-02-24 18:04:44.000000000","message":"expected value should be the first","commit_id":"fa2ea975605e821e25012edd847746a6af9108ab"}],"nova/virt/libvirt/config.py":[{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"a979982f335e72fd05aabea512881bee1437900c","unresolved":false,"context_lines":[{"line_number":1070,"context_line":"                                                         **kwargs)"},{"line_number":1071,"context_line":""},{"line_number":1072,"context_line":"        self.model \u003d \u0027i6300esb\u0027"},{"line_number":1073,"context_line":"        self.action \u003d \u0027disabled\u0027"},{"line_number":1074,"context_line":""},{"line_number":1075,"context_line":"    def format_dom(self):"},{"line_number":1076,"context_line":"        dev \u003d super(LibvirtConfigGuestWatchdog, self).format_dom()"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAUH%2F%2FSGE%3D","line":1073,"updated":"2014-01-15 14:32:15.000000000","message":"\u0027disabled\u0027 isn\u0027t a valid libvirt watchdog action - that\u0027s terminology from the nova watchdog instance setting. \u0027reset\u0027 is the libvirt default, so we should use that here too.","commit_id":"f41190b403790489e6a36a137bba78917b30ccd5"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"a979982f335e72fd05aabea512881bee1437900c","unresolved":false,"context_lines":[{"line_number":1076,"context_line":"        dev \u003d super(LibvirtConfigGuestWatchdog, self).format_dom()"},{"line_number":1077,"context_line":""},{"line_number":1078,"context_line":"        dev.set(\u0027model\u0027, self.model)"},{"line_number":1079,"context_line":"        if self.action is not None and self.action !\u003d \u0027disabled\u0027:"},{"line_number":1080,"context_line":"            dev.set(\u0027action\u0027, self.action)"},{"line_number":1081,"context_line":""},{"line_number":1082,"context_line":"        return dev"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAUH%2F%2FSF4%3D","line":1079,"updated":"2014-01-15 14:32:15.000000000","message":"This conditional can just be removed, and the attribute set unconditionally since it won\u0027t be None.","commit_id":"f41190b403790489e6a36a137bba78917b30ccd5"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"a979982f335e72fd05aabea512881bee1437900c","unresolved":false,"context_lines":[{"line_number":1079,"context_line":"        if self.action is not None and self.action !\u003d \u0027disabled\u0027:"},{"line_number":1080,"context_line":"            dev.set(\u0027action\u0027, self.action)"},{"line_number":1081,"context_line":""},{"line_number":1082,"context_line":"        return dev"},{"line_number":1083,"context_line":""},{"line_number":1084,"context_line":""},{"line_number":1085,"context_line":"class LibvirtConfigGuest(LibvirtConfigObject):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAUH%2F%2FSFw%3D","line":1082,"updated":"2014-01-15 14:32:15.000000000","message":"You need a addition to test_libvirt_config.py for this change","commit_id":"f41190b403790489e6a36a137bba78917b30ccd5"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"a979982f335e72fd05aabea512881bee1437900c","unresolved":false,"context_lines":[{"line_number":3247,"context_line":"        if inst_type[\u0027watchdog_action\u0027] !\u003d \u0027disabled\u0027:"},{"line_number":3248,"context_line":"            bark \u003d vconfig.LibvirtConfigGuestWatchdog()"},{"line_number":3249,"context_line":"            bark.action \u003d inst_type[\u0027watchdog_action\u0027]"},{"line_number":3250,"context_line":"            guest.add_device(bark)"},{"line_number":3251,"context_line":""},{"line_number":3252,"context_line":"        return guest"},{"line_number":3253,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAUH%2F%2FSFI%3D","line":3250,"updated":"2014-01-15 14:32:15.000000000","message":"You need an addition to test_libvirt.py to validate this in some way. Also we shouldn\u0027t blindly pass through the \u0027inst_type\u0027 value - we should validate that it is one of the permitted settings before setting the action attribute.\n\nPerhaps have a shared code module somewhere in Nova tree which has constants for the permitted watchdog settings and a method to validate them that all drivers / code can use.","commit_id":"f41190b403790489e6a36a137bba78917b30ccd5"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"1030289db8249b2c2cb8a68f927447d8f8d7e68e","unresolved":false,"context_lines":[{"line_number":3253,"context_line":"                                                       \u0027disabled\u0027)"},{"line_number":3254,"context_line":"        if (image_meta is not None and"},{"line_number":3255,"context_line":"                image_meta.get(\u0027properties\u0027, {}).get(\u0027watchdog:action\u0027)):"},{"line_number":3256,"context_line":"            watchdog_action \u003d image_meta[\u0027properties\u0027][\u0027watchdog:action\u0027]"},{"line_number":3257,"context_line":""},{"line_number":3258,"context_line":"        # NB(sross): currently only actually supported by KVM/QEmu"},{"line_number":3259,"context_line":"        if watchdog_action !\u003d \u0027disabled\u0027:"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUH%2F%2Bo2o%3D","line":3256,"updated":"2014-01-23 17:05:39.000000000","message":"I think the property name should probably be  \u0027hw_watchdog_action\u0027  since we have standardized on \u0027hw_\u0027 prefix for these kind of tunables.","commit_id":"c61696e56a8558974221d8de0d3b66476398d47c"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"9688bf60c6ed02663f1a7236b1600ed8f1f84583","unresolved":false,"context_lines":[{"line_number":68,"context_line":"from nova.compute import utils as compute_utils"},{"line_number":69,"context_line":"from nova.compute import vm_mode"},{"line_number":70,"context_line":"from nova import context as nova_context"},{"line_number":71,"context_line":"import nova.virt.libvirt.watchdog_actions as watchdog_actions"},{"line_number":72,"context_line":"from nova import exception"},{"line_number":73,"context_line":"from nova.image import glance"},{"line_number":74,"context_line":"from nova.objects import instance as instance_obj"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAUX%2F%2F1S4%3D","line":71,"updated":"2014-02-10 14:36:19.000000000","message":"Keep a consistent import style, eg\n\nfrom nova.virt import wachdog_actions","commit_id":"ccddedfdc10b96f94a958bf616933046386aa007"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"73f16f97376e39d4f5742f1d7041ffde6196b57d","unresolved":false,"context_lines":[{"line_number":3306,"context_line":"                raise exception.PciDeviceUnsupportedHypervisor("},{"line_number":3307,"context_line":"                    type\u003dCONF.libvirt.virt_type)"},{"line_number":3308,"context_line":""},{"line_number":3309,"context_line":"        watchdog_action \u003d flavor.extra_specs.get(\u0027hw_watchdog_action\u0027,"},{"line_number":3310,"context_line":"                                                 \u0027disabled\u0027)"},{"line_number":3311,"context_line":"        if (image_meta is not None and"},{"line_number":3312,"context_line":"                image_meta.get(\u0027properties\u0027, {}).get(\u0027hw_watchdog_action\u0027)):"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAUX%2F%2BhpM%3D","line":3309,"updated":"2014-02-20 17:51:45.000000000","message":"why not use instance metadata or even instance system metadata? i am not sure that this should be part of a flavor. is this something that one would want run on specific instances? any thoughts?","commit_id":"7520444b86f2cb9855d910549d3952340ded3624"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"e4ac7969c1e71f33f922575177439c7f3eb80ef0","unresolved":false,"context_lines":[{"line_number":3306,"context_line":"                raise exception.PciDeviceUnsupportedHypervisor("},{"line_number":3307,"context_line":"                    type\u003dCONF.libvirt.virt_type)"},{"line_number":3308,"context_line":""},{"line_number":3309,"context_line":"        watchdog_action \u003d flavor.extra_specs.get(\u0027hw_watchdog_action\u0027,"},{"line_number":3310,"context_line":"                                                 \u0027disabled\u0027)"},{"line_number":3311,"context_line":"        if (image_meta is not None and"},{"line_number":3312,"context_line":"                image_meta.get(\u0027properties\u0027, {}).get(\u0027hw_watchdog_action\u0027)):"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAU3%2F%2F2EU%3D","line":3309,"in_reply_to":"AAAAUX%2F%2BavI%3D","updated":"2014-02-24 14:41:50.000000000","message":"If this is related to specific guest then my not load it as a image metadata? that is actually done below on line 3313. this will remove all of the flavor usage.","commit_id":"7520444b86f2cb9855d910549d3952340ded3624"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"7e45cf7f504ac8b5e63f2673804afce955b96005","unresolved":false,"context_lines":[{"line_number":3306,"context_line":"                raise exception.PciDeviceUnsupportedHypervisor("},{"line_number":3307,"context_line":"                    type\u003dCONF.libvirt.virt_type)"},{"line_number":3308,"context_line":""},{"line_number":3309,"context_line":"        watchdog_action \u003d flavor.extra_specs.get(\u0027hw_watchdog_action\u0027,"},{"line_number":3310,"context_line":"                                                 \u0027disabled\u0027)"},{"line_number":3311,"context_line":"        if (image_meta is not None and"},{"line_number":3312,"context_line":"                image_meta.get(\u0027properties\u0027, {}).get(\u0027hw_watchdog_action\u0027)):"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAUX%2F%2BavI%3D","line":3309,"in_reply_to":"AAAAUX%2F%2Bb5A%3D","updated":"2014-02-21 09:22:36.000000000","message":"This code isn\u0027t forcing admins to use flavours for enabling the watchdog - it is just giving them the option to do so if they so wish. If the admin hasn\u0027t enabled it on a flavour the user can still ask for it against the image property.\n\nThe reason to not unconditionally enable it is that Windows device manager will complain to the user if they have new hardware enabled but not driver present. So you don\u0027t want to force windows users to have this new device. \n\nNo guest hardware config sttubgs should ever be done at the compute node level. This is not a libvirt specific concept - a watchdog is a standard hardware that any hypervisor technology is logically capable of supporting.","commit_id":"7520444b86f2cb9855d910549d3952340ded3624"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"2ff65b8c902f158300541f36e04127f3a6a049f4","unresolved":false,"context_lines":[{"line_number":3306,"context_line":"                raise exception.PciDeviceUnsupportedHypervisor("},{"line_number":3307,"context_line":"                    type\u003dCONF.libvirt.virt_type)"},{"line_number":3308,"context_line":""},{"line_number":3309,"context_line":"        watchdog_action \u003d flavor.extra_specs.get(\u0027hw_watchdog_action\u0027,"},{"line_number":3310,"context_line":"                                                 \u0027disabled\u0027)"},{"line_number":3311,"context_line":"        if (image_meta is not None and"},{"line_number":3312,"context_line":"                image_meta.get(\u0027properties\u0027, {}).get(\u0027hw_watchdog_action\u0027)):"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAUX%2F%2Bb5A%3D","line":3309,"in_reply_to":"AAAAUX%2F%2BhKQ%3D","updated":"2014-02-21 06:14:57.000000000","message":"The problem with flavors is that the admin/user will need to create extra flavors for a feature like this. Do they want to update all of their flavors to have this? Or have flavors with and without it. So we will have for example a tiny flavor and a tiy flavor with a watchdog feature.\nIn addition to this the user is not aware that they are running libvirt and this is something that is really specific for libvirt. Will the scheduler take this into account and only select a host that runs libvirt - say if you have a hbrid back?\nHave you thought about having this as a configuration variable on the compute node? I am not familiar with the watchdog and not sure what the overhead is for it. If this is marginal then why not run it for all instances that have the criteria defined below?","commit_id":"7520444b86f2cb9855d910549d3952340ded3624"},{"author":{"_account_id":7677,"name":"Solly Ross","email":"sross@redhat.com","username":"sross"},"change_message_id":"3e8865501ac60b7edf3bd6c008737624034ea9e5","unresolved":false,"context_lines":[{"line_number":3306,"context_line":"                raise exception.PciDeviceUnsupportedHypervisor("},{"line_number":3307,"context_line":"                    type\u003dCONF.libvirt.virt_type)"},{"line_number":3308,"context_line":""},{"line_number":3309,"context_line":"        watchdog_action \u003d flavor.extra_specs.get(\u0027hw_watchdog_action\u0027,"},{"line_number":3310,"context_line":"                                                 \u0027disabled\u0027)"},{"line_number":3311,"context_line":"        if (image_meta is not None and"},{"line_number":3312,"context_line":"                image_meta.get(\u0027properties\u0027, {}).get(\u0027hw_watchdog_action\u0027)):"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAUX%2F%2BhKQ%3D","line":3309,"in_reply_to":"AAAAUX%2F%2BhpM%3D","updated":"2014-02-20 18:51:16.000000000","message":"I actually think that the current options make a lot of sense: either you want to apply it across an entire tier of servers (flavor extra sepcs) or any server that uses a particular OS (image metadata).  I\u0027m not sure that you\u0027d really want to apply it to only specific instances.  What do other people think?","commit_id":"7520444b86f2cb9855d910549d3952340ded3624"},{"author":{"_account_id":4573,"name":"yunhong-jiang","email":"yunhong.jiang@intel.com","username":"yunhong-jiang"},"change_message_id":"a79638b5c2ae15948cd93850cfde51d4a91b42e9","unresolved":false,"context_lines":[{"line_number":3311,"context_line":"                image_meta.get(\u0027properties\u0027, {}).get(\u0027hw_watchdog_action\u0027)):"},{"line_number":3312,"context_line":"            watchdog_action \u003d image_meta[\u0027properties\u0027][\u0027hw_watchdog_action\u0027]"},{"line_number":3313,"context_line":""},{"line_number":3314,"context_line":"        # NB(sross): currently only actually supported by KVM/QEmu"},{"line_number":3315,"context_line":"        if watchdog_action !\u003d \u0027disabled\u0027:"},{"line_number":3316,"context_line":"            if watchdog_actions.is_valid_watchdog_action(watchdog_action):"},{"line_number":3317,"context_line":"                bark \u003d vconfig.LibvirtConfigGuestWatchdog()"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAU3%2F%2Fu2c%3D","line":3314,"updated":"2014-02-25 02:19:29.000000000","message":"Should we raise a exception if user request watchdog while the hypervisor does not support it, like what did in the above check of PCI device?","commit_id":"5865054c0e60f088c9e6b122f7ce2ed7bbe73163"},{"author":{"_account_id":4573,"name":"yunhong-jiang","email":"yunhong.jiang@intel.com","username":"yunhong-jiang"},"change_message_id":"91a64213b3eb571d3dab46acd631f69cb087f756","unresolved":false,"context_lines":[{"line_number":3311,"context_line":"                image_meta.get(\u0027properties\u0027, {}).get(\u0027hw_watchdog_action\u0027)):"},{"line_number":3312,"context_line":"            watchdog_action \u003d image_meta[\u0027properties\u0027][\u0027hw_watchdog_action\u0027]"},{"line_number":3313,"context_line":""},{"line_number":3314,"context_line":"        # NB(sross): currently only actually supported by KVM/QEmu"},{"line_number":3315,"context_line":"        if watchdog_action !\u003d \u0027disabled\u0027:"},{"line_number":3316,"context_line":"            if watchdog_actions.is_valid_watchdog_action(watchdog_action):"},{"line_number":3317,"context_line":"                bark \u003d vconfig.LibvirtConfigGuestWatchdog()"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAU3%2F%2FlxQ%3D","line":3314,"in_reply_to":"AAAAU3%2F%2Fl6A%3D","updated":"2014-02-25 19:17:11.000000000","message":"But at least a exception here is needed and helpful, right?","commit_id":"5865054c0e60f088c9e6b122f7ce2ed7bbe73163"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"2bf7cb91d34bfdc1511470ad43a7991c9d44cafe","unresolved":false,"context_lines":[{"line_number":3311,"context_line":"                image_meta.get(\u0027properties\u0027, {}).get(\u0027hw_watchdog_action\u0027)):"},{"line_number":3312,"context_line":"            watchdog_action \u003d image_meta[\u0027properties\u0027][\u0027hw_watchdog_action\u0027]"},{"line_number":3313,"context_line":""},{"line_number":3314,"context_line":"        # NB(sross): currently only actually supported by KVM/QEmu"},{"line_number":3315,"context_line":"        if watchdog_action !\u003d \u0027disabled\u0027:"},{"line_number":3316,"context_line":"            if watchdog_actions.is_valid_watchdog_action(watchdog_action):"},{"line_number":3317,"context_line":"                bark \u003d vconfig.LibvirtConfigGuestWatchdog()"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAU3%2F%2Fl6A%3D","line":3314,"in_reply_to":"AAAAU3%2F%2FnIE%3D","updated":"2014-02-25 19:03:24.000000000","message":"I don\u0027t think there\u0027s any new problem introduced here by the watchdog property. The point about scheduler behaviour in case where nova has a mixture of different hypervisors deployed is going to break pretty much all existing use of the \u0027hw_XXXX\u0027 image properties. eg if you set  hw_disk_bus\u003dvirtio  that\u0027s not going to work if you have a mixture of lxc + qemu hypervisors.\n\nI don\u0027t think we need to hold up the watchdog patch because of this existing bug since it isn\u0027t making the problem any worse IMHO.\n\nWhat\u0027s needed is probably a way to set a hypervisor type image property and have the scheduler honour that. Then anytime you set one of the hw_xxxx properties you should also set the hypervisor type property.\n\nSomeone should file a bug about this problem...","commit_id":"5865054c0e60f088c9e6b122f7ce2ed7bbe73163"},{"author":{"_account_id":7677,"name":"Solly Ross","email":"sross@redhat.com","username":"sross"},"change_message_id":"ad6a280f440e949098f2a608a9b30a747a33e05e","unresolved":false,"context_lines":[{"line_number":3311,"context_line":"                image_meta.get(\u0027properties\u0027, {}).get(\u0027hw_watchdog_action\u0027)):"},{"line_number":3312,"context_line":"            watchdog_action \u003d image_meta[\u0027properties\u0027][\u0027hw_watchdog_action\u0027]"},{"line_number":3313,"context_line":""},{"line_number":3314,"context_line":"        # NB(sross): currently only actually supported by KVM/QEmu"},{"line_number":3315,"context_line":"        if watchdog_action !\u003d \u0027disabled\u0027:"},{"line_number":3316,"context_line":"            if watchdog_actions.is_valid_watchdog_action(watchdog_action):"},{"line_number":3317,"context_line":"                bark \u003d vconfig.LibvirtConfigGuestWatchdog()"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAU3%2F%2FnAk%3D","line":3314,"in_reply_to":"AAAAU3%2F%2FnIE%3D","updated":"2014-02-25 17:29:40.000000000","message":"That\u0027s a good point about the scheduler.  Let\u0027s see what Dan has to say, though.","commit_id":"5865054c0e60f088c9e6b122f7ce2ed7bbe73163"},{"author":{"_account_id":4573,"name":"yunhong-jiang","email":"yunhong.jiang@intel.com","username":"yunhong-jiang"},"change_message_id":"627be4c9371d3cc53adf0391ad9e20911d60b5ce","unresolved":false,"context_lines":[{"line_number":3311,"context_line":"                image_meta.get(\u0027properties\u0027, {}).get(\u0027hw_watchdog_action\u0027)):"},{"line_number":3312,"context_line":"            watchdog_action \u003d image_meta[\u0027properties\u0027][\u0027hw_watchdog_action\u0027]"},{"line_number":3313,"context_line":""},{"line_number":3314,"context_line":"        # NB(sross): currently only actually supported by KVM/QEmu"},{"line_number":3315,"context_line":"        if watchdog_action !\u003d \u0027disabled\u0027:"},{"line_number":3316,"context_line":"            if watchdog_actions.is_valid_watchdog_action(watchdog_action):"},{"line_number":3317,"context_line":"                bark \u003d vconfig.LibvirtConfigGuestWatchdog()"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAU3%2F%2FnIE%3D","line":3314,"in_reply_to":"AAAAU3%2F%2FnU8%3D","updated":"2014-02-25 17:17:33.000000000","message":"I think the key thing is, the scheduler has no idea of the instance\u0027s watch dog requirement and may schedule the server to arbitrary host and will cause silent error in such situation.\n\nseems check hypervisor type is quite common in this file :) Of course, let\u0027s wait Daniel\u0027s input.","commit_id":"5865054c0e60f088c9e6b122f7ce2ed7bbe73163"},{"author":{"_account_id":7677,"name":"Solly Ross","email":"sross@redhat.com","username":"sross"},"change_message_id":"e968882429075162cf2512111626899d0f9b1ad2","unresolved":false,"context_lines":[{"line_number":3311,"context_line":"                image_meta.get(\u0027properties\u0027, {}).get(\u0027hw_watchdog_action\u0027)):"},{"line_number":3312,"context_line":"            watchdog_action \u003d image_meta[\u0027properties\u0027][\u0027hw_watchdog_action\u0027]"},{"line_number":3313,"context_line":""},{"line_number":3314,"context_line":"        # NB(sross): currently only actually supported by KVM/QEmu"},{"line_number":3315,"context_line":"        if watchdog_action !\u003d \u0027disabled\u0027:"},{"line_number":3316,"context_line":"            if watchdog_actions.is_valid_watchdog_action(watchdog_action):"},{"line_number":3317,"context_line":"                bark \u003d vconfig.LibvirtConfigGuestWatchdog()"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAU3%2F%2FnU8%3D","line":3314,"in_reply_to":"AAAAU3%2F%2Fu2c%3D","updated":"2014-02-25 16:59:30.000000000","message":"interesting idea.  On the one hand, the point of libvirt is so that we don\u0027t have to know about the underlying hypervisor.  On the other hand, we do explicitly check the hypervisor above (as you noted), so there is precedent.  What do other people think?","commit_id":"5865054c0e60f088c9e6b122f7ce2ed7bbe73163"},{"author":{"_account_id":4573,"name":"yunhong-jiang","email":"yunhong.jiang@intel.com","username":"yunhong-jiang"},"change_message_id":"a79638b5c2ae15948cd93850cfde51d4a91b42e9","unresolved":false,"context_lines":[{"line_number":3312,"context_line":"            watchdog_action \u003d image_meta[\u0027properties\u0027][\u0027hw_watchdog_action\u0027]"},{"line_number":3313,"context_line":""},{"line_number":3314,"context_line":"        # NB(sross): currently only actually supported by KVM/QEmu"},{"line_number":3315,"context_line":"        if watchdog_action !\u003d \u0027disabled\u0027:"},{"line_number":3316,"context_line":"            if watchdog_actions.is_valid_watchdog_action(watchdog_action):"},{"line_number":3317,"context_line":"                bark \u003d vconfig.LibvirtConfigGuestWatchdog()"},{"line_number":3318,"context_line":"                bark.action \u003d watchdog_action"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAU3%2F%2Fu3A%3D","line":3315,"updated":"2014-02-25 02:19:29.000000000","message":"I\u0027m not sure if we really need set watchdog_action and then check it. I think possibly it\u0027s left from the flavor check, which is removed in patchset 9.\n\n Would it be better to:\n\n        if (image_meta is not None and\n               image_meta.get(\u0027properties\u0027, {}).get(\u0027hw_watchdog_action\u0027)):\n                      watchdog_action \u003d image_meta[\u0027properties\u0027][\u0027hw_watchdog_action\u0027]\n                  if watchdog_actions.is_valid_watchdog_action(watchdog_action):\n \t                bark \u003d vconfig.LibvirtConfigGuestWatchdog()\n \t                bark.action \u003d watchdog_action\n \t                guest.add_device(bark)\n \t            else:\n \t                raise exception.InvalidWatchdogAction(action\u003dwatchdog_action)\n\nThus we only do it when the watchdog_action is specified in the image metadata? Did I miss anything?","commit_id":"5865054c0e60f088c9e6b122f7ce2ed7bbe73163"},{"author":{"_account_id":4573,"name":"yunhong-jiang","email":"yunhong.jiang@intel.com","username":"yunhong-jiang"},"change_message_id":"627be4c9371d3cc53adf0391ad9e20911d60b5ce","unresolved":false,"context_lines":[{"line_number":3312,"context_line":"            watchdog_action \u003d image_meta[\u0027properties\u0027][\u0027hw_watchdog_action\u0027]"},{"line_number":3313,"context_line":""},{"line_number":3314,"context_line":"        # NB(sross): currently only actually supported by KVM/QEmu"},{"line_number":3315,"context_line":"        if watchdog_action !\u003d \u0027disabled\u0027:"},{"line_number":3316,"context_line":"            if watchdog_actions.is_valid_watchdog_action(watchdog_action):"},{"line_number":3317,"context_line":"                bark \u003d vconfig.LibvirtConfigGuestWatchdog()"},{"line_number":3318,"context_line":"                bark.action \u003d watchdog_action"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAU3%2F%2FnLE%3D","line":3315,"in_reply_to":"AAAAU3%2F%2FnVE%3D","updated":"2014-02-25 17:17:33.000000000","message":"Aha, I didn\u0027t notice the followed patch, then it make sense otherwise it\u0027s really meaningless :)","commit_id":"5865054c0e60f088c9e6b122f7ce2ed7bbe73163"},{"author":{"_account_id":7677,"name":"Solly Ross","email":"sross@redhat.com","username":"sross"},"change_message_id":"e968882429075162cf2512111626899d0f9b1ad2","unresolved":false,"context_lines":[{"line_number":3312,"context_line":"            watchdog_action \u003d image_meta[\u0027properties\u0027][\u0027hw_watchdog_action\u0027]"},{"line_number":3313,"context_line":""},{"line_number":3314,"context_line":"        # NB(sross): currently only actually supported by KVM/QEmu"},{"line_number":3315,"context_line":"        if watchdog_action !\u003d \u0027disabled\u0027:"},{"line_number":3316,"context_line":"            if watchdog_actions.is_valid_watchdog_action(watchdog_action):"},{"line_number":3317,"context_line":"                bark \u003d vconfig.LibvirtConfigGuestWatchdog()"},{"line_number":3318,"context_line":"                bark.action \u003d watchdog_action"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAU3%2F%2FnVE%3D","line":3315,"in_reply_to":"AAAAU3%2F%2Fu3A%3D","updated":"2014-02-25 16:59:30.000000000","message":"eh, 6 to 1, half a dozen to the other.  I don\u0027t really think it makes that much of a difference, and it makes the next patch simpler.","commit_id":"5865054c0e60f088c9e6b122f7ce2ed7bbe73163"}],"nova/virt/libvirt/watchdog_actions.py":[{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"9688bf60c6ed02663f1a7236b1600ed8f1f84583","unresolved":false,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":"def is_valid_watchdog_action(val):"},{"line_number":23,"context_line":"    \"\"\"Check if the given value is a valid watchdog device parameter.\"\"\""},{"line_number":24,"context_line":"    return val in RAW_WATCHDOG_ACTIONS"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAUX%2F%2F1TI%3D","line":24,"updated":"2014-02-10 14:36:19.000000000","message":"I know you were just asked to move this, but image properties are supposed to be portable across virt drivers.  ie \u0027hw_watchdog_action\u0027 ought to be supportable by XenAPi or VMWare, etc. So can you move this up one level, so it is \u0027nova.virt.watchdog_actions\u0027. You might need to whitelist it in the _get_virt_name method nova/hacking/checks.py if you hit pep8 problems after moving it.","commit_id":"ccddedfdc10b96f94a958bf616933046386aa007"}]}