Re: [PATCH 2/3] tools/hotplug: Fix dhcpd symlink removal in vif-nat

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Wei,

> On 7 Sep 2020, at 15:36, Wei Liu <wl@xxxxxxx> wrote:
> 
> On Thu, Aug 20, 2020 at 12:00:23PM +0100, Diego Sueiro wrote:
>> Copy temp files used to add/remove dhcpd configurations to avoid
>> replacing potential symlinks.
>> 
> 
> Can you clarify the issue you saw a bit?
> 
> Which one of the parameter is a symlink (I assume the latter) and what
> problem you see with replacing the symlinks?

Maybe i can explain here.

If you have this:
/etc/dhcp.conf -> dhcp.conf.real

mv will create a new file dhcp.conf where cp will actually modify
dhcp.conf.real instead of replacing the symlink with a real file.

This prevents some mistakes where the user will actually continue to
modify dhcp.conf.real where it would not be the one used anymore.

Bertrand

> 
>> Signed-off-by: Diego Sueiro <diego.sueiro@xxxxxxx>
>> ---
>> tools/hotplug/Linux/vif-nat | 12 +++++++-----
>> 1 file changed, 7 insertions(+), 5 deletions(-)
>> 
>> diff --git a/tools/hotplug/Linux/vif-nat b/tools/hotplug/Linux/vif-nat
>> index 2614435..1ab80ed 100644
>> --- a/tools/hotplug/Linux/vif-nat
>> +++ b/tools/hotplug/Linux/vif-nat
>> @@ -99,7 +99,8 @@ dhcparg_remove_entry()
>>   then
>>     rm "$tmpfile"
>>   else
>> -    mv "$tmpfile" "$dhcpd_arg_file"
>> +    cp "$tmpfile" "$dhcpd_arg_file"
>> +    rm "$tmpfile"
>>   fi
> 
> You could've simplified the code a bit here and below now that both
> branches issue the same rm command.
> 
> But don't resend just yet. Please help me understand your issue first.
> 
> Wei.
> 
>> }
>> 
>> @@ -109,11 +110,11 @@ dhcparg_add_entry()
>>   local tmpfile=$(mktemp)
>>   # handle Red Hat, SUSE, and Debian styles, with or without quotes
>>   sed -e 's/^DHCPDARGS="*\([^"]*\)"*/DHCPDARGS="\1'"${dev} "'"/' \
>> -     "$dhcpd_arg_file" >"$tmpfile" && mv "$tmpfile" "$dhcpd_arg_file"
>> +     "$dhcpd_arg_file" >"$tmpfile" && cp "$tmpfile" "$dhcpd_arg_file"
>>   sed -e 's/^DHCPD_INTERFACE="*\([^"]*\)"*/DHCPD_INTERFACE="\1'"${dev} "'"/' 
>> \
>> -     "$dhcpd_arg_file" >"$tmpfile" && mv "$tmpfile" "$dhcpd_arg_file"
>> +     "$dhcpd_arg_file" >"$tmpfile" && cp "$tmpfile" "$dhcpd_arg_file"
>>   sed -e 's/^INTERFACES="*\([^"]*\)"*/INTERFACES="\1'"${dev} "'"/' \
>> -     "$dhcpd_arg_file" >"$tmpfile" && mv "$tmpfile" "$dhcpd_arg_file"
>> +     "$dhcpd_arg_file" >"$tmpfile" && cp "$tmpfile" "$dhcpd_arg_file"
>>   rm -f "$tmpfile"
>> }
>> 
>> @@ -125,7 +126,8 @@ dhcp_remove_entry()
>>   then
>>     rm "$tmpfile"
>>   else
>> -    mv "$tmpfile" "$dhcpd_conf_file"
>> +    cp "$tmpfile" "$dhcpd_conf_file"
>> +    rm "$tmpfile"
>>   fi
>>   dhcparg_remove_entry
>> }
>> -- 
>> 2.7.4