)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"34348f422349d00be9fb126a9254a4cc8ea7ddd2","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"libvirt: add support for virtio-net rx/tx queue sizes"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Add support for configuring the `rx_queue_size` and `tx_queue_size`"},{"line_number":10,"context_line":"options in the virtio-net driver by way of image metadata"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Implements: bp libvirt-virtio-set-queue-sizes"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"ff346bd7_84886bc6","line":9,"range":{"start_line":9,"start_character":52,"end_line":9,"end_character":67},"updated":"2017-07-24 14:14:31.000000000","message":"from what i can tell tx_queue_size is not configurable with qemu currently \n\nsee http://patchwork.ozlabs.org/patch/771133/ which discusses enableing that.","commit_id":"01a15b24c9261520f623f7235a68495dde3ea0e0"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"3ac73486941cf28f2aa457dd5fb35c6bb5102fc0","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"libvirt: add support for virtio-net rx/tx queue sizes"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Add support for configuring the `rx_queue_size` and `tx_queue_size`"},{"line_number":10,"context_line":"options in the virtio-net driver by way of image metadata"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Implements: bp libvirt-virtio-set-queue-sizes"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"df3967d1_5cc3e957","line":9,"updated":"2017-08-04 12:22:24.000000000","message":"This is not how we should handle such option.\n\nThere are some concerns about mixing several queue size on the same segment of network. We should not provide such tunable to users since there are performances implications on the compute host.\n\nBasically that option should be provided by host. and operator should put the hosts on different aggregate. So we can have the \"default\" aggregate which leaves the rx/tx queue size alone and a \"performance\" aggregate where all the VMs will take advantage of that option.","commit_id":"d4d3d8af683555bcb34df6c87c26c302bbfaa2cc"},{"author":{"_account_id":16351,"name":"Jan Scheurich","email":"jan.scheurich@ericsson.com","username":"jan.scheurich"},"change_message_id":"3f80cf98175e8f940b157a0692915e3f9c452782","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"libvirt: add support for virtio-net rx/tx queue sizes"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Add support for configuring the `rx_queue_size` and `tx_queue_size`"},{"line_number":10,"context_line":"options in the virtio-net driver by way of image metadata"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Implements: bp libvirt-virtio-set-queue-sizes"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"7f287b81_2f24c8ae","line":9,"in_reply_to":"df3967d1_5cc3e957","updated":"2017-09-05 15:45:08.000000000","message":"Actually, the queue length properties are strictly local to the Qemu virtio-net device and there is no technical reason for all ports on the network segment to have the same queue lengths.\n\nHaving said that, I agree with Balazs (below) that we do not think it appropriate for Nova instance to explicitly request specific queue lengths. That is a performance tuning detail that applications should not need to be aware of.\n\nI would rather vote for a per-host configuration of the queue lengths.","commit_id":"d4d3d8af683555bcb34df6c87c26c302bbfaa2cc"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d87c039afa34e94f4fbfa98dedcee56415ce4f6f","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"libvirt: add support for virtio-net rx/tx queue sizes"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Add support for configuring the `rx_queue_size` and `tx_queue_size`"},{"line_number":10,"context_line":"options in the virtio-net driver by way of image metadata"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Implements: bp libvirt-virtio-set-queue-sizes"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"7f287b81_70b43af3","line":9,"in_reply_to":"df3967d1_5cc3e957","updated":"2017-09-01 11:13:50.000000000","message":"I was just about to open a new blueprint for supporting this when I found that it already proposed. From my perspective it seems enough to support the queue setting on compute host level.","commit_id":"d4d3d8af683555bcb34df6c87c26c302bbfaa2cc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5d9f189645e959ba6c38f395166c9a9c622b0d79","unresolved":false,"context_lines":[{"line_number":10,"context_line":"options in the virtio-net driver by way of image metadata"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Requires QEMU 2.10.0 or newer, and libvirt 3.7.0 or newer."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Implements: bp libvirt-virtio-set-queue-sizes"},{"line_number":15,"context_line":"Change-Id: Ib6d4a2d6b9072db42d11ecdde0950cf7a7781944"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"5f2577a7_c753a638","line":13,"updated":"2017-09-15 16:47:54.000000000","message":"There should be a DocImpact tag in here so it generates a bug. If it\u0027s image metadata, it should be documented in the image metadata page. If it\u0027s flavor extra specs, it should be documented in this patch since the flavor extra specs docs are now in tree.","commit_id":"345bdf371eeb9720191b4b62114954f1e65a792a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a380ac6b9d529872ff42c8eff921cf4a892058af","unresolved":false,"context_lines":[{"line_number":10,"context_line":"options in the virtio-net driver by way of image metadata"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Requires QEMU 2.10.0 or newer, and libvirt 3.7.0 or newer."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Implements: bp libvirt-virtio-set-queue-sizes"},{"line_number":15,"context_line":"Change-Id: Ib6d4a2d6b9072db42d11ecdde0950cf7a7781944"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"5f2577a7_2a3d8fc3","line":13,"in_reply_to":"5f2577a7_c753a638","updated":"2017-09-15 16:52:06.000000000","message":"image meta keys docs:\n\nhttps://docs.openstack.org/python-glanceclient/pike/cli/property-keys.html\n\nflavor extra specs:\n\nhttps://docs.openstack.org/nova/latest/admin/flavors.html#extra-specs","commit_id":"345bdf371eeb9720191b4b62114954f1e65a792a"}],"nova/conf/libvirt.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3c8fbadd42767df6d524477403eb86d83c60f5a2","unresolved":false,"context_lines":[{"line_number":1095,"context_line":"* None - Remain the default behavior"},{"line_number":1096,"context_line":"* [256, 1026] - Power of 2 value queue size"},{"line_number":1097,"context_line":"               \"\"\"),"},{"line_number":1098,"context_line":"    cfg.IntOpt(\u0027tx_queue_size\u0027,"},{"line_number":1099,"context_line":"               default\u003dNone,"},{"line_number":1100,"context_line":"               min\u003d256,"},{"line_number":1101,"context_line":"               max\u003d1024,"}],"source_content_type":"text/x-python","patch_set":13,"id":"bf659307_42b8f828","line":1098,"updated":"2018-04-10 14:19:46.000000000","message":"As I say later, I think maybe not using an IntOpt for this and instead either defining our own subclass, or just providing a bare option with type\u003d and doing our own validation here would be best. We can confirm that it\u0027s a power of two at config load time which will be much better for the operator than a warning and ignore at runtime.","commit_id":"45994be9669b49280356d0d33b2256e9fb9217f5"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"6394cc4a41e4ce519bdae64d843b86472262711f","unresolved":false,"context_lines":[{"line_number":1095,"context_line":"* None - Remain the default behavior"},{"line_number":1096,"context_line":"* [256, 1026] - Power of 2 value queue size"},{"line_number":1097,"context_line":"               \"\"\"),"},{"line_number":1098,"context_line":"    cfg.IntOpt(\u0027tx_queue_size\u0027,"},{"line_number":1099,"context_line":"               default\u003dNone,"},{"line_number":1100,"context_line":"               min\u003d256,"},{"line_number":1101,"context_line":"               max\u003d1024,"}],"source_content_type":"text/x-python","patch_set":13,"id":"bf659307_b4d80eef","line":1098,"in_reply_to":"bf659307_42b8f828","updated":"2018-04-11 14:50:17.000000000","message":"Yes that makes sense. Actually no need to let operators any values they want. a set of 256, 512, 1024 should be enough, basically they are all going to use 1024 anyway.","commit_id":"45994be9669b49280356d0d33b2256e9fb9217f5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3ff241fd3a1800542d346fc2a972f65999adfda4","unresolved":false,"context_lines":[{"line_number":1134,"context_line":"),"},{"line_number":1135,"context_line":"]"},{"line_number":1136,"context_line":""},{"line_number":1137,"context_line":"QueueSizeType \u003d types.Integer(choices\u003d(256, 512, 1024))"},{"line_number":1138,"context_line":""},{"line_number":1139,"context_line":"libvirt_virtio_queue_sizes \u003d ["},{"line_number":1140,"context_line":"    cfg.Opt(\u0027rx_queue_size\u0027,"}],"source_content_type":"text/x-python","patch_set":14,"id":"bf659307_9744e948","line":1137,"updated":"2018-04-11 16:42:01.000000000","message":"This isn\u0027t actually what I had in mind, but I guess it works okay. Since they have to be powers of two and between those ranges, I guess just enumerating the options is legit, right?\n\nSo, I dunno how other reviewers feel about this, but I think it\u0027s a lot better to have load-time config-based validation of these instead of doing it late at runtime.","commit_id":"0957c45b5407d6a016f0de082de2d772c0582574"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8f0a160a658c02644bc7785dfee5d1d9bb71188e","unresolved":false,"context_lines":[{"line_number":1134,"context_line":"),"},{"line_number":1135,"context_line":"]"},{"line_number":1136,"context_line":""},{"line_number":1137,"context_line":"QueueSizeType \u003d types.Integer(choices\u003d(256, 512, 1024))"},{"line_number":1138,"context_line":""},{"line_number":1139,"context_line":"libvirt_virtio_queue_sizes \u003d ["},{"line_number":1140,"context_line":"    cfg.Opt(\u0027rx_queue_size\u0027,"}],"source_content_type":"text/x-python","patch_set":14,"id":"bf659307_f780c5f9","line":1137,"in_reply_to":"bf659307_9744e948","updated":"2018-04-11 16:48:20.000000000","message":"\u003e So, I dunno how other reviewers feel about this, but I think it\u0027s a lot better to have load-time config-based validation of these instead of doing it late at runtime.\n\nAgree with that yes.\n\nWouldn\u0027t it also be simple to inline a subclass of Integer? Seems we should subclass that instead of what is being done here.","commit_id":"0957c45b5407d6a016f0de082de2d772c0582574"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"59afdc3a4c687f784d7f9f56934864175e8891d4","unresolved":false,"context_lines":[{"line_number":1134,"context_line":"),"},{"line_number":1135,"context_line":"]"},{"line_number":1136,"context_line":""},{"line_number":1137,"context_line":"QueueSizeType \u003d types.Integer(choices\u003d(256, 512, 1024))"},{"line_number":1138,"context_line":""},{"line_number":1139,"context_line":"libvirt_virtio_queue_sizes \u003d ["},{"line_number":1140,"context_line":"    cfg.Opt(\u0027rx_queue_size\u0027,"}],"source_content_type":"text/x-python","patch_set":14,"id":"bf659307_306163aa","line":1137,"in_reply_to":"bf659307_f780c5f9","updated":"2018-04-11 18:22:42.000000000","message":"I\u0027m not sure you get to send choices if you do that. It\u0027s also maybe a tiny bit nicer to do this so you have one definition of it. But, I don\u0027t much care either way, as long as we end up with validation like this.","commit_id":"0957c45b5407d6a016f0de082de2d772c0582574"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a566e21d6f0ee1905a8bdb3a9d796cf8524f61f8","unresolved":false,"context_lines":[{"line_number":1138,"context_line":""},{"line_number":1139,"context_line":"libvirt_virtio_queue_sizes \u003d ["},{"line_number":1140,"context_line":"    cfg.Opt(\u0027rx_queue_size\u0027,"},{"line_number":1141,"context_line":"            default\u003dNone,"},{"line_number":1142,"context_line":"            type\u003dQueueSizeType,"},{"line_number":1143,"context_line":"            help\u003d\"\"\""},{"line_number":1144,"context_line":"Configure virtio rx queue size."}],"source_content_type":"text/x-python","patch_set":14,"id":"bf659307_363f0a26","line":1141,"range":{"start_line":1141,"start_character":0,"end_line":1141,"end_character":25},"updated":"2018-04-12 09:58:30.000000000","message":"nit: You don\u0027t need this.","commit_id":"0957c45b5407d6a016f0de082de2d772c0582574"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a566e21d6f0ee1905a8bdb3a9d796cf8524f61f8","unresolved":false,"context_lines":[{"line_number":1145,"context_line":""},{"line_number":1146,"context_line":"This option is only usable for vhost and vhostuser backend"},{"line_number":1147,"context_line":""},{"line_number":1148,"context_line":"Possible values:"},{"line_number":1149,"context_line":""},{"line_number":1150,"context_line":"* None - Remain the default behavior"},{"line_number":1151,"context_line":"               \"\"\"),"},{"line_number":1152,"context_line":"    cfg.Opt(\u0027tx_queue_size\u0027,"},{"line_number":1153,"context_line":"            default\u003dNone,"}],"source_content_type":"text/x-python","patch_set":14,"id":"bf659307_16f7665b","line":1150,"range":{"start_line":1148,"start_character":0,"end_line":1150,"end_character":36},"updated":"2018-04-12 09:58:30.000000000","message":"This isn\u0027t necessary anymore as it will be automatically generated in 5.2.0 (see oslo.config commit bc9b7f5d). If you want the help text, your choices could/should be formatted like so.\n\n  choices\u003d[(None, \u0027Retain the default behavior\u0027),\n           (256, None), (512, None), (1024, None)],","commit_id":"0957c45b5407d6a016f0de082de2d772c0582574"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a566e21d6f0ee1905a8bdb3a9d796cf8524f61f8","unresolved":false,"context_lines":[{"line_number":1147,"context_line":""},{"line_number":1148,"context_line":"Possible values:"},{"line_number":1149,"context_line":""},{"line_number":1150,"context_line":"* None - Remain the default behavior"},{"line_number":1151,"context_line":"               \"\"\"),"},{"line_number":1152,"context_line":"    cfg.Opt(\u0027tx_queue_size\u0027,"},{"line_number":1153,"context_line":"            default\u003dNone,"}],"source_content_type":"text/x-python","patch_set":14,"id":"bf659307_769b4216","line":1150,"range":{"start_line":1150,"start_character":9,"end_line":1150,"end_character":15},"updated":"2018-04-12 09:58:30.000000000","message":"s/Remain/Retain/","commit_id":"0957c45b5407d6a016f0de082de2d772c0582574"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"aecc5fe1fcdf7909db7bd0d9ea73b83d8a35043b","unresolved":false,"context_lines":[{"line_number":1149,"context_line":"            help\u003d\"\"\""},{"line_number":1150,"context_line":"Configure virtio tx queue size."},{"line_number":1151,"context_line":""},{"line_number":1152,"context_line":"This option is only usable for vhostuser backend"},{"line_number":1153,"context_line":"\"\"\"),"},{"line_number":1154,"context_line":""},{"line_number":1155,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":16,"id":"bf659307_4ea43a4b","line":1152,"updated":"2018-04-13 18:20:08.000000000","message":"The documentation for both the config attributes is a bit sparse.  It doesn\u0027t tell what exactly does it do, nor what are the valid values (the commit message says, but we should add that here), or when (maybe it\u0027s too much to ask) is it useful.\n\nThings you might want to spell out:\n\n(a) The value to be a power of two from [256, 1024] range -- this applies only to specific versions of QEMU, but that check is taken care in libvirt/vif.py.\n\n(b) If you\u0027re using QEMU v2.9, then the `tx_queue_size` only applies for the \u0027vhostuser\u0027 backend.  \n\n(c) This feature applies only for QEMU / KVM hypervisors.\n\nOptionally, add a link / reference to the `{rx,tx}_queue_size` documentation from here: https://libvirt.org/formatdomain.html#elementsDriverBackendOptions","commit_id":"328117870e1f61491b53dbd3e689f3a93a912685"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"0cb8cfa3b1e4af7cad3af55744fff2f369bfbc5c","unresolved":false,"context_lines":[{"line_number":1149,"context_line":"            help\u003d\"\"\""},{"line_number":1150,"context_line":"Configure virtio tx queue size."},{"line_number":1151,"context_line":""},{"line_number":1152,"context_line":"This option is only usable for vhostuser backend"},{"line_number":1153,"context_line":"\"\"\"),"},{"line_number":1154,"context_line":""},{"line_number":1155,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":16,"id":"9f6a8fd7_cfaa5818","line":1152,"in_reply_to":"bf659307_4ea43a4b","updated":"2018-04-16 09:36:42.000000000","message":"OK I added couple of notes.","commit_id":"328117870e1f61491b53dbd3e689f3a93a912685"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"34e77a9d491dc21d1a094f1144312d0cbac49ffe","unresolved":false,"context_lines":[{"line_number":1134,"context_line":"),"},{"line_number":1135,"context_line":"]"},{"line_number":1136,"context_line":""},{"line_number":1137,"context_line":"QueueSizeType \u003d types.Integer(choices\u003d(256, 512, 1024))"},{"line_number":1138,"context_line":""},{"line_number":1139,"context_line":"libvirt_virtio_queue_sizes \u003d ["},{"line_number":1140,"context_line":"    cfg.Opt(\u0027rx_queue_size\u0027,"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f6a8fd7_06acad8a","line":1137,"range":{"start_line":1137,"start_character":16,"end_line":1137,"end_character":55},"updated":"2018-04-16 19:52:30.000000000","message":"nit: it might be nice to link to the libvirt docs for this\nhttps://libvirt.org/formatdomain.html#elementsDriverBackendOptions\nwhich state that a power of two between 256 and 1024 is required by qemu.","commit_id":"bbba28d7e5c6ad72abfc6e7b52f52014d1b45c9c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"34e77a9d491dc21d1a094f1144312d0cbac49ffe","unresolved":false,"context_lines":[{"line_number":1142,"context_line":"            help\u003d\"\"\""},{"line_number":1143,"context_line":"Configure virtio rx queue size."},{"line_number":1144,"context_line":""},{"line_number":1145,"context_line":"This option is only usable for vhost and vhostuser backend. Available"},{"line_number":1146,"context_line":"only with QEMU/KVM. Requires libvirt v2.3 QEMU v2.7."},{"line_number":1147,"context_line":"\"\"\"),"},{"line_number":1148,"context_line":"    cfg.Opt(\u0027tx_queue_size\u0027,"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f6a8fd7_a1884b1f","line":1145,"range":{"start_line":1145,"start_character":31,"end_line":1145,"end_character":50},"updated":"2018-04-16 19:52:30.000000000","message":"nit: is that the actully reqiurement or is the requirement that you use virtio","commit_id":"bbba28d7e5c6ad72abfc6e7b52f52014d1b45c9c"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"16b25c78218945db2dacebc91e8602ed7905db2a","unresolved":false,"context_lines":[{"line_number":1142,"context_line":"            help\u003d\"\"\""},{"line_number":1143,"context_line":"Configure virtio rx queue size."},{"line_number":1144,"context_line":""},{"line_number":1145,"context_line":"This option is only usable for vhost and vhostuser backend. Available"},{"line_number":1146,"context_line":"only with QEMU/KVM. Requires libvirt v2.3 QEMU v2.7."},{"line_number":1147,"context_line":"\"\"\"),"},{"line_number":1148,"context_line":"    cfg.Opt(\u0027tx_queue_size\u0027,"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f6a8fd7_dfbbcae2","line":1145,"range":{"start_line":1145,"start_character":31,"end_line":1145,"end_character":50},"in_reply_to":"9f6a8fd7_a1884b1f","updated":"2018-04-17 08:27:34.000000000","message":"virtio with vhost or vhost-user backend are the requirements. That because you can also use virtio in an other userspace mode (qemu).","commit_id":"bbba28d7e5c6ad72abfc6e7b52f52014d1b45c9c"}],"nova/objects/image_meta.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"463b03d00310318fb71c989268a0063037c3de4c","unresolved":false,"context_lines":[{"line_number":341,"context_line":"        \u0027hw_vif_model\u0027: fields.VIFModelField(),"},{"line_number":342,"context_line":""},{"line_number":343,"context_line":"        # size of virtio-net rx/tx queues eg 256, 1024"},{"line_number":344,"context_line":"        \u0027hw_virtio_net_rx_queue_size\u0027: fields.IntegerField(),"},{"line_number":345,"context_line":"        \u0027hw_virtio_net_tx_queue_size\u0027: fields.IntegerField(),"},{"line_number":346,"context_line":""},{"line_number":347,"context_line":"        # \"xen\" vs \"hvm\""},{"line_number":348,"context_line":"        \u0027hw_vm_mode\u0027: fields.VMModeField(),"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_e9e04b99","line":345,"range":{"start_line":344,"start_character":8,"end_line":345,"end_character":61},"updated":"2017-07-23 06:02:50.000000000","message":"according to http://lists.nongnu.org/archive/html/qemu-devel/2016-08/msg02029.html it looks like we would need to clamp these values between 64 and 1024 or throw an excpetion.","commit_id":"01a15b24c9261520f623f7235a68495dde3ea0e0"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"113a49d4adf69953f8b360ba870fa5652dd58ce5","unresolved":false,"context_lines":[{"line_number":341,"context_line":"        \u0027hw_vif_model\u0027: fields.VIFModelField(),"},{"line_number":342,"context_line":""},{"line_number":343,"context_line":"        # size of virtio-net rx/tx queues eg 256, 1024"},{"line_number":344,"context_line":"        \u0027hw_virtio_net_rx_queue_size\u0027: fields.IntegerField(),"},{"line_number":345,"context_line":"        \u0027hw_virtio_net_tx_queue_size\u0027: fields.IntegerField(),"},{"line_number":346,"context_line":""},{"line_number":347,"context_line":"        # \"xen\" vs \"hvm\""},{"line_number":348,"context_line":"        \u0027hw_vm_mode\u0027: fields.VMModeField(),"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_867273e2","line":345,"range":{"start_line":344,"start_character":8,"end_line":345,"end_character":61},"in_reply_to":"ff346bd7_44f043ca","updated":"2017-07-26 00:30:29.000000000","message":"It was merged a little while back, and is expected to be in the 2.10.0 release.  The libvirt support is similarly \"forward-looking\".","commit_id":"01a15b24c9261520f623f7235a68495dde3ea0e0"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"34348f422349d00be9fb126a9254a4cc8ea7ddd2","unresolved":false,"context_lines":[{"line_number":341,"context_line":"        \u0027hw_vif_model\u0027: fields.VIFModelField(),"},{"line_number":342,"context_line":""},{"line_number":343,"context_line":"        # size of virtio-net rx/tx queues eg 256, 1024"},{"line_number":344,"context_line":"        \u0027hw_virtio_net_rx_queue_size\u0027: fields.IntegerField(),"},{"line_number":345,"context_line":"        \u0027hw_virtio_net_tx_queue_size\u0027: fields.IntegerField(),"},{"line_number":346,"context_line":""},{"line_number":347,"context_line":"        # \"xen\" vs \"hvm\""},{"line_number":348,"context_line":"        \u0027hw_vm_mode\u0027: fields.VMModeField(),"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_44f043ca","line":345,"range":{"start_line":344,"start_character":8,"end_line":345,"end_character":61},"in_reply_to":"ff346bd7_e9e04b99","updated":"2017-07-24 14:14:31.000000000","message":"actully looking at http://patchwork.ozlabs.org/patch/771133/\ni think qemu only supports setting this on RX only and requires that patch series to enable tx.","commit_id":"01a15b24c9261520f623f7235a68495dde3ea0e0"}],"nova/tests/unit/virt/libvirt/test_config.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"463b03d00310318fb71c989268a0063037c3de4c","unresolved":false,"context_lines":[{"line_number":1578,"context_line":"        obj.target_dev \u003d \"vnet0\""},{"line_number":1579,"context_line":"        obj.driver_name \u003d \"vhost\""},{"line_number":1580,"context_line":"        obj.vhost_queues \u003d 4"},{"line_number":1581,"context_line":"        obj.vhost_rx_queue_size \u003d 1024"},{"line_number":1582,"context_line":"        obj.vhost_tx_queue_size \u003d 1024"},{"line_number":1583,"context_line":""},{"line_number":1584,"context_line":"        xml \u003d obj.to_xml()"},{"line_number":1585,"context_line":"        self.assertXmlEqual(xml, \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_69ee9b89","line":1582,"range":{"start_line":1581,"start_character":8,"end_line":1582,"end_character":38},"updated":"2017-07-23 06:02:50.000000000","message":"this is the upperbound. it would be good to test that value above this are not supported and simalary below 64.","commit_id":"01a15b24c9261520f623f7235a68495dde3ea0e0"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"e48917ac4c190f7b0548f7a50a8e8b6916fa69ba","unresolved":false,"context_lines":[{"line_number":1578,"context_line":"        obj.target_dev \u003d \"vnet0\""},{"line_number":1579,"context_line":"        obj.driver_name \u003d \"vhost\""},{"line_number":1580,"context_line":"        obj.vhost_queues \u003d 4"},{"line_number":1581,"context_line":"        obj.vhost_rx_queue_size \u003d 1024"},{"line_number":1582,"context_line":"        obj.vhost_tx_queue_size \u003d 1024"},{"line_number":1583,"context_line":""},{"line_number":1584,"context_line":"        xml \u003d obj.to_xml()"},{"line_number":1585,"context_line":"        self.assertXmlEqual(xml, \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_738f2f75","line":1582,"range":{"start_line":1581,"start_character":8,"end_line":1582,"end_character":38},"in_reply_to":"ff346bd7_69ee9b89","updated":"2017-07-26 22:22:43.000000000","message":"Done","commit_id":"01a15b24c9261520f623f7235a68495dde3ea0e0"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"34e77a9d491dc21d1a094f1144312d0cbac49ffe","unresolved":false,"context_lines":[{"line_number":1945,"context_line":"        obj.vhost_tx_queue_size \u003d 1024"},{"line_number":1946,"context_line":"        obj.model \u003d \"virtio\""},{"line_number":1947,"context_line":"        xml \u003d obj.to_xml()"},{"line_number":1948,"context_line":"        self.assertXmlEqual(xml, \"\"\""},{"line_number":1949,"context_line":"            \u003cinterface type\u003d\"vhostuser\"\u003e"},{"line_number":1950,"context_line":"              \u003cmac address\u003d\"DE:AD:BE:EF:CA:FE\"/\u003e"},{"line_number":1951,"context_line":"              \u003cmodel type\u003d\"virtio\"/\u003e"},{"line_number":1952,"context_line":"              \u003cdriver name\u003d\"vhostuser\" rx_queue_size\u003d\"512\""},{"line_number":1953,"context_line":"                      tx_queue_size\u003d\"1024\"/\u003e"},{"line_number":1954,"context_line":"              \u003csource type\u003d\"unix\" mode\u003d\"server\" path\u003d\"/vhost-user/test.sock\"/\u003e"},{"line_number":1955,"context_line":"            \u003c/interface\u003e\"\"\")"},{"line_number":1956,"context_line":""},{"line_number":1957,"context_line":"        # parse the xml from the first object into a new object and make sure"},{"line_number":1958,"context_line":"        # they are the same"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f6a8fd7_21fafb41","line":1955,"range":{"start_line":1948,"start_character":6,"end_line":1955,"end_character":28},"updated":"2018-04-16 19:52:30.000000000","message":"vhostuser interface usually look like this \n\n \u003cinterface type\u003d\u0027vhostuser\u0027\u003e\n      \u003cmac address\u003d\u0027fa:16:3e:d0:da:bc\u0027/\u003e\n      \u003csource type\u003d\u0027unix\u0027 path\u003d\u0027/var/run/openvswitch/vhud9c87008-d8\u0027 mode\u003d\u0027server\u0027/\u003e\n      \u003cmodel type\u003d\u0027virtio\u0027/\u003e\n      \u003caddress type\u003d\u0027pci\u0027 domain\u003d\u00270x0000\u0027 bus\u003d\u00270x00\u0027 slot\u003d\u00270x03\u0027 function\u003d\u00270x0\u0027/\u003e\n    \u003c/interface\u003e\n\nso im not sure the driver element is correct\n\nlooking at \nhttps://libvirt.org/formatdomain.html#elementsDriverBackendOptions\n\nsince the name filed is optional and since its always ignored for vhost user interfaes.\n\n\"For interfaces of type\u003d\u0027vhostuser\u0027, the name attribute is ignored. The backend driver used is always vhost-user. \"\n\nperhaps we should not generate it?\n\nif we do keep it we should proably change it form\n\n\u003cdriver name\u003d\"vhostuser\" rx_queue_size\u003d\"512\"\n                      tx_queue_size\u003d\"1024\"/\u003e\n\nto \n\n\u003cdriver name\u003d\"vhost-user\" rx_queue_size\u003d\"512\"\n                      tx_queue_size\u003d\"1024\"/\u003e\n\nto match the documentation\n\nthat said as written you always set the driver to vhost in \n\nhttps://review.openstack.org/#/c/484997/17/nova/virt/libvirt/vif.py line 171\n\nthis test is currently only passing because you have not mocked out self._get_virtio_queue_sizes(host) but have set obj.vhost_rx_queue_size \u003d 512.\n\nin a real code path the xml fragment above will never get created.","commit_id":"bbba28d7e5c6ad72abfc6e7b52f52014d1b45c9c"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"16b25c78218945db2dacebc91e8602ed7905db2a","unresolved":false,"context_lines":[{"line_number":1945,"context_line":"        obj.vhost_tx_queue_size \u003d 1024"},{"line_number":1946,"context_line":"        obj.model \u003d \"virtio\""},{"line_number":1947,"context_line":"        xml \u003d obj.to_xml()"},{"line_number":1948,"context_line":"        self.assertXmlEqual(xml, \"\"\""},{"line_number":1949,"context_line":"            \u003cinterface type\u003d\"vhostuser\"\u003e"},{"line_number":1950,"context_line":"              \u003cmac address\u003d\"DE:AD:BE:EF:CA:FE\"/\u003e"},{"line_number":1951,"context_line":"              \u003cmodel type\u003d\"virtio\"/\u003e"},{"line_number":1952,"context_line":"              \u003cdriver name\u003d\"vhostuser\" rx_queue_size\u003d\"512\""},{"line_number":1953,"context_line":"                      tx_queue_size\u003d\"1024\"/\u003e"},{"line_number":1954,"context_line":"              \u003csource type\u003d\"unix\" mode\u003d\"server\" path\u003d\"/vhost-user/test.sock\"/\u003e"},{"line_number":1955,"context_line":"            \u003c/interface\u003e\"\"\")"},{"line_number":1956,"context_line":""},{"line_number":1957,"context_line":"        # parse the xml from the first object into a new object and make sure"},{"line_number":1958,"context_line":"        # they are the same"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f6a8fd7_7f52be69","line":1955,"range":{"start_line":1948,"start_character":6,"end_line":1955,"end_character":28},"in_reply_to":"9f6a8fd7_21fafb41","updated":"2018-04-17 08:27:34.000000000","message":"For you first point you are right it\u0027s vhost-user and not vhostuser. Thanks for pointing me that.\n\nNot sure I understand the second point; This is \u0027test_config\u0027 the aim is to test whether when we set attributes to the conf object the XML is well generated and inverse. We are not testing any \"orchestration\". \n\nAbout the missing PCI address element you are pointing in your example. The point is that Nova never generates it, it\u0027s libvirt which takes care of doing that.  \n\nMakes sense?","commit_id":"bbba28d7e5c6ad72abfc6e7b52f52014d1b45c9c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"cdc5bd89691cb8eb9220fe8c2903f672540ceb16","unresolved":false,"context_lines":[{"line_number":1936,"context_line":"    def test_config_vhostuser_queue_size(self):"},{"line_number":1937,"context_line":"        obj \u003d config.LibvirtConfigGuestInterface()"},{"line_number":1938,"context_line":"        obj.net_type \u003d \"vhostuser\""},{"line_number":1939,"context_line":"        obj.driver_name \u003d \"vhostuser\""},{"line_number":1940,"context_line":"        obj.vhostuser_type \u003d \"unix\""},{"line_number":1941,"context_line":"        obj.vhostuser_mode \u003d \"server\""},{"line_number":1942,"context_line":"        obj.mac_addr \u003d \"DE:AD:BE:EF:CA:FE\""}],"source_content_type":"text/x-python","patch_set":18,"id":"9f6a8fd7_759f3f50","line":1939,"range":{"start_line":1939,"start_character":8,"end_line":1939,"end_character":37},"updated":"2018-04-17 09:00:18.000000000","message":"nit: vhost-user","commit_id":"6cd15a202b4be7ee4975a51722fb4f44430f986e"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"a4940ae86bb3d0aac171cbbc97706f616ae66e9d","unresolved":false,"context_lines":[{"line_number":1936,"context_line":"    def test_config_vhostuser_queue_size(self):"},{"line_number":1937,"context_line":"        obj \u003d config.LibvirtConfigGuestInterface()"},{"line_number":1938,"context_line":"        obj.net_type \u003d \"vhostuser\""},{"line_number":1939,"context_line":"        obj.driver_name \u003d \"vhostuser\""},{"line_number":1940,"context_line":"        obj.vhostuser_type \u003d \"unix\""},{"line_number":1941,"context_line":"        obj.vhostuser_mode \u003d \"server\""},{"line_number":1942,"context_line":"        obj.mac_addr \u003d \"DE:AD:BE:EF:CA:FE\""}],"source_content_type":"text/x-python","patch_set":18,"id":"9f6a8fd7_86cc2b8a","line":1939,"range":{"start_line":1939,"start_character":8,"end_line":1939,"end_character":37},"in_reply_to":"9f6a8fd7_759f3f50","updated":"2018-04-17 10:40:21.000000000","message":"It\u0027s good point I prefered to fix that. Thanks.","commit_id":"6cd15a202b4be7ee4975a51722fb4f44430f986e"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"cdc5bd89691cb8eb9220fe8c2903f672540ceb16","unresolved":false,"context_lines":[{"line_number":1949,"context_line":"            \u003cinterface type\u003d\"vhostuser\"\u003e"},{"line_number":1950,"context_line":"              \u003cmac address\u003d\"DE:AD:BE:EF:CA:FE\"/\u003e"},{"line_number":1951,"context_line":"              \u003cmodel type\u003d\"virtio\"/\u003e"},{"line_number":1952,"context_line":"              \u003cdriver name\u003d\"vhostuser\" rx_queue_size\u003d\"512\""},{"line_number":1953,"context_line":"                      tx_queue_size\u003d\"1024\"/\u003e"},{"line_number":1954,"context_line":"              \u003csource type\u003d\"unix\" mode\u003d\"server\" path\u003d\"/vhost-user/test.sock\"/\u003e"},{"line_number":1955,"context_line":"            \u003c/interface\u003e\"\"\")"}],"source_content_type":"text/x-python","patch_set":18,"id":"9f6a8fd7_35a9c77a","line":1952,"range":{"start_line":1952,"start_character":28,"end_line":1952,"end_character":37},"updated":"2018-04-17 09:00:18.000000000","message":"nit: vhost-user","commit_id":"6cd15a202b4be7ee4975a51722fb4f44430f986e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3b938724ae9baad78259ba7df5828f5ab7891621","unresolved":false,"context_lines":[{"line_number":1964,"context_line":"        obj.net_type \u003d \"vhostuser\""},{"line_number":1965,"context_line":"        obj.vhostuser_type \u003d \"unix\""},{"line_number":1966,"context_line":"        obj.vhostuser_mode \u003d \"server\""},{"line_number":1967,"context_line":"        obj.mac_addr \u003d \"DE:AD:BE:EF:CA:FE\""},{"line_number":1968,"context_line":"        obj.vhostuser_path \u003d \"/vhost-user/test.sock\""},{"line_number":1969,"context_line":"        obj.vhost_rx_queue_size \u003d 512"},{"line_number":1970,"context_line":"        obj.vhost_tx_queue_size \u003d 1024"}],"source_content_type":"text/x-python","patch_set":25,"id":"9f6a8fd7_0f71870c","line":1967,"range":{"start_line":1967,"start_character":23,"end_line":1967,"end_character":42},"updated":"2018-04-23 15:28:28.000000000","message":"they always have the best menu.","commit_id":"a1eca36557d773123d1096edcbfaa55dba63de15"}],"nova/tests/unit/virt/libvirt/test_designer.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e74e0f15b2174b424d52ff24c4a58530fcacde6e","unresolved":false,"context_lines":[{"line_number":40,"context_line":"        conf \u003d config.LibvirtConfigGuestInterface()"},{"line_number":41,"context_line":"        designer.set_vif_guest_frontend_config(conf, \u0027fake-mac\u0027,"},{"line_number":42,"context_line":"                                               \u0027fake-model\u0027, \u0027fake-driver\u0027,"},{"line_number":43,"context_line":"                                               \u0027fake-queues\u0027, None)"},{"line_number":44,"context_line":"        self.assertEqual(\u0027fake-mac\u0027, conf.mac_addr)"},{"line_number":45,"context_line":"        self.assertEqual(\u0027fake-model\u0027, conf.model)"},{"line_number":46,"context_line":"        self.assertEqual(\u0027fake-driver\u0027, conf.driver_name)"}],"source_content_type":"text/x-python","patch_set":16,"id":"bf659307_a2240d0e","line":43,"updated":"2018-04-13 15:29:38.000000000","message":"Don\u0027t we want a test for this that does provide them?","commit_id":"328117870e1f61491b53dbd3e689f3a93a912685"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"0cb8cfa3b1e4af7cad3af55744fff2f369bfbc5c","unresolved":false,"context_lines":[{"line_number":40,"context_line":"        conf \u003d config.LibvirtConfigGuestInterface()"},{"line_number":41,"context_line":"        designer.set_vif_guest_frontend_config(conf, \u0027fake-mac\u0027,"},{"line_number":42,"context_line":"                                               \u0027fake-model\u0027, \u0027fake-driver\u0027,"},{"line_number":43,"context_line":"                                               \u0027fake-queues\u0027, None)"},{"line_number":44,"context_line":"        self.assertEqual(\u0027fake-mac\u0027, conf.mac_addr)"},{"line_number":45,"context_line":"        self.assertEqual(\u0027fake-model\u0027, conf.model)"},{"line_number":46,"context_line":"        self.assertEqual(\u0027fake-driver\u0027, conf.driver_name)"}],"source_content_type":"text/x-python","patch_set":16,"id":"9f6a8fd7_4f988869","line":43,"in_reply_to":"bf659307_a2240d0e","updated":"2018-04-16 09:36:42.000000000","message":"Done","commit_id":"328117870e1f61491b53dbd3e689f3a93a912685"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e74e0f15b2174b424d52ff24c4a58530fcacde6e","unresolved":false,"context_lines":[{"line_number":175,"context_line":"    def test_set_vif_host_backend_vhostuser_config(self):"},{"line_number":176,"context_line":"        conf \u003d config.LibvirtConfigGuestInterface()"},{"line_number":177,"context_line":"        designer.set_vif_host_backend_vhostuser_config(conf, \u0027fake-mode\u0027,"},{"line_number":178,"context_line":"                                                       \u0027fake-path\u0027, None, None)"},{"line_number":179,"context_line":"        self.assertEqual(\u0027vhostuser\u0027, conf.net_type)"},{"line_number":180,"context_line":"        self.assertEqual(\u0027unix\u0027, conf.vhostuser_type)"},{"line_number":181,"context_line":"        self.assertEqual(\u0027fake-mode\u0027, conf.vhostuser_mode)"}],"source_content_type":"text/x-python","patch_set":16,"id":"bf659307_227ebde3","line":178,"updated":"2018-04-13 15:29:38.000000000","message":"Same","commit_id":"328117870e1f61491b53dbd3e689f3a93a912685"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"0cb8cfa3b1e4af7cad3af55744fff2f369bfbc5c","unresolved":false,"context_lines":[{"line_number":175,"context_line":"    def test_set_vif_host_backend_vhostuser_config(self):"},{"line_number":176,"context_line":"        conf \u003d config.LibvirtConfigGuestInterface()"},{"line_number":177,"context_line":"        designer.set_vif_host_backend_vhostuser_config(conf, \u0027fake-mode\u0027,"},{"line_number":178,"context_line":"                                                       \u0027fake-path\u0027, None, None)"},{"line_number":179,"context_line":"        self.assertEqual(\u0027vhostuser\u0027, conf.net_type)"},{"line_number":180,"context_line":"        self.assertEqual(\u0027unix\u0027, conf.vhostuser_type)"},{"line_number":181,"context_line":"        self.assertEqual(\u0027fake-mode\u0027, conf.vhostuser_mode)"}],"source_content_type":"text/x-python","patch_set":16,"id":"9f6a8fd7_6f9d4c58","line":178,"in_reply_to":"bf659307_227ebde3","updated":"2018-04-16 09:36:42.000000000","message":"Done","commit_id":"328117870e1f61491b53dbd3e689f3a93a912685"}],"nova/tests/unit/virt/libvirt/test_vif.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a566e21d6f0ee1905a8bdb3a9d796cf8524f61f8","unresolved":false,"context_lines":[{"line_number":714,"context_line":"        self.assertEqual(1024, conf.vhost_tx_queue_size)"},{"line_number":715,"context_line":""},{"line_number":716,"context_line":"    @mock.patch.object(host.Host, \"has_min_version\", return_value\u003dFalse)"},{"line_number":717,"context_line":"    def test_virtio_vhostuser_osvif_queue_sizes_nover(self, has_min_version):"},{"line_number":718,"context_line":"        hostimpl, v, conf \u003d self._test_virtio_config_queue_sizes()"},{"line_number":719,"context_line":"        v._set_config_VIFVHostUser(self.instance, self.os_vif_vhostuser,"},{"line_number":720,"context_line":"                                   conf, hostimpl)"}],"source_content_type":"text/x-python","patch_set":14,"id":"bf659307_966e16fe","line":717,"range":{"start_line":717,"start_character":48,"end_line":717,"end_character":53},"updated":"2018-04-12 09:58:30.000000000","message":"nover?","commit_id":"0957c45b5407d6a016f0de082de2d772c0582574"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"9f5feac57314abdc3d565ca6d327c5930efc1bb6","unresolved":false,"context_lines":[{"line_number":714,"context_line":"        self.assertEqual(1024, conf.vhost_tx_queue_size)"},{"line_number":715,"context_line":""},{"line_number":716,"context_line":"    @mock.patch.object(host.Host, \"has_min_version\", return_value\u003dFalse)"},{"line_number":717,"context_line":"    def test_virtio_vhostuser_osvif_queue_sizes_nover(self, has_min_version):"},{"line_number":718,"context_line":"        hostimpl, v, conf \u003d self._test_virtio_config_queue_sizes()"},{"line_number":719,"context_line":"        v._set_config_VIFVHostUser(self.instance, self.os_vif_vhostuser,"},{"line_number":720,"context_line":"                                   conf, hostimpl)"}],"source_content_type":"text/x-python","patch_set":14,"id":"bf659307_19fc79b3","line":717,"range":{"start_line":717,"start_character":48,"end_line":717,"end_character":53},"in_reply_to":"bf659307_966e16fe","updated":"2018-04-12 13:06:48.000000000","message":"\"no version\" :) .. I updated the name.","commit_id":"0957c45b5407d6a016f0de082de2d772c0582574"}],"nova/virt/libvirt/config.py":[{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"9bcd28df4fe116ab5a94324c13fb5db7b180b9df","unresolved":false,"context_lines":[{"line_number":1259,"context_line":"        self.vhostuser_type \u003d None"},{"line_number":1260,"context_line":"        self.vhost_queues \u003d None"},{"line_number":1261,"context_line":"        self.vhost_rx_queue_size \u003d None"},{"line_number":1262,"context_line":"        self.vhost_tx_queue_size \u003d None"},{"line_number":1263,"context_line":"        self.vif_inbound_peak \u003d None"},{"line_number":1264,"context_line":"        self.vif_inbound_burst \u003d None"},{"line_number":1265,"context_line":"        self.vif_inbound_average \u003d None"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_b10dfe38","line":1262,"updated":"2017-07-24 07:47:08.000000000","message":"The rx/tx_queue_size are options related to the NIC, in that case virtio, they can use a backend in user-space (qemu) or kernel space (vhost). I don\u0027t think you need to prefix them by \u0027vhost\u0027.","commit_id":"01a15b24c9261520f623f7235a68495dde3ea0e0"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"9bcd28df4fe116ab5a94324c13fb5db7b180b9df","unresolved":false,"context_lines":[{"line_number":1283,"context_line":"            drv_elem \u003d etree.Element(\"driver\", name\u003dself.driver_name)"},{"line_number":1284,"context_line":"            if self.vhost_queues is not None:"},{"line_number":1285,"context_line":"                drv_elem.set(\u0027queues\u0027, str(self.vhost_queues))"},{"line_number":1286,"context_line":"            if self.vhost_rx_queue_size is not None:"},{"line_number":1287,"context_line":"                drv_elem.set(\u0027rx_queue_size\u0027, str(self.vhost_rx_queue_size))"},{"line_number":1288,"context_line":"            if self.vhost_tx_queue_size is not None:"},{"line_number":1289,"context_line":"                drv_elem.set(\u0027tx_queue_size\u0027, str(self.vhost_tx_queue_size))"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_114dea71","line":1286,"updated":"2017-07-24 07:47:08.000000000","message":"No need to put them under the driver_name condition (which should be renamed as backend_name).\n\nBasically if no backend is specified libvirt is going to choose vhost by default or fallback to qemu if the kernel module is not present. As indicated before the options are not related to the \"backend\" but to the NIC driver.","commit_id":"01a15b24c9261520f623f7235a68495dde3ea0e0"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"e48917ac4c190f7b0548f7a50a8e8b6916fa69ba","unresolved":false,"context_lines":[{"line_number":1283,"context_line":"            drv_elem \u003d etree.Element(\"driver\", name\u003dself.driver_name)"},{"line_number":1284,"context_line":"            if self.vhost_queues is not None:"},{"line_number":1285,"context_line":"                drv_elem.set(\u0027queues\u0027, str(self.vhost_queues))"},{"line_number":1286,"context_line":"            if self.vhost_rx_queue_size is not None:"},{"line_number":1287,"context_line":"                drv_elem.set(\u0027rx_queue_size\u0027, str(self.vhost_rx_queue_size))"},{"line_number":1288,"context_line":"            if self.vhost_tx_queue_size is not None:"},{"line_number":1289,"context_line":"                drv_elem.set(\u0027tx_queue_size\u0027, str(self.vhost_tx_queue_size))"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_139c630d","line":1286,"in_reply_to":"ff346bd7_114dea71","updated":"2017-07-26 22:22:43.000000000","message":"But this is configuring the guest front-end...\n\nI\u0027ve dodged this by forcing a driver name when these are set, which is the same trick that  \"vhost_queues\" uses.  I could refactor it to remove the dependency, but it might get kind of ugly.","commit_id":"01a15b24c9261520f623f7235a68495dde3ea0e0"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"6d9eee0a3c042a14deb177f732a266f240788a20","unresolved":false,"context_lines":[{"line_number":1283,"context_line":"            drv_elem \u003d etree.Element(\"driver\", name\u003dself.driver_name)"},{"line_number":1284,"context_line":"            if self.vhost_queues is not None:"},{"line_number":1285,"context_line":"                drv_elem.set(\u0027queues\u0027, str(self.vhost_queues))"},{"line_number":1286,"context_line":"            if self.vhost_rx_queue_size is not None:"},{"line_number":1287,"context_line":"                drv_elem.set(\u0027rx_queue_size\u0027, str(self.vhost_rx_queue_size))"},{"line_number":1288,"context_line":"            if self.vhost_tx_queue_size is not None:"},{"line_number":1289,"context_line":"                drv_elem.set(\u0027tx_queue_size\u0027, str(self.vhost_tx_queue_size))"}],"source_content_type":"text/x-python","patch_set":2,"id":"df3967d1_7c534de3","line":1286,"in_reply_to":"ff346bd7_139c630d","updated":"2017-08-04 12:05:38.000000000","message":"You are not configure the guest front-end at all. We are talking about a NIC. That virtual NIC is on the host and plugged to the guest. That virtual NIC can have a backend in user-space (qemu) or kernel space (vhost).\n\nThe rx/tx_queue_size can be configured on both qemu/vhost but only model virtio is supporting that option (currently)","commit_id":"01a15b24c9261520f623f7235a68495dde3ea0e0"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"3ac73486941cf28f2aa457dd5fb35c6bb5102fc0","unresolved":false,"context_lines":[{"line_number":1283,"context_line":"            drv_elem \u003d etree.Element(\"driver\", name\u003dself.driver_name)"},{"line_number":1284,"context_line":"            if self.vhost_queues is not None:"},{"line_number":1285,"context_line":"                drv_elem.set(\u0027queues\u0027, str(self.vhost_queues))"},{"line_number":1286,"context_line":"            if self.vhost_rx_queue_size is not None:"},{"line_number":1287,"context_line":"                drv_elem.set(\u0027rx_queue_size\u0027, str(self.vhost_rx_queue_size))"},{"line_number":1288,"context_line":"            if self.vhost_tx_queue_size is not None:"},{"line_number":1289,"context_line":"                drv_elem.set(\u0027tx_queue_size\u0027, str(self.vhost_tx_queue_size))"}],"source_content_type":"text/x-python","patch_set":5,"id":"df3967d1_5c988973","line":1286,"updated":"2017-08-04 12:22:24.000000000","message":"As indicated before please remove the prefix \u0027vhost\u0027 tunning queue size is available for both backends and so this should no be under driver_name which represent the backend used. This should be under \u0027model\u0027 to ensure that virtio is used (since it\u0027s the only model which handle it currently).","commit_id":"d4d3d8af683555bcb34df6c87c26c302bbfaa2cc"}],"nova/virt/libvirt/designer.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3ff241fd3a1800542d346fc2a972f65999adfda4","unresolved":false,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"def set_vif_guest_frontend_config(conf, mac, model, driver, queues\u003dNone,"},{"line_number":29,"context_line":"                                  rx_queue_size\u003dNone):"},{"line_number":30,"context_line":"    \"\"\"Populate a LibvirtConfigGuestInterface instance"},{"line_number":31,"context_line":"    with guest frontend details."},{"line_number":32,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":14,"id":"bf659307_37615daa","line":29,"range":{"start_line":29,"start_character":34,"end_line":29,"end_character":52},"updated":"2018-04-11 16:42:01.000000000","message":"Like below, this also is always passed, as far as I can tell and not by kwarg. I\u0027m guessing maybe queues is the same?\n\nIMHO, just make it a posarg here to avoid confusion about how/if/when it\u0027s called with the queue_size.","commit_id":"0957c45b5407d6a016f0de082de2d772c0582574"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3ff241fd3a1800542d346fc2a972f65999adfda4","unresolved":false,"context_lines":[{"line_number":151,"context_line":"    conf.model \u003d \"virtio\""},{"line_number":152,"context_line":""},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"def set_vif_host_backend_vhostuser_config(conf, mode, path, rx_queue_size\u003dNone,"},{"line_number":155,"context_line":"                                          tx_queue_size\u003dNone):"},{"line_number":156,"context_line":"    \"\"\"Populate a LibvirtConfigGuestInterface instance"},{"line_number":157,"context_line":"    with host backend details for vhostuser socket."},{"line_number":158,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":14,"id":"bf659307_f73d258e","line":155,"range":{"start_line":154,"start_character":60,"end_line":155,"end_character":60},"updated":"2018-04-11 16:42:01.000000000","message":"It looks to me like these are always passed, so they should not be kwargs, IMHO. Just note in the docstring that they can be None if unset.","commit_id":"0957c45b5407d6a016f0de082de2d772c0582574"},{"author":{"_account_id":23811,"name":"Oliver Walsh","email":"owalsh@redhat.com","username":"owalsh"},"change_message_id":"903150e41474487d9a74a185466dffb557e1a2e4","unresolved":false,"context_lines":[{"line_number":166,"context_line":"    conf.vhostuser_path \u003d path"},{"line_number":167,"context_line":"    if rx_queue_size:"},{"line_number":168,"context_line":"        conf.vhost_rx_queue_size \u003d rx_queue_size"},{"line_number":169,"context_line":"    if rx_queue_size:"},{"line_number":170,"context_line":"        conf.vhost_tx_queue_size \u003d tx_queue_size"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":""}],"source_content_type":"text/x-python","patch_set":25,"id":"9f6a8fd7_0eaec4e6","line":169,"range":{"start_line":169,"start_character":7,"end_line":169,"end_character":20},"updated":"2018-05-01 20:02:10.000000000","message":"should be tx_queue_size - https://review.openstack.org/565573","commit_id":"a1eca36557d773123d1096edcbfaa55dba63de15"}],"nova/virt/libvirt/vif.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"463b03d00310318fb71c989268a0063037c3de4c","unresolved":false,"context_lines":[{"line_number":44,"context_line":"CONF \u003d nova.conf.CONF"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"# virtio-net.tx_queue_size support"},{"line_number":47,"context_line":"MIN_LIBVIRT_TX_QUEUE_SIZE \u003d (3, 6, 0)"},{"line_number":48,"context_line":"# virtio-net.rx_queue_size support"},{"line_number":49,"context_line":"MIN_LIBVIRT_RX_QUEUE_SIZE \u003d (2, 3, 0)"},{"line_number":50,"context_line":"# vhostuser queues support"},{"line_number":51,"context_line":"MIN_LIBVIRT_VHOSTUSER_MQ \u003d (1, 2, 17)"},{"line_number":52,"context_line":"#  vlan tag for macvtap passthrough mode on SRIOV VFs"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_29d773c2","line":49,"range":{"start_line":47,"start_character":0,"end_line":49,"end_character":37},"updated":"2017-07-23 06:02:50.000000000","message":"you also need to add a qemu check as\nthe queue size was fixed before qemu 2.7.\n\neven if you libvirt supports this config option that does not mean qemu does.","commit_id":"01a15b24c9261520f623f7235a68495dde3ea0e0"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"e48917ac4c190f7b0548f7a50a8e8b6916fa69ba","unresolved":false,"context_lines":[{"line_number":44,"context_line":"CONF \u003d nova.conf.CONF"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"# virtio-net.tx_queue_size support"},{"line_number":47,"context_line":"MIN_LIBVIRT_TX_QUEUE_SIZE \u003d (3, 6, 0)"},{"line_number":48,"context_line":"# virtio-net.rx_queue_size support"},{"line_number":49,"context_line":"MIN_LIBVIRT_RX_QUEUE_SIZE \u003d (2, 3, 0)"},{"line_number":50,"context_line":"# vhostuser queues support"},{"line_number":51,"context_line":"MIN_LIBVIRT_VHOSTUSER_MQ \u003d (1, 2, 17)"},{"line_number":52,"context_line":"#  vlan tag for macvtap passthrough mode on SRIOV VFs"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_13f0e3fd","line":49,"range":{"start_line":47,"start_character":0,"end_line":49,"end_character":37},"in_reply_to":"ff346bd7_29d773c2","updated":"2017-07-26 22:22:43.000000000","message":"Done","commit_id":"01a15b24c9261520f623f7235a68495dde3ea0e0"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"9bcd28df4fe116ab5a94324c13fb5db7b180b9df","unresolved":false,"context_lines":[{"line_number":144,"context_line":"                                                                   inst_type)"},{"line_number":145,"context_line":"            driver \u003d vhost_drv or driver"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"        if (virt_type in (\u0027kvm\u0027, \u0027qemu\u0027) and"},{"line_number":148,"context_line":"                    model \u003d\u003d network_model.VIF_MODEL_VIRTIO):"},{"line_number":149,"context_line":"            rx_queue_size, tx_queue_size \u003d self._get_virtio_queues(image_meta)"},{"line_number":150,"context_line":"            if not host.has_min_version(MIN_LIBVIRT_RX_QUEUE_SIZE):"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_9121da50","line":147,"updated":"2017-07-24 07:47:08.000000000","message":"All of that check should be in driver so you can avoid to pass \u0027host\u0027.","commit_id":"01a15b24c9261520f623f7235a68495dde3ea0e0"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"463b03d00310318fb71c989268a0063037c3de4c","unresolved":false,"context_lines":[{"line_number":147,"context_line":"        if (virt_type in (\u0027kvm\u0027, \u0027qemu\u0027) and"},{"line_number":148,"context_line":"                    model \u003d\u003d network_model.VIF_MODEL_VIRTIO):"},{"line_number":149,"context_line":"            rx_queue_size, tx_queue_size \u003d self._get_virtio_queues(image_meta)"},{"line_number":150,"context_line":"            if not host.has_min_version(MIN_LIBVIRT_RX_QUEUE_SIZE):"},{"line_number":151,"context_line":"                LOG.debug(\u0027rx_queue_size is an unsupported feature.\u0027)"},{"line_number":152,"context_line":"                rx_queue_size \u003d None"},{"line_number":153,"context_line":"            if not host.has_min_version(MIN_LIBVIRT_TX_QUEUE_SIZE):"},{"line_number":154,"context_line":"                LOG.debug(\u0027tx_queue_size is an unsupported feature.\u0027)"},{"line_number":155,"context_line":"                tx_queue_size \u003d None"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"        designer.set_vif_guest_frontend_config(conf, mac, model, driver,"},{"line_number":158,"context_line":"                                               vhost_queues, rx_queue_size,"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_09fdcf39","line":155,"range":{"start_line":150,"start_character":12,"end_line":155,"end_character":36},"updated":"2017-07-23 06:02:50.000000000","message":"also need to check qemu version here.","commit_id":"01a15b24c9261520f623f7235a68495dde3ea0e0"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"e48917ac4c190f7b0548f7a50a8e8b6916fa69ba","unresolved":false,"context_lines":[{"line_number":147,"context_line":"        if (virt_type in (\u0027kvm\u0027, \u0027qemu\u0027) and"},{"line_number":148,"context_line":"                    model \u003d\u003d network_model.VIF_MODEL_VIRTIO):"},{"line_number":149,"context_line":"            rx_queue_size, tx_queue_size \u003d self._get_virtio_queues(image_meta)"},{"line_number":150,"context_line":"            if not host.has_min_version(MIN_LIBVIRT_RX_QUEUE_SIZE):"},{"line_number":151,"context_line":"                LOG.debug(\u0027rx_queue_size is an unsupported feature.\u0027)"},{"line_number":152,"context_line":"                rx_queue_size \u003d None"},{"line_number":153,"context_line":"            if not host.has_min_version(MIN_LIBVIRT_TX_QUEUE_SIZE):"},{"line_number":154,"context_line":"                LOG.debug(\u0027tx_queue_size is an unsupported feature.\u0027)"},{"line_number":155,"context_line":"                tx_queue_size \u003d None"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"        designer.set_vif_guest_frontend_config(conf, mac, model, driver,"},{"line_number":158,"context_line":"                                               vhost_queues, rx_queue_size,"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_d3f5bbee","line":155,"range":{"start_line":150,"start_character":12,"end_line":155,"end_character":36},"in_reply_to":"ff346bd7_09fdcf39","updated":"2017-07-26 22:22:43.000000000","message":"Done","commit_id":"01a15b24c9261520f623f7235a68495dde3ea0e0"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"9bcd28df4fe116ab5a94324c13fb5db7b180b9df","unresolved":false,"context_lines":[{"line_number":190,"context_line":""},{"line_number":191,"context_line":"        return (driver, vhost_queues)"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"    def _get_virtio_queues(self, image_meta):"},{"line_number":194,"context_line":"        \"\"\"A methods to set the size of virtio-net rx/tx queues,"},{"line_number":195,"context_line":"           if requested in extra specs."},{"line_number":196,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_d1276264","line":193,"updated":"2017-07-24 07:47:08.000000000","message":"This should be renamed to remove \u0027virtio\u0027 and you should to have this in virt/hardware.py","commit_id":"01a15b24c9261520f623f7235a68495dde3ea0e0"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"9bcd28df4fe116ab5a94324c13fb5db7b180b9df","unresolved":false,"context_lines":[{"line_number":192,"context_line":""},{"line_number":193,"context_line":"    def _get_virtio_queues(self, image_meta):"},{"line_number":194,"context_line":"        \"\"\"A methods to set the size of virtio-net rx/tx queues,"},{"line_number":195,"context_line":"           if requested in extra specs."},{"line_number":196,"context_line":"        \"\"\""},{"line_number":197,"context_line":"        if not isinstance(image_meta, objects.ImageMeta):"},{"line_number":198,"context_line":"            image_meta \u003d objects.ImageMeta.from_dict(image_meta)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_d1564207","line":195,"updated":"2017-07-24 07:47:08.000000000","message":"s/extra specs/image props","commit_id":"01a15b24c9261520f623f7235a68495dde3ea0e0"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"e48917ac4c190f7b0548f7a50a8e8b6916fa69ba","unresolved":false,"context_lines":[{"line_number":192,"context_line":""},{"line_number":193,"context_line":"    def _get_virtio_queues(self, image_meta):"},{"line_number":194,"context_line":"        \"\"\"A methods to set the size of virtio-net rx/tx queues,"},{"line_number":195,"context_line":"           if requested in extra specs."},{"line_number":196,"context_line":"        \"\"\""},{"line_number":197,"context_line":"        if not isinstance(image_meta, objects.ImageMeta):"},{"line_number":198,"context_line":"            image_meta \u003d objects.ImageMeta.from_dict(image_meta)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_f3f0bfff","line":195,"in_reply_to":"ff346bd7_d1564207","updated":"2017-07-26 22:22:43.000000000","message":"Done","commit_id":"01a15b24c9261520f623f7235a68495dde3ea0e0"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"9bcd28df4fe116ab5a94324c13fb5db7b180b9df","unresolved":false,"context_lines":[{"line_number":197,"context_line":"        if not isinstance(image_meta, objects.ImageMeta):"},{"line_number":198,"context_line":"            image_meta \u003d objects.ImageMeta.from_dict(image_meta)"},{"line_number":199,"context_line":"        img_props \u003d image_meta.properties"},{"line_number":200,"context_line":"        return (img_props.get(\u0027hw_virtio_net_rx_queue_size\u0027),"},{"line_number":201,"context_line":"                img_props.get(\u0027hw_virtio_net_tx_queue_size\u0027))"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"    def _get_max_tap_queues(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_316a2e80","line":200,"updated":"2017-07-24 07:47:08.000000000","message":"I think we should be more general at some points an other NIC could support theses options.","commit_id":"01a15b24c9261520f623f7235a68495dde3ea0e0"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"afd0349b73f39e27244c154fd1cb8b756d64b7fb","unresolved":false,"context_lines":[{"line_number":144,"context_line":"                                                                   inst_type)"},{"line_number":145,"context_line":"            driver \u003d vhost_drv or driver"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"        if (virt_type in (\u0027kvm\u0027, \u0027qemu\u0027) and"},{"line_number":148,"context_line":"                    model \u003d\u003d network_model.VIF_MODEL_VIRTIO):"},{"line_number":149,"context_line":"            rx_queue_size, tx_queue_size \u003d self._get_virtio_queues(image_meta)"},{"line_number":150,"context_line":"            # rx and tx queue size support were implemented in separate"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff346bd7_d696d43e","line":147,"updated":"2017-07-26 15:17:57.000000000","message":"This validation should be in libvirt/driver.py so you don\u0027t have to import \u0027host\u0027","commit_id":"db6bb099d89befdb04de6f373d11f27f588dfe50"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"1520a904c026c8b7677fd9af951acbbbd77a014b","unresolved":false,"context_lines":[{"line_number":144,"context_line":"                                                                   inst_type)"},{"line_number":145,"context_line":"            driver \u003d vhost_drv or driver"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"        if (virt_type in (\u0027kvm\u0027, \u0027qemu\u0027) and"},{"line_number":148,"context_line":"                    model \u003d\u003d network_model.VIF_MODEL_VIRTIO):"},{"line_number":149,"context_line":"            rx_queue_size, tx_queue_size \u003d self._get_virtio_queues(image_meta)"},{"line_number":150,"context_line":"            # rx and tx queue size support were implemented in separate"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff346bd7_bf3d60de","line":147,"in_reply_to":"ff346bd7_d696d43e","updated":"2017-07-26 17:40:03.000000000","message":"All the code in libvirt/driver.py seems to hand over decisions on how to configure the VIFs into this file, and this file largely hands over the decision-making to os-vif.  Moving the decision-making over there seems like the wrong direction.\n\nMostly, I\u0027m parroting how the virtio-net multi-queue support was done, since it seems to be dealing with a highly similar issue.","commit_id":"db6bb099d89befdb04de6f373d11f27f588dfe50"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"afd0349b73f39e27244c154fd1cb8b756d64b7fb","unresolved":false,"context_lines":[{"line_number":196,"context_line":""},{"line_number":197,"context_line":"        return (driver, vhost_queues)"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"    def _get_virtio_queues(self, image_meta):"},{"line_number":200,"context_line":"        \"\"\"A methods to set the size of virtio-net rx/tx queues,"},{"line_number":201,"context_line":"           if requested in extra specs."},{"line_number":202,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"ff346bd7_f6955834","line":199,"updated":"2017-07-26 15:17:57.000000000","message":"You forgot to address that:\n\n - this function should be in virt/hardware.py\n - the name should not refer virtio has some other NIC might have this option.","commit_id":"db6bb099d89befdb04de6f373d11f27f588dfe50"},{"author":{"_account_id":5280,"name":"Nicolas Simonds","email":"nic@metacloud.com","username":"nicolas.simonds"},"change_message_id":"1520a904c026c8b7677fd9af951acbbbd77a014b","unresolved":false,"context_lines":[{"line_number":196,"context_line":""},{"line_number":197,"context_line":"        return (driver, vhost_queues)"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"    def _get_virtio_queues(self, image_meta):"},{"line_number":200,"context_line":"        \"\"\"A methods to set the size of virtio-net rx/tx queues,"},{"line_number":201,"context_line":"           if requested in extra specs."},{"line_number":202,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"ff346bd7_ec32245e","line":199,"in_reply_to":"ff346bd7_f6955834","updated":"2017-07-26 17:40:03.000000000","message":"These tunables are entirely specific to the virtio-net driver in QEMU, at least according to the libvirt docs.  virt/hardware.py seems to be the place for \"hypervisor-apathetic\", or at least broadly-applicable concepts, so moving this there, and giving it a name that implies that its intended scope is more than it is, would seem to invite higher expectations from this code than are warranted.\n\nThis patch set compels Nova to expose a specific option in a specific hypervisor to yield a specific desired outcome.  The names are intended to reflect that.","commit_id":"db6bb099d89befdb04de6f373d11f27f588dfe50"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"3ac73486941cf28f2aa457dd5fb35c6bb5102fc0","unresolved":false,"context_lines":[{"line_number":191,"context_line":""},{"line_number":192,"context_line":"        return (driver, vhost_queues)"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"    def _get_virtio_net_queue_sizes(self, image_meta, host):"},{"line_number":195,"context_line":"        \"\"\"Set the size of virtio-net rx/tx queues,"},{"line_number":196,"context_line":"           if requested in image props."},{"line_number":197,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"df3967d1_bca535a4","line":194,"updated":"2017-08-04 12:22:24.000000000","message":"Please remove the virtio prefix, today only that model handles it but we can have more NICs to provide such option so it\u0027s better to be general.","commit_id":"d4d3d8af683555bcb34df6c87c26c302bbfaa2cc"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"3ac73486941cf28f2aa457dd5fb35c6bb5102fc0","unresolved":false,"context_lines":[{"line_number":199,"context_line":"            image_meta \u003d objects.ImageMeta.from_dict(image_meta)"},{"line_number":200,"context_line":"        img_props \u003d image_meta.properties"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"        def fetch_valid_queue_size(property, lv_ver, hv_ver):"},{"line_number":203,"context_line":"            \"\"\"encapsulate all the required sanity checks. validates that:"},{"line_number":204,"context_line":"            - the value has been set in image props"},{"line_number":205,"context_line":"            - the host is running a compatible libvirt/qemu"}],"source_content_type":"text/x-python","patch_set":5,"id":"df3967d1_3ce025cd","line":202,"updated":"2017-08-04 12:22:24.000000000","message":"This should be in virt/hardware.py","commit_id":"d4d3d8af683555bcb34df6c87c26c302bbfaa2cc"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"3ac73486941cf28f2aa457dd5fb35c6bb5102fc0","unresolved":false,"context_lines":[{"line_number":219,"context_line":""},{"line_number":220,"context_line":"        # rx and tx queue size support were implemented in separate"},{"line_number":221,"context_line":"        # versions of libvirt"},{"line_number":222,"context_line":"        return (fetch_valid_queue_size(\u0027hw_virtio_net_rx_queue_size\u0027,"},{"line_number":223,"context_line":"                                       MIN_LIBVIRT_RX_QUEUE_SIZE,"},{"line_number":224,"context_line":"                                       MIN_QEMU_RX_QUEUE_SIZE),"},{"line_number":225,"context_line":"                fetch_valid_queue_size(\u0027hw_virtio_net_tx_queue_size\u0027,"}],"source_content_type":"text/x-python","patch_set":5,"id":"df3967d1_1ce321db","line":222,"updated":"2017-08-04 12:22:24.000000000","message":"The check of qemu/libvirt version should be in libvirt/driver.py so you don\u0027t have to bring host in vif.py","commit_id":"d4d3d8af683555bcb34df6c87c26c302bbfaa2cc"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3c8fbadd42767df6d524477403eb86d83c60f5a2","unresolved":false,"context_lines":[{"line_number":98,"context_line":"    return vif_model in valid_models[virt_type]"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"def version_to_string(version):"},{"line_number":102,"context_line":"    return \u0027.\u0027.join([str(x) for x in version])"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"bf659307_d73a88e5","line":101,"updated":"2018-04-10 14:19:46.000000000","message":"This should probably be brought out somewhere since you\u0027re duplicating this from driver.py, and probably elsewhere.","commit_id":"45994be9669b49280356d0d33b2256e9fb9217f5"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"6394cc4a41e4ce519bdae64d843b86472262711f","unresolved":false,"context_lines":[{"line_number":98,"context_line":"    return vif_model in valid_models[virt_type]"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"def version_to_string(version):"},{"line_number":102,"context_line":"    return \u0027.\u0027.join([str(x) for x in version])"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"bf659307_b401ee3a","line":101,"in_reply_to":"bf659307_d73a88e5","updated":"2018-04-11 14:50:17.000000000","message":"Sure, please find the patch here:\n\n  https://review.openstack.org/#/c/560455/","commit_id":"45994be9669b49280356d0d33b2256e9fb9217f5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3c8fbadd42767df6d524477403eb86d83c60f5a2","unresolved":false,"context_lines":[{"line_number":119,"context_line":"        return prefix + devname[3:]"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"    def get_base_config(self, instance, mac, image_meta, inst_type,"},{"line_number":122,"context_line":"                        virt_type, vnic_type, host):"},{"line_number":123,"context_line":"        # TODO(sahid): We should rewrote it. This method handle too"},{"line_number":124,"context_line":"        # many unrelated things. We probably need to have a specific"},{"line_number":125,"context_line":"        # virtio; vhost, vhostuser functions."}],"source_content_type":"text/x-python","patch_set":13,"id":"bf659307_e2726cad","line":122,"updated":"2018-04-10 14:19:46.000000000","message":"Can you split out the refactor of this method and all the related test change into a patch that goes ahead of this one? It would make it easier to validate the test change for the actual functional bit in here.","commit_id":"45994be9669b49280356d0d33b2256e9fb9217f5"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"6394cc4a41e4ce519bdae64d843b86472262711f","unresolved":false,"context_lines":[{"line_number":119,"context_line":"        return prefix + devname[3:]"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"    def get_base_config(self, instance, mac, image_meta, inst_type,"},{"line_number":122,"context_line":"                        virt_type, vnic_type, host):"},{"line_number":123,"context_line":"        # TODO(sahid): We should rewrote it. This method handle too"},{"line_number":124,"context_line":"        # many unrelated things. We probably need to have a specific"},{"line_number":125,"context_line":"        # virtio; vhost, vhostuser functions."}],"source_content_type":"text/x-python","patch_set":13,"id":"bf659307_748cb6d6","line":122,"in_reply_to":"bf659307_e2726cad","updated":"2018-04-11 14:50:17.000000000","message":"Patch here:\n\n   https://review.openstack.org/#/c/560456/","commit_id":"45994be9669b49280356d0d33b2256e9fb9217f5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3c8fbadd42767df6d524477403eb86d83c60f5a2","unresolved":false,"context_lines":[{"line_number":120,"context_line":""},{"line_number":121,"context_line":"    def get_base_config(self, instance, mac, image_meta, inst_type,"},{"line_number":122,"context_line":"                        virt_type, vnic_type, host):"},{"line_number":123,"context_line":"        # TODO(sahid): We should rewrote it. This method handle too"},{"line_number":124,"context_line":"        # many unrelated things. We probably need to have a specific"},{"line_number":125,"context_line":"        # virtio; vhost, vhostuser functions."},{"line_number":126,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"bf659307_77a7fca8","line":123,"range":{"start_line":123,"start_character":57,"end_line":123,"end_character":63},"updated":"2018-04-10 14:19:46.000000000","message":"handles","commit_id":"45994be9669b49280356d0d33b2256e9fb9217f5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3c8fbadd42767df6d524477403eb86d83c60f5a2","unresolved":false,"context_lines":[{"line_number":120,"context_line":""},{"line_number":121,"context_line":"    def get_base_config(self, instance, mac, image_meta, inst_type,"},{"line_number":122,"context_line":"                        virt_type, vnic_type, host):"},{"line_number":123,"context_line":"        # TODO(sahid): We should rewrote it. This method handle too"},{"line_number":124,"context_line":"        # many unrelated things. We probably need to have a specific"},{"line_number":125,"context_line":"        # virtio; vhost, vhostuser functions."},{"line_number":126,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"bf659307_57ac388b","line":123,"range":{"start_line":123,"start_character":33,"end_line":123,"end_character":40},"updated":"2018-04-10 14:19:46.000000000","message":"rewrite","commit_id":"45994be9669b49280356d0d33b2256e9fb9217f5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3c8fbadd42767df6d524477403eb86d83c60f5a2","unresolved":false,"context_lines":[{"line_number":122,"context_line":"                        virt_type, vnic_type, host):"},{"line_number":123,"context_line":"        # TODO(sahid): We should rewrote it. This method handle too"},{"line_number":124,"context_line":"        # many unrelated things. We probably need to have a specific"},{"line_number":125,"context_line":"        # virtio; vhost, vhostuser functions."},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"        conf \u003d vconfig.LibvirtConfigGuestInterface()"},{"line_number":128,"context_line":"        # Default to letting libvirt / the hypervisor choose the model"}],"source_content_type":"text/x-python","patch_set":13,"id":"bf659307_97cbb051","line":125,"range":{"start_line":125,"start_character":16,"end_line":125,"end_character":17},"updated":"2018-04-10 14:19:46.000000000","message":",","commit_id":"45994be9669b49280356d0d33b2256e9fb9217f5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3c8fbadd42767df6d524477403eb86d83c60f5a2","unresolved":false,"context_lines":[{"line_number":162,"context_line":"            # the driver should be \u0027vhostuser\u0027. That currently does"},{"line_number":163,"context_line":"            # not create any issue since QEMU ignores the driver"},{"line_number":164,"context_line":"            # argument for vhostuser interface but we should probably"},{"line_number":165,"context_line":"            # fix that anyway. Also we should enforce driver to use"},{"line_number":166,"context_line":"            # vhost and not None."},{"line_number":167,"context_line":"            driver \u003d vhost_drv or driver"},{"line_number":168,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"bf659307_57c5183b","line":165,"range":{"start_line":165,"start_character":54,"end_line":165,"end_character":67},"updated":"2018-04-10 14:19:46.000000000","message":"that the driver uses","commit_id":"45994be9669b49280356d0d33b2256e9fb9217f5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3c8fbadd42767df6d524477403eb86d83c60f5a2","unresolved":false,"context_lines":[{"line_number":168,"context_line":""},{"line_number":169,"context_line":"        rx_queue_size \u003d None"},{"line_number":170,"context_line":"        if driver \u003d\u003d \u0027vhost\u0027 or driver is None:"},{"line_number":171,"context_line":"            # vhost backend only support update of RX queue size"},{"line_number":172,"context_line":"            rx_queue_size, _ \u003d self._get_virtio_queue_sizes(host)"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"        designer.set_vif_guest_frontend_config("}],"source_content_type":"text/x-python","patch_set":13,"id":"bf659307_17dd6013","line":171,"range":{"start_line":171,"start_character":33,"end_line":171,"end_character":40},"updated":"2018-04-10 14:19:46.000000000","message":"supports","commit_id":"45994be9669b49280356d0d33b2256e9fb9217f5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3c8fbadd42767df6d524477403eb86d83c60f5a2","unresolved":false,"context_lines":[{"line_number":492,"context_line":"        \"\"\""},{"line_number":493,"context_line":"        def validate_queue_size(size):"},{"line_number":494,"context_line":"            # Size should be between 256 and 1024 and power of 2."},{"line_number":495,"context_line":"            if (size \u003c MIN_RXTX_QUEUE_SIZE or"},{"line_number":496,"context_line":"                size \u003e MAX_RXTX_QUEUE_SIZE or"},{"line_number":497,"context_line":"                bin(size).count(\"1\") !\u003d 1):"},{"line_number":498,"context_line":"                raise ValueError"},{"line_number":499,"context_line":"            return size"}],"source_content_type":"text/x-python","patch_set":13,"id":"bf659307_57f3d89e","line":496,"range":{"start_line":495,"start_character":16,"end_line":496,"end_character":42},"updated":"2018-04-10 14:19:46.000000000","message":"You don\u0027t need this as oslo_config will have done this for you.","commit_id":"45994be9669b49280356d0d33b2256e9fb9217f5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3c8fbadd42767df6d524477403eb86d83c60f5a2","unresolved":false,"context_lines":[{"line_number":494,"context_line":"            # Size should be between 256 and 1024 and power of 2."},{"line_number":495,"context_line":"            if (size \u003c MIN_RXTX_QUEUE_SIZE or"},{"line_number":496,"context_line":"                size \u003e MAX_RXTX_QUEUE_SIZE or"},{"line_number":497,"context_line":"                bin(size).count(\"1\") !\u003d 1):"},{"line_number":498,"context_line":"                raise ValueError"},{"line_number":499,"context_line":"            return size"},{"line_number":500,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"bf659307_b7391440","line":497,"updated":"2018-04-10 14:19:46.000000000","message":"All of these lines should be indented four more spaces to distinguish them from the body of the block below.\n\nAlso, this is kindof an odd way to validate the power-of-two-ness. This is typically a very fast way:\n\n is_power_of_two \u003d (size \u0026 (size - 1)) \u003d\u003d 0\n\nAlso it would be a lot nicer to see this blow up at startup time if it\u0027s a hard requirement. I don\u0027t see any examples in the current config, but you can define bare options yourself, and provide a \"type\" handler which can do the validation and coercion. I wonder if it would be better to do that so you don\u0027t need to do any of the validation here every time we go to build an instance xml?","commit_id":"45994be9669b49280356d0d33b2256e9fb9217f5"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"c7513120fee8f69ef842824e7c923179f89a57cb","unresolved":false,"context_lines":[{"line_number":60,"context_line":"MIN_QEMU_RX_QUEUE_SIZE \u003d (2, 7, 0)"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"# min/max value allowed"},{"line_number":63,"context_line":"MIN_RXTX_QUEUE_SIZE \u003d 256"},{"line_number":64,"context_line":"MAX_RXTX_QUEUE_SIZE \u003d 1024"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"bf659307_37d39d62","line":63,"updated":"2018-04-11 16:38:49.000000000","message":"Hum... no need of this anymore. I\u0027m waiting for comments before to respine and remove them.","commit_id":"0957c45b5407d6a016f0de082de2d772c0582574"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3ff241fd3a1800542d346fc2a972f65999adfda4","unresolved":false,"context_lines":[{"line_number":507,"context_line":"                        libvirt_utils.version_to_string("},{"line_number":508,"context_line":"                            MIN_QEMU_TX_QUEUE_SIZE))"},{"line_number":509,"context_line":"            tx \u003d None"},{"line_number":510,"context_line":"        return rx, tx"},{"line_number":511,"context_line":""},{"line_number":512,"context_line":"    def get_config_ib_hostdev(self, instance, vif, image_meta,"},{"line_number":513,"context_line":"                              inst_type, virt_type, host):"}],"source_content_type":"text/x-python","patch_set":14,"id":"bf659307_b7980dc0","line":510,"updated":"2018-04-11 16:42:01.000000000","message":"This is much cleaner and easier to follow, IMHO, thanks.","commit_id":"0957c45b5407d6a016f0de082de2d772c0582574"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"9f5feac57314abdc3d565ca6d327c5930efc1bb6","unresolved":false,"context_lines":[{"line_number":507,"context_line":"                        libvirt_utils.version_to_string("},{"line_number":508,"context_line":"                            MIN_QEMU_TX_QUEUE_SIZE))"},{"line_number":509,"context_line":"            tx \u003d None"},{"line_number":510,"context_line":"        return rx, tx"},{"line_number":511,"context_line":""},{"line_number":512,"context_line":"    def get_config_ib_hostdev(self, instance, vif, image_meta,"},{"line_number":513,"context_line":"                              inst_type, virt_type, host):"}],"source_content_type":"text/x-python","patch_set":14,"id":"bf659307_79e6d5b2","line":510,"in_reply_to":"bf659307_7610225a","updated":"2018-04-12 13:06:48.000000000","message":"I don\u0027t know; The options introduced are not related to the service itself so blocking it to start-up looks a bit hard, no? and we only want to do this validation for certain kind of vif types.","commit_id":"0957c45b5407d6a016f0de082de2d772c0582574"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1dc099aad25296f97a427afe820eb278a293351b","unresolved":false,"context_lines":[{"line_number":507,"context_line":"                        libvirt_utils.version_to_string("},{"line_number":508,"context_line":"                            MIN_QEMU_TX_QUEUE_SIZE))"},{"line_number":509,"context_line":"            tx \u003d None"},{"line_number":510,"context_line":"        return rx, tx"},{"line_number":511,"context_line":""},{"line_number":512,"context_line":"    def get_config_ib_hostdev(self, instance, vif, image_meta,"},{"line_number":513,"context_line":"                              inst_type, virt_type, host):"}],"source_content_type":"text/x-python","patch_set":14,"id":"bf659307_49f19c94","line":510,"in_reply_to":"bf659307_79e6d5b2","updated":"2018-04-13 14:07:06.000000000","message":"We could do it at startup, during init_driver(). I\u0027m not all that concerned though I guess. You could make the case that you could start up nova, then shutdown and upgrade libvirt underneath it and things should continue working.\n\nAnyway, I could go either way, tbh.","commit_id":"0957c45b5407d6a016f0de082de2d772c0582574"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a566e21d6f0ee1905a8bdb3a9d796cf8524f61f8","unresolved":false,"context_lines":[{"line_number":507,"context_line":"                        libvirt_utils.version_to_string("},{"line_number":508,"context_line":"                            MIN_QEMU_TX_QUEUE_SIZE))"},{"line_number":509,"context_line":"            tx \u003d None"},{"line_number":510,"context_line":"        return rx, tx"},{"line_number":511,"context_line":""},{"line_number":512,"context_line":"    def get_config_ib_hostdev(self, instance, vif, image_meta,"},{"line_number":513,"context_line":"                              inst_type, virt_type, host):"}],"source_content_type":"text/x-python","patch_set":14,"id":"bf659307_7610225a","line":510,"in_reply_to":"bf659307_b7980dc0","updated":"2018-04-12 09:58:30.000000000","message":"Given that this is a global configuration option, I think it would make more sense to do this validation at boot time and fail to start rather than emitting a warning like this for every single VIF. mriedem suggested something similar in another review. The \u0027_do_quality_warnings\u0027 function isn\u0027t quite suitable but something else in \u0027init_host\u0027 would do.","commit_id":"0957c45b5407d6a016f0de082de2d772c0582574"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3ff241fd3a1800542d346fc2a972f65999adfda4","unresolved":false,"context_lines":[{"line_number":545,"context_line":"        if rx_queue_size or tx_queue_size:"},{"line_number":546,"context_line":"            # TODO(sahid): Specifically override driver to use"},{"line_number":547,"context_line":"            # vhostuser. It seems that the get_base_config method does"},{"line_number":548,"context_line":"            # not do that job correctly."},{"line_number":549,"context_line":"            conf.driver_name \u003d \"vhostuser\""},{"line_number":550,"context_line":"        designer.set_vif_host_backend_vhostuser_config("},{"line_number":551,"context_line":"            conf, vif.mode, vif.path, rx_queue_size, tx_queue_size)"}],"source_content_type":"text/x-python","patch_set":14,"id":"bf659307_57d89179","line":548,"updated":"2018-04-11 16:42:01.000000000","message":"This isn\u0027t really a TODO, since you\u0027re doing what you say here. What\u0027s the actual todo item? Address the method above where you question its logic (L158)?","commit_id":"0957c45b5407d6a016f0de082de2d772c0582574"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"9f5feac57314abdc3d565ca6d327c5930efc1bb6","unresolved":false,"context_lines":[{"line_number":545,"context_line":"        if rx_queue_size or tx_queue_size:"},{"line_number":546,"context_line":"            # TODO(sahid): Specifically override driver to use"},{"line_number":547,"context_line":"            # vhostuser. It seems that the get_base_config method does"},{"line_number":548,"context_line":"            # not do that job correctly."},{"line_number":549,"context_line":"            conf.driver_name \u003d \"vhostuser\""},{"line_number":550,"context_line":"        designer.set_vif_host_backend_vhostuser_config("},{"line_number":551,"context_line":"            conf, vif.mode, vif.path, rx_queue_size, tx_queue_size)"}],"source_content_type":"text/x-python","patch_set":14,"id":"bf659307_e5c5565d","line":548,"in_reply_to":"bf659307_57d89179","updated":"2018-04-12 13:06:48.000000000","message":"Yes.. I updated them to refer the fact that this code can be removed when get_base_config will be rewrite.","commit_id":"0957c45b5407d6a016f0de082de2d772c0582574"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1dc099aad25296f97a427afe820eb278a293351b","unresolved":false,"context_lines":[{"line_number":168,"context_line":"                # TODO(sahid): Specifically override driver to use"},{"line_number":169,"context_line":"                # vhost. This can be removed when get_base_config will be"},{"line_number":170,"context_line":"                # fixed and rewrite to set the correct backend."},{"line_number":171,"context_line":"                driver \u003d \u0027vhost\u0027"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"        designer.set_vif_guest_frontend_config("},{"line_number":174,"context_line":"            conf, mac, model, driver, vhost_queues, rx_queue_size)"}],"source_content_type":"text/x-python","patch_set":16,"id":"bf659307_c9ce0c2c","line":171,"updated":"2018-04-13 14:07:06.000000000","message":"This changed since PS14 and doesn\u0027t have a test.","commit_id":"328117870e1f61491b53dbd3e689f3a93a912685"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"648a3414eb5cc0a82902985e381d461aad7f8536","unresolved":false,"context_lines":[{"line_number":168,"context_line":"                # TODO(sahid): Specifically override driver to use"},{"line_number":169,"context_line":"                # vhost. This can be removed when get_base_config will be"},{"line_number":170,"context_line":"                # fixed and rewrite to set the correct backend."},{"line_number":171,"context_line":"                driver \u003d \u0027vhost\u0027"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"        designer.set_vif_guest_frontend_config("},{"line_number":174,"context_line":"            conf, mac, model, driver, vhost_queues, rx_queue_size)"}],"source_content_type":"text/x-python","patch_set":16,"id":"bf659307_33d84d1c","line":171,"in_reply_to":"bf659307_b31f7d59","updated":"2018-04-13 17:25:54.000000000","message":"Okay, this is turning into a mess of me thinking out loud, but:\n\ndriver\u003dvhost was kinda the default returning from this method anyway because of L152 which was setting it in almost all the non-special cases anyway. This is really just setting it if we didn\u0027t above and rx_queue_size is set in the config.\n\nAlso, answering my question above, we still need the driver in (vhost,None) check because we need to get rx_queue_size for the call to set_vif_guest_frontend_config() below.","commit_id":"328117870e1f61491b53dbd3e689f3a93a912685"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"cc1ab2743325bdc8c1edc5fc84a55b4d2941969b","unresolved":false,"context_lines":[{"line_number":168,"context_line":"                # TODO(sahid): Specifically override driver to use"},{"line_number":169,"context_line":"                # vhost. This can be removed when get_base_config will be"},{"line_number":170,"context_line":"                # fixed and rewrite to set the correct backend."},{"line_number":171,"context_line":"                driver \u003d \u0027vhost\u0027"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"        designer.set_vif_guest_frontend_config("},{"line_number":174,"context_line":"            conf, mac, model, driver, vhost_queues, rx_queue_size)"}],"source_content_type":"text/x-python","patch_set":16,"id":"bf659307_b31f7d59","line":171,"in_reply_to":"bf659307_c9ce0c2c","updated":"2018-04-13 16:59:40.000000000","message":"Also, this is a little confusing because it might already be vhost. Maybe the if on L164 should just be \"if not driver\" ?\n\nI think what this code is doing is saying \"if we haven\u0027t picked a driver yet, and rx_queue_size was set, then default to vhost\". The problem is, that seems to be a bit of a generalized guess, because I assume this is why you need to fix up the vhostuser cases below, right?\n\nI was asking what other vif_types use vhost, because it would be nicer if we didn\u0027t have any of this in the base config, and instead just set driver\u003dvhost and driver\u003dvhostuser in the right places in the per-type methods below. Is that hard to do for some reason?","commit_id":"328117870e1f61491b53dbd3e689f3a93a912685"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"cc1ab2743325bdc8c1edc5fc84a55b4d2941969b","unresolved":false,"context_lines":[{"line_number":456,"context_line":"                                                        vif_id\u003dvif[\u0027id\u0027])"},{"line_number":457,"context_line":"        return mode, sock_path"},{"line_number":458,"context_line":""},{"line_number":459,"context_line":"    def get_config_vhostuser(self, instance, vif, image_meta,"},{"line_number":460,"context_line":"                            inst_type, virt_type, host):"},{"line_number":461,"context_line":"        conf \u003d self.get_base_config(instance, vif[\u0027address\u0027], image_meta,"},{"line_number":462,"context_line":"                                    inst_type, virt_type, vif[\u0027vnic_type\u0027],"}],"source_content_type":"text/x-python","patch_set":16,"id":"bf659307_98005836","line":459,"updated":"2018-04-13 16:59:40.000000000","message":"Talking to Sahid on IRC and looking at how/when we call this. I think this might be dead code, as we have handling for vhostuser stuff in os_vif_util, which should mean that we aren\u0027t ever calling this anymore.","commit_id":"328117870e1f61491b53dbd3e689f3a93a912685"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"cc1ab2743325bdc8c1edc5fc84a55b4d2941969b","unresolved":false,"context_lines":[{"line_number":469,"context_line":"            # be fixed and rewrite to set the correct backend."},{"line_number":470,"context_line":"            conf.driver_name \u003d \"vhostuser\""},{"line_number":471,"context_line":"        designer.set_vif_host_backend_vhostuser_config("},{"line_number":472,"context_line":"            conf, mode, sock_path, rx_queue_size, tx_queue_size)"},{"line_number":473,"context_line":""},{"line_number":474,"context_line":"        # (vladikr) Not setting up driver and queues for vhostuser"},{"line_number":475,"context_line":"        # as queues are not supported in Libvirt until version 1.2.17"}],"source_content_type":"text/x-python","patch_set":16,"id":"bf659307_b36d3dcd","line":472,"updated":"2018-04-13 16:59:40.000000000","message":"The queue sizes are only set here, and this is only called if we _don\u0027t_ do the os_vif path in get_config() below. That means that os_vif_util._nov_to_osvif_vhostuser() had to raise NotImplementedError so that we would fall through and call get_config_%(vif_type)s().\n\nI would imagine that the more common case is that we _do_ convert to os_vif in os_vif_utils for OVS and VROUTER models, and that we want the queues set in those cases, at least, right?","commit_id":"328117870e1f61491b53dbd3e689f3a93a912685"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"cc1ab2743325bdc8c1edc5fc84a55b4d2941969b","unresolved":false,"context_lines":[{"line_number":540,"context_line":"        conf.target_dev \u003d vif.vif_name"},{"line_number":541,"context_line":"        self._set_config_VIFPortProfile(instance, vif, conf)"},{"line_number":542,"context_line":""},{"line_number":543,"context_line":"    def _set_config_VIFVHostUser(self, instance, vif, conf, host\u003dNone):"},{"line_number":544,"context_line":"        rx_queue_size, tx_queue_size \u003d self._get_virtio_queue_sizes(host)"},{"line_number":545,"context_line":"        if rx_queue_size or tx_queue_size:"},{"line_number":546,"context_line":"            # TODO(sahid): Specifically override driver to use"}],"source_content_type":"text/x-python","patch_set":16,"id":"bf659307_733d65c2","line":543,"updated":"2018-04-13 16:59:40.000000000","message":"This gets called when we take the os_vif path below. But, we\u0027re never setting the tx and rx queue attributes in this case. So if we have a vif_type that is handled by an os_vif type, we\u0027ll never get the queue sizes set right?","commit_id":"328117870e1f61491b53dbd3e689f3a93a912685"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1dc099aad25296f97a427afe820eb278a293351b","unresolved":false,"context_lines":[{"line_number":544,"context_line":"        rx_queue_size, tx_queue_size \u003d self._get_virtio_queue_sizes(host)"},{"line_number":545,"context_line":"        if rx_queue_size or tx_queue_size:"},{"line_number":546,"context_line":"            # TODO(sahid): Specifically override driver to use"},{"line_number":547,"context_line":"            # vhostuser. This can be removed when get_base_config will"},{"line_number":548,"context_line":"            # be fixed and rewrite to set the correct backend."},{"line_number":549,"context_line":"            conf.driver_name \u003d \"vhostuser\""},{"line_number":550,"context_line":"        designer.set_vif_host_backend_vhostuser_config("},{"line_number":551,"context_line":"            conf, vif.mode, vif.path, rx_queue_size, tx_queue_size)"}],"source_content_type":"text/x-python","patch_set":16,"id":"bf659307_a93b9835","line":548,"range":{"start_line":547,"start_character":25,"end_line":548,"end_character":62},"updated":"2018-04-13 14:07:06.000000000","message":"Heh, okay with this updating I understand the todo item and this would be appropriate for a TODO, but.. not a huge deal.","commit_id":"328117870e1f61491b53dbd3e689f3a93a912685"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"648a3414eb5cc0a82902985e381d461aad7f8536","unresolved":false,"context_lines":[{"line_number":546,"context_line":"            # TODO(sahid): Specifically override driver to use"},{"line_number":547,"context_line":"            # vhostuser. This can be removed when get_base_config will"},{"line_number":548,"context_line":"            # be fixed and rewrite to set the correct backend."},{"line_number":549,"context_line":"            conf.driver_name \u003d \"vhostuser\""},{"line_number":550,"context_line":"        designer.set_vif_host_backend_vhostuser_config("},{"line_number":551,"context_line":"            conf, vif.mode, vif.path, rx_queue_size, tx_queue_size)"},{"line_number":552,"context_line":"        if not host.has_min_version(MIN_LIBVIRT_VHOSTUSER_MQ):"}],"source_content_type":"text/x-python","patch_set":16,"id":"bf659307_f32c1503","line":549,"updated":"2018-04-13 17:25:54.000000000","message":"This line is cleanup, but we think it\u0027s being ignored today anyway, per the TODO above. I want to move this out to a separate patch so that it doesn\u0027t confuse the fact that this doesn\u0027t actually change behavior in this patch.","commit_id":"328117870e1f61491b53dbd3e689f3a93a912685"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"545cab42408f821887e2d5028a0ff11c997b65d8","unresolved":false,"context_lines":[{"line_number":548,"context_line":"            # be fixed and rewrite to set the correct backend."},{"line_number":549,"context_line":"            conf.driver_name \u003d \"vhostuser\""},{"line_number":550,"context_line":"        designer.set_vif_host_backend_vhostuser_config("},{"line_number":551,"context_line":"            conf, vif.mode, vif.path, rx_queue_size, tx_queue_size)"},{"line_number":552,"context_line":"        if not host.has_min_version(MIN_LIBVIRT_VHOSTUSER_MQ):"},{"line_number":553,"context_line":"            LOG.debug(\u0027Queues are not a vhostuser supported feature.\u0027)"},{"line_number":554,"context_line":"            # TODO(sahid): No need of unset driver name."}],"source_content_type":"text/x-python","patch_set":16,"id":"bf659307_93703974","line":551,"updated":"2018-04-13 17:06:07.000000000","message":"Nevermind that last point, I was looking at a diff not from base here. This is where it gets set for vhostuser for the os_vif path","commit_id":"328117870e1f61491b53dbd3e689f3a93a912685"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a3626d4691ba8777550986454298822fe38b2cd6","unresolved":false,"context_lines":[{"line_number":157,"context_line":"            # not create any issue since QEMU ignores the driver"},{"line_number":158,"context_line":"            # argument for vhostuser interface but we should probably"},{"line_number":159,"context_line":"            # fix that anyway. Also we should enforce that the driver"},{"line_number":160,"context_line":"            # use vhost and not None."},{"line_number":161,"context_line":"            driver \u003d vhost_drv or driver"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        rx_queue_size \u003d None"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f6a8fd7_66f569cb","line":160,"updated":"2018-04-16 18:36:19.000000000","message":"FWIW I would have put all of these todo comments in the patch below this. Not a huge deal, but that would be better.","commit_id":"bbba28d7e5c6ad72abfc6e7b52f52014d1b45c9c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"34e77a9d491dc21d1a094f1144312d0cbac49ffe","unresolved":false,"context_lines":[{"line_number":157,"context_line":"            # not create any issue since QEMU ignores the driver"},{"line_number":158,"context_line":"            # argument for vhostuser interface but we should probably"},{"line_number":159,"context_line":"            # fix that anyway. Also we should enforce that the driver"},{"line_number":160,"context_line":"            # use vhost and not None."},{"line_number":161,"context_line":"            driver \u003d vhost_drv or driver"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        rx_queue_size \u003d None"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f6a8fd7_3c121e5a","line":160,"in_reply_to":"9f6a8fd7_66f569cb","updated":"2018-04-16 19:52:30.000000000","message":"well looking at the docs for the driver option \nhttps://libvirt.org/formatdomain.html#elementsDriverBackendOptions\n\nin the name section it says the name is ignored but it also says the backend driver is always \"vhost-user\"\n\nso personally i think we should be using vhost-user not vhost if we do generate the name attribute but for saftey we should just not set the name at all incase future libvirt versions start supporting name on vhost-user interfaces and use someting other then vhost.","commit_id":"bbba28d7e5c6ad72abfc6e7b52f52014d1b45c9c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"34e77a9d491dc21d1a094f1144312d0cbac49ffe","unresolved":false,"context_lines":[{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        rx_queue_size \u003d None"},{"line_number":164,"context_line":"        if driver \u003d\u003d \u0027vhost\u0027 or driver is None:"},{"line_number":165,"context_line":"            # vhost backend only supports update of RX queue size"},{"line_number":166,"context_line":"            rx_queue_size, _ \u003d self._get_virtio_queue_sizes(host)"},{"line_number":167,"context_line":"            if rx_queue_size:"},{"line_number":168,"context_line":"                # TODO(sahid): Specifically override driver to use"},{"line_number":169,"context_line":"                # vhost. This can be removed when get_base_config will be"},{"line_number":170,"context_line":"                # fixed and rewrite to set the correct backend."},{"line_number":171,"context_line":"                driver \u003d \u0027vhost\u0027"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"        designer.set_vif_guest_frontend_config("},{"line_number":174,"context_line":"            conf, mac, model, driver, vhost_queues, rx_queue_size)"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f6a8fd7_1cbcfa03","line":171,"range":{"start_line":165,"start_character":12,"end_line":171,"end_character":32},"updated":"2018-04-16 19:52:30.000000000","message":"are we sure the queue length is not supported by the \"qemu\" driver e.g. when using virtio in qemu with our kvm offloading networking via kernel or userspce vhost?\n\nforcing vhost here will prevent this working without kvm or xen.\n\nif we look at https://libvirt.org/formatdomain.html#elementsDriverBackendOptions and go down to the rx_queue_size option. \n\nit does not state that it only works with vhost dirver and implies it will work for and virtio interface.","commit_id":"bbba28d7e5c6ad72abfc6e7b52f52014d1b45c9c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ffff2f6748c4ec7b5d04bfd1d9afb3e9bac497e8","unresolved":false,"context_lines":[{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        rx_queue_size \u003d None"},{"line_number":164,"context_line":"        if driver \u003d\u003d \u0027vhost\u0027 or driver is None:"},{"line_number":165,"context_line":"            # vhost backend only supports update of RX queue size"},{"line_number":166,"context_line":"            rx_queue_size, _ \u003d self._get_virtio_queue_sizes(host)"},{"line_number":167,"context_line":"            if rx_queue_size:"},{"line_number":168,"context_line":"                # TODO(sahid): Specifically override driver to use"},{"line_number":169,"context_line":"                # vhost. This can be removed when get_base_config will be"},{"line_number":170,"context_line":"                # fixed and rewrite to set the correct backend."},{"line_number":171,"context_line":"                driver \u003d \u0027vhost\u0027"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"        designer.set_vif_guest_frontend_config("},{"line_number":174,"context_line":"            conf, mac, model, driver, vhost_queues, rx_queue_size)"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f6a8fd7_1ce15a9b","line":171,"range":{"start_line":165,"start_character":12,"end_line":171,"end_character":32},"in_reply_to":"9f6a8fd7_1cbcfa03","updated":"2018-04-16 20:02:48.000000000","message":"Ah, also, after talking to Sean on IRC, I see that we don\u0027t actually always set vhost prior to this patch, only if hw_multiqueue is requested.\n\nSo, going back to a previous comment of mine, I think we should avoid adding this shortcut in the base config and go to just setting it explicitly in each handler if/when we know it belongs.","commit_id":"bbba28d7e5c6ad72abfc6e7b52f52014d1b45c9c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f10acd80deb6209c835df4a84fd4f868c08c5fa3","unresolved":false,"context_lines":[{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        rx_queue_size \u003d None"},{"line_number":164,"context_line":"        if driver \u003d\u003d \u0027vhost\u0027 or driver is None:"},{"line_number":165,"context_line":"            # vhost backend only supports update of RX queue size"},{"line_number":166,"context_line":"            rx_queue_size, _ \u003d self._get_virtio_queue_sizes(host)"},{"line_number":167,"context_line":"            if rx_queue_size:"},{"line_number":168,"context_line":"                # TODO(sahid): Specifically override driver to use"},{"line_number":169,"context_line":"                # vhost. This can be removed when get_base_config will be"},{"line_number":170,"context_line":"                # fixed and rewrite to set the correct backend."},{"line_number":171,"context_line":"                driver \u003d \u0027vhost\u0027"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"        designer.set_vif_guest_frontend_config("},{"line_number":174,"context_line":"            conf, mac, model, driver, vhost_queues, rx_queue_size)"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f6a8fd7_b775e99a","line":171,"range":{"start_line":165,"start_character":12,"end_line":171,"end_character":32},"in_reply_to":"9f6a8fd7_1ce15a9b","updated":"2018-04-16 20:33:36.000000000","message":"so looking at https://github.com/libvirt/libvirt/blob/ad24406440e8572446f528c428dfae13f347a82c/tests/qemuxml2argvdata/net-virtio-rxtxqueuesize.xml\nit would appear that interface type user support these options also.\nhttps://libvirt.org/formatdomain.html#elementsNICSSlirp\n\nthe corresponding qemu args for that xml\n\nhttps://github.com/libvirt/libvirt/blob/ad24406440e8572446f528c428dfae13f347a82c/tests/qemuxml2argvdata/net-virtio-rxtxqueuesize.args\n\nlook correct so to me.\n\nall of libvirts unit tests omit the driver name when setting the rx_queue_size i think its safe for us to also not set it.\n\nlooking at the code libvirt asserts that qemu supprots\nthe queue lenght option here:\n\nhttps://github.com/libvirt/libvirt/blob/5b2c3d404ec0ce9f1c476f7ce879c485120745ba/src/qemu/qemu_command.c#L3478-L3493\n\nand libivrt asserts the queue length is a power of 2 here:\n\nhttps://github.com/libvirt/libvirt/blob/c247ff55293161d92b3c127334492252836e7ec4/src/qemu/qemu_domain.c#L3982-L3993\n\nin both cases the only precondition i see is that the interface model is virtio so i dont think there is a hard requirement for vhost or vhost user at least in libvirt i just requires that the vif model is virtio.","commit_id":"bbba28d7e5c6ad72abfc6e7b52f52014d1b45c9c"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"16b25c78218945db2dacebc91e8602ed7905db2a","unresolved":false,"context_lines":[{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        rx_queue_size \u003d None"},{"line_number":164,"context_line":"        if driver \u003d\u003d \u0027vhost\u0027 or driver is None:"},{"line_number":165,"context_line":"            # vhost backend only supports update of RX queue size"},{"line_number":166,"context_line":"            rx_queue_size, _ \u003d self._get_virtio_queue_sizes(host)"},{"line_number":167,"context_line":"            if rx_queue_size:"},{"line_number":168,"context_line":"                # TODO(sahid): Specifically override driver to use"},{"line_number":169,"context_line":"                # vhost. This can be removed when get_base_config will be"},{"line_number":170,"context_line":"                # fixed and rewrite to set the correct backend."},{"line_number":171,"context_line":"                driver \u003d \u0027vhost\u0027"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"        designer.set_vif_guest_frontend_config("},{"line_number":174,"context_line":"            conf, mac, model, driver, vhost_queues, rx_queue_size)"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f6a8fd7_d2898126","line":171,"range":{"start_line":165,"start_character":12,"end_line":171,"end_character":32},"in_reply_to":"9f6a8fd7_b775e99a","updated":"2018-04-17 08:27:34.000000000","message":"If we don\u0027t set the driver name we don\u0027t generate the XML node driver which means we can\u0027t pass the tx/rx queue size element. That is why I limited the scope on forcing the correct driver only when tx/rx queue size are requested.\n\nIf I start to change that behavior in ConfigGuestInterface I will start to rewrite and change lot of code where I would like if possible backport this change.","commit_id":"bbba28d7e5c6ad72abfc6e7b52f52014d1b45c9c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"cdc5bd89691cb8eb9220fe8c2903f672540ceb16","unresolved":false,"context_lines":[{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        rx_queue_size \u003d None"},{"line_number":164,"context_line":"        if driver \u003d\u003d \u0027vhost\u0027 or driver is None:"},{"line_number":165,"context_line":"            # vhost backend only supports update of RX queue size"},{"line_number":166,"context_line":"            rx_queue_size, _ \u003d self._get_virtio_queue_sizes(host)"},{"line_number":167,"context_line":"            if rx_queue_size:"},{"line_number":168,"context_line":"                # TODO(sahid): Specifically override driver to use"},{"line_number":169,"context_line":"                # vhost. This can be removed when get_base_config will be"},{"line_number":170,"context_line":"                # fixed and rewrite to set the correct backend."},{"line_number":171,"context_line":"                driver \u003d \u0027vhost\u0027"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"        designer.set_vif_guest_frontend_config("},{"line_number":174,"context_line":"            conf, mac, model, driver, vhost_queues, rx_queue_size)"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f6a8fd7_35c6a769","line":171,"range":{"start_line":165,"start_character":12,"end_line":171,"end_character":32},"in_reply_to":"9f6a8fd7_d2898126","updated":"2018-04-17 09:00:18.000000000","message":"i don\u0027t think this is a backport candidate under normal stable branch rules as it introduces new functionality but does not fix any outstanding bug or security issue.\n\nas for limiting the scope i would tend to agree that changing the behavior of ConfigGuestInterface should likely be a separate patch. long term I think that is desirable or we should confirm with libvirt that they will use the vhost name for the driver for vhost-user interfaces in the future.","commit_id":"bbba28d7e5c6ad72abfc6e7b52f52014d1b45c9c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"cdc5bd89691cb8eb9220fe8c2903f672540ceb16","unresolved":false,"context_lines":[{"line_number":546,"context_line":""},{"line_number":547,"context_line":"    def _set_config_VIFVHostUser(self, instance, vif, conf, host\u003dNone):"},{"line_number":548,"context_line":"        rx_queue_size, tx_queue_size \u003d self._get_virtio_queue_sizes(host)"},{"line_number":549,"context_line":"        if rx_queue_size or tx_queue_size:"},{"line_number":550,"context_line":"            # TODO(sahid): Specifically override driver to use"},{"line_number":551,"context_line":"            # vhost-user. This can be removed when get_base_config"},{"line_number":552,"context_line":"            # will be fixed and rewrite to set the correct backend."},{"line_number":553,"context_line":"            conf.driver_name \u003d \"vhost-user\""},{"line_number":554,"context_line":""},{"line_number":555,"context_line":"        designer.set_vif_host_backend_vhostuser_config("},{"line_number":556,"context_line":"            conf, vif.mode, vif.path, rx_queue_size, tx_queue_size)"}],"source_content_type":"text/x-python","patch_set":18,"id":"9f6a8fd7_35746735","line":553,"range":{"start_line":549,"start_character":6,"end_line":553,"end_character":43},"updated":"2018-04-17 09:00:18.000000000","message":"ok im more ok with this. we are still using an unoffical value for the driver name but i think this is less likely to cause issues in the future so +1","commit_id":"6cd15a202b4be7ee4975a51722fb4f44430f986e"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"a4940ae86bb3d0aac171cbbc97706f616ae66e9d","unresolved":false,"context_lines":[{"line_number":546,"context_line":""},{"line_number":547,"context_line":"    def _set_config_VIFVHostUser(self, instance, vif, conf, host\u003dNone):"},{"line_number":548,"context_line":"        rx_queue_size, tx_queue_size \u003d self._get_virtio_queue_sizes(host)"},{"line_number":549,"context_line":"        if rx_queue_size or tx_queue_size:"},{"line_number":550,"context_line":"            # TODO(sahid): Specifically override driver to use"},{"line_number":551,"context_line":"            # vhost-user. This can be removed when get_base_config"},{"line_number":552,"context_line":"            # will be fixed and rewrite to set the correct backend."},{"line_number":553,"context_line":"            conf.driver_name \u003d \"vhost-user\""},{"line_number":554,"context_line":""},{"line_number":555,"context_line":"        designer.set_vif_host_backend_vhostuser_config("},{"line_number":556,"context_line":"            conf, vif.mode, vif.path, rx_queue_size, tx_queue_size)"}],"source_content_type":"text/x-python","patch_set":18,"id":"9f6a8fd7_86918b55","line":553,"range":{"start_line":549,"start_character":6,"end_line":553,"end_character":43},"in_reply_to":"9f6a8fd7_35746735","updated":"2018-04-17 10:40:21.000000000","message":"It\u0027s not unofficial it\u0027s really what is passed to QEMU:\n\n  -netdev vhost-user","commit_id":"6cd15a202b4be7ee4975a51722fb4f44430f986e"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"bed859e1480eae134e27dbcd8684986e45ceb23d","unresolved":false,"context_lines":[{"line_number":546,"context_line":""},{"line_number":547,"context_line":"    def _set_config_VIFVHostUser(self, instance, vif, conf, host\u003dNone):"},{"line_number":548,"context_line":"        rx_queue_size, tx_queue_size \u003d self._get_virtio_queue_sizes(host)"},{"line_number":549,"context_line":"        if rx_queue_size or tx_queue_size:"},{"line_number":550,"context_line":"            # TODO(sahid): Specifically override driver to use"},{"line_number":551,"context_line":"            # vhost-user. This can be removed when get_base_config"},{"line_number":552,"context_line":"            # will be fixed and rewrite to set the correct backend."},{"line_number":553,"context_line":"            conf.driver_name \u003d \"vhost-user\""},{"line_number":554,"context_line":""},{"line_number":555,"context_line":"        designer.set_vif_host_backend_vhostuser_config("},{"line_number":556,"context_line":"            conf, vif.mode, vif.path, rx_queue_size, tx_queue_size)"}],"source_content_type":"text/x-python","patch_set":18,"id":"9f6a8fd7_810ad523","line":553,"range":{"start_line":549,"start_character":6,"end_line":553,"end_character":43},"in_reply_to":"9f6a8fd7_86918b55","updated":"2018-04-17 11:33:49.000000000","message":"oh really thats good to know.\nhad assumed that was caluated for the interface type atribute in the xml.","commit_id":"6cd15a202b4be7ee4975a51722fb4f44430f986e"}],"releasenotes/notes/bp-libvirt-virtio-set-queue-sizes-6c54a2ce3dc30d18.yaml":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a566e21d6f0ee1905a8bdb3a9d796cf8524f61f8","unresolved":false,"context_lines":[{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    libvirt: add support for virtio-net rx/tx queue sizes"},{"line_number":5,"context_line":""},{"line_number":6,"context_line":"    Add support for configuring the `rx_queue_size` and"},{"line_number":7,"context_line":"    `tx_queue_size` options in the QEMU virtio-net driver by way of"},{"line_number":8,"context_line":"    nova.conf. Only supported for vhost/vhostuser interfaces"},{"line_number":9,"context_line":""}],"source_content_type":"text/x-yaml","patch_set":14,"id":"bf659307_7637c2ff","line":6,"range":{"start_line":6,"start_character":36,"end_line":6,"end_character":51},"updated":"2018-04-12 09:58:30.000000000","message":"``literal``","commit_id":"0957c45b5407d6a016f0de082de2d772c0582574"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"34e77a9d491dc21d1a094f1144312d0cbac49ffe","unresolved":false,"context_lines":[{"line_number":10,"context_line":"    Currently, valid values for the ring buffer sizes are 256, 512,"},{"line_number":11,"context_line":"    and 1024."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"    Adjustable RX queue sizes requires QEMU 2.7.0, and libvirt 2.3.0"},{"line_number":14,"context_line":"    (or newer) Adjustable TX queue sizes requires QEMU 2.10.0, and"},{"line_number":15,"context_line":"    libvirt 3.7.0 (or newer)"}],"source_content_type":"text/x-yaml","patch_set":17,"id":"9f6a8fd7_9ce1ea01","line":15,"range":{"start_line":13,"start_character":3,"end_line":15,"end_character":28},"updated":"2018-04-16 19:52:30.000000000","message":"i assume for vhost-user there is a minium dpdk version need too for this to work. do you know what that is? or what version of ovs-dpdk this would work with?","commit_id":"bbba28d7e5c6ad72abfc6e7b52f52014d1b45c9c"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"16b25c78218945db2dacebc91e8602ed7905db2a","unresolved":false,"context_lines":[{"line_number":10,"context_line":"    Currently, valid values for the ring buffer sizes are 256, 512,"},{"line_number":11,"context_line":"    and 1024."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"    Adjustable RX queue sizes requires QEMU 2.7.0, and libvirt 2.3.0"},{"line_number":14,"context_line":"    (or newer) Adjustable TX queue sizes requires QEMU 2.10.0, and"},{"line_number":15,"context_line":"    libvirt 3.7.0 (or newer)"}],"source_content_type":"text/x-yaml","patch_set":17,"id":"9f6a8fd7_723bd537","line":15,"range":{"start_line":13,"start_character":3,"end_line":15,"end_character":28},"in_reply_to":"9f6a8fd7_9ce1ea01","updated":"2018-04-17 08:27:34.000000000","message":"Internally we use OVS 2.9 and DPDK 17.11 But it\u0027s surely supported by earlier versions","commit_id":"bbba28d7e5c6ad72abfc6e7b52f52014d1b45c9c"}]}
