Code review comment for lp://qastaging/~liuyq0307/lava-lab/fast-model-network

Revision history for this message
Ryan Harkin (ryanharkin) wrote :

On 26 June 2013 03:13, Yongqin Liu <email address hidden> wrote:
> Hi, Ryan
>
> Thanks for your review.
>
>> On 25 June 2013 12:03, Yongqin Liu <email address hidden> wrote:
>> > Hi, Ryan
>> >
>> > Could you help to check if there is any problem with the
>> lava/fastmodels/FMNetwork script please?
>> > I have tested it on LAVA manually, and it worked.
>> > created two interfaces, and the fast model instance can use them to ping
>> locally and internet when set the dns correctly.
>> > But since I am not sure I have the correct understanding about the bridge
>> usage,
>> > so please tell me if there is anything not good.
>> >
>>
>> OK, I've had a look and it seems generally OK. But without testing it,
>> I can't be sure.
>
> I have tested it on LAVA, and it worked.

Great :-) That's what counts.

>
>>
>> I doubt it's important for these virtual machines, but I added a check
>> to my FMNetwork script so that it doesn't start if it's already
>> started and doesn't stop if not running.
> On LAVA, we use the static ip for the bridge interface directly, not use the dhclient command.
> so I think we can't check the status by checking the existence of the dhclient process.
>
> And on LAVA, this script will be run only one time when the node boots up,
> we will not run it many times. that means we will not run the start again after the node booted up,
> and will not run the stop until the node shutdown.
>
> So I think it's OK to not have the status check.

Agreed. I thought this was how it worked, so that's OK with me.

>
> Thanks,
> Yongqin Liu
>> I also added a Status() function, but that's not important.
>>
>>
>> > Thanks,
>> > Yongqin Liu
>> > --
>> > https://code.launchpad.net/~liuyq0307/lava-lab/fast-model-
>> network/+merge/171271
>> > You are requested to review the proposed merge of lp:~liuyq0307/lava-lab
>> /fast-model-network into lp:lava-lab.
> --
> https://code.launchpad.net/~liuyq0307/lava-lab/fast-model-network/+merge/171271
> You are requested to review the proposed merge of lp:~liuyq0307/lava-lab/fast-model-network into lp:lava-lab.

« Back to merge proposal