| opendevreview | Merged openstack/os-brick master: nvmeof connector check controller already connected https://review.opendev.org/c/openstack/os-brick/+/811718 | 00:28 |
|---|---|---|
| opendevreview | Merged openstack/cinder master: PureStorage FlashArray: Add active/active replication https://review.opendev.org/c/openstack/cinder/+/829473 | 00:51 |
| opendevreview | Brian Rosmaita proposed openstack/python-cinderclient master: Update requirements minima for Yoga release https://review.opendev.org/c/openstack/python-cinderclient/+/829625 | 03:09 |
| opendevreview | Simon Dodsley proposed openstack/cinder stable/xena: PureStorage FlashArray: Add active/active replication https://review.opendev.org/c/openstack/cinder/+/829630 | 03:58 |
| *** akekane_ is now known as abhishekk | 06:02 | |
| opendevreview | Harsh Ailani proposed openstack/cinder master: [SVf]:Fix multiple lsvdisk calls for GMCV create volume operation. https://review.opendev.org/c/openstack/cinder/+/829110 | 06:30 |
| opendevreview | Tushar Trambak Gite proposed openstack/cinder master: Reset state robustification for volume os-reset_status https://review.opendev.org/c/openstack/cinder/+/773985 | 08:58 |
| opendevreview | Tushar Trambak Gite proposed openstack/cinder master: Reset state robustification for snapshot os-reset_status https://review.opendev.org/c/openstack/cinder/+/804035 | 09:04 |
| opendevreview | Tushar Trambak Gite proposed openstack/cinder master: Reset state robustification for backup os-reset_status https://review.opendev.org/c/openstack/cinder/+/778193 | 09:04 |
| opendevreview | Tushar Trambak Gite proposed openstack/cinder master: Reset state robustification for group-snapshot os-reset_status https://review.opendev.org/c/openstack/cinder/+/804757 | 09:04 |
| opendevreview | Tushar Trambak Gite proposed openstack/cinder master: Reset state robustification for group os-reset_status https://review.opendev.org/c/openstack/cinder/+/804735 | 09:04 |
| opendevreview | yuval proposed openstack/os-brick master: Failure to generate hostnqn in case missing "show-hostnqn" sub-command https://review.opendev.org/c/openstack/os-brick/+/823654 | 09:07 |
| opendevreview | Gorka Eguileor proposed openstack/cinder master: Fix replication in A/A https://review.opendev.org/c/openstack/cinder/+/829664 | 09:41 |
| opendevreview | Gorka Eguileor proposed openstack/cinder master: Pure: Fix replication in A/A https://review.opendev.org/c/openstack/cinder/+/829664 | 09:41 |
| opendevreview | Rajat Dhasmana proposed openstack/python-cinderclient master: Add volume reimage command https://review.opendev.org/c/openstack/python-cinderclient/+/606891 | 09:58 |
| opendevreview | Rajat Dhasmana proposed openstack/cinder master: Support volume re-image https://review.opendev.org/c/openstack/cinder/+/606346 | 10:11 |
| opendevreview | Rajat Dhasmana proposed openstack/cinder master: Support volume re-image https://review.opendev.org/c/openstack/cinder/+/606346 | 10:54 |
| *** dviroel|out is now known as dviroel | 11:00 | |
| opendevreview | Tushar Trambak Gite proposed openstack/cinder master: Reset state robustification for snapshot os-reset_status https://review.opendev.org/c/openstack/cinder/+/804035 | 11:56 |
| raghavendrat | hi geguileo: are you around ? | 12:05 |
| geguileo | raghavendrat: yup | 12:05 |
| raghavendrat | if you get time, it would be great if you can have a look at https://review.opendev.org/c/openstack/cinder/+/824911 | 12:08 |
| raghavendrat | thanks | 12:08 |
| sean-k-mooney | o/ i have a design issue with os-brick that i want to highlight to folks. | 12:11 |
| sean-k-mooney | https://github.com/openstack/os-brick/blob/master/os_brick/executor.py#L69 is highly problematic | 12:11 |
| sean-k-mooney | os-brick is loaded into nova as a lib which means its also eventlet monkey patched | 12:12 |
| sean-k-mooney | so those thread are actully green threads | 12:12 |
| sean-k-mooney | which means if any tread is started in os-brick and it does work and does not yeild it can block the nova-compute agent | 12:12 |
| sean-k-mooney | so we really need to figure out how to remove the use of threading in os-brick or actully use native threads. | 12:13 |
| sean-k-mooney | os-brick really should not be running any long runing agents in teh background anyway but this current design pattern is not something we should continue doing IMO | 12:14 |
| geguileo | sean-k-mooney: currently only 1 connector uses it, the iSCSI, and only when doing multipathing | 13:00 |
| geguileo | sean-k-mooney: it's to do the N connections in parallel, since they are usually slow processes | 13:01 |
| sean-k-mooney | not quite | 13:03 |
| sean-k-mooney | the way the lightos plugin is being propsoed to integrate with nova would use it | 13:04 |
| sean-k-mooney | form nova | 13:04 |
| sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/821606/8/nova/virt/libvirt/volume/lightos.py#58 | 13:06 |
| sean-k-mooney | geguileo: the nvmeof connector was also considering using a agent https://github.com/openstack/os-brick/blob/master/os_brick/initiator/connectors/nvmeof.py#L28 | 13:07 |
| sean-k-mooney | https://github.com/openstack/os-brick/blob/master/os_brick/initiator/connectors/nvmeof.py#L469-L470 | 13:07 |
| sean-k-mooney | https://review.opendev.org/c/openstack/os-brick/+/768576/7 | 13:08 |
| sean-k-mooney | however that was abandoed | 13:08 |
| sean-k-mooney | that was also going to use the same pattern | 13:08 |
| sean-k-mooney | geguileo: in general you are correct isci is the only thing that does it now | 13:09 |
| sean-k-mooney | however the idea of running an agent within an agent or haveign other pluggins start create thread is concerning to say the least | 13:10 |
| sean-k-mooney | geguileo: using a util funciton that detected if os-brick was moneky patche and either uses eventlet.swan or a native tread or process likely would be a better pattern | 13:13 |
| sean-k-mooney | this is not new by any means just we shoudl be very very carful what is run this way | 13:16 |
| sean-k-mooney | and idealy driver shoudl avoid assuming that they can set up backround processes. | 13:16 |
| sean-k-mooney | well backgorund threads | 13:17 |
| rosmaita | sean-k-mooney: the agent is not dead yet: https://review.opendev.org/c/openstack/os-brick/+/802691 | 13:21 |
| sean-k-mooney | ... | 13:21 |
| rosmaita | though it won't be in yoga | 13:22 |
| sean-k-mooney | well that looks like it will be a seperate process? | 13:22 |
| sean-k-mooney | which is what i orginly pushed for in the spec | 13:22 |
| rosmaita | yes | 13:22 |
| sean-k-mooney | ok seperate processes are fine | 13:22 |
| sean-k-mooney | they cant block the eventlet event loop in nova-compute | 13:23 |
| rosmaita | yes, i think they took your concern seriously | 13:23 |
| sean-k-mooney | ya so im fine with the agent in that case | 13:23 |
| rosmaita | cool | 13:24 |
| sean-k-mooney | what im not really clear on is why lightos need to monitor the "db" in a tight loop | 13:24 |
| sean-k-mooney | when we are not making any request to os-brick | 13:24 |
| rosmaita | this is in the nova patch? | 13:25 |
| sean-k-mooney | the loop is in os-brick but the thread is spwaned form nova | 13:25 |
| sean-k-mooney | https://github.com/openstack/os-brick/blob/master/os_brick/initiator/connectors/lightos.py#L141-L182 | 13:26 |
| sean-k-mooney | and spwaned here https://review.opendev.org/c/openstack/nova/+/821606/8/nova/virt/libvirt/volume/lightos.py#58 | 13:26 |
| sean-k-mooney | by calling into os-bicks executor.Thread( | 13:26 |
| rosmaita | for why they need to do it, you'll have to ask yuval | 13:29 |
| rosmaita | for how they do it, there's probably an alternative way to call it on the nova side? | 13:30 |
| sean-k-mooney | well thhe only thing the loopp does other then some manament of a dict is call dsc_connect_volume | 13:31 |
| sean-k-mooney | https://github.com/openstack/os-brick/blob/master/os_brick/initiator/connectors/lightos.py#L107 | 13:31 |
| sean-k-mooney | and thats called in connect_volume https://github.com/openstack/os-brick/blob/5902166149ba46da5f36eb352e62f3361269fb2c/os_brick/initiator/connectors/lightos.py#L277 | 13:32 |
| sean-k-mooney | so i dont know why we would need to key writing the connection info to a tempory file every second | 13:32 |
| sean-k-mooney | i guess it this | 13:33 |
| sean-k-mooney | https://github.com/openstack/os-brick/blob/5902166149ba46da5f36eb352e62f3361269fb2c/os_brick/initiator/connectors/lightos.py#L126 | 13:33 |
| sean-k-mooney | they seam to be movign the file and presumable they have something reading it | 13:33 |
| sean-k-mooney | but that shoudl only be updated really when we connect or disconect a volume | 13:33 |
| sean-k-mooney | ideally we shoudl not have to keep telling lightos every second about all the connection info in the background | 13:35 |
| sean-k-mooney | yuval: ^ if you can expand on why you need to do that it would help | 13:35 |
| sean-k-mooney | rosmaita: correct me if im wrong but connection infomation/attachments shoudl only ever change as a result of a call to cinder or nova's api right so yuval its not clear why this design choice was made | 13:38 |
| rosmaita | sean-k-mooney: i'm not sure, but i think this is kind of second-order connection info that's kept locally so that the cinder/nova connection info doesn't need to change, but i really don't know a lot about their solution | 13:45 |
| rosmaita | cinder-cores: we are down to 2 patches holding up os-brick release, both have one +2 | 13:47 |
| rosmaita | https://review.opendev.org/c/openstack/os-brick/+/823654 | 13:47 |
| rosmaita | https://review.opendev.org/c/openstack/os-brick/+/829588 | 13:47 |
| rosmaita | e0ne eharney geguileo hemna jungleboyj smcginnis whoami-rajat enriquetaso ^^ | 13:47 |
| sean-k-mooney | rosmaita: ack | 13:48 |
| sean-k-mooney | rosmaita: dont worry i dont expect you to know everything going on | 13:49 |
| sean-k-mooney | rosmaita: there wasnt a cinder spec for this no? | 13:49 |
| rosmaita | sean-k-mooney: that's a relief! | 13:49 |
| sean-k-mooney | we did not have one on nova because we taught it was more or less trivial and mainly in os brick | 13:49 |
| sean-k-mooney | but we did not see what it was doing with thread until just now | 13:50 |
| rosmaita | sean-k-mooney: no, we don't require anything other than a bp because usually this kind of thing is straightforward, just implement all the required driver functions | 13:50 |
| rosmaita | and usually use an existing connector | 13:50 |
| sean-k-mooney | ack | 13:50 |
| sean-k-mooney | ya i think im basicaly suffering form a lack of documentaion of how this is ment to work and why | 13:50 |
| sean-k-mooney | which we would normally use the spec to capture partly for docs and partly for review context | 13:51 |
| rosmaita | sean-k-mooney: there was some discussion on the brick connector patch about why they couldn't use the existing nvmeof connector | 13:51 |
| rosmaita | let me see if i can find that | 13:51 |
| jungleboyj | rosmaita Looking. | 13:52 |
| sean-k-mooney | rosmaita: this was the os-brick patch https://review.opendev.org/c/openstack/os-brick/+/821603 | 13:52 |
| sean-k-mooney | rosmaita: ill read back through it | 13:52 |
| rosmaita | it was real early, i think gorka raised the question | 13:53 |
| sean-k-mooney | Show all entries (102 hidden) | 13:53 |
| sean-k-mooney | ah there are just one or two commets :) | 13:53 |
| sean-k-mooney | https://review.opendev.org/c/openstack/os-brick/+/821603/15#message-ad931cf05d8ffad7ad2d081b2761ecf091b01506 | 13:54 |
| rosmaita | yeah, not too much there | 13:54 |
| sean-k-mooney | so they have there own connector to work arond a limiation in nvme-cli speicific nvme/tcp dicovery | 13:56 |
| sean-k-mooney | of the culster nodes? | 13:56 |
| sean-k-mooney | but ya that does not expliaing why this need to be updated but ill wait for yuval to reply and quickly read over some of the other comments | 13:57 |
| TusharTgite | rosmaita: you were right about the policy test https://github.com/openstack/cinder/blob/db0f9974014855a48175ce4ac7abe91288411a84/cinder/tests/unit/policies/test_volume_actions.py#L192 this is reset policy test whcih cause the error for https://review.opendev.org/c/openstack/cinder/+/773985 | 14:06 |
| enriquetaso | 829588 looks good but I need more time regarding 823654 | 14:08 |
| rosmaita | sean-k-mooney: sorry, i stopped paying attention ... yuval is usually good about responding quickly, and I'm sure he will be watching the nova patch carefully | 14:08 |
| sean-k-mooney | rosmaita: thats ok | 14:08 |
| yuval | rosmaita, sean-k-mooney, sorry I was away | 14:09 |
| yuval | I understand the concern about the monitor thread | 14:09 |
| yuval | I can say that we are also inspecting the need of it | 14:10 |
| yuval | since this code is running for few year we dont want to rush for changes | 14:10 |
| rosmaita | yuval: did you get a chance to file a bug about nvme-cli 2.0? if not, i can do it | 14:10 |
| yuval | I have not done that | 14:10 |
| rosmaita | yuval: that's fine, i'll do it ... just wanted to make sure we're not duplicating efforts | 14:13 |
| opendevreview | Merged openstack/os-brick master: Add "known issues" note to yoga os-brick release https://review.opendev.org/c/openstack/os-brick/+/829588 | 14:17 |
| yuval | sean-k-mooney regarding your question - because a certain limitation in the nvme-cli we are using a homemade service that is called discovery-client which connect to discovery-client. this is allow connection to multiple node in different clusters | 14:22 |
| yuval | the monitor thread is making sure live connections data is stored in the discovery-client dir | 14:24 |
| yuval | but today, we no longer think this is needed, I am now testing the removal of it. if all go right, I will update the Patch without it | 14:25 |
| TusharTgite | rosmaita: this code seems right to me https://github.com/openstack/cinder/blob/db0f9974014855a48175ce4ac7abe91288411a84/cinder/tests/unit/policies/test_volume_actions.py#L192 for policy reset state | 14:59 |
| rosmaita | TusharTgite: got a meeting coming up, will take a look in an hour or so | 14:59 |
| rosmaita | thanks for following up on this | 14:59 |
| TusharTgite | rosmaita: thanks just puting patch link for you https://review.opendev.org/c/openstack/cinder/+/773985 | 15:00 |
| rosmaita | ty | 15:01 |
| sean-k-mooney | yuval_: ack that woudl make it much less concerning from a nova point of view | 15:04 |
| opendevreview | Eric Harney proposed openstack/python-cinderclient master: Move tempest requirement to functional env https://review.opendev.org/c/openstack/python-cinderclient/+/766769 | 15:06 |
| TusharTgite | jungleboyj: will you give a review on this one https://review.opendev.org/c/openstack/cinder/+/773985 it's last patch from my side in this series | 15:21 |
| jungleboyj | TusharTgite: Looking. | 15:21 |
| TusharTgite | jungleboyj: sry that was wrong link https://review.opendev.org/c/openstack/cinder-tempest-plugin/+/828229 | 15:22 |
| jungleboyj | TusharTgite: py36/39 are failing. | 15:22 |
| jungleboyj | Ah, ok. | 15:22 |
| jungleboyj | TusharTgite: That looks ok to me. Is there anyone else that should look at it? | 15:24 |
| TusharTgite | jungleboyj: i've add gorka in reviewers lets see | 15:28 |
| opendevreview | Merged openstack/os-brick master: Failure to generate hostnqn in case missing "show-hostnqn" sub-command https://review.opendev.org/c/openstack/os-brick/+/823654 | 19:47 |
| rosmaita | \o/ | 20:00 |
| *** dviroel is now known as dviroel|out | 20:56 | |
| opendevreview | Merged openstack/python-cinderclient master: Move tempest requirement to functional env https://review.opendev.org/c/openstack/python-cinderclient/+/766769 | 22:09 |
| opendevreview | Brian Rosmaita proposed openstack/python-cinderclient master: Update requirements minima for Yoga release https://review.opendev.org/c/openstack/python-cinderclient/+/829625 | 22:47 |
| opendevreview | Merged openstack/cinder master: docs: Add docs for 'RateLimitingMiddleware' https://review.opendev.org/c/openstack/cinder/+/782518 | 23:04 |
| opendevreview | Brian Rosmaita proposed openstack/cinder-specs master: Add zed directory for specs https://review.opendev.org/c/openstack/cinder-specs/+/829828 | 23:11 |
| opendevreview | Sam Morrison proposed openstack/cinder master: Restore volumes from host in the same AZ as target volume https://review.opendev.org/c/openstack/cinder/+/816104 | 23:50 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!