On Thursday, February 7, 2019 at 4:11:14 AM UTC-5, Nadav Har'El wrote:
>
> Following my previous email where I outlined my comments, here are
> specific comments on your patch (sorry about the long delay)
> ...
>
> I don't think we need this flag, we can always write this file (if we
> can), I think the performance penalty is trivial.
>
> This will cut this patch size in half, and will avoid frustrations for
> future users who won't even know this option exist, and not get
> /etc/resolv.conf and wonder again why things don't work.
>
Makes sense to me - I was trying to preserve compatibility with current
defaults, since you guys are the ones that can do that. :)
> Why is it possible that /etc doesn't exist yet? I think we start DHCP
> *after* mounting the root filesystem.
>
I'm overly cautious by nature and entirely unsure whether /etc is a
standard OSv directory or not.
I have also suggested in my other email to have (in usr_rofs.manifest.skel)
> a symlink from /etc/resolv.conf to /tmp/resolv.conf, so it will be writable
> even on read-only filesystems. I'm not an expert on the new rofs setup,
> Waldek can comment if that makes sense.
>
> Please use - here and below - the OSv coding style. We put a space after
> the "if" but not after the ( or before the ), and the "{" is
> on the same line. You can see examples in dhcp.cc and in other source
> files. There is also an incomplete CODINGSTYLE.md.
>
Apologies - Will get that cleaned up.
Why "w+"? "w+" is for when you will also need to read from the same file.
> Here you just plan to truncate and write to it.
> So a normal "w" is exactly what you need.
>
Good point, I forgot that + means "write" instead of "create if doesn't
exist." Can also convert this to an open() call if that fits better with
OSv's preferred code paths.
Thanks so much for the feedback, I'm off to revise the patches now!
--
You received this message because you are subscribed to the Google Groups "OSv
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
