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

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

 



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
>>      &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@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
>

--
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]