Re: [libvirt] [PATCH 2/4] conf: introduce new family attribute in graphics listen for chose IP family

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.

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(a)redhat.com&gt; > --- > 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 > &lt;graphics type='rdp' autoport='yes' multiUser='yes' /&gt; > &lt;graphics type='desktop' fullscreen='yes'/&gt; > &lt;graphics type='spice'&gt; > - &lt;listen type='network' network='rednet'/&gt; > + &lt;listen type='network' network='rednet' family='default'/&gt; This changes to ipv6='yes' > &lt;/graphics&gt; > &lt;/devices&gt; > ...</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(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list