[Devel] [PATCH] Backup network config file if it lacks of warning
Kir Kolyshkin
kir at openvz.org
Tue Apr 30 18:10:32 PDT 2013
On 04/30/2013 05:50 PM, Igor M Podlesny wrote:
> On 1 May 2013 08:28, Kir Kolyshkin <kir at openvz.org> wrote:
>> On 04/30/2013 04:44 PM, Igor Podlesny wrote:
> […]
>>> setup_network()
>>> {
>>> + local dont_edit='WARNING: Do not edit this file'
>>
>> 1 Add an empty line between variables declaration and code
> ok
>> 2 I suggest using the very first line of comment we are leaving, i.e.
>>
>>
>> # This configuration file is auto-generated.
>>
>> and using it as a whole, so we can grep for the whole line (less probable
>> false positive).
> Well, I thought about it and now I can suggest using
> "openvz-vzctl:WARNING_DONT_EDIT" pattern.
> This one surely wouldn't make false positives.
I thought about it as well.
Unfortunately we must reuse existing line, so new version of vzctl will
not lead to those .bak files in every debian container.
>>
>>> + [ -r "$CFGFILE" ] &&
>>> + head -n20 "$CFGFILE" | fgrep -q -- "$dont_edit" || {
>>
>> 1 Lots of && and || and { makes this code a bit cryptic to read.
> Well, there're only 2 of them )
>> 2 If we are using the first line we can do head -n1 (again less probable
>> false positive)
> Hm, I can argue saying that some other don't edit participants
> could also add a line or two, couldn't they?
>> 3 Also, we can remove check for file and do head $CFGFILE 2>/dev/null
>> instead
> That's not cosy — why muting utility if there's no file at all? I
> disagree on that.
>
>>> + # -- It has no our warning inside, so we'd better
>>> + # back it up:
>>
>> Moving this comment out of this block can make it one-liner, easier to read
>>
>>
>>> + cp "$CFGFILE" \
>>> + "${CFGFILE}-$(date
>>> --rfc-3339=seconds).bak"
>>
>> 1 Use cp -a to retain file attributes such as times
>> 2 Do you really want spaces in file name? IMHO "interfaces.2013-04-30
>> 17:20:57-07:00.bak" is an ugly name for a file.
> Yep, I see no problems with that, but I surely can convert them to
> '_' if you'd like to.
I dislike spaces in such file names because they make it harder to do
copy-paste with mouse/trackball clicks (word selection)
Converting spaces to undercores will make it yet more complicated. I'd
rather not to, it's already complicated enough.
>
>> 3 Please make sure this date syntax works in ancient date versions (I
>> suggest checking centos4 or debian 3.1, or both).
> It was an issue I thought of, yep. Alas, I don't have anything
> ancient enough ATM, couldn't you try it please?
Me neither. But I can point you out to debian 3.1 and centos 4 ostemplates:
http://download.openvz.org/template/precreated/unsupported/
>
>> 4 Maybe it's easier to just use $$ (current shell pid).
> May be, but precise enough date-time is better.
Why exactly do you need date? I thought you just wanted the file to not
be lost, and using $$.bak is a simple enough way.
I can extrapolate and say something like renaming this to
"$CFGFILE -- a backup copy generated by a component of openvz vzctl,
script $0 at $(date --give-me-nanosecond-precision)"
is better.
But sometimes being perfect is being simple.
>>
>>> + }
>>> +
>>> echo "# This configuration file is auto-generated.
>>> #
>>> -# WARNING: Do not edit this file, your changes will be lost.
>>> +# $dont_edit, your changes will be lost.
>>> # Please create/edit $CFGFILE.head and
>>> # $CFGFILE.tail instead, their contents will be
>>> # inserted at the beginning and at the end of this file, respectively.
>>
>> So, taking into account all the above comments,
>> what about something like this (untested):
>>
>> local warn='# This configuration file is auto-generated.'
>>
>> # Make a backup if the file does not look like generated by us
>> if ! head -n1 "$CFGFILE" 2>/dev/null | grep -q -- "^${warn}$"; then
>> cp -a "$CFGFILE" "$CFGFILE-$$.bak"
>> fi
>>
>> echo "$warn
>> #
>>
>> # WARNING: Do not edit this file, your changes will be lost.
>> ....
More information about the Devel
mailing list