Re: [Qemu-devel] [PATCH] [slirp] add nextserver support in slirp's dhcp-server

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3598 bytes --]

On 07/02/2013 07:03 AM, Bas van Sisseren wrote:
>>> +# @nextserver: #optional IP address of BOOTP server
>>> +#
> 
>> Please mark this field as '(since 1.6)'.
> 
> Added in my local checkout. I now have to find out how to generate a new
> patch and post it into this thread. I'll try to post a new patch somewhere
> in the next days. (I'm new to git :-) )

Feel free to ask questions.  Also, see if the wiki page
http://wiki.qemu.org/Contribute/SubmitAPatch helps, or could be improved.

My typical workflow for amending a series to address review comments is
to 'git rebase -i', change the marker on patches I intend to modify from
'pick' to edit' on the screen it pulls up, then exit that editor.  From
there, git will apply patches in turn, halting on each patch I marked
for edit, where I can make the changes, 'git add' them, then 'git rebase
--continue' to merge them into the patch series in the right place.
Read 'git rebase --help' for a lot more hints on what it can do.

>> Does the field work for both IPv4 and IPv6 addresses?
> 
> This setting is IPv4 only. (btw, I think all slirp settings are IPv4 only)

Just making sure we are considering things if it matters; but it sounds
like it doesn't matter here.

>>>  # @dns: #optional guest-visible address of the virtual nameserver
>>>  #
>>>  # @dnssearch: #optional list of DNS suffixes to search, passed as DHCP option
>>> @@ -2563,6 +2565,7 @@
>>>      '*tftp':      'str',
>>>      '*bootfile':  'str',
>>>      '*dhcpstart': 'str',
>>> +    '*nextserver': 'str',
> 
>> This is a command addition that libvirt can't take advantage of unless
>> we get introspection working in a timely manner.
> 
> It would be a nice addition to libvirt, but is it a problem that libvirt
> doesn't support all new settings (yet)?

The problem is that libvirt supports multiple versions of qemu, but
wants to provide sane error messages to the user.  If we add a mapping
to libvirt to expose this new parameter, then libvirt would like to know
whether the qemu version on the user's machine is new enough to honor
that mapping, or old enough to need an upgrade.  A version number check
is not ideal, because distros like to backport features without bumping
the version number (for instance, RHEL 6 ships a qemu that calls itself
0.12.xx, yet behaves like 1.5 for certain features).  Creating a new
qemu command works (if the command exists or does not exist, libvirt
knows the answer without too much effort).  Introspection would let
libvirt do a query: "what fields does this command support"; and if
nextserver is in (or not in) the list, then libvirt knows the answer
without too much effort.  But introspection is not incorporated yet
(we're working on it, and still hopeful it will make 1.6); so that
leaves us with only one other option for discovering if an optional
parameter is supported - try the command and see if it fails.  This is
not as friendly, and depending on the command may have other negative
side effects from even attempting the command at all.

While that's the theory behind QMP additions, there's also the matter
that libvirt won't support the new option at all (whether via
introspection or try-and-fail probing) unless someone writes a patch for
libvirt.  On the other hand, the schedule at which libvirt adds support
for qemu features shouldn't hold up whether qemu adds the features,
since there are more users of qemu than just libvirt.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]