)]}'
{"nova/privsep/libvirt.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"f1266c624a0067a187b4bf252a1f6e092d2a433a","unresolved":false,"context_lines":[{"line_number":330,"context_line":""},{"line_number":331,"context_line":""},{"line_number":332,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"},{"line_number":333,"context_line":"def systemd_run_qb_mount(qb_vol, mnt_base, cfg_file\u003dNone):"},{"line_number":334,"context_line":"    \"\"\"Mount QB volume in separate CGROUP\"\"\""},{"line_number":335,"context_line":"    sysdr_cmd \u003d [\u0027systemd-run\u0027, \u0027--scope\u0027, \u0027mount.quobyte\u0027, \u0027--disable-xattrs\u0027,"},{"line_number":336,"context_line":"                 qb_vol, mnt_base]"}],"source_content_type":"text/x-python","patch_set":2,"id":"df7087c5_09759598","line":333,"range":{"start_line":333,"start_character":4,"end_line":333,"end_character":24},"updated":"2018-03-19 15:41:46.000000000","message":"Not sure if this is done or if it\u0027s overkill, but a unit test to ensure the expected CLI is invoked based on the args passed in?","commit_id":"5b1a0d16a2d0de1b648fbf771985db89faee09c8"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"db59691f3bd156ffa42850d4484d6cd5e3829eb6","unresolved":false,"context_lines":[{"line_number":330,"context_line":""},{"line_number":331,"context_line":""},{"line_number":332,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"},{"line_number":333,"context_line":"def systemd_run_qb_mount(qb_vol, mnt_base, cfg_file\u003dNone):"},{"line_number":334,"context_line":"    \"\"\"Mount QB volume in separate CGROUP\"\"\""},{"line_number":335,"context_line":"    sysdr_cmd \u003d [\u0027systemd-run\u0027, \u0027--scope\u0027, \u0027mount.quobyte\u0027, \u0027--disable-xattrs\u0027,"},{"line_number":336,"context_line":"                 qb_vol, mnt_base]"}],"source_content_type":"text/x-python","patch_set":2,"id":"df7087c5_ecb05ce0","line":333,"range":{"start_line":333,"start_character":4,"end_line":333,"end_character":24},"in_reply_to":"df7087c5_09759598","updated":"2018-03-20 08:34:00.000000000","message":"I thought about that, too, but there isn\u0027t even a test_libvirt.py file for all the calls in here. Furthermore as those calls mostly pass through hardcoded binary calls i guess unit tests were omitted so far. Other changes to this class do it similarily (e.g. https://review.openstack.org/#/c/507848/7) which makes me think it would be a bit over the top.\nStill, if somebody deems this necessary i\u0027ll add that.","commit_id":"5b1a0d16a2d0de1b648fbf771985db89faee09c8"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8af1e033080ec31647d9149c0fad3342ee33910a","unresolved":false,"context_lines":[{"line_number":336,"context_line":"                 qb_vol, mnt_base]"},{"line_number":337,"context_line":"    if cfg_file:"},{"line_number":338,"context_line":"        sysdr_cmd.extend([\u0027-c\u0027, cfg_file])"},{"line_number":339,"context_line":"    processutils.execute(sysdr_cmd)"}],"source_content_type":"text/x-python","patch_set":2,"id":"df7087c5_0f759598","line":339,"updated":"2018-03-19 16:47:13.000000000","message":"You should return the result here to make this a legit replacement for calling execute() directly (even though it\u0027s not being checked by the caller)","commit_id":"5b1a0d16a2d0de1b648fbf771985db89faee09c8"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"db59691f3bd156ffa42850d4484d6cd5e3829eb6","unresolved":false,"context_lines":[{"line_number":336,"context_line":"                 qb_vol, mnt_base]"},{"line_number":337,"context_line":"    if cfg_file:"},{"line_number":338,"context_line":"        sysdr_cmd.extend([\u0027-c\u0027, cfg_file])"},{"line_number":339,"context_line":"    processutils.execute(sysdr_cmd)"}],"source_content_type":"text/x-python","patch_set":2,"id":"df7087c5_cccb1870","line":339,"in_reply_to":"df7087c5_0f759598","updated":"2018-03-20 08:34:00.000000000","message":"Done","commit_id":"5b1a0d16a2d0de1b648fbf771985db89faee09c8"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"7c54eeaa977692b287ec2c0c7cfae3cdba5be32c","unresolved":false,"context_lines":[{"line_number":332,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"},{"line_number":333,"context_line":"def systemd_run_qb_mount(qb_vol, mnt_base, cfg_file\u003dNone):"},{"line_number":334,"context_line":"    \"\"\"Mount QB volume in separate CGROUP\"\"\""},{"line_number":335,"context_line":"    unprivileged_systemd_run_qb_mount(qb_vol, mnt_base, cfg_file)"},{"line_number":336,"context_line":""},{"line_number":337,"context_line":""},{"line_number":338,"context_line":"def unprivileged_systemd_run_qb_mount(qb_vol, mnt_base, cfg_file\u003dNone):"}],"source_content_type":"text/x-python","patch_set":4,"id":"df7087c5_b1463921","line":335,"range":{"start_line":335,"start_character":56,"end_line":335,"end_character":64},"updated":"2018-03-20 12:46:49.000000000","message":"kwargs should be named","commit_id":"60957737085b208b381dca6e3acaf3bb268eea17"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"f91f6bfdd6dc398d35836433354fe4bdfda94312","unresolved":false,"context_lines":[{"line_number":335,"context_line":"    unprivileged_systemd_run_qb_mount(qb_vol, mnt_base, cfg_file)"},{"line_number":336,"context_line":""},{"line_number":337,"context_line":""},{"line_number":338,"context_line":"def unprivileged_systemd_run_qb_mount(qb_vol, mnt_base, cfg_file\u003dNone):"},{"line_number":339,"context_line":"    \"\"\"Mount QB volume in separate CGROUP\"\"\""},{"line_number":340,"context_line":"    if cfg_file:"},{"line_number":341,"context_line":"        return processutils.execute(\u0027systemd-run\u0027, \u0027--scope\u0027, \u0027mount.quobyte\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf659307_fdf3189d","line":338,"range":{"start_line":338,"start_character":0,"end_line":338,"end_character":71},"updated":"2018-03-27 09:27:24.000000000","message":"nit: These normally have a comment above them indicating that they\u0027re purposefully undecorated","commit_id":"062dc094c6da00547d50be93df946234ef3c0731"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"0aaadcd9817a0e0a1c9dde98966cee85c67f6dd7","unresolved":false,"context_lines":[{"line_number":335,"context_line":"    unprivileged_systemd_run_qb_mount(qb_vol, mnt_base, cfg_file)"},{"line_number":336,"context_line":""},{"line_number":337,"context_line":""},{"line_number":338,"context_line":"def unprivileged_systemd_run_qb_mount(qb_vol, mnt_base, cfg_file\u003dNone):"},{"line_number":339,"context_line":"    \"\"\"Mount QB volume in separate CGROUP\"\"\""},{"line_number":340,"context_line":"    if cfg_file:"},{"line_number":341,"context_line":"        return processutils.execute(\u0027systemd-run\u0027, \u0027--scope\u0027, \u0027mount.quobyte\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf659307_009f835f","line":338,"range":{"start_line":338,"start_character":0,"end_line":338,"end_character":71},"in_reply_to":"bf659307_fdf3189d","updated":"2018-03-27 09:48:10.000000000","message":"Done","commit_id":"062dc094c6da00547d50be93df946234ef3c0731"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"f91f6bfdd6dc398d35836433354fe4bdfda94312","unresolved":false,"context_lines":[{"line_number":337,"context_line":""},{"line_number":338,"context_line":"def unprivileged_systemd_run_qb_mount(qb_vol, mnt_base, cfg_file\u003dNone):"},{"line_number":339,"context_line":"    \"\"\"Mount QB volume in separate CGROUP\"\"\""},{"line_number":340,"context_line":"    if cfg_file:"},{"line_number":341,"context_line":"        return processutils.execute(\u0027systemd-run\u0027, \u0027--scope\u0027, \u0027mount.quobyte\u0027,"},{"line_number":342,"context_line":"                                    \u0027--disable-xattrs\u0027, qb_vol, mnt_base,"},{"line_number":343,"context_line":"                                    \u0027-c\u0027, cfg_file)"},{"line_number":344,"context_line":"    return processutils.execute(\u0027systemd-run\u0027, \u0027--scope\u0027, \u0027mount.quobyte\u0027,"},{"line_number":345,"context_line":"                                \u0027--disable-xattrs\u0027, qb_vol, mnt_base)"},{"line_number":346,"context_line":""},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf659307_1dc12c2f","line":345,"range":{"start_line":340,"start_character":0,"end_line":345,"end_character":69},"updated":"2018-03-27 09:27:24.000000000","message":"nit:\n\n    cmd \u003d [\u0027systemd-run\u0027, \u0027--scope\u0027, ...]\n    if cfg_file:\n        cmd +\u003d [\u0027-c\u0027, cfg_file]\n    return processutils.execute(*cmd)\n\nSeems less duplicate-y.","commit_id":"062dc094c6da00547d50be93df946234ef3c0731"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"0aaadcd9817a0e0a1c9dde98966cee85c67f6dd7","unresolved":false,"context_lines":[{"line_number":337,"context_line":""},{"line_number":338,"context_line":"def unprivileged_systemd_run_qb_mount(qb_vol, mnt_base, cfg_file\u003dNone):"},{"line_number":339,"context_line":"    \"\"\"Mount QB volume in separate CGROUP\"\"\""},{"line_number":340,"context_line":"    if cfg_file:"},{"line_number":341,"context_line":"        return processutils.execute(\u0027systemd-run\u0027, \u0027--scope\u0027, \u0027mount.quobyte\u0027,"},{"line_number":342,"context_line":"                                    \u0027--disable-xattrs\u0027, qb_vol, mnt_base,"},{"line_number":343,"context_line":"                                    \u0027-c\u0027, cfg_file)"},{"line_number":344,"context_line":"    return processutils.execute(\u0027systemd-run\u0027, \u0027--scope\u0027, \u0027mount.quobyte\u0027,"},{"line_number":345,"context_line":"                                \u0027--disable-xattrs\u0027, qb_vol, mnt_base)"},{"line_number":346,"context_line":""},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf659307_209ac76d","line":345,"range":{"start_line":340,"start_character":0,"end_line":345,"end_character":69},"in_reply_to":"bf659307_1dc12c2f","updated":"2018-03-27 09:48:10.000000000","message":"Done","commit_id":"062dc094c6da00547d50be93df946234ef3c0731"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"f91f6bfdd6dc398d35836433354fe4bdfda94312","unresolved":false,"context_lines":[{"line_number":345,"context_line":"                                \u0027--disable-xattrs\u0027, qb_vol, mnt_base)"},{"line_number":346,"context_line":""},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"},{"line_number":349,"context_line":"def qb_mount(qb_vol, mnt_base, cfg_file\u003dNone):"},{"line_number":350,"context_line":"    \"\"\"Mount QB volume\"\"\""},{"line_number":351,"context_line":"    unprivileged_qb_mount(qb_vol, mnt_base, cfg_file)"},{"line_number":352,"context_line":""},{"line_number":353,"context_line":""},{"line_number":354,"context_line":"def unprivileged_qb_mount(qb_vol, mnt_base, cfg_file\u003dNone):"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf659307_dd1734a3","line":351,"range":{"start_line":348,"start_character":0,"end_line":351,"end_character":53},"updated":"2018-03-27 09:27:24.000000000","message":"This doesn\u0027t seem to be called anywhere? I know mikal wanted the unprivileged function to be placed in here (I disagree but eh), but if this straight up isn\u0027t called, it probably shouldn\u0027t be here","commit_id":"062dc094c6da00547d50be93df946234ef3c0731"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"96f5fa83dfdbd1282aba536fca1558aaa0c169a8","unresolved":false,"context_lines":[{"line_number":345,"context_line":"                                \u0027--disable-xattrs\u0027, qb_vol, mnt_base)"},{"line_number":346,"context_line":""},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"},{"line_number":349,"context_line":"def qb_mount(qb_vol, mnt_base, cfg_file\u003dNone):"},{"line_number":350,"context_line":"    \"\"\"Mount QB volume\"\"\""},{"line_number":351,"context_line":"    unprivileged_qb_mount(qb_vol, mnt_base, cfg_file)"},{"line_number":352,"context_line":""},{"line_number":353,"context_line":""},{"line_number":354,"context_line":"def unprivileged_qb_mount(qb_vol, mnt_base, cfg_file\u003dNone):"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf659307_5bfa4c75","line":351,"range":{"start_line":348,"start_character":0,"end_line":351,"end_character":53},"in_reply_to":"bf659307_00ad4387","updated":"2018-03-27 10:31:53.000000000","message":"Yeah, I\u0027ll hold off on further reviews til he get back to you with (hopefully) his very good reasons why he wants this","commit_id":"062dc094c6da00547d50be93df946234ef3c0731"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"0aaadcd9817a0e0a1c9dde98966cee85c67f6dd7","unresolved":false,"context_lines":[{"line_number":345,"context_line":"                                \u0027--disable-xattrs\u0027, qb_vol, mnt_base)"},{"line_number":346,"context_line":""},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"},{"line_number":349,"context_line":"def qb_mount(qb_vol, mnt_base, cfg_file\u003dNone):"},{"line_number":350,"context_line":"    \"\"\"Mount QB volume\"\"\""},{"line_number":351,"context_line":"    unprivileged_qb_mount(qb_vol, mnt_base, cfg_file)"},{"line_number":352,"context_line":""},{"line_number":353,"context_line":""},{"line_number":354,"context_line":"def unprivileged_qb_mount(qb_vol, mnt_base, cfg_file\u003dNone):"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf659307_00ad4387","line":351,"range":{"start_line":348,"start_character":0,"end_line":351,"end_character":53},"in_reply_to":"bf659307_dd1734a3","updated":"2018-03-27 09:48:10.000000000","message":"yeah, i agree, i added this blindly following mikals comments. I pinged him in irc and hope we can clarify this with him later on.","commit_id":"062dc094c6da00547d50be93df946234ef3c0731"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"ab458b34ccd1bacdf63f27c4213d2abd9368429f","unresolved":false,"context_lines":[{"line_number":333,"context_line":"def systemd_run_qb_mount(qb_vol, mnt_base, cfg_file\u003dNone):"},{"line_number":334,"context_line":"    \"\"\"Mount QB volume in separate CGROUP\"\"\""},{"line_number":335,"context_line":"    unprivileged_systemd_run_qb_mount(qb_vol, mnt_base, cfg_file)"},{"line_number":336,"context_line":""},{"line_number":337,"context_line":""},{"line_number":338,"context_line":"# NOTE(kaisers): this method is deliberately not wrapped in a privsep entryp."},{"line_number":339,"context_line":"def unprivileged_systemd_run_qb_mount(qb_vol, mnt_base, cfg_file\u003dNone):"},{"line_number":340,"context_line":"    \"\"\"Mount QB volume in separate CGROUP\"\"\""},{"line_number":341,"context_line":"    sysdr_cmd \u003d [\u0027systemd-run\u0027, \u0027--scope\u0027, \u0027mount.quobyte\u0027, \u0027--disable-xattrs\u0027,"},{"line_number":342,"context_line":"                 qb_vol, mnt_base]"},{"line_number":343,"context_line":"    if cfg_file:"},{"line_number":344,"context_line":"        sysdr_cmd.extend([\u0027-c\u0027, cfg_file])"},{"line_number":345,"context_line":"    return processutils.execute(*sysdr_cmd)"},{"line_number":346,"context_line":""},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"}],"source_content_type":"text/x-python","patch_set":11,"id":"5f7c97a3_e49ef204","line":345,"range":{"start_line":336,"start_character":0,"end_line":345,"end_character":43},"updated":"2018-07-03 13:33:50.000000000","message":"This has no callers so you should probably fold this back into the main function, right?","commit_id":"a2087ba132a497241a58c286950fcb50ad355a8e"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"0d4606242488c08142337b02221905ba8862caf4","unresolved":false,"context_lines":[{"line_number":333,"context_line":"def systemd_run_qb_mount(qb_vol, mnt_base, cfg_file\u003dNone):"},{"line_number":334,"context_line":"    \"\"\"Mount QB volume in separate CGROUP\"\"\""},{"line_number":335,"context_line":"    unprivileged_systemd_run_qb_mount(qb_vol, mnt_base, cfg_file)"},{"line_number":336,"context_line":""},{"line_number":337,"context_line":""},{"line_number":338,"context_line":"# NOTE(kaisers): this method is deliberately not wrapped in a privsep entryp."},{"line_number":339,"context_line":"def unprivileged_systemd_run_qb_mount(qb_vol, mnt_base, cfg_file\u003dNone):"},{"line_number":340,"context_line":"    \"\"\"Mount QB volume in separate CGROUP\"\"\""},{"line_number":341,"context_line":"    sysdr_cmd \u003d [\u0027systemd-run\u0027, \u0027--scope\u0027, \u0027mount.quobyte\u0027, \u0027--disable-xattrs\u0027,"},{"line_number":342,"context_line":"                 qb_vol, mnt_base]"},{"line_number":343,"context_line":"    if cfg_file:"},{"line_number":344,"context_line":"        sysdr_cmd.extend([\u0027-c\u0027, cfg_file])"},{"line_number":345,"context_line":"    return processutils.execute(*sysdr_cmd)"},{"line_number":346,"context_line":""},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"}],"source_content_type":"text/x-python","patch_set":11,"id":"5f7c97a3_91e16fff","line":345,"range":{"start_line":336,"start_character":0,"end_line":345,"end_character":43},"in_reply_to":"5f7c97a3_e49ef204","updated":"2018-07-04 09:10:46.000000000","message":"Done","commit_id":"a2087ba132a497241a58c286950fcb50ad355a8e"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"ab458b34ccd1bacdf63f27c4213d2abd9368429f","unresolved":false,"context_lines":[{"line_number":346,"context_line":""},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"},{"line_number":349,"context_line":"def qb_mount(qb_vol, mnt_base, cfg_file\u003dNone):"},{"line_number":350,"context_line":"    \"\"\"Mount QB volume\"\"\""},{"line_number":351,"context_line":"    unprivileged_qb_mount(qb_vol, mnt_base, cfg_file)"},{"line_number":352,"context_line":""},{"line_number":353,"context_line":""},{"line_number":354,"context_line":"# NOTE(kaisers): this method is deliberately not wrapped in a privsep entryp."}],"source_content_type":"text/x-python","patch_set":11,"id":"5f7c97a3_e4e9127f","line":351,"range":{"start_line":349,"start_character":0,"end_line":351,"end_character":53},"updated":"2018-07-03 13:33:50.000000000","message":"This has no callers, right? Can you remove, if so","commit_id":"a2087ba132a497241a58c286950fcb50ad355a8e"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"0d4606242488c08142337b02221905ba8862caf4","unresolved":false,"context_lines":[{"line_number":346,"context_line":""},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"},{"line_number":349,"context_line":"def qb_mount(qb_vol, mnt_base, cfg_file\u003dNone):"},{"line_number":350,"context_line":"    \"\"\"Mount QB volume\"\"\""},{"line_number":351,"context_line":"    unprivileged_qb_mount(qb_vol, mnt_base, cfg_file)"},{"line_number":352,"context_line":""},{"line_number":353,"context_line":""},{"line_number":354,"context_line":"# NOTE(kaisers): this method is deliberately not wrapped in a privsep entryp."}],"source_content_type":"text/x-python","patch_set":11,"id":"5f7c97a3_11bf9fd0","line":351,"range":{"start_line":349,"start_character":0,"end_line":351,"end_character":53},"in_reply_to":"5f7c97a3_e4e9127f","updated":"2018-07-04 09:10:46.000000000","message":"Done","commit_id":"a2087ba132a497241a58c286950fcb50ad355a8e"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"6457552ad3d32fb3b422efaa95ad6bec68ffb47d","unresolved":false,"context_lines":[{"line_number":267,"context_line":"    return processutils.execute(*sysdr_cmd)"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"# NOTE(kaisers): this method is deliberately not wrapped in a privsep entryp."},{"line_number":271,"context_line":"def unprivileged_qb_mount(qb_vol, mnt_base, cfg_file\u003dNone):"},{"line_number":272,"context_line":"    \"\"\"Mount QB volume\"\"\""},{"line_number":273,"context_line":"    mnt_cmd \u003d [\u0027mount.quobyte\u0027, \u0027--disable-xattrs\u0027, qb_vol, mnt_base]"},{"line_number":274,"context_line":"    if cfg_file:"},{"line_number":275,"context_line":"        mnt_cmd.extend([\u0027-c\u0027, cfg_file])"},{"line_number":276,"context_line":"    return processutils.execute(*mnt_cmd)"},{"line_number":277,"context_line":""},{"line_number":278,"context_line":""},{"line_number":279,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"}],"source_content_type":"text/x-python","patch_set":14,"id":"5f7c97a3_81f4a756","line":276,"range":{"start_line":270,"start_character":0,"end_line":276,"end_character":41},"updated":"2018-07-05 09:02:56.000000000","message":"Still not sure if this belongs here or not, given that it doesn\u0027t have a privileged wrapper, but I\u0027ll let someone else make that call","commit_id":"23b731d7480a95f6ba9812fd3cf6febea64a1139"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"cb26e6e3a26d80b05b1427c5dc65b7d8d6d85892","unresolved":false,"context_lines":[{"line_number":267,"context_line":"    return processutils.execute(*sysdr_cmd)"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"# NOTE(kaisers): this method is deliberately not wrapped in a privsep entryp."},{"line_number":271,"context_line":"def unprivileged_qb_mount(qb_vol, mnt_base, cfg_file\u003dNone):"},{"line_number":272,"context_line":"    \"\"\"Mount QB volume\"\"\""},{"line_number":273,"context_line":"    mnt_cmd \u003d [\u0027mount.quobyte\u0027, \u0027--disable-xattrs\u0027, qb_vol, mnt_base]"},{"line_number":274,"context_line":"    if cfg_file:"},{"line_number":275,"context_line":"        mnt_cmd.extend([\u0027-c\u0027, cfg_file])"},{"line_number":276,"context_line":"    return processutils.execute(*mnt_cmd)"},{"line_number":277,"context_line":""},{"line_number":278,"context_line":""},{"line_number":279,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"}],"source_content_type":"text/x-python","patch_set":14,"id":"5f7c97a3_c98b581b","line":276,"range":{"start_line":270,"start_character":0,"end_line":276,"end_character":41},"in_reply_to":"5f7c97a3_17ae58e3","updated":"2018-07-10 08:20:20.000000000","message":"interesting idea, would prevent having an unprivileged method sitting in here.","commit_id":"23b731d7480a95f6ba9812fd3cf6febea64a1139"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"61bce90e3d7753e6d18f83173196bd119815fca2","unresolved":false,"context_lines":[{"line_number":267,"context_line":"    return processutils.execute(*sysdr_cmd)"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"# NOTE(kaisers): this method is deliberately not wrapped in a privsep entryp."},{"line_number":271,"context_line":"def unprivileged_qb_mount(qb_vol, mnt_base, cfg_file\u003dNone):"},{"line_number":272,"context_line":"    \"\"\"Mount QB volume\"\"\""},{"line_number":273,"context_line":"    mnt_cmd \u003d [\u0027mount.quobyte\u0027, \u0027--disable-xattrs\u0027, qb_vol, mnt_base]"},{"line_number":274,"context_line":"    if cfg_file:"},{"line_number":275,"context_line":"        mnt_cmd.extend([\u0027-c\u0027, cfg_file])"},{"line_number":276,"context_line":"    return processutils.execute(*mnt_cmd)"},{"line_number":277,"context_line":""},{"line_number":278,"context_line":""},{"line_number":279,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"}],"source_content_type":"text/x-python","patch_set":14,"id":"5f7c97a3_e938e1d4","line":276,"range":{"start_line":270,"start_character":0,"end_line":276,"end_character":41},"in_reply_to":"5f7c97a3_81f4a756","updated":"2018-07-05 09:17:51.000000000","message":"I do see you point. My reason for keeping this here was to have these binary calls in one place and that this is not unlikely to become a privileged call in the future.","commit_id":"23b731d7480a95f6ba9812fd3cf6febea64a1139"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e3ffb5cdc104ab704689a0ea4db5ce863c560b68","unresolved":false,"context_lines":[{"line_number":267,"context_line":"    return processutils.execute(*sysdr_cmd)"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"# NOTE(kaisers): this method is deliberately not wrapped in a privsep entryp."},{"line_number":271,"context_line":"def unprivileged_qb_mount(qb_vol, mnt_base, cfg_file\u003dNone):"},{"line_number":272,"context_line":"    \"\"\"Mount QB volume\"\"\""},{"line_number":273,"context_line":"    mnt_cmd \u003d [\u0027mount.quobyte\u0027, \u0027--disable-xattrs\u0027, qb_vol, mnt_base]"},{"line_number":274,"context_line":"    if cfg_file:"},{"line_number":275,"context_line":"        mnt_cmd.extend([\u0027-c\u0027, cfg_file])"},{"line_number":276,"context_line":"    return processutils.execute(*mnt_cmd)"},{"line_number":277,"context_line":""},{"line_number":278,"context_line":""},{"line_number":279,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"}],"source_content_type":"text/x-python","patch_set":14,"id":"5f7c97a3_e8a08938","line":276,"range":{"start_line":270,"start_character":0,"end_line":276,"end_character":41},"in_reply_to":"5f7c97a3_c98b581b","updated":"2018-07-10 12:07:41.000000000","message":"Well, you would still have the unprivileged method in here, but it would be following the same pattern as its brethren in the fs and qemu modules. That is, the unprivileged method contains all of the logic and the privileged method is just a shim around it with the privsep decorator added.","commit_id":"23b731d7480a95f6ba9812fd3cf6febea64a1139"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"aa1cfbcaaaf8c0a1c9b31970cda6fbaa753eef9f","unresolved":false,"context_lines":[{"line_number":267,"context_line":"    return processutils.execute(*sysdr_cmd)"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"# NOTE(kaisers): this method is deliberately not wrapped in a privsep entryp."},{"line_number":271,"context_line":"def unprivileged_qb_mount(qb_vol, mnt_base, cfg_file\u003dNone):"},{"line_number":272,"context_line":"    \"\"\"Mount QB volume\"\"\""},{"line_number":273,"context_line":"    mnt_cmd \u003d [\u0027mount.quobyte\u0027, \u0027--disable-xattrs\u0027, qb_vol, mnt_base]"},{"line_number":274,"context_line":"    if cfg_file:"},{"line_number":275,"context_line":"        mnt_cmd.extend([\u0027-c\u0027, cfg_file])"},{"line_number":276,"context_line":"    return processutils.execute(*mnt_cmd)"},{"line_number":277,"context_line":""},{"line_number":278,"context_line":""},{"line_number":279,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"}],"source_content_type":"text/x-python","patch_set":14,"id":"5f7c97a3_ee109151","line":276,"range":{"start_line":270,"start_character":0,"end_line":276,"end_character":41},"in_reply_to":"5f7c97a3_e8a08938","updated":"2018-07-10 13:23:59.000000000","message":"Yes, i do know the pattern. I added that some iterations ago but was asked to remove it later on as some methods of the pattern weren\u0027t used and therefore superfluous.","commit_id":"23b731d7480a95f6ba9812fd3cf6febea64a1139"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"b8a2292e378dbdbed9668a19d4ae0698584aceed","unresolved":false,"context_lines":[{"line_number":267,"context_line":"    return processutils.execute(*sysdr_cmd)"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"# NOTE(kaisers): this method is deliberately not wrapped in a privsep entryp."},{"line_number":271,"context_line":"def unprivileged_qb_mount(qb_vol, mnt_base, cfg_file\u003dNone):"},{"line_number":272,"context_line":"    \"\"\"Mount QB volume\"\"\""},{"line_number":273,"context_line":"    mnt_cmd \u003d [\u0027mount.quobyte\u0027, \u0027--disable-xattrs\u0027, qb_vol, mnt_base]"},{"line_number":274,"context_line":"    if cfg_file:"},{"line_number":275,"context_line":"        mnt_cmd.extend([\u0027-c\u0027, cfg_file])"},{"line_number":276,"context_line":"    return processutils.execute(*mnt_cmd)"},{"line_number":277,"context_line":""},{"line_number":278,"context_line":""},{"line_number":279,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"}],"source_content_type":"text/x-python","patch_set":14,"id":"5f7c97a3_17ae58e3","line":276,"range":{"start_line":270,"start_character":0,"end_line":276,"end_character":41},"in_reply_to":"5f7c97a3_e938e1d4","updated":"2018-07-09 19:55:43.000000000","message":"This is the pattern followed by other privileged commands - see unprivileged_* in nova.privsep.fs and nova.privsep.qemu.\n\nHowever, in those cases the privileged method executes the unprivileged method, which isn\u0027t quite happening here.\n\nI wonder, would it be appropriate to change the unprivileged method to\n\n def unprivileged_qb_mount(qb_vol, mnt_base, cfg_file\u003dNone, use_systemd\u003dFalse):\n     mount_cmd \u003d [\u0027systemd-run\u0027, \u0027--scope\u0027] if use_systemd else []\n     mount_cmd.extend([\u0027mount.quobyte\u0027, \u0027--disable-xattrs\u0027, qb_vol, mnt_base])\n     ...\n\nAnd then have systemd_run_qb_mount just invoke unprivileged_qb_mount with use_systemd\u003dTrue\n\n?","commit_id":"23b731d7480a95f6ba9812fd3cf6febea64a1139"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"b8a2292e378dbdbed9668a19d4ae0698584aceed","unresolved":false,"context_lines":[{"line_number":278,"context_line":""},{"line_number":279,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"},{"line_number":280,"context_line":"def qb_umount(mnt_base):"},{"line_number":281,"context_line":"    \"\"\"Unmount QB volume\"\"\""},{"line_number":282,"context_line":"    umnt_cmd \u003d [\u0027umount.quobyte\u0027, mnt_base]"},{"line_number":283,"context_line":"    return processutils.execute(*umnt_cmd)"}],"source_content_type":"text/x-python","patch_set":14,"id":"5f7c97a3_174ad822","line":283,"range":{"start_line":281,"start_character":0,"end_line":283,"end_character":42},"updated":"2018-07-09 19:55:43.000000000","message":"So is there no way to unmount without being root?  That seems... asymmetrical.","commit_id":"23b731d7480a95f6ba9812fd3cf6febea64a1139"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"aa1cfbcaaaf8c0a1c9b31970cda6fbaa753eef9f","unresolved":false,"context_lines":[{"line_number":278,"context_line":""},{"line_number":279,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"},{"line_number":280,"context_line":"def qb_umount(mnt_base):"},{"line_number":281,"context_line":"    \"\"\"Unmount QB volume\"\"\""},{"line_number":282,"context_line":"    umnt_cmd \u003d [\u0027umount.quobyte\u0027, mnt_base]"},{"line_number":283,"context_line":"    return processutils.execute(*umnt_cmd)"}],"source_content_type":"text/x-python","patch_set":14,"id":"5f7c97a3_cef27569","line":283,"range":{"start_line":281,"start_character":0,"end_line":283,"end_character":42},"in_reply_to":"5f7c97a3_080ce53f","updated":"2018-07-10 13:23:59.000000000","message":"Done","commit_id":"23b731d7480a95f6ba9812fd3cf6febea64a1139"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"cb26e6e3a26d80b05b1427c5dc65b7d8d6d85892","unresolved":false,"context_lines":[{"line_number":278,"context_line":""},{"line_number":279,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"},{"line_number":280,"context_line":"def qb_umount(mnt_base):"},{"line_number":281,"context_line":"    \"\"\"Unmount QB volume\"\"\""},{"line_number":282,"context_line":"    umnt_cmd \u003d [\u0027umount.quobyte\u0027, mnt_base]"},{"line_number":283,"context_line":"    return processutils.execute(*umnt_cmd)"}],"source_content_type":"text/x-python","patch_set":14,"id":"5f7c97a3_a4d08354","line":283,"range":{"start_line":281,"start_character":0,"end_line":283,"end_character":42},"in_reply_to":"5f7c97a3_174ad822","updated":"2018-07-10 08:20:20.000000000","message":"That is true. The reasons are that the drivers umount method currently is extremely simple by running this as it umounts any variant mounted.\nUsing a symmetrical structure would require the umount method to have logic about which type of mount is active w/o significant advantages and to have the two methods in here. Nothing complex but the current version is dead simple which is why i kept it like that.","commit_id":"23b731d7480a95f6ba9812fd3cf6febea64a1139"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e3ffb5cdc104ab704689a0ea4db5ce863c560b68","unresolved":false,"context_lines":[{"line_number":278,"context_line":""},{"line_number":279,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"},{"line_number":280,"context_line":"def qb_umount(mnt_base):"},{"line_number":281,"context_line":"    \"\"\"Unmount QB volume\"\"\""},{"line_number":282,"context_line":"    umnt_cmd \u003d [\u0027umount.quobyte\u0027, mnt_base]"},{"line_number":283,"context_line":"    return processutils.execute(*umnt_cmd)"}],"source_content_type":"text/x-python","patch_set":14,"id":"5f7c97a3_080ce53f","line":283,"range":{"start_line":281,"start_character":0,"end_line":283,"end_character":42},"in_reply_to":"5f7c97a3_a4d08354","updated":"2018-07-10 12:07:41.000000000","message":"What I\u0027m trying to say is: What\u0027s the point in being able to mount as non-root at all if you\u0027re going to have to be root to unmount? I\u0027m sure there\u0027s a good explanation :) but if not, then perhaps we *only* need the systemd route above.","commit_id":"23b731d7480a95f6ba9812fd3cf6febea64a1139"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"54cbcf4af879e44715615f6e96c4525db4a797a7","unresolved":false,"context_lines":[{"line_number":267,"context_line":"    return processutils.execute(*sysdr_cmd)"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"# NOTE(kaisers): this method is deliberately not wrapped in a privsep entryp."},{"line_number":271,"context_line":"def unprivileged_qb_mount(qb_vol, mnt_base, cfg_file\u003dNone):"},{"line_number":272,"context_line":"    \"\"\"Mount QB volume\"\"\""},{"line_number":273,"context_line":"    mnt_cmd \u003d [\u0027mount.quobyte\u0027, \u0027--disable-xattrs\u0027, qb_vol, mnt_base]"},{"line_number":274,"context_line":"    if cfg_file:"},{"line_number":275,"context_line":"        mnt_cmd.extend([\u0027-c\u0027, cfg_file])"},{"line_number":276,"context_line":"    return processutils.execute(*mnt_cmd)"},{"line_number":277,"context_line":""},{"line_number":278,"context_line":""},{"line_number":279,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"}],"source_content_type":"text/x-python","patch_set":16,"id":"5f7c97a3_b94e6536","line":276,"range":{"start_line":270,"start_character":0,"end_line":276,"end_character":41},"updated":"2018-07-10 13:38:41.000000000","message":"Are we still deliberating on the suggestion from PS14?","commit_id":"63ec405bd7897a5c98e663690951d59ba80a01cd"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"d49b2f4969356e5402c0a64a9775844094e0a2fa","unresolved":false,"context_lines":[{"line_number":267,"context_line":"    return processutils.execute(*sysdr_cmd)"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"# NOTE(kaisers): this method is deliberately not wrapped in a privsep entryp."},{"line_number":271,"context_line":"def unprivileged_qb_mount(qb_vol, mnt_base, cfg_file\u003dNone):"},{"line_number":272,"context_line":"    \"\"\"Mount QB volume\"\"\""},{"line_number":273,"context_line":"    mnt_cmd \u003d [\u0027mount.quobyte\u0027, \u0027--disable-xattrs\u0027, qb_vol, mnt_base]"},{"line_number":274,"context_line":"    if cfg_file:"},{"line_number":275,"context_line":"        mnt_cmd.extend([\u0027-c\u0027, cfg_file])"},{"line_number":276,"context_line":"    return processutils.execute(*mnt_cmd)"},{"line_number":277,"context_line":""},{"line_number":278,"context_line":""},{"line_number":279,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"}],"source_content_type":"text/x-python","patch_set":16,"id":"5f7c97a3_d9932158","line":276,"range":{"start_line":270,"start_character":0,"end_line":276,"end_character":41},"in_reply_to":"5f7c97a3_b94e6536","updated":"2018-07-10 14:23:26.000000000","message":"Yup, I\u0027d like that pattern too, but let\u0027s do it in a follow-up","commit_id":"63ec405bd7897a5c98e663690951d59ba80a01cd"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"d8881699c69274007d4dbe6fa710416b828702ab","unresolved":false,"context_lines":[{"line_number":260,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"},{"line_number":261,"context_line":"def systemd_run_qb_mount(qb_vol, mnt_base, cfg_file\u003dNone):"},{"line_number":262,"context_line":"    \"\"\"Mount QB volume in separate CGROUP\"\"\""},{"line_number":263,"context_line":"    sysdr_cmd \u003d [\u0027systemd-run\u0027, \u0027--scope\u0027, \u0027mount.quobyte\u0027, \u0027--disable-xattrs\u0027,"},{"line_number":264,"context_line":"                 qb_vol, mnt_base]"},{"line_number":265,"context_line":"    if cfg_file:"},{"line_number":266,"context_line":"        sysdr_cmd.extend([\u0027-c\u0027, cfg_file])"},{"line_number":267,"context_line":"    return processutils.execute(*sysdr_cmd)"}],"source_content_type":"text/x-python","patch_set":28,"id":"5fc1f717_72c10954","line":264,"range":{"start_line":263,"start_character":0,"end_line":264,"end_character":34},"updated":"2019-03-27 10:49:26.000000000","message":"Please could you add a comment about the specific systemd bug which means we can\u0027t use --user. Given that the systemd bug has been fixed for some time this is a time-limited workaround, so we should highlight that here.","commit_id":"7231f7dee10fa8f9e6cead026f6a5ae3f5b15ae4"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"b36f76f1d1edba6877669bb5b1d60dd9dc08ab38","unresolved":false,"context_lines":[{"line_number":260,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"},{"line_number":261,"context_line":"def systemd_run_qb_mount(qb_vol, mnt_base, cfg_file\u003dNone):"},{"line_number":262,"context_line":"    \"\"\"Mount QB volume in separate CGROUP\"\"\""},{"line_number":263,"context_line":"    sysdr_cmd \u003d [\u0027systemd-run\u0027, \u0027--scope\u0027, \u0027mount.quobyte\u0027, \u0027--disable-xattrs\u0027,"},{"line_number":264,"context_line":"                 qb_vol, mnt_base]"},{"line_number":265,"context_line":"    if cfg_file:"},{"line_number":266,"context_line":"        sysdr_cmd.extend([\u0027-c\u0027, cfg_file])"},{"line_number":267,"context_line":"    return processutils.execute(*sysdr_cmd)"}],"source_content_type":"text/x-python","patch_set":28,"id":"5fc1f717_6515b33c","line":264,"range":{"start_line":263,"start_character":0,"end_line":264,"end_character":34},"in_reply_to":"5fc1f717_2357b028","updated":"2019-03-27 20:37:19.000000000","message":"We don\u0027t have unit tests for this method.\n\nSee [1] for a mechanism that allows you to test the guts of a privsepified method. (Feel free to peel out that fixture and propose it in your patch.)\n\n[1] https://review.openstack.org/647849","commit_id":"7231f7dee10fa8f9e6cead026f6a5ae3f5b15ae4"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"a39987bdb94ae0d989cefdca9e758b607d01ab0c","unresolved":false,"context_lines":[{"line_number":260,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"},{"line_number":261,"context_line":"def systemd_run_qb_mount(qb_vol, mnt_base, cfg_file\u003dNone):"},{"line_number":262,"context_line":"    \"\"\"Mount QB volume in separate CGROUP\"\"\""},{"line_number":263,"context_line":"    sysdr_cmd \u003d [\u0027systemd-run\u0027, \u0027--scope\u0027, \u0027mount.quobyte\u0027, \u0027--disable-xattrs\u0027,"},{"line_number":264,"context_line":"                 qb_vol, mnt_base]"},{"line_number":265,"context_line":"    if cfg_file:"},{"line_number":266,"context_line":"        sysdr_cmd.extend([\u0027-c\u0027, cfg_file])"},{"line_number":267,"context_line":"    return processutils.execute(*sysdr_cmd)"}],"source_content_type":"text/x-python","patch_set":28,"id":"5fc1f717_22d31803","line":264,"range":{"start_line":263,"start_character":0,"end_line":264,"end_character":34},"in_reply_to":"5fc1f717_6515b33c","updated":"2019-03-28 08:28:58.000000000","message":"Ok, will do that in the follow up change","commit_id":"7231f7dee10fa8f9e6cead026f6a5ae3f5b15ae4"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"b7c97a5e3c4f31505e66faee85806678c3211d8a","unresolved":false,"context_lines":[{"line_number":260,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"},{"line_number":261,"context_line":"def systemd_run_qb_mount(qb_vol, mnt_base, cfg_file\u003dNone):"},{"line_number":262,"context_line":"    \"\"\"Mount QB volume in separate CGROUP\"\"\""},{"line_number":263,"context_line":"    sysdr_cmd \u003d [\u0027systemd-run\u0027, \u0027--scope\u0027, \u0027mount.quobyte\u0027, \u0027--disable-xattrs\u0027,"},{"line_number":264,"context_line":"                 qb_vol, mnt_base]"},{"line_number":265,"context_line":"    if cfg_file:"},{"line_number":266,"context_line":"        sysdr_cmd.extend([\u0027-c\u0027, cfg_file])"},{"line_number":267,"context_line":"    return processutils.execute(*sysdr_cmd)"}],"source_content_type":"text/x-python","patch_set":28,"id":"5fc1f717_c87fea12","line":264,"range":{"start_line":263,"start_character":0,"end_line":264,"end_character":34},"in_reply_to":"5fc1f717_72c10954","updated":"2019-03-27 11:29:31.000000000","message":"Yes, I\u0027ll add that.","commit_id":"7231f7dee10fa8f9e6cead026f6a5ae3f5b15ae4"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"2478209047ed4250e1e17e73f63a6494affc7dea","unresolved":false,"context_lines":[{"line_number":260,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"},{"line_number":261,"context_line":"def systemd_run_qb_mount(qb_vol, mnt_base, cfg_file\u003dNone):"},{"line_number":262,"context_line":"    \"\"\"Mount QB volume in separate CGROUP\"\"\""},{"line_number":263,"context_line":"    sysdr_cmd \u003d [\u0027systemd-run\u0027, \u0027--scope\u0027, \u0027mount.quobyte\u0027, \u0027--disable-xattrs\u0027,"},{"line_number":264,"context_line":"                 qb_vol, mnt_base]"},{"line_number":265,"context_line":"    if cfg_file:"},{"line_number":266,"context_line":"        sysdr_cmd.extend([\u0027-c\u0027, cfg_file])"},{"line_number":267,"context_line":"    return processutils.execute(*sysdr_cmd)"}],"source_content_type":"text/x-python","patch_set":28,"id":"5fc1f717_2357b028","line":264,"range":{"start_line":263,"start_character":0,"end_line":264,"end_character":34},"in_reply_to":"5fc1f717_c87fea12","updated":"2019-03-27 13:37:02.000000000","message":"Added a comment referring to the issues with --user in the follow up change.\n\nHowever i\u0027m not sure how temporal this issue is. While the systemd bug referenced in there is gone in newish releases i currently do not know of workarounds/fixes regarding the processutils related issue.","commit_id":"7231f7dee10fa8f9e6cead026f6a5ae3f5b15ae4"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"d8881699c69274007d4dbe6fa710416b828702ab","unresolved":false,"context_lines":[{"line_number":276,"context_line":"    return processutils.execute(*mnt_cmd)"},{"line_number":277,"context_line":""},{"line_number":278,"context_line":""},{"line_number":279,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"},{"line_number":280,"context_line":"def qb_umount(mnt_base):"},{"line_number":281,"context_line":"    \"\"\"Unmount QB volume\"\"\""},{"line_number":282,"context_line":"    unprivileged_qb_umount(mnt_base)"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"# NOTE(kaisers): this method is deliberately not wrapped in a privsep entryp."},{"line_number":286,"context_line":"def unprivileged_qb_umount(mnt_base):"},{"line_number":287,"context_line":"    \"\"\"Unmount QB volume\"\"\""},{"line_number":288,"context_line":"    umnt_cmd \u003d [\u0027umount.quobyte\u0027, mnt_base]"},{"line_number":289,"context_line":"    return processutils.execute(*umnt_cmd)"}],"source_content_type":"text/x-python","patch_set":28,"id":"5fc1f717_17151749","line":289,"range":{"start_line":279,"start_character":0,"end_line":289,"end_character":42},"updated":"2019-03-27 10:49:26.000000000","message":"I\u0027m pretty sure these don\u0027t need to be quobyte specific. Running \u0027umount\u0027 will do the right thing here. Specifically it will run umount.quobyte for a quobyte mount.","commit_id":"7231f7dee10fa8f9e6cead026f6a5ae3f5b15ae4"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"b7c97a5e3c4f31505e66faee85806678c3211d8a","unresolved":false,"context_lines":[{"line_number":276,"context_line":"    return processutils.execute(*mnt_cmd)"},{"line_number":277,"context_line":""},{"line_number":278,"context_line":""},{"line_number":279,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"},{"line_number":280,"context_line":"def qb_umount(mnt_base):"},{"line_number":281,"context_line":"    \"\"\"Unmount QB volume\"\"\""},{"line_number":282,"context_line":"    unprivileged_qb_umount(mnt_base)"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"# NOTE(kaisers): this method is deliberately not wrapped in a privsep entryp."},{"line_number":286,"context_line":"def unprivileged_qb_umount(mnt_base):"},{"line_number":287,"context_line":"    \"\"\"Unmount QB volume\"\"\""},{"line_number":288,"context_line":"    umnt_cmd \u003d [\u0027umount.quobyte\u0027, mnt_base]"},{"line_number":289,"context_line":"    return processutils.execute(*umnt_cmd)"}],"source_content_type":"text/x-python","patch_set":28,"id":"5fc1f717_a8705e41","line":289,"range":{"start_line":279,"start_character":0,"end_line":289,"end_character":42},"in_reply_to":"5fc1f717_17151749","updated":"2019-03-27 11:29:31.000000000","message":"ok, I\u0027ll generalize.","commit_id":"7231f7dee10fa8f9e6cead026f6a5ae3f5b15ae4"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"2478209047ed4250e1e17e73f63a6494affc7dea","unresolved":false,"context_lines":[{"line_number":276,"context_line":"    return processutils.execute(*mnt_cmd)"},{"line_number":277,"context_line":""},{"line_number":278,"context_line":""},{"line_number":279,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"},{"line_number":280,"context_line":"def qb_umount(mnt_base):"},{"line_number":281,"context_line":"    \"\"\"Unmount QB volume\"\"\""},{"line_number":282,"context_line":"    unprivileged_qb_umount(mnt_base)"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"# NOTE(kaisers): this method is deliberately not wrapped in a privsep entryp."},{"line_number":286,"context_line":"def unprivileged_qb_umount(mnt_base):"},{"line_number":287,"context_line":"    \"\"\"Unmount QB volume\"\"\""},{"line_number":288,"context_line":"    umnt_cmd \u003d [\u0027umount.quobyte\u0027, mnt_base]"},{"line_number":289,"context_line":"    return processutils.execute(*umnt_cmd)"}],"source_content_type":"text/x-python","patch_set":28,"id":"5fc1f717_031f146c","line":289,"range":{"start_line":279,"start_character":0,"end_line":289,"end_character":42},"in_reply_to":"5fc1f717_a8705e41","updated":"2019-03-27 13:37:02.000000000","message":"Done","commit_id":"7231f7dee10fa8f9e6cead026f6a5ae3f5b15ae4"}],"nova/tests/unit/virt/libvirt/volume/test_quobyte.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"f1266c624a0067a187b4bf252a1f6e092d2a433a","unresolved":false,"context_lines":[{"line_number":96,"context_line":""},{"line_number":97,"context_line":"        mock_ensure_tree.assert_called_once_with(export_mnt_base)"},{"line_number":98,"context_line":"        mock_exists.assert_called_once_with(\"/run/systemd/system\")"},{"line_number":99,"context_line":"        mock_privsep_sysdr.assert_called_once_with(quobyte_volume,"},{"line_number":100,"context_line":"                                                   export_mnt_base, None)"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    @mock.patch.object(os.path, \"exists\", return_value\u003dFalse)"},{"line_number":103,"context_line":"    @mock.patch.object(fileutils, \"ensure_tree\")"}],"source_content_type":"text/x-python","patch_set":2,"id":"df7087c5_a937c9e3","line":100,"range":{"start_line":99,"start_character":0,"end_line":100,"end_character":73},"updated":"2018-03-19 15:41:46.000000000","message":"Oh, and what about another test case that passes in the config file?","commit_id":"5b1a0d16a2d0de1b648fbf771985db89faee09c8"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"db59691f3bd156ffa42850d4484d6cd5e3829eb6","unresolved":false,"context_lines":[{"line_number":96,"context_line":""},{"line_number":97,"context_line":"        mock_ensure_tree.assert_called_once_with(export_mnt_base)"},{"line_number":98,"context_line":"        mock_exists.assert_called_once_with(\"/run/systemd/system\")"},{"line_number":99,"context_line":"        mock_privsep_sysdr.assert_called_once_with(quobyte_volume,"},{"line_number":100,"context_line":"                                                   export_mnt_base, None)"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    @mock.patch.object(os.path, \"exists\", return_value\u003dFalse)"},{"line_number":103,"context_line":"    @mock.patch.object(fileutils, \"ensure_tree\")"}],"source_content_type":"text/x-python","patch_set":2,"id":"df7087c5_6cef6ce9","line":100,"range":{"start_line":99,"start_character":0,"end_line":100,"end_character":73},"in_reply_to":"df7087c5_a937c9e3","updated":"2018-03-20 08:34:00.000000000","message":"Done","commit_id":"5b1a0d16a2d0de1b648fbf771985db89faee09c8"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"155f66da4d507f15d23506e117f7fade32c4af66","unresolved":false,"context_lines":[{"line_number":17,"context_line":"import os"},{"line_number":18,"context_line":"import traceback"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"from ddt import data"},{"line_number":21,"context_line":"from ddt import ddt"},{"line_number":22,"context_line":"import mock"},{"line_number":23,"context_line":"from oslo_concurrency import processutils"},{"line_number":24,"context_line":"from oslo_utils import fileutils"}],"source_content_type":"text/x-python","patch_set":25,"id":"9fdfeff1_76ad349a","line":21,"range":{"start_line":20,"start_character":0,"end_line":21,"end_character":19},"updated":"2019-02-19 14:17:14.000000000","message":"nit: I think you should just use \u0027import ddt\u0027 as these are objects [1]\n\n[1] https://docs.openstack.org/hacking/latest/user/hacking.html#imports","commit_id":"7da2e44bb608e93925ce43b173d01cdb46453aa9"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"82ac8f52f7f680ec4387d5995df3c5d383177180","unresolved":false,"context_lines":[{"line_number":17,"context_line":"import os"},{"line_number":18,"context_line":"import traceback"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"from ddt import data"},{"line_number":21,"context_line":"from ddt import ddt"},{"line_number":22,"context_line":"import mock"},{"line_number":23,"context_line":"from oslo_concurrency import processutils"},{"line_number":24,"context_line":"from oslo_utils import fileutils"}],"source_content_type":"text/x-python","patch_set":25,"id":"9fdfeff1_aa9b3064","line":21,"range":{"start_line":20,"start_character":0,"end_line":21,"end_character":19},"in_reply_to":"9fdfeff1_76ad349a","updated":"2019-02-19 14:52:52.000000000","message":"Done","commit_id":"7da2e44bb608e93925ce43b173d01cdb46453aa9"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"155f66da4d507f15d23506e117f7fade32c4af66","unresolved":false,"context_lines":[{"line_number":69,"context_line":"                                  state\u003d\"stopping\"):"},{"line_number":70,"context_line":"        mock_execute.return_value \u003d [state, \"fake stderr\"]"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        self.assertIs(True, quobyte.is_systemd())"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"        mock_exists.assert_called_once_with(quobyte.SYSTEMCTL_PATH)"},{"line_number":75,"context_line":"        mock_execute.assert_called_once_with(quobyte.SYSTEMCTL_PATH,"}],"source_content_type":"text/x-python","patch_set":25,"id":"9fdfeff1_76b454d2","line":72,"range":{"start_line":72,"start_character":0,"end_line":72,"end_character":49},"updated":"2019-02-19 14:17:14.000000000","message":"Doesn\u0027t this assume you\u0027re running on a system with PID1 \u003d\u003d systemd now?","commit_id":"7da2e44bb608e93925ce43b173d01cdb46453aa9"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"82ac8f52f7f680ec4387d5995df3c5d383177180","unresolved":false,"context_lines":[{"line_number":69,"context_line":"                                  state\u003d\"stopping\"):"},{"line_number":70,"context_line":"        mock_execute.return_value \u003d [state, \"fake stderr\"]"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        self.assertIs(True, quobyte.is_systemd())"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"        mock_exists.assert_called_once_with(quobyte.SYSTEMCTL_PATH)"},{"line_number":75,"context_line":"        mock_execute.assert_called_once_with(quobyte.SYSTEMCTL_PATH,"}],"source_content_type":"text/x-python","patch_set":25,"id":"9fdfeff1_4aa08492","line":72,"range":{"start_line":72,"start_character":0,"end_line":72,"end_character":49},"in_reply_to":"9fdfeff1_76b454d2","updated":"2019-02-19 14:52:52.000000000","message":"Done","commit_id":"7da2e44bb608e93925ce43b173d01cdb46453aa9"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"155f66da4d507f15d23506e117f7fade32c4af66","unresolved":false,"context_lines":[{"line_number":63,"context_line":"        mypart.mountpoint \u003d self.TEST_MNT_POINT"},{"line_number":64,"context_line":"        return [mypart]"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    @mock.patch.object(processutils, \"execute\")"},{"line_number":67,"context_line":"    @mock.patch.object(os.path, \"exists\", return_value\u003dTrue)"},{"line_number":68,"context_line":"    def run_quobyte_is_systemd_ok(self, mock_exists, mock_execute,"},{"line_number":69,"context_line":"                                  state\u003d\"stopping\"):"},{"line_number":70,"context_line":"        mock_execute.return_value \u003d [state, \"fake stderr\"]"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        self.assertIs(True, quobyte.is_systemd())"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"        mock_exists.assert_called_once_with(quobyte.SYSTEMCTL_PATH)"},{"line_number":75,"context_line":"        mock_execute.assert_called_once_with(quobyte.SYSTEMCTL_PATH,"},{"line_number":76,"context_line":"                                             \"is-system-running\","},{"line_number":77,"context_line":"                                             check_exit_code\u003d[0, 1])"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"    @data(\"starting\", \"running\", \"degraded\")"},{"line_number":80,"context_line":"    def test_quobyte_is_systemd_ok(self, value):"}],"source_content_type":"text/x-python","patch_set":25,"id":"9fdfeff1_769bf465","line":77,"range":{"start_line":66,"start_character":0,"end_line":77,"end_character":68},"updated":"2019-02-19 14:17:14.000000000","message":"nit: Not sure why this can\u0027t be folded into \u0027test_quobyte_is_systemd_ok\u0027 given that it\u0027s only got one caller (or nested, if \u0027@ddt.data\u0027 doesn\u0027t play nice with \u0027@mock.patch.object\u0027) but OK","commit_id":"7da2e44bb608e93925ce43b173d01cdb46453aa9"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"82ac8f52f7f680ec4387d5995df3c5d383177180","unresolved":false,"context_lines":[{"line_number":63,"context_line":"        mypart.mountpoint \u003d self.TEST_MNT_POINT"},{"line_number":64,"context_line":"        return [mypart]"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    @mock.patch.object(processutils, \"execute\")"},{"line_number":67,"context_line":"    @mock.patch.object(os.path, \"exists\", return_value\u003dTrue)"},{"line_number":68,"context_line":"    def run_quobyte_is_systemd_ok(self, mock_exists, mock_execute,"},{"line_number":69,"context_line":"                                  state\u003d\"stopping\"):"},{"line_number":70,"context_line":"        mock_execute.return_value \u003d [state, \"fake stderr\"]"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        self.assertIs(True, quobyte.is_systemd())"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"        mock_exists.assert_called_once_with(quobyte.SYSTEMCTL_PATH)"},{"line_number":75,"context_line":"        mock_execute.assert_called_once_with(quobyte.SYSTEMCTL_PATH,"},{"line_number":76,"context_line":"                                             \"is-system-running\","},{"line_number":77,"context_line":"                                             check_exit_code\u003d[0, 1])"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"    @data(\"starting\", \"running\", \"degraded\")"},{"line_number":80,"context_line":"    def test_quobyte_is_systemd_ok(self, value):"}],"source_content_type":"text/x-python","patch_set":25,"id":"9fdfeff1_0aaa7c70","line":77,"range":{"start_line":66,"start_character":0,"end_line":77,"end_character":68},"in_reply_to":"9fdfeff1_769bf465","updated":"2019-02-19 14:52:52.000000000","message":"Done","commit_id":"7da2e44bb608e93925ce43b173d01cdb46453aa9"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"155f66da4d507f15d23506e117f7fade32c4af66","unresolved":false,"context_lines":[{"line_number":153,"context_line":"                                                   export_mnt_base,"},{"line_number":154,"context_line":"                                                   cfg_file\u003dcfg_file_path)"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"    @mock.patch.object(os.path, \"exists\", return_value\u003dFalse)"},{"line_number":157,"context_line":"    @mock.patch.object(fileutils, \"ensure_tree\")"},{"line_number":158,"context_line":"    @mock.patch.object(libvirt, \u0027unprivileged_qb_mount\u0027)"},{"line_number":159,"context_line":"    def test_quobyte_mount_volume_with_config(self,"}],"source_content_type":"text/x-python","patch_set":25,"id":"9fdfeff1_560b507c","line":156,"range":{"start_line":156,"start_character":0,"end_line":156,"end_character":61},"updated":"2019-02-19 14:17:14.000000000","message":"This would be clearer and more unit test\u0027y if you switched to mocking out \u0027is_systemd\u0027 instead. As things stand, you\u0027re really testing two functions here (mount_volume, is_systemd)","commit_id":"7da2e44bb608e93925ce43b173d01cdb46453aa9"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"82ac8f52f7f680ec4387d5995df3c5d383177180","unresolved":false,"context_lines":[{"line_number":153,"context_line":"                                                   export_mnt_base,"},{"line_number":154,"context_line":"                                                   cfg_file\u003dcfg_file_path)"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"    @mock.patch.object(os.path, \"exists\", return_value\u003dFalse)"},{"line_number":157,"context_line":"    @mock.patch.object(fileutils, \"ensure_tree\")"},{"line_number":158,"context_line":"    @mock.patch.object(libvirt, \u0027unprivileged_qb_mount\u0027)"},{"line_number":159,"context_line":"    def test_quobyte_mount_volume_with_config(self,"}],"source_content_type":"text/x-python","patch_set":25,"id":"9fdfeff1_0a1fdcb7","line":156,"range":{"start_line":156,"start_character":0,"end_line":156,"end_character":61},"in_reply_to":"9fdfeff1_560b507c","updated":"2019-02-19 14:52:52.000000000","message":"Done","commit_id":"7da2e44bb608e93925ce43b173d01cdb46453aa9"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"155f66da4d507f15d23506e117f7fade32c4af66","unresolved":false,"context_lines":[{"line_number":178,"context_line":"        mock_mount.assert_has_calls(expected_commands)"},{"line_number":179,"context_line":"        mock_exists.assert_called_once_with(\u0027/usr/bin/systemctl\u0027)"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"    @mock.patch.object(os.path, \"exists\", return_value\u003dFalse)"},{"line_number":182,"context_line":"    @mock.patch.object(fileutils, \"ensure_tree\")"},{"line_number":183,"context_line":"    @mock.patch.object(libvirt, \u0027unprivileged_qb_mount\u0027,"},{"line_number":184,"context_line":"                       side_effect\u003d(processutils."}],"source_content_type":"text/x-python","patch_set":25,"id":"9fdfeff1_1625c80a","line":181,"range":{"start_line":181,"start_character":4,"end_line":181,"end_character":61},"updated":"2019-02-19 14:17:14.000000000","message":"As above","commit_id":"7da2e44bb608e93925ce43b173d01cdb46453aa9"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"82ac8f52f7f680ec4387d5995df3c5d383177180","unresolved":false,"context_lines":[{"line_number":178,"context_line":"        mock_mount.assert_has_calls(expected_commands)"},{"line_number":179,"context_line":"        mock_exists.assert_called_once_with(\u0027/usr/bin/systemctl\u0027)"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"    @mock.patch.object(os.path, \"exists\", return_value\u003dFalse)"},{"line_number":182,"context_line":"    @mock.patch.object(fileutils, \"ensure_tree\")"},{"line_number":183,"context_line":"    @mock.patch.object(libvirt, \u0027unprivileged_qb_mount\u0027,"},{"line_number":184,"context_line":"                       side_effect\u003d(processutils."}],"source_content_type":"text/x-python","patch_set":25,"id":"9fdfeff1_2a1c20c0","line":181,"range":{"start_line":181,"start_character":4,"end_line":181,"end_character":61},"in_reply_to":"9fdfeff1_1625c80a","updated":"2019-02-19 14:52:52.000000000","message":"Done","commit_id":"7da2e44bb608e93925ce43b173d01cdb46453aa9"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"155f66da4d507f15d23506e117f7fade32c4af66","unresolved":false,"context_lines":[{"line_number":197,"context_line":"        mock_ensure_tree.assert_called_once_with(export_mnt_base)"},{"line_number":198,"context_line":"        mock_path_exists.assert_called_once_with(\u0027/usr/bin/systemctl\u0027)"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    @mock.patch.object(os.path, \"exists\", return_value\u003dFalse)"},{"line_number":201,"context_line":"    @mock.patch.object(libvirt, \u0027unprivileged_qb_umount\u0027)"},{"line_number":202,"context_line":"    def test_quobyte_umount_volume(self, mock_lv_umount, mock_exists):"},{"line_number":203,"context_line":"        mnt_base \u003d \u0027/mnt\u0027"}],"source_content_type":"text/x-python","patch_set":25,"id":"9fdfeff1_d61e60ba","line":200,"range":{"start_line":200,"start_character":4,"end_line":200,"end_character":61},"updated":"2019-02-19 14:17:14.000000000","message":"and again here","commit_id":"7da2e44bb608e93925ce43b173d01cdb46453aa9"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"82ac8f52f7f680ec4387d5995df3c5d383177180","unresolved":false,"context_lines":[{"line_number":197,"context_line":"        mock_ensure_tree.assert_called_once_with(export_mnt_base)"},{"line_number":198,"context_line":"        mock_path_exists.assert_called_once_with(\u0027/usr/bin/systemctl\u0027)"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    @mock.patch.object(os.path, \"exists\", return_value\u003dFalse)"},{"line_number":201,"context_line":"    @mock.patch.object(libvirt, \u0027unprivileged_qb_umount\u0027)"},{"line_number":202,"context_line":"    def test_quobyte_umount_volume(self, mock_lv_umount, mock_exists):"},{"line_number":203,"context_line":"        mnt_base \u003d \u0027/mnt\u0027"}],"source_content_type":"text/x-python","patch_set":25,"id":"9fdfeff1_0a4d9cc3","line":200,"range":{"start_line":200,"start_character":4,"end_line":200,"end_character":61},"in_reply_to":"9fdfeff1_d61e60ba","updated":"2019-02-19 14:52:52.000000000","message":"Done","commit_id":"7da2e44bb608e93925ce43b173d01cdb46453aa9"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"155f66da4d507f15d23506e117f7fade32c4af66","unresolved":false,"context_lines":[{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    @mock.patch.object(os.path, \"exists\", return_value\u003dFalse)"},{"line_number":201,"context_line":"    @mock.patch.object(libvirt, \u0027unprivileged_qb_umount\u0027)"},{"line_number":202,"context_line":"    def test_quobyte_umount_volume(self, mock_lv_umount, mock_exists):"},{"line_number":203,"context_line":"        mnt_base \u003d \u0027/mnt\u0027"},{"line_number":204,"context_line":"        quobyte_volume \u003d \u0027192.168.1.1/volume-00001\u0027"},{"line_number":205,"context_line":"        export_mnt_base \u003d os.path.join(mnt_base,"}],"source_content_type":"text/x-python","patch_set":25,"id":"9fdfeff1_b67b9cf2","line":202,"updated":"2019-02-19 14:17:14.000000000","message":"..._volume_non_sysd ?","commit_id":"7da2e44bb608e93925ce43b173d01cdb46453aa9"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"82ac8f52f7f680ec4387d5995df3c5d383177180","unresolved":false,"context_lines":[{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    @mock.patch.object(os.path, \"exists\", return_value\u003dFalse)"},{"line_number":201,"context_line":"    @mock.patch.object(libvirt, \u0027unprivileged_qb_umount\u0027)"},{"line_number":202,"context_line":"    def test_quobyte_umount_volume(self, mock_lv_umount, mock_exists):"},{"line_number":203,"context_line":"        mnt_base \u003d \u0027/mnt\u0027"},{"line_number":204,"context_line":"        quobyte_volume \u003d \u0027192.168.1.1/volume-00001\u0027"},{"line_number":205,"context_line":"        export_mnt_base \u003d os.path.join(mnt_base,"}],"source_content_type":"text/x-python","patch_set":25,"id":"9fdfeff1_2a4ae0ab","line":202,"in_reply_to":"9fdfeff1_b67b9cf2","updated":"2019-02-19 14:52:52.000000000","message":"Done","commit_id":"7da2e44bb608e93925ce43b173d01cdb46453aa9"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"cfeff589e8b5f3ad0c83c401fdc53455bd73e5b9","unresolved":false,"context_lines":[{"line_number":70,"context_line":"                                   mock_exists, mock_execute, mock_proc):"},{"line_number":71,"context_line":"        mock_execute.return_value \u003d [value, \"fake stderr\"]"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"        self.assertIs(True, quobyte.is_systemd())"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"        mock_exists.assert_called_once_with(quobyte.SYSTEMCTL_PATH)"},{"line_number":76,"context_line":"        mock_execute.assert_called_once_with(quobyte.SYSTEMCTL_PATH,"}],"source_content_type":"text/x-python","patch_set":26,"id":"9fdfeff1_03ca5519","line":73,"range":{"start_line":73,"start_character":13,"end_line":73,"end_character":28},"updated":"2019-02-20 09:51:19.000000000","message":"assertTrue(","commit_id":"b4bfcf1144000cc9ef28fde373f0a1fae60cb84b"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"0dd5c5ec53070bda2ff3fcbfaab87a48db0a8f6f","unresolved":false,"context_lines":[{"line_number":70,"context_line":"                                   mock_exists, mock_execute, mock_proc):"},{"line_number":71,"context_line":"        mock_execute.return_value \u003d [value, \"fake stderr\"]"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"        self.assertIs(True, quobyte.is_systemd())"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"        mock_exists.assert_called_once_with(quobyte.SYSTEMCTL_PATH)"},{"line_number":76,"context_line":"        mock_execute.assert_called_once_with(quobyte.SYSTEMCTL_PATH,"}],"source_content_type":"text/x-python","patch_set":26,"id":"9fdfeff1_7ed5f4b6","line":73,"range":{"start_line":73,"start_character":13,"end_line":73,"end_character":28},"in_reply_to":"9fdfeff1_03ca5519","updated":"2019-02-20 12:49:07.000000000","message":"Done","commit_id":"b4bfcf1144000cc9ef28fde373f0a1fae60cb84b"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"cfeff589e8b5f3ad0c83c401fdc53455bd73e5b9","unresolved":false,"context_lines":[{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    @mock.patch.object(os.path, \"exists\", return_value\u003dFalse)"},{"line_number":82,"context_line":"    def test_quobyte_is_systemd_no_binary(self, mock_exists):"},{"line_number":83,"context_line":"        self.assertIsNot(True, quobyte.is_systemd())"},{"line_number":84,"context_line":"        mock_exists.assert_called_once_with(quobyte.SYSTEMCTL_PATH)"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    @mock.patch.object(psutil, \"Process\")"}],"source_content_type":"text/x-python","patch_set":26,"id":"9fdfeff1_23e2f9a1","line":83,"range":{"start_line":83,"start_character":13,"end_line":83,"end_character":31},"updated":"2019-02-20 09:51:19.000000000","message":"assertFalse(","commit_id":"b4bfcf1144000cc9ef28fde373f0a1fae60cb84b"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"0dd5c5ec53070bda2ff3fcbfaab87a48db0a8f6f","unresolved":false,"context_lines":[{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    @mock.patch.object(os.path, \"exists\", return_value\u003dFalse)"},{"line_number":82,"context_line":"    def test_quobyte_is_systemd_no_binary(self, mock_exists):"},{"line_number":83,"context_line":"        self.assertIsNot(True, quobyte.is_systemd())"},{"line_number":84,"context_line":"        mock_exists.assert_called_once_with(quobyte.SYSTEMCTL_PATH)"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    @mock.patch.object(psutil, \"Process\")"}],"source_content_type":"text/x-python","patch_set":26,"id":"9fdfeff1_1ee2a8a1","line":83,"range":{"start_line":83,"start_character":13,"end_line":83,"end_character":31},"in_reply_to":"9fdfeff1_23e2f9a1","updated":"2019-02-20 12:49:07.000000000","message":"Done","commit_id":"b4bfcf1144000cc9ef28fde373f0a1fae60cb84b"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"cfeff589e8b5f3ad0c83c401fdc53455bd73e5b9","unresolved":false,"context_lines":[{"line_number":87,"context_line":"    def test_quobyte_is_systemd_no_sysd_pid_one(self, mock_proc):"},{"line_number":88,"context_line":"        mock_proc.return_value \u003d mock.MagicMock(name\u003d\"NOT_systemd\")"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"        self.assertIsNot(True, quobyte.is_systemd())"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"    @mock.patch.object(processutils, \"execute\")"},{"line_number":93,"context_line":"    @mock.patch.object(os.path, \"exists\", return_value\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":26,"id":"9fdfeff1_e3edd18e","line":90,"range":{"start_line":90,"start_character":13,"end_line":90,"end_character":31},"updated":"2019-02-20 09:51:19.000000000","message":"assertFalse(","commit_id":"b4bfcf1144000cc9ef28fde373f0a1fae60cb84b"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"0dd5c5ec53070bda2ff3fcbfaab87a48db0a8f6f","unresolved":false,"context_lines":[{"line_number":87,"context_line":"    def test_quobyte_is_systemd_no_sysd_pid_one(self, mock_proc):"},{"line_number":88,"context_line":"        mock_proc.return_value \u003d mock.MagicMock(name\u003d\"NOT_systemd\")"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"        self.assertIsNot(True, quobyte.is_systemd())"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"    @mock.patch.object(processutils, \"execute\")"},{"line_number":93,"context_line":"    @mock.patch.object(os.path, \"exists\", return_value\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":26,"id":"9fdfeff1_3edf6cd7","line":90,"range":{"start_line":90,"start_character":13,"end_line":90,"end_character":31},"in_reply_to":"9fdfeff1_e3edd18e","updated":"2019-02-20 12:49:07.000000000","message":"Done","commit_id":"b4bfcf1144000cc9ef28fde373f0a1fae60cb84b"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"cfeff589e8b5f3ad0c83c401fdc53455bd73e5b9","unresolved":false,"context_lines":[{"line_number":94,"context_line":"    def test_quobyte_is_systemd_invalid_state(self, mock_exists, mock_execute):"},{"line_number":95,"context_line":"        mock_execute.return_value \u003d [\"FAKE_INACCEPTABLE_STATE\", \"fake stderr\"]"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"        self.assertIsNot(True, quobyte.is_systemd())"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        mock_exists.assert_called_once_with(quobyte.SYSTEMCTL_PATH)"},{"line_number":100,"context_line":"        mock_execute.assert_called_once_with(quobyte.SYSTEMCTL_PATH,"}],"source_content_type":"text/x-python","patch_set":26,"id":"9fdfeff1_a3e7c9ac","line":97,"range":{"start_line":97,"start_character":13,"end_line":97,"end_character":31},"updated":"2019-02-20 09:51:19.000000000","message":"assertFalse(","commit_id":"b4bfcf1144000cc9ef28fde373f0a1fae60cb84b"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"0dd5c5ec53070bda2ff3fcbfaab87a48db0a8f6f","unresolved":false,"context_lines":[{"line_number":94,"context_line":"    def test_quobyte_is_systemd_invalid_state(self, mock_exists, mock_execute):"},{"line_number":95,"context_line":"        mock_execute.return_value \u003d [\"FAKE_INACCEPTABLE_STATE\", \"fake stderr\"]"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"        self.assertIsNot(True, quobyte.is_systemd())"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        mock_exists.assert_called_once_with(quobyte.SYSTEMCTL_PATH)"},{"line_number":100,"context_line":"        mock_execute.assert_called_once_with(quobyte.SYSTEMCTL_PATH,"}],"source_content_type":"text/x-python","patch_set":26,"id":"9fdfeff1_deed008f","line":97,"range":{"start_line":97,"start_character":13,"end_line":97,"end_character":31},"in_reply_to":"9fdfeff1_a3e7c9ac","updated":"2019-02-20 12:49:07.000000000","message":"Done","commit_id":"b4bfcf1144000cc9ef28fde373f0a1fae60cb84b"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"cfeff589e8b5f3ad0c83c401fdc53455bd73e5b9","unresolved":false,"context_lines":[{"line_number":101,"context_line":"                                             \"is-system-running\","},{"line_number":102,"context_line":"                                             check_exit_code\u003d[0, 1])"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"    @mock.patch.object(os.path, \"exists\", return_value\u003dFalse)"},{"line_number":105,"context_line":"    @mock.patch.object(fileutils, \"ensure_tree\")"},{"line_number":106,"context_line":"    @mock.patch.object(libvirt, \"unprivileged_qb_mount\")"},{"line_number":107,"context_line":"    def test_quobyte_mount_volume_not_systemd(self, mock_mount,"}],"source_content_type":"text/x-python","patch_set":26,"id":"9fdfeff1_6386c1bd","line":104,"range":{"start_line":104,"start_character":4,"end_line":104,"end_character":61},"updated":"2019-02-20 09:51:19.000000000","message":"As before, this would be a better _unit_ test (and less tied to the implementation) if you switched this to:\n\n  @mock.patch.object(quobyte, \"is_systemd\", return_value\u003dFalse)","commit_id":"b4bfcf1144000cc9ef28fde373f0a1fae60cb84b"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"0dd5c5ec53070bda2ff3fcbfaab87a48db0a8f6f","unresolved":false,"context_lines":[{"line_number":101,"context_line":"                                             \"is-system-running\","},{"line_number":102,"context_line":"                                             check_exit_code\u003d[0, 1])"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"    @mock.patch.object(os.path, \"exists\", return_value\u003dFalse)"},{"line_number":105,"context_line":"    @mock.patch.object(fileutils, \"ensure_tree\")"},{"line_number":106,"context_line":"    @mock.patch.object(libvirt, \"unprivileged_qb_mount\")"},{"line_number":107,"context_line":"    def test_quobyte_mount_volume_not_systemd(self, mock_mount,"}],"source_content_type":"text/x-python","patch_set":26,"id":"9fdfeff1_1e9068ed","line":104,"range":{"start_line":104,"start_character":4,"end_line":104,"end_character":61},"in_reply_to":"9fdfeff1_6386c1bd","updated":"2019-02-20 12:49:07.000000000","message":"Done","commit_id":"b4bfcf1144000cc9ef28fde373f0a1fae60cb84b"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"cfeff589e8b5f3ad0c83c401fdc53455bd73e5b9","unresolved":false,"context_lines":[{"line_number":138,"context_line":"                                                   export_mnt_base,"},{"line_number":139,"context_line":"                                                   cfg_file\u003dNone)"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"    @mock.patch.object(libvirt, \u0027systemd_run_qb_mount\u0027)"},{"line_number":142,"context_line":"    @mock.patch.object(quobyte, \"is_systemd\", return_value\u003dTrue)"},{"line_number":143,"context_line":"    @mock.patch.object(fileutils, \"ensure_tree\")"},{"line_number":144,"context_line":"    def test_quobyte_mount_volume_sysd_conf(self, mock_ensure_tree,"},{"line_number":145,"context_line":"                                            mock_exists, mock_privsep_sysdr):"},{"line_number":146,"context_line":"        mnt_base \u003d \u0027/mnt\u0027"},{"line_number":147,"context_line":"        quobyte_volume \u003d \u0027192.168.1.1/volume-00001\u0027"},{"line_number":148,"context_line":"        cfg_file_path \u003d \u0027/some/arbitrary/path\u0027"},{"line_number":149,"context_line":"        export_mnt_base \u003d os.path.join(mnt_base,"},{"line_number":150,"context_line":"                                       utils.get_hash_str(quobyte_volume))"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"        quobyte.mount_volume(quobyte_volume, export_mnt_base, cfg_file_path)"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"        mock_ensure_tree.assert_called_once_with(export_mnt_base)"},{"line_number":155,"context_line":"        mock_exists.assert_called_once_with()"},{"line_number":156,"context_line":"        mock_privsep_sysdr.assert_called_once_with(quobyte_volume,"},{"line_number":157,"context_line":"                                                   export_mnt_base,"},{"line_number":158,"context_line":"                                                   cfg_file\u003dcfg_file_path)"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"    @mock.patch.object(quobyte, \"is_systemd\", return_value\u003dFalse)"},{"line_number":161,"context_line":"    @mock.patch.object(fileutils, \"ensure_tree\")"}],"source_content_type":"text/x-python","patch_set":26,"id":"9fdfeff1_63afa12b","line":158,"range":{"start_line":141,"start_character":0,"end_line":158,"end_character":74},"updated":"2019-02-20 09:51:19.000000000","message":"This is essentially the exact same test as above, only you explicitly set the cfg_file parameter. Not sure if you really need both","commit_id":"b4bfcf1144000cc9ef28fde373f0a1fae60cb84b"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"0dd5c5ec53070bda2ff3fcbfaab87a48db0a8f6f","unresolved":false,"context_lines":[{"line_number":138,"context_line":"                                                   export_mnt_base,"},{"line_number":139,"context_line":"                                                   cfg_file\u003dNone)"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"    @mock.patch.object(libvirt, \u0027systemd_run_qb_mount\u0027)"},{"line_number":142,"context_line":"    @mock.patch.object(quobyte, \"is_systemd\", return_value\u003dTrue)"},{"line_number":143,"context_line":"    @mock.patch.object(fileutils, \"ensure_tree\")"},{"line_number":144,"context_line":"    def test_quobyte_mount_volume_sysd_conf(self, mock_ensure_tree,"},{"line_number":145,"context_line":"                                            mock_exists, mock_privsep_sysdr):"},{"line_number":146,"context_line":"        mnt_base \u003d \u0027/mnt\u0027"},{"line_number":147,"context_line":"        quobyte_volume \u003d \u0027192.168.1.1/volume-00001\u0027"},{"line_number":148,"context_line":"        cfg_file_path \u003d \u0027/some/arbitrary/path\u0027"},{"line_number":149,"context_line":"        export_mnt_base \u003d os.path.join(mnt_base,"},{"line_number":150,"context_line":"                                       utils.get_hash_str(quobyte_volume))"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"        quobyte.mount_volume(quobyte_volume, export_mnt_base, cfg_file_path)"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"        mock_ensure_tree.assert_called_once_with(export_mnt_base)"},{"line_number":155,"context_line":"        mock_exists.assert_called_once_with()"},{"line_number":156,"context_line":"        mock_privsep_sysdr.assert_called_once_with(quobyte_volume,"},{"line_number":157,"context_line":"                                                   export_mnt_base,"},{"line_number":158,"context_line":"                                                   cfg_file\u003dcfg_file_path)"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"    @mock.patch.object(quobyte, \"is_systemd\", return_value\u003dFalse)"},{"line_number":161,"context_line":"    @mock.patch.object(fileutils, \"ensure_tree\")"}],"source_content_type":"text/x-python","patch_set":26,"id":"9fdfeff1_3930e6e8","line":158,"range":{"start_line":141,"start_character":0,"end_line":158,"end_character":74},"in_reply_to":"9fdfeff1_63afa12b","updated":"2019-02-20 12:49:07.000000000","message":"I ddt\u0027ed it in order to reduce the overhead but keep the check for the correct config file pass-through.","commit_id":"b4bfcf1144000cc9ef28fde373f0a1fae60cb84b"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"cfeff589e8b5f3ad0c83c401fdc53455bd73e5b9","unresolved":false,"context_lines":[{"line_number":157,"context_line":"                                                   export_mnt_base,"},{"line_number":158,"context_line":"                                                   cfg_file\u003dcfg_file_path)"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"    @mock.patch.object(quobyte, \"is_systemd\", return_value\u003dFalse)"},{"line_number":161,"context_line":"    @mock.patch.object(fileutils, \"ensure_tree\")"},{"line_number":162,"context_line":"    @mock.patch.object(libvirt, \u0027unprivileged_qb_mount\u0027)"},{"line_number":163,"context_line":"    def test_quobyte_mount_volume_with_config(self,"},{"line_number":164,"context_line":"                                              mock_mount,"},{"line_number":165,"context_line":"                                              mock_ensure_tree,"},{"line_number":166,"context_line":"                                              mock_is_sysd):"},{"line_number":167,"context_line":"        mnt_base \u003d \u0027/mnt\u0027"},{"line_number":168,"context_line":"        quobyte_volume \u003d \u0027192.168.1.1/volume-00001\u0027"},{"line_number":169,"context_line":"        export_mnt_base \u003d os.path.join(mnt_base,"},{"line_number":170,"context_line":"                                       utils.get_hash_str(quobyte_volume))"},{"line_number":171,"context_line":"        config_file_dummy \u003d \"/etc/quobyte/dummy.conf\""},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"        quobyte.mount_volume(quobyte_volume,"},{"line_number":174,"context_line":"                             export_mnt_base,"},{"line_number":175,"context_line":"                             config_file_dummy)"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"        mock_ensure_tree.assert_called_once_with(export_mnt_base)"},{"line_number":178,"context_line":"        expected_commands \u003d [mock.call(quobyte_volume,"},{"line_number":179,"context_line":"                                       export_mnt_base,"},{"line_number":180,"context_line":"                                       cfg_file\u003dconfig_file_dummy)"},{"line_number":181,"context_line":"                             ]"},{"line_number":182,"context_line":"        mock_mount.assert_has_calls(expected_commands)"},{"line_number":183,"context_line":"        mock_is_sysd.assert_called_once()"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    @mock.patch.object(quobyte, \"is_systemd\", return_value\u003dFalse)"},{"line_number":186,"context_line":"    @mock.patch.object(fileutils, \"ensure_tree\")"}],"source_content_type":"text/x-python","patch_set":26,"id":"9fdfeff1_c36f6dfa","line":183,"range":{"start_line":160,"start_character":0,"end_line":183,"end_character":41},"updated":"2019-02-20 09:51:19.000000000","message":"This looks like the exact same test as \u0027test_quobyte_mount_volume_not_systemd\u0027 only you\u0027re doing the mocking the right way (IMO :)) and you\u0027re explicitly setting the cfg_file parameter. As before, I don\u0027t think both of these are really necessary. I\u0027d drop one","commit_id":"b4bfcf1144000cc9ef28fde373f0a1fae60cb84b"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"0dd5c5ec53070bda2ff3fcbfaab87a48db0a8f6f","unresolved":false,"context_lines":[{"line_number":157,"context_line":"                                                   export_mnt_base,"},{"line_number":158,"context_line":"                                                   cfg_file\u003dcfg_file_path)"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"    @mock.patch.object(quobyte, \"is_systemd\", return_value\u003dFalse)"},{"line_number":161,"context_line":"    @mock.patch.object(fileutils, \"ensure_tree\")"},{"line_number":162,"context_line":"    @mock.patch.object(libvirt, \u0027unprivileged_qb_mount\u0027)"},{"line_number":163,"context_line":"    def test_quobyte_mount_volume_with_config(self,"},{"line_number":164,"context_line":"                                              mock_mount,"},{"line_number":165,"context_line":"                                              mock_ensure_tree,"},{"line_number":166,"context_line":"                                              mock_is_sysd):"},{"line_number":167,"context_line":"        mnt_base \u003d \u0027/mnt\u0027"},{"line_number":168,"context_line":"        quobyte_volume \u003d \u0027192.168.1.1/volume-00001\u0027"},{"line_number":169,"context_line":"        export_mnt_base \u003d os.path.join(mnt_base,"},{"line_number":170,"context_line":"                                       utils.get_hash_str(quobyte_volume))"},{"line_number":171,"context_line":"        config_file_dummy \u003d \"/etc/quobyte/dummy.conf\""},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"        quobyte.mount_volume(quobyte_volume,"},{"line_number":174,"context_line":"                             export_mnt_base,"},{"line_number":175,"context_line":"                             config_file_dummy)"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"        mock_ensure_tree.assert_called_once_with(export_mnt_base)"},{"line_number":178,"context_line":"        expected_commands \u003d [mock.call(quobyte_volume,"},{"line_number":179,"context_line":"                                       export_mnt_base,"},{"line_number":180,"context_line":"                                       cfg_file\u003dconfig_file_dummy)"},{"line_number":181,"context_line":"                             ]"},{"line_number":182,"context_line":"        mock_mount.assert_has_calls(expected_commands)"},{"line_number":183,"context_line":"        mock_is_sysd.assert_called_once()"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    @mock.patch.object(quobyte, \"is_systemd\", return_value\u003dFalse)"},{"line_number":186,"context_line":"    @mock.patch.object(fileutils, \"ensure_tree\")"}],"source_content_type":"text/x-python","patch_set":26,"id":"9fdfeff1_f986de65","line":183,"range":{"start_line":160,"start_character":0,"end_line":183,"end_character":41},"in_reply_to":"9fdfeff1_c36f6dfa","updated":"2019-02-20 12:49:07.000000000","message":"Again, ddt\u0027ed it.","commit_id":"b4bfcf1144000cc9ef28fde373f0a1fae60cb84b"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"d8881699c69274007d4dbe6fa710416b828702ab","unresolved":false,"context_lines":[{"line_number":86,"context_line":"    @mock.patch.object(os.path, \"exists\", return_value\u003dTrue)"},{"line_number":87,"context_line":"    def test_quobyte_is_systemd_invalid_state(self, mock_exists, mock_execute,"},{"line_number":88,"context_line":"                                              mock_proc):"},{"line_number":89,"context_line":"        mock_execute.return_value \u003d [\"FAKE_INACCEPTABLE_STATE\", \"fake stderr\"]"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"        self.assertFalse(quobyte.is_systemd())"},{"line_number":92,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"5fc1f717_921a756c","line":89,"range":{"start_line":89,"start_character":43,"end_line":89,"end_character":44},"updated":"2019-03-27 10:49:26.000000000","message":"nit: UNACCEPTABLE","commit_id":"7231f7dee10fa8f9e6cead026f6a5ae3f5b15ae4"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"2478209047ed4250e1e17e73f63a6494affc7dea","unresolved":false,"context_lines":[{"line_number":86,"context_line":"    @mock.patch.object(os.path, \"exists\", return_value\u003dTrue)"},{"line_number":87,"context_line":"    def test_quobyte_is_systemd_invalid_state(self, mock_exists, mock_execute,"},{"line_number":88,"context_line":"                                              mock_proc):"},{"line_number":89,"context_line":"        mock_execute.return_value \u003d [\"FAKE_INACCEPTABLE_STATE\", \"fake stderr\"]"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"        self.assertFalse(quobyte.is_systemd())"},{"line_number":92,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"5fc1f717_a6064e1b","line":89,"range":{"start_line":89,"start_character":43,"end_line":89,"end_character":44},"in_reply_to":"5fc1f717_68bd96b5","updated":"2019-03-27 13:37:02.000000000","message":"Done","commit_id":"7231f7dee10fa8f9e6cead026f6a5ae3f5b15ae4"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"b7c97a5e3c4f31505e66faee85806678c3211d8a","unresolved":false,"context_lines":[{"line_number":86,"context_line":"    @mock.patch.object(os.path, \"exists\", return_value\u003dTrue)"},{"line_number":87,"context_line":"    def test_quobyte_is_systemd_invalid_state(self, mock_exists, mock_execute,"},{"line_number":88,"context_line":"                                              mock_proc):"},{"line_number":89,"context_line":"        mock_execute.return_value \u003d [\"FAKE_INACCEPTABLE_STATE\", \"fake stderr\"]"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"        self.assertFalse(quobyte.is_systemd())"},{"line_number":92,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"5fc1f717_68bd96b5","line":89,"range":{"start_line":89,"start_character":43,"end_line":89,"end_character":44},"in_reply_to":"5fc1f717_921a756c","updated":"2019-03-27 11:29:31.000000000","message":"ok","commit_id":"7231f7dee10fa8f9e6cead026f6a5ae3f5b15ae4"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"d8881699c69274007d4dbe6fa710416b828702ab","unresolved":false,"context_lines":[{"line_number":100,"context_line":"    @mock.patch.object(quobyte, \"is_systemd\", return_value\u003dFalse)"},{"line_number":101,"context_line":"    @mock.patch.object(fileutils, \"ensure_tree\")"},{"line_number":102,"context_line":"    @mock.patch.object(libvirt, \"unprivileged_qb_mount\")"},{"line_number":103,"context_line":"    def test_quobyte_mount_volume_not_systemd(self, value, mock_mount,"},{"line_number":104,"context_line":"                                              mock_ensure_tree, mock_is_sysd):"},{"line_number":105,"context_line":"        mnt_base \u003d \u0027/mnt\u0027"},{"line_number":106,"context_line":"        quobyte_volume \u003d \u0027192.168.1.1/volume-00001\u0027"}],"source_content_type":"text/x-python","patch_set":28,"id":"5fc1f717_adb15046","line":103,"range":{"start_line":103,"start_character":52,"end_line":103,"end_character":57},"updated":"2019-03-27 10:49:26.000000000","message":"nit: This would be clearer if you called it cfg_file","commit_id":"7231f7dee10fa8f9e6cead026f6a5ae3f5b15ae4"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"2478209047ed4250e1e17e73f63a6494affc7dea","unresolved":false,"context_lines":[{"line_number":100,"context_line":"    @mock.patch.object(quobyte, \"is_systemd\", return_value\u003dFalse)"},{"line_number":101,"context_line":"    @mock.patch.object(fileutils, \"ensure_tree\")"},{"line_number":102,"context_line":"    @mock.patch.object(libvirt, \"unprivileged_qb_mount\")"},{"line_number":103,"context_line":"    def test_quobyte_mount_volume_not_systemd(self, value, mock_mount,"},{"line_number":104,"context_line":"                                              mock_ensure_tree, mock_is_sysd):"},{"line_number":105,"context_line":"        mnt_base \u003d \u0027/mnt\u0027"},{"line_number":106,"context_line":"        quobyte_volume \u003d \u0027192.168.1.1/volume-00001\u0027"}],"source_content_type":"text/x-python","patch_set":28,"id":"5fc1f717_46152a3c","line":103,"range":{"start_line":103,"start_character":52,"end_line":103,"end_character":57},"in_reply_to":"5fc1f717_28b30eeb","updated":"2019-03-27 13:37:02.000000000","message":"Done","commit_id":"7231f7dee10fa8f9e6cead026f6a5ae3f5b15ae4"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"b7c97a5e3c4f31505e66faee85806678c3211d8a","unresolved":false,"context_lines":[{"line_number":100,"context_line":"    @mock.patch.object(quobyte, \"is_systemd\", return_value\u003dFalse)"},{"line_number":101,"context_line":"    @mock.patch.object(fileutils, \"ensure_tree\")"},{"line_number":102,"context_line":"    @mock.patch.object(libvirt, \"unprivileged_qb_mount\")"},{"line_number":103,"context_line":"    def test_quobyte_mount_volume_not_systemd(self, value, mock_mount,"},{"line_number":104,"context_line":"                                              mock_ensure_tree, mock_is_sysd):"},{"line_number":105,"context_line":"        mnt_base \u003d \u0027/mnt\u0027"},{"line_number":106,"context_line":"        quobyte_volume \u003d \u0027192.168.1.1/volume-00001\u0027"}],"source_content_type":"text/x-python","patch_set":28,"id":"5fc1f717_28b30eeb","line":103,"range":{"start_line":103,"start_character":52,"end_line":103,"end_character":57},"in_reply_to":"5fc1f717_adb15046","updated":"2019-03-27 11:29:31.000000000","message":"ok","commit_id":"7231f7dee10fa8f9e6cead026f6a5ae3f5b15ae4"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"d8881699c69274007d4dbe6fa710416b828702ab","unresolved":false,"context_lines":[{"line_number":119,"context_line":"    @mock.patch.object(libvirt, \u0027systemd_run_qb_mount\u0027)"},{"line_number":120,"context_line":"    @mock.patch.object(quobyte, \"is_systemd\", return_value\u003dTrue)"},{"line_number":121,"context_line":"    @mock.patch.object(fileutils, \"ensure_tree\")"},{"line_number":122,"context_line":"    def test_quobyte_mount_volume_systemd(self, value, mock_ensure_tree,"},{"line_number":123,"context_line":"                                          mock_exists, mock_privsep_sysdr):"},{"line_number":124,"context_line":"        mnt_base \u003d \u0027/mnt\u0027"},{"line_number":125,"context_line":"        quobyte_volume \u003d \u0027192.168.1.1/volume-00001\u0027"}],"source_content_type":"text/x-python","patch_set":28,"id":"5fc1f717_6dee0823","line":122,"range":{"start_line":122,"start_character":48,"end_line":122,"end_character":53},"updated":"2019-03-27 10:49:26.000000000","message":"nit: cfg_file","commit_id":"7231f7dee10fa8f9e6cead026f6a5ae3f5b15ae4"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"b7c97a5e3c4f31505e66faee85806678c3211d8a","unresolved":false,"context_lines":[{"line_number":119,"context_line":"    @mock.patch.object(libvirt, \u0027systemd_run_qb_mount\u0027)"},{"line_number":120,"context_line":"    @mock.patch.object(quobyte, \"is_systemd\", return_value\u003dTrue)"},{"line_number":121,"context_line":"    @mock.patch.object(fileutils, \"ensure_tree\")"},{"line_number":122,"context_line":"    def test_quobyte_mount_volume_systemd(self, value, mock_ensure_tree,"},{"line_number":123,"context_line":"                                          mock_exists, mock_privsep_sysdr):"},{"line_number":124,"context_line":"        mnt_base \u003d \u0027/mnt\u0027"},{"line_number":125,"context_line":"        quobyte_volume \u003d \u0027192.168.1.1/volume-00001\u0027"}],"source_content_type":"text/x-python","patch_set":28,"id":"5fc1f717_c8cdaa66","line":122,"range":{"start_line":122,"start_character":48,"end_line":122,"end_character":53},"in_reply_to":"5fc1f717_6dee0823","updated":"2019-03-27 11:29:31.000000000","message":"ok","commit_id":"7231f7dee10fa8f9e6cead026f6a5ae3f5b15ae4"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"2478209047ed4250e1e17e73f63a6494affc7dea","unresolved":false,"context_lines":[{"line_number":119,"context_line":"    @mock.patch.object(libvirt, \u0027systemd_run_qb_mount\u0027)"},{"line_number":120,"context_line":"    @mock.patch.object(quobyte, \"is_systemd\", return_value\u003dTrue)"},{"line_number":121,"context_line":"    @mock.patch.object(fileutils, \"ensure_tree\")"},{"line_number":122,"context_line":"    def test_quobyte_mount_volume_systemd(self, value, mock_ensure_tree,"},{"line_number":123,"context_line":"                                          mock_exists, mock_privsep_sysdr):"},{"line_number":124,"context_line":"        mnt_base \u003d \u0027/mnt\u0027"},{"line_number":125,"context_line":"        quobyte_volume \u003d \u0027192.168.1.1/volume-00001\u0027"}],"source_content_type":"text/x-python","patch_set":28,"id":"5fc1f717_e629b689","line":122,"range":{"start_line":122,"start_character":48,"end_line":122,"end_character":53},"in_reply_to":"5fc1f717_c8cdaa66","updated":"2019-03-27 13:37:02.000000000","message":"Done","commit_id":"7231f7dee10fa8f9e6cead026f6a5ae3f5b15ae4"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"d8881699c69274007d4dbe6fa710416b828702ab","unresolved":false,"context_lines":[{"line_number":210,"context_line":"    @mock.patch.object(libvirt, \u0027qb_umount\u0027,"},{"line_number":211,"context_line":"                       side_effect\u003d(processutils.ProcessExecutionError))"},{"line_number":212,"context_line":"    def test_quobyte_umount_volume_fails(self,"},{"line_number":213,"context_line":"                                         mock_execute,"},{"line_number":214,"context_line":"                                         mock_exception,"},{"line_number":215,"context_line":"                                         mock_issysd):"},{"line_number":216,"context_line":"        mnt_base \u003d \u0027/mnt\u0027"}],"source_content_type":"text/x-python","patch_set":28,"id":"5fc1f717_6d94c837","line":213,"range":{"start_line":213,"start_character":41,"end_line":213,"end_character":53},"updated":"2019-03-27 10:49:26.000000000","message":"mock_umount? This is a bit confusing.","commit_id":"7231f7dee10fa8f9e6cead026f6a5ae3f5b15ae4"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"2478209047ed4250e1e17e73f63a6494affc7dea","unresolved":false,"context_lines":[{"line_number":210,"context_line":"    @mock.patch.object(libvirt, \u0027qb_umount\u0027,"},{"line_number":211,"context_line":"                       side_effect\u003d(processutils.ProcessExecutionError))"},{"line_number":212,"context_line":"    def test_quobyte_umount_volume_fails(self,"},{"line_number":213,"context_line":"                                         mock_execute,"},{"line_number":214,"context_line":"                                         mock_exception,"},{"line_number":215,"context_line":"                                         mock_issysd):"},{"line_number":216,"context_line":"        mnt_base \u003d \u0027/mnt\u0027"}],"source_content_type":"text/x-python","patch_set":28,"id":"5fc1f717_c6227aa3","line":213,"range":{"start_line":213,"start_character":41,"end_line":213,"end_character":53},"in_reply_to":"5fc1f717_68e276d7","updated":"2019-03-27 13:37:02.000000000","message":"Done","commit_id":"7231f7dee10fa8f9e6cead026f6a5ae3f5b15ae4"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"b7c97a5e3c4f31505e66faee85806678c3211d8a","unresolved":false,"context_lines":[{"line_number":210,"context_line":"    @mock.patch.object(libvirt, \u0027qb_umount\u0027,"},{"line_number":211,"context_line":"                       side_effect\u003d(processutils.ProcessExecutionError))"},{"line_number":212,"context_line":"    def test_quobyte_umount_volume_fails(self,"},{"line_number":213,"context_line":"                                         mock_execute,"},{"line_number":214,"context_line":"                                         mock_exception,"},{"line_number":215,"context_line":"                                         mock_issysd):"},{"line_number":216,"context_line":"        mnt_base \u003d \u0027/mnt\u0027"}],"source_content_type":"text/x-python","patch_set":28,"id":"5fc1f717_68e276d7","line":213,"range":{"start_line":213,"start_character":41,"end_line":213,"end_character":53},"in_reply_to":"5fc1f717_6d94c837","updated":"2019-03-27 11:29:31.000000000","message":"Yes, thats a legacy artifact. I\u0027ll change that, too.","commit_id":"7231f7dee10fa8f9e6cead026f6a5ae3f5b15ae4"}],"nova/virt/libvirt/volume/quobyte.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"f5486e4856dd503fbd678dad315130e7257959a2","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        # prevent Nova service stop/restarts from killing the mount."},{"line_number":50,"context_line":"        LOG.debug(\u0027Mounting volume %s at mount point %s via systemd-run\u0027,"},{"line_number":51,"context_line":"                  volume, mnt_base)"},{"line_number":52,"context_line":"        libvirt.systemd_run_qb_mount(volume, mnt_base, configfile)"},{"line_number":53,"context_line":"    else:"},{"line_number":54,"context_line":"        # NOTE(kaisers): disable xattrs to speed up io as this omits"},{"line_number":55,"context_line":"        # additional metadata requests in the backend. xattrs can be"}],"source_content_type":"text/x-python","patch_set":2,"id":"df7087c5_be44e175","line":52,"range":{"start_line":52,"start_character":55,"end_line":52,"end_character":65},"updated":"2018-03-19 15:40:00.000000000","message":"kwargs should be named.","commit_id":"5b1a0d16a2d0de1b648fbf771985db89faee09c8"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"367728a6238cbb762615aa1e072890d31eac0daf","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        # prevent Nova service stop/restarts from killing the mount."},{"line_number":50,"context_line":"        LOG.debug(\u0027Mounting volume %s at mount point %s via systemd-run\u0027,"},{"line_number":51,"context_line":"                  volume, mnt_base)"},{"line_number":52,"context_line":"        libvirt.systemd_run_qb_mount(volume, mnt_base, configfile)"},{"line_number":53,"context_line":"    else:"},{"line_number":54,"context_line":"        # NOTE(kaisers): disable xattrs to speed up io as this omits"},{"line_number":55,"context_line":"        # additional metadata requests in the backend. xattrs can be"}],"source_content_type":"text/x-python","patch_set":2,"id":"df7087c5_e41aac15","line":52,"range":{"start_line":52,"start_character":55,"end_line":52,"end_character":65},"in_reply_to":"df7087c5_a1bb46e3","updated":"2018-03-20 14:05:41.000000000","message":"yep, of course.\nTotally didn\u0027t get that earlier...","commit_id":"5b1a0d16a2d0de1b648fbf771985db89faee09c8"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"db59691f3bd156ffa42850d4484d6cd5e3829eb6","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        # prevent Nova service stop/restarts from killing the mount."},{"line_number":50,"context_line":"        LOG.debug(\u0027Mounting volume %s at mount point %s via systemd-run\u0027,"},{"line_number":51,"context_line":"                  volume, mnt_base)"},{"line_number":52,"context_line":"        libvirt.systemd_run_qb_mount(volume, mnt_base, configfile)"},{"line_number":53,"context_line":"    else:"},{"line_number":54,"context_line":"        # NOTE(kaisers): disable xattrs to speed up io as this omits"},{"line_number":55,"context_line":"        # additional metadata requests in the backend. xattrs can be"}],"source_content_type":"text/x-python","patch_set":2,"id":"df7087c5_ec585cfb","line":52,"range":{"start_line":52,"start_character":55,"end_line":52,"end_character":65},"in_reply_to":"df7087c5_be44e175","updated":"2018-03-20 08:34:00.000000000","message":"Not sure what you\u0027re aiming at. The configfile is a specific config file path param for the mount command that may optionally be present. There\u0027re no further arguments that would warrant providing a list/dict param.\nPlease drop me a line if that\u0027s not what you\u0027re aiming at.","commit_id":"5b1a0d16a2d0de1b648fbf771985db89faee09c8"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"4730729500b3216b6dd57c7187664fd079546a93","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        # prevent Nova service stop/restarts from killing the mount."},{"line_number":50,"context_line":"        LOG.debug(\u0027Mounting volume %s at mount point %s via systemd-run\u0027,"},{"line_number":51,"context_line":"                  volume, mnt_base)"},{"line_number":52,"context_line":"        libvirt.systemd_run_qb_mount(volume, mnt_base, configfile)"},{"line_number":53,"context_line":"    else:"},{"line_number":54,"context_line":"        # NOTE(kaisers): disable xattrs to speed up io as this omits"},{"line_number":55,"context_line":"        # additional metadata requests in the backend. xattrs can be"}],"source_content_type":"text/x-python","patch_set":2,"id":"df7087c5_a1bb46e3","line":52,"range":{"start_line":52,"start_character":55,"end_line":52,"end_character":65},"in_reply_to":"df7087c5_ec585cfb","updated":"2018-03-20 13:46:23.000000000","message":"Sorry I missed this response earlier.\n\nWhen the method definition lists an argument as a keyword argument, calls to that method should use that key when specifying that parameter.  In this case:\n\n libvirt.systemd_run_qb_mount(volume, mnt_base, cfg_file\u003dconfigfile)\n\nThe reasons are several, including explicitness and readability, but the main one is because it allows the method definition to change in a number of ways without breaking the calls.  For example, if it decides to add a new positional argument, your call won\u0027t explicitly break (so you may not find it) - but you\u0027ll be passing configfile to that new argument, which ain\u0027t what you wanted.  Likewise if we add a new kwarg before cfg_file.  Or if we decide to change to using **kwargs.  Etc.","commit_id":"5b1a0d16a2d0de1b648fbf771985db89faee09c8"},{"author":{"_account_id":2271,"name":"Michael Still","email":"mikal@stillhq.com","username":"mikalstill"},"change_message_id":"cb7bb1831801fa6c1361f9052c7134cc07e1b7d4","unresolved":false,"context_lines":[{"line_number":61,"context_line":"        LOG.debug(\u0027Mounting volume %s at mount point %s via command %s\u0027,"},{"line_number":62,"context_line":"                  volume, mnt_base, command)"},{"line_number":63,"context_line":"        utils.execute(*command)"},{"line_number":64,"context_line":"    LOG.info(\u0027Mounted volume: %s\u0027, volume)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"def umount_volume(mnt_base):"}],"source_content_type":"text/x-python","patch_set":3,"id":"df7087c5_65621fdb","line":64,"updated":"2018-03-20 09:51:00.000000000","message":"I think you\u0027d be better off writing this with the unprivileged pattern that other privsep calls that are only sometimes done as root use. That keeps the logic needed to do this all in one place.\n\nSee https://git.openstack.org/cgit/openstack/nova/tree/nova/privsep/fs.py#n230 for an example.","commit_id":"8447b36ef3302f6195848899c8617b9796db69b7"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"5b9d5f72ef50c3f9c86d9335140cd9f9a7333b0f","unresolved":false,"context_lines":[{"line_number":61,"context_line":"        LOG.debug(\u0027Mounting volume %s at mount point %s via command %s\u0027,"},{"line_number":62,"context_line":"                  volume, mnt_base, command)"},{"line_number":63,"context_line":"        utils.execute(*command)"},{"line_number":64,"context_line":"    LOG.info(\u0027Mounted volume: %s\u0027, volume)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"def umount_volume(mnt_base):"}],"source_content_type":"text/x-python","patch_set":3,"id":"df7087c5_857c13b6","line":64,"in_reply_to":"df7087c5_65621fdb","updated":"2018-03-20 10:27:23.000000000","message":"mmh, i can add the unpriv/entrypoint pattern for the libvirt.systemd_run... method, np.\n\nThe alternate mount call however uses not sysd but the mount binary directly and does not need the root escalation. So i\u0027m not sure it\u0027s useful to add that to privsep.libvirt.\nDo i understand correctly that you\u0027d prefer this second mount call to move to similarly patterned methods in privsep.libvirt, too?","commit_id":"8447b36ef3302f6195848899c8617b9796db69b7"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"f91f6bfdd6dc398d35836433354fe4bdfda94312","unresolved":false,"context_lines":[{"line_number":61,"context_line":"        LOG.debug(\u0027Mounting volume %s at mount point %s via command %s\u0027,"},{"line_number":62,"context_line":"                  volume, mnt_base, command)"},{"line_number":63,"context_line":"        utils.execute(*command)"},{"line_number":64,"context_line":"    LOG.info(\u0027Mounted volume: %s\u0027, volume)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"def umount_volume(mnt_base):"}],"source_content_type":"text/x-python","patch_set":3,"id":"bf659307_3d56705e","line":64,"in_reply_to":"df7087c5_857c13b6","updated":"2018-03-27 09:27:24.000000000","message":"Yeah, I\u0027m not so sure about this. We never need root here.","commit_id":"8447b36ef3302f6195848899c8617b9796db69b7"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"386eb8c9a3e936ff875fd9ca2c48ac59b1334466","unresolved":false,"context_lines":[{"line_number":44,"context_line":"    \"\"\"Wraps execute calls for mounting a Quobyte volume\"\"\""},{"line_number":45,"context_line":"    fileutils.ensure_tree(mnt_base)"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    if os.path.exists(\"/run/systemd/system\"):"},{"line_number":48,"context_line":"        # Note(kaisers): with systemd this requires a separate CGROUP to"},{"line_number":49,"context_line":"        # prevent Nova service stop/restarts from killing the mount."},{"line_number":50,"context_line":"        LOG.debug(\u0027Mounting volume %s at mount point %s via systemd-run\u0027,"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_9e089d30","line":47,"updated":"2018-12-05 14:35:10.000000000","message":"I think there\u0027s a more robust way to do this. is-system-running?","commit_id":"b86a3ad2036663dbe1e9988d59a4271fde8fd1e9"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"ab5a28447dc3632cbf0487f41b41e17d09b1fcd3","unresolved":false,"context_lines":[{"line_number":44,"context_line":"    \"\"\"Wraps execute calls for mounting a Quobyte volume\"\"\""},{"line_number":45,"context_line":"    fileutils.ensure_tree(mnt_base)"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    if os.path.exists(\"/run/systemd/system\"):"},{"line_number":48,"context_line":"        # Note(kaisers): with systemd this requires a separate CGROUP to"},{"line_number":49,"context_line":"        # prevent Nova service stop/restarts from killing the mount."},{"line_number":50,"context_line":"        LOG.debug(\u0027Mounting volume %s at mount point %s via systemd-run\u0027,"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_48555350","line":47,"in_reply_to":"3f79a3b5_9e089d30","updated":"2018-12-06 15:23:14.000000000","message":"Done","commit_id":"b86a3ad2036663dbe1e9988d59a4271fde8fd1e9"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"f6b6dc86e0f8b7710f09d35252a508541d57dd0e","unresolved":false,"context_lines":[{"line_number":44,"context_line":""},{"line_number":45,"context_line":"def is_systemd():"},{"line_number":46,"context_line":"    \"\"\"Checks if the host is running systemd\"\"\""},{"line_number":47,"context_line":"    if os.path.exists(SYSTEMCTL_PATH):"},{"line_number":48,"context_line":"        sysdout, sysderr \u003d processutils.execute(SYSTEMCTL_PATH,"},{"line_number":49,"context_line":"                                       \"is-system-running\","},{"line_number":50,"context_line":"                                        check_exit_code\u003d[0, 1])"},{"line_number":51,"context_line":"        print(\"systemd out is %s\" % sysdout)"},{"line_number":52,"context_line":"        for state in VALID_SYSD_STATES:"},{"line_number":53,"context_line":"            if state in sysdout:"},{"line_number":54,"context_line":"                print(\"systemd in valid state\")"},{"line_number":55,"context_line":"                return True"},{"line_number":56,"context_line":"    return False"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"def mount_volume(volume, mnt_base, configfile\u003dNone):"}],"source_content_type":"text/x-python","patch_set":24,"id":"9fdfeff1_ea976b44","line":56,"range":{"start_line":47,"start_character":0,"end_line":56,"end_character":16},"updated":"2019-02-13 11:14:51.000000000","message":"This still feels incredibly flaky. Before we commit to this, I wonder if we could simply check the PID of the systemd executable? It\u0027d be 1 if it was the init system, right?","commit_id":"b0a1cf703878a8e2f04ab7b4f24fdc6c5ae0b8fd"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"2a66999d3f9c5461a714c006d0e552b1cbe1cd87","unresolved":false,"context_lines":[{"line_number":44,"context_line":""},{"line_number":45,"context_line":"def is_systemd():"},{"line_number":46,"context_line":"    \"\"\"Checks if the host is running systemd\"\"\""},{"line_number":47,"context_line":"    if os.path.exists(SYSTEMCTL_PATH):"},{"line_number":48,"context_line":"        sysdout, sysderr \u003d processutils.execute(SYSTEMCTL_PATH,"},{"line_number":49,"context_line":"                                       \"is-system-running\","},{"line_number":50,"context_line":"                                        check_exit_code\u003d[0, 1])"},{"line_number":51,"context_line":"        print(\"systemd out is %s\" % sysdout)"},{"line_number":52,"context_line":"        for state in VALID_SYSD_STATES:"},{"line_number":53,"context_line":"            if state in sysdout:"},{"line_number":54,"context_line":"                print(\"systemd in valid state\")"},{"line_number":55,"context_line":"                return True"},{"line_number":56,"context_line":"    return False"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"def mount_volume(volume, mnt_base, configfile\u003dNone):"}],"source_content_type":"text/x-python","patch_set":24,"id":"9fdfeff1_bb338513","line":56,"range":{"start_line":47,"start_character":0,"end_line":56,"end_character":16},"in_reply_to":"9fdfeff1_ea976b44","updated":"2019-02-19 13:38:20.000000000","message":"Might add the pid 1 check and then run the is-sys-run check to see if everything is up. That should ensure sysd is here and on.I\u0027ll update this.","commit_id":"b0a1cf703878a8e2f04ab7b4f24fdc6c5ae0b8fd"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"f6b6dc86e0f8b7710f09d35252a508541d57dd0e","unresolved":false,"context_lines":[{"line_number":60,"context_line":"    \"\"\"Wraps execute calls for mounting a Quobyte volume\"\"\""},{"line_number":61,"context_line":"    fileutils.ensure_tree(mnt_base)"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    if is_systemd():"},{"line_number":64,"context_line":"        # Note(kaisers): with systemd this requires a separate CGROUP to"},{"line_number":65,"context_line":"        # prevent Nova service stop/restarts from killing the mount."},{"line_number":66,"context_line":"        LOG.debug(\u0027Mounting volume %s at mount point %s via systemd-run\u0027,"}],"source_content_type":"text/x-python","patch_set":24,"id":"9fdfeff1_0aa3cf9f","line":63,"range":{"start_line":63,"start_character":0,"end_line":63,"end_character":20},"updated":"2019-02-13 11:14:51.000000000","message":"A note about why it\u0027s necessary to do something different if systemd is present would be helpful","commit_id":"b0a1cf703878a8e2f04ab7b4f24fdc6c5ae0b8fd"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"2a66999d3f9c5461a714c006d0e552b1cbe1cd87","unresolved":false,"context_lines":[{"line_number":60,"context_line":"    \"\"\"Wraps execute calls for mounting a Quobyte volume\"\"\""},{"line_number":61,"context_line":"    fileutils.ensure_tree(mnt_base)"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    if is_systemd():"},{"line_number":64,"context_line":"        # Note(kaisers): with systemd this requires a separate CGROUP to"},{"line_number":65,"context_line":"        # prevent Nova service stop/restarts from killing the mount."},{"line_number":66,"context_line":"        LOG.debug(\u0027Mounting volume %s at mount point %s via systemd-run\u0027,"}],"source_content_type":"text/x-python","patch_set":24,"id":"9fdfeff1_d57dfab2","line":63,"range":{"start_line":63,"start_character":0,"end_line":63,"end_character":20},"in_reply_to":"9fdfeff1_0aa3cf9f","updated":"2019-02-19 13:38:20.000000000","message":"That note is down in the two following lines, i can move it up in front of the if clause if that improves readability.","commit_id":"b0a1cf703878a8e2f04ab7b4f24fdc6c5ae0b8fd"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"f6b6dc86e0f8b7710f09d35252a508541d57dd0e","unresolved":false,"context_lines":[{"line_number":65,"context_line":"        # prevent Nova service stop/restarts from killing the mount."},{"line_number":66,"context_line":"        LOG.debug(\u0027Mounting volume %s at mount point %s via systemd-run\u0027,"},{"line_number":67,"context_line":"                  volume, mnt_base)"},{"line_number":68,"context_line":"        # Note(kaisers): This can be moved to an unprivileged mount by using"},{"line_number":69,"context_line":"        # the systemd-run --user flag once that flag is supported by CentOS"},{"line_number":70,"context_line":"        libvirt.systemd_run_qb_mount(volume, mnt_base, cfg_file\u003dconfigfile)"},{"line_number":71,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":24,"id":"9fdfeff1_aa8f2328","line":68,"range":{"start_line":68,"start_character":10,"end_line":68,"end_character":14},"updated":"2019-02-13 11:14:51.000000000","message":"TODO","commit_id":"b0a1cf703878a8e2f04ab7b4f24fdc6c5ae0b8fd"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"2a66999d3f9c5461a714c006d0e552b1cbe1cd87","unresolved":false,"context_lines":[{"line_number":65,"context_line":"        # prevent Nova service stop/restarts from killing the mount."},{"line_number":66,"context_line":"        LOG.debug(\u0027Mounting volume %s at mount point %s via systemd-run\u0027,"},{"line_number":67,"context_line":"                  volume, mnt_base)"},{"line_number":68,"context_line":"        # Note(kaisers): This can be moved to an unprivileged mount by using"},{"line_number":69,"context_line":"        # the systemd-run --user flag once that flag is supported by CentOS"},{"line_number":70,"context_line":"        libvirt.systemd_run_qb_mount(volume, mnt_base, cfg_file\u003dconfigfile)"},{"line_number":71,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":24,"id":"9fdfeff1_b5339613","line":68,"range":{"start_line":68,"start_character":10,"end_line":68,"end_character":14},"in_reply_to":"9fdfeff1_aa8f2328","updated":"2019-02-19 13:38:20.000000000","message":"Actually i\u0027m removing this. As stated in the commit message and the bug report there are multiple reasons, not only the CentOS support, preventing us from using the --user flag. I don\u0027t see all these reasons to be removed in the near future.","commit_id":"b0a1cf703878a8e2f04ab7b4f24fdc6c5ae0b8fd"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"f6b6dc86e0f8b7710f09d35252a508541d57dd0e","unresolved":false,"context_lines":[{"line_number":66,"context_line":"        LOG.debug(\u0027Mounting volume %s at mount point %s via systemd-run\u0027,"},{"line_number":67,"context_line":"                  volume, mnt_base)"},{"line_number":68,"context_line":"        # Note(kaisers): This can be moved to an unprivileged mount by using"},{"line_number":69,"context_line":"        # the systemd-run --user flag once that flag is supported by CentOS"},{"line_number":70,"context_line":"        libvirt.systemd_run_qb_mount(volume, mnt_base, cfg_file\u003dconfigfile)"},{"line_number":71,"context_line":"    else:"},{"line_number":72,"context_line":"        LOG.debug(\u0027Mounting volume %s at mount point %s via mount.quobyte\u0027,"}],"source_content_type":"text/x-python","patch_set":24,"id":"9fdfeff1_aa940338","line":69,"range":{"start_line":69,"start_character":0,"end_line":69,"end_character":75},"updated":"2019-02-13 11:14:51.000000000","message":"What happens exactly if the \u0027--user\u0027 flag is set? Is there something we can use to detect? Could we try the unprivileged command then fallback if it didn\u0027t succeed?","commit_id":"b0a1cf703878a8e2f04ab7b4f24fdc6c5ae0b8fd"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"2a66999d3f9c5461a714c006d0e552b1cbe1cd87","unresolved":false,"context_lines":[{"line_number":66,"context_line":"        LOG.debug(\u0027Mounting volume %s at mount point %s via systemd-run\u0027,"},{"line_number":67,"context_line":"                  volume, mnt_base)"},{"line_number":68,"context_line":"        # Note(kaisers): This can be moved to an unprivileged mount by using"},{"line_number":69,"context_line":"        # the systemd-run --user flag once that flag is supported by CentOS"},{"line_number":70,"context_line":"        libvirt.systemd_run_qb_mount(volume, mnt_base, cfg_file\u003dconfigfile)"},{"line_number":71,"context_line":"    else:"},{"line_number":72,"context_line":"        LOG.debug(\u0027Mounting volume %s at mount point %s via mount.quobyte\u0027,"}],"source_content_type":"text/x-python","patch_set":24,"id":"9fdfeff1_d004281b","line":69,"range":{"start_line":69,"start_character":0,"end_line":69,"end_character":75},"in_reply_to":"9fdfeff1_aa940338","updated":"2019-02-19 13:38:20.000000000","message":"With the flag systemd fails to create the process, reporting an issue with the bus connection / no such file or directory (see [1]). We might try and catch the exception, parsing for the error message elements but that\u0027s not something i\u0027d prefer.\n\n[1] https://bugs.launchpad.net/nova/+bug/1756823","commit_id":"b0a1cf703878a8e2f04ab7b4f24fdc6c5ae0b8fd"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"155f66da4d507f15d23506e117f7fade32c4af66","unresolved":false,"context_lines":[{"line_number":44,"context_line":""},{"line_number":45,"context_line":"def is_systemd():"},{"line_number":46,"context_line":"    \"\"\"Checks if the host is running systemd\"\"\""},{"line_number":47,"context_line":"    if psutil.Process(1).name() \u003d\u003d \"systemd\" and os.path.exists("},{"line_number":48,"context_line":"            SYSTEMCTL_PATH):"},{"line_number":49,"context_line":"        sysdout, sysderr \u003d processutils.execute(SYSTEMCTL_PATH,"},{"line_number":50,"context_line":"                                       \"is-system-running\","},{"line_number":51,"context_line":"                                        check_exit_code\u003d[0, 1])"},{"line_number":52,"context_line":"        print(\"systemd out is %s\" % sysdout)"},{"line_number":53,"context_line":"        for state in VALID_SYSD_STATES:"},{"line_number":54,"context_line":"            if state in sysdout:"},{"line_number":55,"context_line":"                print(\"systemd in valid state\")"},{"line_number":56,"context_line":"                return True"},{"line_number":57,"context_line":"    return False"},{"line_number":58,"context_line":""}],"source_content_type":"text/x-python","patch_set":25,"id":"9fdfeff1_762a142d","line":55,"range":{"start_line":47,"start_character":44,"end_line":55,"end_character":47},"updated":"2019-02-19 14:17:14.000000000","message":"Not sure if any of this is necessary: it seems like systemd is either PID 1 or it\u0027s not, and if it\u0027s PID 1 then it\u0027s the init system, right?","commit_id":"7da2e44bb608e93925ce43b173d01cdb46453aa9"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"14dc3fc20e5c1de3fd145090dad5cd2701e05339","unresolved":false,"context_lines":[{"line_number":44,"context_line":""},{"line_number":45,"context_line":"def is_systemd():"},{"line_number":46,"context_line":"    \"\"\"Checks if the host is running systemd\"\"\""},{"line_number":47,"context_line":"    if psutil.Process(1).name() \u003d\u003d \"systemd\" and os.path.exists("},{"line_number":48,"context_line":"            SYSTEMCTL_PATH):"},{"line_number":49,"context_line":"        sysdout, sysderr \u003d processutils.execute(SYSTEMCTL_PATH,"},{"line_number":50,"context_line":"                                       \"is-system-running\","},{"line_number":51,"context_line":"                                        check_exit_code\u003d[0, 1])"},{"line_number":52,"context_line":"        print(\"systemd out is %s\" % sysdout)"},{"line_number":53,"context_line":"        for state in VALID_SYSD_STATES:"},{"line_number":54,"context_line":"            if state in sysdout:"},{"line_number":55,"context_line":"                print(\"systemd in valid state\")"},{"line_number":56,"context_line":"                return True"},{"line_number":57,"context_line":"    return False"},{"line_number":58,"context_line":""}],"source_content_type":"text/x-python","patch_set":25,"id":"9fdfeff1_6371e177","line":55,"range":{"start_line":47,"start_character":44,"end_line":55,"end_character":47},"in_reply_to":"9fdfeff1_762a142d","updated":"2019-02-20 09:23:16.000000000","message":"This ensures that systemd is in a state that allows running further units.","commit_id":"7da2e44bb608e93925ce43b173d01cdb46453aa9"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"cfeff589e8b5f3ad0c83c401fdc53455bd73e5b9","unresolved":false,"context_lines":[{"line_number":44,"context_line":""},{"line_number":45,"context_line":"def is_systemd():"},{"line_number":46,"context_line":"    \"\"\"Checks if the host is running systemd\"\"\""},{"line_number":47,"context_line":"    if psutil.Process(1).name() \u003d\u003d \"systemd\" and os.path.exists("},{"line_number":48,"context_line":"            SYSTEMCTL_PATH):"},{"line_number":49,"context_line":"        sysdout, sysderr \u003d processutils.execute(SYSTEMCTL_PATH,"},{"line_number":50,"context_line":"                                       \"is-system-running\","},{"line_number":51,"context_line":"                                        check_exit_code\u003d[0, 1])"},{"line_number":52,"context_line":"        for state in VALID_SYSD_STATES:"},{"line_number":53,"context_line":"            if state in sysdout:"},{"line_number":54,"context_line":"                return True"},{"line_number":55,"context_line":"    return False"},{"line_number":56,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"9fdfeff1_8322c500","line":53,"range":{"start_line":47,"start_character":45,"end_line":53,"end_character":32},"updated":"2019-02-20 09:51:19.000000000","message":"I\u0027m still not entirely convinced that this is something worth doing (it has to be running otherwise the system wouldn\u0027t be running, surely) but this is not a hill I want to die on","commit_id":"b4bfcf1144000cc9ef28fde373f0a1fae60cb84b"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"0dd5c5ec53070bda2ff3fcbfaab87a48db0a8f6f","unresolved":false,"context_lines":[{"line_number":44,"context_line":""},{"line_number":45,"context_line":"def is_systemd():"},{"line_number":46,"context_line":"    \"\"\"Checks if the host is running systemd\"\"\""},{"line_number":47,"context_line":"    if psutil.Process(1).name() \u003d\u003d \"systemd\" and os.path.exists("},{"line_number":48,"context_line":"            SYSTEMCTL_PATH):"},{"line_number":49,"context_line":"        sysdout, sysderr \u003d processutils.execute(SYSTEMCTL_PATH,"},{"line_number":50,"context_line":"                                       \"is-system-running\","},{"line_number":51,"context_line":"                                        check_exit_code\u003d[0, 1])"},{"line_number":52,"context_line":"        for state in VALID_SYSD_STATES:"},{"line_number":53,"context_line":"            if state in sysdout:"},{"line_number":54,"context_line":"                return True"},{"line_number":55,"context_line":"    return False"},{"line_number":56,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"9fdfeff1_b9a13649","line":53,"range":{"start_line":47,"start_character":45,"end_line":53,"end_character":32},"in_reply_to":"9fdfeff1_8322c500","updated":"2019-02-20 12:49:07.000000000","message":"I hope this can be merged without hill based dying for anybody involved. ;-)\n\nBasically while systemd is typically the init process (pid 1) this is not absolutely required. Therefore while the pid 1 check is probably the simplest and quickest for most systems the actual call to systemctl is the way systemd itself provides for finding out. Therefore i\u0027d prefer to keep this.\n\nAnd i also just realized that if clause should better be an or instead of an and, in order to be able to detect non-pid-1 installations.","commit_id":"b4bfcf1144000cc9ef28fde373f0a1fae60cb84b"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"86e36fa36fde57f9dd34201ad1ef482899273647","unresolved":false,"context_lines":[{"line_number":42,"context_line":"SYSTEMCTL_CHECK_PATH \u003d \"/run/systemd/system\""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"def is_systemd():"},{"line_number":46,"context_line":"    \"\"\"Checks if the host is running systemd\"\"\""},{"line_number":47,"context_line":"    if psutil.Process(1).name() \u003d\u003d \"systemd\" or os.path.exists("},{"line_number":48,"context_line":"            SYSTEMCTL_CHECK_PATH):"}],"source_content_type":"text/x-python","patch_set":28,"id":"5fc1f717_cb4e7637","line":45,"updated":"2019-03-22 13:10:04.000000000","message":"Suggestion: cache this so we don\u0027t have to run systemctl every time. It\u0027s not going to change, or at least if it did something very odd would be going on.","commit_id":"7231f7dee10fa8f9e6cead026f6a5ae3f5b15ae4"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"b7c97a5e3c4f31505e66faee85806678c3211d8a","unresolved":false,"context_lines":[{"line_number":42,"context_line":"SYSTEMCTL_CHECK_PATH \u003d \"/run/systemd/system\""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"def is_systemd():"},{"line_number":46,"context_line":"    \"\"\"Checks if the host is running systemd\"\"\""},{"line_number":47,"context_line":"    if psutil.Process(1).name() \u003d\u003d \"systemd\" or os.path.exists("},{"line_number":48,"context_line":"            SYSTEMCTL_CHECK_PATH):"}],"source_content_type":"text/x-python","patch_set":28,"id":"5fc1f717_c8fb6a76","line":45,"in_reply_to":"5fc1f717_86df050b","updated":"2019-03-27 11:29:31.000000000","message":"Note: I just uploaded a follow up change regarding caching the sysd detection.","commit_id":"7231f7dee10fa8f9e6cead026f6a5ae3f5b15ae4"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"9cd28237c8b3d04d2b9182d2dd7342570b64778a","unresolved":false,"context_lines":[{"line_number":42,"context_line":"SYSTEMCTL_CHECK_PATH \u003d \"/run/systemd/system\""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"def is_systemd():"},{"line_number":46,"context_line":"    \"\"\"Checks if the host is running systemd\"\"\""},{"line_number":47,"context_line":"    if psutil.Process(1).name() \u003d\u003d \"systemd\" or os.path.exists("},{"line_number":48,"context_line":"            SYSTEMCTL_CHECK_PATH):"}],"source_content_type":"text/x-python","patch_set":28,"id":"5fc1f717_86df050b","line":45,"in_reply_to":"5fc1f717_cb4e7637","updated":"2019-03-22 14:01:06.000000000","message":"Good point, will consider this.\nPossibly in a follow up change as i really want this finally to be fixed.","commit_id":"7231f7dee10fa8f9e6cead026f6a5ae3f5b15ae4"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"d8881699c69274007d4dbe6fa710416b828702ab","unresolved":false,"context_lines":[{"line_number":48,"context_line":"            SYSTEMCTL_CHECK_PATH):"},{"line_number":49,"context_line":"        sysdout, sysderr \u003d processutils.execute(\"systemctl\","},{"line_number":50,"context_line":"                                       \"is-system-running\","},{"line_number":51,"context_line":"                                        check_exit_code\u003d[0, 1])"},{"line_number":52,"context_line":"        for state in VALID_SYSD_STATES:"},{"line_number":53,"context_line":"            if state in sysdout:"},{"line_number":54,"context_line":"                return True"}],"source_content_type":"text/x-python","patch_set":28,"id":"5fc1f717_92a91525","line":51,"range":{"start_line":51,"start_character":40,"end_line":51,"end_character":63},"updated":"2019-03-27 10:49:26.000000000","message":"Although the docs simply say that the failure return code is \u003e1, in practise this is hardcoded to return 1. Backwards compatibility and systemd CODING_STYLE makes this unlikely to change, so I think this is ok.","commit_id":"7231f7dee10fa8f9e6cead026f6a5ae3f5b15ae4"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"b7c97a5e3c4f31505e66faee85806678c3211d8a","unresolved":false,"context_lines":[{"line_number":48,"context_line":"            SYSTEMCTL_CHECK_PATH):"},{"line_number":49,"context_line":"        sysdout, sysderr \u003d processutils.execute(\"systemctl\","},{"line_number":50,"context_line":"                                       \"is-system-running\","},{"line_number":51,"context_line":"                                        check_exit_code\u003d[0, 1])"},{"line_number":52,"context_line":"        for state in VALID_SYSD_STATES:"},{"line_number":53,"context_line":"            if state in sysdout:"},{"line_number":54,"context_line":"                return True"}],"source_content_type":"text/x-python","patch_set":28,"id":"5fc1f717_a8413ea8","line":51,"range":{"start_line":51,"start_character":40,"end_line":51,"end_character":63},"in_reply_to":"5fc1f717_92a91525","updated":"2019-03-27 11:29:31.000000000","message":"I\u0027ll add that as a comment.","commit_id":"7231f7dee10fa8f9e6cead026f6a5ae3f5b15ae4"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"2478209047ed4250e1e17e73f63a6494affc7dea","unresolved":false,"context_lines":[{"line_number":48,"context_line":"            SYSTEMCTL_CHECK_PATH):"},{"line_number":49,"context_line":"        sysdout, sysderr \u003d processutils.execute(\"systemctl\","},{"line_number":50,"context_line":"                                       \"is-system-running\","},{"line_number":51,"context_line":"                                        check_exit_code\u003d[0, 1])"},{"line_number":52,"context_line":"        for state in VALID_SYSD_STATES:"},{"line_number":53,"context_line":"            if state in sysdout:"},{"line_number":54,"context_line":"                return True"}],"source_content_type":"text/x-python","patch_set":28,"id":"5fc1f717_666b86b6","line":51,"range":{"start_line":51,"start_character":40,"end_line":51,"end_character":63},"in_reply_to":"5fc1f717_a8413ea8","updated":"2019-03-27 13:37:02.000000000","message":"Done","commit_id":"7231f7dee10fa8f9e6cead026f6a5ae3f5b15ae4"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"d8881699c69274007d4dbe6fa710416b828702ab","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        sysdout, sysderr \u003d processutils.execute(\"systemctl\","},{"line_number":50,"context_line":"                                       \"is-system-running\","},{"line_number":51,"context_line":"                                        check_exit_code\u003d[0, 1])"},{"line_number":52,"context_line":"        for state in VALID_SYSD_STATES:"},{"line_number":53,"context_line":"            if state in sysdout:"},{"line_number":54,"context_line":"                return True"},{"line_number":55,"context_line":"    return False"},{"line_number":56,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"5fc1f717_323ce1df","line":53,"range":{"start_line":52,"start_character":0,"end_line":53,"end_character":32},"updated":"2019-03-27 10:49:26.000000000","message":"Can\u0027t we just trim sysdout and do a straight string comparison?","commit_id":"7231f7dee10fa8f9e6cead026f6a5ae3f5b15ae4"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"b7c97a5e3c4f31505e66faee85806678c3211d8a","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        sysdout, sysderr \u003d processutils.execute(\"systemctl\","},{"line_number":50,"context_line":"                                       \"is-system-running\","},{"line_number":51,"context_line":"                                        check_exit_code\u003d[0, 1])"},{"line_number":52,"context_line":"        for state in VALID_SYSD_STATES:"},{"line_number":53,"context_line":"            if state in sysdout:"},{"line_number":54,"context_line":"                return True"},{"line_number":55,"context_line":"    return False"},{"line_number":56,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"5fc1f717_681916d5","line":53,"range":{"start_line":52,"start_character":0,"end_line":53,"end_character":32},"in_reply_to":"5fc1f717_323ce1df","updated":"2019-03-27 11:29:31.000000000","message":"Yep, can do.","commit_id":"7231f7dee10fa8f9e6cead026f6a5ae3f5b15ae4"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"2478209047ed4250e1e17e73f63a6494affc7dea","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        sysdout, sysderr \u003d processutils.execute(\"systemctl\","},{"line_number":50,"context_line":"                                       \"is-system-running\","},{"line_number":51,"context_line":"                                        check_exit_code\u003d[0, 1])"},{"line_number":52,"context_line":"        for state in VALID_SYSD_STATES:"},{"line_number":53,"context_line":"            if state in sysdout:"},{"line_number":54,"context_line":"                return True"},{"line_number":55,"context_line":"    return False"},{"line_number":56,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"5fc1f717_e6d3f65e","line":53,"range":{"start_line":52,"start_character":0,"end_line":53,"end_character":32},"in_reply_to":"5fc1f717_681916d5","updated":"2019-03-27 13:37:02.000000000","message":"Done","commit_id":"7231f7dee10fa8f9e6cead026f6a5ae3f5b15ae4"}]}