You do have a point. I will fix both programs to not use flags and
send it to the mailing list.
On Wed, Oct 28, 2009 at 4:23 PM, Michael Goldish <mgoldish@xxxxxxxxxx> wrote:
>
> ----- "Lucas Meneghel Rodrigues" <lmr@xxxxxxxxxx> wrote:
>
>> A new program that evaluates hash strings, intended
>> to help kvm autotest administrators was added, cd_hash.
>>
>> Usage: cd_hash.py [options]
>>
>> Options:
>> -h, --help show this help message and exit
>> -i FILENAME, --iso=FILENAME
>> path to a ISO file whose hash string will be
>> evaluated.
>>
>> This script will calculate:
>>
>> * MD5SUM for the 1st MB of the file
>> * SHA1SUM for the 1st MB of the file
>> * MD5SUM for the whole file
>> * SHA1SUM for the whole file
>>
>> The hashes for the 1st MB are calculated first in the case the
>> user only wants them. This program replaces calc_md5sum_1m.
>>
>> Signed-off-by: Lucas Meneghel Rodrigues <lmr@xxxxxxxxxx>
>> ---
>> client/tests/kvm/calc_md5sum_1m.py | 21 --------------
>> client/tests/kvm/cd_hash.py | 54
>> ++++++++++++++++++++++++++++++++++++
>> 2 files changed, 54 insertions(+), 21 deletions(-)
>> delete mode 100755 client/tests/kvm/calc_md5sum_1m.py
>> create mode 100755 client/tests/kvm/cd_hash.py
>>
>> diff --git a/client/tests/kvm/calc_md5sum_1m.py
>> b/client/tests/kvm/calc_md5sum_1m.py
>> deleted file mode 100755
>> index 153a1e0..0000000
>> --- a/client/tests/kvm/calc_md5sum_1m.py
>> +++ /dev/null
>> @@ -1,21 +0,0 @@
>> -#!/usr/bin/python
>> -"""
>> -Program that calculates the md5sum for the first megabyte of a file.
>> -It's faster than calculating the md5sum for the whole ISO image.
>> -
>> -@copyright: Red Hat 2008-2009
>> -@author: Uri Lublin (uril@xxxxxxxxxx)
>> -"""
>> -
>> -import os, sys
>> -import kvm_utils
>> -
>> -
>> -if len(sys.argv) < 2:
>> - print 'usage: %s <iso-filename>' % sys.argv[0]
>> -else:
>> - fname = sys.argv[1]
>> - if not os.access(fname, os.F_OK) or not os.access(fname,
>> os.R_OK):
>> - print 'bad file name or permissions'
>> - else:
>> - print kvm_utils.hash_file(fname, 1024*1024, method="md5")
>> diff --git a/client/tests/kvm/cd_hash.py
>> b/client/tests/kvm/cd_hash.py
>> new file mode 100755
>> index 0000000..483d71c
>> --- /dev/null
>> +++ b/client/tests/kvm/cd_hash.py
>> @@ -0,0 +1,54 @@
>> +#!/usr/bin/python
>> +"""
>> +Program that calculates several hashes for a given CD image.
>> +
>> +@copyright: Red Hat 2008-2009
>> +"""
>> +
>> +import os, sys, optparse, logging
>> +import common, kvm_utils
>> +from autotest_lib.client.common_lib import logging_config,
>> logging_manager
>> +
>> +
>> +class KvmLoggingConfig(logging_config.LoggingConfig):
>> + def configure_logging(self, results_dir=None, verbose=False):
>> + super(KvmLoggingConfig,
>> self).configure_logging(use_console=True,
>> +
>> verbose=verbose)
>> +
>> +if __name__ == "__main__":
>> + parser = optparse.OptionParser()
>> + parser.add_option('-i', '--iso', type="string", dest="filename",
>> + action='store',
>> + help='path to a ISO file whose hash string will be '
>> + 'evaluated.')
>> +
>> + options, args = parser.parse_args()
>> + filename = options.filename
>
> I think it's a little annoying to have to use -i or --iso.
> Why not just make the iso a mandatory argument that appears in 'args'?
> If I understand correctly, all we need to do is change the last line to
> 'filename = args[0]' and modify the documentation accordingly.
> But then if we only take a single argument, why use OptionParser at all?
> The code will be simpler if we just look at sys.argv directly.
>
> What do you think?
>
> I'm having very similar thoughts about kvm_config.py, whose option parsing
> I think is needlessly complex.
>
>> + logging_manager.configure_logging(KvmLoggingConfig())
>> +
>> + if not filename:
>> + parser.print_help()
>> + sys.exit(1)
>> +
>> + filename = os.path.abspath(filename)
>> +
>> + file_exists = os.path.isfile(filename)
>> + can_read_file = os.access(filename, os.R_OK)
>> + if not file_exists:
>> + logging.critical("File %s does not exist. Aborting...",
>> filename)
>> + sys.exit(1)
>> + if not can_read_file:
>> + logging.critical("File %s does not have read permissions. "
>> + "Aborting...", filename)
>> + sys.exit(1)
>> +
>> + logging.info("Hash values for file %s",
>> os.path.basename(filename))
>> + logging.info("md5 (1m): %s", kvm_utils.hash_file(filename,
>> 1024*1024,
>> +
>> method="md5"))
>> + logging.info("sha1 (1m): %s", kvm_utils.hash_file(filename,
>> 1024*1024,
>> +
>> method="sha1"))
>> + logging.info("md5 (full): %s", kvm_utils.hash_file(filename,
>> +
>> method="md5"))
>> + logging.info("sha1 (full): %s", kvm_utils.hash_file(filename,
>> +
>> method="sha1"))
>> --
>> 1.6.2.5
>>
>> _______________________________________________
>> Autotest mailing list
>> Autotest@xxxxxxxxxxxxxxx
>> http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
> _______________________________________________
> Autotest mailing list
> Autotest@xxxxxxxxxxxxxxx
> http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
>
--
Lucas
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
