[Linux-ha-dev] SFEX resource agent for heartbeat

[Xinwei Hu]

Hi all,

  Another update to sfex resource agent, with following changes:

  . wrap the validate code into one function, so that we support validate-all.
  . correct typo of pid-of ( my bad to do that after diff :( )
  . handle the issue if HA_RSCTMP doesn't exist
  . return OCF_ERR_CONFIGURED for clone.

2008/10/21 Xinwei Hu <hxinwei at gmail.com>:
> Hi all,
>
>  As reviewed by Lars, I cleaned up the code a little more. Here's the
> brief change list since last
> time.
>
>  . resource agent is cleaned up.
>  . syslog is replaced by cl_log
>  . the blocksize in metadata is sector_size now. sfex_init will write
> the sector_size in, then every sfex_daemon will compare the blocksize
> with the sector_size got by it's own.
>  . memory is aligned by _SC_PAGESIZE, and on disk metadata is aligned
> by sector_size
>  . replace all hard coded offset/length with sizeof.
>  . checking the error code of read/write.
>
>  The attached diff contains the updated code, and can be applied to the tip.
>
>  Would you spend some time to see it again ? Thanks.
>
> 2008/10/17 Lars Marowsky-Bree <lmb at suse.de>:
>> On 2008-10-17T01:03:07, Xinwei Hu <hxinwei at gmail.com> wrote:
>>
>>> > This is incorrect. O_DIRECT needs to be page-aligned. valign()
>>> > provides the proper guarantees w/o needing to know the pagesize.
>>> According to "man 2 open", "Under Linux 2.6, alignment to 512-byte
>>> boundaries suffices." em.
>>
>> That's true for sector_size == 512, I think, but it could be larger.
>>
>>> And I also can't find where's valign from.
>>> Will that be OK if I fallback to posix_memalign(ptr,
>>> sysconf(_SC_PAGESIZE), ...) ?
>>
>> My mistake; the call is valloc(), not valign().
>>
>> And, well, I now see it is just a convenient wrapper around
>> posix_memalign(), so this part seems to be right - I stand corrected,
>> even though valloc() is easier to read ;-)
>>
>>> >> +typedef struct sfex_controldata_ondisk {
>>> >> +  uint8_t magic[4];
>>> >> +  uint8_t version[4];
>>> >> +  uint8_t revision[4];
>>> >> +  uint8_t blocksize[8];
>>> >> +  uint8_t numlocks[4];
>>> >> +  uint8_t padding[SFEX_ODIRECT_ALIGNMENT - 24 ];
>>> >> +} sfex_controldata_ondisk;
>>> >
>>> > Why have both? uint32_t etc exist, and the read/write IO functions can
>>> > convert as needed. That saves double definitions.
>>>
>>> I think the double definitions help to make it clear that on disk meta
>>> data contains strings only, while the in-memory meta data contains
>>> integers. :)
>>
>> Well, it still needs to be sector-aligned.
>>
>>> > This simply needs to call valloc() and all is well.
>>> valloc is obsoleted by posix_memalign, as far as I remember :)
>>
>> Ah, well, who can keep track of all this modern stuff ;-)
>>
>>> >> +  if (uname(&u)) {
>>> >> +    SFEX_LOG_ERR("%s: ERROR: %s\n", progname, strerror(errno));
>>> >> +    exit(3);
>>> >> +  }
>>> >
>>> > I'm surprised this worked. Are you sure it shouldn't read "uname() < 0"?
>>>
>>> uname returns 0 on success and -1 on failure.
>>> I don't see difference of if(uname() < 0) and if(uname()) ?
>>
>> It's clearer, I think - apparently it managed to confuse me briefly ;-)
>> I'd write != 0 or < 0.
>>
>>
>> Regards,
>>    Lars
>>
>> --
>> Teamlead Kernel, SuSE Labs, Research and Development
>> SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nürnberg)
>> "Experience is the name everyone gives to their mistakes." -- Oscar Wilde
>>
>> _______________________________________________________
>> Linux-HA-Dev: Linux-HA-Dev at lists.linux-ha.org
>> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
>> Home Page: http://linux-ha.org/
>>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: sfex.diff
Type: text/x-patch
Size: 46430 bytes
Desc: not available
Url : http://lists.community.tummy.com/pipermail/linux-ha-dev/attachments/20081023/bc023fe9/sfex-0001.bin