[GE dev] Review for CR 6675570

mpospisil michael.pospisil at sun.com
Fri May 22 16:02:52 BST 2009


Hello Harald,
thanks for looking at my changes, see my comments below.

pollinger wrote:

>Hi Michael,
>
>so you just moved the "answer_list_add()" into the if and the else 
>block. In the "if" block, which is called in case the 
>host_notify_about_kill() function returns an error, you changed the 
>status to "STATUS_EDENIED2HOST". That's all, right?
>
>
>There are three things:
>
>1. In the if statement, please write to what you are comparing. This 
>would be "if (.... != 0)" in this case. But:
>  
>
OK

>2. The host_notify_about_kill() function returns two different error 
>values. Shouldn't both be forwarded to the answer list? I don't know if 
>there are two different states defined that would be suitable to reflect 
>the return values of nost_notify_about_kill(), but if there are, it 
>would be a good idea to use them.
>  
>
I did not know that host_notify_about_kill() is returning two different 
errors. I will have a look if there are different states that could 
reflect the error message received from host_notify_about_kill() 
function. This sounds like a good idea.

>3. Shouldn't the ANSWER_QUALITY in the if block be ANSWER_QUALITY_ERROR?
>What effect does the ANSWER_QUALITY have?
>  
>
Of course it should be ANSWER_QUALITY_ERROR, this is a problem of 
copying the other line and just changing the status.

I will try to make these modifications and resend my updates.

Michael

>Regards,
>Harald
>
>
>
>
>
>Index: sge_host_qmaster.c
>===================================================================
>RCS file: /cvs/gridengine/source/daemons/qmaster/sge_host_qmaster.c,v
>retrieving revision 1.127
>diff -u -p -r1.127 sge_host_qmaster.c
>--- sge_host_qmaster.c	19 May 2009 11:32:06 -0000	1.127
>+++ sge_host_qmaster.c	21 May 2009 13:57:07 -0000
>@@ -1128,12 +1128,13 @@ notify(sge_gdi_ctx_class_t *ctx, lListEl
>        if (host_notify_about_kill(ctx, lel, kill_jobs)) {
>           INFO((SGE_EVENT, MSG_COM_NONOTIFICATION_SSS, action_str,
>                 (execd_alive ? "" : MSG_OBJ_UNKNOWN), hostname));
>+         answer_list_add(&(task->answer_list), SGE_EVENT, 
>STATUS_EDENIED2HOST, ANSWER_QUALITY_INFO);
>        } else {
>           INFO((SGE_EVENT, MSG_COM_NOTIFICATION_SSS, action_str,
>                 (execd_alive ? "" : MSG_OBJ_UNKNOWN), hostname));
>+         answer_list_add(&(task->answer_list), SGE_EVENT, STATUS_OK, 
>ANSWER_QUALITY_INFO);
>        }
>-      DPRINTF((SGE_EVENT));
>-      answer_list_add(&(task->answer_list), SGE_EVENT, STATUS_OK, 
>ANSWER_QUALITY_INFO);
>+      DPRINTF((SGE_EVENT));
>     }
>
>     if (kill_jobs) {
>
>------------------------------------------------------
>http://gridengine.sunsource.net/ds/viewMessage.do?dsForumId=39&dsMessageId=198252
>
>To unsubscribe from this discussion, e-mail: [dev-unsubscribe at gridengine.sunsource.net].
>  
>

------------------------------------------------------
http://gridengine.sunsource.net/ds/viewMessage.do?dsForumId=39&dsMessageId=198285

To unsubscribe from this discussion, e-mail: [dev-unsubscribe at gridengine.sunsource.net].



More information about the gridengine-users mailing list