<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/1609/">https://reviewboard.asterisk.org/r/1609/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">The only issue that I have is this appears to introduce two different behaviors with how penalties are set, based on whether or not you have the queues stored in realtime.
* If realtime is not being used, then you can either specify a queue name or not. If you specify a queue name, it will update the specified queue, otherwise it will update all queues
* If realtime is being used, you have to specify a queue name, and there is no way to update the penalty on all queues at once
I'm not sure if this behavior should be different.</pre>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://reviewboard.asterisk.org/r/1609/diff/1/?file=22116#file22116line5442" style="color: black; font-weight: bold; text-decoration: underline;">/branches/10/apps/app_queue.c</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">static int set_member_paused(const char *queuename, const char *interface, const char *reason, int paused)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">5442</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">while</span> <span class="p">((</span><span class="n">q</span> <span class="o">=</span> <span class="n">ao2_t_iterator_next</span><span class="p">(</span><span class="o">&</span><span class="n">queue_iter</span><span class="p">,</span> <span class="s">"Iterate through queues"</span><span class="p">)))</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">You may want to consider structuring this a bit differently, as right now if a user specifies a queue name, you have to iterate over every queue in the container instead of using the hash lookup. You may want to go with something more like the following:
if the queue name is specified:
use ao2_find and update the queue, or error if no queue found
otherwise:
update all queues
Since this may introduce some code duplication with the realtime method below, you may want to defer some of the actual work of updating the penalty and sending out the manager event to a private method.</pre>
</div>
<br />
<p>- mjordan</p>
<br />
<p>On December 6th, 2011, 10:06 a.m., jrose wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers and irroot.</div>
<div>By jrose.</div>
<p style="color: grey;"><i>Updated Dec. 6, 2011, 10:06 a.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">r325483 caused Asterisk to lose the ability to set penalty on an interface without specifying the queue, and worse than that also introduces a segfault from trying to. The segfault only occurs when penalty is set using the queue set penalty command in CLI and not when using QueuePenalty from the manager, however the manager action wouldn't work without specifying a queue either, it just wouldn't crash Asterisk.
This patch attempts to fix that by changing irroot's realtime patch so that the new method for picking the queue being acted on is only employed if the method used prior to that can't find a queue to work on. If no queuename is specified, this is fine since the crash would only occur if there was a queue attached to the interface anyway. This doesn't seem to cause any problems in normal queues, but I haven't tested it with realtime queues.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Various uses of queue set penalty and QueuePenalty from manager. Also, I'm currently working on an automated test for this within the Asterisk testsuite.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/branches/10/apps/app_queue.c <span style="color: grey">(346971)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1609/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>