)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"05f8d7b77fd2fa68f08c42d0f203d5e247a9c8a6","unresolved":true,"context_lines":[{"line_number":32,"context_line":"that are causing a performance penalty, or other trouble."},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"Closes-Bug: 1852437"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"Change-Id: I2ef7c5bef87bd64c087f3b136c2faac9a3865f10"},{"line_number":37,"context_line":"Signed-off-by: Kashyap Chamarthy \u003ckchamart@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"2ae403c0_2b2d9d88","line":35,"updated":"2021-02-05 14:29:56.000000000","message":"this is a new feature which requires a spec appoved or a specless blueprint as was pointed out in the bug already\nhttps://bugs.launchpad.net/nova/+bug/1852437/comments/1\n\nso at a minium you need to add the blueprint link here \nhttps://blueprints.launchpad.net/nova/+spec/allow-disabling-cpu-flags\n\nyou also need a release note declauring the new feature\nand the specless blueprint need to be accpeted and approved for the wallaby cycle before this can merge.\n\ngiven we are passed the spec approval deadline im not sure we can still accpet new blueprints at this point\nbut this need to be preensted to the nova team meeting to review it and approve it or not in anycase.","commit_id":"f5f513806bb35053168dfeeed4f545f758839444"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"0da5551582502c66a481d1274cb5dad08d9e3e83","unresolved":true,"context_lines":[{"line_number":32,"context_line":"that are causing a performance penalty, or other trouble."},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"Closes-Bug: 1852437"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"Change-Id: I2ef7c5bef87bd64c087f3b136c2faac9a3865f10"},{"line_number":37,"context_line":"Signed-off-by: Kashyap Chamarthy \u003ckchamart@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"684dba5f_ebd888db","line":35,"in_reply_to":"2ae403c0_2b2d9d88","updated":"2021-02-05 16:04:47.000000000","message":"On paperwork: Yes, I\u0027m going to link to the spec-less blueprint that I filed above more tha na year ago.  (But spec is simply overkill.)\n\nThis is a small feature *and* a bug-fix.  I\u0027ll add an explanatory note on the breaking scenario.  And yes, release note was on my mind too.","commit_id":"f5f513806bb35053168dfeeed4f545f758839444"}],"nova/conf/libvirt.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9da1a3ef70a4131c3db0e37cf2f26fa6504b69b5","unresolved":true,"context_lines":[{"line_number":587,"context_line":"ask for it."},{"line_number":588,"context_line":""},{"line_number":589,"context_line":"Further, it is also possible to explicitly enable or disable CPU flags"},{"line_number":590,"context_line":"with the \"+\" or \"-\" notation: i.e. if you specify a CPU flag with \"+\","},{"line_number":591,"context_line":"it will be enabled for the guest, \"-\" will disable it, and if neither"},{"line_number":592,"context_line":"\"+\" nor \"-\" is specified, the flag will be enabled by default.  For"},{"line_number":593,"context_line":"example, if you specify the following::"}],"source_content_type":"text/x-python","patch_set":6,"id":"e377753b_7477b503","line":590,"range":{"start_line":590,"start_character":9,"end_line":590,"end_character":12},"updated":"2021-02-15 18:44:23.000000000","message":"nit:\n\n  ``+``","commit_id":"5f3742475a0c5491194e14dd3c0c23aac8897319"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9da1a3ef70a4131c3db0e37cf2f26fa6504b69b5","unresolved":true,"context_lines":[{"line_number":587,"context_line":"ask for it."},{"line_number":588,"context_line":""},{"line_number":589,"context_line":"Further, it is also possible to explicitly enable or disable CPU flags"},{"line_number":590,"context_line":"with the \"+\" or \"-\" notation: i.e. if you specify a CPU flag with \"+\","},{"line_number":591,"context_line":"it will be enabled for the guest, \"-\" will disable it, and if neither"},{"line_number":592,"context_line":"\"+\" nor \"-\" is specified, the flag will be enabled by default.  For"},{"line_number":593,"context_line":"example, if you specify the following::"}],"source_content_type":"text/x-python","patch_set":6,"id":"f7f65074_56329f34","line":590,"range":{"start_line":590,"start_character":16,"end_line":590,"end_character":19},"updated":"2021-02-15 18:44:23.000000000","message":"nit:\n\n  ``-``\n\nditto for below","commit_id":"5f3742475a0c5491194e14dd3c0c23aac8897319"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"eac60d8655be356015a03f24df1e26693ffc4fe1","unresolved":true,"context_lines":[{"line_number":587,"context_line":"ask for it."},{"line_number":588,"context_line":""},{"line_number":589,"context_line":"Further, it is also possible to explicitly enable or disable CPU flags"},{"line_number":590,"context_line":"with the \"+\" or \"-\" notation: i.e. if you specify a CPU flag with \"+\","},{"line_number":591,"context_line":"it will be enabled for the guest, \"-\" will disable it, and if neither"},{"line_number":592,"context_line":"\"+\" nor \"-\" is specified, the flag will be enabled by default.  For"},{"line_number":593,"context_line":"example, if you specify the following::"}],"source_content_type":"text/x-python","patch_set":6,"id":"244d46bd_07dacc77","line":590,"range":{"start_line":590,"start_character":16,"end_line":590,"end_character":19},"in_reply_to":"f7f65074_56329f34","updated":"2021-02-16 12:46:03.000000000","message":"Will fix both.","commit_id":"5f3742475a0c5491194e14dd3c0c23aac8897319"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"b9c3036b2870231566bc4de780ceed49a1bb81e6","unresolved":true,"context_lines":[{"line_number":555,"context_line":"        default\u003d[],"},{"line_number":556,"context_line":"        help\u003d\"\"\""},{"line_number":557,"context_line":"This allows specifying granular CPU feature flags when configuring CPU"},{"line_number":558,"context_line":"models.  For example, to explicitly specify the ``pcid``"},{"line_number":559,"context_line":"(Process-Context ID, an Intel processor feature -- which is now required"},{"line_number":560,"context_line":"to address the guest performance degradation as a result of applying the"},{"line_number":561,"context_line":"\"Meltdown\" CVE fixes to certain Intel CPU models) flag to the"}],"source_content_type":"text/x-python","patch_set":7,"id":"76e4db6a_9bbc9cd6","line":558,"updated":"2021-02-17 11:29:37.000000000","message":"nit: this would read better if the whole doc text for this option was refactored. Now it feels like this last para explaining + and - was grafted on (which it was). I\u0027d start straight away here wit the + and - bit, so something like:\n\n\u003e Use ``+\u003cflag\u003e`` or just ``\u003cflag\u003e`` to add the flag to the guest CPU, and ``-\u003cflag\u003e\u003e`` to explicitly remove it. For example, the following two config snippets both specify the ``pci`` flag ([..]):\n\n  [...]\n  cpu_model_extra_flags \u003d pcid\n\n  cpu_model_extra_flags \u003d +pcid","commit_id":"0d80b1b0c794e338bfb1b03b1ac4ab79360cf266"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b3cbd1a791f1c4a8a6ac5c20c0f5238deed72026","unresolved":true,"context_lines":[{"line_number":555,"context_line":"        default\u003d[],"},{"line_number":556,"context_line":"        help\u003d\"\"\""},{"line_number":557,"context_line":"This allows specifying granular CPU feature flags when configuring CPU"},{"line_number":558,"context_line":"models.  For example, to explicitly specify the ``pcid``"},{"line_number":559,"context_line":"(Process-Context ID, an Intel processor feature -- which is now required"},{"line_number":560,"context_line":"to address the guest performance degradation as a result of applying the"},{"line_number":561,"context_line":"\"Meltdown\" CVE fixes to certain Intel CPU models) flag to the"}],"source_content_type":"text/x-python","patch_set":7,"id":"e8436a8c_12fe07c2","line":558,"in_reply_to":"76e4db6a_9bbc9cd6","updated":"2021-02-17 11:56:22.000000000","message":"Good call","commit_id":"0d80b1b0c794e338bfb1b03b1ac4ab79360cf266"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"433db5542611c2169feb87e2f4185be2d9fa9971","unresolved":true,"context_lines":[{"line_number":555,"context_line":"        default\u003d[],"},{"line_number":556,"context_line":"        help\u003d\"\"\""},{"line_number":557,"context_line":"This allows specifying granular CPU feature flags when configuring CPU"},{"line_number":558,"context_line":"models.  For example, to explicitly specify the ``pcid``"},{"line_number":559,"context_line":"(Process-Context ID, an Intel processor feature -- which is now required"},{"line_number":560,"context_line":"to address the guest performance degradation as a result of applying the"},{"line_number":561,"context_line":"\"Meltdown\" CVE fixes to certain Intel CPU models) flag to the"}],"source_content_type":"text/x-python","patch_set":7,"id":"03cd2662_6a56c4ab","line":558,"in_reply_to":"e8436a8c_12fe07c2","updated":"2021-02-17 12:42:49.000000000","message":"Right; can rewrite it.  I\u0027m all for more clarity.","commit_id":"0d80b1b0c794e338bfb1b03b1ac4ab79360cf266"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"dd4f86d48c6f8c9aef9b21b620d7f3b5f8c19988","unresolved":true,"context_lines":[{"line_number":548,"context_line":"    hardware."},{"line_number":549,"context_line":"\"\"\"),"},{"line_number":550,"context_line":"    cfg.ListOpt("},{"line_number":551,"context_line":"        \u0027cpu_model_extra_flags\u0027,"},{"line_number":552,"context_line":"        item_type\u003dtypes.String("},{"line_number":553,"context_line":"            ignore_case\u003dTrue,"},{"line_number":554,"context_line":"        ),"}],"source_content_type":"text/x-python","patch_set":8,"id":"887401c2_181dd8ad","line":551,"updated":"2021-02-18 18:30:08.000000000","message":"The more I think about this, the less I like doing this vs introducing new options. The prefixes are far too special flower for my liking. What is the opposition to \u0027cpu_model_extra_enabled_flags\u0027 and \u0027cpu_model_extra_disabled_flags\u0027? Most of this is managed by tooling so it\u0027s not worse for our users, but it\u0027s objectively easier for us to reason about.","commit_id":"7597c4b97f0e7424583c8eb33f55c5ccf13c6873"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"86d8559497920ce6fae2f65e4a671c7f01587cb2","unresolved":true,"context_lines":[{"line_number":548,"context_line":"    hardware."},{"line_number":549,"context_line":"\"\"\"),"},{"line_number":550,"context_line":"    cfg.ListOpt("},{"line_number":551,"context_line":"        \u0027cpu_model_extra_flags\u0027,"},{"line_number":552,"context_line":"        item_type\u003dtypes.String("},{"line_number":553,"context_line":"            ignore_case\u003dTrue,"},{"line_number":554,"context_line":"        ),"}],"source_content_type":"text/x-python","patch_set":8,"id":"53e6bbba_a04f38c3","line":551,"in_reply_to":"887401c2_181dd8ad","updated":"2021-02-19 11:20:48.000000000","message":"My objection against two flags: needlessly introducing more config options (even if they\u0027re handled by management tooling), the verbosity, more work for higher-level tooling without much benefit, and not to mention the deprecation dance.\n\nI don\u0027t think the +/- usage is too special-flowery.  It feels \"new\", but once you get past that, you can see how it is intuitive, clear, short, and reusing an existing flag in a meaningful way.  You can\u0027t misunderstand it even if you try to.","commit_id":"7597c4b97f0e7424583c8eb33f55c5ccf13c6873"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"b6a23471e3ae359efe2dc74ea608cccd44011559","unresolved":true,"context_lines":[{"line_number":573,"context_line":"The CPU flags are also case-insensitve.  As can be expected, in the"},{"line_number":574,"context_line":"following example, the ``pdpe1gb`` flag will be disabled, Intel ``vmx``"},{"line_number":575,"context_line":"and ``pcid`` (an Intel processor feature that is now required to address"},{"line_number":576,"context_line":"the guest performance degradation as a result of applying the \"Meltdown\""},{"line_number":577,"context_line":"CVE fixes to certain Intel CPU models) will be enabled for the guest::"},{"line_number":578,"context_line":""},{"line_number":579,"context_line":"    [libvirt]"}],"source_content_type":"text/x-python","patch_set":8,"id":"1895a5c5_44804050","line":576,"updated":"2021-02-18 15:06:42.000000000","message":"nit: I\u0027m not sure this explanation is relevant for config option doctext.","commit_id":"7597c4b97f0e7424583c8eb33f55c5ccf13c6873"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"dd4f86d48c6f8c9aef9b21b620d7f3b5f8c19988","unresolved":true,"context_lines":[{"line_number":573,"context_line":"The CPU flags are also case-insensitve.  As can be expected, in the"},{"line_number":574,"context_line":"following example, the ``pdpe1gb`` flag will be disabled, Intel ``vmx``"},{"line_number":575,"context_line":"and ``pcid`` (an Intel processor feature that is now required to address"},{"line_number":576,"context_line":"the guest performance degradation as a result of applying the \"Meltdown\""},{"line_number":577,"context_line":"CVE fixes to certain Intel CPU models) will be enabled for the guest::"},{"line_number":578,"context_line":""},{"line_number":579,"context_line":"    [libvirt]"}],"source_content_type":"text/x-python","patch_set":8,"id":"c02285f2_e3777e4e","line":576,"in_reply_to":"1895a5c5_44804050","updated":"2021-02-18 18:30:08.000000000","message":"+1","commit_id":"7597c4b97f0e7424583c8eb33f55c5ccf13c6873"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"86d8559497920ce6fae2f65e4a671c7f01587cb2","unresolved":true,"context_lines":[{"line_number":573,"context_line":"The CPU flags are also case-insensitve.  As can be expected, in the"},{"line_number":574,"context_line":"following example, the ``pdpe1gb`` flag will be disabled, Intel ``vmx``"},{"line_number":575,"context_line":"and ``pcid`` (an Intel processor feature that is now required to address"},{"line_number":576,"context_line":"the guest performance degradation as a result of applying the \"Meltdown\""},{"line_number":577,"context_line":"CVE fixes to certain Intel CPU models) will be enabled for the guest::"},{"line_number":578,"context_line":""},{"line_number":579,"context_line":"    [libvirt]"}],"source_content_type":"text/x-python","patch_set":8,"id":"eb0041ff_c987d400","line":576,"in_reply_to":"c02285f2_e3777e4e","updated":"2021-02-19 11:20:48.000000000","message":"\u003e nit: I\u0027m not sure this explanation is relevant for config option doctext.\n\nI can rephrase it now.  \n\nPast context: that explanation _was_ relevant at one point—when we first introduced the flag, we hard-coded the default choice to \"PCID\".  Refer to the commit message here[1] as to why we did it.  And later lifted[2] that restriction.\n\n[1] https://opendev.org/openstack/nova/commit/6b601b7cf6 (libvirt: Allow to specify granular CPU feature flags)\n[2] https://opendev.org/openstack/nova/commit/cc27a2007f314 (libvirt: Lift the restriction of choices for `cpu_model_extra_flags`)","commit_id":"7597c4b97f0e7424583c8eb33f55c5ccf13c6873"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5cd7b267b3b86f3585e1d99ece2c746fce65a7c1","unresolved":true,"context_lines":[{"line_number":554,"context_line":"        ),"},{"line_number":555,"context_line":"        default\u003d[],"},{"line_number":556,"context_line":"        help\u003d\"\"\""},{"line_number":557,"context_line":"To explicitly enable or disable CPU flags, use the ``+flag`` or"},{"line_number":558,"context_line":"``-flag`` notation -- the `+`` will enable CPU flag for the guest, ``-``"},{"line_number":559,"context_line":"will disable it, and if neither ``+`` nor ``-`` is specified, the flag"},{"line_number":560,"context_line":"will be enabled by default.  For example, if you specify the following"}],"source_content_type":"text/x-python","patch_set":11,"id":"6ea3b147_c21a9b31","line":557,"updated":"2021-03-04 15:45:24.000000000","message":"This is missing a summary line. Something as simple as:\n\n  Enable or disable guest CPU flags.\n\nWould do","commit_id":"d897ce2a545fd1b774d6c65017e71c016606eedf"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"9b0bb29855a4a2793b35d9e0e01d843dc24752e3","unresolved":true,"context_lines":[{"line_number":554,"context_line":"        ),"},{"line_number":555,"context_line":"        default\u003d[],"},{"line_number":556,"context_line":"        help\u003d\"\"\""},{"line_number":557,"context_line":"To explicitly enable or disable CPU flags, use the ``+flag`` or"},{"line_number":558,"context_line":"``-flag`` notation -- the `+`` will enable CPU flag for the guest, ``-``"},{"line_number":559,"context_line":"will disable it, and if neither ``+`` nor ``-`` is specified, the flag"},{"line_number":560,"context_line":"will be enabled by default.  For example, if you specify the following"}],"source_content_type":"text/x-python","patch_set":11,"id":"5068895d_ef436dee","line":557,"in_reply_to":"6ea3b147_c21a9b31","updated":"2021-03-04 16:25:02.000000000","message":"Sure; added.","commit_id":"d897ce2a545fd1b774d6c65017e71c016606eedf"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5cd7b267b3b86f3585e1d99ece2c746fce65a7c1","unresolved":true,"context_lines":[{"line_number":566,"context_line":"    cpu_models \u003d Cascadelake-Server"},{"line_number":567,"context_line":"    cpu_model_extra_flags \u003d -hle, -rtm, +ssbd, mtrr"},{"line_number":568,"context_line":""},{"line_number":569,"context_line":"Nova will disable the \"hle\" and \"rtm\" flags for the guest; and it will"},{"line_number":570,"context_line":"enable \"ssbd\" and \"mttr\" (because it was specified with neither"},{"line_number":571,"context_line":"``+`` nor ``-`` prefix)."},{"line_number":572,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"85014410_9e64ee8f","line":569,"range":{"start_line":569,"start_character":22,"end_line":569,"end_character":28},"updated":"2021-03-04 15:45:24.000000000","message":"``hle`` (the rest of the flags should be literal also)","commit_id":"d897ce2a545fd1b774d6c65017e71c016606eedf"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"9b0bb29855a4a2793b35d9e0e01d843dc24752e3","unresolved":true,"context_lines":[{"line_number":566,"context_line":"    cpu_models \u003d Cascadelake-Server"},{"line_number":567,"context_line":"    cpu_model_extra_flags \u003d -hle, -rtm, +ssbd, mtrr"},{"line_number":568,"context_line":""},{"line_number":569,"context_line":"Nova will disable the \"hle\" and \"rtm\" flags for the guest; and it will"},{"line_number":570,"context_line":"enable \"ssbd\" and \"mttr\" (because it was specified with neither"},{"line_number":571,"context_line":"``+`` nor ``-`` prefix)."},{"line_number":572,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"d32427ef_f7590eb7","line":569,"range":{"start_line":569,"start_character":22,"end_line":569,"end_character":28},"in_reply_to":"85014410_9e64ee8f","updated":"2021-03-04 16:25:02.000000000","message":"Yep; fixed.","commit_id":"d897ce2a545fd1b774d6c65017e71c016606eedf"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5cd7b267b3b86f3585e1d99ece2c746fce65a7c1","unresolved":true,"context_lines":[{"line_number":570,"context_line":"enable \"ssbd\" and \"mttr\" (because it was specified with neither"},{"line_number":571,"context_line":"``+`` nor ``-`` prefix)."},{"line_number":572,"context_line":""},{"line_number":573,"context_line":"The CPU flags are case-insensitve.  In the following example, the"},{"line_number":574,"context_line":"``pdpe1gb`` flag will be disabled for the guest; ``vmx`` and ``pcid``"},{"line_number":575,"context_line":"flags will be enabled::"},{"line_number":576,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"c55092cf_e93d4350","line":573,"range":{"start_line":573,"start_character":23,"end_line":573,"end_character":33},"updated":"2021-03-04 15:45:24.000000000","message":"insensitive","commit_id":"d897ce2a545fd1b774d6c65017e71c016606eedf"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"9b0bb29855a4a2793b35d9e0e01d843dc24752e3","unresolved":true,"context_lines":[{"line_number":570,"context_line":"enable \"ssbd\" and \"mttr\" (because it was specified with neither"},{"line_number":571,"context_line":"``+`` nor ``-`` prefix)."},{"line_number":572,"context_line":""},{"line_number":573,"context_line":"The CPU flags are case-insensitve.  In the following example, the"},{"line_number":574,"context_line":"``pdpe1gb`` flag will be disabled for the guest; ``vmx`` and ``pcid``"},{"line_number":575,"context_line":"flags will be enabled::"},{"line_number":576,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"2835cc0f_096c0c2c","line":573,"range":{"start_line":573,"start_character":23,"end_line":573,"end_character":33},"in_reply_to":"c55092cf_e93d4350","updated":"2021-03-04 16:25:02.000000000","message":"Fixed; eagle eyes :-)","commit_id":"d897ce2a545fd1b774d6c65017e71c016606eedf"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5cd7b267b3b86f3585e1d99ece2c746fce65a7c1","unresolved":true,"context_lines":[{"line_number":592,"context_line":""},{"line_number":593,"context_line":"The possible values for ``cpu_model_extra_flags`` depends on the CPU"},{"line_number":594,"context_line":"model in use.  Refer to ``/usr/share/libvirt/cpu_map.xml`` for libvirt"},{"line_number":595,"context_line":"prior to version 4.7.0 or ``/usr/share/libvirt/cpu_map/*.xml`` thereafter"},{"line_number":596,"context_line":"for possible CPU feature flags for a given CPU model."},{"line_number":597,"context_line":""},{"line_number":598,"context_line":"A special note on a particular CPU flag: ``pcid`` (an Intel processor"}],"source_content_type":"text/x-python","patch_set":11,"id":"f6169b4c_29f8a079","line":595,"updated":"2021-03-04 15:45:24.000000000","message":"Can you drop references to 4.7.0. This is below our minimum now and no longer valid.","commit_id":"d897ce2a545fd1b774d6c65017e71c016606eedf"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"9b0bb29855a4a2793b35d9e0e01d843dc24752e3","unresolved":true,"context_lines":[{"line_number":592,"context_line":""},{"line_number":593,"context_line":"The possible values for ``cpu_model_extra_flags`` depends on the CPU"},{"line_number":594,"context_line":"model in use.  Refer to ``/usr/share/libvirt/cpu_map.xml`` for libvirt"},{"line_number":595,"context_line":"prior to version 4.7.0 or ``/usr/share/libvirt/cpu_map/*.xml`` thereafter"},{"line_number":596,"context_line":"for possible CPU feature flags for a given CPU model."},{"line_number":597,"context_line":""},{"line_number":598,"context_line":"A special note on a particular CPU flag: ``pcid`` (an Intel processor"}],"source_content_type":"text/x-python","patch_set":11,"id":"ad4854de_66e6faf4","line":595,"in_reply_to":"f6169b4c_29f8a079","updated":"2021-03-04 16:25:02.000000000","message":"Yep; good catch.\n\nIn the next iter I\u0027ve also added a friendlier way of finding CPU model names: `virsh cpu-models $ARCH`.","commit_id":"d897ce2a545fd1b774d6c65017e71c016606eedf"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"05f8d7b77fd2fa68f08c42d0f203d5e247a9c8a6","unresolved":true,"context_lines":[{"line_number":8026,"context_line":"        mock_warn.assert_not_called()"},{"line_number":8027,"context_line":""},{"line_number":8028,"context_line":"    @mock.patch.object(libvirt_driver.LOG, \u0027warning\u0027)"},{"line_number":8029,"context_line":"    def test_get_guest_cpu_config_custom_flags_enabled_and_disabled(self,"},{"line_number":8030,"context_line":"            mock_warn):"},{"line_number":8031,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)"},{"line_number":8032,"context_line":"        instance_ref \u003d objects.Instance(**self.test_instance)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3ea353b6_76cc07ca","line":8029,"range":{"start_line":8029,"start_character":8,"end_line":8029,"end_character":67},"updated":"2021-02-05 14:29:56.000000000","message":"we also need test to ensure that they are removed form the traits reported for the host.","commit_id":"f5f513806bb35053168dfeeed4f545f758839444"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ed4339589ddcd00ed1b636bfd2c3b492c41d190a","unresolved":true,"context_lines":[{"line_number":1521,"context_line":"                       \"mttr\"], group\u003d\"libvirt\")"},{"line_number":1522,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)"},{"line_number":1523,"context_line":"        self.assertRaises(exception.InvalidCPUInfo,"},{"line_number":1524,"context_line":"                          drvr.init_host, \"dummyhost\")"},{"line_number":1525,"context_line":""},{"line_number":1526,"context_line":"    def test__check_cpu_compatibility_invalid_virt_type(self):"},{"line_number":1527,"context_line":"        \"\"\"Test getting CPU traits when using a virt_type that doesn\u0027t support"}],"source_content_type":"text/x-python","patch_set":5,"id":"005b9e5e_628e0229","line":1524,"updated":"2021-02-09 12:39:58.000000000","message":"hm, this test does hit the change in _check_cpu_compatibility() but it does not assert anything about the new behavior. The failed comparison is simply by the model name. Debug log from the test run:\n2021-02-09 12:32:34,369 DEBUG [nova.virt.libvirt.driver] cpu compare xml: \u003ccpu match\u003d\"exact\"\u003e\n  \u003cmodel\u003eCascadelake-Server\u003c/model\u003e\n\u003c/cpu\u003e\n\n2021-02-09 12:32:34,369 DEBUG [nova.virt.libvirt.driver] cpu compare xml: \u003ccpu match\u003d\"exact\"\u003e\n  \u003cmodel\u003ePenryn\u003c/model\u003e\n\u003c/cpu\u003e\n\n2021-02-09 12:32:34,370 ERROR [nova.virt.libvirt.driver] CPU doesn\u0027t have compatibility.\n\n0\n\nRefer to http://libvirt.org/html/libvirt-libvirt-host.html#virCPUCompareResult\n\nDoes the _check_cpu_compatibility() really checks compatbility on CPU flag level?","commit_id":"5a450a0ab3a7428aea839f38f5ef70726590ccc1"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"84e811b4a0796ff808a9a025680843c838582876","unresolved":true,"context_lines":[{"line_number":1521,"context_line":"                       \"mttr\"], group\u003d\"libvirt\")"},{"line_number":1522,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)"},{"line_number":1523,"context_line":"        self.assertRaises(exception.InvalidCPUInfo,"},{"line_number":1524,"context_line":"                          drvr.init_host, \"dummyhost\")"},{"line_number":1525,"context_line":""},{"line_number":1526,"context_line":"    def test__check_cpu_compatibility_invalid_virt_type(self):"},{"line_number":1527,"context_line":"        \"\"\"Test getting CPU traits when using a virt_type that doesn\u0027t support"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a8d201b_31b5d02e","line":1524,"in_reply_to":"005b9e5e_628e0229","updated":"2021-02-11 10:30:39.000000000","message":"I\u0027m not 100% sure if it does.  That said, as we talked on upstream IRC, it is\nnot worth it to invest much effort to rewrite the tests here —  the existing usage of _check_cpu_compatibility() is problematic in Nova\u0027s default configuratoin; and we\u0027d be replacing the usage of compareCPU() with compareHypervisorCPU() API (and baselineCPU() with baselineHypervisorCPU()).  A WIP patch initiated by chengsheng here:\n\nhttps://review.opendev.org/c/openstack/nova/+/762330/ — CPU selection with hypervisor consideration\n\n(Which is implementing an old design I wrote, which is what the above patch quotes in its commit message.  And it needs to be split out into two or three patches)\n\nNext steps:\n\n- If there are no genuine objections from any others, get this merged\n- Then, rework https://review.opendev.org/c/openstack/nova/+/762330/ on top of this patch (that allows selective-disabling of CPU flags)","commit_id":"5a450a0ab3a7428aea839f38f5ef70726590ccc1"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9da1a3ef70a4131c3db0e37cf2f26fa6504b69b5","unresolved":true,"context_lines":[{"line_number":1514,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.libvirt.Connection.compareCPU\u0027)"},{"line_number":1515,"context_line":"    def test__check_cpu_compatibility_enabled_and_disabled_flags(self,"},{"line_number":1516,"context_line":"            mocked_compare):"},{"line_number":1517,"context_line":"        mocked_compare.side_effect \u003d (2, 0)"},{"line_number":1518,"context_line":"        self.flags(cpu_mode\u003d\"custom\","},{"line_number":1519,"context_line":"                   cpu_models\u003d[\"Cascadelake-Server\"],"},{"line_number":1520,"context_line":"                   cpu_model_extra_flags \u003d [\"-hle\", \"-rtm\", \"+ssbd\","}],"source_content_type":"text/x-python","patch_set":6,"id":"f8a05641_12820636","line":1517,"updated":"2021-02-15 18:44:23.000000000","message":"An explanation of what this is doing would be helpful (I assume a non-zero return code indicates errors)","commit_id":"5f3742475a0c5491194e14dd3c0c23aac8897319"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"eac60d8655be356015a03f24df1e26693ffc4fe1","unresolved":true,"context_lines":[{"line_number":1514,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.libvirt.Connection.compareCPU\u0027)"},{"line_number":1515,"context_line":"    def test__check_cpu_compatibility_enabled_and_disabled_flags(self,"},{"line_number":1516,"context_line":"            mocked_compare):"},{"line_number":1517,"context_line":"        mocked_compare.side_effect \u003d (2, 0)"},{"line_number":1518,"context_line":"        self.flags(cpu_mode\u003d\"custom\","},{"line_number":1519,"context_line":"                   cpu_models\u003d[\"Cascadelake-Server\"],"},{"line_number":1520,"context_line":"                   cpu_model_extra_flags \u003d [\"-hle\", \"-rtm\", \"+ssbd\","}],"source_content_type":"text/x-python","patch_set":6,"id":"e52b40c8_c41fcfee","line":1517,"in_reply_to":"f8a05641_12820636","updated":"2021-02-16 12:46:03.000000000","message":"Yes, that\u0027s what I assume; frankly, I went with the existing pattern from the surrounding tests, that was introducd in this change:\n\nhttps://review.opendev.org/c/openstack/nova/+/670299 -- Add compatibility checks for CPU mode and CPU models and extra flags; Sep 16, 2019","commit_id":"5f3742475a0c5491194e14dd3c0c23aac8897319"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b3cbd1a791f1c4a8a6ac5c20c0f5238deed72026","unresolved":true,"context_lines":[{"line_number":1515,"context_line":""},{"line_number":1516,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.libvirt.Connection.compareCPU\u0027)"},{"line_number":1517,"context_line":"    def test__check_cpu_compatibility_enabled_and_disabled_flags(self,"},{"line_number":1518,"context_line":"            mocked_compare):"},{"line_number":1519,"context_line":"        mocked_compare.side_effect \u003d (2, 0)"},{"line_number":1520,"context_line":"        self.flags(cpu_mode\u003d\"custom\","},{"line_number":1521,"context_line":"                   cpu_models\u003d[\"Cascadelake-Server\"],"}],"source_content_type":"text/x-python","patch_set":7,"id":"a20f62f1_4ae1e8b1","line":1518,"updated":"2021-02-17 11:56:22.000000000","message":"style nit: can you avoid the massive hanging indent and just do:\n\n  def test__check_cpu_compatibility_enabled_and_disabled_flags(\n      self, mocked_compare,\n  ):","commit_id":"0d80b1b0c794e338bfb1b03b1ac4ab79360cf266"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b3cbd1a791f1c4a8a6ac5c20c0f5238deed72026","unresolved":true,"context_lines":[{"line_number":1520,"context_line":"        self.flags(cpu_mode\u003d\"custom\","},{"line_number":1521,"context_line":"                   cpu_models\u003d[\"Cascadelake-Server\"],"},{"line_number":1522,"context_line":"                   cpu_model_extra_flags \u003d [\"-hle\", \"-rtm\", \"+ssbd\","},{"line_number":1523,"context_line":"                       \"mttr\"], group\u003d\"libvirt\")"},{"line_number":1524,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)"},{"line_number":1525,"context_line":"        self.assertRaises(exception.InvalidCPUInfo,"},{"line_number":1526,"context_line":"                          drvr.init_host, \"dummyhost\")"}],"source_content_type":"text/x-python","patch_set":7,"id":"96550982_aad5c532","line":1523,"updated":"2021-02-17 11:56:22.000000000","message":"ditto:\n\n  self.flags(\n      cpu_mode\u003d\"custom\",\n      cpu_models\u003d[\"Cascadelake-Server\"],\n      cpu_model_extra_flags \u003d [\"-hle\", \"-rtm\", \"+ssbd\", \"mttr\"],\n      group\u003d\"libvirt\")\n\nand so on below","commit_id":"0d80b1b0c794e338bfb1b03b1ac4ab79360cf266"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b3cbd1a791f1c4a8a6ac5c20c0f5238deed72026","unresolved":true,"context_lines":[{"line_number":8055,"context_line":"                                            image_meta)"},{"line_number":8056,"context_line":"        conf \u003d drvr._get_guest_config(instance_ref,"},{"line_number":8057,"context_line":"                                      _fake_network_info(self),"},{"line_number":8058,"context_line":"                                      image_meta, disk_info)"},{"line_number":8059,"context_line":"        # features \u003d [feature.name for feature in conf.cpu.features]"},{"line_number":8060,"context_line":"        features \u003d [(feature.name, feature.policy)"},{"line_number":8061,"context_line":"                    for feature in conf.cpu.features]"}],"source_content_type":"text/x-python","patch_set":7,"id":"99c70fa7_c3d28ec8","line":8058,"updated":"2021-02-17 11:56:22.000000000","message":"Same comment about avoiding massive hanging indents as above","commit_id":"0d80b1b0c794e338bfb1b03b1ac4ab79360cf266"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b3cbd1a791f1c4a8a6ac5c20c0f5238deed72026","unresolved":true,"context_lines":[{"line_number":8056,"context_line":"        conf \u003d drvr._get_guest_config(instance_ref,"},{"line_number":8057,"context_line":"                                      _fake_network_info(self),"},{"line_number":8058,"context_line":"                                      image_meta, disk_info)"},{"line_number":8059,"context_line":"        # features \u003d [feature.name for feature in conf.cpu.features]"},{"line_number":8060,"context_line":"        features \u003d [(feature.name, feature.policy)"},{"line_number":8061,"context_line":"                    for feature in conf.cpu.features]"},{"line_number":8062,"context_line":"        self.assertIsInstance(conf.cpu,"}],"source_content_type":"text/x-python","patch_set":7,"id":"4575071c_5267a340","line":8059,"updated":"2021-02-17 11:56:22.000000000","message":"this should have been dropped?","commit_id":"0d80b1b0c794e338bfb1b03b1ac4ab79360cf266"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"433db5542611c2169feb87e2f4185be2d9fa9971","unresolved":true,"context_lines":[{"line_number":8056,"context_line":"        conf \u003d drvr._get_guest_config(instance_ref,"},{"line_number":8057,"context_line":"                                      _fake_network_info(self),"},{"line_number":8058,"context_line":"                                      image_meta, disk_info)"},{"line_number":8059,"context_line":"        # features \u003d [feature.name for feature in conf.cpu.features]"},{"line_number":8060,"context_line":"        features \u003d [(feature.name, feature.policy)"},{"line_number":8061,"context_line":"                    for feature in conf.cpu.features]"},{"line_number":8062,"context_line":"        self.assertIsInstance(conf.cpu,"}],"source_content_type":"text/x-python","patch_set":7,"id":"bc2005a6_7dc3edc6","line":8059,"in_reply_to":"4575071c_5267a340","updated":"2021-02-17 12:42:49.000000000","message":"Yes","commit_id":"0d80b1b0c794e338bfb1b03b1ac4ab79360cf266"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b3cbd1a791f1c4a8a6ac5c20c0f5238deed72026","unresolved":true,"context_lines":[{"line_number":8060,"context_line":"        features \u003d [(feature.name, feature.policy)"},{"line_number":8061,"context_line":"                    for feature in conf.cpu.features]"},{"line_number":8062,"context_line":"        self.assertIsInstance(conf.cpu,"},{"line_number":8063,"context_line":"                              vconfig.LibvirtConfigGuestCPU)"},{"line_number":8064,"context_line":"        self.assertEqual(conf.cpu.mode, \"custom\")"},{"line_number":8065,"context_line":"        self.assertEqual(conf.cpu.model, \"Cascadelake-Server\")"},{"line_number":8066,"context_line":"        self.assertIn((\"ssbd\", \u0027require\u0027), features)"}],"source_content_type":"text/x-python","patch_set":7,"id":"6e672556_0d584a86","line":8063,"updated":"2021-02-17 11:56:22.000000000","message":"This can fit on one line","commit_id":"0d80b1b0c794e338bfb1b03b1ac4ab79360cf266"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b3cbd1a791f1c4a8a6ac5c20c0f5238deed72026","unresolved":true,"context_lines":[{"line_number":8070,"context_line":"        self.assertEqual(conf.cpu.sockets, instance_ref.flavor.vcpus)"},{"line_number":8071,"context_line":"        self.assertEqual(conf.cpu.cores, 1)"},{"line_number":8072,"context_line":"        self.assertEqual(conf.cpu.threads, 1)"},{"line_number":8073,"context_line":"        mock_warn.assert_not_called()"},{"line_number":8074,"context_line":""},{"line_number":8075,"context_line":"    def test_get_guest_cpu_config_custom_upper_cpu_model(self):"},{"line_number":8076,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)"}],"source_content_type":"text/x-python","patch_set":7,"id":"2a7c65f0_d471e97d","line":8073,"updated":"2021-02-17 11:56:22.000000000","message":"A context comment about why this shouldn\u0027t have been called would be helpful","commit_id":"0d80b1b0c794e338bfb1b03b1ac4ab79360cf266"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"dd4f86d48c6f8c9aef9b21b620d7f3b5f8c19988","unresolved":true,"context_lines":[{"line_number":1463,"context_line":"        drvr._edit_cpu_flag(feat1.name, cpu)"},{"line_number":1464,"context_line":"        drvr._edit_cpu_flag(feat2.name, cpu)"},{"line_number":1465,"context_line":""},{"line_number":1466,"context_line":"        expected_xml \u003d \u0027\u0027\u0027"},{"line_number":1467,"context_line":"              \u003ccpu match\u003d\u0027exact\u0027\u003e"},{"line_number":1468,"context_line":"                \u003cfeature policy\u003d\u0027require\u0027 name\u003d\u0027md-clear\u0027/\u003e"},{"line_number":1469,"context_line":"                \u003cfeature policy\u003d\u0027disable\u0027 name\u003d\u0027ssbd\u0027/\u003e"}],"source_content_type":"text/x-python","patch_set":8,"id":"dd516536_ad4d4ee7","line":1466,"range":{"start_line":1466,"start_character":23,"end_line":1466,"end_character":26},"updated":"2021-02-18 18:30:08.000000000","message":"style nit: \"\"\"","commit_id":"7597c4b97f0e7424583c8eb33f55c5ccf13c6873"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"dd4f86d48c6f8c9aef9b21b620d7f3b5f8c19988","unresolved":true,"context_lines":[{"line_number":1464,"context_line":"        drvr._edit_cpu_flag(feat2.name, cpu)"},{"line_number":1465,"context_line":""},{"line_number":1466,"context_line":"        expected_xml \u003d \u0027\u0027\u0027"},{"line_number":1467,"context_line":"              \u003ccpu match\u003d\u0027exact\u0027\u003e"},{"line_number":1468,"context_line":"                \u003cfeature policy\u003d\u0027require\u0027 name\u003d\u0027md-clear\u0027/\u003e"},{"line_number":1469,"context_line":"                \u003cfeature policy\u003d\u0027disable\u0027 name\u003d\u0027ssbd\u0027/\u003e"},{"line_number":1470,"context_line":"              \u003c/cpu\u003e\u0027\u0027\u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"e48e10f3_46ab131f","line":1467,"range":{"start_line":1467,"start_character":12,"end_line":1467,"end_character":14},"updated":"2021-02-18 18:30:08.000000000","message":"nit: unnecessary indentation","commit_id":"7597c4b97f0e7424583c8eb33f55c5ccf13c6873"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"ee45348a5670cfcbee5abf67a741d7bb7fc93662","unresolved":true,"context_lines":[{"line_number":1475,"context_line":"        )"},{"line_number":1476,"context_line":""},{"line_number":1477,"context_line":"    @mock.patch.object(libvirt_driver.LibvirtDriver,"},{"line_number":1478,"context_line":"                       \u0027_register_instance_machine_type\u0027, new\u003dmock.Mock())"},{"line_number":1479,"context_line":"    def test__check_cpu_compatibility_start_ok(self):"},{"line_number":1480,"context_line":"        self.flags(cpu_mode\u003d\"custom\","},{"line_number":1481,"context_line":"                   cpu_models\u003d[\"Penryn\"],"}],"source_content_type":"text/x-python","patch_set":10,"id":"613b1cab_526d7431","side":"PARENT","line":1478,"updated":"2021-03-01 15:30:29.000000000","message":"Oops, that\u0027s accidental damage during rebase.  Lee spotted it on IRC.\n\nFixed in next iteration.","commit_id":"ded25f33c734ebff963f06984707a99fe76a9ee1"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5cd7b267b3b86f3585e1d99ece2c746fce65a7c1","unresolved":true,"context_lines":[{"line_number":1490,"context_line":"        self.assertIsInstance(feat2,"},{"line_number":1491,"context_line":"                              vconfig.LibvirtConfigGuestCPUFeature)"},{"line_number":1492,"context_line":"        self.assertIsInstance(feat3,"},{"line_number":1493,"context_line":"                              vconfig.LibvirtConfigGuestCPUFeature)"},{"line_number":1494,"context_line":""},{"line_number":1495,"context_line":"        cpu \u003d vconfig.LibvirtConfigGuestCPU()"},{"line_number":1496,"context_line":"        cpu.add_feature(feat1)"}],"source_content_type":"text/x-python","patch_set":11,"id":"9b95e1e1_1cbd7d35","line":1493,"updated":"2021-03-04 15:45:24.000000000","message":"nit: all these would fit on one line","commit_id":"d897ce2a545fd1b774d6c65017e71c016606eedf"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5cd7b267b3b86f3585e1d99ece2c746fce65a7c1","unresolved":true,"context_lines":[{"line_number":1508,"context_line":"        self.assertXmlEqual(expected_xml, cpu.to_xml())"},{"line_number":1509,"context_line":""},{"line_number":1510,"context_line":"    @mock.patch.object(libvirt_driver.LibvirtDriver,"},{"line_number":1511,"context_line":"                       \u0027_register_instance_machine_type\u0027, new\u003dmock.Mock())"},{"line_number":1512,"context_line":"    def test__check_cpu_compatibility_start_ok(self):"},{"line_number":1513,"context_line":"        self.flags(cpu_mode\u003d\"custom\","},{"line_number":1514,"context_line":"                   cpu_models\u003d[\"Penryn\"],"}],"source_content_type":"text/x-python","patch_set":11,"id":"4672685c_c59e5e0f","line":1511,"updated":"2021-03-04 15:45:24.000000000","message":"This doesn\u0027t look relevant?","commit_id":"d897ce2a545fd1b774d6c65017e71c016606eedf"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e48c674be603d3af0918a10fb9a9244fe68b862b","unresolved":false,"context_lines":[{"line_number":1508,"context_line":"        self.assertXmlEqual(expected_xml, cpu.to_xml())"},{"line_number":1509,"context_line":""},{"line_number":1510,"context_line":"    @mock.patch.object(libvirt_driver.LibvirtDriver,"},{"line_number":1511,"context_line":"                       \u0027_register_instance_machine_type\u0027, new\u003dmock.Mock())"},{"line_number":1512,"context_line":"    def test__check_cpu_compatibility_start_ok(self):"},{"line_number":1513,"context_line":"        self.flags(cpu_mode\u003d\"custom\","},{"line_number":1514,"context_line":"                   cpu_models\u003d[\"Penryn\"],"}],"source_content_type":"text/x-python","patch_set":11,"id":"d4f63adc_714caa9f","line":1511,"in_reply_to":"4672685c_c59e5e0f","updated":"2021-03-04 15:50:50.000000000","message":"Whoops, the diff simply moved these around. Ignore this","commit_id":"d897ce2a545fd1b774d6c65017e71c016606eedf"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5cd7b267b3b86f3585e1d99ece2c746fce65a7c1","unresolved":true,"context_lines":[{"line_number":1576,"context_line":""},{"line_number":1577,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.libvirt.Connection.compareCPU\u0027)"},{"line_number":1578,"context_line":"    def test__check_cpu_compatibility_enabled_and_disabled_flags("},{"line_number":1579,"context_line":"            self, mocked_compare):"},{"line_number":1580,"context_line":"        mocked_compare.side_effect \u003d (2, 0)"},{"line_number":1581,"context_line":"        self.flags("},{"line_number":1582,"context_line":"            cpu_mode\u003d\"custom\","}],"source_content_type":"text/x-python","patch_set":11,"id":"1bd94da9_ea3962b1","line":1579,"updated":"2021-03-04 15:45:24.000000000","message":"style nit:\n\n  def foo(\n      self, bar,\n  ):\n      stuff()\n\nbelow also","commit_id":"d897ce2a545fd1b774d6c65017e71c016606eedf"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5cd7b267b3b86f3585e1d99ece2c746fce65a7c1","unresolved":true,"context_lines":[{"line_number":8169,"context_line":"            cpu_model_extra_flags\u003d[\u0027-hle\u0027, \u0027-rtm\u0027, \u0027+ssbd\u0027, \u0027mtrr\u0027],"},{"line_number":8170,"context_line":"            group\u003d\u0027libvirt\u0027)"},{"line_number":8171,"context_line":"        disk_info \u003d blockinfo.get_disk_info("},{"line_number":8172,"context_line":"                        CONF.libvirt.virt_type,"},{"line_number":8173,"context_line":"                        instance_ref,"},{"line_number":8174,"context_line":"                        image_meta)"},{"line_number":8175,"context_line":"        conf \u003d drvr._get_guest_config("}],"source_content_type":"text/x-python","patch_set":11,"id":"ea217f2b_4d82e024","line":8172,"range":{"start_line":8172,"start_character":12,"end_line":8172,"end_character":24},"updated":"2021-03-04 15:45:24.000000000","message":"whoops","commit_id":"d897ce2a545fd1b774d6c65017e71c016606eedf"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5cd7b267b3b86f3585e1d99ece2c746fce65a7c1","unresolved":true,"context_lines":[{"line_number":8173,"context_line":"                        instance_ref,"},{"line_number":8174,"context_line":"                        image_meta)"},{"line_number":8175,"context_line":"        conf \u003d drvr._get_guest_config("},{"line_number":8176,"context_line":"                        instance_ref,"},{"line_number":8177,"context_line":"                        _fake_network_info(self),"},{"line_number":8178,"context_line":"                        image_meta, disk_info)"},{"line_number":8179,"context_line":"        features \u003d [(feature.name, feature.policy)"}],"source_content_type":"text/x-python","patch_set":11,"id":"20636f00_d19193ba","line":8176,"range":{"start_line":8176,"start_character":12,"end_line":8176,"end_character":24},"updated":"2021-03-04 15:45:24.000000000","message":"whoops","commit_id":"d897ce2a545fd1b774d6c65017e71c016606eedf"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"8464e991819af36df45d789d556c10717a0a6ccb","unresolved":true,"context_lines":[{"line_number":1561,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.libvirt.Connection.compareCPU\u0027)"},{"line_number":1562,"context_line":"    def test__check_cpu_compatibility_wrong_flag(self, mocked_compare):"},{"line_number":1563,"context_line":"        # here, and in the surrounding similar tests, the non-zero error"},{"line_number":1564,"context_line":"        # code in the compareCPU() side effect indicates error"},{"line_number":1565,"context_line":"        mocked_compare.side_effect \u003d (2, 0)"},{"line_number":1566,"context_line":"        self.flags(cpu_mode\u003d\"custom\","},{"line_number":1567,"context_line":"                   cpu_models\u003d[\"Broadwell-noTSX\"],"}],"source_content_type":"text/x-python","patch_set":12,"id":"a7b934a9_06e67af6","line":1564,"updated":"2021-03-04 17:11:11.000000000","message":"This isn\u0027t strictly speaking related to this patch, right?","commit_id":"bcd6b42047ea9422a58a4273d831e23f2ea27092"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"8e2bec78f19561471f8b40fd93080e8b82a40bad","unresolved":true,"context_lines":[{"line_number":1561,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.libvirt.Connection.compareCPU\u0027)"},{"line_number":1562,"context_line":"    def test__check_cpu_compatibility_wrong_flag(self, mocked_compare):"},{"line_number":1563,"context_line":"        # here, and in the surrounding similar tests, the non-zero error"},{"line_number":1564,"context_line":"        # code in the compareCPU() side effect indicates error"},{"line_number":1565,"context_line":"        mocked_compare.side_effect \u003d (2, 0)"},{"line_number":1566,"context_line":"        self.flags(cpu_mode\u003d\"custom\","},{"line_number":1567,"context_line":"                   cpu_models\u003d[\"Broadwell-noTSX\"],"}],"source_content_type":"text/x-python","patch_set":12,"id":"afbcfe65_d37260a2","line":1564,"in_reply_to":"a7b934a9_06e67af6","updated":"2021-03-05 08:39:27.000000000","message":"It\u0027s indeed not; Stephen remarked in an earlier review it would be nice to have a comment about its meaning in an earlier review.  I just did it.","commit_id":"bcd6b42047ea9422a58a4273d831e23f2ea27092"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"8464e991819af36df45d789d556c10717a0a6ccb","unresolved":true,"context_lines":[{"line_number":1569,"context_line":"                   group\u003d\"libvirt\")"},{"line_number":1570,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)"},{"line_number":1571,"context_line":"        self.assertRaises(exception.InvalidCPUInfo,"},{"line_number":1572,"context_line":"                          drvr.init_host, \"dummyhost\")"},{"line_number":1573,"context_line":""},{"line_number":1574,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.libvirt.Connection.compareCPU\u0027)"},{"line_number":1575,"context_line":"    def test__check_cpu_compatibility_enabled_and_disabled_flags("}],"source_content_type":"text/x-python","patch_set":12,"id":"5befb9b1_f8ec737b","line":1572,"range":{"start_line":1572,"start_character":31,"end_line":1572,"end_character":40},"updated":"2021-03-04 17:11:11.000000000","message":"Completely unrelated to your patch, but in trying to understand your unit test I started wondering why this is calling init_host() instead of _check_cpu_compatibility() directly...","commit_id":"bcd6b42047ea9422a58a4273d831e23f2ea27092"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"8464e991819af36df45d789d556c10717a0a6ccb","unresolved":true,"context_lines":[{"line_number":1572,"context_line":"                          drvr.init_host, \"dummyhost\")"},{"line_number":1573,"context_line":""},{"line_number":1574,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.libvirt.Connection.compareCPU\u0027)"},{"line_number":1575,"context_line":"    def test__check_cpu_compatibility_enabled_and_disabled_flags("},{"line_number":1576,"context_line":"            self, mocked_compare"},{"line_number":1577,"context_line":"    ):"},{"line_number":1578,"context_line":"        mocked_compare.side_effect \u003d (2, 0)"}],"source_content_type":"text/x-python","patch_set":12,"id":"5fff1b80_d34d8a82","line":1575,"updated":"2021-03-04 17:11:11.000000000","message":"Does this new test add any value? Wrong flags are already being tested in test__check_cpu_compatibility_wrong_flag...","commit_id":"bcd6b42047ea9422a58a4273d831e23f2ea27092"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"8e2bec78f19561471f8b40fd93080e8b82a40bad","unresolved":true,"context_lines":[{"line_number":1572,"context_line":"                          drvr.init_host, \"dummyhost\")"},{"line_number":1573,"context_line":""},{"line_number":1574,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.libvirt.Connection.compareCPU\u0027)"},{"line_number":1575,"context_line":"    def test__check_cpu_compatibility_enabled_and_disabled_flags("},{"line_number":1576,"context_line":"            self, mocked_compare"},{"line_number":1577,"context_line":"    ):"},{"line_number":1578,"context_line":"        mocked_compare.side_effect \u003d (2, 0)"}],"source_content_type":"text/x-python","patch_set":12,"id":"a593c8c2_bb2eea35","line":1575,"in_reply_to":"5fff1b80_d34d8a82","updated":"2021-03-05 08:39:27.000000000","message":"The value is making sure I\u0027m testing the methods (this and the one below) which are parsing the CPU flags.  And we\u0027re not testing wrong flags here, but a combination of valid flags with different prefixes.","commit_id":"bcd6b42047ea9422a58a4273d831e23f2ea27092"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"8464e991819af36df45d789d556c10717a0a6ccb","unresolved":true,"context_lines":[{"line_number":8155,"context_line":"        mock_warn.assert_not_called()"},{"line_number":8156,"context_line":""},{"line_number":8157,"context_line":"    @mock.patch.object(libvirt_driver.LOG, \u0027warning\u0027)"},{"line_number":8158,"context_line":"    def test_get_guest_cpu_config_custom_flags_enabled_and_disabled("},{"line_number":8159,"context_line":"            self, mock_warn"},{"line_number":8160,"context_line":"    ):"},{"line_number":8161,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)"}],"source_content_type":"text/x-python","patch_set":12,"id":"b1277089_cd1dbf40","line":8158,"updated":"2021-03-04 17:11:11.000000000","message":"Same question about new test value... I guess it\u0027s a *really* thorough way to test you\u0027re calling _prepare_cpu_flag() correctly. I\u0027d have been satisfied with a mock and a \"asssert_called_with()\"","commit_id":"bcd6b42047ea9422a58a4273d831e23f2ea27092"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"8e2bec78f19561471f8b40fd93080e8b82a40bad","unresolved":true,"context_lines":[{"line_number":8155,"context_line":"        mock_warn.assert_not_called()"},{"line_number":8156,"context_line":""},{"line_number":8157,"context_line":"    @mock.patch.object(libvirt_driver.LOG, \u0027warning\u0027)"},{"line_number":8158,"context_line":"    def test_get_guest_cpu_config_custom_flags_enabled_and_disabled("},{"line_number":8159,"context_line":"            self, mock_warn"},{"line_number":8160,"context_line":"    ):"},{"line_number":8161,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)"}],"source_content_type":"text/x-python","patch_set":12,"id":"7e677bde_8f896559","line":8158,"in_reply_to":"b1277089_cd1dbf40","updated":"2021-03-05 08:39:27.000000000","message":"Indeed; I wanted to make sure that _prepare_cpu_flag() is being correctly called.","commit_id":"bcd6b42047ea9422a58a4273d831e23f2ea27092"}],"nova/virt/libvirt/config.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"7d71a03136492e2bd4a491cbc974d47ae6d6312d","unresolved":true,"context_lines":[{"line_number":729,"context_line":"                self.cores \u003d int(c.get(\"cores\"))"},{"line_number":730,"context_line":"                self.threads \u003d int(c.get(\"threads\"))"},{"line_number":731,"context_line":"            elif c.tag \u003d\u003d \"feature\":"},{"line_number":732,"context_line":"                f \u003d LibvirtConfigCPUFeature()"},{"line_number":733,"context_line":"                f.parse_dom(c)"},{"line_number":734,"context_line":"                if f.policy !\u003d \"disable\":"},{"line_number":735,"context_line":"                    self.add_feature(f)"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a43fa2a_ba0a4d84","line":732,"range":{"start_line":732,"start_character":20,"end_line":732,"end_character":43},"updated":"2021-02-08 13:29:21.000000000","message":"Should this be LibvirtConfigGuestCPUFeature also? Below we only add non disable policies, but we could also use LibvirtConfigGuestCPUFeature and store the policy directly.","commit_id":"31a4646b33a38260075a9d65d823ce31fa6030fc"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"aa90923eb8e1ba3d40336c8880c29c10629fbd06","unresolved":true,"context_lines":[{"line_number":729,"context_line":"                self.cores \u003d int(c.get(\"cores\"))"},{"line_number":730,"context_line":"                self.threads \u003d int(c.get(\"threads\"))"},{"line_number":731,"context_line":"            elif c.tag \u003d\u003d \"feature\":"},{"line_number":732,"context_line":"                f \u003d LibvirtConfigCPUFeature()"},{"line_number":733,"context_line":"                f.parse_dom(c)"},{"line_number":734,"context_line":"                if f.policy !\u003d \"disable\":"},{"line_number":735,"context_line":"                    self.add_feature(f)"}],"source_content_type":"text/x-python","patch_set":3,"id":"8341c95f_7a7d527a","line":732,"range":{"start_line":732,"start_character":20,"end_line":732,"end_character":43},"in_reply_to":"1a43fa2a_ba0a4d84","updated":"2021-02-08 14:24:47.000000000","message":"I don\u0027t think so, as LibvirtConfigCPUFeature() is the main base class for defining CPU features.  As a refresher (to myself) the function of four classes are:\n\n- LibvirtConfigCPUFeature - the base class for defining CPU features\n- LibvirtConfigCPU - base class for defining CPU models\n- LibvirtConfigGuestCPUFeature - extension for setting the guest-specific CPU \"feature\" policy — here, libvirt allows five possible values: force, require, optional, disable, forbid\n- LibvirtConfigGuestCPU - extension for setting the guest-specific \"match\" policy, and allowing use of host CPU model passthrough — here, libvirt allows three possible values: minimum, exact, and strict","commit_id":"31a4646b33a38260075a9d65d823ce31fa6030fc"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ed4339589ddcd00ed1b636bfd2c3b492c41d190a","unresolved":true,"context_lines":[{"line_number":729,"context_line":"                self.cores \u003d int(c.get(\"cores\"))"},{"line_number":730,"context_line":"                self.threads \u003d int(c.get(\"threads\"))"},{"line_number":731,"context_line":"            elif c.tag \u003d\u003d \"feature\":"},{"line_number":732,"context_line":"                f \u003d LibvirtConfigCPUFeature()"},{"line_number":733,"context_line":"                f.parse_dom(c)"},{"line_number":734,"context_line":"                if f.policy !\u003d \"disable\":"},{"line_number":735,"context_line":"                    self.add_feature(f)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9308e5d9_3d006e13","line":732,"range":{"start_line":732,"start_character":20,"end_line":732,"end_character":43},"in_reply_to":"8341c95f_7a7d527a","updated":"2021-02-09 12:39:58.000000000","message":"OK, this is more complicated than I originally thought. I trust your judgement here.","commit_id":"31a4646b33a38260075a9d65d823ce31fa6030fc"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"05f8d7b77fd2fa68f08c42d0f203d5e247a9c8a6","unresolved":true,"context_lines":[{"line_number":702,"context_line":"            # If neither \u0027+\u0027 nor \u0027-\u0027 is specified, the CPU feature is"},{"line_number":703,"context_line":"            # enabled."},{"line_number":704,"context_line":"            if flag.startswith(\u0027-\u0027):"},{"line_number":705,"context_line":"                flag \u003d flag.strip(\u0027-\u0027)"},{"line_number":706,"context_line":"                cpu.remove_feature(vconfig.LibvirtConfigCPUFeature(flag))"},{"line_number":707,"context_line":"            else:"},{"line_number":708,"context_line":"                flag \u003d flag.strip(\u0027+\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"0620cf22_3cd0b86a","line":705,"range":{"start_line":705,"start_character":16,"end_line":705,"end_character":38},"updated":"2021-02-05 14:29:56.000000000","message":"no this will strip all - in the flag. \nwe shoudl not assume the kernel wont add one in the futrue\nyou should do \n\nflag \u003d flag[1:]","commit_id":"f5f513806bb35053168dfeeed4f545f758839444"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"0da5551582502c66a481d1274cb5dad08d9e3e83","unresolved":true,"context_lines":[{"line_number":702,"context_line":"            # If neither \u0027+\u0027 nor \u0027-\u0027 is specified, the CPU feature is"},{"line_number":703,"context_line":"            # enabled."},{"line_number":704,"context_line":"            if flag.startswith(\u0027-\u0027):"},{"line_number":705,"context_line":"                flag \u003d flag.strip(\u0027-\u0027)"},{"line_number":706,"context_line":"                cpu.remove_feature(vconfig.LibvirtConfigCPUFeature(flag))"},{"line_number":707,"context_line":"            else:"},{"line_number":708,"context_line":"                flag \u003d flag.strip(\u0027+\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"d2eb99f3_516b26d2","line":705,"range":{"start_line":705,"start_character":16,"end_line":705,"end_character":38},"in_reply_to":"0620cf22_3cd0b86a","updated":"2021-02-05 16:04:47.000000000","message":"I realize.  We don\u0027t need *all* \"-\" in the flag to be stripped; but only from the beginning (and at the end); which is what strip() does.\n\nI can tweak it to use sliced index.\n\nThanks for the review.","commit_id":"f5f513806bb35053168dfeeed4f545f758839444"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"05f8d7b77fd2fa68f08c42d0f203d5e247a9c8a6","unresolved":true,"context_lines":[{"line_number":705,"context_line":"                flag \u003d flag.strip(\u0027-\u0027)"},{"line_number":706,"context_line":"                cpu.remove_feature(vconfig.LibvirtConfigCPUFeature(flag))"},{"line_number":707,"context_line":"            else:"},{"line_number":708,"context_line":"                flag \u003d flag.strip(\u0027+\u0027)"},{"line_number":709,"context_line":"                cpu.add_feature(vconfig.LibvirtConfigCPUFeature(flag))"},{"line_number":710,"context_line":"            try:"},{"line_number":711,"context_line":"                self._compare_cpu(cpu, self._get_cpu_info(), None)"}],"source_content_type":"text/x-python","patch_set":1,"id":"0a2f1917_0a83267e","line":708,"range":{"start_line":708,"start_character":15,"end_line":708,"end_character":38},"updated":"2021-02-05 14:29:56.000000000","message":"flag \u003d flag[1:]","commit_id":"f5f513806bb35053168dfeeed4f545f758839444"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"afdff7bdd9c3ea8b994b5a8b05c333508f21cf2c","unresolved":true,"context_lines":[{"line_number":705,"context_line":"                flag \u003d flag.strip(\u0027-\u0027)"},{"line_number":706,"context_line":"                cpu.remove_feature(vconfig.LibvirtConfigCPUFeature(flag))"},{"line_number":707,"context_line":"            else:"},{"line_number":708,"context_line":"                flag \u003d flag.strip(\u0027+\u0027)"},{"line_number":709,"context_line":"                cpu.add_feature(vconfig.LibvirtConfigCPUFeature(flag))"},{"line_number":710,"context_line":"            try:"},{"line_number":711,"context_line":"                self._compare_cpu(cpu, self._get_cpu_info(), None)"}],"source_content_type":"text/x-python","patch_set":1,"id":"c1bca6c6_10b19b35","line":708,"range":{"start_line":708,"start_character":15,"end_line":708,"end_character":38},"in_reply_to":"0a2f1917_0a83267e","updated":"2021-02-08 10:06:50.000000000","message":"Actually, using index slicing is wrong here.  That way, it will strip the first character as well, if you don\u0027t have a \"+\" sign, which is undesirable.\n\nI\u0027m instead going to use lstrip() — that is the cleanest and most robust.","commit_id":"f5f513806bb35053168dfeeed4f545f758839444"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"2843aa21a1cb0a299cd7e46b699f0518810f24f2","unresolved":true,"context_lines":[{"line_number":705,"context_line":"                flag \u003d flag.strip(\u0027-\u0027)"},{"line_number":706,"context_line":"                cpu.remove_feature(vconfig.LibvirtConfigCPUFeature(flag))"},{"line_number":707,"context_line":"            else:"},{"line_number":708,"context_line":"                flag \u003d flag.strip(\u0027+\u0027)"},{"line_number":709,"context_line":"                cpu.add_feature(vconfig.LibvirtConfigCPUFeature(flag))"},{"line_number":710,"context_line":"            try:"},{"line_number":711,"context_line":"                self._compare_cpu(cpu, self._get_cpu_info(), None)"}],"source_content_type":"text/x-python","patch_set":1,"id":"2d3061a8_ba64c495","line":708,"range":{"start_line":708,"start_character":15,"end_line":708,"end_character":38},"in_reply_to":"c1bca6c6_10b19b35","updated":"2021-02-08 11:48:05.000000000","message":"ya lstrip works","commit_id":"f5f513806bb35053168dfeeed4f545f758839444"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"aa90923eb8e1ba3d40336c8880c29c10629fbd06","unresolved":true,"context_lines":[{"line_number":705,"context_line":"                flag \u003d flag.lstrip(\u0027-\u0027)"},{"line_number":706,"context_line":"                cpu.add.feature("},{"line_number":707,"context_line":"                        vconfig.LibvirtConfigGuestCPUFeature(flag,"},{"line_number":708,"context_line":"                            policy\u003d\u0027disable\u0027))"},{"line_number":709,"context_line":"            else:"},{"line_number":710,"context_line":"                flag \u003d flag.lstrip(\u0027+\u0027)"},{"line_number":711,"context_line":"                cpu.add_feature(vconfig.LibvirtConfigGuestCPUFeature(flag))"}],"source_content_type":"text/x-python","patch_set":3,"id":"38275849_3ab5ab1b","line":708,"updated":"2021-02-08 14:24:47.000000000","message":"Good catch; I only tested_get_guest_cpu_config(), but not _check_cpu_compatibility()","commit_id":"31a4646b33a38260075a9d65d823ce31fa6030fc"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"7d71a03136492e2bd4a491cbc974d47ae6d6312d","unresolved":true,"context_lines":[{"line_number":705,"context_line":"                flag \u003d flag.lstrip(\u0027-\u0027)"},{"line_number":706,"context_line":"                cpu.add.feature("},{"line_number":707,"context_line":"                        vconfig.LibvirtConfigGuestCPUFeature(flag,"},{"line_number":708,"context_line":"                            policy\u003d\u0027disable\u0027))"},{"line_number":709,"context_line":"            else:"},{"line_number":710,"context_line":"                flag \u003d flag.lstrip(\u0027+\u0027)"},{"line_number":711,"context_line":"                cpu.add_feature(vconfig.LibvirtConfigGuestCPUFeature(flag))"}],"source_content_type":"text/x-python","patch_set":3,"id":"f261994a_f4c3e6ad","line":708,"updated":"2021-02-08 13:29:21.000000000","message":"I think this change is not covered with test","commit_id":"31a4646b33a38260075a9d65d823ce31fa6030fc"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"7d71a03136492e2bd4a491cbc974d47ae6d6312d","unresolved":true,"context_lines":[{"line_number":8802,"context_line":"            cpu.cores \u003d info[\u0027topology\u0027][\u0027cores\u0027]"},{"line_number":8803,"context_line":"            cpu.threads \u003d info[\u0027topology\u0027][\u0027threads\u0027]"},{"line_number":8804,"context_line":"            for f in info[\u0027features\u0027]:"},{"line_number":8805,"context_line":"                cpu.add_feature(vconfig.LibvirtConfigCPUFeature(f))"},{"line_number":8806,"context_line":"        elif isinstance(guest_cpu, vconfig.LibvirtConfigGuestCPU):"},{"line_number":8807,"context_line":"            cpu \u003d guest_cpu"},{"line_number":8808,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"6b42eca3_1ad69a00","line":8805,"range":{"start_line":8805,"start_character":40,"end_line":8805,"end_character":63},"updated":"2021-02-08 13:29:21.000000000","message":"Is it ok to use LibvirtConfigCPUFeature here instead of LibvirtConfigGuestCPUFeature? This means here we consider all host cpu as required as LibvirtConfigCPUFeature hardcodes that policy.","commit_id":"31a4646b33a38260075a9d65d823ce31fa6030fc"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"aa90923eb8e1ba3d40336c8880c29c10629fbd06","unresolved":true,"context_lines":[{"line_number":8802,"context_line":"            cpu.cores \u003d info[\u0027topology\u0027][\u0027cores\u0027]"},{"line_number":8803,"context_line":"            cpu.threads \u003d info[\u0027topology\u0027][\u0027threads\u0027]"},{"line_number":8804,"context_line":"            for f in info[\u0027features\u0027]:"},{"line_number":8805,"context_line":"                cpu.add_feature(vconfig.LibvirtConfigCPUFeature(f))"},{"line_number":8806,"context_line":"        elif isinstance(guest_cpu, vconfig.LibvirtConfigGuestCPU):"},{"line_number":8807,"context_line":"            cpu \u003d guest_cpu"},{"line_number":8808,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"c733d0c8_d32c9d97","line":8805,"range":{"start_line":8805,"start_character":40,"end_line":8805,"end_character":63},"in_reply_to":"6b42eca3_1ad69a00","updated":"2021-02-08 14:24:47.000000000","message":"Near as I see, this should be okay.  The \"require\" from LibvirtConfigCPUFeature() is correct.\n\nWhat it means, in libvirt-lingo is that the guest creation will fail unless the CPU feature satisfies either of these two conditions: (a) it is supported by the host CPU or (b) the hypervisor (KVM/QEMU) is able to emulate it.","commit_id":"31a4646b33a38260075a9d65d823ce31fa6030fc"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ed4339589ddcd00ed1b636bfd2c3b492c41d190a","unresolved":false,"context_lines":[{"line_number":8802,"context_line":"            cpu.cores \u003d info[\u0027topology\u0027][\u0027cores\u0027]"},{"line_number":8803,"context_line":"            cpu.threads \u003d info[\u0027topology\u0027][\u0027threads\u0027]"},{"line_number":8804,"context_line":"            for f in info[\u0027features\u0027]:"},{"line_number":8805,"context_line":"                cpu.add_feature(vconfig.LibvirtConfigCPUFeature(f))"},{"line_number":8806,"context_line":"        elif isinstance(guest_cpu, vconfig.LibvirtConfigGuestCPU):"},{"line_number":8807,"context_line":"            cpu \u003d guest_cpu"},{"line_number":8808,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"07e36ba6_a23d82f1","line":8805,"range":{"start_line":8805,"start_character":40,"end_line":8805,"end_character":63},"in_reply_to":"c733d0c8_d32c9d97","updated":"2021-02-09 12:39:58.000000000","message":"Ack","commit_id":"31a4646b33a38260075a9d65d823ce31fa6030fc"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9da1a3ef70a4131c3db0e37cf2f26fa6504b69b5","unresolved":true,"context_lines":[{"line_number":704,"context_line":"            if flag.startswith(\u0027-\u0027):"},{"line_number":705,"context_line":"                flag \u003d flag.lstrip(\u0027-\u0027)"},{"line_number":706,"context_line":"                cpu.add_feature("},{"line_number":707,"context_line":"                        vconfig.LibvirtConfigGuestCPUFeature(flag,"},{"line_number":708,"context_line":"                            policy\u003d\u0027disable\u0027))"},{"line_number":709,"context_line":"            else:"},{"line_number":710,"context_line":"                flag \u003d flag.lstrip(\u0027+\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"d7dadba6_b351d3d6","line":707,"range":{"start_line":707,"start_character":20,"end_line":707,"end_character":24},"updated":"2021-02-15 18:44:23.000000000","message":"drop","commit_id":"5f3742475a0c5491194e14dd3c0c23aac8897319"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"eac60d8655be356015a03f24df1e26693ffc4fe1","unresolved":true,"context_lines":[{"line_number":704,"context_line":"            if flag.startswith(\u0027-\u0027):"},{"line_number":705,"context_line":"                flag \u003d flag.lstrip(\u0027-\u0027)"},{"line_number":706,"context_line":"                cpu.add_feature("},{"line_number":707,"context_line":"                        vconfig.LibvirtConfigGuestCPUFeature(flag,"},{"line_number":708,"context_line":"                            policy\u003d\u0027disable\u0027))"},{"line_number":709,"context_line":"            else:"},{"line_number":710,"context_line":"                flag \u003d flag.lstrip(\u0027+\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"01e09565_cd61eaf3","line":707,"range":{"start_line":707,"start_character":20,"end_line":707,"end_character":24},"in_reply_to":"d7dadba6_b351d3d6","updated":"2021-02-16 12:46:03.000000000","message":"Yep","commit_id":"5f3742475a0c5491194e14dd3c0c23aac8897319"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9da1a3ef70a4131c3db0e37cf2f26fa6504b69b5","unresolved":true,"context_lines":[{"line_number":708,"context_line":"                            policy\u003d\u0027disable\u0027))"},{"line_number":709,"context_line":"            else:"},{"line_number":710,"context_line":"                flag \u003d flag.lstrip(\u0027+\u0027)"},{"line_number":711,"context_line":"                cpu.add_feature(vconfig.LibvirtConfigGuestCPUFeature(flag))"},{"line_number":712,"context_line":"            try:"},{"line_number":713,"context_line":"                self._compare_cpu(cpu, self._get_cpu_info(), None)"},{"line_number":714,"context_line":"            except exception.InvalidCPUInfo as e:"}],"source_content_type":"text/x-python","patch_set":6,"id":"8d1fe6e5_5f1dfec2","line":711,"updated":"2021-02-15 18:44:23.000000000","message":"lstrip can accept multiple characters and will strip them all, in any order. Using this, we can simplify this like so:\n\n  feature \u003d vconfig.LibvirtConfigGuestCPUFeature(\n      flag.lstrip(\u0027+-\u0027), policy\u003dflag.startswith(\u0027+\u0027))\n  cpu.add_feature(feature)\n\n Also, we should ensure that it only starts with a + or -, right?\n\n  if not flag.startswith((\u0027+\u0027, \u0027-\u0027)):\n      raise exception...","commit_id":"5f3742475a0c5491194e14dd3c0c23aac8897319"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"eac60d8655be356015a03f24df1e26693ffc4fe1","unresolved":true,"context_lines":[{"line_number":708,"context_line":"                            policy\u003d\u0027disable\u0027))"},{"line_number":709,"context_line":"            else:"},{"line_number":710,"context_line":"                flag \u003d flag.lstrip(\u0027+\u0027)"},{"line_number":711,"context_line":"                cpu.add_feature(vconfig.LibvirtConfigGuestCPUFeature(flag))"},{"line_number":712,"context_line":"            try:"},{"line_number":713,"context_line":"                self._compare_cpu(cpu, self._get_cpu_info(), None)"},{"line_number":714,"context_line":"            except exception.InvalidCPUInfo as e:"}],"source_content_type":"text/x-python","patch_set":6,"id":"51d44356_c9f0a39e","line":711,"in_reply_to":"8d1fe6e5_5f1dfec2","updated":"2021-02-16 12:46:03.000000000","message":"\u003e lstrip can accept multiple characters and will strip them all, in any order. Using this, we can simplify this like so:\n\u003e \n\u003e   feature \u003d vconfig.LibvirtConfigGuestCPUFeature(\n\u003e       flag.lstrip(\u0027+-\u0027), policy\u003dflag.startswith(\u0027+\u0027))\n\u003e   cpu.add_feature(feature)\n\nI see; noted.  I\u0027d prefer to retain the more explicit variant of this patch.\n\n\u003e  Also, we should ensure that it only starts with a + or -, right?\n\nNo; as the code-comment states, if neither \u0027+\u0027 nor \u0027-\u0027 is specified, the CPU flag will be *enabled* — which is the default behavior.  We need to retain this behavior.\n\n\u003e   if not flag.startswith((\u0027+\u0027, \u0027-\u0027)):\n\u003e       raise exception...\n\nNo, we don\u0027t want to that; that\u0027ll cause a regression.","commit_id":"5f3742475a0c5491194e14dd3c0c23aac8897319"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b3cbd1a791f1c4a8a6ac5c20c0f5238deed72026","unresolved":true,"context_lines":[{"line_number":704,"context_line":"            if flag.startswith(\u0027-\u0027):"},{"line_number":705,"context_line":"                flag \u003d flag.lstrip(\u0027-\u0027)"},{"line_number":706,"context_line":"                cpu.add_feature("},{"line_number":707,"context_line":"                        vconfig.LibvirtConfigGuestCPUFeature(flag,"},{"line_number":708,"context_line":"                            policy\u003d\u0027disable\u0027))"},{"line_number":709,"context_line":"            else:"},{"line_number":710,"context_line":"                flag \u003d flag.lstrip(\u0027+\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"7b74175c_5ec7580a","line":707,"range":{"start_line":707,"start_character":20,"end_line":707,"end_character":24},"updated":"2021-02-17 11:56:22.000000000","message":"This is still indented too much","commit_id":"0d80b1b0c794e338bfb1b03b1ac4ab79360cf266"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"433db5542611c2169feb87e2f4185be2d9fa9971","unresolved":true,"context_lines":[{"line_number":704,"context_line":"            if flag.startswith(\u0027-\u0027):"},{"line_number":705,"context_line":"                flag \u003d flag.lstrip(\u0027-\u0027)"},{"line_number":706,"context_line":"                cpu.add_feature("},{"line_number":707,"context_line":"                        vconfig.LibvirtConfigGuestCPUFeature(flag,"},{"line_number":708,"context_line":"                            policy\u003d\u0027disable\u0027))"},{"line_number":709,"context_line":"            else:"},{"line_number":710,"context_line":"                flag \u003d flag.lstrip(\u0027+\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"e07afe21_a581d698","line":707,"range":{"start_line":707,"start_character":20,"end_line":707,"end_character":24},"in_reply_to":"7b74175c_5ec7580a","updated":"2021-02-17 12:42:49.000000000","message":"Forgot to fix that bit; will do.","commit_id":"0d80b1b0c794e338bfb1b03b1ac4ab79360cf266"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"1c80da22b067a0c62273f106fc77319f8be99013","unresolved":true,"context_lines":[{"line_number":694,"context_line":"        # configured extra_flags"},{"line_number":695,"context_line":"        cpu \u003d vconfig.LibvirtConfigGuestCPU()"},{"line_number":696,"context_line":"        cpu.model \u003d self._host.get_capabilities().host.cpu.model"},{"line_number":697,"context_line":"        for flag in set(x.lower() for x in CONF.libvirt.cpu_model_extra_flags):"},{"line_number":698,"context_line":"            # NOTE(kchamart) While computing CPU compatibility, the"},{"line_number":699,"context_line":"            # below will take into account a comma-separated list of CPU"},{"line_number":700,"context_line":"            # flags from `[libvirt]/cpu_model_extra_flags`.  If the CPU"},{"line_number":701,"context_line":"            # flag starts with \u0027+\u0027, it is enabled for the guest; if it"},{"line_number":702,"context_line":"            # starts with \u0027-\u0027, it is disabled.  If neither \u0027+\u0027 nor \u0027-\u0027"},{"line_number":703,"context_line":"            # is specified, the CPU flag is enabled."},{"line_number":704,"context_line":"            if flag.startswith(\u0027-\u0027):"},{"line_number":705,"context_line":"                flag \u003d flag.lstrip(\u0027-\u0027)"},{"line_number":706,"context_line":"                cpu.add_feature("},{"line_number":707,"context_line":"                        vconfig.LibvirtConfigGuestCPUFeature(flag,"},{"line_number":708,"context_line":"                            policy\u003d\u0027disable\u0027))"},{"line_number":709,"context_line":"            else:"},{"line_number":710,"context_line":"                flag \u003d flag.lstrip(\u0027+\u0027)"},{"line_number":711,"context_line":"                cpu.add_feature(vconfig.LibvirtConfigGuestCPUFeature(flag))"},{"line_number":712,"context_line":"            try:"},{"line_number":713,"context_line":"                self._compare_cpu(cpu, self._get_cpu_info(), None)"},{"line_number":714,"context_line":"            except exception.InvalidCPUInfo as e:"}],"source_content_type":"text/x-python","patch_set":7,"id":"590975de_a4da74ad","line":711,"range":{"start_line":697,"start_character":2,"end_line":711,"end_character":75},"updated":"2021-02-16 20:30:27.000000000","message":"What about adding this to LibvirtConfigCPU under something like add_extra_flags() given this is duplicated below?","commit_id":"0d80b1b0c794e338bfb1b03b1ac4ab79360cf266"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"05a761496ff5b6f92499c450db9cdd99cab8036c","unresolved":true,"context_lines":[{"line_number":694,"context_line":"        # configured extra_flags"},{"line_number":695,"context_line":"        cpu \u003d vconfig.LibvirtConfigGuestCPU()"},{"line_number":696,"context_line":"        cpu.model \u003d self._host.get_capabilities().host.cpu.model"},{"line_number":697,"context_line":"        for flag in set(x.lower() for x in CONF.libvirt.cpu_model_extra_flags):"},{"line_number":698,"context_line":"            # NOTE(kchamart) While computing CPU compatibility, the"},{"line_number":699,"context_line":"            # below will take into account a comma-separated list of CPU"},{"line_number":700,"context_line":"            # flags from `[libvirt]/cpu_model_extra_flags`.  If the CPU"},{"line_number":701,"context_line":"            # flag starts with \u0027+\u0027, it is enabled for the guest; if it"},{"line_number":702,"context_line":"            # starts with \u0027-\u0027, it is disabled.  If neither \u0027+\u0027 nor \u0027-\u0027"},{"line_number":703,"context_line":"            # is specified, the CPU flag is enabled."},{"line_number":704,"context_line":"            if flag.startswith(\u0027-\u0027):"},{"line_number":705,"context_line":"                flag \u003d flag.lstrip(\u0027-\u0027)"},{"line_number":706,"context_line":"                cpu.add_feature("},{"line_number":707,"context_line":"                        vconfig.LibvirtConfigGuestCPUFeature(flag,"},{"line_number":708,"context_line":"                            policy\u003d\u0027disable\u0027))"},{"line_number":709,"context_line":"            else:"},{"line_number":710,"context_line":"                flag \u003d flag.lstrip(\u0027+\u0027)"},{"line_number":711,"context_line":"                cpu.add_feature(vconfig.LibvirtConfigGuestCPUFeature(flag))"},{"line_number":712,"context_line":"            try:"},{"line_number":713,"context_line":"                self._compare_cpu(cpu, self._get_cpu_info(), None)"},{"line_number":714,"context_line":"            except exception.InvalidCPUInfo as e:"}],"source_content_type":"text/x-python","patch_set":7,"id":"6a82b148_d3fc2108","line":711,"range":{"start_line":697,"start_character":2,"end_line":711,"end_character":75},"in_reply_to":"521a6149_c9e65e11","updated":"2021-02-17 13:21:09.000000000","message":"Another thought. Do we need to keep the config option around? Couldn\u0027t we deprecate it in favour of \u0027[libvirt] cpu_model_enabled_extra_flags\u0027 and \u0027[libvirt] cpu_model_disabled_extra_flags\u0027. It would certainly make this option less of a special flower. I\u0027m not aware of any other option in nova that uses prefixes like this.","commit_id":"0d80b1b0c794e338bfb1b03b1ac4ab79360cf266"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"b9c3036b2870231566bc4de780ceed49a1bb81e6","unresolved":true,"context_lines":[{"line_number":694,"context_line":"        # configured extra_flags"},{"line_number":695,"context_line":"        cpu \u003d vconfig.LibvirtConfigGuestCPU()"},{"line_number":696,"context_line":"        cpu.model \u003d self._host.get_capabilities().host.cpu.model"},{"line_number":697,"context_line":"        for flag in set(x.lower() for x in CONF.libvirt.cpu_model_extra_flags):"},{"line_number":698,"context_line":"            # NOTE(kchamart) While computing CPU compatibility, the"},{"line_number":699,"context_line":"            # below will take into account a comma-separated list of CPU"},{"line_number":700,"context_line":"            # flags from `[libvirt]/cpu_model_extra_flags`.  If the CPU"},{"line_number":701,"context_line":"            # flag starts with \u0027+\u0027, it is enabled for the guest; if it"},{"line_number":702,"context_line":"            # starts with \u0027-\u0027, it is disabled.  If neither \u0027+\u0027 nor \u0027-\u0027"},{"line_number":703,"context_line":"            # is specified, the CPU flag is enabled."},{"line_number":704,"context_line":"            if flag.startswith(\u0027-\u0027):"},{"line_number":705,"context_line":"                flag \u003d flag.lstrip(\u0027-\u0027)"},{"line_number":706,"context_line":"                cpu.add_feature("},{"line_number":707,"context_line":"                        vconfig.LibvirtConfigGuestCPUFeature(flag,"},{"line_number":708,"context_line":"                            policy\u003d\u0027disable\u0027))"},{"line_number":709,"context_line":"            else:"},{"line_number":710,"context_line":"                flag \u003d flag.lstrip(\u0027+\u0027)"},{"line_number":711,"context_line":"                cpu.add_feature(vconfig.LibvirtConfigGuestCPUFeature(flag))"},{"line_number":712,"context_line":"            try:"},{"line_number":713,"context_line":"                self._compare_cpu(cpu, self._get_cpu_info(), None)"},{"line_number":714,"context_line":"            except exception.InvalidCPUInfo as e:"}],"source_content_type":"text/x-python","patch_set":7,"id":"716e9dfa_48edc464","line":711,"range":{"start_line":697,"start_character":2,"end_line":711,"end_character":75},"in_reply_to":"590975de_a4da74ad","updated":"2021-02-17 11:29:37.000000000","message":"Agreed, though I wouldn\u0027t put it in LibvirtConfigCPU - it seems we avoid putting anything except XML-related logic in there, so I see it more as a helper right here in driver.py...\n\nIt would also simplify your unit tests - add a test for the new helper, and just add an assertion that it was called in the other places.","commit_id":"0d80b1b0c794e338bfb1b03b1ac4ab79360cf266"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"2cae237c8a3a6b45ffef63763f1bf833512af1bf","unresolved":true,"context_lines":[{"line_number":694,"context_line":"        # configured extra_flags"},{"line_number":695,"context_line":"        cpu \u003d vconfig.LibvirtConfigGuestCPU()"},{"line_number":696,"context_line":"        cpu.model \u003d self._host.get_capabilities().host.cpu.model"},{"line_number":697,"context_line":"        for flag in set(x.lower() for x in CONF.libvirt.cpu_model_extra_flags):"},{"line_number":698,"context_line":"            # NOTE(kchamart) While computing CPU compatibility, the"},{"line_number":699,"context_line":"            # below will take into account a comma-separated list of CPU"},{"line_number":700,"context_line":"            # flags from `[libvirt]/cpu_model_extra_flags`.  If the CPU"},{"line_number":701,"context_line":"            # flag starts with \u0027+\u0027, it is enabled for the guest; if it"},{"line_number":702,"context_line":"            # starts with \u0027-\u0027, it is disabled.  If neither \u0027+\u0027 nor \u0027-\u0027"},{"line_number":703,"context_line":"            # is specified, the CPU flag is enabled."},{"line_number":704,"context_line":"            if flag.startswith(\u0027-\u0027):"},{"line_number":705,"context_line":"                flag \u003d flag.lstrip(\u0027-\u0027)"},{"line_number":706,"context_line":"                cpu.add_feature("},{"line_number":707,"context_line":"                        vconfig.LibvirtConfigGuestCPUFeature(flag,"},{"line_number":708,"context_line":"                            policy\u003d\u0027disable\u0027))"},{"line_number":709,"context_line":"            else:"},{"line_number":710,"context_line":"                flag \u003d flag.lstrip(\u0027+\u0027)"},{"line_number":711,"context_line":"                cpu.add_feature(vconfig.LibvirtConfigGuestCPUFeature(flag))"},{"line_number":712,"context_line":"            try:"},{"line_number":713,"context_line":"                self._compare_cpu(cpu, self._get_cpu_info(), None)"},{"line_number":714,"context_line":"            except exception.InvalidCPUInfo as e:"}],"source_content_type":"text/x-python","patch_set":7,"id":"bd28c8d6_02c041da","line":711,"range":{"start_line":697,"start_character":2,"end_line":711,"end_character":75},"in_reply_to":"6a82b148_d3fc2108","updated":"2021-02-17 13:44:20.000000000","message":"In case you went with the custom type, this should do the trick. I personally am swinging towards separate config options though.\n\n  class CPUFeatureFlag(types.ConfigType):\n\n      def __init__(self, type_name\u003d\u0027cpu feature flag\u0027):\n          super().__init__(type_name\u003dtype_name)\n\n      def __call__(self, value):\n          if isinstance(value, tuple) and len(value) \u003d\u003d 2:\n              return value\n\n          if not isinstance(value, str):\n              raise ValueError(\u0027unexpected CPU feature flag value %r\u0027 % value)\n\n          if not value.startswith(\u0027+-\u0027):\n              LOG.warning(\n                  \"Support for unprefixed flags has been deprecated. Flags \"\n                  \"should be prefixed with \u0027+\u0027 (enable) or \u0027-\u0027 (disable)\"\n              )\n\n          return value.lower().lstrip(\u0027-+\u0027), value.startswith(\u0027+\u0027)\n\n      def __repr__(self):\n          return \u0027CPU feature flag\u0027\n\n      def _formatter(self, value):\n          return str(value).lower()","commit_id":"0d80b1b0c794e338bfb1b03b1ac4ab79360cf266"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"574d00c08fb211c9773d17aed8e6d7654fcadd40","unresolved":true,"context_lines":[{"line_number":694,"context_line":"        # configured extra_flags"},{"line_number":695,"context_line":"        cpu \u003d vconfig.LibvirtConfigGuestCPU()"},{"line_number":696,"context_line":"        cpu.model \u003d self._host.get_capabilities().host.cpu.model"},{"line_number":697,"context_line":"        for flag in set(x.lower() for x in CONF.libvirt.cpu_model_extra_flags):"},{"line_number":698,"context_line":"            # NOTE(kchamart) While computing CPU compatibility, the"},{"line_number":699,"context_line":"            # below will take into account a comma-separated list of CPU"},{"line_number":700,"context_line":"            # flags from `[libvirt]/cpu_model_extra_flags`.  If the CPU"},{"line_number":701,"context_line":"            # flag starts with \u0027+\u0027, it is enabled for the guest; if it"},{"line_number":702,"context_line":"            # starts with \u0027-\u0027, it is disabled.  If neither \u0027+\u0027 nor \u0027-\u0027"},{"line_number":703,"context_line":"            # is specified, the CPU flag is enabled."},{"line_number":704,"context_line":"            if flag.startswith(\u0027-\u0027):"},{"line_number":705,"context_line":"                flag \u003d flag.lstrip(\u0027-\u0027)"},{"line_number":706,"context_line":"                cpu.add_feature("},{"line_number":707,"context_line":"                        vconfig.LibvirtConfigGuestCPUFeature(flag,"},{"line_number":708,"context_line":"                            policy\u003d\u0027disable\u0027))"},{"line_number":709,"context_line":"            else:"},{"line_number":710,"context_line":"                flag \u003d flag.lstrip(\u0027+\u0027)"},{"line_number":711,"context_line":"                cpu.add_feature(vconfig.LibvirtConfigGuestCPUFeature(flag))"},{"line_number":712,"context_line":"            try:"},{"line_number":713,"context_line":"                self._compare_cpu(cpu, self._get_cpu_info(), None)"},{"line_number":714,"context_line":"            except exception.InvalidCPUInfo as e:"}],"source_content_type":"text/x-python","patch_set":7,"id":"350d18bf_7b883e7b","line":711,"range":{"start_line":697,"start_character":2,"end_line":711,"end_character":75},"in_reply_to":"6a82b148_d3fc2108","updated":"2021-02-17 13:47:53.000000000","message":"this was previously discussed several times in the ptg and why i orginally wondered fi this should have a spec or not because its been an openquestion that has been raised every tiem we disucssed it.\n\npeople have experssed that both are simpler to reason about for different reasons.\n\npart of me likes the +/- sysntax but two option are simpler to document and implemnt\nnot that the parsing is partcally hard currently.\nit also would give a clean way to optionally deprecated the legacy bhvior by deprecating the legacy config option\nif we wanted too.\n\ni was trying to avoid bring this back up but its a valid concern.\ni think the last time we dicussed this in a ptg i was eventually conviced to go the two option route but\ninitally i was in the +/- camp.","commit_id":"0d80b1b0c794e338bfb1b03b1ac4ab79360cf266"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b3cbd1a791f1c4a8a6ac5c20c0f5238deed72026","unresolved":true,"context_lines":[{"line_number":694,"context_line":"        # configured extra_flags"},{"line_number":695,"context_line":"        cpu \u003d vconfig.LibvirtConfigGuestCPU()"},{"line_number":696,"context_line":"        cpu.model \u003d self._host.get_capabilities().host.cpu.model"},{"line_number":697,"context_line":"        for flag in set(x.lower() for x in CONF.libvirt.cpu_model_extra_flags):"},{"line_number":698,"context_line":"            # NOTE(kchamart) While computing CPU compatibility, the"},{"line_number":699,"context_line":"            # below will take into account a comma-separated list of CPU"},{"line_number":700,"context_line":"            # flags from `[libvirt]/cpu_model_extra_flags`.  If the CPU"},{"line_number":701,"context_line":"            # flag starts with \u0027+\u0027, it is enabled for the guest; if it"},{"line_number":702,"context_line":"            # starts with \u0027-\u0027, it is disabled.  If neither \u0027+\u0027 nor \u0027-\u0027"},{"line_number":703,"context_line":"            # is specified, the CPU flag is enabled."},{"line_number":704,"context_line":"            if flag.startswith(\u0027-\u0027):"},{"line_number":705,"context_line":"                flag \u003d flag.lstrip(\u0027-\u0027)"},{"line_number":706,"context_line":"                cpu.add_feature("},{"line_number":707,"context_line":"                        vconfig.LibvirtConfigGuestCPUFeature(flag,"},{"line_number":708,"context_line":"                            policy\u003d\u0027disable\u0027))"},{"line_number":709,"context_line":"            else:"},{"line_number":710,"context_line":"                flag \u003d flag.lstrip(\u0027+\u0027)"},{"line_number":711,"context_line":"                cpu.add_feature(vconfig.LibvirtConfigGuestCPUFeature(flag))"},{"line_number":712,"context_line":"            try:"},{"line_number":713,"context_line":"                self._compare_cpu(cpu, self._get_cpu_info(), None)"},{"line_number":714,"context_line":"            except exception.InvalidCPUInfo as e:"}],"source_content_type":"text/x-python","patch_set":7,"id":"7cceab89_b041cce3","line":711,"range":{"start_line":697,"start_character":2,"end_line":711,"end_character":75},"in_reply_to":"716e9dfa_48edc464","updated":"2021-02-17 11:56:22.000000000","message":"\u003e Agreed, though I wouldn\u0027t put it in LibvirtConfigCPU - it seems we avoid putting anything except XML-related logic in there, so I see it more as a helper right here in driver.py...\n\nLoathe as I am to put more logic into driver.py, this doesn\u0027t feel like it belongs in config.py, no. We could go fancy and provide a new opt type. Currently it\u0027s using \u0027oslo_config.types.String\u0027, but there\u0027s no reason we couldn\u0027t add a new type to return a tuple. This would then become:\n\n  for flag, policy in CONF.libvirt.cpu_model_extra_flags.items():\n      cpu.add_feature(vconfig.LibvirtConfigGuestCPUFeature(flag, policy\u003dpolicy))\n\n\u003e It would also simplify your unit tests - add a test for the new helper, and just add an assertion that it was called in the other places.\n\nWhile we\u0027re doing this, I\u0027d also like to deprecate support for the non-prefixed variants. Having two ways to do the same thing seems confusing. A simple LOG.warning if the flag doesn\u0027t start with + or - should suffice.","commit_id":"0d80b1b0c794e338bfb1b03b1ac4ab79360cf266"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"433db5542611c2169feb87e2f4185be2d9fa9971","unresolved":true,"context_lines":[{"line_number":694,"context_line":"        # configured extra_flags"},{"line_number":695,"context_line":"        cpu \u003d vconfig.LibvirtConfigGuestCPU()"},{"line_number":696,"context_line":"        cpu.model \u003d self._host.get_capabilities().host.cpu.model"},{"line_number":697,"context_line":"        for flag in set(x.lower() for x in CONF.libvirt.cpu_model_extra_flags):"},{"line_number":698,"context_line":"            # NOTE(kchamart) While computing CPU compatibility, the"},{"line_number":699,"context_line":"            # below will take into account a comma-separated list of CPU"},{"line_number":700,"context_line":"            # flags from `[libvirt]/cpu_model_extra_flags`.  If the CPU"},{"line_number":701,"context_line":"            # flag starts with \u0027+\u0027, it is enabled for the guest; if it"},{"line_number":702,"context_line":"            # starts with \u0027-\u0027, it is disabled.  If neither \u0027+\u0027 nor \u0027-\u0027"},{"line_number":703,"context_line":"            # is specified, the CPU flag is enabled."},{"line_number":704,"context_line":"            if flag.startswith(\u0027-\u0027):"},{"line_number":705,"context_line":"                flag \u003d flag.lstrip(\u0027-\u0027)"},{"line_number":706,"context_line":"                cpu.add_feature("},{"line_number":707,"context_line":"                        vconfig.LibvirtConfigGuestCPUFeature(flag,"},{"line_number":708,"context_line":"                            policy\u003d\u0027disable\u0027))"},{"line_number":709,"context_line":"            else:"},{"line_number":710,"context_line":"                flag \u003d flag.lstrip(\u0027+\u0027)"},{"line_number":711,"context_line":"                cpu.add_feature(vconfig.LibvirtConfigGuestCPUFeature(flag))"},{"line_number":712,"context_line":"            try:"},{"line_number":713,"context_line":"                self._compare_cpu(cpu, self._get_cpu_info(), None)"},{"line_number":714,"context_line":"            except exception.InvalidCPUInfo as e:"}],"source_content_type":"text/x-python","patch_set":7,"id":"9df90a0c_aed22b0d","line":711,"range":{"start_line":697,"start_character":2,"end_line":711,"end_character":75},"in_reply_to":"7cceab89_b041cce3","updated":"2021-02-17 12:42:49.000000000","message":"Two things:\n\n(1) I\u0027m going to make a helper method _here_ to reuse the logic; and not in LibvirtConfigCPU()\n\n(2) I think we should just keep the non-prefixed variant too.  No need for deprecation.  It is not confusing: if you intuitive to assume that if a flag is given without any prefix, it is to be enabled — it was the previous expectation.  We keep that intact.","commit_id":"0d80b1b0c794e338bfb1b03b1ac4ab79360cf266"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"63036ddc8418fc7a47051cfaa32646c1198668df","unresolved":true,"context_lines":[{"line_number":694,"context_line":"        # configured extra_flags"},{"line_number":695,"context_line":"        cpu \u003d vconfig.LibvirtConfigGuestCPU()"},{"line_number":696,"context_line":"        cpu.model \u003d self._host.get_capabilities().host.cpu.model"},{"line_number":697,"context_line":"        for flag in set(x.lower() for x in CONF.libvirt.cpu_model_extra_flags):"},{"line_number":698,"context_line":"            # NOTE(kchamart) While computing CPU compatibility, the"},{"line_number":699,"context_line":"            # below will take into account a comma-separated list of CPU"},{"line_number":700,"context_line":"            # flags from `[libvirt]/cpu_model_extra_flags`.  If the CPU"},{"line_number":701,"context_line":"            # flag starts with \u0027+\u0027, it is enabled for the guest; if it"},{"line_number":702,"context_line":"            # starts with \u0027-\u0027, it is disabled.  If neither \u0027+\u0027 nor \u0027-\u0027"},{"line_number":703,"context_line":"            # is specified, the CPU flag is enabled."},{"line_number":704,"context_line":"            if flag.startswith(\u0027-\u0027):"},{"line_number":705,"context_line":"                flag \u003d flag.lstrip(\u0027-\u0027)"},{"line_number":706,"context_line":"                cpu.add_feature("},{"line_number":707,"context_line":"                        vconfig.LibvirtConfigGuestCPUFeature(flag,"},{"line_number":708,"context_line":"                            policy\u003d\u0027disable\u0027))"},{"line_number":709,"context_line":"            else:"},{"line_number":710,"context_line":"                flag \u003d flag.lstrip(\u0027+\u0027)"},{"line_number":711,"context_line":"                cpu.add_feature(vconfig.LibvirtConfigGuestCPUFeature(flag))"},{"line_number":712,"context_line":"            try:"},{"line_number":713,"context_line":"                self._compare_cpu(cpu, self._get_cpu_info(), None)"},{"line_number":714,"context_line":"            except exception.InvalidCPUInfo as e:"}],"source_content_type":"text/x-python","patch_set":7,"id":"521a6149_c9e65e11","line":711,"range":{"start_line":697,"start_character":2,"end_line":711,"end_character":75},"in_reply_to":"7cceab89_b041cce3","updated":"2021-02-17 12:47:04.000000000","message":"well we have to support non prefixed for upgrades.\nwe can deprecate it if we want but we will have to supprot it for a few cycles.\n\n+1 for a helper method that construcs a  vconfig.LibvirtConfigGuestCPU() with all the flags set\nwe could move this into the designer.py file although that proably should also be removed at some point\ndesigner.py would be more corect then config.py and still allow you to move it out of the driver.py.\n\nif you do that then \n\n        cpu \u003d vconfig.LibvirtConfigGuestCPU()\n        cpu.model \u003d self._host.get_capabilities().host.cpu.model\n        for flag in set(x.lower() for x in CONF.libvirt.cpu_model_extra_flags):\n            cpu.add_feature(vconfig.LibvirtConfigCPUFeature(flag))\n\n\nbecomes\n\n        cpu \u003d designer.build_guest_cpu(self._host)\n\nwith all of the new logic contaiend in build_guest_cpu\n\nthat coudl just be a helper in this file.\n\nim not sure i like the idea of an even more complex conf option type for this.\nits valid but i dont think it reusable enough to warrent that.","commit_id":"0d80b1b0c794e338bfb1b03b1ac4ab79360cf266"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"1c80da22b067a0c62273f106fc77319f8be99013","unresolved":true,"context_lines":[{"line_number":4595,"context_line":"        # do fine-grained validation of a certain CPU model + CPU flags"},{"line_number":4596,"context_line":"        # against a specific QEMU binary (the libvirt RFE bug for that:"},{"line_number":4597,"context_line":"        # https://bugzilla.redhat.com/show_bug.cgi?id\u003d1559832)."},{"line_number":4598,"context_line":"        #"},{"line_number":4599,"context_line":"        # NOTE(kchamart) Similar to what was done in"},{"line_number":4600,"context_line":"        # _check_cpu_compatibility(), the below parses a comma-separated"},{"line_number":4601,"context_line":"        # list of CPU flags from `[libvirt]/cpu_model_extra_flags` and"}],"source_content_type":"text/x-python","patch_set":7,"id":"56925537_164907d7","line":4598,"range":{"start_line":4598,"start_character":8,"end_line":4598,"end_character":9},"updated":"2021-02-16 20:30:27.000000000","message":"nit - not required","commit_id":"0d80b1b0c794e338bfb1b03b1ac4ab79360cf266"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"1c80da22b067a0c62273f106fc77319f8be99013","unresolved":true,"context_lines":[{"line_number":4601,"context_line":"        # list of CPU flags from `[libvirt]/cpu_model_extra_flags` and"},{"line_number":4602,"context_line":"        # will selectively enable or disable a given CPU flag for the"},{"line_number":4603,"context_line":"        # guest, before it is launched by Nova."},{"line_number":4604,"context_line":"        for flag in extra_flags:"},{"line_number":4605,"context_line":"            if flag.startswith(\u0027-\u0027):"},{"line_number":4606,"context_line":"                flag \u003d flag.lstrip(\u0027-\u0027)"},{"line_number":4607,"context_line":"                cpu.add_feature("},{"line_number":4608,"context_line":"                    vconfig.LibvirtConfigGuestCPUFeature(flag,"},{"line_number":4609,"context_line":"                        policy\u003d\u0027disable\u0027))"},{"line_number":4610,"context_line":"            else:"},{"line_number":4611,"context_line":"                flag \u003d flag.lstrip(\u0027+\u0027)"},{"line_number":4612,"context_line":"                cpu.add_feature(vconfig.LibvirtConfigGuestCPUFeature(flag))"},{"line_number":4613,"context_line":"        return cpu"},{"line_number":4614,"context_line":""},{"line_number":4615,"context_line":"    def _match_cpu_model_by_flags(self, models, flags):"}],"source_content_type":"text/x-python","patch_set":7,"id":"7f9ebe35_c8bc1f48","line":4612,"range":{"start_line":4604,"start_character":1,"end_line":4612,"end_character":75},"updated":"2021-02-16 20:30:27.000000000","message":"As above, I think we can share this in LibvirtConfigCPU.","commit_id":"0d80b1b0c794e338bfb1b03b1ac4ab79360cf266"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"b6a23471e3ae359efe2dc74ea608cccd44011559","unresolved":true,"context_lines":[{"line_number":657,"context_line":"        self._check_vtpm_support()"},{"line_number":658,"context_line":""},{"line_number":659,"context_line":"    def _edit_cpu_flag(self, flag, cpu):"},{"line_number":660,"context_line":"        # NOTE(kchamart) This helper method will be used while computing"},{"line_number":661,"context_line":"        # guest CPU compatibility.  It will take into account a"},{"line_number":662,"context_line":"        # comma-separated list of CPU flags from"},{"line_number":663,"context_line":"        # `[libvirt]cpu_model_extra_flags`.  If the CPU flag starts"}],"source_content_type":"text/x-python","patch_set":8,"id":"e7e6ee98_71f67f2f","line":660,"updated":"2021-02-18 15:06:42.000000000","message":"Let\u0027s avoid side effects if we can, especially for brand new functions like this. The cleaner way of doing this would be:\n\n  def _make_cpu_feature(self, flag):\n    cpu_feature \u003d \u003cmagic to parse flag and create a LibvirtConfigGuestCPUFeature object\u003e\n    return cpu_feature\n\nAnd then later when you\u0027re actually adding the flags:\n\n  for flag in CONF.libvirt.cpu_model_extra_flags:\n    cpu_feature \u003d self._make_cpu_feature(flag)\n    cpu.add_feature(feature)\n\nOr if you want to be more concise:\n\n  [cpu.add_feature(self._make_cpu_feature(flag)) for flag in flags]","commit_id":"7597c4b97f0e7424583c8eb33f55c5ccf13c6873"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"16cd07d8ff733fdd64bd167c1b5e0822cc6532dc","unresolved":true,"context_lines":[{"line_number":657,"context_line":"        self._check_vtpm_support()"},{"line_number":658,"context_line":""},{"line_number":659,"context_line":"    def _edit_cpu_flag(self, flag, cpu):"},{"line_number":660,"context_line":"        # NOTE(kchamart) This helper method will be used while computing"},{"line_number":661,"context_line":"        # guest CPU compatibility.  It will take into account a"},{"line_number":662,"context_line":"        # comma-separated list of CPU flags from"},{"line_number":663,"context_line":"        # `[libvirt]cpu_model_extra_flags`.  If the CPU flag starts"}],"source_content_type":"text/x-python","patch_set":8,"id":"9dba5128_ed0753b3","line":660,"in_reply_to":"2410d095_1ec60927","updated":"2021-02-18 17:25:00.000000000","message":"I admittedly don\u0027t have a strong argument in the sense of \"If you do this, bad thing X will happen.\" It\u0027s more that methods modifying things that are passed to them is considered more \"dangerous\" because those kinds of side effects are not obvious. This is one of the more harmless cases, and it\u0027ll probably remain that way for a while. But there\u0027s a reason that functional programming, with functions accepting inputs and returning output (and not changing anything) is considered \"pure\".","commit_id":"7597c4b97f0e7424583c8eb33f55c5ccf13c6873"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"94d8d891a2815b3f451d4c268315a8e0ee500d00","unresolved":true,"context_lines":[{"line_number":657,"context_line":"        self._check_vtpm_support()"},{"line_number":658,"context_line":""},{"line_number":659,"context_line":"    def _edit_cpu_flag(self, flag, cpu):"},{"line_number":660,"context_line":"        # NOTE(kchamart) This helper method will be used while computing"},{"line_number":661,"context_line":"        # guest CPU compatibility.  It will take into account a"},{"line_number":662,"context_line":"        # comma-separated list of CPU flags from"},{"line_number":663,"context_line":"        # `[libvirt]cpu_model_extra_flags`.  If the CPU flag starts"}],"source_content_type":"text/x-python","patch_set":8,"id":"647236ff_ef1a7296","line":660,"in_reply_to":"3e516145_e50217b7","updated":"2021-02-19 12:32:49.000000000","message":"on methods modifying there inputs, the general rule it should either moficy its inputs or return something not both.\nthis just modifies its inputs but in my previous review i was expecting you to add a function that constucts and returns a new object not modifes in place.\n\n\ni would much rather you repaced this with\n    def generate_guest_cpu(self):\n        cpu \u003d vconfig.LibvirtConfigGuestCPU()\n        cpu.model \u003d self._host.get_capabilities().host.cpu.model\n        for flag in set(x.lower() for x in CONF.libvirt.cpu_model_extra_flags):\n            if flag.startswith(\u0027-\u0027):\n                flag \u003d flag.lstrip(\u0027-\u0027)\n                policy_value \u003d \u0027disable\u0027\n            else:\n                flag \u003d flag.lstrip(\u0027+\u0027)\n                policy_value \u003d \u0027require\u0027\n        cpu.add_feature(\n            vconfig.LibvirtConfigGuestCPUFeature(flag,\n                policy\u003dpolicy_value))\n            try:\n                self._compare_cpu(cpu, self._get_cpu_info(), None)\n            except exception.InvalidCPUInfo as e:\n                msg \u003d (_(\"Configured extra flag: %(flag)s it not correct, or \"\n                         \"the host CPU does not support this flag. Please \"\n                         \"correct the config and try again. %(e)s\") % {\n                            \u0027flag\u0027: flag, \u0027e\u0027: e})\n                raise exception.InvalidCPUInfo(msg)\n          return cpu\n\ni think this was the name i suggetgested before or similar.\n\nif you keep it as is i would change the name to _update_cpu_flags","commit_id":"7597c4b97f0e7424583c8eb33f55c5ccf13c6873"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"d15948cef7ffef4269279bad1739fa37e62de1f4","unresolved":true,"context_lines":[{"line_number":657,"context_line":"        self._check_vtpm_support()"},{"line_number":658,"context_line":""},{"line_number":659,"context_line":"    def _edit_cpu_flag(self, flag, cpu):"},{"line_number":660,"context_line":"        # NOTE(kchamart) This helper method will be used while computing"},{"line_number":661,"context_line":"        # guest CPU compatibility.  It will take into account a"},{"line_number":662,"context_line":"        # comma-separated list of CPU flags from"},{"line_number":663,"context_line":"        # `[libvirt]cpu_model_extra_flags`.  If the CPU flag starts"}],"source_content_type":"text/x-python","patch_set":8,"id":"b1314d94_71f88621","line":660,"in_reply_to":"647236ff_ef1a7296","updated":"2021-02-19 14:28:41.000000000","message":"\u003e on methods modifying there inputs, the general rule it should either moficy its inputs or return something not both.\n\u003e this just modifies its inputs but in my previous review i was expecting you to add a function that constucts and returns a new object not modifes in place.\n\u003e \n\u003e \n\u003e i would much rather you repaced this with\n\u003e     def generate_guest_cpu(self):\n\u003e         cpu \u003d vconfig.LibvirtConfigGuestCPU()\n\u003e         cpu.model \u003d self._host.get_capabilities().host.cpu.model\n\u003e         for flag in set(x.lower() for x in CONF.libvirt.cpu_model_extra_flags):\n\u003e             if flag.startswith(\u0027-\u0027):\n\u003e                 flag \u003d flag.lstrip(\u0027-\u0027)\n\u003e                 policy_value \u003d \u0027disable\u0027\n\u003e             else:\n\u003e                 flag \u003d flag.lstrip(\u0027+\u0027)\n\u003e                 policy_value \u003d \u0027require\u0027\n\u003e         cpu.add_feature(\n\u003e             vconfig.LibvirtConfigGuestCPUFeature(flag,\n\u003e                 policy\u003dpolicy_value))\n\u003e             try:\n\u003e                 self._compare_cpu(cpu, self._get_cpu_info(), None)\n\u003e             except exception.InvalidCPUInfo as e:\n\u003e                 msg \u003d (_(\"Configured extra flag: %(flag)s it not correct, or \"\n\u003e                          \"the host CPU does not support this flag. Please \"\n\u003e                          \"correct the config and try again. %(e)s\") % {\n\u003e                             \u0027flag\u0027: flag, \u0027e\u0027: e})\n\u003e                 raise exception.InvalidCPUInfo(msg)\n\u003e           return cpu\n\nI did consider this, but went against it for good reasons: it creates a rather noisy `diff`, due to correcting tests, etc.  And further, the above means, _compare_cpu() will end up getting called again in _get_guest_cpu(), which will need further modification.  Which again, creates more noise.\n\nI\u0027d like to keep the refactoring small.  (Bear in mind the backporting aspect.)\n\nAgain, as noted earlier, we\u0027d be replacing parts of this code *anyway* with the newer APIs.  So it\u0027s just not worth it to do it now.\n\n\u003e i think this was the name i suggetgested before or similar.\n\u003e \n\u003e if you keep it as is i would change the name to _update_cpu_flags\n\nI\u0027m going with Artom\u0027s nice-to-have suggestion; so locally I went with _prepare_cpu_flag(), which is simliar to \"update\"; hope that works for you too.\n\n\n    def _prepare_cpu_flag(self, flag):\n        if flag.startswith(\u0027-\u0027):\n            flag \u003d flag.lstrip(\u0027-\u0027)\n            policy_value \u003d \u0027disable\u0027\n        else:\n            flag \u003d flag.lstrip(\u0027+\u0027)\n            policy_value \u003d \u0027require\u0027\n\n        cpu_feature \u003d vconfig.LibvirtConfigGuestCPUFeature(\n                        flag, policy\u003dpolicy_value)\n        return cpu_feature\n\nAnd then, later in the other two methods, call:\n\n    cpu_feature \u003d self._prepare_cpu_flag(flag)\n    cpu.add_feature(cpu_feature)\n\nPS: I\u0027m on PTO next week; will come back and follow through.","commit_id":"7597c4b97f0e7424583c8eb33f55c5ccf13c6873"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"86d8559497920ce6fae2f65e4a671c7f01587cb2","unresolved":true,"context_lines":[{"line_number":657,"context_line":"        self._check_vtpm_support()"},{"line_number":658,"context_line":""},{"line_number":659,"context_line":"    def _edit_cpu_flag(self, flag, cpu):"},{"line_number":660,"context_line":"        # NOTE(kchamart) This helper method will be used while computing"},{"line_number":661,"context_line":"        # guest CPU compatibility.  It will take into account a"},{"line_number":662,"context_line":"        # comma-separated list of CPU flags from"},{"line_number":663,"context_line":"        # `[libvirt]cpu_model_extra_flags`.  If the CPU flag starts"}],"source_content_type":"text/x-python","patch_set":8,"id":"3e516145_e50217b7","line":660,"in_reply_to":"78bed7b6_2bf6bf1b","updated":"2021-02-19 11:20:48.000000000","message":"\u003e Fastidiousness is a virtue when working on a project with hundreds of contributors and tens or hundreds of thousands of lines of code. Without agreed upon best practices, code quickly devolves into an unmaintainable mush.\n\nBeing meticulous is a virtue; but as with anything, moderation matters.  I\u0027m all for maintainability, even to the point that I often emphasize it over new features.  And the term \"best practices\" is loaded, and doesn\u0027t mean very much until it is unpacked.\n\n\u003e \u003e I admittedly don\u0027t have a strong argument in the sense of \"If you do this, bad thing X will happen.\" It\u0027s more that methods modifying things that are passed to them is considered more \"dangerous\" because those kinds of side effects are not obvious. This is one of the more harmless cases, and it\u0027ll probably remain that way for a while. But there\u0027s a reason that functional programming, with functions accepting inputs and returning output (and not changing anything) is considered \"pure\".\n\nI see what you mean; but let me know if you\u0027re insisting on rewriting this part, as this way is not unheard of in our codebase (as Stephen points out below).\n\n\u003e In general, I\u0027d agree with you. This isn\u0027t C, after all. With that said, we have a large number of \u0027_add_foo_device\u0027 and \u0027_guest_add_foo\u0027 helper methods that rely on this pattern so it\u0027s not unheard of and I could let this slide. The name of the function, on the other hand, could do with some work. It\u0027s too generic and doesn\u0027t give us any information about what\u0027s going on. Even \u0027_set_cpu_flag\u0027 would be better. Also, the arguments are backwards. Both \u0027_set_cpu_flag\u0027 and \u0027_edit_cpu_flag\u0027 suggest cpu should come first followed by the flag\n\nOn the name of the function, I did ask suggestions earlier :-)  I first went with _add_or_remove_cpu_flag(), but _set_cpu_flag(). is also nice.  Thanks!","commit_id":"7597c4b97f0e7424583c8eb33f55c5ccf13c6873"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"dd4f86d48c6f8c9aef9b21b620d7f3b5f8c19988","unresolved":true,"context_lines":[{"line_number":657,"context_line":"        self._check_vtpm_support()"},{"line_number":658,"context_line":""},{"line_number":659,"context_line":"    def _edit_cpu_flag(self, flag, cpu):"},{"line_number":660,"context_line":"        # NOTE(kchamart) This helper method will be used while computing"},{"line_number":661,"context_line":"        # guest CPU compatibility.  It will take into account a"},{"line_number":662,"context_line":"        # comma-separated list of CPU flags from"},{"line_number":663,"context_line":"        # `[libvirt]cpu_model_extra_flags`.  If the CPU flag starts"}],"source_content_type":"text/x-python","patch_set":8,"id":"78bed7b6_2bf6bf1b","line":660,"in_reply_to":"9dba5128_ed0753b3","updated":"2021-02-18 18:30:08.000000000","message":"\u003e You do sound a little too \"fastidious\" :-)\n\nFastidiousness is a virtue when working on a project with hundreds of contributors and tens or hundreds of thousands of lines of code. Without agreed upon best practices, code quickly devolves into an unmaintainable mush.\n\n\u003e I admittedly don\u0027t have a strong argument in the sense of \"If you do this, bad thing X will happen.\" It\u0027s more that methods modifying things that are passed to them is considered more \"dangerous\" because those kinds of side effects are not obvious. This is one of the more harmless cases, and it\u0027ll probably remain that way for a while. But there\u0027s a reason that functional programming, with functions accepting inputs and returning output (and not changing anything) is considered \"pure\".\n\nIn general, I\u0027d agree with you. This isn\u0027t C, after all. With that said, we have a large number of \u0027_add_foo_device\u0027 and \u0027_guest_add_foo\u0027 helper methods that rely on this pattern so it\u0027s not unheard of and I could let this slide. The name of the function, on the other hand, could do with some work. It\u0027s too generic and doesn\u0027t give us any information about what\u0027s going on. Even \u0027_set_cpu_flag\u0027 would be better. Also, the arguments are backwards. Both \u0027_set_cpu_flag\u0027 and \u0027_edit_cpu_flag\u0027 suggest cpu should come first followed by the flag","commit_id":"7597c4b97f0e7424583c8eb33f55c5ccf13c6873"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"3c37e36e3da91c20861d0e9bca54b625d068d633","unresolved":true,"context_lines":[{"line_number":657,"context_line":"        self._check_vtpm_support()"},{"line_number":658,"context_line":""},{"line_number":659,"context_line":"    def _edit_cpu_flag(self, flag, cpu):"},{"line_number":660,"context_line":"        # NOTE(kchamart) This helper method will be used while computing"},{"line_number":661,"context_line":"        # guest CPU compatibility.  It will take into account a"},{"line_number":662,"context_line":"        # comma-separated list of CPU flags from"},{"line_number":663,"context_line":"        # `[libvirt]cpu_model_extra_flags`.  If the CPU flag starts"}],"source_content_type":"text/x-python","patch_set":8,"id":"8e663756_08cd99b2","line":660,"in_reply_to":"b1314d94_71f88621","updated":"2021-02-19 14:50:31.000000000","message":"yes that works since it returns the constucted cpu feate.\n\nthis will not be backported upstream and i dont think that is a good argument in this case\nas the test change are not nessalary large. just because you factored out some logic into a funciton does not mean you ahve to update all the tests. it should not be viabel to the tests for the most part.\n\nanyway im ok with the _prepare_cpu_flag name with it returnign a LibvirtConfigGuestCPUFeature object","commit_id":"7597c4b97f0e7424583c8eb33f55c5ccf13c6873"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"9d12fe47088b112009ed6617697c7dadff4a9d6b","unresolved":true,"context_lines":[{"line_number":657,"context_line":"        self._check_vtpm_support()"},{"line_number":658,"context_line":""},{"line_number":659,"context_line":"    def _edit_cpu_flag(self, flag, cpu):"},{"line_number":660,"context_line":"        # NOTE(kchamart) This helper method will be used while computing"},{"line_number":661,"context_line":"        # guest CPU compatibility.  It will take into account a"},{"line_number":662,"context_line":"        # comma-separated list of CPU flags from"},{"line_number":663,"context_line":"        # `[libvirt]cpu_model_extra_flags`.  If the CPU flag starts"}],"source_content_type":"text/x-python","patch_set":8,"id":"2410d095_1ec60927","line":660,"in_reply_to":"e7e6ee98_71f67f2f","updated":"2021-02-18 17:07:49.000000000","message":"You do sound a little too \"fastidious\" :-) I did think of going that way; but I\u0027m not sure if what you\u0027re saying buys us much.  Unless you can convince me of any meaningful (or otherwise) side effects.","commit_id":"7597c4b97f0e7424583c8eb33f55c5ccf13c6873"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"dd4f86d48c6f8c9aef9b21b620d7f3b5f8c19988","unresolved":true,"context_lines":[{"line_number":4600,"context_line":"        # upstream libvirt intends to add an additional new API that can"},{"line_number":4601,"context_line":"        # do fine-grained validation of a certain CPU model + CPU flags"},{"line_number":4602,"context_line":"        # against a specific QEMU binary (the libvirt RFE bug for that:"},{"line_number":4603,"context_line":"        # https://bugzilla.redhat.com/show_bug.cgi?id\u003d1559832)."},{"line_number":4604,"context_line":"        #"},{"line_number":4605,"context_line":"        # NOTE(kchamart) Similar to what was done in"},{"line_number":4606,"context_line":"        # _check_cpu_compatibility(), the below parses a comma-separated"}],"source_content_type":"text/x-python","patch_set":8,"id":"b5515498_6ec3dbc1","line":4603,"updated":"2021-02-18 18:30:08.000000000","message":"Unrelated: the presence of the \u0027_compare_cpu\u0027 function above suggests this is no longer the case and these two code paths should be merged in the near term?","commit_id":"7597c4b97f0e7424583c8eb33f55c5ccf13c6873"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"86d8559497920ce6fae2f65e4a671c7f01587cb2","unresolved":true,"context_lines":[{"line_number":4600,"context_line":"        # upstream libvirt intends to add an additional new API that can"},{"line_number":4601,"context_line":"        # do fine-grained validation of a certain CPU model + CPU flags"},{"line_number":4602,"context_line":"        # against a specific QEMU binary (the libvirt RFE bug for that:"},{"line_number":4603,"context_line":"        # https://bugzilla.redhat.com/show_bug.cgi?id\u003d1559832)."},{"line_number":4604,"context_line":"        #"},{"line_number":4605,"context_line":"        # NOTE(kchamart) Similar to what was done in"},{"line_number":4606,"context_line":"        # _check_cpu_compatibility(), the below parses a comma-separated"}],"source_content_type":"text/x-python","patch_set":8,"id":"1468f6ae_cd9200c7","line":4603,"in_reply_to":"b5515498_6ec3dbc1","updated":"2021-02-19 11:20:48.000000000","message":"I am aware of this stale comment, and I intentionally did not merge it.  Because it should removed or rewritten; libvirt upstream has now added what I\u0027m asking here (see the solved libvirt bug linked).  It is provided by the newer libvirt CPU APIs: compareHypervisorCPU() and baselineHypervisorCPU() --\n\nI have a TODO to rework this as part of splitting out a later patch (started by chengsheng): https://review.opendev.org/c/openstack/nova/+/762330/","commit_id":"7597c4b97f0e7424583c8eb33f55c5ccf13c6873"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5cd7b267b3b86f3585e1d99ece2c746fce65a7c1","unresolved":true,"context_lines":[{"line_number":694,"context_line":"            policy_value \u003d \u0027require\u0027"},{"line_number":695,"context_line":""},{"line_number":696,"context_line":"        cpu_feature \u003d vconfig.LibvirtConfigGuestCPUFeature("},{"line_number":697,"context_line":"                        flag, policy\u003dpolicy_value)"},{"line_number":698,"context_line":"        return cpu_feature"},{"line_number":699,"context_line":""},{"line_number":700,"context_line":"    def _check_cpu_compatibility(self):"}],"source_content_type":"text/x-python","patch_set":11,"id":"20c0333d_d35d89f9","line":697,"range":{"start_line":697,"start_character":12,"end_line":697,"end_character":24},"updated":"2021-03-04 15:45:24.000000000","message":"whoops","commit_id":"d897ce2a545fd1b774d6c65017e71c016606eedf"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"8464e991819af36df45d789d556c10717a0a6ccb","unresolved":true,"context_lines":[{"line_number":694,"context_line":"            policy_value \u003d \u0027require\u0027"},{"line_number":695,"context_line":""},{"line_number":696,"context_line":"        cpu_feature \u003d vconfig.LibvirtConfigGuestCPUFeature("},{"line_number":697,"context_line":"                        flag, policy\u003dpolicy_value)"},{"line_number":698,"context_line":"        return cpu_feature"},{"line_number":699,"context_line":""},{"line_number":700,"context_line":"    def _check_cpu_compatibility(self):"}],"source_content_type":"text/x-python","patch_set":12,"id":"a618f485_f41c1a64","line":697,"updated":"2021-03-04 17:11:11.000000000","message":"Ugly indent","commit_id":"bcd6b42047ea9422a58a4273d831e23f2ea27092"}],"releasenotes/notes/allow-disabling-cpu-flags-cc861a3bdfffadf8.yaml":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ed4339589ddcd00ed1b636bfd2c3b492c41d190a","unresolved":true,"context_lines":[{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    The libvirt driver now allows explicitly disabling CPU flags for"},{"line_number":5,"context_line":"    guests vi the ``libvirt]/cpu_model_extra_flags`` config attribute."},{"line_number":6,"context_line":"    This is possible via a \"+\" / \"-\" notation, where if you specify a"},{"line_number":7,"context_line":"    CPU flag prefixed with a \"+\" sign (without quotes), it will be"},{"line_number":8,"context_line":"    enabled for the guest, a prefix of \"-\" will disable it, and if"}],"source_content_type":"text/x-yaml","patch_set":5,"id":"2bad5639_af468c55","line":5,"range":{"start_line":5,"start_character":18,"end_line":5,"end_character":28},"updated":"2021-02-09 12:39:58.000000000","message":"``[libvirt]","commit_id":"5a450a0ab3a7428aea839f38f5ef70726590ccc1"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ed4339589ddcd00ed1b636bfd2c3b492c41d190a","unresolved":true,"context_lines":[{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    The libvirt driver now allows explicitly disabling CPU flags for"},{"line_number":5,"context_line":"    guests vi the ``libvirt]/cpu_model_extra_flags`` config attribute."},{"line_number":6,"context_line":"    This is possible via a \"+\" / \"-\" notation, where if you specify a"},{"line_number":7,"context_line":"    CPU flag prefixed with a \"+\" sign (without quotes), it will be"},{"line_number":8,"context_line":"    enabled for the guest, a prefix of \"-\" will disable it, and if"}],"source_content_type":"text/x-yaml","patch_set":5,"id":"f7be322e_7c8b2635","line":5,"range":{"start_line":5,"start_character":11,"end_line":5,"end_character":13},"updated":"2021-02-09 12:39:58.000000000","message":"via","commit_id":"5a450a0ab3a7428aea839f38f5ef70726590ccc1"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9da1a3ef70a4131c3db0e37cf2f26fa6504b69b5","unresolved":true,"context_lines":[{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    The libvirt driver now allows explicitly disabling CPU flags for"},{"line_number":5,"context_line":"    guests via the ``libvirt]/cpu_model_extra_flags`` config attribute."},{"line_number":6,"context_line":"    This is possible via a \"+\" / \"-\" notation, where if you specify a"},{"line_number":7,"context_line":"    CPU flag prefixed with a \"+\" sign (without quotes), it will be"},{"line_number":8,"context_line":"    enabled for the guest, a prefix of \"-\" will disable it, and if"}],"source_content_type":"text/x-yaml","patch_set":6,"id":"04735f26_d108d3a5","line":5,"range":{"start_line":5,"start_character":19,"end_line":5,"end_character":53},"updated":"2021-02-15 18:44:23.000000000","message":"You missed a bracket:\n\n  ``[libvirt] cpu_model_extra_flags``","commit_id":"5f3742475a0c5491194e14dd3c0c23aac8897319"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9da1a3ef70a4131c3db0e37cf2f26fa6504b69b5","unresolved":true,"context_lines":[{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    The libvirt driver now allows explicitly disabling CPU flags for"},{"line_number":5,"context_line":"    guests via the ``libvirt]/cpu_model_extra_flags`` config attribute."},{"line_number":6,"context_line":"    This is possible via a \"+\" / \"-\" notation, where if you specify a"},{"line_number":7,"context_line":"    CPU flag prefixed with a \"+\" sign (without quotes), it will be"},{"line_number":8,"context_line":"    enabled for the guest, a prefix of \"-\" will disable it, and if"},{"line_number":9,"context_line":"    neither \"+\" nor \"-\" is specified, the CPU flag will be enabled,"}],"source_content_type":"text/x-yaml","patch_set":6,"id":"6f4d0a30_aa9998fc","line":6,"range":{"start_line":6,"start_character":27,"end_line":6,"end_character":36},"updated":"2021-02-15 18:44:23.000000000","message":"``+`` / ``-``\n\n(and below)","commit_id":"5f3742475a0c5491194e14dd3c0c23aac8897319"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"eac60d8655be356015a03f24df1e26693ffc4fe1","unresolved":true,"context_lines":[{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    The libvirt driver now allows explicitly disabling CPU flags for"},{"line_number":5,"context_line":"    guests via the ``libvirt]/cpu_model_extra_flags`` config attribute."},{"line_number":6,"context_line":"    This is possible via a \"+\" / \"-\" notation, where if you specify a"},{"line_number":7,"context_line":"    CPU flag prefixed with a \"+\" sign (without quotes), it will be"},{"line_number":8,"context_line":"    enabled for the guest, a prefix of \"-\" will disable it, and if"},{"line_number":9,"context_line":"    neither \"+\" nor \"-\" is specified, the CPU flag will be enabled,"}],"source_content_type":"text/x-yaml","patch_set":6,"id":"6f27e981_3dd88cb0","line":6,"range":{"start_line":6,"start_character":27,"end_line":6,"end_character":36},"in_reply_to":"6f4d0a30_aa9998fc","updated":"2021-02-16 12:46:03.000000000","message":"Yep; will fix all.","commit_id":"5f3742475a0c5491194e14dd3c0c23aac8897319"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9da1a3ef70a4131c3db0e37cf2f26fa6504b69b5","unresolved":true,"context_lines":[{"line_number":13,"context_line":"    x86_64`` on the compute node to see what models are supported on"},{"line_number":14,"context_line":"    your host) and the following CPU flags:"},{"line_number":15,"context_line":"    ``[libvirt]/cpu_model_extra_flags \u003d +pcid,-mtrr,ssbd`` in"},{"line_number":16,"context_line":"    ``nova.conf``.  Then the \"pcid\" flag will be enabled for the guest,"},{"line_number":17,"context_line":"    \"mttr\" disabled, and \"ssbd\" will be enabled, because it was given"},{"line_number":18,"context_line":"    with no prefix sign."},{"line_number":19,"context_line":""}],"source_content_type":"text/x-yaml","patch_set":6,"id":"d990d9be_87cb5428","line":16,"range":{"start_line":16,"start_character":29,"end_line":16,"end_character":35},"updated":"2021-02-15 18:44:23.000000000","message":"``pcid``","commit_id":"5f3742475a0c5491194e14dd3c0c23aac8897319"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9da1a3ef70a4131c3db0e37cf2f26fa6504b69b5","unresolved":true,"context_lines":[{"line_number":14,"context_line":"    your host) and the following CPU flags:"},{"line_number":15,"context_line":"    ``[libvirt]/cpu_model_extra_flags \u003d +pcid,-mtrr,ssbd`` in"},{"line_number":16,"context_line":"    ``nova.conf``.  Then the \"pcid\" flag will be enabled for the guest,"},{"line_number":17,"context_line":"    \"mttr\" disabled, and \"ssbd\" will be enabled, because it was given"},{"line_number":18,"context_line":"    with no prefix sign."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"    Refer to the ``[libvirt]/cpu_model_extra_flags`` documentation for a"}],"source_content_type":"text/x-yaml","patch_set":6,"id":"dcfcabd7_8f3d624a","line":17,"range":{"start_line":17,"start_character":11,"end_line":17,"end_character":19},"updated":"2021-02-15 18:44:23.000000000","message":"will be disabled","commit_id":"5f3742475a0c5491194e14dd3c0c23aac8897319"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9da1a3ef70a4131c3db0e37cf2f26fa6504b69b5","unresolved":true,"context_lines":[{"line_number":18,"context_line":"    with no prefix sign."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"    Refer to the ``[libvirt]/cpu_model_extra_flags`` documentation for a"},{"line_number":21,"context_line":"    fuller example, and related details."}],"source_content_type":"text/x-yaml","patch_set":6,"id":"a605e342_743a639b","line":21,"range":{"start_line":21,"start_character":4,"end_line":21,"end_character":40},"updated":"2021-02-15 18:44:23.000000000","message":"more information.","commit_id":"5f3742475a0c5491194e14dd3c0c23aac8897319"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"433db5542611c2169feb87e2f4185be2d9fa9971","unresolved":false,"context_lines":[{"line_number":18,"context_line":"    with no prefix sign."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"    Refer to the ``[libvirt]/cpu_model_extra_flags`` documentation for a"},{"line_number":21,"context_line":"    fuller example, and related details."}],"source_content_type":"text/x-yaml","patch_set":6,"id":"094377da_d39460f5","line":21,"range":{"start_line":21,"start_character":4,"end_line":21,"end_character":40},"in_reply_to":"a605e342_743a639b","updated":"2021-02-17 12:42:49.000000000","message":"Ack","commit_id":"5f3742475a0c5491194e14dd3c0c23aac8897319"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b3cbd1a791f1c4a8a6ac5c20c0f5238deed72026","unresolved":true,"context_lines":[{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    The libvirt driver now allows explicitly disabling CPU flags for"},{"line_number":5,"context_line":"    guests via the ``[libvirt]/cpu_model_extra_flags`` config attribute."},{"line_number":6,"context_line":"    This is possible via a ``+`` / ``-`` notation, where if you specify"},{"line_number":7,"context_line":"    a CPU flag prefixed with a ``+`` sign (without quotes), it will be"},{"line_number":8,"context_line":"    enabled for the guest, a prefix of ``-`` will disable it, and if"}],"source_content_type":"text/x-yaml","patch_set":7,"id":"dfd1fb15_3c6f50e0","line":5,"range":{"start_line":5,"start_character":30,"end_line":5,"end_character":31},"updated":"2021-02-17 11:56:22.000000000","message":"nit: unnecessary (the group is already delineated by the square brackets)","commit_id":"0d80b1b0c794e338bfb1b03b1ac4ab79360cf266"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5cd7b267b3b86f3585e1d99ece2c746fce65a7c1","unresolved":true,"context_lines":[{"line_number":5,"context_line":"    guests via the ``[libvirt]cpu_model_extra_flags`` config attribute."},{"line_number":6,"context_line":"    This is possible via a ``+`` / ``-`` notation, where if you specify"},{"line_number":7,"context_line":"    a CPU flag prefixed with a ``+`` sign (without quotes), it will be"},{"line_number":8,"context_line":"    enabled for the guest, a prefix of ``-`` will disable it, and if"},{"line_number":9,"context_line":"    neither ``+`` nor ``-`` is specified, the CPU flag will be enabled,"},{"line_number":10,"context_line":"    which is the default behaviour."},{"line_number":11,"context_line":""}],"source_content_type":"text/x-yaml","patch_set":11,"id":"db4273cd_be1d64b1","line":8,"range":{"start_line":8,"start_character":60,"end_line":8,"end_character":65},"updated":"2021-03-04 15:45:24.000000000","message":"it. If neither..","commit_id":"d897ce2a545fd1b774d6c65017e71c016606eedf"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5cd7b267b3b86f3585e1d99ece2c746fce65a7c1","unresolved":true,"context_lines":[{"line_number":5,"context_line":"    guests via the ``[libvirt]cpu_model_extra_flags`` config attribute."},{"line_number":6,"context_line":"    This is possible via a ``+`` / ``-`` notation, where if you specify"},{"line_number":7,"context_line":"    a CPU flag prefixed with a ``+`` sign (without quotes), it will be"},{"line_number":8,"context_line":"    enabled for the guest, a prefix of ``-`` will disable it, and if"},{"line_number":9,"context_line":"    neither ``+`` nor ``-`` is specified, the CPU flag will be enabled,"},{"line_number":10,"context_line":"    which is the default behaviour."},{"line_number":11,"context_line":""}],"source_content_type":"text/x-yaml","patch_set":11,"id":"d8c32139_e632bb83","line":8,"range":{"start_line":8,"start_character":25,"end_line":8,"end_character":27},"updated":"2021-03-04 15:45:24.000000000","message":"while","commit_id":"d897ce2a545fd1b774d6c65017e71c016606eedf"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5cd7b267b3b86f3585e1d99ece2c746fce65a7c1","unresolved":true,"context_lines":[{"line_number":10,"context_line":"    which is the default behaviour."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"    For example, specify a custom CPU model (refer to ``virsh cpu-models"},{"line_number":13,"context_line":"    x86_64`` on the compute node to see what models are supported on"},{"line_number":14,"context_line":"    your host) and the following CPU flags:"},{"line_number":15,"context_line":"    ``[libvirt]cpu_model_extra_flags \u003d +pcid,-mtrr,ssbd`` in"},{"line_number":16,"context_line":"    ``nova.conf``.  Then ``pcid`` will be enabled for the"}],"source_content_type":"text/x-yaml","patch_set":11,"id":"47618fde_814067bb","line":13,"range":{"start_line":13,"start_character":4,"end_line":13,"end_character":10},"updated":"2021-03-04 15:45:24.000000000","message":"nit:\n\n  ARCH``, where ARCH is your host architecture,\n\n(this step is meaningless on non-x86_64 hosts otherwise)","commit_id":"d897ce2a545fd1b774d6c65017e71c016606eedf"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5cd7b267b3b86f3585e1d99ece2c746fce65a7c1","unresolved":true,"context_lines":[{"line_number":15,"context_line":"    ``[libvirt]cpu_model_extra_flags \u003d +pcid,-mtrr,ssbd`` in"},{"line_number":16,"context_line":"    ``nova.conf``.  Then ``pcid`` will be enabled for the"},{"line_number":17,"context_line":"    guest, ``mttr`` will be disabled, and ``ssbd`` will be enabled,"},{"line_number":18,"context_line":"    because it was given with no prefix sign."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"    Refer to the ``[libvirt]cpu_model_extra_flags`` documentation for"},{"line_number":21,"context_line":"    more information."}],"source_content_type":"text/x-yaml","patch_set":11,"id":"cbee93cb_70246272","line":18,"updated":"2021-03-04 15:45:24.000000000","message":"Actually, we could nearly just drop this entire paragraph. These are release notes and should provide a summary of the feature. More expansive docs can be and already are provided in the main docs.","commit_id":"d897ce2a545fd1b774d6c65017e71c016606eedf"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"9b0bb29855a4a2793b35d9e0e01d843dc24752e3","unresolved":true,"context_lines":[{"line_number":15,"context_line":"    ``[libvirt]cpu_model_extra_flags \u003d +pcid,-mtrr,ssbd`` in"},{"line_number":16,"context_line":"    ``nova.conf``.  Then ``pcid`` will be enabled for the"},{"line_number":17,"context_line":"    guest, ``mttr`` will be disabled, and ``ssbd`` will be enabled,"},{"line_number":18,"context_line":"    because it was given with no prefix sign."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"    Refer to the ``[libvirt]cpu_model_extra_flags`` documentation for"},{"line_number":21,"context_line":"    more information."}],"source_content_type":"text/x-yaml","patch_set":11,"id":"869db3cc_7b44c2e2","line":18,"in_reply_to":"cbee93cb_70246272","updated":"2021-03-04 16:25:02.000000000","message":"Yeah ... I first elided it, but then included here \"just in case\".\n\nI nuked the above paragraph.","commit_id":"d897ce2a545fd1b774d6c65017e71c016606eedf"}]}