[libvirt] [PATCH 24/28] conf: allow setting peer address in <ip> element of <interface>
John Ferlan
jferlan at redhat.com
Fri Jun 24 20:40:40 UTC 2016
On 06/24/2016 03:59 PM, Laine Stump wrote:
> On 06/24/2016 09:29 AM, John Ferlan wrote:
>>
>> On 06/22/2016 01:37 PM, Laine Stump wrote:
>>> From: Vasiliy Tolstov <v.tolstov at selfip.ru>
>>>
>>> The peer attribute is used to set the property of the same name in the
>>> interface IP info:
>>>
>>> <interface type='ethernet'>
>>> ...
>>> <ip family='ipv4' address='192.168.122.5'
>>> prefix='32' peer='192.168.122.6'/>
>>> ...
>>> </interface>
>>>
>>> Note that this element is used to set the IP information on the
>>> *guest* side interface, not the host side interface - that will be
>>> supported in an upcoming patch.
>>>
>>> (This is an updated *re*-commit of commit 690969af, which was
>>> subsequently reverted in commit 1d14b13f).
>>>
>>> Signed-off-by: Vasiliy Tolstov <v.tolstov at selfip.ru>
>>> Signed-off-by: Laine Stump <laine at laine.org>
>>> ---
>>> docs/formatdomain.html.in | 40
>>> +++++++++++++++++++++++++---------------
>>> docs/schemas/domaincommon.rng | 5 +++++
>>> src/conf/domain_conf.c | 16 +++++++++++++++-
>>> src/conf/domain_conf.h | 1 +
>>> src/util/virnetdevip.h | 5 +++--
>>> 5 files changed, 49 insertions(+), 18 deletions(-)
>>>
>> It seems an earlier comment I made about freeaddrinfo for the
>> corresponding getaddrinfo becomes even more important now... So
>> somewhere between there and this patch, I think you need to generate a
>> Free API for virNetDevIPAddrPtr
>>
>> Then everywhere that current just does a VIR_FREE(ip) should call it...
>> Thus this change "gets it" for free - well mostly free, you'd have to
>> add the freeaddrinfo() for the 'peer' as well as the 'address'
>>
>> Of course I could be wrong and I'm sure you'll let me know ;-)
>
> Yep. We already talked about this in IRC. the virNetDevIPAddr doesn't
> contain the info returned from getaddrinfo. virSocketAddrParseInternal
> (that's who calls getaddrinfo) is called by two other functions, and
> both of those functions copy out the important stuff from getaddrinfo,
> then call freeaddrinfo. So everything is correct.
>
>>
>>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>>> index f660aa6..2466df7 100644
>>> --- a/docs/formatdomain.html.in
>>> +++ b/docs/formatdomain.html.in
>>> @@ -4967,6 +4967,7 @@ qemu-kvm -net nic,model=? /dev/null
>>> <source network='default'/>
>>> <target dev='vnet0'/>
>>> <b><ip address='192.168.122.5' prefix='24'/></b>
>>> + <b><ip address='192.168.122.5' prefix='24'
>>> peer='10.0.0.10'/></b>
>>> <b><route family='ipv4' address='192.168.122.0'
>>> prefix='24' gateway='192.168.122.1'/></b>
>>> <b><route family='ipv4' address='192.168.122.8'
>>> gateway='192.168.122.1'/></b>
>>> </interface>
>>> @@ -4985,21 +4986,30 @@ qemu-kvm -net nic,model=? /dev/null
>>> </pre>
>>> <p>
>>> - <span class="since">Since 1.2.12</span> the network devices and
>>> host devices
>>> - with network capabilities can be provided zero or more IP
>>> addresses to set
>>> - on the target device. Note that some hypervisors or network
>>> device types
>>> - will simply ignore them or only use the first one. The
>>> <code>family</code>
>>> - attribute can be set to either <code>ipv4</code> or
>>> <code>ipv6</code>, the
>>> - <code>address</code> attribute holds the IP address. The
>>> <code>prefix</code>
>>> - is not mandatory since some hypervisors do not handle it.
>>> - </p>
>>> -
>>> - <p>
>>> - <span class="since">Since 1.2.12</span> route elements can also
>>> be added
>>> - to define the network routes to use for the network device. The
>>> attributes
>>> - of this element are described in the documentation for the
>>> <code>route</code>
>>> - element in <a
>>> href="formatnetwork.html#elementsStaticroute">network definitions</a>.
>>> - This is only used by the LXC driver.
>>> + <span class="since">Since 1.2.12</span> network devices and
>>> + hostdev devices with network capabilities can optionally be
>>> provided
>>> + one or more IP addresses to set on the network device in the
>>> + guest. Note that some hypervisors or network device types will
>>> + simply ignore them or only use the first one.
>>> + The <code>family</code> attribute can be set to
>>> + either <code>ipv4</code> or <code>ipv6</code>, and the
>>> + <code>address</code> attribute contains the IP address. The
>>> + optional <code>prefix</code> is the number of 1 bits in the
>>> + netmask, and will be automatically set if not specified - for
>>> + IPv4 the default prefix is determined according to the network
>>> + "class" (A, B, or C - see RFC870), and for IPv6 the default
>>> + prefix is 64. The optional <code>peer</code> attribute holds the
>>> + IP address of the other end of a point-to-point network
>>> + device <span class="since">(since 2.0.0)</span>.
>>> + </p>
>>> +
>>> + <p>
>>> + <span class="since">Since 1.2.12</span> route elements can also be
>>> + added to define IP routes to add in the guest. The attributes of
>>> + this element are described in the documentation for
>>> + the <code>route</code> element
>>> + in <a href="formatnetwork.html#elementsStaticroute">network
>>> + definitions</a>. This is used by the LXC driver.
>>> </p>
>>> <h5><a name="elementVhostuser">vhost-user interface</a></h5>
>>> diff --git a/docs/schemas/domaincommon.rng
>>> b/docs/schemas/domaincommon.rng
>>> index 563cb3c..2d12da9 100644
>>> --- a/docs/schemas/domaincommon.rng
>>> +++ b/docs/schemas/domaincommon.rng
>>> @@ -2629,6 +2629,11 @@
>>> <ref name="ipPrefix"/>
>>> </attribute>
>>> </optional>
>>> + <optional>
>>> + <attribute name="peer">
>>> + <ref name="ipAddr"/>
>>> + </attribute>
>>> + </optional>
>>> <empty/>
>>> </element>
>>> </zeroOrMore>
>>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>>> index df52ac9..ad2d983 100644
>>> --- a/src/conf/domain_conf.c
>>> +++ b/src/conf/domain_conf.c
>>> @@ -6108,7 +6108,7 @@ virDomainNetIPParseXML(xmlNodePtr node)
>>> unsigned int prefixValue = 0;
>>> char *familyStr = NULL;
>>> int family = AF_UNSPEC;
>>> - char *address = NULL;
>>> + char *address = NULL, *peer = NULL;
>>> if (!(address = virXMLPropString(node, "address"))) {
>>> virReportError(VIR_ERR_XML_ERROR, "%s",
>>> @@ -6146,6 +6146,13 @@ virDomainNetIPParseXML(xmlNodePtr node)
>>> }
>>> ip->prefix = prefixValue;
>>> + if ((peer = virXMLPropString(node, "peer")) != NULL &&
>>> + virSocketAddrParse(&ip->peer, peer, family) < 0) {
>>> + virReportError(VIR_ERR_INVALID_ARG,
>>> + _("Invalid peer '%s' in <ip>"), peer);
>>> + goto cleanup;
>>> + }
>>> +
>>> ret = ip;
>>> ip = NULL;
>>> @@ -6153,6 +6160,7 @@ virDomainNetIPParseXML(xmlNodePtr node)
>>> VIR_FREE(prefixStr);
>>> VIR_FREE(familyStr);
>>> VIR_FREE(address);
>>> + VIR_FREE(peer);
>>> VIR_FREE(ip);
>>> return ret;
>>> }
>>> @@ -20254,6 +20262,12 @@ virDomainNetIPInfoFormat(virBufferPtr buf,
>>> virBufferAsprintf(buf, " family='%s'", familyStr);
>>> if (def->ips[i]->prefix)
>>> virBufferAsprintf(buf, " prefix='%u'",
>>> def->ips[i]->prefix);
>>> + if (VIR_SOCKET_ADDR_VALID(&def->ips[i]->peer)) {
>>> + if (!(ipStr = virSocketAddrFormat(&def->ips[i]->peer)))
>>> + return -1;
>>> + virBufferAsprintf(buf, " peer='%s'", ipStr);
>>> + VIR_FREE(ipStr);
>>> + }
>>> virBufferAddLit(buf, "/>\n");
>>> }
>>> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>>> index 0df5579..7ff966f 100644
>>> --- a/src/conf/domain_conf.h
>>> +++ b/src/conf/domain_conf.h
>>> @@ -383,6 +383,7 @@ typedef enum {
>>> VIR_DOMAIN_HOSTDEV_CAPS_TYPE_LAST
>>> } virDomainHostdevCapsType;
>>> +
>> Spurious change that can be dropped.
>>
>> This change is ACK'able with the mentioned adjustments (whether you want
>> to repost stuff or not is your call).
>
> You mean the extra line in domain_conf.h? :-P (since the other comment
> is a non-issue).
Yeah - I was probably thinking more about the Free thing, but since
you've pointed out that the freeaddrinfo is done - it's a moot point.
John
>
>> John
>>
More information about the libvir-list
mailing list