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

[Wei Liu <wl@xxxxxxx>]

On Mon, Sep 07, 2020 at 03:01:37PM +0000, Bertrand Marquis wrote:
> 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.

OK. Now I understand the use case. Thanks.

I think you explanation should be part of the commit message.

Diego, can you please incorporate Bertrand's explanation and deal with
my comment below?

> >> ---
> >> 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.

Wei.