On Tue, Aug 14, 2012 at 2:32 AM, Alex Jia <[email protected]> wrote:
> Ping.

I see. Ok, I will rebase those patches against next today. Next time,
please don't send pull requests to my private fork, as they're likely
to be forgotten.

Thanks,

Lucas

> --
> Regards,
> Alex
>
>
> ----- Original Message -----
> From: "Alex Jia" <[email protected]>
> To: "Lucas Meneghel Rodrigues" <[email protected]>
> Cc: "autotest-kernel" <[email protected]>
> Sent: Thursday, August 9, 2012 5:22:30 PM
> Subject: Re: [Autotest] virt_v2v patches dropped from next
>
> On 08/09/2012 04:39 PM, Alex Jia wrote:
>> On 08/07/2012 08:51 PM, Lucas Meneghel Rodrigues wrote:
>>> On Tue, 2012-08-07 at 13:55 +0800, Wayne Sun wrote:
>>>> On 08/07/2012 01:34 PM, Alex Jia wrote:
>>>>> On 08/07/2012 06:25 AM, Lucas Meneghel Rodrigues wrote:
>>
>> A new pull request:https://github.com/autotest/autotest/pull/508
>
> @lmr, I should commit a pull request to your repo rather than autotest,
> so close this pull request.
>
> It should be https://github.com/lmr/autotest/pull/3
>
>>
>> For v3 patchset, we done some small changes such as sharing SSH
>> creation method for
>> both server and client side, and fixed a parameter match issue in the
>> constructor
>> function of the class LinuxVMCheck. Please review our v3 patchset,
>> thanks in advance!
>>
>> Regards,
>> Alex
>>
>>>>>> Hi Alex and Wayne:
>>>>>>
>>>>>> I've dropped your virt_v2v patchset (12 patches) from next,
>>>>>> considering
>>>>>> they were causing the following regression on next:
>>>>>>
>>>>>> 08/06 16:49:24 DEBUG|  parallel:0030| UnhandledTestError:
>>>>>> Unhandled ImportError: No module named server.hosts.ssh_host
>>>>>> 08/06 16:49:24 DEBUG|  parallel:0030| Traceback (most recent call
>>>>>> last):
>>>>>> 08/06 16:49:24 DEBUG|  parallel:0030|   File
>>>>>> "/usr/local/autotest/parallel.py", line 18, in fork_start
>>>>>> 08/06 16:49:24 DEBUG|  parallel:0030|     l()
>>>>>> 08/06 16:49:24 DEBUG|  parallel:0030|   File
>>>>>> "/usr/local/autotest/job.py", line 529, in<lambda>
>>>>>> 08/06 16:49:24 DEBUG|  parallel:0030|     l = lambda :
>>>>>> test.runtest(self, url, tag, args, dargs)
>>>>>> 08/06 16:49:24 DEBUG|  parallel:0030|   File
>>>>>> "/usr/local/autotest/test.py", line 115, in runtest
>>>>>> 08/06 16:49:24 DEBUG|  parallel:0030|
>>>>>> job.sysinfo.log_after_each_iteration)
>>>>>> 08/06 16:49:24 DEBUG|  parallel:0030|   File
>>>>>> "/usr/local/autotest/shared/test.py", line 929, in runtest
>>>>>> 08/06 16:49:24 DEBUG|  parallel:0030|     exec ('import %s' %
>>>>>> modulename, local_namespace, global_namespace)
>>>>>> 08/06 16:49:24 DEBUG|  parallel:0030|   File "<string>", line 1,
>>>>>> in<module>
>>>>>> 08/06 16:49:24 DEBUG|  parallel:0030|   File
>>>>>> "/usr/local/autotest/tests/kvm/kvm.py", line 1, in<module>
>>>>>> 08/06 16:49:24 DEBUG|  parallel:0030|     from
>>>>>> autotest.client.virt import virt_test
>>>>>> 08/06 16:49:24 DEBUG|  parallel:0030|   File
>>>>>> "/usr/local/autotest/virt/virt_test.py", line 4, in<module>
>>>>>> 08/06 16:49:24 DEBUG|  parallel:0030|     import virt_utils,
>>>>>> virt_env_process
>>>>>> 08/06 16:49:24 DEBUG|  parallel:0030|   File
>>>>>> "/usr/local/autotest/virt/virt_env_process.py", line 6,
>>>>>> in<module>
>>>>>> 08/06 16:49:24 DEBUG|  parallel:0030|     import virt_remote,
>>>>>> virt_v2v, ovirt
>>>>>> 08/06 16:49:24 DEBUG|  parallel:0030|   File
>>>>>> "/usr/local/autotest/virt/virt_v2v.py", line 10, in<module>
>>>>>> 08/06 16:49:24 DEBUG|  parallel:0030|     from
>>>>>> autotest.server.hosts.ssh_host import SSHHost
>>>>>> 08/06 16:49:24 DEBUG|  parallel:0030| ImportError: No module named
>>>>>> server.hosts.ssh_host
>>>>>>
>>>>>> Bottom line, you'll have to find another way to set up the ssh key
>>>>>> in
>>>>>> virt_v2v, anything out of client or shared modules are out of
>>>>>> limits for
>>>>>> code that is supposed to run on a client.
>>>>> There are some methods are common for client and server such as
>>>>> setup a ssh connection,
>>>>> if we want to reuse codes rather than implementing them in client
>>>>> again, IMHO, we should
>>>>> abstract them as a module/method and put them into utils directory,
>>>>> @lmr, what do you think?
>>> Not entirely necessary, as Wayne has pointed out.
>>>
>>>>> in addition, which's module right place? utils/common.py?
>>>> I think the directory should be client/shared where storing libraries
>>>> which are common
>>>> to both client and server.
>>> Yes, that's the idea.
>>>
>>>>>> I have an up to date version of the patches on my tree:
>>>>>>
>>>>>> https://github.com/lmr/autotest/tree/v2v
>>>>>>
>>>>>> So you can recover the patches and fix out those 2 bugs (one,
>>>>>> wrong
>>>>>> arguments passed to one class constructor and this problem just
>>>>>> mentioned here. Please recover the patches, fix out those bugs
>>>>>> then
>>>>>> re-send a pull request.
>>>>> If we can confirm above question, it's okay for us to fix them asap.
>>>>> However, I'm worrying about it will have effect on all of server
>>>>> codes.
>>>> Yeah,  the change will including move following functions:
>>>> 1. setup_ssh and setup_ssh_key functions in server/hosts/ssh_host.py,
>>>> 2. get_public_key in server/base_utils.py
>>> Wayne's assessment is correct. In fact, there is another option I was
>>> considering - using aexpect and ssh-copy-id to perform the setup. This
>>> is a more self contained, better approach in the long term.
>>>
>>> But moving setup_ssh, setup_ssh_key from ssh_host to utils might as well
>>> do the job.
>>>
>>
>> _______________________________________________
>> Autotest-kernel mailing list
>> [email protected]
>> https://www.redhat.com/mailman/listinfo/autotest-kernel
>
> _______________________________________________
> Autotest-kernel mailing list
> [email protected]
> https://www.redhat.com/mailman/listinfo/autotest-kernel
>
> _______________________________________________
> Autotest-kernel mailing list
> [email protected]
> https://www.redhat.com/mailman/listinfo/autotest-kernel



-- 
Lucas

_______________________________________________
Autotest-kernel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/autotest-kernel

Reply via email to