LGTM

On Fri, May 7, 2010 at 6:53 AM, Lucas Meneghel Rodrigues <[email protected]> 
wrote:
>  * Makefile: Get rid of -static flag on disktest's Makefile (fixing Subrata's
>   problem with it running inside F12 KVM guests)
>  * C program: Fix a compiler warning by using the right format string on 
> printf
>  * Remove an unneeded Makefile at the top of the test's module
>  * Python module: Added docstring documentation
>  * Python module: Made tests for 'is None' explicitly as per coding style
>  * Python module: Log the disktest commands we're running
>
> Signed-off-by: Lucas Meneghel Rodrigues <[email protected]>
> ---
>  client/tests/disktest/Makefile       |    2 -
>  client/tests/disktest/disktest.py    |   62 
> ++++++++++++++++++++++++++--------
>  client/tests/disktest/src/Makefile   |    2 +-
>  client/tests/disktest/src/disktest.c |    4 +-
>  4 files changed, 51 insertions(+), 19 deletions(-)
>  delete mode 100644 client/tests/disktest/Makefile
>
> diff --git a/client/tests/disktest/Makefile b/client/tests/disktest/Makefile
> deleted file mode 100644
> index b93f66c..0000000
> --- a/client/tests/disktest/Makefile
> +++ /dev/null
> @@ -1,2 +0,0 @@
> -disktest: disktest.c
> -       cc disktest.c -Wall -D_FILE_OFFSET_BITS=64 -D _GNU_SOURCE -static -o 
> disktest
> diff --git a/client/tests/disktest/disktest.py 
> b/client/tests/disktest/disktest.py
> index f432dc3..c46a2b6 100644
> --- a/client/tests/disktest/disktest.py
> +++ b/client/tests/disktest/disktest.py
> @@ -4,50 +4,84 @@ from autotest_lib.client.common_lib import error
>
>
>  class disktest(test.test):
> -    version = 1
> +    """
> +    Autotest module for disktest.
> +
> +    Pattern test of the disk, using unique signatures for each block and each
> +    iteration of the test. Designed to check for data corruption issues in 
> the
> +    disk and disk controller.
> +
> +    It writes 50MB/s of 500KB size ops.
> +
> +   �...@author: Martin Bligh ([email protected])
> +    """
> +    version = 2
>     preserve_srcdir = True
>
>     def setup(self):
> +        """
> +        Compiles disktest.
> +        """
>         os.chdir(self.srcdir)
>         utils.system('make clean')
>         utils.system('make')
>
>
>     def initialize(self):
> +        """
> +        Verifies if we have gcc to compile disktest.
> +        """
>         self.job.require_gcc()
>
>
>     def test_one_disk_chunk(self, disk, chunk):
> -        logging.info("testing %d MB files on %s in %d MB memory",
> -                     self.chunk_mb, disk, self.memory_mb)
> -        cmd = "%s/disktest -m %d -f %s/testfile.%d -i -S" % \
> -                                (self.srcdir, self.chunk_mb, disk, chunk)
> +        """
> +        Tests one part of the disk by spawning a disktest instance.
> +
> +       �...@param disk: Directory (usually a mountpoint).
> +       �...@param chunk: Portion of the disk used.
> +        """
> +        logging.info("Testing %d MB files on %s in %d MB memory, chunk %s",
> +                     self.chunk_mb, disk, self.memory_mb, chunk)
> +        cmd = ("%s/disktest -m %d -f %s/testfile.%d -i -S" %
> +               (self.srcdir, self.chunk_mb, disk, chunk))
> +        logging.debug("Running '%s'", cmd)
>         p = subprocess.Popen(cmd, shell=True)
>         return(p.pid)
>
>
> -    def execute(self, disks = None, gigabytes = None,
> -                chunk_mb = utils.memtotal() / 1024):
> -        os.chdir(self.srcdir)
> +    def run_once(self, disks=None, gigabytes=None, chunk_mb=None):
> +        """
> +        Runs one iteration of disktest.
>
> -        if not disks:
> +       �...@param disks: List of directories (usually mountpoints) to be 
> passed
> +                to the test.
> +       �...@param gigabytes: Disk space that will be used for the test to 
> run.
> +       �...@param chunk_mb: Size of the portion of the disk used to run the 
> test.
> +                Cannot be larger than the total amount of free RAM.
> +        """
> +        os.chdir(self.srcdir)
> +        if chunk_mb is None:
> +            chunk_mb = utils.memtotal() / 1024
> +        if disks is None:
>             disks = [self.tmpdir]
> -        if not gigabytes:
> -            free = 100       # cap it at 100GB by default
> +        if gigabytes is None:
> +            free = 100 # cap it at 100GB by default
>             for disk in disks:
>                 free = min(utils.freespace(disk) / 1024**3, free)
>             gigabytes = free
> -            logging.info("resizing to %s GB", gigabytes)
> +            logging.info("Resizing to %s GB", gigabytes)
>             sys.stdout.flush()
>
>         self.chunk_mb = chunk_mb
>         self.memory_mb = utils.memtotal()/1024
>         if self.memory_mb > chunk_mb:
> -            e_msg = "Too much RAM (%dMB) for this test to work" % 
> self.memory_mb
> -            raise error.TestError(e_msg)
> +            raise error.TestError("Too much RAM (%dMB) for this test to 
> work" %
> +                                  self.memory_mb)
>
>         chunks = (1024 * gigabytes) / chunk_mb
>
> +        logging.info("Total of disk chunks that will be used: %s", chunks)
>         for i in range(chunks):
>             pids = []
>             for disk in disks:
> diff --git a/client/tests/disktest/src/Makefile 
> b/client/tests/disktest/src/Makefile
> index 40c4d7e..8a20c57 100644
> --- a/client/tests/disktest/src/Makefile
> +++ b/client/tests/disktest/src/Makefile
> @@ -1,4 +1,4 @@
> -CFLAGS = -O2 -Wall -D_FILE_OFFSET_BITS=64 -D _GNU_SOURCE -static
> +CFLAGS = -O2 -Wall -D_FILE_OFFSET_BITS=64 -D _GNU_SOURCE
>  TARGET = disktest
>
>
> diff --git a/client/tests/disktest/src/disktest.c 
> b/client/tests/disktest/src/disktest.c
> index c7659e9..d2dbec7 100644
> --- a/client/tests/disktest/src/disktest.c
> +++ b/client/tests/disktest/src/disktest.c
> @@ -116,13 +116,13 @@ int verify_block(int fd, unsigned int block, struct 
> pattern *buffer, char *err)
>                        }
>                }
>                if (sector_errors)
> -                       printf("Block %d (from %d to %d) sector %08x has 
> wrong sector number %08x (%d/%d) filename %s %s\n",
> +                       printf("Block %d (from %d to %d) sector %08x has 
> wrong sector number %08x (%d/%lu) filename %s %s\n",
>                                        block, start_block, start_block+blocks,
>                                        sector, read_sector,
>                                        sector_errors, PATTERN_PER_SECTOR,
>                                        filename, err);
>                if (signature_errors)
> -                       printf("Block %d (from %d to %d) sector %08x 
> signature is %08x should be %08x (%d/%d) filename %s %s\n",
> +                       printf("Block %d (from %d to %d) sector %08x 
> signature is %08x should be %08x (%d/%lu) filename %s %s\n",
>                                block, start_block, start_block+blocks,
>                                sector, read_signature, signature,
>                                signature_errors, PATTERN_PER_SECTOR,
> --
> 1.7.0.1
>
>
_______________________________________________
Autotest mailing list
[email protected]
http://test.kernel.org/cgi-bin/mailman/listinfo/autotest

Reply via email to