[pve-devel] [RFC installer] add button for renewing dhcp lease

Thomas Lamprecht t.lamprecht at proxmox.com
Thu Jan 31 12:09:40 CET 2019


On 1/31/19 12:09 PM, Oguz Bektas wrote:
> On Thu, Jan 31, 2019 at 08:18:16AM +0100, Thomas Lamprecht wrote:
>> On 1/30/19 3:39 PM, Oguz Bektas wrote:
>>> +    my $dhcp_button = Gtk3::Button->new("Renew DHCP");
>>
>> I'd use something like:
>> Gtk3::Button->new_from_icon_name('view-refresh', 1);
> that actually does look better :D
> another idea i had was to put the button not all the way at the bottom,
> but maybe a smaller button to the right side of the management interface
> combobox. what do you think?

that was my proposal below ("and place it besides the $device_cb"), so yeah
I'll agree with that ;) I.e.,

$devicebox->pack_start($dhcp_button, 0, 0, 0);

>>
>> plus a Tooltip for explain it a bit better for the confused ones:
>> $dhcp_button->set_tooltip_text("Retrigger DHCP request on interface");
>>
>> (above text may not be ideal)
> how about: "Renew DHCP Lease on interface"
>>
>> and place it besides the $device_cb
>>
>>> +    $dhcp_button->signal_connect(clicked => sub {
>>> +	run_command("dhclient -r");
>>> +	run_command("dhclient -v");
>>
>> so why did you chose this exact command set? Even if this is a POC for you,
>> description for why you do things like you do them is wanted.
> 'dhclient -r' will release the current lease and stop the running
> dhclient instance from the pid file. i'll comment it in. (dhclient runs
> by default on the iso, so it needs to be stopped first)
>>
>> why don't you pass only the currently selected NIC to dhclient? Wouldn't that
>> make sense at this stage?
> makes sense, i'll add that too.
>>
>>
>>> +	create_ipconf_view();
>>
>> maybe we should only do this if there where actually changes?
> not sure about this one. i think it doesn't make us lose much to
> just renew the view, instead of binding it to a check for changes which
> might take too long in some situations or environments. what do you
> think?

Yeah, is really easier this way, lets keep it for now, at least then it's
ensured that we do not muss updating or clearing some state.




More information about the pve-devel mailing list