On 02/24/2015 12:42 PM, John Ferlan wrote:
>
> On 02/13/2015 02:17 AM, Luyao Huang wrote:
>> If a interface or network have both ipv6 and ipv4 address which can be used,
>> we do not know use which be a listen address. So introduce a new attribute
>> to help us chose this.
>>
>> graphics XML will like this after this commit:
>>
>> <graphics type='spice' port='5900' autoport='yes'>
>> <listen type='network' address='192.168.0.1' network='vepa-net' family='default'/>
>> </graphics>
> Generally don't want to see 'default' in there - I would say if not
> provided, then on output we don't print default, especially since if
> someone tried to port this XML to a version of libvirt that doesn't
> support the family attribute, then you've got other issues.
>
>> and this ip family can be set 3 type:
>>
>> default: check if the interface or network have a can be used ipv4 address,
>> if yes use this address, if not will try to get a ipv6 address.
>>
>> ipv4: check if the interface or network have a can be used ipv4 address
>> ipv6: check if the interface or network have a can be used ipv6 address
>>
>> fix some test which will be break by this commit.
> And this is why you don't add "family='default'"...
>
>
> I see family as a "tristate" value. That is not provided is 0 and not
> printed... Thus, the value turns into tristate where of ipv6='yes|no',
> where the default is no.
There is precedent in libvirt config for a "family" which can be set to
ipv6 or ipv4 or not be set at all (e.g. see the <ip> and <route>
elements in a libvirt network). I agree that "default" shouldn't be an
allowed setting, but I think we can/should stick with family for the
attribute rather than ipv6='(yes|no)'
>
> The problem you have is that existing configurations don't have the
> value *and* the documentation states it'll return the first IPv4 address
> found - which means by default (eg, when ipv6 is not provided or if it's
> set to 'no'), we have to return an ipv4 address.
>> Signed-off-by: Luyao Huang <lhuang@xxxxxxxxxx>
>> ---
>> docs/formatdomain.html.in | 10 +++++++++-
>> docs/schemas/domaincommon.rng | 9 +++++++++
>> src/conf/domain_conf.c | 21 +++++++++++++++++++++
>> src/conf/domain_conf.h | 10 ++++++++++
>> .../qemuxml2argv-graphics-listen-network.xml | 2 +-
>> .../qemuxml2argv-graphics-listen-network2.xml | 2 +-
>> .../qemuxml2xmlout-graphics-listen-network2.xml | 2 +-
>> 7 files changed, 52 insertions(+), 4 deletions(-)
>>
>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>> index fcf5984..7a07ca0 100644
>> --- a/docs/formatdomain.html.in
>> +++ b/docs/formatdomain.html.in
>> @@ -4512,7 +4512,7 @@ qemu-kvm -net nic,model=? /dev/null
>> <graphics type='rdp' autoport='yes' multiUser='yes' />
>> <graphics type='desktop' fullscreen='yes'/>
>> <graphics type='spice'>
>> - <listen type='network' network='rednet'/>
>> + <listen type='network' network='rednet' family='default'/>
> This changes to ipv6='yes'
>
>> </graphics>
>> </devices>
>> ...</pre>
>> @@ -4752,6 +4752,14 @@ qemu-kvm -net nic,model=? /dev/null
>> the first forward dev will be used.
>> </dd>
>> </dl>
>> + <dl>
>> + <dt><code>family</code></dt>
>> + <dd>if <code>type='network'</code>, the <code>family</code>
>> + attribute will contain an IP family. This tells which IP address
>> + will be got for the network. IP family can be set to default, ipv4
>> + or ipv6.
>> + </dd>
>> + </dl>
> This then describes that if the attribute "ipv6='yes'" is provided, then
> rather than searching for the first IPv4 address, the code will search
> for the first IPv6 address. If there is no IPv6 address, then failure
> will occur and no fall back to IPv4 happens.
>
>>
>> <h4><a name="elementsVideo">Video devices</a></h4>
>> <p>
>> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
>> index 7a1d299..fb8d084 100644
>> --- a/docs/schemas/domaincommon.rng
>> +++ b/docs/schemas/domaincommon.rng
>> @@ -2888,6 +2888,15 @@
>> <ref name="addrIPorName"/>
>> </attribute>
>> </optional>
>> + <optional>
>> + <attribute name="family">
>> + <choice>
>> + <value>default</value>
>> + <value>ipv4</value>
>> + <value>ipv6</value>
>> + </choice>
>> + </attribute>
>> + </optional>
> This becomes:
>
> <attribute name='ipv6'>
> <ref name="virYesNo"/>
> </attribute>
>
>> </group>
>> </choice>
>> </element>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index fd36063..307801d 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -522,6 +522,12 @@ VIR_ENUM_IMPL(virDomainGraphicsListen, VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST,
>> "address",
>> "network")
>>
>> +VIR_ENUM_IMPL(virDomainGraphicsListenFamily,
>> + VIR_DOMAIN_GRAPHICS_LISTEN_FAMILY_LAST,
>> + "default",
>> + "ipv4",
>> + "ipv6")
>> +
> won't be necessary...
>
>> VIR_ENUM_IMPL(virDomainGraphicsAuthConnected,
>> VIR_DOMAIN_GRAPHICS_AUTH_CONNECTED_LAST,
>> "default",
>> @@ -9396,6 +9402,7 @@ virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDefPtr def,
>> char *address = virXMLPropString(node, "address");
>> char *network = virXMLPropString(node, "network");
>> char *fromConfig = virXMLPropString(node, "fromConfig");
>> + char *family = virXMLPropString(node, "family");
> Changes to
>
> char *ipv6 = virXMLPropString(node, "ipv6");
>
>
>> int tmp;
>>
>> if (!type) {
>> @@ -9433,6 +9440,16 @@ virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDefPtr def,
>> network = NULL;
>> }
>>
>> + if (def->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK) {
>> + if (!family) {
>> + def->family = VIR_DOMAIN_GRAPHICS_LISTEN_FAMILY_DEFAULT;
>> + } else if ((def->family = virDomainGraphicsListenFamilyTypeFromString(family)) < 0) {
>> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> + _("unknown graphics listen IP family '%s'"), family);
>> + goto error;
>> + }
> All this changes to:
>
> if (rawio) {
> if ((def->ipv6 = virTristateBoolTypeFromString(ipv6)) <= 0) {
> virReportError(VIR_ERR_XML_ERROR,
> _("unknown hostdev ipv6 setting '%s'"),
> ipv6);
> goto error;
> }
>
>> + }
>> +
>> if (fromConfig &&
>> flags & VIR_DOMAIN_DEF_PARSE_STATUS) {
>> if (virStrToLong_i(fromConfig, NULL, 10, &tmp) < 0) {
>> @@ -9452,6 +9469,7 @@ virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDefPtr def,
>> VIR_FREE(address);
>> VIR_FREE(network);
>> VIR_FREE(fromConfig);
>> + VIR_FREE(family);
> Changes to VIR_FREE(ipv6);
>
>> return ret;
>> }
>>
>> @@ -19044,6 +19062,9 @@ virDomainGraphicsListenDefFormat(virBufferPtr buf,
>> if (def->network &&
>> (def->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK)) {
>> virBufferEscapeString(buf, " network='%s'", def->network);
>> +
>> + virBufferAsprintf(buf, " family='%s'",
>> + virDomainGraphicsListenFamilyTypeToString(def->family));
> This becomes
> if (def->ipv6)
> virBufferAsprintf(buf, " ipv6='%s'",
> virTristateBoolTypeToString(def->ipv6));
>> }
>>
>> if (flags & VIR_DOMAIN_DEF_FORMAT_STATUS)
>> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>> index da21bce..7806bc6 100644
>> --- a/src/conf/domain_conf.h
>> +++ b/src/conf/domain_conf.h
>> @@ -1438,6 +1438,14 @@ typedef enum {
>> } virDomainGraphicsListenType;
>>
>> typedef enum {
>> + VIR_DOMAIN_GRAPHICS_LISTEN_FAMILY_DEFAULT = 0,
>> + VIR_DOMAIN_GRAPHICS_LISTEN_FAMILY_IPV4,
>> + VIR_DOMAIN_GRAPHICS_LISTEN_FAMILY_IPV6,
>> +
>> + VIR_DOMAIN_GRAPHICS_LISTEN_FAMILY_LAST
>> +} virDomainGraphicsListenFamily;
> Unnecessary
>
>> +
>> +typedef enum {
>> VIR_DOMAIN_HUB_TYPE_USB,
>>
>> VIR_DOMAIN_HUB_TYPE_LAST
>> @@ -1450,6 +1458,7 @@ struct _virDomainGraphicsListenDef {
>> char *address;
>> char *network;
>> bool fromConfig; /* true if the @address is config file originated */
>> + int family; /*enum virDomainGraphicsListenFamily*/
> This becomes
>
> int ipv6; /* enum virTristateBool */
>
>> };
>>
>> struct _virDomainGraphicsDef {
>> @@ -2862,6 +2871,7 @@ VIR_ENUM_DECL(virDomainInput)
>> VIR_ENUM_DECL(virDomainInputBus)
>> VIR_ENUM_DECL(virDomainGraphics)
>> VIR_ENUM_DECL(virDomainGraphicsListen)
>> +VIR_ENUM_DECL(virDomainGraphicsListenFamily)
>> VIR_ENUM_DECL(virDomainGraphicsAuthConnected)
>> VIR_ENUM_DECL(virDomainGraphicsSpiceChannelName)
>> VIR_ENUM_DECL(virDomainGraphicsSpiceChannelMode)
>> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network.xml
>> index bf78ca8..1962474 100644
>> --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network.xml
>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network.xml
>> @@ -25,7 +25,7 @@
>> <input type='mouse' bus='ps2'/>
>> <input type='keyboard' bus='ps2'/>
>> <graphics type='vnc' port='5903' autoport='no'>
>> - <listen type='network' network='Bobsnetwork'/>
>> + <listen type='network' network='Bobsnetwork' family='default'/>
>> </graphics>
>> <video>
>> <model type='cirrus' vram='16384' heads='1'/>
>> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network2.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network2.xml
>> index 62353e9..d11eead 100644
>> --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network2.xml
>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network2.xml
>> @@ -25,7 +25,7 @@
>> <input type='keyboard' bus='ps2'/>
>> <graphics type='vnc' listen='1.2.3.4' autoport='yes'>
>> <listen type='address' address='1.2.3.4'/>
>> - <listen type='network' network='Bobsnetwork'/>
>> + <listen type='network' network='Bobsnetwork' family='default'/>
>> </graphics>
>> <video>
>> <model type='cirrus' vram='16384' heads='1'/>
>> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-listen-network2.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-listen-network2.xml
>> index abee7b6..e69c29a 100644
>> --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-listen-network2.xml
>> +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-listen-network2.xml
>> @@ -26,7 +26,7 @@
>> <input type='keyboard' bus='ps2'/>
>> <graphics type='vnc' port='-1' autoport='yes' listen='1.2.3.4'>
>> <listen type='address' address='1.2.3.4'/>
>> - <listen type='network' network='Bobsnetwork'/>
>> + <listen type='network' network='Bobsnetwork' family='default'/>
>> </graphics>
>> <video>
>> <model type='cirrus' vram='16384' heads='1'/>
>>
> None of these are necessary... BUT, you should add one for IPv6...
>
> That is - you need to add an "ipv6=yes" and an "ipv6=no" test case in
> order to make sure both are processed properly.
>
> John
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list