[GE dev] Review for CR 6675570

mpospisil michael.pospisil at sun.com
Mon Jun 1 18:34:20 BST 2009


Hello Harald,
I have had a look at the changes that you suggested and the 
notify_about_kill() function returns two errors. The second error is 
that comproc with id progname_id (a variable to the function) not known 
on host (another variable to the function). The problem with this is 
that the valid values for AN_status defined in sge_answer.h does not 
include anything about such an error.
Do you think it is necessary to create a new entry in the enumeration 
for this error, or can I just use the unknown error value STATUS_EUNKNOWN?

Michael

mpospisil wrote:

>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].
>  
>

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

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



More information about the gridengine-users mailing list