[libvirt] [PATCHv2 2/4] conf: introduce new family attribute in graphics listen for chose IP family
lhuang
lhuang at redhat.com
Mon Mar 9 03:43:34 UTC 2015
On 03/09/2015 07:50 AM, John Ferlan wrote:
>
> On 02/28/2015 04:08 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='ipv4'/>
>> </graphics>
>>
>> and this ip family can be set 2 type, and the default one is ipv4:
>>
>> 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 add a new test.
>>
> Adjusting the commit text slightly to match the review below
>
>> Signed-off-by: Luyao Huang <lhuang at redhat.com>
>> ---
>> v2: remove family='default' and add a test.
>>
>> docs/formatdomain.html.in | 10 ++++++-
>> docs/schemas/domaincommon.rng | 8 +++++
>> src/conf/domain_conf.c | 20 +++++++++++++
>> src/conf/domain_conf.h | 9 ++++++
>> .../qemuxml2argv-graphics-listen-network-ipv6.xml | 35 ++++++++++++++++++++++
>> .../qemuxml2argv-graphics-listen-network.xml | 2 +-
>> .../qemuxml2xmlout-graphics-listen-network2.xml | 2 +-
>> tests/qemuxml2xmltest.c | 1 +
>> 8 files changed, 84 insertions(+), 3 deletions(-)
>> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network-ipv6.xml
>>
>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>> index fb0a0d1..e8ea6fa 100644
>> --- a/docs/formatdomain.html.in
>> +++ b/docs/formatdomain.html.in
>> @@ -4545,7 +4545,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='ipv4'/>
>> </graphics>
>> </devices>
>> ...</pre>
>> @@ -4785,6 +4785,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 ipv4
>> + or ipv6.
> Adjusted to:
>
> <dd>if <code>type='network'</code>, the <code>family</code>
> attribute may contain the IP family. The <code>family</code>
> can be set to either <code>ipv4</code> or <code>ipv6</code>.
> This advises the graphics device which IP address family
> to use as listen address for the network. The listen address
> used will be the first found address of the <code>family</code>
> type defined for the host.
> <span class="since">Since 1.2.14</span>
>
Looks good to me.
>> + </dd>
>> + </dl>
>>
>> <h4><a name="elementsVideo">Video devices</a></h4>
>> <p>
>> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
>> index 4e4fe9f..e84b213 100644
>> --- a/docs/schemas/domaincommon.rng
>> +++ b/docs/schemas/domaincommon.rng
>> @@ -2909,6 +2909,14 @@
>> <ref name="addrIPorName"/>
>> </attribute>
>> </optional>
>> + <optional>
>> + <attribute name="family">
>> + <choice>
>> + <value>ipv4</value>
>> + <value>ipv6</value>
>> + </choice>
>> + </attribute>
>> + </optional>
>> </group>
>> </choice>
>> </element>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 9b7ae3f..97f1250 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -522,6 +522,11 @@ VIR_ENUM_IMPL(virDomainGraphicsListen, VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST,
>> "address",
>> "network")
>>
>> +VIR_ENUM_IMPL(virDomainGraphicsListenFamily,
>> + VIR_DOMAIN_GRAPHICS_LISTEN_FAMILY_LAST,
>> + "ipv4",
>> + "ipv6")
>> +
> Need to add a "none"... As with the previous review - adding an
> attribute that's optional, but then generating it on output isn't good.
> So we need a way to signify it didn't exist on input. If provided that's
> fine, we can output it. If not provided, then sure we're going to
> eventually default to IPv4, but that's why I said to have the boolean be
> "if ipv6 desired"...
yes, thanks for pointing out, add a "none" is better than auto generate
it to ipv4.
Hmm, use family='ipv4|ipv6' in this place , another reason is : maybe
there will be a new ip family in the future (ipv7? :-P ), so use ip
family in this place will be a good choice in that day ( seems so far
away ;) )
>> VIR_ENUM_IMPL(virDomainGraphicsAuthConnected,
>> VIR_DOMAIN_GRAPHICS_AUTH_CONNECTED_LAST,
>> "default",
>> @@ -9464,6 +9469,7 @@ virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDefPtr def,
>> char *address = virXMLPropString(node, "address");
>> char *network = virXMLPropString(node, "network");
>> char *fromConfig = virXMLPropString(node, "fromConfig");
>> + char *family = virXMLPropString(node, "family");
>> int tmp;
>>
>> if (!type) {
>> @@ -9501,6 +9507,16 @@ virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDefPtr def,
>> network = NULL;
>> }
>>
>> + if (def->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK) {
>> + if (!family) {
>> + def->family = VIR_DOMAIN_GRAPHICS_LISTEN_FAMILY_IPV4;
> strike this check
>
>> + } else if ((def->family = virDomainGraphicsListenFamilyTypeFromString(family)) < 0) {
>> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> + _("unknown graphics listen IP family '%s'"), family);
>> + goto error;
>> + }
>> + }
>> +
>> if (fromConfig &&
>> flags & VIR_DOMAIN_DEF_PARSE_STATUS) {
>> if (virStrToLong_i(fromConfig, NULL, 10, &tmp) < 0) {
>> @@ -9520,6 +9536,7 @@ virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDefPtr def,
>> VIR_FREE(address);
>> VIR_FREE(network);
>> VIR_FREE(fromConfig);
>> + VIR_FREE(family);
>> return ret;
>> }
>>
>> @@ -19084,6 +19101,9 @@ virDomainGraphicsListenDefFormat(virBufferPtr buf,
>> if (def->network &&
>> (def->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK)) {
>> virBufferEscapeString(buf, " network='%s'", def->network);
>> +
> Add a condition
> if (def->family != VIR_DOMAIN_GRAPHICS_LISTEN_FAMILY_NONE)
>
>> + virBufferAsprintf(buf, " family='%s'",
>> + virDomainGraphicsListenFamilyTypeToString(def->family));
>> }
>>
>> if (flags & VIR_DOMAIN_DEF_FORMAT_STATUS)
>> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>> index 02ddd93..afffe9c 100644
>> --- a/src/conf/domain_conf.h
>> +++ b/src/conf/domain_conf.h
>> @@ -1441,6 +1441,13 @@ typedef enum {
>> } virDomainGraphicsListenType;
>>
>> typedef enum {
> Add:
> VIR_DOMAIN_GRAPHICS_LISTEN_FAMILY_NONE = 0,
>> + VIR_DOMAIN_GRAPHICS_LISTEN_FAMILY_IPV4,
>> + VIR_DOMAIN_GRAPHICS_LISTEN_FAMILY_IPV6,
>> +
>> + VIR_DOMAIN_GRAPHICS_LISTEN_FAMILY_LAST
>> +} virDomainGraphicsListenFamily;
>> +
>> +typedef enum {
>> VIR_DOMAIN_HUB_TYPE_USB,
>>
>> VIR_DOMAIN_HUB_TYPE_LAST
>> @@ -1453,6 +1460,7 @@ struct _virDomainGraphicsListenDef {
>> char *address;
>> char *network;
>> bool fromConfig; /* true if the @address is config file originated */
>> + int family; /*enum virDomainGraphicsListenFamily*/
>> };
>>
>> struct _virDomainGraphicsDef {
>> @@ -2866,6 +2874,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-ipv6.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network-ipv6.xml
>> new file mode 100644
>> index 0000000..6cce7a8
>> --- /dev/null
>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network-ipv6.xml
>> @@ -0,0 +1,35 @@
>> +<domain type='qemu'>
>> + <name>QEMUGuest1</name>
>> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
>> + <memory unit='KiB'>219100</memory>
>> + <currentMemory unit='KiB'>219100</currentMemory>
>> + <vcpu placement='static'>1</vcpu>
>> + <os>
>> + <type arch='i686' machine='pc'>hvm</type>
>> + <boot dev='hd'/>
>> + </os>
>> + <clock offset='utc'/>
>> + <on_poweroff>destroy</on_poweroff>
>> + <on_reboot>restart</on_reboot>
>> + <on_crash>destroy</on_crash>
>> + <devices>
>> + <emulator>/usr/bin/qemu</emulator>
>> + <disk type='block' device='disk'>
>> + <source dev='/dev/HostVG/QEMUGuest1'/>
>> + <target dev='hda' bus='ide'/>
>> + <address type='drive' controller='0' bus='0' target='0' unit='0'/>
>> + </disk>
>> + <controller type='usb' index='0'/>
>> + <controller type='ide' index='0'/>
>> + <controller type='pci' index='0' model='pci-root'/>
>> + <input type='mouse' bus='ps2'/>
>> + <input type='keyboard' bus='ps2'/>
>> + <graphics type='vnc' port='5903' autoport='no'>
>> + <listen type='network' network='Bobsnetwork' family='ipv6'/>
>> + </graphics>
>> + <video>
>> + <model type='cirrus' vram='16384' heads='1'/>
>> + </video>
>> + <memballoon model='virtio'/>
>> + </devices>
>> +</domain>
> Also create an -ipv4 specific file...
>
>> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network.xml
>> index bf78ca8..3b5c2de 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='ipv4'/>
>> </graphics>
>> <video>
>> <model type='cirrus' vram='16384' heads='1'/>
> Revert this to having no change
>
>> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-listen-network2.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-listen-network2.xml
>> index abee7b6..2b2d78a 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='ipv4'/>
>> </graphics>
>> <video>
>> <model type='cirrus' vram='16384' heads='1'/>
> Revert to having no change
>
>> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
>> index 99d4629..c966eb5 100644
>> --- a/tests/qemuxml2xmltest.c
>> +++ b/tests/qemuxml2xmltest.c
>> @@ -248,6 +248,7 @@ mymain(void)
>> DO_TEST_FULL("disk-mirror", true, WHEN_INACTIVE);
>> DO_TEST_FULL("disk-active-commit", false, WHEN_ACTIVE);
>> DO_TEST("graphics-listen-network");
> Add
> DO_TEST("graphics-listen-network-ipv4");
>
> and of course the corresponding -ipv4.xml file.
Yes, need add a test for ipv4 and no need fix the existing tests if we
won't auto generate family='ipv4'.
Thanks for your review.
>> + DO_TEST("graphics-listen-network-ipv6");
>> DO_TEST("graphics-vnc");
>> DO_TEST("graphics-vnc-websocket");
>> DO_TEST("graphics-vnc-sasl");
>>
>
> John
Luyao
More information about the libvir-list
mailing list