Hi,

Thanks.

>Hi, 
>
>On Wed, Mar 19, 2008 at 12:59:30PM +0900, HIDEO YAMAUCHI wrote: 
>> Hi, 
>> 
>> I send the patch which I revised. 
>
>Thanks. A few notes/questions: 
>
>1. The backup mode processing seems to be overly complicated. 
>The check if this attribute is set is invoked six times and then 
>different actions taken. Is that necessary? Furthermore, there is 
>only one check if the instance is in backup mode. It somehow 
>defies logic. For example: 
>
>"STARTED") 
>if is_clear_backupmode_set; then 
>output=`dbasql dbmount` 
>else 
>output=`dbasql dbmount dbopen` 
>fi 
>;; 
>
>I don't understand why the instance is treated in two different 
>ways here. Even if it has to be so, it shouldn't depend on the 
>attribute setting but on the actual state of affairs, i.e. is the 
>instance in the backup mode or not. In short, the processing 
>became extremely complex and very hard to follow. It has to be 
>simplified. 
>
>Shouldn't it be something like this: 
>
>if instance is in backup mode 
>end the backup mode 
>fi 
>continue with start/mount as usual 

1)Only mount was necessary to examine a backup mode.
  In addition, I did not want to affect handling of old oracle-RA, and 'if 
is_clear_backupmode_set'
has increased.
  Possibly I may give more revisions to start method.
  I think about a revision a little more.

>2. I guess that alter system checkpoint is not necessary before 
>shutdown immediate. 

2)I confirm it.
  In the case of the unnecessary, I delete it.

>
>3. is_clear_backup_mode and is_backup_mode are somewhat 
>confusing. Should probably be renamed, something like: 
>is_clear_backupmode_set and is_instance_in_backup_mode. Still 
>verbose, but at least easier to understand. 

3)I understood it. 
  Let's change a name as you say.

>
>4. This looks somewhat strange: 
>
>count="`dbasql db_backup_mode | tr -d COUNT`" 
>
>If I understood your intention correctly, it would be more clear: 
>
>count="`dbasql db_backup_mode | sed 's/COUNT//'`" 


4)I understood it. 
  I change it for the use of sed.

>
>5. Things around backup mode are too verbose. I'd remove quite a 
>few ocf_log statements. 

5)Because start was not carried out very much, start increased the log.
  Please revise it if worried about numerousness of the log.

>
>> Regards, 
>> Hideo Yamauchi. 
>
>Cheers, 
>
>Dejan 


Please wait, while I will send the patch which revised later.

Regards, 
Hideo Yamauchi. 



_______________________________________________________
Linux-HA-Dev: [email protected]
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/

Reply via email to