[RFCv3 08/25] util: Add helper aliases and functions for 'bool' and 'time_t' to cooperate with xmlgen

[Shi Lei]

Add three aliases virBoolYesNo, virBoolOnOff and virBoolTrueFalse for
type 'bool'. The tool xmlgen depends on them to determine their ture
values in XML. Also, add corresponding functions to parse them.

Add virStrToTime and virTimeFormatBuf to convert 'time_t' and 'string'.

Add virStrToLong_u8p to convert 'string' to 'uint_8'.

Signed-off-by: Shi Lei 
---
 src/internal.h   |   8 +++
 src/libvirt_private.syms |   5 ++
 src/util/virstring.c | 102 +++
 src/util/virstring.h |  15 ++
 4 files changed, 130 insertions(+)

diff --git a/src/internal.h b/src/internal.h
index 0a03dfc4..3342934f 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -529,3 +529,11 @@ enum {
 # define fprintf(fh, ...) g_fprintf(fh, __VA_ARGS__)
 
 #endif /* VIR_NO_GLIB_STDIO */
+
+/* These are aliases for bool.
+ * The xmlgen depends on them to determine their true values
+ * in XML.
+ */
+typedef bool virBoolYesNo;  /* True values: 'yes', 'no' */
+typedef bool virBoolOnOff;  /* True values: 'on', 'off' */
+typedef bool virBoolTrueFalse;  /* True values: 'true', 'false' */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index fad0ff71..e78491dc 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -3275,6 +3275,9 @@ virStringStripIPv6Brackets;
 virStringStripSuffix;
 virStringToUpper;
 virStringTrimOptionalNewline;
+virStrToBoolOnOff;
+virStrToBoolTrueFalse;
+virStrToBoolYesNo;
 virStrToDouble;
 virStrToLong_i;
 virStrToLong_l;
@@ -3285,6 +3288,8 @@ virStrToLong_ul;
 virStrToLong_ull;
 virStrToLong_ullp;
 virStrToLong_ulp;
+virStrToTime;
+virTimeFormatBuf;
 virTrimSpaces;
 
 
diff --git a/src/util/virstring.c b/src/util/virstring.c
index a2c07e5c..ac6c0aae 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -233,6 +233,21 @@ virStrToLong_ulp(char const *s, char **end_ptr, int base,
 return 0;
 }
 
+/* Just like virStrToLong_uip, above, but produce an "uint8_t" value.
+ * This version rejects any negative signs.  */
+int
+virStrToLong_u8p(char const *s, char **end_ptr, int base, uint8_t *result)
+{
+unsigned int val;
+int ret;
+ret = virStrToLong_uip(s, end_ptr, base, &val);
+if (ret < 0 || val > 0xff)
+return -1;
+
+*result = (uint8_t) val;
+return 0;
+}
+
 /* Just like virStrToLong_i, above, but produce a "long long" value.  */
 int
 virStrToLong_ll(char const *s, char **end_ptr, int base, long long *result)
@@ -1077,3 +1092,90 @@ int virStringParseYesNo(const char *str, bool *result)
 
 return 0;
 }
+
+
+int
+virStrToBoolYesNo(const char *str, bool *result)
+{
+if (STREQ(str, "yes"))
+*result = true;
+else if (STREQ(str, "no"))
+*result = false;
+else
+return -1;
+
+return 0;
+}
+
+
+int
+virStrToBoolOnOff(const char *str, bool *result)
+{
+if (STREQ(str, "on"))
+*result = true;
+else if (STREQ(str, "off"))
+*result = false;
+else
+return -1;
+
+return 0;
+}
+
+
+int
+virStrToBoolTrueFalse(const char *str, bool *result)
+{
+if (STREQ(str, "true"))
+*result = true;
+else if (STREQ(str, "false"))
+*result = false;
+else
+return -1;
+
+return 0;
+}
+
+
+int virStrToTime(const char *str, time_t *result)
+{
+g_autoptr(GDateTime) then = NULL;
+g_autoptr(GTimeZone) tz = g_time_zone_new_utc();
+char *tmp;
+int year, mon, mday, hour, min, sec;
+
+/* Expect: -MM-DDTHH:MM:SS (%d-%d-%dT%d:%d:%d)  eg 2010-11-28T14:29:01 
*/
+if (/* year */
+virStrToLong_i(str, &tmp, 10, &year) < 0 || *tmp != '-' ||
+/* month */
+virStrToLong_i(tmp+1, &tmp, 10, &mon) < 0 || *tmp != '-' ||
+/* day */
+virStrToLong_i(tmp+1, &tmp, 10, &mday) < 0 || *tmp != 'T' ||
+/* hour */
+virStrToLong_i(tmp+1, &tmp, 10, &hour) < 0 || *tmp != ':' ||
+/* minute */
+virStrToLong_i(tmp+1, &tmp, 10, &min) < 0 || *tmp != ':' ||
+/* second */
+virStrToLong_i(tmp+1, &tmp, 10, &sec) < 0 || *tmp != '\0') {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("invalid time '%s', expect -MM-DDTHH:MM:SS"),
+   str);
+return -1;
+}
+
+then = g_date_time_new(tz, year, mon, mday, hour, min, sec);
+*result = (time_t)g_date_time_to_unix(then);
+return 0;
+}
+
+
+int
+virTimeFormatBuf(virBuffer *buf, const char *layout, const time_t time)
+{
+g_autoptr(GDateTime) then = NULL;
+g_autofree char *thenstr = NULL;
+
+then = g_date_time_new_from_unix_utc(time);
+thenstr = g_date_time_format(then, "%Y-%m-%dT%H:%M:%S");
+virBufferAsprintf(buf, layout, thenstr);
+return 0;
+}
diff --git a/src/util/virstring.h b/src/util/virstring.h
index 7cc1d8c5..e16da824 100644
--- a/src/util/virstring.h
+++ b/src/util/virstring.h
@@ -19,6 +19,7 @@
 #pragma once
 
 #include "internal.h"
+#include "virbuffer.h"
 
 #define VIR_INT64_STR_BUFLEN 21
 
@@ -5

[RFCv3 19/25] conf: Generate virNetworkDNSHostDefFormatBuf

[Shi Lei]

Signed-off-by: Shi Lei 
---
 src/conf/network_conf.c | 14 +++---
 src/conf/network_conf.h |  2 +-
 2 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 90b1e0ee..ba67eab1 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -2095,7 +2095,7 @@ static int
 virNetworkDNSDefFormat(virBuffer *buf,
const virNetworkDNSDef *def)
 {
-size_t i, j;
+size_t i;
 
 if (!(def->enable || def->forwardPlainNames || def->nfwds || def->nhosts ||
   def->nsrvs || def->ntxts))
@@ -2164,16 +2164,8 @@ virNetworkDNSDefFormat(virBuffer *buf,
 
 if (def->nhosts) {
 for (i = 0; i < def->nhosts; i++) {
-g_autofree char *ip = virSocketAddrFormat(&def->hosts[i].ip);
-
-virBufferAsprintf(buf, "\n", ip);
-virBufferAdjustIndent(buf, 2);
-for (j = 0; j < def->hosts[i].nnames; j++)
-virBufferEscapeString(buf, "%s\n",
-  def->hosts[i].names[j]);
-
-virBufferAdjustIndent(buf, -2);
-virBufferAddLit(buf, "\n");
+if (virNetworkDNSHostDefFormatBuf(buf, "host", &def->hosts[i], 
def, NULL) < 0)
+return -1;
 }
 }
 virBufferAdjustIndent(buf, -2);
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 998411be..836d088d 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -142,7 +142,7 @@ struct _virNetworkDNSSrvDef {   /* genparse, genformat */
 };
 
 typedef struct _virNetworkDNSHostDef virNetworkDNSHostDef;
-struct _virNetworkDNSHostDef {  /* genparse */
+struct _virNetworkDNSHostDef {  /* genparse, genformat */
 virSocketAddr ip;   /* xmlattr */
 size_t nnames;
 char **names;   /* xmlelem:hostname, array */
-- 
2.25.1

[RFCv3 24/25] conf: Replace virNetworkDNSDefParseXML(hardcoded) with namesake(generated)

[Shi Lei]

Reorder the members of virNetworkDNSDef according to their orders in
format function.

Signed-off-by: Shi Lei 
---
 src/conf/network_conf.c | 160 +---
 src/conf/network_conf.h |  19 ++---
 2 files changed, 12 insertions(+), 167 deletions(-)

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 19408987..364c10e2 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -174,32 +174,6 @@ virNetworkIPDefClear(virNetworkIPDef *def)
 }
 
 
-static void
-virNetworkDNSDefClear(virNetworkDNSDef *def)
-{
-if (def->forwarders) {
-while (def->nfwds)
-virNetworkDNSForwarderClear(&def->forwarders[--def->nfwds]);
-VIR_FREE(def->forwarders);
-}
-if (def->txts) {
-while (def->ntxts)
-virNetworkDNSTxtDefClear(&def->txts[--def->ntxts]);
-VIR_FREE(def->txts);
-}
-if (def->hosts) {
-while (def->nhosts)
-virNetworkDNSHostDefClear(&def->hosts[--def->nhosts]);
-VIR_FREE(def->hosts);
-}
-if (def->srvs) {
-while (def->nsrvs)
-virNetworkDNSSrvDefClear(&def->srvs[--def->nsrvs]);
-VIR_FREE(def->srvs);
-}
-}
-
-
 static void
 virNetworkForwardDefClear(virNetworkForwardDef *def)
 {
@@ -883,7 +857,7 @@ virNetworkDNSForwarderParseHook(xmlNodePtr node 
G_GNUC_UNUSED,
 }
 
 
-static int
+int
 virNetworkDNSDefParseHook(xmlNodePtr node G_GNUC_UNUSED,
   virNetworkDNSDef *def,
   const char *networkName,
@@ -908,136 +882,6 @@ virNetworkDNSDefParseHook(xmlNodePtr node G_GNUC_UNUSED,
 }
 
 
-static int
-virNetworkDNSDefParseXML(const char *networkName,
- xmlNodePtr node,
- xmlXPathContextPtr ctxt,
- virNetworkDNSDef *def)
-{
-g_autofree xmlNodePtr *hostNodes = NULL;
-g_autofree xmlNodePtr *srvNodes = NULL;
-g_autofree xmlNodePtr *txtNodes = NULL;
-g_autofree xmlNodePtr *fwdNodes = NULL;
-g_autofree char *forwardPlainNames = NULL;
-g_autofree char *enable = NULL;
-int nhosts, nsrvs, ntxts, nfwds;
-size_t i;
-VIR_XPATH_NODE_AUTORESTORE(ctxt)
-
-ctxt->node = node;
-
-enable = virXPathString("string(./@enable)", ctxt);
-if (enable) {
-def->enable = virTristateBoolTypeFromString(enable);
-if (def->enable <= 0) {
-virReportError(VIR_ERR_XML_ERROR,
-   _("Invalid dns enable setting '%s' "
- "in network '%s'"),
-   enable, networkName);
-return -1;
-}
-}
-
-forwardPlainNames = virXPathString("string(./@forwardPlainNames)", ctxt);
-if (forwardPlainNames) {
-def->forwardPlainNames = 
virTristateBoolTypeFromString(forwardPlainNames);
-if (def->forwardPlainNames <= 0) {
-virReportError(VIR_ERR_XML_ERROR,
-   _("Invalid dns forwardPlainNames setting '%s' "
- "in network '%s'"),
-   forwardPlainNames, networkName);
-return -1;
-}
-}
-
-nfwds = virXPathNodeSet("./forwarder", ctxt, &fwdNodes);
-if (nfwds < 0) {
-virReportError(VIR_ERR_XML_ERROR,
-   _("invalid  element found in  of 
network %s"),
-   networkName);
-return -1;
-}
-if (nfwds > 0) {
-def->forwarders = g_new0(virNetworkDNSForwarder, nfwds);
-
-for (i = 0; i < nfwds; i++) {
-if (virNetworkDNSForwarderParseXML(fwdNodes[i],
-   &def->forwarders[i],
-   networkName,
-   def,
-   NULL) < 0)
-return -1;
-
-def->nfwds++;
-}
-}
-
-nhosts = virXPathNodeSet("./host", ctxt, &hostNodes);
-if (nhosts < 0) {
-virReportError(VIR_ERR_XML_ERROR,
-   _("invalid  element found in  of network 
%s"),
-   networkName);
-return -1;
-}
-if (nhosts > 0) {
-def->hosts = g_new0(virNetworkDNSHostDef, nhosts);
-
-for (i = 0; i < nhosts; i++) {
-if (virNetworkDNSHostDefParseXML(hostNodes[i], 
&def->hosts[def->nhosts],
- networkName, def, NULL) < 0) {
-return -1;
-}
-def->nhosts++;
-}
-}
-
-nsrvs = virXPathNodeSet("./srv", ctxt, &srvNodes);
-if (nsrvs < 0) {
-virReportError(VIR_ERR_XML_ERROR,
-   _("invalid  element found in  of network %s"),
-   networkName);
-return -1;
-}
-if (nsrvs > 0) {
-def->srvs = g_new0(virNetworkDNSSrvDef, nsrvs);
-
-for (i = 0; i < nsrvs; i++) {
-if (virNetworkDNSSrvDefPa

[RFCv3 18/25] conf: Replace virNetworkDNSHostDefParseXML(hardcoded) with namesake(generated)

[Shi Lei]

Signed-off-by: Shi Lei 
---
 src/conf/network_conf.c | 71 -
 src/conf/network_conf.h |  7 ++--
 2 files changed, 10 insertions(+), 68 deletions(-)

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index b326ef5f..90b1e0ee 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -174,15 +174,6 @@ virNetworkIPDefClear(virNetworkIPDef *def)
 }
 
 
-static void
-virNetworkDNSHostDefClear(virNetworkDNSHostDef *def)
-{
-while (def->nnames)
-VIR_FREE(def->names[--def->nnames]);
-VIR_FREE(def->names);
-}
-
-
 static void
 virNetworkDNSForwarderClear(virNetworkDNSForwarder *def)
 {
@@ -675,7 +666,7 @@ virNetworkDHCPDefParseXML(const char *networkName,
 }
 
 
-static int
+int
 virNetworkDNSHostDefParseHook(xmlNodePtr node G_GNUC_UNUSED,
   virNetworkDNSHostDef *def,
   const char *networkName,
@@ -717,58 +708,6 @@ virNetworkDNSHostDefParseHook(xmlNodePtr node 
G_GNUC_UNUSED,
 }
 
 
-static int
-virNetworkDNSHostDefParseXML(const char *networkName,
- xmlNodePtr node,
- virNetworkDNSHostDef *def,
- bool partialOkay)
-{
-xmlNodePtr cur;
-g_autofree char *ip = NULL;
-
-ip = virXMLPropString(node, "ip");
-if (ip && (virSocketAddrParse(&def->ip, ip, AF_UNSPEC) < 0)) {
-virReportError(VIR_ERR_XML_DETAIL,
-   _("Invalid IP address in network '%s' DNS HOST record"),
-   networkName);
-goto error;
-}
-
-cur = node->children;
-while (cur != NULL) {
-if (cur->type == XML_ELEMENT_NODE &&
-virXMLNodeNameEqual(cur, "hostname")) {
-  if (cur->children != NULL) {
-  g_autofree char *name = virXMLNodeContentString(cur);
-
-  if (!name)
-  goto error;
-
-  if (!name[0]) {
-  virReportError(VIR_ERR_XML_DETAIL,
- _("Missing hostname in network '%s' DNS 
HOST record"),
- networkName);
-  goto error;
-  }
-  if (VIR_APPEND_ELEMENT(def->names, def->nnames, name) < 0)
-  goto error;
-  }
-}
-cur = cur->next;
-}
-
-if (virNetworkDNSHostDefParseHook(node, def, networkName, def, 
&partialOkay,
-  ip, def->nnames) < 0)
-goto error;
-
-return 0;
-
- error:
-virNetworkDNSHostDefClear(def);
-return -1;
-}
-
-
 /* This includes all characters used in the names of current
  * /etc/services and /etc/protocols files (on Fedora 20), except ".",
  * which we can't allow because it would conflict with the use of "."
@@ -1017,8 +956,8 @@ virNetworkDNSDefParseXML(const char *networkName,
 def->hosts = g_new0(virNetworkDNSHostDef, nhosts);
 
 for (i = 0; i < nhosts; i++) {
-if (virNetworkDNSHostDefParseXML(networkName, hostNodes[i],
- &def->hosts[def->nhosts], false) 
< 0) {
+if (virNetworkDNSHostDefParseXML(hostNodes[i], 
&def->hosts[def->nhosts],
+ networkName, def, NULL) < 0) {
 return -1;
 }
 def->nhosts++;
@@ -3398,6 +3337,7 @@ virNetworkDefUpdateDNSHost(virNetworkDef *def,
 bool isAdd = (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST ||
   command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST);
 int foundCt = 0;
+bool notAdd;
 
 memset(&host, 0, sizeof(host));
 
@@ -3411,7 +3351,8 @@ virNetworkDefUpdateDNSHost(virNetworkDef *def,
 if (virNetworkDefUpdateCheckElementName(def, ctxt->node, "host") < 0)
 goto cleanup;
 
-if (virNetworkDNSHostDefParseXML(def->name, ctxt->node, &host, !isAdd) < 0)
+notAdd = !isAdd;
+if (virNetworkDNSHostDefParseXML(ctxt->node, &host, def->name, def, 
¬Add) < 0)
 goto cleanup;
 
 for (i = 0; i < dns->nhosts; i++) {
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 052ccb58..998411be 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -142,10 +142,10 @@ struct _virNetworkDNSSrvDef {   /* genparse, genformat */
 };
 
 typedef struct _virNetworkDNSHostDef virNetworkDNSHostDef;
-struct _virNetworkDNSHostDef {
-virSocketAddr ip;
+struct _virNetworkDNSHostDef {  /* genparse */
+virSocketAddr ip;   /* xmlattr */
 size_t nnames;
-char **names;
+char **names;   /* xmlelem:hostname, array */
 };
 
 
@@ -429,6 +429,7 @@ virNetworkDefUpdateSection(virNetworkDef *def,
 
 VIR_ENUM_DECL(virNetworkTaint);
 
+#define ENABLE_VIR_NETWORK_DNSHOST_DEF_PARSE_HOOK
 #define ENABLE_VIR_NETWORK_DNSSRV_DEF_PARSE_HOOK
 #define ENABLE_VIR_NETWORK_DNSTXT_DEF_PARSE_HOOK
 #include "network_conf.generated.

[RFCv3 02/25] maint: Check python3-clang and libclang

[Shi Lei]

Make sure 'python3-clang' and 'libclang' have been installed
and can work. Also, add 'python3-clang' into libvirt.spec.in
and mingw-libvirt.spec.in.

Signed-off-by: Shi Lei 
---
 libvirt.spec.in   |  1 +
 meson.build   | 10 ++
 mingw-libvirt.spec.in |  1 +
 3 files changed, 12 insertions(+)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index be74964b..4ebd67ce 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -277,6 +277,7 @@ BuildRequires: perl-interpreter
 BuildRequires: perl
 %endif
 BuildRequires: python3
+BuildRequires: python3-clang
 BuildRequires: systemd-units
 %if %{with_libxl}
 BuildRequires: xen-devel
diff --git a/meson.build b/meson.build
index 837955de..a99be250 100644
--- a/meson.build
+++ b/meson.build
@@ -2406,3 +2406,13 @@ if conf.has('WITH_QEMU')
   }
   summary(priv_summary, section: 'Privileges')
 endif
+
+py3_clang = run_command('python3', '-c', 'import clang.cindex;print("ok")')
+if py3_clang.returncode() != 0
+  error('python3-clang is required.')
+endif
+
+py3_clang_working = run_command('python3', '-c', 'import 
clang.cindex;clang.cindex.Index.create()')
+if py3_clang_working.returncode() != 0
+  error('python3-clang is present, but not working. Perhaps libclang is 
missing?')
+endif
diff --git a/mingw-libvirt.spec.in b/mingw-libvirt.spec.in
index 288f533d..00b54d4a 100644
--- a/mingw-libvirt.spec.in
+++ b/mingw-libvirt.spec.in
@@ -52,6 +52,7 @@ BuildRequires:  pkgconfig
 BuildRequires:  gettext
 BuildRequires:  libxslt
 BuildRequires:  python3
+BuildRequires:  python3-clang
 BuildRequires:  perl-interpreter
 BuildRequires:  perl(Getopt::Long)
 BuildRequires:  meson
-- 
2.25.1

[RFCv3 13/25] conf: Generate virNetworkDNSTxtDefFormatBuf

[Shi Lei]

Signed-off-by: Shi Lei 
---
 src/conf/network_conf.c | 4 ++--
 src/conf/network_conf.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index a6c2f11a..bb976a78 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -2199,8 +2199,8 @@ virNetworkDNSDefFormat(virBuffer *buf,
 }
 
 for (i = 0; i < def->ntxts; i++) {
-virBufferEscapeString(buf, "txts[i].name);
-virBufferEscapeString(buf, "value='%s'/>\n", def->txts[i].value);
+if (virNetworkDNSTxtDefFormatBuf(buf, "txt", &def->txts[i], def, NULL) 
< 0)
+return -1;
 }
 
 for (i = 0; i < def->nsrvs; i++) {
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index f5720e5e..a4c83b46 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -125,7 +125,7 @@ struct _virNetworkDHCPHostDef {
 };
 
 typedef struct _virNetworkDNSTxtDef virNetworkDNSTxtDef;
-struct _virNetworkDNSTxtDef {   /* genparse */
+struct _virNetworkDNSTxtDef {   /* genparse, genformat */
 char *name; /* xmlattr, required */
 char *value;/* xmlattr */
 };
-- 
2.25.1

[RFCv3 04/25] docs: Add xmlgen.rst to explain how to use it

[Shi Lei]

Signed-off-by: Shi Lei 
---
 docs/meson.build |   1 +
 docs/xmlgen.rst  | 684 +++
 2 files changed, 685 insertions(+)
 create mode 100644 docs/xmlgen.rst

diff --git a/docs/meson.build b/docs/meson.build
index f550629d..a8a58815 100644
--- a/docs/meson.build
+++ b/docs/meson.build
@@ -124,6 +124,7 @@ docs_rst_files = [
   'programming-languages',
   'styleguide',
   'submitting-patches',
+  'xmlgen',
 ]
 
 # list of web targets to build for docs/web rule
diff --git a/docs/xmlgen.rst b/docs/xmlgen.rst
new file mode 100644
index ..caea1f99
--- /dev/null
+++ b/docs/xmlgen.rst
@@ -0,0 +1,684 @@
+==
+xmlgen
+==
+
+In libvirt, developers usually have to implement and maintain parse/format 
functions. This tool xmlgen aims to generate most of these functions 
automatically and help relieve developers' burden.
+
+A Quick Start
+=
+
+Take virNetworkDNSDef for example, which is in 'src/conf/network_conf.h'.
+
+In the past, we have to manually implement virNetworkDNSDefParseXML and 
virNetworkDNSFormatBuf.
+And now, we can just take several steps to have xmlgen generate these 
functions and apply them into libvirt project.
+
+Step1. Add directives on the struct's declaration.
+--
+
+Directives for xmlgen are used to help direct the generating process. As below:
+
+ ::
+
+  typedef struct _virNetworkDNSDef virNetworkDNSDef;
+  struct _virNetworkDNSDef {  /* genparse, genformat */
+  virTristateBool enable; /* xmlattr */
+  virTristateBool forwardPlainNames;  /* xmlattr */
+  size_t nfwds;
+  virNetworkDNSForwarder *forwarders; /* xmlelem, array:nfwds */
+  size_t ntxts;
+  virNetworkDNSTxtDef *txts;  /* xmlelem, array */
+  ... ...
+  };
+
+On the line of struct's declaration, we set two directives **genparse** and 
**genformat**, which direct xmlgen to generate parse/format functions for this 
struct respectively. In this example, these functions include 
virNetworkDNSDefParseXML, virNetworkDNSDefFormatBuf and some auxilliary 
functions. Other directives are for members. They direct xmlgen to generate 
code blocks in the parse/format functions. Directive **xmlattr** indicates that 
the member matches an xml attribute, and **xmlelem** is for an xml element. 
Additional directive **array** indicates that the member matches an array of 
xml node.
+
+Step2. Preview generated functions.
+---
+
+By the below command line:
+
+ ::
+
+  # ./scripts/xmlgen/go list
+
+Got a list of structs detected by xmlgen, including *virNetworkDNSDef*.
+Then we execute the command line as below:
+
+ ::
+
+  # ./scripts/xmlgen/go show virNetworkDNSDef
+
+All the generated functions related to virNetworkDNSDef are displayed, then we 
ought to preview them before really use them into libvirt project.
+There is a special part **[Tips]** except the generated functions and the 
declaration of hooks.
+
+[**Tips**] provides instructions about how to apply these generated functions 
into project and how to enable hooks. [**Tips**] will be used in step3.
+
+Also, we got the declaration of hooks. In step4, we will implement a hook 
according to it.
+
+Step3. Enable hooks and include generated functions.
+
+
+According to [**Tips**] that we got in step2:
+
+ ::
+
+  [Tips]
+
+  /* Put these lines at the bottom of "conf/network_conf.h" */
+  /* Makesure "network_conf.h" to be appended into conf_xmlgen_input in 
src/conf/meson.build */
+
+  /* Define macro to enable hook or redefine check when necessary */
+  /* #define ENABLE_VIR_NETWORK_DNSDEF_PARSE_HOOK */
+  /* #define ENABLE_VIR_NETWORK_DNSDEF_PARSE_HOOK_SET_ARGS */
+  /* #define ENABLE_VIR_NETWORK_DNSDEF_FORMAT_HOOK */
+
+  /* #define RESET_VIR_NETWORK_DNSDEF_CHECK */
+
+  /* Makesure below is the bottom line! */
+  #include "network_conf.generated.h"
+
+Uncomment macros and enable hooks when necessary. In this example, we only 
need to define ENABLE_VIR_NETWORK_DNSDEF_PARSE_HOOK to enable the post-hook for 
parse-function.
+
+Include the header-file to apply the generated functions.
+In this example, we just include "network_conf.generated.h" into 
"conf/network_conf.h" and modify "src/conf/meson.build" as instructed.
+
+Step4. Implement hooks when necessary.
+--
+
+In original implementation of virNetworkDNSDefParseXML, there's a piece of 
error-checking code as below:
+
+ ::
+
+  if (def->enable == VIR_TRISTATE_BOOL_NO && (nfwds || nhosts || nsrvs || 
ntxts)) {
+  virReportError(VIR_ERR_XML_ERROR,
+  _("Extra data in disabled network '%s'"),
+  networkName);
+  }
+
+Since this piece of code can't be generated, we need a hook to do the check.
+In step2, we have gotten the declaration of virNetworkDNSDefParseHook, as 
below:
+
+ 

[RFCv3 01/25] scripts: Add a tool to generate xml parse/format functions

[Shi Lei]

This tool is used to generate parsexml/formatbuf/clear functions.
It is based on libclang and its python-binding.
Some directives (such as genparse, xmlattr, etc.) need to be added on
the declarations of structs to direct the tool.

Signed-off-by: Shi Lei 
---
 po/POTFILES.in  |1 +
 scripts/xmlgen/directive.py | 1192 +++
 scripts/xmlgen/go   |   29 +
 scripts/xmlgen/main.py  |  534 
 scripts/xmlgen/utils.py |  121 
 5 files changed, 1877 insertions(+)
 create mode 100644 scripts/xmlgen/directive.py
 create mode 100755 scripts/xmlgen/go
 create mode 100755 scripts/xmlgen/main.py
 create mode 100644 scripts/xmlgen/utils.py

diff --git a/po/POTFILES.in b/po/POTFILES.in
index 413783ee..9740bb2b 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -5,6 +5,7 @@
 @BUILDDIR@src/admin/admin_server_dispatch_stubs.h
 @BUILDDIR@src/remote/remote_client_bodies.h
 @BUILDDIR@src/remote/remote_daemon_dispatch_stubs.h
+@SRCDIR@scripts/xmlgen/directive.py
 @SRCDIR@src/access/viraccessdriverpolkit.c
 @SRCDIR@src/access/viraccessmanager.c
 @SRCDIR@src/admin/admin_server.c
diff --git a/scripts/xmlgen/directive.py b/scripts/xmlgen/directive.py
new file mode 100644
index ..cc8fb5aa
--- /dev/null
+++ b/scripts/xmlgen/directive.py
@@ -0,0 +1,1192 @@
+#
+# Copyright (C) 2021 Shandong Massclouds Co.,Ltd.
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library.  If not, see
+# .
+#
+
+import json
+from collections import OrderedDict
+from utils import singleton, dedup, Block, Terms, render
+
+BUILTIN_TYPES = {
+'String': {},
+'Bool': {
+'conv': 'virStrToBoolYesNo(${mdvar}, &def->${name})'
+},
+'BoolYesNo': {
+'conv': 'virStrToBoolYesNo(${mdvar}, &def->${name})'
+},
+'BoolOnOff': {
+'conv': 'virStrToBoolOnOff(${mdvar}, &def->${name})'
+},
+'BoolTrueFalse': {
+'conv': 'virStrToBoolTrueFalse(${mdvar}, &def->${name})'
+},
+'Chars': {
+'conv': 'virStrcpyStatic(def->${name}, ${name}Str)'
+},
+'UChars': {
+'conv': 'virStrcpyStatic((char *)def->${name}, ${mdvar})'
+},
+'Int': {
+'fmt': '%d',
+'conv': 'virStrToLong_i(${mdvar}, NULL, 0, &def->${name})'
+},
+'UInt': {
+'fmt': '%u',
+'conv': 'virStrToLong_uip(${mdvar}, NULL, 0, &def->${name})'
+},
+'ULong': {
+'fmt': '%lu',
+'conv': 'virStrToLong_ulp(${mdvar}, NULL, 0, &def->${name})'
+},
+'ULongLong': {
+'fmt': '%llu',
+'conv': 'virStrToLong_ullp(${mdvar}, NULL, 0, &def->${name})'
+},
+'U8': {
+'fmt': '%u',
+'conv': 'virStrToLong_u8p(${mdvar}, NULL, 0, &def->${name})'
+},
+'U32': {
+'fmt': '%u',
+'conv': 'virStrToLong_uip(${mdvar}, NULL, 0, &def->${name})'
+},
+'Time': {
+'conv': 'virStrToTime(${mdvar}, &def->${name})'
+},
+}
+
+
+@singleton
+class TypeTable(OrderedDict):
+def __init__(self):
+OrderedDict.__init__(self)
+for name, kvs in BUILTIN_TYPES.items():
+kvs['name'] = name
+kvs['meta'] = 'Builtin'
+self[name] = kvs
+
+def register(self, kvs):
+name = kvs['name']
+if name not in self:
+self[name] = kvs
+return name
+
+def get(self, name):
+if name in self:
+return self[name]
+return {'meta': 'Struct', 'name': name, 'external': True}
+
+def check(self, name):
+return name in self
+
+
+T_NAMESPACE_PARSE = [
+'if (xmlopt)',
+'def->ns = xmlopt->ns;',
+'if (def->ns.parse) {',
+'if (virXMLNamespaceRegister(ctxt, &def->ns) < 0)',
+'goto error;',
+'if ((def->ns.parse)(ctxt, &def->namespaceData) < 0)',
+'goto error;',
+'}'
+]
+
+T_NAMESPACE_FORMAT_BEGIN = [
+'if (def->namespaceData && def->ns.format)',
+'virXMLNamespaceFormatNS(buf, &def->ns);'
+]
+
+T_NAMESPACE_FORMAT_END = [
+'if (def->namespaceData && def->ns.format) {',
+'if ((def->ns.format)(buf, def->namespaceData) < 0)',
+'return -1;',
+'}'
+]
+
+
+def funcSignature(rtype, name, args, semicolon=''):
+alignment = ' ' * (len(name) + 1)
+connector = ',\n' + alignment
+
+ret = []
+ret.append(rtype)
+ret.append('%s(%s)%s' % (name, connector.join(args), semic

[RFCv3 03/25] maint: Call xmlgen automatically when c-head-files change

[Shi Lei]

Monitor changes of header-files in src/util and src/conf.
Whenever that happens, the tool xmlgen will generate
parse/format functions based on these files automatically.

Signed-off-by: Shi Lei 
---
 scripts/meson.build  |  8 
 src/conf/meson.build | 36 
 src/meson.build  |  6 ++
 src/util/meson.build | 36 
 tests/meson.build|  2 ++
 tools/meson.build|  3 +++
 6 files changed, 91 insertions(+)

diff --git a/scripts/meson.build b/scripts/meson.build
index 421e3d2a..5399868c 100644
--- a/scripts/meson.build
+++ b/scripts/meson.build
@@ -35,3 +35,11 @@ foreach name : scripts
   sname = name.split('.')[0].underscorify()
   set_variable('@0@_prog'.format(sname), find_program(name))
 endforeach
+
+xmlgen_self = files(
+  'xmlgen/main.py',
+  'xmlgen/directive.py',
+  'xmlgen/utils.py'
+)
+
+set_variable('virxmlgen_prog', find_program('xmlgen/main.py'))
diff --git a/src/conf/meson.build b/src/conf/meson.build
index bd35d87e..1439c31d 100644
--- a/src/conf/meson.build
+++ b/src/conf/meson.build
@@ -1,3 +1,38 @@
+conf_xmlgen_input = [
+]
+
+conf_xmlgen_output = []
+foreach name : conf_xmlgen_input
+  conf_xmlgen_output += '@0@.generated.c'.format(name.split('.')[0])
+  conf_xmlgen_output += '@0@.generated.h'.format(name.split('.')[0])
+endforeach
+
+conf_xmlgen_headers = []
+if conf_xmlgen_output.length() > 0
+  conf_xmlgen_objects = custom_target(
+'virxmlgen',
+input: xmlgen_self + conf_xmlgen_input,
+output: conf_xmlgen_output,
+command: [
+  meson_python_prog, python3_prog.path(), '-B', virxmlgen_prog.path(),
+  '-s', meson.source_root() / 'src', '-b', meson.build_root() / 'src',
+  '-d', 'conf', 'generate',
+],
+  )
+
+  index = 0
+  foreach header : conf_xmlgen_objects.to_list()
+if index % 2 == 1
+  conf_xmlgen_headers += header
+endif
+index += 1
+  endforeach
+else
+  conf_xmlgen_objects = []
+endif
+
+conf_xmlgen_dep = declare_dependency(sources: conf_xmlgen_headers)
+
 netdev_conf_sources = [
   'netdev_bandwidth_conf.c',
   'netdev_vlan_conf.c',
@@ -90,6 +125,7 @@ device_conf_sources = [
 virt_conf_lib = static_library(
   'virt_conf',
   [
+conf_xmlgen_objects,
 chrdev_conf_sources,
 cpu_conf_sources,
 device_conf_sources,
diff --git a/src/meson.build b/src/meson.build
index c7ff9e97..f8ae47b4 100644
--- a/src/meson.build
+++ b/src/meson.build
@@ -245,6 +245,12 @@ src_dep = declare_dependency(
 )
 
 subdir('conf')
+
+src_dep = declare_dependency(
+  dependencies: [ src_dep, util_xmlgen_dep, conf_xmlgen_dep ],
+  include_directories: [ conf_inc_dir ],
+)
+
 subdir('rpc')
 subdir('access')
 subdir('cpu')
diff --git a/src/util/meson.build b/src/util/meson.build
index 05934f68..0d41de92 100644
--- a/src/util/meson.build
+++ b/src/util/meson.build
@@ -1,3 +1,38 @@
+util_xmlgen_input = [
+]
+
+util_xmlgen_output = []
+foreach name : util_xmlgen_input
+  util_xmlgen_output += '@0@.generated.c'.format(name.split('.')[0])
+  util_xmlgen_output += '@0@.generated.h'.format(name.split('.')[0])
+endforeach
+
+util_xmlgen_headers = []
+if util_xmlgen_output.length() > 0
+  util_xmlgen_objects = custom_target(
+'virxmlgen',
+input: xmlgen_self + util_xmlgen_input,
+output: util_xmlgen_output,
+command: [
+  meson_python_prog, python3_prog.path(), '-B', virxmlgen_prog.path(),
+  '-s', meson.source_root() / 'src', '-b', meson.build_root() / 'src',
+  '-d', 'util', 'generate',
+],
+  )
+
+  index = 0
+  foreach header : util_xmlgen_objects.to_list()
+if index % 2 == 1
+  util_xmlgen_headers += header
+endif
+index += 1
+  endforeach
+else
+  util_xmlgen_objects = []
+endif
+
+util_xmlgen_dep = declare_dependency(sources: util_xmlgen_headers)
+
 util_sources = [
   'glibcompat.c',
   'viralloc.c',
@@ -179,6 +214,7 @@ io_helper_sources = [
 virt_util_lib = static_library(
   'virt_util',
   [
+util_xmlgen_objects,
 util_sources,
 util_public_sources,
 keycode_gen_sources,
diff --git a/tests/meson.build b/tests/meson.build
index 05c3e901..14ace476 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -17,6 +17,8 @@ tests_dep = declare_dependency(
 selinux_dep,
 xdr_dep,
 yajl_dep,
+util_xmlgen_dep,
+conf_xmlgen_dep,
   ],
   include_directories: [
 conf_inc_dir,
diff --git a/tools/meson.build b/tools/meson.build
index 2acf7b0a..162db0e8 100644
--- a/tools/meson.build
+++ b/tools/meson.build
@@ -5,11 +5,14 @@ tools_dep = declare_dependency(
   dependencies: [
 libxml_dep,
 glib_dep,
+util_xmlgen_dep,
+conf_xmlgen_dep,
   ],
   include_directories: [
 libvirt_inc,
 src_inc_dir,
 util_inc_dir,
+conf_inc_dir,
 top_inc_dir,
   ],
   link_args: (
-- 
2.25.1

[RFCv3 12/25] conf: Replace virNetworkDNSTxtDefParseXML(hardcoded) with namesake(generated)

[Shi Lei]

Signed-off-by: Shi Lei 
---
 po/POTFILES.in  |  1 +
 src/conf/meson.build|  1 +
 src/conf/network_conf.c | 45 ++---
 src/conf/network_conf.h |  9 ++---
 4 files changed, 14 insertions(+), 42 deletions(-)

diff --git a/po/POTFILES.in b/po/POTFILES.in
index 9740bb2b..fe20b9d7 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -3,6 +3,7 @@
 @BUILDDIR@src/access/viraccessapicheckqemu.c
 @BUILDDIR@src/admin/admin_client.h
 @BUILDDIR@src/admin/admin_server_dispatch_stubs.h
+@BUILDDIR@src/conf/network_conf.generated.c
 @BUILDDIR@src/remote/remote_client_bodies.h
 @BUILDDIR@src/remote/remote_daemon_dispatch_stubs.h
 @SRCDIR@scripts/xmlgen/directive.py
diff --git a/src/conf/meson.build b/src/conf/meson.build
index 1439c31d..f1cfe35c 100644
--- a/src/conf/meson.build
+++ b/src/conf/meson.build
@@ -1,4 +1,5 @@
 conf_xmlgen_input = [
+  'network_conf.h',
 ]
 
 conf_xmlgen_output = []
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 87157591..a6c2f11a 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -174,14 +174,6 @@ virNetworkIPDefClear(virNetworkIPDef *def)
 }
 
 
-static void
-virNetworkDNSTxtDefClear(virNetworkDNSTxtDef *def)
-{
-VIR_FREE(def->name);
-VIR_FREE(def->value);
-}
-
-
 static void
 virNetworkDNSHostDefClear(virNetworkDNSHostDef *def)
 {
@@ -886,7 +878,7 @@ virNetworkDNSSrvDefParseXML(const char *networkName,
 }
 
 
-static int
+int
 virNetworkDNSTxtDefParseHook(xmlNodePtr node G_GNUC_UNUSED,
  virNetworkDNSTxtDef *def,
  const char *networkName,
@@ -927,33 +919,6 @@ virNetworkDNSTxtDefParseHook(xmlNodePtr node G_GNUC_UNUSED,
 }
 
 
-static int
-virNetworkDNSTxtDefParseXML(const char *networkName,
-xmlNodePtr node,
-virNetworkDNSTxtDef *def,
-bool partialOkay)
-{
-if (!(def->name = virXMLPropString(node, "name"))) {
-virReportError(VIR_ERR_XML_DETAIL,
-   _("missing required name attribute in DNS TXT record "
- "of network %s"), networkName);
-goto error;
-}
-
-def->value = virXMLPropString(node, "value");
-
-if (virNetworkDNSTxtDefParseHook(node, def, networkName,
- NULL, &partialOkay) < 0)
-goto error;
-
-return 0;
-
- error:
-virNetworkDNSTxtDefClear(def);
-return -1;
-}
-
-
 static int
 virNetworkDNSDefParseXML(const char *networkName,
  xmlNodePtr node,
@@ -1077,8 +1042,8 @@ virNetworkDNSDefParseXML(const char *networkName,
 def->txts = g_new0(virNetworkDNSTxtDef, ntxts);
 
 for (i = 0; i < ntxts; i++) {
-if (virNetworkDNSTxtDefParseXML(networkName, txtNodes[i],
-&def->txts[def->ntxts], false) < 
0) {
+if (virNetworkDNSTxtDefParseXML(txtNodes[i], 
&def->txts[def->ntxts],
+networkName, def, NULL) < 0) {
 return -1;
 }
 def->ntxts++;
@@ -3614,6 +3579,7 @@ virNetworkDefUpdateDNSTxt(virNetworkDef *def,
 virNetworkDNSTxtDef txt;
 bool isAdd = (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST ||
   command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST);
+bool notAdd;
 
 memset(&txt, 0, sizeof(txt));
 
@@ -3627,7 +3593,8 @@ virNetworkDefUpdateDNSTxt(virNetworkDef *def,
 if (virNetworkDefUpdateCheckElementName(def, ctxt->node, "txt") < 0)
 goto cleanup;
 
-if (virNetworkDNSTxtDefParseXML(def->name, ctxt->node, &txt, !isAdd) < 0)
+notAdd = !isAdd;
+if (virNetworkDNSTxtDefParseXML(ctxt->node, &txt, def->name, def, ¬Add) 
< 0)
 goto cleanup;
 
 for (foundIdx = 0; foundIdx < dns->ntxts; foundIdx++) {
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index a7e6b7a2..f5720e5e 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -125,9 +125,9 @@ struct _virNetworkDHCPHostDef {
 };
 
 typedef struct _virNetworkDNSTxtDef virNetworkDNSTxtDef;
-struct _virNetworkDNSTxtDef {
-char *name;
-char *value;
+struct _virNetworkDNSTxtDef {   /* genparse */
+char *name; /* xmlattr, required */
+char *value;/* xmlattr */
 };
 
 typedef struct _virNetworkDNSSrvDef virNetworkDNSSrvDef;
@@ -428,3 +428,6 @@ virNetworkDefUpdateSection(virNetworkDef *def,
unsigned int flags);  /* virNetworkUpdateFlags */
 
 VIR_ENUM_DECL(virNetworkTaint);
+
+#define ENABLE_VIR_NETWORK_DNSTXT_DEF_PARSE_HOOK
+#include "network_conf.generated.h"
-- 
2.25.1

[RFCv3 16/25] conf: Generate virNetworkDNSSrvDefFormatBuf

[Shi Lei]

Signed-off-by: Shi Lei 
---
 src/conf/network_conf.c | 18 ++
 src/conf/network_conf.h |  2 +-
 2 files changed, 3 insertions(+), 17 deletions(-)

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index cb2f3163..146c4977 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -2191,22 +2191,8 @@ virNetworkDNSDefFormat(virBuffer *buf,
 
 for (i = 0; i < def->nsrvs; i++) {
 if (def->srvs[i].service && def->srvs[i].protocol) {
-virBufferEscapeString(buf, "srvs[i].service);
-virBufferEscapeString(buf, "protocol='%s'", def->srvs[i].protocol);
-
-if (def->srvs[i].domain)
-virBufferEscapeString(buf, " domain='%s'", 
def->srvs[i].domain);
-if (def->srvs[i].target)
-virBufferEscapeString(buf, " target='%s'", 
def->srvs[i].target);
-if (def->srvs[i].port)
-virBufferAsprintf(buf, " port='%d'", def->srvs[i].port);
-if (def->srvs[i].priority)
-virBufferAsprintf(buf, " priority='%d'", 
def->srvs[i].priority);
-if (def->srvs[i].weight)
-virBufferAsprintf(buf, " weight='%d'", def->srvs[i].weight);
-
-virBufferAddLit(buf, "/>\n");
+if (virNetworkDNSSrvDefFormatBuf(buf, "srv", &def->srvs[i], def, 
NULL) < 0)
+return -1;
 }
 }
 
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index a58d8953..052ccb58 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -131,7 +131,7 @@ struct _virNetworkDNSTxtDef {   /* genparse, genformat */
 };
 
 typedef struct _virNetworkDNSSrvDef virNetworkDNSSrvDef;
-struct _virNetworkDNSSrvDef {   /* genparse */
+struct _virNetworkDNSSrvDef {   /* genparse, genformat */
 char *service;  /* xmlattr */
 char *protocol; /* xmlattr */
 char *domain;   /* xmlattr */
-- 
2.25.1

[RFCv3 23/25] conf: Extract error-checking code from virNetworkDNSDefParseXML

[Shi Lei]

Signed-off-by: Shi Lei 
---
 src/conf/network_conf.c | 34 --
 1 file changed, 28 insertions(+), 6 deletions(-)

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index be639500..19408987 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -883,6 +883,31 @@ virNetworkDNSForwarderParseHook(xmlNodePtr node 
G_GNUC_UNUSED,
 }
 
 
+static int
+virNetworkDNSDefParseHook(xmlNodePtr node G_GNUC_UNUSED,
+  virNetworkDNSDef *def,
+  const char *networkName,
+  void *parent G_GNUC_UNUSED,
+  void *opaque G_GNUC_UNUSED,
+  const char *enable G_GNUC_UNUSED,
+  const char *forwardPlainNames G_GNUC_UNUSED,
+  int nfwds,
+  int ntxts,
+  int nsrvs,
+  int nhosts)
+{
+if (def->enable == VIR_TRISTATE_BOOL_NO &&
+(nfwds || nhosts || nsrvs || ntxts)) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("Extra data in disabled network '%s'"),
+   networkName);
+return -1;
+}
+
+return 0;
+}
+
+
 static int
 virNetworkDNSDefParseXML(const char *networkName,
  xmlNodePtr node,
@@ -1004,13 +1029,10 @@ virNetworkDNSDefParseXML(const char *networkName,
 }
 }
 
-if (def->enable == VIR_TRISTATE_BOOL_NO &&
-(nfwds || nhosts || nsrvs || ntxts)) {
-virReportError(VIR_ERR_XML_ERROR,
-   _("Extra data in disabled network '%s'"),
-   networkName);
+if (virNetworkDNSDefParseHook(node, def, networkName, def, NULL,
+  enable, forwardPlainNames,
+  nfwds, ntxts, nsrvs, nhosts) < 0)
 return -1;
-}
 
 return 0;
 }
-- 
2.25.1

[RFCv3 05/25] build-aux: Only check *.[ch] for sc_prohibit_useless_translation

[Shi Lei]

Signed-off-by: Shi Lei 
---
 build-aux/syntax-check.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
index 552d6391..614f21bc 100644
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@@ -735,7 +735,7 @@ sc_prohibit_useless_translation:
halt='found useless translation' \
  $(_sc_search_regexp)
@prohibit='\

[RFCv3 25/25] conf: Generate virNetworkDNSDefFormatBuf

[Shi Lei]

Signed-off-by: Shi Lei 
---
 src/conf/network_conf.c | 73 +
 src/conf/network_conf.h |  2 +-
 2 files changed, 2 insertions(+), 73 deletions(-)

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 364c10e2..a652110c 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -1959,77 +1959,6 @@ virNetworkDefParseNode(xmlDocPtr xml,
 }
 
 
-static int
-virNetworkDNSDefFormat(virBuffer *buf,
-   const virNetworkDNSDef *def)
-{
-size_t i;
-
-if (!(def->enable || def->forwardPlainNames || def->nfwds || def->nhosts ||
-  def->nsrvs || def->ntxts))
-return 0;
-
-virBufferAddLit(buf, "enable) {
-const char *fwd = virTristateBoolTypeToString(def->enable);
-
-if (!fwd) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("Unknown enable type %d in network"),
-   def->enable);
-return -1;
-}
-virBufferAsprintf(buf, " enable='%s'", fwd);
-}
-if (def->forwardPlainNames) {
-const char *fwd = virTristateBoolTypeToString(def->forwardPlainNames);
-
-if (!fwd) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("Unknown forwardPlainNames type %d in network"),
-   def->forwardPlainNames);
-return -1;
-}
-virBufferAsprintf(buf, " forwardPlainNames='%s'", fwd);
-}
-if (!(def->nfwds || def->nhosts || def->nsrvs || def->ntxts)) {
-virBufferAddLit(buf, "/>\n");
-return 0;
-}
-
-virBufferAddLit(buf, ">\n");
-virBufferAdjustIndent(buf, 2);
-
-for (i = 0; i < def->nfwds; i++) {
-if (virNetworkDNSForwarderFormatBuf(buf, "forwarder",
-&def->forwarders[i], def, NULL) < 
0)
-return -1;
-}
-
-for (i = 0; i < def->ntxts; i++) {
-if (virNetworkDNSTxtDefFormatBuf(buf, "txt", &def->txts[i], def, NULL) 
< 0)
-return -1;
-}
-
-for (i = 0; i < def->nsrvs; i++) {
-if (def->srvs[i].service && def->srvs[i].protocol) {
-if (virNetworkDNSSrvDefFormatBuf(buf, "srv", &def->srvs[i], def, 
NULL) < 0)
-return -1;
-}
-}
-
-if (def->nhosts) {
-for (i = 0; i < def->nhosts; i++) {
-if (virNetworkDNSHostDefFormatBuf(buf, "host", &def->hosts[i], 
def, NULL) < 0)
-return -1;
-}
-}
-virBufferAdjustIndent(buf, -2);
-virBufferAddLit(buf, "\n");
-return 0;
-}
-
-
 static int
 virNetworkIPDefFormat(virBuffer *buf,
   const virNetworkIPDef *def)
@@ -2436,7 +2365,7 @@ virNetworkDefFormatBuf(virBuffer *buf,
 virBufferAddLit(buf, "/>\n");
 }
 
-if (virNetworkDNSDefFormat(buf, &def->dns) < 0)
+if (virNetworkDNSDefFormatBuf(buf, "dns", &def->dns, def, NULL) < 0)
 return -1;
 
 if (virNetDevVlanFormat(&def->vlan, buf) < 0)
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 9968d962..b0675175 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -156,7 +156,7 @@ struct _virNetworkDNSForwarder {/* genparse, genformat 
*/
 };
 
 typedef struct _virNetworkDNSDef virNetworkDNSDef;
-struct _virNetworkDNSDef {  /* genparse */
+struct _virNetworkDNSDef {  /* genparse, genformat */
 virTristateBool enable; /* xmlattr */
 virTristateBool forwardPlainNames;  /* xmlattr */
 size_t nfwds;
-- 
2.25.1

[RFCv3 20/25] conf: Extract virNetworkDNSForwarderParseXML from virNetworkDNSParseXML

[Shi Lei]

Signed-off-by: Shi Lei 
---
 src/conf/network_conf.c | 68 +++--
 1 file changed, 52 insertions(+), 16 deletions(-)

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index ba67eab1..cf9e77d3 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -871,6 +871,52 @@ virNetworkDNSTxtDefParseHook(xmlNodePtr node G_GNUC_UNUSED,
 }
 
 
+static int
+virNetworkDNSForwarderParseHook(xmlNodePtr node G_GNUC_UNUSED,
+virNetworkDNSForwarder *def,
+const char *instname G_GNUC_UNUSED,
+void *parent G_GNUC_UNUSED,
+void *opaque G_GNUC_UNUSED,
+const char *addr)
+{
+if (!(addr || def->domain)) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("Invalid forwarder element, must contain "
+ "at least one of addr or domain"));
+return -1;
+}
+
+return 0;
+}
+
+
+static int
+virNetworkDNSForwarderParseXML(xmlNodePtr node,
+   virNetworkDNSForwarder *def,
+   const char *networkName,
+   void *parent G_GNUC_UNUSED,
+   void *opaque)
+{
+g_autofree char *addr = virXMLPropString(node, "addr");
+
+if (addr && virSocketAddrParse(&def->addr, addr, AF_UNSPEC) < 0) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("Invalid forwarder IP address '%s' "
+ "in network '%s'"),
+   addr, networkName);
+return -1;
+}
+
+def->domain = virXMLPropString(node, "domain");
+
+if (virNetworkDNSForwarderParseHook(node, def, networkName, def, opaque,
+addr) < 0)
+return -1;
+
+return 0;
+}
+
+
 static int
 virNetworkDNSDefParseXML(const char *networkName,
  xmlNodePtr node,
@@ -924,23 +970,13 @@ virNetworkDNSDefParseXML(const char *networkName,
 def->forwarders = g_new0(virNetworkDNSForwarder, nfwds);
 
 for (i = 0; i < nfwds; i++) {
-g_autofree char *addr = virXMLPropString(fwdNodes[i], "addr");
-
-if (addr && virSocketAddrParse(&def->forwarders[i].addr,
-   addr, AF_UNSPEC) < 0) {
-virReportError(VIR_ERR_XML_ERROR,
-   _("Invalid forwarder IP address '%s' "
- "in network '%s'"),
-   addr, networkName);
+if (virNetworkDNSForwarderParseXML(fwdNodes[i],
+   &def->forwarders[i],
+   networkName,
+   def,
+   NULL) < 0)
 return -1;
-}
-def->forwarders[i].domain = virXMLPropString(fwdNodes[i], 
"domain");
-if (!(addr || def->forwarders[i].domain)) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
-   _("Invalid forwarder element, must contain "
- "at least one of addr or domain"));
-return -1;
-}
+
 def->nfwds++;
 }
 }
-- 
2.25.1

[RFCv3 06/25] tests: Add tests for xmlgen

[Shi Lei]

Add some tests to make sure xmlgen's validity.

Signed-off-by: Shi Lei 
---
 tests/meson.build|   1 +
 tests/xmlgenin/conf/array.h  |  17 +
 tests/xmlgenin/conf/empty.h  |   7 +
 tests/xmlgenin/conf/enum-first-item.h|  12 +
 tests/xmlgenin/conf/external.h   |   9 +
 tests/xmlgenin/conf/genformat-separate.h |  11 +
 tests/xmlgenin/conf/genformat.h  |  11 +
 tests/xmlgenin/conf/genparse.h   |  11 +
 tests/xmlgenin/conf/namespace.h  |  12 +
 tests/xmlgenin/conf/required.h   |   9 +
 tests/xmlgenin/conf/skipparse.h  |  10 +
 tests/xmlgenin/conf/specify.h|  13 +
 tests/xmlgenin/conf/xmlattr.h|  10 +
 tests/xmlgenin/conf/xmlelem.h|  10 +
 tests/xmlgenin/conf/xmlgroup.h   |   8 +
 tests/xmlgenin/conf/xmlswitch.h  |  17 +
 tests/xmlgenin/util/enums.h  |  58 +++
 tests/xmlgenin/util/structs.h|  67 
 tests/xmlgenout/array.txt| 364 ++
 tests/xmlgenout/empty.txt| 181 +
 tests/xmlgenout/enum-first-item.txt  | 297 ++
 tests/xmlgenout/external.txt | 205 ++
 tests/xmlgenout/genformat-separate.txt   | 190 +
 tests/xmlgenout/genformat.txt| 142 +++
 tests/xmlgenout/genparse.txt | 154 
 tests/xmlgenout/namespace.txt| 222 +++
 tests/xmlgenout/required.txt | 236 
 tests/xmlgenout/skipparse.txt| 223 +++
 tests/xmlgenout/specify.txt  | 291 ++
 tests/xmlgenout/xmlattr.txt  | 252 
 tests/xmlgenout/xmlelem.txt  | 243 
 tests/xmlgenout/xmlgroup.txt | 204 ++
 tests/xmlgenout/xmlswitch.txt| 470 +++
 tests/xmlgentest.c   | 107 ++
 34 files changed, 4074 insertions(+)
 create mode 100644 tests/xmlgenin/conf/array.h
 create mode 100644 tests/xmlgenin/conf/empty.h
 create mode 100644 tests/xmlgenin/conf/enum-first-item.h
 create mode 100644 tests/xmlgenin/conf/external.h
 create mode 100644 tests/xmlgenin/conf/genformat-separate.h
 create mode 100644 tests/xmlgenin/conf/genformat.h
 create mode 100644 tests/xmlgenin/conf/genparse.h
 create mode 100644 tests/xmlgenin/conf/namespace.h
 create mode 100644 tests/xmlgenin/conf/required.h
 create mode 100644 tests/xmlgenin/conf/skipparse.h
 create mode 100644 tests/xmlgenin/conf/specify.h
 create mode 100644 tests/xmlgenin/conf/xmlattr.h
 create mode 100644 tests/xmlgenin/conf/xmlelem.h
 create mode 100644 tests/xmlgenin/conf/xmlgroup.h
 create mode 100644 tests/xmlgenin/conf/xmlswitch.h
 create mode 100644 tests/xmlgenin/util/enums.h
 create mode 100644 tests/xmlgenin/util/structs.h
 create mode 100644 tests/xmlgenout/array.txt
 create mode 100644 tests/xmlgenout/empty.txt
 create mode 100644 tests/xmlgenout/enum-first-item.txt
 create mode 100644 tests/xmlgenout/external.txt
 create mode 100644 tests/xmlgenout/genformat-separate.txt
 create mode 100644 tests/xmlgenout/genformat.txt
 create mode 100644 tests/xmlgenout/genparse.txt
 create mode 100644 tests/xmlgenout/namespace.txt
 create mode 100644 tests/xmlgenout/required.txt
 create mode 100644 tests/xmlgenout/skipparse.txt
 create mode 100644 tests/xmlgenout/specify.txt
 create mode 100644 tests/xmlgenout/xmlattr.txt
 create mode 100644 tests/xmlgenout/xmlelem.txt
 create mode 100644 tests/xmlgenout/xmlgroup.txt
 create mode 100644 tests/xmlgenout/xmlswitch.txt
 create mode 100644 tests/xmlgentest.c

diff --git a/tests/meson.build b/tests/meson.build
index 14ace476..166416cc 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -340,6 +340,7 @@ tests += [
   { 'name': 'viruritest' },
   { 'name': 'vshtabletest', 'link_with': [ libvirt_shell_lib ] },
   { 'name': 'virmigtest' },
+  { 'name': 'xmlgentest' },
 ]
 
 if host_machine.system() == 'linux'
diff --git a/tests/xmlgenin/conf/array.h b/tests/xmlgenin/conf/array.h
new file mode 100644
index ..8f003be4
--- /dev/null
+++ b/tests/xmlgenin/conf/array.h
@@ -0,0 +1,17 @@
+/* Test array features */
+
+#pragma once
+
+typedef struct _virArrayDef virArrayDef;
+struct _virArrayDef {   /* genparse, genformat */
+size_t nnames;
+char **names;   /* xmlelem:hostname, array */
+size_t nfwds;
+virNetworkDNSForwarder *forwarders; /* xmlelem, array:nfwds */
+size_t ntxts;
+virNetworkDNSTxtDef *txts;  /* xmlelem, array */
+
+size_t nroutes;
+/* ptr to array of static routes on this interface */
+virNetDevIPRoute **routes;  /* xmlelem, array */
+};
diff --git a/tests/xmlgenin/conf/empty.h b/tests/xmlgenin/conf/empty.h
new file mode 100644
index ..c8292c8e
--- /dev/null
+++ b/tests/xmlgenin/conf/empty.h
@@ -0,0 +1,7 @@
+/* Test empty struct */
+
+#pragma once
+
+typedef struct _virEmp

[RFCv3 09/25] util: Add parsexml/formatbuf helper functions for virSocketAddr

[Shi Lei]

Implement the parsexml/formatbuf functions for virSocketAddr.

Signed-off-by: Shi Lei 
---
 src/libvirt_private.syms |  2 ++
 src/util/virsocketaddr.c | 42 
 src/util/virsocketaddr.h | 23 --
 3 files changed, 65 insertions(+), 2 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index e78491dc..055396d0 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -3213,6 +3213,7 @@ virSocketAddrBroadcastByPrefix;
 virSocketAddrCheckNetmask;
 virSocketAddrEqual;
 virSocketAddrFormat;
+virSocketAddrFormatBuf;
 virSocketAddrFormatFull;
 virSocketAddrGetIPPrefix;
 virSocketAddrGetNumNetmaskBits;
@@ -3230,6 +3231,7 @@ virSocketAddrParse;
 virSocketAddrParseAny;
 virSocketAddrParseIPv4;
 virSocketAddrParseIPv6;
+virSocketAddrParseXML;
 virSocketAddrPrefixToNetmask;
 virSocketAddrPTRDomain;
 virSocketAddrResolveService;
diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c
index 94cbfc62..fcad7f8a 100644
--- a/src/util/virsocketaddr.c
+++ b/src/util/virsocketaddr.c
@@ -154,6 +154,15 @@ int virSocketAddrParse(virSocketAddr *addr, const char 
*val, int family)
 return len;
 }
 
+int virSocketAddrParseXML(const char *val,
+  virSocketAddr *addr,
+  const char *instname G_GNUC_UNUSED,
+  void *parent G_GNUC_UNUSED,
+  void *opaque G_GNUC_UNUSED)
+{
+return virSocketAddrParse(addr, val, AF_UNSPEC);
+}
+
 /**
  * virSocketAddrParseAny:
  * @addr: where to store the return value, optional.
@@ -1306,3 +1315,36 @@ virSocketAddrFree(virSocketAddr *addr)
 {
 g_free(addr);
 }
+
+void
+virSocketAddrClear(virSocketAddr *addr)
+{
+memset(addr, 0, sizeof(virSocketAddr));
+}
+
+int
+virSocketAddrFormatBuf(virBuffer *buf,
+   const char *fmt,
+   const virSocketAddr *addr,
+   const void *parent G_GNUC_UNUSED,
+   void *opaque G_GNUC_UNUSED)
+{
+g_autofree char *str = NULL;
+if (!VIR_SOCKET_ADDR_VALID(addr))
+return 0;
+
+str = virSocketAddrFormatFull(addr, false, NULL);
+if (!str)
+return -1;
+
+virBufferAsprintf(buf, fmt, str);
+return 0;
+}
+
+bool
+virSocketAddrCheck(const virSocketAddr *addr,
+   const void *parent G_GNUC_UNUSED,
+   void *opaque G_GNUC_UNUSED)
+{
+return VIR_SOCKET_ADDR_VALID(addr);
+}
diff --git a/src/util/virsocketaddr.h b/src/util/virsocketaddr.h
index f76e2297..cba87390 100644
--- a/src/util/virsocketaddr.h
+++ b/src/util/virsocketaddr.h
@@ -18,11 +18,13 @@
 
 #pragma once
 
+#include "virbuffer.h"
 #include "virsocket.h"
 
 #define VIR_LOOPBACK_IPV4_ADDR "127.0.0.1"
 
-typedef struct {
+typedef struct _virSocketAddr virSocketAddr;
+struct _virSocketAddr {
 union {
 struct sockaddr sa;
 struct sockaddr_storage stor;
@@ -33,7 +35,7 @@ typedef struct {
 #endif
 } data;
 socklen_t len;
-} virSocketAddr;
+};
 
 #define VIR_SOCKET_ADDR_VALID(s) \
 ((s)->data.sa.sa_family != AF_UNSPEC)
@@ -66,6 +68,12 @@ int virSocketAddrParse(virSocketAddr *addr,
const char *val,
int family);
 
+int virSocketAddrParseXML(const char *val,
+  virSocketAddr *addr,
+  const char *instname,
+  void *parent,
+  void *opaque);
+
 int virSocketAddrParseAny(virSocketAddr *addr,
   const char *val,
   int family,
@@ -89,6 +97,12 @@ char *virSocketAddrFormatFull(const virSocketAddr *addr,
   bool withService,
   const char *separator);
 
+int virSocketAddrFormatBuf(virBuffer *buf,
+   const char *fmt,
+   const virSocketAddr *addr,
+   const void *parent,
+   void *opaque);
+
 char *virSocketAddrGetPath(virSocketAddr *addr);
 
 int virSocketAddrSetPort(virSocketAddr *addr, int port);
@@ -141,5 +155,10 @@ int virSocketAddrPTRDomain(const virSocketAddr *addr,
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3);
 
 void virSocketAddrFree(virSocketAddr *addr);
+void virSocketAddrClear(virSocketAddr *addr);
 
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(virSocketAddr, virSocketAddrFree);
+
+bool virSocketAddrCheck(const virSocketAddr *addr,
+const void *parent,
+void *opaque);
-- 
2.25.1

[RFCv3 10/25] util: Add virUUID type and parse/format functions

[Shi Lei]

Signed-off-by: Shi Lei 
---
 src/util/viruuid.c | 31 +++
 src/util/viruuid.h | 18 ++
 2 files changed, 49 insertions(+)

diff --git a/src/util/viruuid.c b/src/util/viruuid.c
index 558fbb9c..c6e6272f 100644
--- a/src/util/viruuid.c
+++ b/src/util/viruuid.c
@@ -134,6 +134,18 @@ virUUIDParse(const char *uuidstr, unsigned char *uuid)
 return 0;
 }
 
+
+int
+virUUIDParseXML(const char *uuidstr,
+virUUID *puuid,
+const char *instname G_GNUC_UNUSED,
+void *parent G_GNUC_UNUSED,
+void *opaque G_GNUC_UNUSED)
+{
+return virUUIDParse(uuidstr, *puuid);
+}
+
+
 /**
  * virUUIDFormat:
  * @uuid: array of VIR_UUID_BUFLEN bytes to store the raw UUID
@@ -159,6 +171,20 @@ virUUIDFormat(const unsigned char *uuid, char *uuidstr)
 }
 
 
+int
+virUUIDFormatBuf(virBuffer *buf,
+ const char *fmt,
+ const virUUID *puuid,
+ const void *parent G_GNUC_UNUSED,
+ void *opaque G_GNUC_UNUSED)
+{
+char uuidstr[VIR_UUID_STRING_BUFLEN];
+virUUIDFormat(*puuid, uuidstr);
+virBufferAsprintf(buf, fmt, uuidstr);
+
+return 0;
+}
+
 
 /**
  * virUUIDIsValid
@@ -263,3 +289,8 @@ int virGetHostUUID(unsigned char *uuid)
 
 return ret;
 }
+
+void virUUIDClear(virUUID *puuid G_GNUC_UNUSED)
+{
+memset(*puuid, 0, VIR_UUID_BUFLEN);
+}
diff --git a/src/util/viruuid.h b/src/util/viruuid.h
index b403b190..36766356 100644
--- a/src/util/viruuid.h
+++ b/src/util/viruuid.h
@@ -21,6 +21,7 @@
 #pragma once
 
 #include "internal.h"
+#include "virbuffer.h"
 
 
 /**
@@ -39,6 +40,7 @@
 } \
 } while (0)
 
+typedef unsigned char virUUID[VIR_UUID_BUFLEN];
 
 int virSetHostUUIDStr(const char *host_uuid);
 int virGetHostUUID(unsigned char *host_uuid) ATTRIBUTE_NONNULL(1) 
G_GNUC_NO_INLINE;
@@ -51,5 +53,21 @@ int virUUIDParse(const char *uuidstr,
  unsigned char *uuid)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT;
 
+int
+virUUIDParseXML(const char *uuidstr,
+virUUID *puuid,
+const char *instname,
+void *parent,
+void *opaque);
+
 const char *virUUIDFormat(const unsigned char *uuid,
   char *uuidstr) ATTRIBUTE_NONNULL(1) 
ATTRIBUTE_NONNULL(2);
+
+int
+virUUIDFormatBuf(virBuffer *buf,
+ const char *fmt,
+ const virUUID *puuid,
+ const void *parent,
+ void *opaque);
+
+void virUUIDClear(virUUID *puuid);
-- 
2.25.1

[RFCv3 22/25] conf: Generate virNetworkDNSForwarderFormatBuf

[Shi Lei]

Signed-off-by: Shi Lei 
---
 src/conf/network_conf.c | 18 +++---
 src/conf/network_conf.h |  4 ++--
 2 files changed, 5 insertions(+), 17 deletions(-)

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index ef28bb4d..be639500 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -2135,21 +2135,9 @@ virNetworkDNSDefFormat(virBuffer *buf,
 virBufferAdjustIndent(buf, 2);
 
 for (i = 0; i < def->nfwds; i++) {
-
-virBufferAddLit(buf, "forwarders[i].domain) {
-virBufferEscapeString(buf, " domain='%s'",
-  def->forwarders[i].domain);
-}
-if (VIR_SOCKET_ADDR_VALID(&def->forwarders[i].addr)) {
-g_autofree char *addr = 
virSocketAddrFormat(&def->forwarders[i].addr);
-
-if (!addr)
-return -1;
-
-virBufferAsprintf(buf, " addr='%s'", addr);
-}
-virBufferAddLit(buf, "/>\n");
+if (virNetworkDNSForwarderFormatBuf(buf, "forwarder",
+&def->forwarders[i], def, NULL) < 
0)
+return -1;
 }
 
 for (i = 0; i < def->ntxts; i++) {
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 17f6c309..577c1568 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -150,9 +150,9 @@ struct _virNetworkDNSHostDef {  /* genparse, genformat */
 
 
 typedef struct _virNetworkDNSForwarder virNetworkDNSForwarder;
-struct _virNetworkDNSForwarder {/* genparse */
-virSocketAddr addr; /* xmlattr */
+struct _virNetworkDNSForwarder {/* genparse, genformat */
 char *domain;   /* xmlattr */
+virSocketAddr addr; /* xmlattr */
 };
 
 typedef struct _virNetworkDNSDef virNetworkDNSDef;
-- 
2.25.1

[RFCv3 00/25] RFC: Generate parsexml/formatbuf functions based on directives

[Shi Lei]

V2 here: 
[https://listman.redhat.com/archives/libvir-list/2020-September/msg00204.html]

Differ from V2:

  * Add tests for xmlgen to illustrate all the different features we can use
and make sure its proper functions in the future.

  * Add docs/xmlgen.rst to explain the usage of all directives and the tool
itself.

  * Now xmlgen can check whether the first item of enum ends with _NONE, 
_DEFAULT
or _ABSENT and generate proper code. So we no longer need to add extra
'default' item for enum.

  * Now xmlgen can provide extra [tips] when we execute its command-line to show
generated code for preview.

  * Enable/disable hooks by macros rather than by special directives.

  * Add virStrToBoolYesNo/virStrToBoolTrueFalse/virStrToBoolOnOff
and explicitly check both the true and false values.

  * Stronger check for python3-clang and libclang.so to make sure it can work.

  * Add python3-clang to the libvirt.spec.in and the mingw-libvirt.spec.in.

  Thanks!


Shi Lei (25):
  scripts: Add a tool to generate xml parse/format functions
  maint: Check python3-clang and libclang
  maint: Call xmlgen automatically when c-head-files change
  docs: Add xmlgen.rst to explain how to use it
  build-aux: Only check *.[ch] for sc_prohibit_useless_translation
  tests: Add tests for xmlgen
  util: Add some xml-helper-functions to cooperate with xmlgen
  util: Add helper aliases and functions for 'bool' and 'time_t' to cooperate 
with xmlgen
  util: Add parsexml/formatbuf helper functions for virSocketAddr
  util: Add virUUID type and parse/format functions
  conf: Extract error-checking code from virNetworkDNSTxtDefParseXML
  conf: Replace virNetworkDNSTxtDefParseXML(hardcoded) with namesake(generated)
  conf: Generate virNetworkDNSTxtDefFormatBuf
  conf: Extract error-checking code from virNetworkDNSSrvDefParseXML
  conf: Replace virNetworkDNSSrvDefParseXML(hardcoded) with namesake(generated)
  conf: Generate virNetworkDNSSrvDefFormatBuf
  conf: Extract error-checking code from virNetworkDNSHostDefParseXML
  conf: Replace virNetworkDNSHostDefParseXML(hardcoded) with namesake(generated)
  conf: Generate virNetworkDNSHostDefFormatBuf
  conf: Extract virNetworkDNSForwarderParseXML from virNetworkDNSParseXML
  conf: Replace virNetworkDNSForwarderParseXML(hardcoded) with 
namesake(generated)
  conf: Generate virNetworkDNSForwarderFormatBuf
  conf: Extract error-checking code from virNetworkDNSDefParseXML
  conf: Replace virNetworkDNSDefParseXML(hardcoded) with namesake(generated)
  conf: Generate virNetworkDNSDefFormatBuf

 build-aux/syntax-check.mk|2 +-
 docs/meson.build |1 +
 docs/xmlgen.rst  |  684 +
 libvirt.spec.in  |1 +
 meson.build  |   10 +
 mingw-libvirt.spec.in|1 +
 po/POTFILES.in   |2 +
 scripts/meson.build  |8 +
 scripts/xmlgen/directive.py  | 1192 ++
 scripts/xmlgen/go|   29 +
 scripts/xmlgen/main.py   |  534 ++
 scripts/xmlgen/utils.py  |  121 +++
 src/conf/meson.build |   37 +
 src/conf/network_conf.c  |  463 ++---
 src/conf/network_conf.h  |   59 +-
 src/internal.h   |8 +
 src/libvirt_private.syms |   13 +
 src/meson.build  |6 +
 src/util/meson.build |   36 +
 src/util/virbuffer.c |   44 +
 src/util/virbuffer.h |8 +-
 src/util/virsocketaddr.c |   42 +
 src/util/virsocketaddr.h |   23 +-
 src/util/virstring.c |  102 ++
 src/util/virstring.h |   15 +
 src/util/viruuid.c   |   31 +
 src/util/viruuid.h   |   18 +
 src/util/virxml.c|  120 +++
 src/util/virxml.h|6 +
 tests/meson.build|3 +
 tests/xmlgenin/conf/array.h  |   17 +
 tests/xmlgenin/conf/empty.h  |7 +
 tests/xmlgenin/conf/enum-first-item.h|   12 +
 tests/xmlgenin/conf/external.h   |9 +
 tests/xmlgenin/conf/genformat-separate.h |   11 +
 tests/xmlgenin/conf/genformat.h  |   11 +
 tests/xmlgenin/conf/genparse.h   |   11 +
 tests/xmlgenin/conf/namespace.h  |   12 +
 tests/xmlgenin/conf/required.h   |9 +
 tests/xmlgenin/conf/skipparse.h  |   10 +
 tests/xmlgenin/conf/specify.h|   13 +
 tests/xmlgenin/conf/xmlattr.h|   10 +
 tests/xmlgenin/conf/xmlelem.h|   10 +
 tests/xmlgenin/conf/xmlgroup.h   |8 +
 tests/xmlgenin/conf/xmlswitch.h  |   17 +
 tests/xmlgenin/util/enums.h  |   58 ++
 tests/xmlgenin/util/st

[RFCv3 21/25] conf: Replace virNetworkDNSForwarderParseXML(hardcoded) with namesake(generated)

[Shi Lei]

Signed-off-by: Shi Lei 
---
 src/conf/network_conf.c | 36 +---
 src/conf/network_conf.h |  7 ---
 2 files changed, 5 insertions(+), 38 deletions(-)

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index cf9e77d3..ef28bb4d 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -174,13 +174,6 @@ virNetworkIPDefClear(virNetworkIPDef *def)
 }
 
 
-static void
-virNetworkDNSForwarderClear(virNetworkDNSForwarder *def)
-{
-VIR_FREE(def->domain);
-}
-
-
 static void
 virNetworkDNSDefClear(virNetworkDNSDef *def)
 {
@@ -871,7 +864,7 @@ virNetworkDNSTxtDefParseHook(xmlNodePtr node G_GNUC_UNUSED,
 }
 
 
-static int
+int
 virNetworkDNSForwarderParseHook(xmlNodePtr node G_GNUC_UNUSED,
 virNetworkDNSForwarder *def,
 const char *instname G_GNUC_UNUSED,
@@ -890,33 +883,6 @@ virNetworkDNSForwarderParseHook(xmlNodePtr node 
G_GNUC_UNUSED,
 }
 
 
-static int
-virNetworkDNSForwarderParseXML(xmlNodePtr node,
-   virNetworkDNSForwarder *def,
-   const char *networkName,
-   void *parent G_GNUC_UNUSED,
-   void *opaque)
-{
-g_autofree char *addr = virXMLPropString(node, "addr");
-
-if (addr && virSocketAddrParse(&def->addr, addr, AF_UNSPEC) < 0) {
-virReportError(VIR_ERR_XML_ERROR,
-   _("Invalid forwarder IP address '%s' "
- "in network '%s'"),
-   addr, networkName);
-return -1;
-}
-
-def->domain = virXMLPropString(node, "domain");
-
-if (virNetworkDNSForwarderParseHook(node, def, networkName, def, opaque,
-addr) < 0)
-return -1;
-
-return 0;
-}
-
-
 static int
 virNetworkDNSDefParseXML(const char *networkName,
  xmlNodePtr node,
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 836d088d..17f6c309 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -150,9 +150,9 @@ struct _virNetworkDNSHostDef {  /* genparse, genformat */
 
 
 typedef struct _virNetworkDNSForwarder virNetworkDNSForwarder;
-struct _virNetworkDNSForwarder {
-virSocketAddr addr;
-char *domain;
+struct _virNetworkDNSForwarder {/* genparse */
+virSocketAddr addr; /* xmlattr */
+char *domain;   /* xmlattr */
 };
 
 typedef struct _virNetworkDNSDef virNetworkDNSDef;
@@ -429,6 +429,7 @@ virNetworkDefUpdateSection(virNetworkDef *def,
 
 VIR_ENUM_DECL(virNetworkTaint);
 
+#define ENABLE_VIR_NETWORK_DNSFORWARDER_PARSE_HOOK
 #define ENABLE_VIR_NETWORK_DNSHOST_DEF_PARSE_HOOK
 #define ENABLE_VIR_NETWORK_DNSSRV_DEF_PARSE_HOOK
 #define ENABLE_VIR_NETWORK_DNSTXT_DEF_PARSE_HOOK
-- 
2.25.1

[RFCv3 07/25] util: Add some xml-helper-functions to cooperate with xmlgen

[Shi Lei]

1) virXMLChildNode and virXMLChildNodeSet
   Parse xml based on 'node' rather than 'ctxt'.

2) virXMLChildPropString
   Support to parse child node's property value by path,
   which is as "child_node_name/prop_name".

3) virXMLChildNodeContent
   Support to parse child node's content.

4) virBufferIgnore
   Mark a buffer with the flag 'ignored'.

5) virBufferTouched
   Check whether the buffer has been touched, which means
   this buffer has been used or ignored.

6) virBufferInheritIndent
   Inherit the indentation from another buffer.

Signed-off-by: Shi Lei 
---
 src/libvirt_private.syms |   6 ++
 src/util/virbuffer.c |  44 ++
 src/util/virbuffer.h |   8 ++-
 src/util/virxml.c| 120 +++
 src/util/virxml.h|   6 ++
 5 files changed, 182 insertions(+), 2 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index e9bb2391..fad0ff71 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1864,9 +1864,12 @@ virBufferEscapeString;
 virBufferFreeAndReset;
 virBufferGetEffectiveIndent;
 virBufferGetIndent;
+virBufferIgnore;
+virBufferInheritIndent;
 virBufferSetIndent;
 virBufferStrcat;
 virBufferStrcatVArgs;
+virBufferTouched;
 virBufferTrim;
 virBufferTrimChars;
 virBufferTrimLen;
@@ -3539,6 +3542,9 @@ virVsockSetGuestCid;
 virParseScaledValue;
 virXMLBufferCreate;
 virXMLCheckIllegalChars;
+virXMLChildNode;
+virXMLChildNodeSet;
+virXMLChildPropString;
 virXMLExtractNamespaceXML;
 virXMLFormatElement;
 virXMLNewNode;
diff --git a/src/util/virbuffer.c b/src/util/virbuffer.c
index 8f9cd57e..b6f936c8 100644
--- a/src/util/virbuffer.c
+++ b/src/util/virbuffer.c
@@ -80,6 +80,24 @@ virBufferSetIndent(virBuffer *buf, int indent)
 }
 
 
+/**
+ * virBufferInheritIndent:
+ * @buf: the buffer
+ * @frombuf: new indentation size from it.
+ *
+ * Just like virBufferSetIndent, but the indentation comes from
+ * another buffer.
+ */
+void
+virBufferInheritIndent(virBuffer *buf, virBuffer *frombuf)
+{
+if (!buf || !frombuf)
+return;
+
+buf->indent = virBufferGetEffectiveIndent(frombuf);
+}
+
+
 /**
  * virBufferGetIndent:
  * @buf: the buffer
@@ -291,6 +309,32 @@ virBufferUse(const virBuffer *buf)
 return buf->str->len;
 }
 
+/**
+ * virBufferTouched:
+ * @buf: the buffer to be checked
+ *
+ * Return true when the buffer has content or
+ * it has been ignored(by calling virBufferIgnore);
+ * otherwise, return false.
+ */
+bool
+virBufferTouched(virBuffer *buf)
+{
+return buf->ignored || virBufferUse(buf);
+}
+
+/**
+ * virBufferIgnore:
+ * @buf: the buffer to be ignored
+ *
+ * Ignore the buffer explicitly.
+ */
+void
+virBufferIgnore(virBuffer *buf)
+{
+buf->ignored = true;
+}
+
 /**
  * virBufferAsprintf:
  * @buf: the buffer to append to
diff --git a/src/util/virbuffer.h b/src/util/virbuffer.h
index 0e72d078..ddb29cb5 100644
--- a/src/util/virbuffer.h
+++ b/src/util/virbuffer.h
@@ -32,7 +32,7 @@
  */
 typedef struct _virBuffer virBuffer;
 
-#define VIR_BUFFER_INITIALIZER { NULL, 0 }
+#define VIR_BUFFER_INITIALIZER { NULL, 0, false }
 
 /**
  * VIR_BUFFER_INIT_CHILD:
@@ -41,11 +41,12 @@ typedef struct _virBuffer virBuffer;
  * Initialize a virBuffer structure and set up the indentation level for
  * formatting XML subelements of @parentbuf.
  */
-#define VIR_BUFFER_INIT_CHILD(parentbuf) { NULL, (parentbuf)->indent + 2 }
+#define VIR_BUFFER_INIT_CHILD(parentbuf) { NULL, (parentbuf)->indent + 2, 
false }
 
 struct _virBuffer {
 GString *str;
 int indent;
+bool ignored;
 };
 
 const char *virBufferCurrentContent(virBuffer *buf);
@@ -86,6 +87,7 @@ void virBufferURIEncodeString(virBuffer *buf, const char 
*str);
 
 void virBufferAdjustIndent(virBuffer *buf, int indent);
 void virBufferSetIndent(virBuffer *, int indent);
+void virBufferInheritIndent(virBuffer *buf, virBuffer *frombuf);
 
 size_t virBufferGetIndent(const virBuffer *buf);
 size_t virBufferGetEffectiveIndent(const virBuffer *buf);
@@ -94,3 +96,5 @@ void virBufferTrim(virBuffer *buf, const char *trim);
 void virBufferTrimChars(virBuffer *buf, const char *trim);
 void virBufferTrimLen(virBuffer *buf, int len);
 void virBufferAddStr(virBuffer *buf, const char *str);
+bool virBufferTouched(virBuffer *buf);
+void virBufferIgnore(virBuffer *buf);
diff --git a/src/util/virxml.c b/src/util/virxml.c
index 5ceef738..a6e50459 100644
--- a/src/util/virxml.c
+++ b/src/util/virxml.c
@@ -1746,3 +1746,123 @@ virXMLNewNode(xmlNsPtr ns,
 
 return ret;
 }
+
+
+/**
+ * virXMLChildNode:
+ * @node: Parent XML dom node pointer
+ * @name: Name of the child element
+ *
+ * Convenience function to return the child element of a XML node.
+ *
+ * Returns the pointer of child element node or NULL in case of failure.
+ * If there are many nodes match condition, it only returns the first node.
+ */
+xmlNodePtr
+virXMLChildNode(xmlNodePtr node, const char *name)
+{
+xmlNodePtr cur = 

[RFCv3 11/25] conf: Extract error-checking code from virNetworkDNSTxtDefParseXML

[Shi Lei]

Signed-off-by: Shi Lei 
---
 src/conf/network_conf.c | 49 -
 1 file changed, 38 insertions(+), 11 deletions(-)

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index d6eafa3f..87157591 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -887,26 +887,26 @@ virNetworkDNSSrvDefParseXML(const char *networkName,
 
 
 static int
-virNetworkDNSTxtDefParseXML(const char *networkName,
-xmlNodePtr node,
-virNetworkDNSTxtDef *def,
-bool partialOkay)
+virNetworkDNSTxtDefParseHook(xmlNodePtr node G_GNUC_UNUSED,
+ virNetworkDNSTxtDef *def,
+ const char *networkName,
+ void *parent G_GNUC_UNUSED,
+ void *opaque)
 {
 const char *bad = " ,";
+bool partialOkay = false;
+
+if (opaque)
+partialOkay = *((bool *) opaque);
 
-if (!(def->name = virXMLPropString(node, "name"))) {
-virReportError(VIR_ERR_XML_DETAIL,
-   _("missing required name attribute in DNS TXT record "
- "of network %s"), networkName);
-goto error;
-}
 if (strcspn(def->name, bad) != strlen(def->name)) {
 virReportError(VIR_ERR_XML_DETAIL,
_("prohibited character in DNS TXT record "
  "name '%s' of network %s"), def->name, networkName);
 goto error;
 }
-if (!(def->value = virXMLPropString(node, "value")) && !partialOkay) {
+
+if (!def->value && !partialOkay) {
 virReportError(VIR_ERR_XML_DETAIL,
_("missing required value attribute in DNS TXT record "
  "named '%s' of network %s"), def->name, networkName);
@@ -919,6 +919,33 @@ virNetworkDNSTxtDefParseXML(const char *networkName,
  "in DNS TXT record of network %s"), networkName);
 goto error;
 }
+
+return 0;
+
+ error:
+return -1;
+}
+
+
+static int
+virNetworkDNSTxtDefParseXML(const char *networkName,
+xmlNodePtr node,
+virNetworkDNSTxtDef *def,
+bool partialOkay)
+{
+if (!(def->name = virXMLPropString(node, "name"))) {
+virReportError(VIR_ERR_XML_DETAIL,
+   _("missing required name attribute in DNS TXT record "
+ "of network %s"), networkName);
+goto error;
+}
+
+def->value = virXMLPropString(node, "value");
+
+if (virNetworkDNSTxtDefParseHook(node, def, networkName,
+ NULL, &partialOkay) < 0)
+goto error;
+
 return 0;
 
  error:
-- 
2.25.1

[RFCv3 15/25] conf: Replace virNetworkDNSSrvDefParseXML(hardcoded) with namesake(generated)

[Shi Lei]

Signed-off-by: Shi Lei 
---
 src/conf/network_conf.c | 83 +++--
 src/conf/network_conf.h | 17 +
 2 files changed, 15 insertions(+), 85 deletions(-)

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 20128af0..cb2f3163 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -183,16 +183,6 @@ virNetworkDNSHostDefClear(virNetworkDNSHostDef *def)
 }
 
 
-static void
-virNetworkDNSSrvDefClear(virNetworkDNSSrvDef *def)
-{
-VIR_FREE(def->domain);
-VIR_FREE(def->service);
-VIR_FREE(def->protocol);
-VIR_FREE(def->target);
-}
-
-
 static void
 virNetworkDNSForwarderClear(virNetworkDNSForwarder *def)
 {
@@ -767,7 +757,7 @@ virNetworkDNSHostDefParseXML(const char *networkName,
 "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789" \
 "_-+/*"
 
-static int
+int
 virNetworkDNSSrvDefParseHook(xmlNodePtr node G_GNUC_UNUSED,
  virNetworkDNSSrvDef *def,
  const char *networkName,
@@ -874,69 +864,6 @@ virNetworkDNSSrvDefParseHook(xmlNodePtr node G_GNUC_UNUSED,
 }
 
 
-static int
-virNetworkDNSSrvDefParseXML(const char *networkName,
-xmlNodePtr node,
-xmlXPathContextPtr ctxt G_GNUC_UNUSED,
-virNetworkDNSSrvDef *def,
-bool partialOkay)
-{
-g_autofree char *portStr = NULL;
-g_autofree char *priorityStr = NULL;
-g_autofree char *weightStr = NULL;
-
-def->service = virXMLPropString(node, "service");
-def->protocol = virXMLPropString(node, "protocol");
-
-/* Following attributes are optional */
-def->domain = virXMLPropString(node, "domain");
-def->target = virXMLPropString(node, "target");
-
-portStr = virXMLPropString(node, "port");
-if (portStr) {
-if (virStrToLong_uip(portStr, NULL, 0, &def->port) < 0) {
-virReportError(VIR_ERR_XML_DETAIL,
-   _("invalid DNS SRV port attribute "
- "for service '%s' in network '%s'"),
-   def->service, networkName);
-goto error;
-}
-}
-
-priorityStr = virXMLPropString(node, "priority");
-if (priorityStr) {
-if (virStrToLong_uip(priorityStr, NULL, 0, &def->priority) < 0) {
-virReportError(VIR_ERR_XML_DETAIL,
-   _("Invalid DNS SRV priority attribute "
- "for service '%s' in network '%s'"),
-   def->service, networkName);
-goto error;
-}
-}
-
-weightStr = virXMLPropString(node, "weight");
-if (weightStr) {
-if (virStrToLong_uip(weightStr, NULL, 0, &def->weight) < 0) {
-virReportError(VIR_ERR_XML_DETAIL,
-   _("invalid DNS SRV weight attribute "
- "for service '%s' in network '%s'"),
-   def->service, networkName);
-goto error;
-}
-}
-
-if (virNetworkDNSSrvDefParseHook(node, def, networkName, def, &partialOkay,
- portStr, priorityStr, weightStr) < 0)
-goto error;
-
-return 0;
-
- error:
-virNetworkDNSSrvDefClear(def);
-return -1;
-}
-
-
 int
 virNetworkDNSTxtDefParseHook(xmlNodePtr node G_GNUC_UNUSED,
  virNetworkDNSTxtDef *def,
@@ -1082,8 +1009,8 @@ virNetworkDNSDefParseXML(const char *networkName,
 def->srvs = g_new0(virNetworkDNSSrvDef, nsrvs);
 
 for (i = 0; i < nsrvs; i++) {
-if (virNetworkDNSSrvDefParseXML(networkName, srvNodes[i], ctxt,
-&def->srvs[def->nsrvs], false) < 
0) {
+if (virNetworkDNSSrvDefParseXML(srvNodes[i], 
&def->srvs[def->nsrvs],
+networkName, def, NULL) < 0) {
 return -1;
 }
 def->nsrvs++;
@@ -3553,6 +3480,7 @@ virNetworkDefUpdateDNSSrv(virNetworkDef *def,
 bool isAdd = (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST ||
   command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST);
 int foundCt = 0;
+bool notAdd;
 
 memset(&srv, 0, sizeof(srv));
 
@@ -3566,7 +3494,8 @@ virNetworkDefUpdateDNSSrv(virNetworkDef *def,
 if (virNetworkDefUpdateCheckElementName(def, ctxt->node, "srv") < 0)
 goto cleanup;
 
-if (virNetworkDNSSrvDefParseXML(def->name, ctxt->node, ctxt, &srv, !isAdd) 
< 0)
+notAdd = !isAdd;
+if (virNetworkDNSSrvDefParseXML(ctxt->node, &srv, def->name, def, ¬Add) 
< 0)
 goto cleanup;
 
 for (i = 0; i < dns->nsrvs; i++) {
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index a4c83b46..a58d8953 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -131,14 +131,14 @@ struct _virNetworkDNSTxtDef {   /* genparse, genformat */
 };
 
 typedef struct _vir

[RFCv3 17/25] conf: Extract error-checking code from virNetworkDNSHostDefParseXML

[Shi Lei]

Signed-off-by: Shi Lei 
---
 src/conf/network_conf.c | 63 +
 1 file changed, 45 insertions(+), 18 deletions(-)

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 146c4977..b326ef5f 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -676,21 +676,57 @@ virNetworkDHCPDefParseXML(const char *networkName,
 
 
 static int
-virNetworkDNSHostDefParseXML(const char *networkName,
- xmlNodePtr node,
- virNetworkDNSHostDef *def,
- bool partialOkay)
+virNetworkDNSHostDefParseHook(xmlNodePtr node G_GNUC_UNUSED,
+  virNetworkDNSHostDef *def,
+  const char *networkName,
+  void *parent G_GNUC_UNUSED,
+  void *opaque,
+  const char *ip,
+  int nHostnameNodes G_GNUC_UNUSED)
 {
-xmlNodePtr cur;
-g_autofree char *ip = NULL;
+bool partialOkay = false;
 
-if (!(ip = virXMLPropString(node, "ip")) && !partialOkay) {
+if (opaque)
+partialOkay = *((bool *) opaque);
+
+if (!ip && !partialOkay) {
 virReportError(VIR_ERR_XML_DETAIL,
_("Missing IP address in network '%s' DNS HOST record"),
networkName);
 goto error;
 }
 
+if (def->nnames == 0 && !partialOkay) {
+virReportError(VIR_ERR_XML_DETAIL,
+   _("Missing hostname in network '%s' DNS HOST record"),
+   networkName);
+goto error;
+}
+
+if (!VIR_SOCKET_ADDR_VALID(&def->ip) && def->nnames == 0) {
+virReportError(VIR_ERR_XML_DETAIL,
+   _("Missing ip and hostname in network '%s' DNS HOST 
record"),
+   networkName);
+goto error;
+}
+
+return 0;
+
+ error:
+return -1;
+}
+
+
+static int
+virNetworkDNSHostDefParseXML(const char *networkName,
+ xmlNodePtr node,
+ virNetworkDNSHostDef *def,
+ bool partialOkay)
+{
+xmlNodePtr cur;
+g_autofree char *ip = NULL;
+
+ip = virXMLPropString(node, "ip");
 if (ip && (virSocketAddrParse(&def->ip, ip, AF_UNSPEC) < 0)) {
 virReportError(VIR_ERR_XML_DETAIL,
_("Invalid IP address in network '%s' DNS HOST record"),
@@ -720,19 +756,10 @@ virNetworkDNSHostDefParseXML(const char *networkName,
 }
 cur = cur->next;
 }
-if (def->nnames == 0 && !partialOkay) {
-virReportError(VIR_ERR_XML_DETAIL,
-   _("Missing hostname in network '%s' DNS HOST record"),
-   networkName);
-goto error;
-}
 
-if (!VIR_SOCKET_ADDR_VALID(&def->ip) && def->nnames == 0) {
-virReportError(VIR_ERR_XML_DETAIL,
-   _("Missing ip and hostname in network '%s' DNS HOST 
record"),
-   networkName);
+if (virNetworkDNSHostDefParseHook(node, def, networkName, def, 
&partialOkay,
+  ip, def->nnames) < 0)
 goto error;
-}
 
 return 0;
 
-- 
2.25.1

[RFCv3 14/25] conf: Extract error-checking code from virNetworkDNSSrvDefParseXML

[Shi Lei]

Signed-off-by: Shi Lei 
---
 src/conf/network_conf.c | 107 +++-
 1 file changed, 83 insertions(+), 24 deletions(-)

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index bb976a78..20128af0 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -768,23 +768,26 @@ virNetworkDNSHostDefParseXML(const char *networkName,
 "_-+/*"
 
 static int
-virNetworkDNSSrvDefParseXML(const char *networkName,
-xmlNodePtr node,
-xmlXPathContextPtr ctxt,
-virNetworkDNSSrvDef *def,
-bool partialOkay)
+virNetworkDNSSrvDefParseHook(xmlNodePtr node G_GNUC_UNUSED,
+ virNetworkDNSSrvDef *def,
+ const char *networkName,
+ void *parent G_GNUC_UNUSED,
+ void *opaque,
+ const char *portStr,
+ const char *priorityStr,
+ const char *weightStr)
 {
-int ret;
-VIR_XPATH_NODE_AUTORESTORE(ctxt)
-
-ctxt->node = node;
+bool partialOkay = false;
+if (opaque)
+partialOkay = *((bool *) opaque);
 
-if (!(def->service = virXMLPropString(node, "service")) && !partialOkay) {
+if (!def->service && !partialOkay) {
 virReportError(VIR_ERR_XML_DETAIL,
_("missing required service attribute in DNS SRV record 
"
  "of network '%s'"), networkName);
 goto error;
 }
+
 if (def->service) {
 if (strlen(def->service) > DNS_RECORD_LENGTH_SRV) {
 virReportError(VIR_ERR_XML_DETAIL,
@@ -802,13 +805,14 @@ virNetworkDNSSrvDefParseXML(const char *networkName,
 }
 }
 
-if (!(def->protocol = virXMLPropString(node, "protocol")) && !partialOkay) 
{
+if (!def->protocol && !partialOkay) {
 virReportError(VIR_ERR_XML_DETAIL,
_("missing required protocol attribute "
  "in DNS SRV record '%s' of network '%s'"),
def->service, networkName);
 goto error;
 }
+
 if (def->protocol &&
 strspn(def->protocol, PROTOCOL_CHARS) < strlen(def->protocol)) {
 virReportError(VIR_ERR_XML_DETAIL,
@@ -818,19 +822,14 @@ virNetworkDNSSrvDefParseXML(const char *networkName,
 goto error;
 }
 
-/* Following attributes are optional */
-def->domain = virXMLPropString(node, "domain");
-def->target = virXMLPropString(node, "target");
-
-ret = virXPathUInt("string(./@port)", ctxt, &def->port);
-if (ret >= 0 && !def->target) {
+if (portStr && !def->target) {
 virReportError(VIR_ERR_XML_DETAIL,
_("DNS SRV port attribute not permitted without "
  "target for service '%s' in network '%s'"),
def->service, networkName);
 goto error;
 }
-if (ret == -2 || (ret >= 0 && (def->port < 1 || def->port > 65535))) {
+if (portStr && (def->port < 1 || def->port > 65535)) {
 virReportError(VIR_ERR_XML_DETAIL,
_("invalid DNS SRV port attribute "
  "for service '%s' in network '%s'"),
@@ -838,15 +837,14 @@ virNetworkDNSSrvDefParseXML(const char *networkName,
 goto error;
 }
 
-ret = virXPathUInt("string(./@priority)", ctxt, &def->priority);
-if (ret >= 0 && !def->target) {
+if (priorityStr && !def->target) {
 virReportError(VIR_ERR_XML_DETAIL,
_("DNS SRV priority attribute not permitted without "
  "target for service '%s' in network '%s'"),
def->service, networkName);
 goto error;
 }
-if (ret == -2 || (ret >= 0 && def->priority > 65535)) {
+if (priorityStr && def->priority > 65535) {
 virReportError(VIR_ERR_XML_DETAIL,
_("Invalid DNS SRV priority attribute "
  "for service '%s' in network '%s'"),
@@ -854,15 +852,14 @@ virNetworkDNSSrvDefParseXML(const char *networkName,
 goto error;
 }
 
-ret = virXPathUInt("string(./@weight)", ctxt, &def->weight);
-if (ret >= 0 && !def->target) {
+if (weightStr && !def->target) {
 virReportError(VIR_ERR_XML_DETAIL,
_("DNS SRV weight attribute not permitted without "
  "target for service '%s' in network '%s'"),
def->service, networkName);
 goto error;
 }
-if (ret == -2 || (ret >= 0 && def->weight > 65535)) {
+if (weightStr && def->weight > 65535) {
 virReportError(VIR_ERR_XML_DETAIL,
_("invalid DNS SRV weight attribute "
  "for service '%s' in network '%s'"),
@@ -872,6 +869,68 @@ virNetworkDNSSrvDefParseXML(const char *networkNa

Re: [PATCH] bash-completion: Fix argument passing to $1

[Erik Skultety]

On Tue, Apr 20, 2021 at 03:34:53PM +0200, Michal Privoznik wrote:
> Our vsh bash completion string is merely just a wrapper over
> virsh/virt-admin complete (cmdComplete) - a hidden command that
> uses internal readline completion to generate list of candidates.
> But this means that we have to pass some additional arguments to
> the helper process: e.g. connection URI and R/O flag.
> 
> Candidates are printed on a separate line each (and can contain
> space), which means that when bash is reading the helper's output
> into an array, it needs to split items on '\n' char - hence the
> IFS=$'\n' prefix on the line executing the helper. This was
> introduced in b889594a70.
> 
> But this introduced a regression - those extra arguments we might
> pass are stored in a string and previously were split on a space
> character (because $IFS was kept untouched and by default
> contains space). But now, after the fix that's no longer the case
> and thus virsh/virt-admin sees ' -r -c URI' as one argument.
> 
> The solution is to take $IFS out of the picture by storing the
> extra arguments in an array instead of string.
> 
> Fixes: b889594a7092440dc916e3f43eeeaca2684571ee
> Signed-off-by: Michal Privoznik 
> ---
>  tools/bash-completion/vsh | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)

Makes sense and works.
Reviewed-by: Erik Skultety 

[PATCH] formatdomain.rst: update slice xml in "Hard drives" part

[“Meina]

Update slice xml from block lun disk because it doesn't support storage slice

Signed-off-by: Meina Li 
---
 docs/formatdomain.rst | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 1b9b221611..44aee9fa77 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -2381,9 +2381,6 @@ paravirtualized driver is specified via the ``disk`` 
element.
  


- 
-   
- 
  

  
@@ -2393,10 +2390,14 @@ paravirtualized driver is specified via the ``disk`` 
element.
  
  

-   
+   
+ 
+   
+ 
+   


-   
+   
  
  

-- 
2.27.0

Re: [PATCH] formatdomain.rst: update slice xml in "Hard drives" part

[Han Han]

On Thu, Apr 22, 2021 at 4:50 PM “Meina  wrote:

> Update slice xml from block lun disk because it doesn't support storage
> slice
>
> Better if mentioning the bug or the commit of forbidding slice element
with lun device
https://bugzilla.redhat.com/show_bug.cgi?id=1820040
5d72c3ce28 qemu: domain: Forbid slice/encryption/copy_on_read with disk
type='lun'


> Signed-off-by: Meina Li 
> ---
>  docs/formatdomain.rst | 11 ++-
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index 1b9b221611..44aee9fa77 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst
> @@ -2381,9 +2381,6 @@ paravirtualized driver is specified via the ``disk``
> element.
>   
> 
> 
> - 
> -   
> - 
>   
>  mode='client'/>
>   
> @@ -2393,10 +2390,14 @@ paravirtualized driver is specified via the
> ``disk`` element.
>   
>   
> 
> -   
> +   
> + 
> +   
> + 
> +   
> 
> 
> -   
> +   
>   
>   
> 
> --
> 2.27.0
>
>

Re: [PATCH] formatdomain.rst: update slice xml in "Hard drives" part

[Han Han]

On Thu, Apr 22, 2021 at 4:50 PM “Meina  wrote:

> Update slice xml from block lun disk because it doesn't support storage
> slice
>
> Signed-off-by: Meina Li 
> ---
>  docs/formatdomain.rst | 11 ++-
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index 1b9b221611..44aee9fa77 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst
> @@ -2381,9 +2381,6 @@ paravirtualized driver is specified via the ``disk``
> element.
>   
> 
> 
> - 
> -   
> - 
>   
>  mode='client'/>
>   
> @@ -2393,10 +2390,14 @@ paravirtualized driver is specified via the
> ``disk`` element.
>   
>   
> 
> -   
> +   
> + 
> +   
> + 
> +   
> 


> -   
>
Unluckily, the disk->geometry.trans is only supported on IDE disk:
https://github.com/libvirt/libvirt/blob/master/src/qemu/qemu_validate.c#L2461
Please don't update the disk target.

> +   
>   
>   
> 
> --
> 2.27.0
>
>

Re: [libvirt PATCH 00/10] Refactor more XML parsing boilerplate code, part III

[Michal Privoznik]

On 4/21/21 5:51 PM, Tim Wiederhake wrote:

For background, see
https://listman.redhat.com/archives/libvir-list/2021-April/msg00668.html

Tim Wiederhake (10):
   virDomainKeyWrapCipherDefParseXML: Use virXMLProp*
   virDomainKeyWrapDef: Make members virTristateSwitch
   qemuAppendKeyWrapMachineParm: Stricten parameter types
   virxml: Add virXMLPropULongLong
   virDomainDeviceDimmAddressParseXML: Use virXMLProp*
   virDomainHostdevSubsysSCSIHostDefParseXML: Use virXMLProp*
   virStorageEncryptionInfoParseCipher: Use virXMLProp*
   virDomainDeviceInfoParseXML: Use virXMLProp*
   virDomainDiskSourceNetworkParse: Use virXMLProp*
   virDomainMemorytuneDefParseMemory: Use virXMLProp*

  src/conf/domain_conf.c | 214 -
  src/conf/domain_conf.h |   4 +-
  src/conf/storage_encryption_conf.c |  16 +--
  src/libvirt_private.syms   |   1 +
  src/qemu/qemu_command.c|   3 +-
  src/util/virxml.c  |  56 
  src/util/virxml.h  |   8 ++
  7 files changed, 125 insertions(+), 177 deletions(-)



Reviewed-by: Michal Privoznik 

and pushed.

Michal

[libvirt PATCH 03/10] virDomainDiskDef: Change type of rerror_policy to virDomainDiskErrorPolicy

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/conf/domain_conf.c | 16 ++--
 src/conf/domain_conf.h |  2 +-
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 4a0358831b..0b12ec61f9 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8988,12 +8988,16 @@ virDomainDiskDefDriverParseXML(virDomainDiskDef *def,
 }
 VIR_FREE(tmp);
 
-if ((tmp = virXMLPropString(cur, "rerror_policy")) &&
-(((def->rerror_policy = virDomainDiskErrorPolicyTypeFromString(tmp)) 
<= 0) ||
- (def->rerror_policy == VIR_DOMAIN_DISK_ERROR_POLICY_ENOSPACE))) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("unknown disk read error policy '%s'"), tmp);
-return -1;
+if ((tmp = virXMLPropString(cur, "rerror_policy"))) {
+int rerror_policy;
+
+if (((rerror_policy = virDomainDiskErrorPolicyTypeFromString(tmp)) <= 
0) ||
+(rerror_policy == VIR_DOMAIN_DISK_ERROR_POLICY_ENOSPACE)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("unknown disk read error policy '%s'"), tmp);
+return -1;
+}
+def->rerror_policy = rerror_policy;
 }
 VIR_FREE(tmp);
 
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index cacb9d0430..d808f5b260 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -568,7 +568,7 @@ struct _virDomainDiskDef {
 char *product;
 virDomainDiskCache cachemode;
 virDomainDiskErrorPolicy error_policy;
-int rerror_policy; /* enum virDomainDiskErrorPolicy */
+virDomainDiskErrorPolicy rerror_policy;
 int iomode; /* enum virDomainDiskIo */
 virTristateSwitch ioeventfd;
 virTristateSwitch event_idx;
-- 
2.26.3

[libvirt PATCH 02/10] virDomainDiskDef: Change type of error_policy to virDomainDiskErrorPolicy

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/conf/domain_conf.c | 14 +-
 src/conf/domain_conf.h |  2 +-
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 33c6412642..4a0358831b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8976,11 +8976,15 @@ virDomainDiskDefDriverParseXML(virDomainDiskDef *def,
 }
 VIR_FREE(tmp);
 
-if ((tmp = virXMLPropString(cur, "error_policy")) &&
-(def->error_policy = virDomainDiskErrorPolicyTypeFromString(tmp)) <= 
0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("unknown disk error policy '%s'"), tmp);
-return -1;
+if ((tmp = virXMLPropString(cur, "error_policy"))) {
+int error_policy;
+
+if ((error_policy = virDomainDiskErrorPolicyTypeFromString(tmp)) <= 0) 
{
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("unknown disk error policy '%s'"), tmp);
+return -1;
+}
+def->error_policy = error_policy;
 }
 VIR_FREE(tmp);
 
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 3675e26eb0..cacb9d0430 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -567,7 +567,7 @@ struct _virDomainDiskDef {
 char *vendor;
 char *product;
 virDomainDiskCache cachemode;
-int error_policy;  /* enum virDomainDiskErrorPolicy */
+virDomainDiskErrorPolicy error_policy;
 int rerror_policy; /* enum virDomainDiskErrorPolicy */
 int iomode; /* enum virDomainDiskIo */
 virTristateSwitch ioeventfd;
-- 
2.26.3

[libvirt PATCH 00/10] Refactor more XML parsing boilerplate code, part IV

[Tim Wiederhake]

For background, see
https://listman.redhat.com/archives/libvir-list/2021-April/msg00668.html

Tim Wiederhake (10):
  virDomainDiskDef: Change type of cachemode to virDomainDiskCache
  virDomainDiskDef: Change type of error_policy to
virDomainDiskErrorPolicy
  virDomainDiskDef: Change type of rerror_policy to
virDomainDiskErrorPolicy
  virDomainDiskDef: Change type of iomode to virDomainDiskInfo
  virDomainDiskDef: Change type of discard to virDomainDiskDiscard
  virDomainDiskDef: Change type of detect_zeroes to
virDomainDiskDetectZeroes
  virDomainDiskDefDriverParseXML: Use virXMLProp*
  domain_conf: Introduce function virDomainChrSouceModeTypeFromString
  domain_conf: Remove function virDomainChrSourceDefParseMode
  virDomainChrSourceDefParseTCP: Use virXMLProp*

 src/conf/domain_conf.c | 176 +
 src/conf/domain_conf.h |  12 +--
 2 files changed, 61 insertions(+), 127 deletions(-)

-- 
2.26.3

[libvirt PATCH 05/10] virDomainDiskDef: Change type of discard to virDomainDiskDiscard

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/conf/domain_conf.c | 14 +-
 src/conf/domain_conf.h |  2 +-
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 72664bd72b..9a4252099e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -9046,11 +9046,15 @@ virDomainDiskDefDriverParseXML(virDomainDiskDef *def,
 }
 VIR_FREE(tmp);
 
-if ((tmp = virXMLPropString(cur, "discard")) &&
-(def->discard = virDomainDiskDiscardTypeFromString(tmp)) <= 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("unknown disk discard mode '%s'"), tmp);
-return -1;
+if ((tmp = virXMLPropString(cur, "discard"))) {
+int discard;
+
+if ((discard = virDomainDiskDiscardTypeFromString(tmp)) <= 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("unknown disk discard mode '%s'"), tmp);
+return -1;
+}
+def->discard = discard;
 }
 VIR_FREE(tmp);
 
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 3402b3f85a..eb4991a312 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -579,7 +579,7 @@ struct _virDomainDiskDef {
 virDomainDeviceInfo info;
 virTristateBool rawio;
 virDomainDeviceSGIO sgio;
-int discard; /* enum virDomainDiskDiscard */
+virDomainDiskDiscard discard;
 unsigned int iothread; /* unused = 0, > 0 specific thread # */
 int detect_zeroes; /* enum virDomainDiskDetectZeroes */
 char *domain_name; /* backend domain name */
-- 
2.26.3

[libvirt PATCH 07/10] virDomainDiskDefDriverParseXML: Use virXMLProp*

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/conf/domain_conf.c | 150 +++--
 1 file changed, 39 insertions(+), 111 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 60f961c2b9..1bbf907596 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8964,109 +8964,51 @@ virDomainDiskDefDriverParseXML(virDomainDiskDef *def,
 
 def->driverName = virXMLPropString(cur, "name");
 
-if ((tmp = virXMLPropString(cur, "cache"))) {
-int cachemode;
-
-if ((cachemode = virDomainDiskCacheTypeFromString(tmp)) < 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("unknown disk cache mode '%s'"), tmp);
-return -1;
-}
-def->cachemode = cachemode;
-}
-VIR_FREE(tmp);
-
-if ((tmp = virXMLPropString(cur, "error_policy"))) {
-int error_policy;
-
-if ((error_policy = virDomainDiskErrorPolicyTypeFromString(tmp)) <= 0) 
{
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("unknown disk error policy '%s'"), tmp);
-return -1;
-}
-def->error_policy = error_policy;
-}
-VIR_FREE(tmp);
-
-if ((tmp = virXMLPropString(cur, "rerror_policy"))) {
-int rerror_policy;
+if (virXMLPropEnum(cur, "cache", virDomainDiskCacheTypeFromString,
+   VIR_XML_PROP_NONE, &def->cachemode) < 0)
+return -1;
 
-if (((rerror_policy = virDomainDiskErrorPolicyTypeFromString(tmp)) <= 
0) ||
-(rerror_policy == VIR_DOMAIN_DISK_ERROR_POLICY_ENOSPACE)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("unknown disk read error policy '%s'"), tmp);
-return -1;
-}
-def->rerror_policy = rerror_policy;
-}
-VIR_FREE(tmp);
+if (virXMLPropEnum(cur, "error_policy",
+   virDomainDiskErrorPolicyTypeFromString,
+   VIR_XML_PROP_NONZERO, &def->error_policy) < 0)
+return -1;
 
-if ((tmp = virXMLPropString(cur, "io"))) {
-int iomode;
+if (virXMLPropEnum(cur, "rerror_policy",
+   virDomainDiskErrorPolicyTypeFromString,
+   VIR_XML_PROP_NONZERO, &def->rerror_policy) < 0)
+return -1;
 
-if ((iomode = virDomainDiskIoTypeFromString(tmp)) <= 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("unknown disk io mode '%s'"), tmp);
-return -1;
-}
-def->iomode = iomode;
+if (def->rerror_policy == VIR_DOMAIN_DISK_ERROR_POLICY_ENOSPACE) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("Invalid disk read error policy: '%s'"),
+   
virDomainDiskErrorPolicyTypeToString(def->rerror_policy));
+return -1;
 }
-VIR_FREE(tmp);
 
-if ((tmp = virXMLPropString(cur, "ioeventfd"))) {
-int value;
-if ((value = virTristateSwitchTypeFromString(tmp)) <= 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("unknown disk ioeventfd mode '%s'"), tmp);
-return -1;
-}
-def->ioeventfd = value;
-}
-VIR_FREE(tmp);
+if (virXMLPropEnum(cur, "io", virDomainDiskIoTypeFromString,
+   VIR_XML_PROP_NONZERO, &def->iomode) < 0)
+return -1;
 
-if ((tmp = virXMLPropString(cur, "event_idx"))) {
-int value;
-if ((value = virTristateSwitchTypeFromString(tmp)) <= 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("unknown disk event_idx mode '%s'"), tmp);
-return -1;
-}
-def->event_idx = value;
-}
-VIR_FREE(tmp);
+if (virXMLPropTristateSwitch(cur, "ioeventfd", VIR_XML_PROP_NONE,
+ &def->ioeventfd) < 0)
+return -1;
 
-if ((tmp = virXMLPropString(cur, "copy_on_read"))) {
-int value;
-if ((value = virTristateSwitchTypeFromString(tmp)) <= 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("unknown disk copy_on_read mode '%s'"), tmp);
-return -1;
-}
-def->copy_on_read = value;
-}
-VIR_FREE(tmp);
+if (virXMLPropTristateSwitch(cur, "event_idx", VIR_XML_PROP_NONE,
+ &def->event_idx) < 0)
+return -1;
 
-if ((tmp = virXMLPropString(cur, "discard"))) {
-int discard;
+if (virXMLPropTristateSwitch(cur, "copy_on_read", VIR_XML_PROP_NONE,
+ &def->copy_on_read) < 0)
+return -1;
 
-if ((discard = virDomainDiskDiscardTypeFromString(tmp)) <= 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("unknown disk discard mode '%s'"), tmp);
-return -1;
-}
-def->discard = discard;
-}
-   

[libvirt PATCH 09/10] domain_conf: Remove function virDomainChrSourceDefParseMode

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/conf/domain_conf.c | 40 +---
 1 file changed, 9 insertions(+), 31 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index cdc32e52dd..447fc7dfcb 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11469,31 +11469,6 @@ virDomainChrSourceModeTypeFromString(const char *str)
 return -1;
 }
 
-/**
- * virDomainChrSourceDefParseMode:
- * @source: XML dom node
- *
- * Returns: -1 in case of error,
- *  virDomainChrSourceModeType in case of success
- */
-static int
-virDomainChrSourceDefParseMode(xmlNodePtr source)
-{
-g_autofree char *mode = virXMLPropString(source, "mode");
-int result;
-
-if (!mode)
-return VIR_DOMAIN_CHR_SOURCE_MODE_CONNECT;
-
-if ((result = virDomainChrSourceModeTypeFromString(mode)) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("Unknown source mode '%s'"), mode);
-return -1;
-}
-
-return result;
-}
-
 
 static int
 virDomainChrSourceDefParseTCP(virDomainChrSourceDef *def,
@@ -11501,11 +11476,12 @@ virDomainChrSourceDefParseTCP(virDomainChrSourceDef 
*def,
   xmlXPathContextPtr ctxt,
   unsigned int flags)
 {
-int mode;
+virDomainChrSourceModeType mode = VIR_DOMAIN_CHR_SOURCE_MODE_CONNECT;
 int tmpVal;
 g_autofree char *tmp = NULL;
 
-if ((mode = virDomainChrSourceDefParseMode(source)) < 0)
+if (virXMLPropEnum(source, "mode", virDomainChrSourceModeTypeFromString,
+   VIR_XML_PROP_NONE, &mode) < 0)
 return -1;
 
 def->data.tcp.listen = mode == VIR_DOMAIN_CHR_SOURCE_MODE_BIND;
@@ -11549,9 +11525,10 @@ static int
 virDomainChrSourceDefParseUDP(virDomainChrSourceDef *def,
   xmlNodePtr source)
 {
-int mode;
+virDomainChrSourceModeType mode = VIR_DOMAIN_CHR_SOURCE_MODE_CONNECT;
 
-if ((mode = virDomainChrSourceDefParseMode(source)) < 0)
+if (virXMLPropEnum(source, "mode", virDomainChrSourceModeTypeFromString,
+   VIR_XML_PROP_NONE, &mode) < 0)
 return -1;
 
 if (mode == VIR_DOMAIN_CHR_SOURCE_MODE_CONNECT &&
@@ -11573,9 +11550,10 @@ virDomainChrSourceDefParseUnix(virDomainChrSourceDef 
*def,
xmlNodePtr source,
xmlXPathContextPtr ctxt)
 {
-int mode;
+virDomainChrSourceModeType mode = VIR_DOMAIN_CHR_SOURCE_MODE_CONNECT;
 
-if ((mode = virDomainChrSourceDefParseMode(source)) < 0)
+if (virXMLPropEnum(source, "mode", virDomainChrSourceModeTypeFromString,
+   VIR_XML_PROP_NONE, &mode) < 0)
 return -1;
 
 def->data.nix.listen = mode == VIR_DOMAIN_CHR_SOURCE_MODE_BIND;
-- 
2.26.3

[libvirt PATCH 10/10] virDomainChrSourceDefParseTCP: Use virXMLProp*

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/conf/domain_conf.c | 28 
 1 file changed, 8 insertions(+), 20 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 447fc7dfcb..24c0943d62 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11477,8 +11477,6 @@ virDomainChrSourceDefParseTCP(virDomainChrSourceDef 
*def,
   unsigned int flags)
 {
 virDomainChrSourceModeType mode = VIR_DOMAIN_CHR_SOURCE_MODE_CONNECT;
-int tmpVal;
-g_autofree char *tmp = NULL;
 
 if (virXMLPropEnum(source, "mode", virDomainChrSourceModeTypeFromString,
VIR_XML_PROP_NONE, &mode) < 0)
@@ -11488,26 +11486,16 @@ virDomainChrSourceDefParseTCP(virDomainChrSourceDef 
*def,
 def->data.tcp.host = virXMLPropString(source, "host");
 def->data.tcp.service = virXMLPropString(source, "service");
 
-if ((tmp = virXMLPropString(source, "tls"))) {
-int value;
-if ((value = virTristateBoolTypeFromString(tmp)) <= 0) {
-virReportError(VIR_ERR_XML_ERROR,
-   _("unknown chardev 'tls' setting '%s'"),
-   tmp);
-return -1;
-}
-def->data.tcp.haveTLS = value;
-VIR_FREE(tmp);
-}
+if (virXMLPropTristateBool(source, "tls", VIR_XML_PROP_NONE,
+   &def->data.tcp.haveTLS) < 0)
+return -1;
 
-if ((flags & VIR_DOMAIN_DEF_PARSE_STATUS) &&
-(tmp = virXMLPropString(source, "tlsFromConfig"))) {
-if (virStrToLong_i(tmp, NULL, 10, &tmpVal) < 0) {
-virReportError(VIR_ERR_XML_ERROR,
-   _("Invalid tlsFromConfig value: %s"),
-   tmp);
+if (flags & VIR_DOMAIN_DEF_PARSE_STATUS) {
+int tmpVal;
+
+if (virXMLPropInt(source, "tlsFromConfig", 10, VIR_XML_PROP_NONE,
+  &tmpVal) < 0)
 return -1;
-}
 def->data.tcp.tlsFromConfig = !!tmpVal;
 }
 
-- 
2.26.3

[libvirt PATCH 01/10] virDomainDiskDef: Change type of cachemode to virDomainDiskCache

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/conf/domain_conf.c | 14 +-
 src/conf/domain_conf.h |  2 +-
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 720d56cf69..33c6412642 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8964,11 +8964,15 @@ virDomainDiskDefDriverParseXML(virDomainDiskDef *def,
 
 def->driverName = virXMLPropString(cur, "name");
 
-if ((tmp = virXMLPropString(cur, "cache")) &&
-(def->cachemode = virDomainDiskCacheTypeFromString(tmp)) < 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("unknown disk cache mode '%s'"), tmp);
-return -1;
+if ((tmp = virXMLPropString(cur, "cache"))) {
+int cachemode;
+
+if ((cachemode = virDomainDiskCacheTypeFromString(tmp)) < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("unknown disk cache mode '%s'"), tmp);
+return -1;
+}
+def->cachemode = cachemode;
 }
 VIR_FREE(tmp);
 
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index fab7a0208e..3675e26eb0 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -566,7 +566,7 @@ struct _virDomainDiskDef {
 char *wwn;
 char *vendor;
 char *product;
-int cachemode; /* enum virDomainDiskCache */
+virDomainDiskCache cachemode;
 int error_policy;  /* enum virDomainDiskErrorPolicy */
 int rerror_policy; /* enum virDomainDiskErrorPolicy */
 int iomode; /* enum virDomainDiskIo */
-- 
2.26.3

[libvirt PATCH 08/10] domain_conf: Introduce function virDomainChrSouceModeTypeFromString

[Tim Wiederhake]

Preparatory step to remove virDomainChrSourceDefParseMode.

Signed-off-by: Tim Wiederhake 
---
 src/conf/domain_conf.c | 28 ++--
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 1bbf907596..cdc32e52dd 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11455,6 +11455,20 @@ typedef enum {
 } virDomainChrSourceModeType;
 
 
+static int
+virDomainChrSourceModeTypeFromString(const char *str)
+{
+if (!str)
+return -1;
+
+if (STREQ(str, "connect"))
+return VIR_DOMAIN_CHR_SOURCE_MODE_CONNECT;
+if (STREQ(str, "bind"))
+return VIR_DOMAIN_CHR_SOURCE_MODE_BIND;
+
+return -1;
+}
+
 /**
  * virDomainChrSourceDefParseMode:
  * @source: XML dom node
@@ -11466,16 +11480,18 @@ static int
 virDomainChrSourceDefParseMode(xmlNodePtr source)
 {
 g_autofree char *mode = virXMLPropString(source, "mode");
+int result;
 
-if (!mode || STREQ(mode, "connect")) {
+if (!mode)
 return VIR_DOMAIN_CHR_SOURCE_MODE_CONNECT;
-} else if (STREQ(mode, "bind")) {
-return VIR_DOMAIN_CHR_SOURCE_MODE_BIND;
+
+if ((result = virDomainChrSourceModeTypeFromString(mode)) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Unknown source mode '%s'"), mode);
+return -1;
 }
 
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("Unknown source mode '%s'"), mode);
-return -1;
+return result;
 }
 
 
-- 
2.26.3

[libvirt PATCH 06/10] virDomainDiskDef: Change type of detect_zeroes to virDomainDiskDetectZeroes

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/conf/domain_conf.c | 14 +-
 src/conf/domain_conf.h |  2 +-
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9a4252099e..60f961c2b9 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -9083,11 +9083,15 @@ virDomainDiskDefDriverParseXML(virDomainDiskDef *def,
 VIR_FREE(tmp);
 }
 
-if ((tmp = virXMLPropString(cur, "detect_zeroes")) &&
-(def->detect_zeroes = virDomainDiskDetectZeroesTypeFromString(tmp)) <= 
0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("unknown driver detect_zeroes value '%s'"), tmp);
-return -1;
+if ((tmp = virXMLPropString(cur, "detect_zeroes"))) {
+int detect_zeroes;
+
+if ((detect_zeroes = virDomainDiskDetectZeroesTypeFromString(tmp)) <= 
0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("unknown driver detect_zeroes value '%s'"), tmp);
+return -1;
+}
+def->detect_zeroes = detect_zeroes;
 }
 VIR_FREE(tmp);
 
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index eb4991a312..a7cad31896 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -581,7 +581,7 @@ struct _virDomainDiskDef {
 virDomainDeviceSGIO sgio;
 virDomainDiskDiscard discard;
 unsigned int iothread; /* unused = 0, > 0 specific thread # */
-int detect_zeroes; /* enum virDomainDiskDetectZeroes */
+virDomainDiskDetectZeroes detect_zeroes;
 char *domain_name; /* backend domain name */
 unsigned int queues;
 virDomainDiskModel model;
-- 
2.26.3

[libvirt PATCH 04/10] virDomainDiskDef: Change type of iomode to virDomainDiskInfo

[Tim Wiederhake]

Signed-off-by: Tim Wiederhake 
---
 src/conf/domain_conf.c | 14 +-
 src/conf/domain_conf.h |  2 +-
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0b12ec61f9..72664bd72b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -9001,11 +9001,15 @@ virDomainDiskDefDriverParseXML(virDomainDiskDef *def,
 }
 VIR_FREE(tmp);
 
-if ((tmp = virXMLPropString(cur, "io")) &&
-(def->iomode = virDomainDiskIoTypeFromString(tmp)) <= 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("unknown disk io mode '%s'"), tmp);
-return -1;
+if ((tmp = virXMLPropString(cur, "io"))) {
+int iomode;
+
+if ((iomode = virDomainDiskIoTypeFromString(tmp)) <= 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("unknown disk io mode '%s'"), tmp);
+return -1;
+}
+def->iomode = iomode;
 }
 VIR_FREE(tmp);
 
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index d808f5b260..3402b3f85a 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -569,7 +569,7 @@ struct _virDomainDiskDef {
 virDomainDiskCache cachemode;
 virDomainDiskErrorPolicy error_policy;
 virDomainDiskErrorPolicy rerror_policy;
-int iomode; /* enum virDomainDiskIo */
+virDomainDiskIo iomode;
 virTristateSwitch ioeventfd;
 virTristateSwitch event_idx;
 virTristateSwitch copy_on_read;
-- 
2.26.3

Re: [PATCH 1/4] lxc_controller: Initialize ctrl->handshakeFd properly

[Martin Kletzander]

On Tue, Apr 20, 2021 at 04:15:15PM +0200, Michal Privoznik wrote:

The lxc_controller has a structure that's keeping its internal
state, including so called handshakeFd which is the write end of
a pipe that's used to signal to the LXC driver that the container
is set up and ready to run. However, the struct member is not
initialized to -1, so if anything fails before it is set then the
virLXCControllerFree() function tries to close FD 0 (stdin).

Signed-off-by: Michal Privoznik 
---
src/lxc/lxc_controller.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)



Reviewed-by: Martin Kletzander 


diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index ab5fc8b88f..50b2987d9a 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -193,8 +193,8 @@ static virLXCController *virLXCControllerNew(const char 
*name)

ctrl->timerShutdown = -1;
ctrl->firstClient = true;
-
ctrl->name = g_strdup(name);
+ctrl->handshakeFd = -1;

if (!(driver = virLXCControllerDriverNew()))
goto error;
--
2.26.3



signature.asc
Description: PGP signature

Re: [PATCH 2/4] lxc_controller: Move closing of handshakeFd out of virLXCControllerDaemonHandshake()

[Martin Kletzander]

On Tue, Apr 20, 2021 at 04:15:16PM +0200, Michal Privoznik wrote:

Future commits will want to reuse the handshakeFd and thus it
mustn't be closed in virLXCControllerDaemonHandshake(). Do the
closing explicitly afterwards.

Signed-off-by: Michal Privoznik 
---
src/lxc/lxc_controller.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)



Reviewed-by: Martin Kletzander 


diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 50b2987d9a..797547b05c 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -353,7 +353,6 @@ static int virLXCControllerDaemonHandshake(virLXCController 
*ctrl)
 _("error sending continue signal to daemon"));
return -1;
}
-VIR_FORCE_CLOSE(ctrl->handshakeFd);
return 0;
}

@@ -2403,6 +2402,9 @@ virLXCControllerRun(virLXCController *ctrl)
if (virLXCControllerDaemonHandshake(ctrl) < 0)
goto cleanup;

+/* and preemptively close handshakeFd */
+VIR_FORCE_CLOSE(ctrl->handshakeFd);
+
/* We must not hold open a dbus connection for life
 * of LXC instance, since dbus-daemon is limited to
 * only a few 100 connections by default
--
2.26.3



signature.asc
Description: PGP signature

Re: [PATCH 3/4] lxc: Pass another pipe to lxc_controller

[Martin Kletzander]

On Tue, Apr 20, 2021 at 04:15:17PM +0200, Michal Privoznik wrote:

Currently, there is only a single pipe passed to lxc_controller
and it is used by lxc_controller to signal to the LXC driver that
the container is set up and ready to run. However, in the next
commit we will need to signal that the LXC driver has done its
part of startup process and thus the controller can proceed.
Unfortunately, virCommand handshake can't be used for this,
because it's already used to read controller's PID.

Signed-off-by: Michal Privoznik 
---
src/lxc/lxc_controller.c | 46 
src/lxc/lxc_process.c| 21 +++---
2 files changed, 46 insertions(+), 21 deletions(-)



[...]


diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index ac635efe7a..493e19f03d 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -939,7 +939,8 @@ virLXCProcessBuildControllerCmd(virLXCDriver *driver,
int *nsInheritFDs,
int *files,
size_t nfiles,
-int handshakefd,
+int handshakefdW,
+int handshakefdR,
int * const logfd,
const char *pidfile)
{
@@ -1003,12 +1004,13 @@ virLXCProcessBuildControllerCmd(virLXCDriver *driver,
 virSecurityManagerGetModel(driver->securityManager));

virCommandAddArg(cmd, "--handshakefd");


Maybe using plural here?


-virCommandAddArgFormat(cmd, "%d", handshakefd);
+virCommandAddArgFormat(cmd, "%d:%d", handshakefdR, handshakefdW);



This was a little bit confusing as I would put it the other way, but
ultimately does not matter.  It would be easier to spot if you also
changed the help output and maybe s/handshakefd/handshakefds/ in the
struct option passed to getopt_long().

Anyway, with at least the help output changed:

Reviewed-by: Martin Kletzander 


signature.asc
Description: PGP signature

Re: [PATCH 4/4] lxc: Let the driver detect CGroups earlier

[Martin Kletzander]

On Tue, Apr 20, 2021 at 04:15:18PM +0200, Michal Privoznik wrote:

This is the bug I'm facing. I deliberately configured a container
so that the source of a  to passthrough doesn't
exist. The start fails with:

 lxcContainerPivotRoot:669 : Failed to create /non-existent/path/.oldroot: 
Permission denied

which is expected. But what is NOT expected is that CGroup
hierarchy is left behind. This is because the controller sets up
the CGroup hierarchy, user namespace, moves interfaces, etc. and
finally checks whether container setup (done in a separate
process) succeeded. Only after all this the error is propagated
to the LXC driver. The driver aborts the startup and tries to
perform the cleanup, but this is missing CGroups because those
weren't detected yet.

Ideally, whenever a function fails, it tries to unroll back so
that is has no artifacts left behind (look at all those frees/FD
closes/etc. at end of functions). But with CGroups it is
different - the controller process can't clean up after itself,
because it is still running inside that CGroup.

Therefore, what we have to do is to let the driver detect CGroups
as soon as they are created, and proceed with controller
execution only after that.

Signed-off-by: Michal Privoznik 
---
src/lxc/lxc_controller.c | 19 +--
src/lxc/lxc_process.c| 20 
2 files changed, 37 insertions(+), 2 deletions(-)



[...]


diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 493e19f03d..55e74e913b 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -1473,6 +1473,7 @@ int virLXCProcessStart(virConnectPtr conn,
if (g_atomic_int_add(&driver->nactive, 1) == 0 && driver->inhibitCallback)
driver->inhibitCallback(true, driver->inhibitOpaque);

+/* The first synchronization point is when the controller creates CGroups. 
*/
if (lxcContainerWaitForContinue(handshakefds[0]) < 0) {
char out[1024];

@@ -1504,6 +1505,25 @@ int virLXCProcessStart(virConnectPtr conn,
goto cleanup;
}

+if (lxcContainerSendContinue(handshakefds[3]) < 0) {
+virReportSystemError(errno, "%s",
+ _("Failed to send continue signal to 
controller"));
+goto cleanup;
+}
+
+/* The second synchronization point is when the controller finished
+ * creating the container. */
+if (lxcContainerWaitForContinue(handshakefds[0]) < 0) {
+char out[1024];
+
+if (!(virLXCProcessReadLogOutput(vm, logfile, pos, out, 1024) < 0)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("guest failed to start: %s"), out);


It would be nice to differentiate the error message reported here to the
one reported originally handful of lines above.  Just for the sake of
future debugging.

Other than that the patches could be nicer to read if the variables were
split or named differently, but after reading through a lot of the code
around these parts I see that such a request would be unreasonable for
this patch.  Maybe a rewrite of the signalling could be an item for a
TODO list, although I am not sure what would be the best way to handle
this.  Can we use for example eventfd()s?  Would that help us at all?

Anyway, with one of the error messages tweaked a bit:

Reviewed-by: Martin Kletzander 


signature.asc
Description: PGP signature

[libvirt PATCH 2/3] docs: virtiofs: add section about externally-launched virtiofsd

[Ján Tomko]

Provide an exmple in a place more visible than formatdomain.html.

Signed-off-by: Ján Tomko 
---
 docs/kbase/virtiofs.rst | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/docs/kbase/virtiofs.rst b/docs/kbase/virtiofs.rst
index c0bc07a68d..09674a4a76 100644
--- a/docs/kbase/virtiofs.rst
+++ b/docs/kbase/virtiofs.rst
@@ -149,3 +149,17 @@ More optional elements can be specified
 
 
   
+
+Externally-launched virtiofsd
+=
+
+Libvirtd can also connect the ``vhost-user-fs`` device to a ``virtiofsd``
+daemon launched outside of libvirtd. In that case socket permissions,
+
+
+::
+
+  
+
+
+  
-- 
2.29.2

[libvirt PATCH 0/3] docs: qemu: add socket for virtiofs filesystems

[Ján Tomko]

Add documentation for the new attribute.

Ján Tomko (3):
  docs: document new socket attribute for virtiofs
  docs: virtiofs: add section about externally-launched virtiofsd
  NEWS: qemu: add socket for virtiofs filesystems

 NEWS.rst|  6 ++
 docs/formatdomain.rst   | 10 +-
 docs/kbase/virtiofs.rst | 14 ++
 3 files changed, 29 insertions(+), 1 deletion(-)

-- 
2.29.2

[libvirt PATCH 1/3] docs: document new socket attribute for virtiofs

[Ján Tomko]

Describe the attribute and add an example.

Signed-off-by: Ján Tomko 
---
 docs/formatdomain.rst | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 1b9b221611..282176c4f4 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -3242,6 +3242,10 @@ A directory on the host that can be accessed directly 
from the guest.
  
  
  
+ 
+ 
+ 
+ 
  ...

...
@@ -3369,7 +3373,11 @@ A directory on the host that can be accessed directly 
from the guest.
 ``source``
The resource on the host that is being accessed in the guest. The ``name``
attribute must be used with ``type='template'``, and the ``dir`` attribute
-   must be used with ``type='mount'``. The ``usage`` attribute is used with
+   must be used with ``type='mount'``. For ``virtiofs``, the ``socket`` 
attribute
+   can be used to connect to a virtiofsd daemon launched outside of libvirt.
+   In that case, the ``target`` element does not apply and neither do most
+   virtiofs-related options, since they are controlled by virtiofsd, not 
libvirtd.
+   The ``usage`` attribute is used with
``type='ram'`` to set the memory limit in KiB, unless units are specified by
the ``units`` attribute.
 ``target``
-- 
2.29.2

[libvirt PATCH 3/3] NEWS: qemu: add socket for virtiofs filesystems

[Ján Tomko]

Signed-off-by: Ján Tomko 
---
 NEWS.rst | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/NEWS.rst b/NEWS.rst
index 5f8b0ae02d..7b55dd056f 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -18,6 +18,12 @@ v7.3.0 (unreleased)
 The xen driver now supports domains with more than 4TB of memory with
 xen >= 4.13.
 
+  * qemu: add socket for virtiofs filesystems
+
+Libvirt now supports ``filesystem`` devices that connect to
+a ``virtiofsd`` daemon launched outside of libvirtd, via the
+``socket`` attribute of the ``source`` element.
+
 * **Improvements**
 
 * **Bug fixes**
-- 
2.29.2

Re: [libvirt] [PATCH 0/2] virsh completer fixes

[Michal Privoznik]

On 4/22/21 12:38 PM, Lin Ma wrote:

Lin Ma (2):
   virsh: Add mountpoint completion to domfsfreeze/domfsthaw command
   virsh: Fix completion logic to guestvcpus command

  tools/virsh-completer-domain.c | 104 +++--
  tools/virsh-completer-domain.h |   4 ++
  tools/virsh-domain.c   |   2 +
  3 files changed, 93 insertions(+), 17 deletions(-)



Reviewed-by: Michal Privoznik 

and pushed.

Michal

Re: [libvirt] [PATCH 1/2] virsh: Add mountpoint completion to domfsfreeze/domfsthaw command

[Michal Privoznik]

On 4/22/21 12:38 PM, Lin Ma wrote:

Signed-off-by: Lin Ma 
---
  tools/virsh-completer-domain.c | 43 ++
  tools/virsh-completer-domain.h |  4 
  tools/virsh-domain.c   |  2 ++
  3 files changed, 49 insertions(+)

diff --git a/tools/virsh-completer-domain.c b/tools/virsh-completer-domain.c
index 99b36a2f9f..f56dc40125 100644
--- a/tools/virsh-completer-domain.c
+++ b/tools/virsh-completer-domain.c
@@ -872,3 +872,46 @@ virshKeycodeNameCompleter(vshControl *ctl,
  
  return g_steal_pointer(&tmp);

  }
+
+
+char **
+virshDomainFSMountpointsCompleter(vshControl *ctl,
+  const vshCmd *cmd,
+  unsigned int flags)
+{
+g_auto(GStrv) tmp = NULL;
+virDomainPtr dom = NULL;
+int rc = -1;
+size_t i;
+virDomainFSInfoPtr *info = NULL;
+size_t ninfos = 0;
+char **ret = NULL;
+
+virCheckFlags(0, NULL);
+
+if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
+return NULL;
+
+rc = virDomainGetFSInfo(dom, &info, 0);
+if (rc <= 0) {
+goto cleanup;
+}
+ninfos = rc;
+
+if (info) {


This feels redundant. @ninfos is > 0 at this point, thus @info must be 
!NULL. I know you took inspiration from cmdDomFSInfo() but the same 
argument applies there.



+tmp = g_new0(char *, ninfos + 1);
+for (i = 0; i < ninfos; i++) {
+tmp[i] = g_strdup(info[i]->mountpoint);
+}
+ret = g_steal_pointer(&tmp);
+}
+
+ cleanup:
+if (info) {
+for (i = 0; i < ninfos; i++)
+virDomainFSInfoFree(info[i]);
+VIR_FREE(info);
+}
+virshDomainFree(dom);
+return ret;
+}
diff --git a/tools/virsh-completer-domain.h b/tools/virsh-completer-domain.h
index 04a3705ff9..ef242d0c68 100644
--- a/tools/virsh-completer-domain.h
+++ b/tools/virsh-completer-domain.h
@@ -118,3 +118,7 @@ char ** virshCodesetNameCompleter(vshControl *ctl,
  char ** virshKeycodeNameCompleter(vshControl *ctl,
const vshCmd *cmd,
unsigned int flags);
+
+char ** virshDomainFSMountpointsCompleter(vshControl *ctl,
+  const vshCmd *cmd,
+  unsigned int flags);
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index fac590fbc6..9d315bdbcf 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -13920,6 +13920,7 @@ static const vshCmdOptDef opts_domfsfreeze[] = {
  VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE),
  {.name = "mountpoint",
   .type = VSH_OT_ARGV,
+ .completer = virshDomainFSMountpointsCompleter,
   .help = N_("mountpoint path to be frozen")
  },
  {.name = NULL}
@@ -13969,6 +13970,7 @@ static const vshCmdOptDef opts_domfsthaw[] = {
  VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE),
  {.name = "mountpoint",
   .type = VSH_OT_ARGV,
+ .completer = virshDomainFSMountpointsCompleter,
   .help = N_("mountpoint path to be thawed")
  },
  {.name = NULL}



Missed one more: opts_domfstrim.

Michal

[libvirt PATCH] vahDeinit: Fix memory leak

[Tim Wiederhake]

Calling VIR_FREE on a virDomainDef* does not free its various contained
pointers.

Signed-off-by: Tim Wiederhake 
---
 src/security/virt-aa-helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 68ac817f47..2331cc6648 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -79,7 +79,7 @@ vahDeinit(vahControl * ctl)
 if (ctl == NULL)
 return -1;
 
-VIR_FREE(ctl->def);
+virDomainDefFree(ctl->def);
 virObjectUnref(ctl->caps);
 virObjectUnref(ctl->xmlopt);
 VIR_FREE(ctl->files);
-- 
2.26.3

Re: [libvirt PATCH] vahDeinit: Fix memory leak

[Ján Tomko]

On a Thursday in 2021, Tim Wiederhake wrote:

Calling VIR_FREE on a virDomainDef* does not free its various contained
pointers.

Signed-off-by: Tim Wiederhake 
---
src/security/virt-aa-helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature

Re: [libvirt PATCH 00/10] Refactor more XML parsing boilerplate code, part IV

[Michal Privoznik]

On 4/22/21 12:32 PM, Tim Wiederhake wrote:

For background, see
https://listman.redhat.com/archives/libvir-list/2021-April/msg00668.html

Tim Wiederhake (10):
   virDomainDiskDef: Change type of cachemode to virDomainDiskCache
   virDomainDiskDef: Change type of error_policy to
 virDomainDiskErrorPolicy
   virDomainDiskDef: Change type of rerror_policy to
 virDomainDiskErrorPolicy
   virDomainDiskDef: Change type of iomode to virDomainDiskInfo
   virDomainDiskDef: Change type of discard to virDomainDiskDiscard
   virDomainDiskDef: Change type of detect_zeroes to
 virDomainDiskDetectZeroes
   virDomainDiskDefDriverParseXML: Use virXMLProp*
   domain_conf: Introduce function virDomainChrSouceModeTypeFromString
   domain_conf: Remove function virDomainChrSourceDefParseMode
   virDomainChrSourceDefParseTCP: Use virXMLProp*

  src/conf/domain_conf.c | 176 +
  src/conf/domain_conf.h |  12 +--
  2 files changed, 61 insertions(+), 127 deletions(-)



Reviewed-by: Michal Privoznik 

and pushed.

Michal

Re: [libvirt PATCH 2/3] docs: virtiofs: add section about externally-launched virtiofsd

[Jonathon Jongsma]

On Thu, 22 Apr 2021 13:40:53 +0200
Ján Tomko  wrote:

> Provide an exmple in a place more visible than formatdomain.html.
> 
> Signed-off-by: Ján Tomko 
> ---
>  docs/kbase/virtiofs.rst | 14 ++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/docs/kbase/virtiofs.rst b/docs/kbase/virtiofs.rst
> index c0bc07a68d..09674a4a76 100644
> --- a/docs/kbase/virtiofs.rst
> +++ b/docs/kbase/virtiofs.rst
> @@ -149,3 +149,17 @@ More optional elements can be specified
>  
>  
>
> +
> +Externally-launched virtiofsd
> +=
> +
> +Libvirtd can also connect the ``vhost-user-fs`` device to a
> ``virtiofsd`` +daemon launched outside of libvirtd. In that case
> socket permissions, 


Is there some text missing here? It seems to end abruptly.


> +
> +::
> +
> +  
> +
> +
> +  

Re: [PATCH] rpc: libssh2: Enable EC host keys

[Michal Privoznik]

On 3/28/21 11:49 PM, Neal Gompa wrote:

On Sun, Mar 28, 2021 at 5:10 PM Bastian Germann
 wrote:


libssh2 has ECDSA and ED25519 support beginning with v1.9.0. libvirt cannot
make use of those because it will handle them as unknown key types.

Add support for those host key types.

Signed-off-by: Bastian Germann 
---
  src/rpc/virnetsshsession.c | 16 +++-
  1 file changed, 15 insertions(+), 1 deletion(-)





LGTM.

Reviewed-by: Neal Gompa 




Pushed now. Congratulations on your first libvirt contribution!

Michal

[libvirt] [PATCH 0/2] virsh completer fixes

[Lin Ma]

Lin Ma (2):
  virsh: Add mountpoint completion to domfsfreeze/domfsthaw command
  virsh: Fix completion logic to guestvcpus command

 tools/virsh-completer-domain.c | 104 +++--
 tools/virsh-completer-domain.h |   4 ++
 tools/virsh-domain.c   |   2 +
 3 files changed, 93 insertions(+), 17 deletions(-)

-- 
2.26.2

[libvirt] [PATCH 1/2] virsh: Add mountpoint completion to domfsfreeze/domfsthaw command

[Lin Ma]

Signed-off-by: Lin Ma 
---
 tools/virsh-completer-domain.c | 43 ++
 tools/virsh-completer-domain.h |  4 
 tools/virsh-domain.c   |  2 ++
 3 files changed, 49 insertions(+)

diff --git a/tools/virsh-completer-domain.c b/tools/virsh-completer-domain.c
index 99b36a2f9f..f56dc40125 100644
--- a/tools/virsh-completer-domain.c
+++ b/tools/virsh-completer-domain.c
@@ -872,3 +872,46 @@ virshKeycodeNameCompleter(vshControl *ctl,
 
 return g_steal_pointer(&tmp);
 }
+
+
+char **
+virshDomainFSMountpointsCompleter(vshControl *ctl,
+  const vshCmd *cmd,
+  unsigned int flags)
+{
+g_auto(GStrv) tmp = NULL;
+virDomainPtr dom = NULL;
+int rc = -1;
+size_t i;
+virDomainFSInfoPtr *info = NULL;
+size_t ninfos = 0;
+char **ret = NULL;
+
+virCheckFlags(0, NULL);
+
+if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
+return NULL;
+
+rc = virDomainGetFSInfo(dom, &info, 0);
+if (rc <= 0) {
+goto cleanup;
+}
+ninfos = rc;
+
+if (info) {
+tmp = g_new0(char *, ninfos + 1);
+for (i = 0; i < ninfos; i++) {
+tmp[i] = g_strdup(info[i]->mountpoint);
+}
+ret = g_steal_pointer(&tmp);
+}
+
+ cleanup:
+if (info) {
+for (i = 0; i < ninfos; i++)
+virDomainFSInfoFree(info[i]);
+VIR_FREE(info);
+}
+virshDomainFree(dom);
+return ret;
+}
diff --git a/tools/virsh-completer-domain.h b/tools/virsh-completer-domain.h
index 04a3705ff9..ef242d0c68 100644
--- a/tools/virsh-completer-domain.h
+++ b/tools/virsh-completer-domain.h
@@ -118,3 +118,7 @@ char ** virshCodesetNameCompleter(vshControl *ctl,
 char ** virshKeycodeNameCompleter(vshControl *ctl,
   const vshCmd *cmd,
   unsigned int flags);
+
+char ** virshDomainFSMountpointsCompleter(vshControl *ctl,
+  const vshCmd *cmd,
+  unsigned int flags);
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index fac590fbc6..9d315bdbcf 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -13920,6 +13920,7 @@ static const vshCmdOptDef opts_domfsfreeze[] = {
 VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE),
 {.name = "mountpoint",
  .type = VSH_OT_ARGV,
+ .completer = virshDomainFSMountpointsCompleter,
  .help = N_("mountpoint path to be frozen")
 },
 {.name = NULL}
@@ -13969,6 +13970,7 @@ static const vshCmdOptDef opts_domfsthaw[] = {
 VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE),
 {.name = "mountpoint",
  .type = VSH_OT_ARGV,
+ .completer = virshDomainFSMountpointsCompleter,
  .help = N_("mountpoint path to be thawed")
 },
 {.name = NULL}
-- 
2.26.2

[libvirt] [PATCH 2/2] virsh: Fix completion logic to guestvcpus command

[Lin Ma]

In case of non-continuous vCPU topology, We can't infer the bitmap size
from the combination of onlineVcpuStr and nvcpus.
We should use virBitmapParseUnlimited here instead of virBitmapParse due
to the bitmap size is unknown.

e.g.:

  








  

 # virsh guestvcpus --domain VM
vcpus  : 0-5
online : 0-5
offlinable : 1-5

 # virsh setvcpu --domain VM --disable --vcpulist 2

 # virsh guestvcpus --domain VM --disable --cpulist 4,5

 # virsh guestvcpus --domain VM
vcpus  : 0-1,3-5
online : 0-1,3
offlinable : 1,3-5

Before:
 # virsh guestvcpus --domain VM --enable --cpulist 
2  4

After:
 # virsh guestvcpus --domain VM --enable --cpulist 
4  5

Signed-off-by: Lin Ma 
---
 tools/virsh-completer-domain.c | 61 --
 1 file changed, 44 insertions(+), 17 deletions(-)

diff --git a/tools/virsh-completer-domain.c b/tools/virsh-completer-domain.c
index f56dc40125..998aa5d646 100644
--- a/tools/virsh-completer-domain.c
+++ b/tools/virsh-completer-domain.c
@@ -624,36 +624,63 @@ virshDomainVcpulistViaAgentCompleter(vshControl *ctl,
 cpulist[i] = g_strdup_printf("%zu", i);
 } else {
 g_autofree char *onlineVcpuStr = NULL;
-g_autofree unsigned char *vcpumap = NULL;
-g_autoptr(virBitmap) vcpus = NULL;
-size_t offset = 0;
+g_autofree char *offlinableVcpuStr = NULL;
+g_autofree unsigned char *onlineVcpumap = NULL;
+g_autofree unsigned char *offlinableVcpumap = NULL;
+g_autoptr(virBitmap) onlineVcpus = NULL;
+g_autoptr(virBitmap) offlinableVcpus = NULL;
+size_t j = 0;
+int lastcpu;
 int dummy;
 
 if (virDomainGetGuestVcpus(dom, ¶ms, &nparams, 0) < 0)
 goto cleanup;
 
 onlineVcpuStr = vshGetTypedParamValue(ctl, ¶ms[1]);
-if (virBitmapParse(onlineVcpuStr, &vcpus, nvcpus) < 0)
+if (!(onlineVcpus = virBitmapParseUnlimited(onlineVcpuStr)))
 goto cleanup;
 
-if (virBitmapToData(vcpus, &vcpumap, &dummy) < 0)
+if (virBitmapToData(onlineVcpus, &onlineVcpumap, &dummy) < 0)
 goto cleanup;
 
 if (enable) {
-cpulist = g_new0(char *, nvcpus - virBitmapCountBits(vcpus) + 1);
-for (i = 0; i < nvcpus; i++) {
-if (VIR_CPU_USED(vcpumap, i) != 0)
-continue;
-
-cpulist[offset++] = g_strdup_printf("%zu", i);
+offlinableVcpuStr = vshGetTypedParamValue(ctl, ¶ms[2]);
+
+if (!(offlinableVcpus = 
virBitmapParseUnlimited(offlinableVcpuStr)))
+goto cleanup;
+
+if (virBitmapToData(offlinableVcpus, &offlinableVcpumap, &dummy) < 
0)
+goto cleanup;
+
+lastcpu = virBitmapLastSetBit(offlinableVcpus);
+cpulist = g_new0(char *, nvcpus - virBitmapCountBits(onlineVcpus) 
+ 1);
+for (i = 0; i < nvcpus - virBitmapCountBits(onlineVcpus); i++) {
+while (j <= lastcpu) {
+if (VIR_CPU_USED(onlineVcpumap, j) != 0 ||
+VIR_CPU_USED(offlinableVcpumap, j) == 0) {
+j += 1;
+continue;
+} else {
+break;
+}
+}
+
+cpulist[i] = g_strdup_printf("%zu", j++);
 }
 } else if (disable) {
-cpulist = g_new0(char *, virBitmapCountBits(vcpus) + 1);
-for (i = 0; i < nvcpus; i++) {
-if (VIR_CPU_USED(vcpumap, i) == 0)
-continue;
-
-cpulist[offset++] = g_strdup_printf("%zu", i);
+lastcpu = virBitmapLastSetBit(onlineVcpus);
+cpulist = g_new0(char *, virBitmapCountBits(onlineVcpus) + 1);
+for (i = 0; i < virBitmapCountBits(onlineVcpus); i++) {
+while (j <= lastcpu) {
+if (VIR_CPU_USED(onlineVcpumap, j) == 0) {
+j += 1;
+continue;
+} else {
+break;
+}
+}
+
+cpulist[i] = g_strdup_printf("%zu", j++);
 }
 }
 }
-- 
2.26.2