Re: [PATCH] Expose SLIRP attributes

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

 



On 03/28/2014 12:34 PM, Michal Privoznik wrote:
> On 28.03.2014 09:33, Laine Stump wrote:
>> On 03/27/2014 07:17 PM, Michal Privoznik wrote:
>>> We allow users to use SLIRP stack. However, there are some knobs
>>> which are not exposed to users, such as host network address, DNS
>>> server, smb, and others.
>>>
>>> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
>>> ---
>>>   docs/formatdomain.html.in                          |  7 +-
>>>   docs/schemas/domaincommon.rng                      | 23 +++++-
>>>   src/conf/domain_conf.c                             | 88
>>> ++++++++++++++++++++++
>>>   src/conf/domain_conf.h                             |  6 ++
>>>   src/qemu/qemu_command.c                            | 19 +++++
>>>   .../qemuxml2argvdata/qemuxml2argv-net-user-ip.args |  7 ++
>>>   .../qemuxml2argvdata/qemuxml2argv-net-user-ip.xml  | 33 ++++++++
>>>   tests/qemuxml2argvtest.c                           |  1 +
>>>   8 files changed, 180 insertions(+), 4 deletions(-)
>>>   create mode 100644
>>> tests/qemuxml2argvdata/qemuxml2argv-net-user-ip.args
>>>   create mode 100644
>>> tests/qemuxml2argvdata/qemuxml2argv-net-user-ip.xml
>>
>> It is essential that this new <ip> element be rejected, preferably at
>> parse time in the qemu-specific post-parse callback, for any interface
>> type that doesn't support it. Since it is the most obvious way of
>> specifying an IP address for a guest (and it is a way that has been
>> requested in the past) we are otherwise certain to have a lot of support
>> questions asking why the IP address setting isn't being used.
>
> Well, I think rejecting it goes against current policy 'silently
> ignore elements unknown to the libvirt'. But I can live with that here.

It's no longer unknown :-)

>
>>
>> Also, the attribute names seem confusing. The "address" attribute is the
>> address of the *network*, not of the interface, and "dhcpstart" is the
>> address of the interface. Even though qemu specifies the network address
>> and interface address separately, the network address is really
>> pointless, as it can/should be derived from the interface address.
>
> It is, so what XML schema do you propose? I'm not pleased with <ip/>
> either. But it was the best I could come up with so far. Maybe we are
> aiming for more structured XML here:
>
>   <interface type='user'>
>     <network address='192.168.2.0' prefix='24'/>
>     <dns address='92.168.2.3'/>
>     <dhcp start='192.168.2.9'/>
>     <mac address='00:11:22:33:44:55'/>
>     <model type='rtl8139'/>
>   </interface>
>
> Or even more neting:
>
>   <interface type='user'>
>     <network>
>       <ip address='192.168.2.0' prefix='24'/>
>       <dns address='92.168.2.3'/>
>       <dhcp start='192.168.2.9'/>
>     </network>

Interesting idea - a mini <network> inside the <interface>. If we were
going to go this far, it should be in order to replicate the schema of
<network> as much as possible though. Otherwise it could lead to confusion.

For that reason, I think just a single <ip> element is fine, I just
think the attribute names should follow function rather than following
qemu (see my response to Rich). This will make it more likely that we
can add support for <ip> on some other network type in the future. (For
example, I could see us possibly adding the capability to use the <ip>
inside an interface to automatically populate the static IP list of a
libvirt network's dnsmasq instance before the domain is started, then
removing it once the domain is stopped - it would really only be
interested in the guest IP address, but would fail if any other info was
present and didn't match the network's config).

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]