On 04/18/2012 08:56 AM, Lucas Meneghel Rodrigues wrote: > On Wed, 2012-04-18 at 06:05 -0400, Miroslav Rezanina wrote: >> >> ----- Original Message ----- >>> From: "Amos Kong"<[email protected]> >>> To: "Lin Qing"<[email protected]> >>> Cc: [email protected] >>> Sent: Wednesday, April 18, 2012 11:22:54 AM >>> Subject: Re: [Autotest] [PATCH v2] virt: add "--machine ?" parameter in >>> libvirt command line wrappers >>> >>> On 18/04/12 16:36, Lin Qing wrote: >>>> --machine MACHINE :set the machine type to emulate. >>>> add libvirt command line parameter "--machine ?" wrapper in >>>> libvirt_vm.py. >>>> >>>> Signed-off-by: Qing Lin<[email protected]> >>>> --- >>>> client/virt/libvirt_vm.py | 10 ++++++++++ >>>> 1 files changed, 10 insertions(+), 0 deletions(-) >>>> >>>> diff --git a/client/virt/libvirt_vm.py b/client/virt/libvirt_vm.py >>>> index 355ac5e..6512cb9 100644 >>>> --- a/client/virt/libvirt_vm.py >>>> +++ b/client/virt/libvirt_vm.py >>>> @@ -663,6 +663,12 @@ class VM(virt_vm.BaseVM): >>>> def add_name(help, name): >>>> return " --name '%s'" % name >>>> >>>> + def add_machine_type(help, machine_type): >>>> + if has_option(help, "machine"): >>>> + return " --machine %s" % machine_type >>> >>> >>> qemu-upstream has "-machine" option, "-M" is an alias. >>> qemu-kvm-rhel6 only has "-M" option. >>> >> >>> I would add option check in kvm_vm.py: >> >> Hi Amos, >> depends what is original reason for this patch as virt-install used by >> libvirt_vm supports only --machine option. I think both classes kvm_vm >> and libvirt_vm should provide support for this option. >> >> @Lin: Can you describe use case for this. > > Ok, fair enough, I have applied both patches (the libvirt one required > some editing), so both classes can use the --machine option > > https://github.com/autotest/autotest/commit/d81707c6f88c799298640fe1117e0069a46edd93 > https://github.com/autotest/autotest/commit/a96e6ef7d1415fb166ad60c6da83ab44b4cfd9ed > > Thanks Amos, Miroslav and Quing.
This is a neat feature, I wanted to make small suggestion on...but...wow, you guys are too quick! Remember we're not all in the same time-zone, and I'm morally opposed to reviewing patches while snoring loudly (I'm fighting a cold, normally I don't snore) :P In case it's worthy, my humble comment was: If an option is going into both KVM and LibVirt, it may be worth considering add it to the BaseVM class (with specializations made to sub-classes as necessary). This helps generalization of the interface and reduce code duplication (sometimes). -- Chris Evich, RHCA, RHCE, RHCDS, RHCSS Quality Assurance Engineer e-mail: cevich + `@' + redhat.com o: 1-888-RED-HAT1 x44214 _______________________________________________ Autotest mailing list [email protected] http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
