{"action":"created","issue":{"url":"https://api.github.com/repos/pmem/nvml/issues/1344","repository_url":"https://api.github.com/repos/pmem/nvml","labels_url":"https://api.github.com/repos/pmem/nvml/issues/1344/labels{/name}","comments_url":"https://api.github.com/repos/pmem/nvml/issues/1344/comments","events_url":"https://api.github.com/repos/pmem/nvml/issues/1344/events","html_url":"https://github.com/pmem/nvml/pull/1344","id":185872129,"number":1344,"title":"common: add device dax support","user":{"login":"pbalcer","id":8775610,"avatar_url":"https://avatars.githubusercontent.com/u/8775610?v=3","gravatar_id":"","url":"https://api.github.com/users/pbalcer","html_url":"https://github.com/pbalcer","followers_url":"https://api.github.com/users/pbalcer/followers","following_url":"https://api.github.com/users/pbalcer/following{/other_user}","gists_url":"https://api.github.com/users/pbalcer/gists{/gist_id}","starred_url":"https://api.github.com/users/pbalcer/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pbalcer/subscriptions","organizations_url":"https://api.github.com/users/pbalcer/orgs","repos_url":"https://api.github.com/users/pbalcer/repos","events_url":"https://api.github.com/users/pbalcer/events{/privacy}","received_events_url":"https://api.github.com/users/pbalcer/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":10,"created_at":"2016-10-28T08:58:11Z","updated_at":"2016-11-02T15:48:05Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/pmem/nvml/pulls/1344","html_url":"https://github.com/pmem/nvml/pull/1344","diff_url":"https://github.com/pmem/nvml/pull/1344.diff","patch_url":"https://github.com/pmem/nvml/pull/1344.patch"},"body":"<!-- Reviewable:start -->\n\nThis change is [<img src=\"https://reviewable.io/review_button.svg\" height=\"34\" align=\"absmiddle\" alt=\"Reviewable\"/>](https://reviewable.io/reviews/pmem/nvml/1344)\n\n<!-- Reviewable:end -->\n"},"comment":{"url":"https://api.github.com/repos/pmem/nvml/issues/comments/257905995","html_url":"https://github.com/pmem/nvml/pull/1344#issuecomment-257905995","issue_url":"https://api.github.com/repos/pmem/nvml/issues/1344","id":257905995,"user":{"login":"pbalcer","id":8775610,"avatar_url":"https://avatars.githubusercontent.com/u/8775610?v=3","gravatar_id":"","url":"https://api.github.com/users/pbalcer","html_url":"https://github.com/pbalcer","followers_url":"https://api.github.com/users/pbalcer/followers","following_url":"https://api.github.com/users/pbalcer/following{/other_user}","gists_url":"https://api.github.com/users/pbalcer/gists{/gist_id}","starred_url":"https://api.github.com/users/pbalcer/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pbalcer/subscriptions","organizations_url":"https://api.github.com/users/pbalcer/orgs","repos_url":"https://api.github.com/users/pbalcer/repos","events_url":"https://api.github.com/users/pbalcer/events{/privacy}","received_events_url":"https://api.github.com/users/pbalcer/received_events","type":"User","site_admin":false},"created_at":"2016-11-02T15:48:05Z","updated_at":"2016-11-02T15:48:05Z","body":"\n\n\n\nReview status: 35 of 114 files reviewed at latest revision, 19 unresolved discussions.\n\n---\n\n*[src/common/mmap.h, line 63 at r5](https://reviewable.io:443/reviews/pmem/nvml/1344#-KV_8Su8WSFbr0cqxDG2:-KV_iAf5nD6CycKLQzCM:b5a9ugq) ([raw file](https://github.com/pmem/nvml/blob/0ca84a6543502b3f5ae18fa4887ed20d0ffc012c/src/common/mmap.h#L63)):*\n<details><summary><i>Previously, krzycz (Krzysztof Czurylo) wrote…</i></summary>\n> I think it would be easier to just modify these macros to take additional argument \"is_dax\", instead of adding \"POOL_RANGE_*\" macros.\n</details>\nI can imagine a situation in which you would want to call the RANGE_* macro that actually does something.\n\n---\n\n*[src/common/mmap.h, line 85 at r5](https://reviewable.io:443/reviews/pmem/nvml/1344#-KV_8jDpFW5K5e5GBxFk:-KV_i7nVZNo-7Qa1_WfO:bj16ap) ([raw file](https://github.com/pmem/nvml/blob/0ca84a6543502b3f5ae18fa4887ed20d0ffc012c/src/common/mmap.h#L85)):*\n<details><summary><i>Previously, krzycz (Krzysztof Czurylo) wrote…</i></summary>\n> Would it make sense to add \"is_dax\" to the pool descriptor? So, we can use it like `pop->is_dax`?\n</details>\nPool being mapped on a device dax is a detail that should not be exposed above the \"set.c\" layer. POOL_RANGE_* macros do a decent job at making it easy to use. \n\n---\n\n*[src/common/set.c, line 613 at r5](https://reviewable.io:443/reviews/pmem/nvml/1344#-KV_CbguXrkm8ZGmvVGa:-KV_iKZ02m21SrckueI9:b-896fix) ([raw file](https://github.com/pmem/nvml/blob/0ca84a6543502b3f5ae18fa4887ed20d0ffc012c/src/common/set.c#L613)):*\n<details><summary><i>Previously, krzycz (Krzysztof Czurylo) wrote…</i></summary>\n> What about a few additional test cases for util_poolset_parse/util_poolset_size ?\n</details>\nDone.\n\n---\n\n*[src/common/set.h, line 68 at r2](https://reviewable.io:443/reviews/pmem/nvml/1344#-KV9gtpSlFSAvTWuHSF4:-KV_iM2Wh-qh7k6gGcL7:b-cqauag) ([raw file](https://github.com/pmem/nvml/blob/ba1d27f8119c9ffb8c37698ef356a33652119c9a/src/common/set.h#L68)):*\n<details><summary><i>Previously, marcinslusarz (Marcin Ślusarz) wrote…</i></summary>\n> \"is_device\" / \"is_device_dax\" ?\n</details>\nIs the name ambiguous?\n\n---\n\n\n*Comments from [Reviewable](https://reviewable.io:443/reviews/pmem/nvml/1344)*\n<!-- Sent from Reviewable.io -->\n"}}
{"actor":{"display_login":"pbalcer"}}