On Wed, Oct 07, 2009 at 10:05:40AM -0400, Laine Stump wrote:
From: root <root(a)vlap.laine.org>
This patch adds the flag VIR_INTERFACE_XML_INACTIVE to
virInterfaceGetXMLDesc's flags. When it is*not* set (the default), the
live interface info will be returned in the XML (in particular, the IP
address(es) and netmask(s) will be retrieved by querying the interface
directly, rather than reporting what's in the config file). The
backend of this is in netcf's ncf_if_xml_state() function.
Note that you need at least netcf 0.1.2 for this to work correctly.
The configure.ac pkgconfig check for netcf should be updated to
match the new minimal requirement here, otherwise people will get
a compile error, rather than a clear message about required version.
Similarly for the BuildRequires in libvirt.spec.in
@@ -1005,6 +1002,7 @@ virInterfaceVlanDefFormat(virConnectPtr conn,
virBufferPtr buf,
static int
virInterfaceProtocolDefFormat(virConnectPtr conn ATTRIBUTE_UNUSED,
virBufferPtr buf, const virInterfaceDefPtr def) {
+ char prefixStr[16];
if (def->proto.family == NULL)
return(0);
virBufferVSprintf(buf, " <protocol family='%s'>\n",
def->proto.family);
@@ -1015,20 +1013,20 @@ virInterfaceProtocolDefFormat(virConnectPtr conn
ATTRIBUTE_UNUSED,
virBufferAddLit(buf, " <dhcp
peerdns='yes'/>\n");
else
virBufferAddLit(buf, " <dhcp/>\n");
- } else {
- /* theorically if we don't have dhcp we should have an address */
- if (def->proto.address != NULL) {
- if (def->proto.prefix != 0)
- virBufferVSprintf(buf, " <ip address='%s'
prefix='%d'/>\n",
- def->proto.address, def->proto.prefix);
- else
- virBufferVSprintf(buf, " <ip
address='%s'/>\n",
- def->proto.address);
- }
- if (def->proto.gateway != NULL) {
- virBufferVSprintf(buf, " <route
gateway='%s'/>\n",
- def->proto.gateway);
- }
+ }
+ if (def->proto.address != NULL) {
+ if (def->proto.prefix != 0)
+ snprintf(prefixStr, sizeof(prefixStr), "%d",
def->proto.prefix);
+
+ virBufferVSprintf(buf, " <ip
address='%s'%s%s%s%s%s%s/>\n",
+ def->proto.address,
+ def->proto.prefix ? " prefix='" :
"",
+ def->proto.prefix ? prefixStr : "",
+ def->proto.prefix ? "'" :
"");
+ }
This is a rather unreadable way to format the XML, and has a rather
redundant extra snprintf call there. I think it'd look better as
virBufferVSprintf(buf, " <ip address='%s'",
def->proto.address);
if (def->proto.prefix)
virBufferVSprintf(buf, " prefix='%d'", def->proto.prefix);
virBufferVSprintf(buf, "/>\n");
But should prefix really be optional ? You really need a prefix to
meaningfully interpret an address. If it is left out of the original
sysconfig file in /etc/sysconfig/network-scripts, there are defined
default values that should be used. IMHO, libvirt output XML should
always include the prefix, otherwise applications all have to add
the logic to calculate default prefix values themselves. So I'd
prefer to see prefix mandatory in the XML we output, optional in
the XML we accept for input.
Daniel
--
|: Red Hat, Engineering, London -o-
http://people.redhat.com/berrange/ :|
|:
http://libvirt.org -o-
http://virt-manager.org -o-
http://ovirt.org :|
|:
http://autobuild.org -o-
http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|