[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH RESEND] Add support for <option> tag in network config



On 02/26/2013 03:38 PM, Pieter Hollants wrote:
> [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.


[very thorough summary snipped]


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


Correct.


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

Right. I haven't changed my mind about the "number" attribute, but since
the "value" will be re-used when we have named (and more thoroughly
parsed/validated) options, and since there are some options that take a
*list* of values rather than a single value, if maybe we shouldn't do
something like:

    <option number='6'>
       <value type='ipv4' data='1.2.3.4'/>
       <value type='ipv4' data='4.5.6.7'/>
    </option>

or something similar. (on known options, the "type" would be implicit,
and if specified would be required to match what was already known).

Since that would create conflicts with what we'd decided / you
implemented, and since we can't change anything once it's been in an
official release, I figured the safest thing to do was revert the patch
until after the release, giving us more time to ponder (I reverted it
this morning). Definitely *something* will go in between 1.0.3 and
1.0.4, though!

(BTW, thanks for posting a patch for this so quickly. It really helped
to get me motivated about the issue.)


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]