Re: [libvirt PATCH 0/4] move virParseVersionString to virstring.c

[Laine Stump]

On 1/28/22 3:58 PM, Ján Tomko wrote:

And clean up some includes while doing it.

Ján Tomko (4):
   maint: add required includes
   util: virParseVersionString: move to virstring.c
   virParseVersionString: rename to virStringParseVersion
   maint: remove unnecessary virutil.h includes


Reviewed-by: Laine Stump 

but beware the slippery slope (e.g. - if you're going to move this one, 
then what about virIndexToDiskName and virDiskNameToIndex - they just 
operate on a string too...). Do we really want such specific functions 
in a file made for generally useful string functions? (Maybe we do, I 
don't have a strong opinion one way or the other, just thought I'd bring 
it up)





  src/bhyve/bhyve_driver.c  |  2 +-
  src/ch/ch_conf.c  |  2 +-
  src/esx/esx_vi.c  |  9 ++---
  src/libvirt_private.syms  |  2 +-
  src/lxc/lxc_driver.c  |  2 +-
  src/nwfilter/nwfilter_ebiptables_driver.c |  6 +--
  src/openvz/openvz_conf.c  |  3 +-
  src/util/virdnsmasq.c |  3 +-
  src/util/virfirewalld.c   |  4 +-
  src/util/virstring.c  | 48 +++
  src/util/virstring.h  |  4 ++
  src/util/virutil.c| 46 --
  src/util/virutil.h|  3 --
  src/vbox/vbox_common.c|  2 +-
  src/vmware/vmware_conf.c  |  3 +-
  src/vz/vz_utils.c |  2 +-
  tests/testutilsqemu.c |  3 +-
  tests/utiltest.c  |  2 +-
  tools/virt-host-validate-common.c |  2 +-
  19 files changed, 73 insertions(+), 75 deletions(-)

Re: [libvirt PATCH] conf: network: remove hostname validation

[Laine Stump]

On 1/27/22 4:33 PM, Ján Tomko wrote:

We used to validate that the first character of the hostname is a
letter. Later, RFC1123 relaxed the requirements to allow a number
as well.

Drop the validation completely, since we do not care about the
following characters, and neither does dnsmasq (even if it's a comma,
which is a delimiter in the hosts file).


Was there some discussion somewhere that prompted this patch (and thus 
invalidates the opinion I'm about to spout)? The only email I could find 
about it was the email of the "reverted" patch itself (sent by Peter on 
behalf of the author, with r-b given in the same email).


My opinion is that if a current RFC restricts the first letter of a 
hostname, then we should validate that too, *especially* if dnsmasq 
doesn't; who knows what entity beyond dnsmasq will barf on it in some 
way, and the closer to the source the non-compliance is reported, the 
easier it will be to fix. (Additionally, it's easy to remove extra 
validation, but much more difficult to add it back later if we decide it 
shouldn't have been removed)





Reverts: 673b74be5fda928da5e9f3c2cfbf6c1cb1eda0c6
Signed-off-by: Ján Tomko 
---
  src/conf/network_conf.c | 6 --
  1 file changed, 6 deletions(-)

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index c769bbaeb5..8f50e22be5 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -548,12 +548,6 @@ virNetworkDHCPHostDefParseXML(const char *networkName,
  }
  
  name = virXMLPropString(node, "name");

-if (name && !(g_ascii_isalpha(name[0]) || g_ascii_isdigit(name[0]))) {
-virReportError(VIR_ERR_XML_ERROR,
-   _("Cannot use host name '%s' in network '%s'"),
-   name, networkName);
-return -1;
-}
  
  ip = virXMLPropString(node, "ip");

  if (ip && (virSocketAddrParse(, ip, AF_UNSPEC) < 0)) {