[Bcc problems] Ah, indeed, I never got some of those mails, just saw my
original patch merged last Sunday and already wondered why nobody
commented :)
For brevity, I'll summarize the points we've been discussing. Forgive me
if I oversee something important.
- We're thinking about importing DHCP options as "first class citizens"
into our XML language.
- Issue: There are no official names in any RFC.
- Possible solution: use what dnsmasq offers ("dnsmasq --help dhcp").
- Issue here: We do not want to depend on dnsmasq peculiariaties
too much. Consider the situation where we choose a different
DHCP server and that one uses different names. We'd have to
convert from nameA to nameB.
- Issue here: The selection made by dnsmasq could be called
somewhat subjective. There is a great possibility that users
need additional options, be it more rarely used,
hardware-specific options for some access point, or the custom
option range (224-252).
- Possible solution: go with names for everything dnsmasq also
has a name for and allow numbers for other DHCP options.
- Issue here: If we take this route, we need to keep the names
synchronous to dnsmasq. Eg. if dnsmasq learns a new option, we
should add it to our XML schema as well for consistency reasons.
- Possible solution: invent own names for further options.
- Issue here: Not much utility and names won't be too intuitive.
- Issue: whatever names we take, it hurts much more if in retrospect
we chose the "wrong" ones than it does to allow <option>. So while
allowing numbers at first sight looks like a possible trapdoor,
I guess getting the names right is harder because it's sort of a
decision "for eternity".
- Issue: If we learn names, we might want to validate values just as
well.
- Issue: One could implement different depths of option validation.
For example, according to RFC 4578 DHCP option 94 (Client network
interface identifier) has a three octets-sized value and currently
the first one can only be 1. Would we enforce this or just enforce
three integer octets?
- Issue: As a variation, some clients might not be 100%
RFC-conformant and expect certain option values that we forbid
because _we_ try to conform 100% to the appropriate RFC.
- Issue: We would have to select for WHICH options we'd do
validation. The underlying DHCP server, be it dnsmasq or something
else in the future, is possibly not as restrictive as we are. The
alternative approach "be as smart as dnsmasq is" will be hard to
maintain.
- Issue: Validation can't be done for the "private use" DHCP
options.
- Possible solution: Just allow anything for THESE.
Unrelated to this discussion there was the idea to add a "force="
attribute. I don't think there's much argument about it, it's a good idea.
Am 26.02.2013 19:05, schrieb Laine Stump:
On the other hand, I don't want to over-engineer this problem so much
that we never push *anything*.
Yes, unless I missed something I think the bottom line of my summary
above is that there would have to be a really good argument _against_
"number=xxx". Implementing "name=yyy" does not automatically mean that
"number=xxx" is a bad thing to do. Most of all, it's a very pragmatic
solution whereas "name=xxx" most probably requires some design decisions.
Without even arriving at a decision about this, I'm now thinking that
maybe we should revert the earlier <option> patch until after the
release, and re-push it after the named-option support is done
(potentially with some changes).
Any other opinions?
I'm used to having to compile it myself anyway, so that'd be fine with
me if we want
to give it more thought.
On the other hand, I don't see that we can do WITHOUT "number=..."
unless we want to implement only a restricted subset of DHCP options
known by name.
+ ipdef->options[r].number,
+ ipdef->options[r].value);
We've done no sanity checking on the contents of value. Is there any
danger of a rogue metacharacter in dnsmasq.conf causing problems? I seem
to recall asking that question before and being told "no", and I'm going
to continue assuming that for now, but I still think it would be good
practice to do some basic validation of the value if we can determine an
all-encompassing valid character set that is something smaller than
"everything" :-)
dnsmasq (or any other DHCP server we support in the future) could use a
"test config" mode that simply returns success or failure and that we
could use when reading in XML fragments. That way, we wouldn't have to
duplicate dnsmasq's work.
@@ -959,6 +968,7 @@ networkDnsmasqConfContents(virNetworkObjPtr network,
virBufferAsprintf(&configbuf, "addn-hosts=%s\n",
dctx->addnhostsfile->path);
+
This looks odd.
And it got overseen during the merge ;)
Overall status, I'm tentative to give ACK. However, I'd like to hear one more
opinion on the correctness of XML schema. Elder libvirt developers remember
times when XML schema change needed ACK from at least one of Daniels :)
Well, I'm usually hesitant about any xml addition for fear of getting it
wrong, but since it was me who originally suggested this schema, and
there was no negative comment about it on the list at the time, I'm
giving my ACK (even though I'm not lucky enough to be named Daniel :-)).
I guess sometimes even after intensive discussions we just have to try
and see if things work out.
--
Pieter Hollants <pie...@hollants.com>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
