Re: [KVM-AUTOTEST PATCH] KVM test: use command line option wrapper functions

[Michael Goldish <mgoldish@redhat.com>]

On 05/19/2010 11:25 AM, Feng Yang wrote:
> Hi, Michael
> 
> Thanks for your patch.
> We plan add "netdev" parameter support in make_qemu_command.  Since you are working on this part. Could you add netdev support in your patch? hopeful netdev can be default supported in make_qemu_command if qemu support it. Thanks very much!

Sure, I'll look into it.

> I think the point of this patch is good and we need this kinds of patch.
> But I think we need not add so many new function.  Especially some function only directly return the string and do nothing more.
> This will increase the function call consumption.
> 
> 
> ----- "Michael Goldish" <mgoldish@redhat.com> wrote:
> 
>> From: "Michael Goldish" <mgoldish@redhat.com>
>> To: autotest@test.kernel.org, kvm@vger.kernel.org
>> Cc: "Michael Goldish" <mgoldish@redhat.com>
>> Sent: Monday, May 17, 2010 9:29:35 PM GMT +08:00 Beijing / Chongqing / Hong Kong / Urumqi
>> Subject: [KVM-AUTOTEST PATCH] KVM test: use command line option wrapper functions
>>
>> In order to support multiple versions of qemu which use different
>> command line
>> options or syntaxes, wrap all command line options in small helper
>> functions,
>> which append text to the command line according to the output of 'qemu
>> -help'.
>>
>> Signed-off-by: Michael Goldish <mgoldish@redhat.com>
>> ---
>>  client/tests/kvm/kvm_vm.py |  198
>> ++++++++++++++++++++++++++++++--------------
>>  1 files changed, 135 insertions(+), 63 deletions(-)
>>
>> diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
>> index 047505a..94bacdf 100755
>> --- a/client/tests/kvm/kvm_vm.py
>> +++ b/client/tests/kvm/kvm_vm.py
>> @@ -186,12 +186,100 @@ class VM:
>>                 nic_model -- string to pass as 'model' parameter for
>> this
>>                 NIC (e.g. e1000)
>>          """
>> -        if name is None:
>> -            name = self.name
>> -        if params is None:
>> -            params = self.params
>> -        if root_dir is None:
>> -            root_dir = self.root_dir
>> +        # Helper function for command line option wrappers
>> +        def has_option(help, option):
>> +            return bool(re.search(r"^-%s(\s|$)" % option, help,
>> re.MULTILINE))
>> +
>> +        # Wrappers for all supported qemu command line parameters.
>> +        # This is meant to allow support for multiple qemu versions.
>> +        # Each of these functions receives the output of 'qemu -help'
>> as a
>> +        # parameter, and should add the requested command line
>> option
>> +        # accordingly.
>> +
>> +        def add_name(help, name):
>> +            return " -name '%s'" % name
> 
> I think we need not add so many new function.  Especially some function only directly return the string and do nothing more.
> This will increase the function call consumption.
> 
>> +
>> +        def add_unix_socket_monitor(help, filename):
>> +            return " -monitor unix:%s,server,nowait" % filename
> Same as above
>> +
>> +        def add_mem(help, mem):
>> +            return " -m %s" % mem
> Same as above
>> +
>> +        def add_smp(help, smp):
>> +            return " -smp %s" % smp
> Same as above.

All these helper functions are meant to be extended and modified in the
future.  They're only there to minimize future effort involved in adding
support for new command line syntaxes.
Right now add_smp() just returns " -smp %s", but in the future we may
have to support different syntaxes for -smp, and then add_smp() will
consult the output of 'qemu -help' and return the proper string.
What do you mean by function call consumption?  I don't think these
functions cause a measurable slowdown, and make_qemu_command() is called
very few times, so this really isn't a concern IMO.