[libvirt][PATCH v3 1/3] docs: add docs for 'restrictive' option for mode in numatune

2021-01-05 Thread Luyao Zhong
When user would like use cgroups to restrict the allowed memory
nodes, and require not setting any specific memory policy, then
'restrictive' mode is useful.

Reviewed-by: Daniel Henrique Barboza 
Signed-off-by: Luyao Zhong 
---
 docs/formatdomain.rst | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 512939679b..3b6d71219a 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -1086,8 +1086,11 @@ NUMA Node Tuning
 ``memory``
The optional ``memory`` element specifies how to allocate memory for the
domain process on a NUMA host. It contains several optional attributes.
-   Attribute ``mode`` is either 'interleave', 'strict', or 'preferred', 
defaults
-   to 'strict'. Attribute ``nodeset`` specifies the NUMA nodes, using the same
+   Attribute ``mode`` is either 'interleave', 'strict', 'preferred' or
+   'restrictive', defaults to 'strict'. The value 'restrictive' specifies
+   using system default policy and only cgroups is used to restrict the
+   memory nodes, and it requires setting mode to 'restrictive' in ``memnode``
+   elements. Attribute ``nodeset`` specifies the NUMA nodes, using the same
syntax as attribute ``cpuset`` of element ``vcpu``. Attribute ``placement`` 
(
:since:`since 0.9.12` ) can be used to indicate the memory placement mode 
for
domain process, its value can be either "static" or "auto", defaults to
-- 
2.25.4



[libvirt][PATCH v3 3/3] qemu: add parser and formatter for 'restrictive' mode in numatune

2021-01-05 Thread Luyao Zhong
Reviewed-by: Daniel Henrique Barboza 
Signed-off-by: Luyao Zhong 
---
 include/libvirt/libvirt-domain.h  |  1 +
 src/conf/numa_conf.c  |  9 +
 src/qemu/qemu_command.c   |  6 ++-
 src/qemu/qemu_process.c   | 27 +
 src/util/virnuma.c|  3 ++
 .../numatune-memnode-invalid-mode.err |  1 +
 .../numatune-memnode-invalid-mode.xml | 33 +++
 ...emnode-restrictive-mode.x86_64-latest.args | 40 +++
 .../numatune-memnode-restrictive-mode.xml | 33 +++
 tests/qemuxml2argvtest.c  |  2 +
 ...memnode-restrictive-mode.x86_64-latest.xml | 40 +++
 tests/qemuxml2xmltest.c   |  1 +
 12 files changed, 195 insertions(+), 1 deletion(-)
 create mode 100644 tests/qemuxml2argvdata/numatune-memnode-invalid-mode.err
 create mode 100644 tests/qemuxml2argvdata/numatune-memnode-invalid-mode.xml
 create mode 100644 
tests/qemuxml2argvdata/numatune-memnode-restrictive-mode.x86_64-latest.args
 create mode 100644 tests/qemuxml2argvdata/numatune-memnode-restrictive-mode.xml
 create mode 100644 
tests/qemuxml2xmloutdata/numatune-memnode-restrictive-mode.x86_64-latest.xml

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index de2456812c..eabb3c091b 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -1527,6 +1527,7 @@ typedef enum {
 VIR_DOMAIN_NUMATUNE_MEM_STRICT  = 0,
 VIR_DOMAIN_NUMATUNE_MEM_PREFERRED   = 1,
 VIR_DOMAIN_NUMATUNE_MEM_INTERLEAVE  = 2,
+VIR_DOMAIN_NUMATUNE_MEM_RESTRICTIVE = 3,
 
 # ifdef VIR_ENUM_SENTINELS
 VIR_DOMAIN_NUMATUNE_MEM_LAST /* This constant is subject to change */
diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index f8a7a01ac9..df888a8dfb 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -43,6 +43,7 @@ VIR_ENUM_IMPL(virDomainNumatuneMemMode,
   "strict",
   "preferred",
   "interleave",
+  "restrictive",
 );
 
 VIR_ENUM_IMPL(virDomainNumatunePlacement,
@@ -234,6 +235,14 @@ virDomainNumatuneNodeParseXML(virDomainNumaPtr numa,
_("Invalid mode attribute in memnode element"));
 goto cleanup;
 }
+
+if (numa->memory.mode == VIR_DOMAIN_NUMATUNE_MEM_RESTRICTIVE &&
+mode != VIR_DOMAIN_NUMATUNE_MEM_RESTRICTIVE) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("'restrictive' mode is required in memnode 
element "
+ "when mode is 'restrictive' in memory 
element"));
+goto cleanup;
+}
 VIR_FREE(tmp);
 mem_node->mode = mode;
 }
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index b06a086e18..9bf2cc8ae8 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -174,6 +174,7 @@ VIR_ENUM_IMPL(qemuNumaPolicy,
   "bind",
   "preferred",
   "interleave",
+  "restricted",
 );
 
 
@@ -3159,7 +3160,10 @@ qemuBuildMemoryBackendProps(virJSONValuePtr 
*backendProps,
 return -1;
 }
 
-if (nodemask) {
+/* If mode is "restrictive", we should only use cgroups setting allowed 
memory
+ * nodes, and skip passing the host-nodes and policy parameters to QEMU 
command
+ * line which means we will use system default memory policy. */
+if (nodemask && mode != VIR_DOMAIN_NUMATUNE_MEM_RESTRICTIVE) {
 if (!virNumaNodesetIsAvailable(nodemask))
 return -1;
 if (virJSONValueObjectAdd(props,
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index e7421b415f..0080985dd7 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2713,6 +2713,7 @@ qemuProcessSetupPid(virDomainObjPtr vm,
 g_autoptr(virBitmap) hostcpumap = NULL;
 g_autofree char *mem_mask = NULL;
 int ret = -1;
+size_t i;
 
 if ((period || quota) &&
 !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) {
@@ -2753,6 +2754,32 @@ qemuProcessSetupPid(virDomainObjPtr vm,
 _mask, -1) < 0)
 goto cleanup;
 
+/* For vCPU threads, mem_mask is different among cells and mem_mask
+ * is used to set cgroups cpuset.mems for vcpu threads. If we specify
+ * 'restrictive' mode, that means we will set system default memory
+ * policy and only use cgroups to restrict allowed memory nodes. */
+if (nameval == VIR_CGROUP_THREAD_VCPU) {
+virDomainNumaPtr numatune = vm->def->numa;
+virBitmapPtr numanode_cpumask = NULL;
+for (i = 0; i < virDomainNumaGetNodeCount(numatune); i++) {
+numanode_cpumask = virDomainNumaGetNodeCpumask(numatune, i);
+/* 

[libvirt][PATCH v3 2/3] schema: add 'restrictive' config option for mode in numatune

2021-01-05 Thread Luyao Zhong
support 'restrictive' mode in memory element and memnode
element in numatune:
  
...

  
  
  

...
  

Reviewed-by: Daniel Henrique Barboza 
Signed-off-by: Luyao Zhong 
---
 docs/schemas/domaincommon.rng | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 17e25f14f2..aaec402658 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1097,6 +1097,7 @@
   strict
   preferred
   interleave
+  restrictive
 
   
 
@@ -1129,6 +1130,7 @@
 strict
 preferred
 interleave
+restrictive
   
 
 
-- 
2.25.4



[libvirt][PATCH v3 0/3] introduce 'restrictive' mode in numatune

2021-01-05 Thread Luyao Zhong
Before this patch set, numatune only has three memory modes:
static, interleave and prefered. These memory policies are
ultimately set by mbind() system call.

Memory policy could be 'hard coded' into the kernel, but none of
above policies fit our requirment under this case. mbind() support
default memory policy, but it requires a NULL nodemask. So obviously
setting allowed memory nodes is cgroups' mission under this case.
So we introduce a new option for mode in numatune named 'restrictive'.


   
   
   


The config above means we only use cgroups to restrict the allowed
memory nodes and not setting any specific memory policies explicitly.

RFC discussion:
https://www.redhat.com/archives/libvir-list/2020-November/msg01256.html

Regards,
Luyao

Luyao Zhong (3):
  docs: add docs for 'restrictive' option for mode in numatune
  schema: add 'restrictive' config option for mode in numatune
  qemu: add parser and formatter for 'restrictive' mode in numatune

 docs/formatdomain.rst |  7 +++-
 docs/schemas/domaincommon.rng |  2 +
 include/libvirt/libvirt-domain.h  |  1 +
 src/conf/numa_conf.c  |  9 +
 src/qemu/qemu_command.c   |  6 ++-
 src/qemu/qemu_process.c   | 27 +
 src/util/virnuma.c|  3 ++
 .../numatune-memnode-invalid-mode.err |  1 +
 .../numatune-memnode-invalid-mode.xml | 33 +++
 ...emnode-restrictive-mode.x86_64-latest.args | 40 +++
 .../numatune-memnode-restrictive-mode.xml | 33 +++
 tests/qemuxml2argvtest.c  |  2 +
 ...memnode-restrictive-mode.x86_64-latest.xml | 40 +++
 tests/qemuxml2xmltest.c   |  1 +
 14 files changed, 202 insertions(+), 3 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/numatune-memnode-invalid-mode.err
 create mode 100644 tests/qemuxml2argvdata/numatune-memnode-invalid-mode.xml
 create mode 100644 
tests/qemuxml2argvdata/numatune-memnode-restrictive-mode.x86_64-latest.args
 create mode 100644 tests/qemuxml2argvdata/numatune-memnode-restrictive-mode.xml
 create mode 100644 
tests/qemuxml2xmloutdata/numatune-memnode-restrictive-mode.x86_64-latest.xml

-- 
2.25.4



RE: [libvirt][PATCH v2 0/3] introduce 'restrictive' mode in numatune

2021-01-05 Thread Zhong, Luyao


Thanks Daniel, I forgot it. 

Regards,
Luyao

-Original Message-
From: Daniel Henrique Barboza  
Sent: Tuesday, January 5, 2021 8:29 PM
To: Zhong, Luyao ; libvir-list@redhat.com
Subject: Re: [libvirt][PATCH v2 0/3] introduce 'restrictive' mode in numatune

Luyao,

I failed to realize, back in the v1 version of your patches, that you didn't 
sign them with a "Signed-off-by" tag. This is required to assert that you agree 
with the terms of the Developer Certificate of Origin, described here:

https://developercertificate.org/

This is described in detail in this link:

https://libvirt.org/submitting-patches.html


If you agree with the DCO, please re-send the patches with your "Signed-off-by"
tag in the patches. You can do that by adding a "-s" tag in "git commit" when 
creating/amending the commit. My code review still stands, so feel free to keep 
my "Reviewed-by" tag in all of them.


I apologize for not bringing this up back in the v1 review.


Thanks,


DHB



On 1/3/21 7:39 AM, Luyao Zhong wrote:
> Before this patch set, numatune only has three memory modes:
> static, interleave and prefered. These memory policies are ultimately 
> set by mbind() system call.
> 
> Memory policy could be 'hard coded' into the kernel, but none of above 
> policies fit our requirment under this case. mbind() support default 
> memory policy, but it requires a NULL nodemask. So obviously setting 
> allowed memory nodes is cgroups' mission under this case.
> So we introduce a new option for mode in numatune named 'restrictive'.
> 
> 
> 
> 
>  
> 
> The config above means we only use cgroups to restrict the allowed 
> memory nodes and not setting any specific memory policies explicitly.
> 
> RFC discussion:
> https://www.redhat.com/archives/libvir-list/2020-November/msg01256.htm
> l
> 
> Regards,
> Luyao
> 
> Luyao Zhong (3):
>docs: add docs for 'restrictive' option for mode in numatune
>schema: add 'restrictive' config option for mode in numatune
>qemu: add parser and formatter for 'restrictive' mode in numatune
> 
>   docs/formatdomain.rst |  7 +++-
>   docs/schemas/domaincommon.rng |  2 +
>   include/libvirt/libvirt-domain.h  |  1 +
>   src/conf/numa_conf.c  |  9 +
>   src/qemu/qemu_command.c   |  6 ++-
>   src/qemu/qemu_process.c   | 27 +
>   src/util/virnuma.c|  3 ++
>   .../numatune-memnode-invalid-mode.err |  1 +
>   .../numatune-memnode-invalid-mode.xml | 33 +++
>   ...emnode-restrictive-mode.x86_64-latest.args | 40 +++
>   .../numatune-memnode-restrictive-mode.xml | 33 +++
>   tests/qemuxml2argvtest.c  |  2 +
>   ...memnode-restrictive-mode.x86_64-latest.xml | 40 +++
>   tests/qemuxml2xmltest.c   |  1 +
>   14 files changed, 202 insertions(+), 3 deletions(-)
>   create mode 100644 tests/qemuxml2argvdata/numatune-memnode-invalid-mode.err
>   create mode 100644 tests/qemuxml2argvdata/numatune-memnode-invalid-mode.xml
>   create mode 100644 
> tests/qemuxml2argvdata/numatune-memnode-restrictive-mode.x86_64-latest.args
>   create mode 100644 
> tests/qemuxml2argvdata/numatune-memnode-restrictive-mode.xml
>   create mode 100644 
> tests/qemuxml2xmloutdata/numatune-memnode-restrictive-mode.x86_64-late
> st.xml
> 



Re: [libvirt PATCH v3 21/21] libvirt-nodedev.h: remove space-padded alignment

2021-01-05 Thread Erik Skultety
On Thu, Dec 24, 2020 at 08:14:45AM -0600, Jonathon Jongsma wrote:
> Most of the header files no longer use the space-padded function name
> and parameter alignment. Update the style to be more consistent with
> other headers.

It's actually half of the public ones (9 out of 19 including this one) that are
inconsistent, so I would suggest dropping this patch from this series and
posting a standalone series to fix the inconsistencies in every header affected
header (public or private).

Erik



[PATCHv2 3/4] netlink: Introduce macro NETLINK_MSG_APPEND to wrap nlmsg_append

2021-01-05 Thread Shi Lei
Introduce a macro NETLINK_MSG_APPEND to wrap nlmsg_append and
simplify code. Remove those labels 'buffer_too_small', since they
are now useless.

Signed-off-by: Shi Lei 
---
 src/util/virnetlink.c | 42 +-
 1 file changed, 13 insertions(+), 29 deletions(-)

diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
index 0b55b124..fdcb0dc0 100644
--- a/src/util/virnetlink.c
+++ b/src/util/virnetlink.c
@@ -72,6 +72,15 @@ do { \
 } \
 } while (0)
 
+# define NETLINK_MSG_APPEND(msg, datalen, dataptr) \
+do { \
+if (nlmsg_append(msg, dataptr, datalen, NLMSG_ALIGNTO) < 0) { \
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s", \
+   _("allocated netlink buffer is too small")); \
+return -1; \
+} \
+} while (0)
+
 
 /* State for a single netlink event handle */
 struct virNetlinkEventHandle {
@@ -432,8 +441,7 @@ virNetlinkDumpLink(const char *ifname, int ifindex,
 return -1;
 }
 
-if (nlmsg_append(nl_msg,  , sizeof(ifinfo), NLMSG_ALIGNTO) < 0)
-goto buffer_too_small;
+NETLINK_MSG_APPEND(nl_msg, sizeof(ifinfo), );
 
 if (ifname)
 NETLINK_MSG_PUT(nl_msg, IFLA_IFNAME, (strlen(ifname) + 1), ifname);
@@ -491,11 +499,6 @@ virNetlinkDumpLink(const char *ifname, int ifindex,
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("malformed netlink response message"));
 return rc;
-
- buffer_too_small:
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("allocated netlink buffer is too small"));
-return rc;
 }
 
 
@@ -545,8 +548,7 @@ virNetlinkNewLink(const char *ifname,
 return -1;
 }
 
-if (nlmsg_append(nl_msg,  , sizeof(ifinfo), NLMSG_ALIGNTO) < 0)
-goto buffer_too_small;
+NETLINK_MSG_APPEND(nl_msg, sizeof(ifinfo), );
 
 NETLINK_MSG_PUT(nl_msg, IFLA_IFNAME, (strlen(ifname) + 1), ifname);
 
@@ -620,11 +622,6 @@ virNetlinkNewLink(const char *ifname,
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("malformed netlink response message"));
 return -1;
-
- buffer_too_small:
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("allocated netlink buffer is too small"));
-return -1;
 }
 
 
@@ -658,8 +655,7 @@ virNetlinkDelLink(const char *ifname, 
virNetlinkDelLinkFallback fallback)
 return -1;
 }
 
-if (nlmsg_append(nl_msg,  , sizeof(ifinfo), NLMSG_ALIGNTO) < 0)
-goto buffer_too_small;
+NETLINK_MSG_APPEND(nl_msg, sizeof(ifinfo), );
 
 NETLINK_MSG_PUT(nl_msg, IFLA_IFNAME, (strlen(ifname) + 1), ifname);
 
@@ -701,11 +697,6 @@ virNetlinkDelLink(const char *ifname, 
virNetlinkDelLinkFallback fallback)
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("malformed netlink response message"));
 return -1;
-
- buffer_too_small:
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("allocated netlink buffer is too small"));
-return -1;
 }
 
 /**
@@ -740,9 +731,7 @@ virNetlinkGetNeighbor(void **nlData, uint32_t src_pid, 
uint32_t dst_pid)
 return -1;
 }
 
-if (nlmsg_append(nl_msg, , sizeof(ndinfo), NLMSG_ALIGNTO) < 0)
-goto buffer_too_small;
-
+NETLINK_MSG_APPEND(nl_msg, sizeof(ndinfo), );
 
 if (virNetlinkCommand(nl_msg, , ,
   src_pid, dst_pid, NETLINK_ROUTE, 0) < 0)
@@ -778,11 +767,6 @@ virNetlinkGetNeighbor(void **nlData, uint32_t src_pid, 
uint32_t dst_pid)
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("malformed netlink response message"));
 return -1;
-
- buffer_too_small:
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("allocated netlink buffer is too small"));
-return -1;
 }
 
 int
-- 
2.25.1




[PATCHv2 4/4] netlink: Introduce a helper function to simplify netlink functions

2021-01-05 Thread Shi Lei
Extract common code as helper function virNetlinkTalk, then simplify
the functions virNetlink[DumpLink|NewLink|DelLink|GetNeighbor].

Signed-off-by: Shi Lei 
---
 src/util/virnetlink.c | 225 +-
 src/util/virnetlink.h |   4 +-
 2 files changed, 94 insertions(+), 135 deletions(-)

diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
index fdcb0dc0..2936a3ef 100644
--- a/src/util/virnetlink.c
+++ b/src/util/virnetlink.c
@@ -353,6 +353,52 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
 return 0;
 }
 
+
+static int
+virNetlinkTalk(const char *ifname,
+   virNetlinkMsg *nl_msg,
+   uint32_t src_pid,
+   uint32_t dst_pid,
+   struct nlmsghdr **resp,
+   unsigned int *resp_len,
+   int *error,
+   virNetlinkTalkFallback fallback)
+{
+if (virNetlinkCommand(nl_msg, resp, resp_len,
+  src_pid, dst_pid, NETLINK_ROUTE, 0) < 0)
+return -1;
+
+if (*resp_len < NLMSG_LENGTH(0) || resp == NULL)
+goto malformed_resp;
+
+if ((*resp)->nlmsg_type == NLMSG_ERROR) {
+struct nlmsgerr *err = (struct nlmsgerr *) NLMSG_DATA(resp);
+if ((*resp)->nlmsg_len < NLMSG_LENGTH(sizeof(*err)))
+goto malformed_resp;
+
+if (-err->error == EOPNOTSUPP && fallback)
+return fallback(ifname);
+
+if (err->error < 0) {
+if (error)
+*error = err->error;
+else
+virReportSystemError(-err->error,
+ "%s", _("netlink error"));
+
+return -1;
+}
+}
+
+return 0;
+
+ malformed_resp:
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("malformed netlink response message"));
+return -1;
+}
+
+
 int
 virNetlinkDumpCommand(struct nl_msg *nl_msg,
   virNetlinkDumpCallback callback,
@@ -396,6 +442,7 @@ virNetlinkDumpCommand(struct nl_msg *nl_msg,
 return 0;
 }
 
+
 /**
  * virNetlinkDumpLink:
  *
@@ -420,15 +467,14 @@ virNetlinkDumpLink(const char *ifname, int ifindex,
void **nlData, struct nlattr **tb,
uint32_t src_pid, uint32_t dst_pid)
 {
-int rc = -1;
-struct nlmsgerr *err;
 struct ifinfomsg ifinfo = {
 .ifi_family = AF_UNSPEC,
 .ifi_index  = ifindex
 };
-unsigned int recvbuflen;
 g_autoptr(virNetlinkMsg) nl_msg = NULL;
 g_autofree struct nlmsghdr *resp = NULL;
+unsigned int resp_len = 0;
+int error = 0;
 
 if (ifname && ifindex <= 0 && virNetDevGetIndex(ifname, ) < 0)
 return -1;
@@ -459,46 +505,23 @@ virNetlinkDumpLink(const char *ifname, int ifindex,
 }
 # endif
 
-if (virNetlinkCommand(nl_msg, , ,
-  src_pid, dst_pid, NETLINK_ROUTE, 0) < 0)
+if (virNetlinkTalk(ifname, nl_msg, src_pid, dst_pid,
+   , _len, , NULL) < 0) {
+virReportSystemError(error,
+ _("error dumping %s (%d) interface"),
+ ifname, ifindex);
 return -1;
+}
 
-if (recvbuflen < NLMSG_LENGTH(0) || resp == NULL)
-goto malformed_resp;
-
-switch (resp->nlmsg_type) {
-case NLMSG_ERROR:
-err = (struct nlmsgerr *)NLMSG_DATA(resp);
-if (resp->nlmsg_len < NLMSG_LENGTH(sizeof(*err)))
-goto malformed_resp;
-
-if (err->error) {
-virReportSystemError(-err->error,
- _("error dumping %s (%d) interface"),
- ifname, ifindex);
-return -1;
-}
-break;
-
-case GENL_ID_CTRL:
-case NLMSG_DONE:
-rc = nlmsg_parse(resp, sizeof(struct ifinfomsg),
- tb, IFLA_MAX, NULL);
-if (rc < 0)
-goto malformed_resp;
-break;
-
-default:
-goto malformed_resp;
+if ((resp->nlmsg_type != GENL_ID_CTRL && resp->nlmsg_type != NLMSG_DONE) ||
+nlmsg_parse(resp, sizeof(struct ifinfomsg), tb, IFLA_MAX, NULL) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("malformed netlink response message"));
+return -1;
 }
 
 *nlData = g_steal_pointer();
 return 0;
-
- malformed_resp:
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("malformed netlink response message"));
-return rc;
 }
 
 
@@ -523,13 +546,12 @@ virNetlinkNewLink(const char *ifname,
   virNetlinkNewLinkDataPtr extra_args,
   int *error)
 {
-struct nlmsgerr *err;
 struct nlattr *linkinfo = NULL;
 struct nlattr *infodata = NULL;
-unsigned int buflen;
 struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC };
 g_autoptr(virNetlinkMsg) nl_msg = NULL;
 g_autofree struct nlmsghdr *resp = NULL;
+unsigned int resp_len = 0;
 
 *error = 0;
 
@@ -591,37 +613,17 @@ 

[PATCHv2 0/4] netlink: Extract common code to simplify netlink functions

2021-01-05 Thread Shi Lei
V1 here: https://www.redhat.com/archives/libvir-list/2020-December/msg00836.html

Since V1:
- Minor fixes for reporting system error in Patch 4.

Shi Lei (4):
  netlink: Remove invalid flags(NLM_F_CREATE and NLM_F_EXCL) for RTM_DELLINK
  netlink: Minor changes for macros NETLINK_MSG_[NEST_START|NEST_END|PUT]
  netlink: Introduce macro NETLINK_MSG_APPEND to wrap nlmsg_append
  netlink: Introduce a helper function to simplify netlink functions

 src/util/virnetlink.c | 314 +++---
 src/util/virnetlink.h |  27 +---
 2 files changed, 142 insertions(+), 199 deletions(-)

-- 
2.25.1




[PATCHv2 1/4] netlink: Remove invalid flags(NLM_F_CREATE and NLM_F_EXCL) for RTM_DELLINK

2021-01-05 Thread Shi Lei
NLM_F_CREATE and NLM_F_EXCL are invalid for RTM_DELLINK,
so remove them.

Signed-off-by: Shi Lei 
---
 src/util/virnetlink.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
index ca735bb8..17e6eeb9 100644
--- a/src/util/virnetlink.c
+++ b/src/util/virnetlink.c
@@ -627,8 +627,7 @@ virNetlinkDelLink(const char *ifname, 
virNetlinkDelLinkFallback fallback)
 g_autoptr(virNetlinkMsg) nl_msg = NULL;
 g_autofree struct nlmsghdr *resp = NULL;
 
-nl_msg = nlmsg_alloc_simple(RTM_DELLINK,
-NLM_F_REQUEST | NLM_F_CREATE | NLM_F_EXCL);
+nl_msg = nlmsg_alloc_simple(RTM_DELLINK, NLM_F_REQUEST);
 if (!nl_msg) {
 virReportOOMError();
 return -1;
-- 
2.25.1




[PATCHv2 2/4] netlink: Minor changes for macros NETLINK_MSG_[NEST_START|NEST_END|PUT]

2021-01-05 Thread Shi Lei
Move macros NETLINK_MSG_[NEST_START|NEST_END|PUT] from .h into .c;
within these macros, replace 'goto' with reporting error and returning;
simplify virNetlinkDumpLink and virNetlinkDelLink by using NETLINK_MSG_PUT.

Signed-off-by: Shi Lei 
---
 src/util/virnetlink.c | 44 +--
 src/util/virnetlink.h | 23 --
 2 files changed, 34 insertions(+), 33 deletions(-)

diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
index 17e6eeb9..0b55b124 100644
--- a/src/util/virnetlink.c
+++ b/src/util/virnetlink.c
@@ -44,6 +44,35 @@ VIR_LOG_INIT("util.netlink");
 
 # include 
 
+# define NETLINK_MSG_NEST_START(msg, container, attrtype) \
+do { \
+container = nla_nest_start(msg, attrtype); \
+if (!container) { \
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s", \
+   _("allocated netlink buffer is too small")); \
+return -1; \
+} \
+} while (0)
+
+# define NETLINK_MSG_NEST_END(msg, container) \
+do { nla_nest_end(msg, container); } while (0)
+
+/*
+ * we need to use an intermediary pointer to @data as compilers may sometimes
+ * complain about @data not being a pointer type:
+ * error: the address of 'foo' will always evaluate as 'true' [-Werror=address]
+ */
+# define NETLINK_MSG_PUT(msg, attrtype, datalen, data) \
+do { \
+const void *dataptr = data; \
+if (dataptr && nla_put(msg, attrtype, datalen, dataptr) < 0) { \
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s", \
+   _("allocated netlink buffer is too small")); \
+return -1; \
+} \
+} while (0)
+
+
 /* State for a single netlink event handle */
 struct virNetlinkEventHandle {
 int watch;
@@ -406,10 +435,8 @@ virNetlinkDumpLink(const char *ifname, int ifindex,
 if (nlmsg_append(nl_msg,  , sizeof(ifinfo), NLMSG_ALIGNTO) < 0)
 goto buffer_too_small;
 
-if (ifname) {
-if (nla_put(nl_msg, IFLA_IFNAME, strlen(ifname)+1, ifname) < 0)
-goto buffer_too_small;
-}
+if (ifname)
+NETLINK_MSG_PUT(nl_msg, IFLA_IFNAME, (strlen(ifname) + 1), ifname);
 
 # ifdef RTEXT_FILTER_VF
 /* if this filter exists in the kernel's netlink implementation,
@@ -419,10 +446,8 @@ virNetlinkDumpLink(const char *ifname, int ifindex,
 {
 uint32_t ifla_ext_mask = RTEXT_FILTER_VF;
 
-if (nla_put(nl_msg, IFLA_EXT_MASK,
-sizeof(ifla_ext_mask), _ext_mask) < 0) {
-goto buffer_too_small;
-}
+NETLINK_MSG_PUT(nl_msg, IFLA_EXT_MASK,
+sizeof(ifla_ext_mask), _ext_mask);
 }
 # endif
 
@@ -636,8 +661,7 @@ virNetlinkDelLink(const char *ifname, 
virNetlinkDelLinkFallback fallback)
 if (nlmsg_append(nl_msg,  , sizeof(ifinfo), NLMSG_ALIGNTO) < 0)
 goto buffer_too_small;
 
-if (nla_put(nl_msg, IFLA_IFNAME, strlen(ifname)+1, ifname) < 0)
-goto buffer_too_small;
+NETLINK_MSG_PUT(nl_msg, IFLA_IFNAME, (strlen(ifname) + 1), ifname);
 
 if (virNetlinkCommand(nl_msg, , , 0, 0,
   NETLINK_ROUTE, 0) < 0) {
diff --git a/src/util/virnetlink.h b/src/util/virnetlink.h
index 7c4ed202..966d6db3 100644
--- a/src/util/virnetlink.h
+++ b/src/util/virnetlink.h
@@ -38,29 +38,6 @@ struct nlmsghdr;
 
 #endif /* WITH_LIBNL */
 
-#define NETLINK_MSG_NEST_START(msg, container, attrtype) \
-do { \
-container = nla_nest_start(msg, attrtype); \
-if (!container) \
-goto buffer_too_small; \
-} while(0)
-
-#define NETLINK_MSG_NEST_END(msg, container) \
-do { nla_nest_end(msg, container); } while(0)
-
-/*
- * we need to use an intermediary pointer to @data as compilers may sometimes
- * complain about @data not being a pointer type:
- * error: the address of 'foo' will always evaluate as 'true' [-Werror=address]
- */
-#define NETLINK_MSG_PUT(msg, attrtype, datalen, data) \
-do { \
-const void *dataptr = data; \
-if (dataptr && nla_put(msg, attrtype, datalen, dataptr) < 0) \
-goto buffer_too_small; \
-} while(0)
-
-
 int virNetlinkStartup(void);
 void virNetlinkShutdown(void);
 
-- 
2.25.1




Re: Re: [PATCH 4/4] netlink: Introduce a helper function to simplify netlink functions

2021-01-05 Thread Shi Lei
On 2021-01-06 at 00:00, Michal Privoznik wrote:
>On 12/21/20 4:23 AM, Shi Lei wrote:
>> Extract common code as helper function virNetlinkTalk, then simplify
>> the functions virNetlink[DumpLink|NewLink|DelLink|GetNeighbor].
>>
>> Signed-off-by: Shi Lei 
>> ---
>>   src/util/virnetlink.c | 210 +++---
>>   src/util/virnetlink.h |   4 +-
>>   2 files changed, 78 insertions(+), 136 deletions(-)
>
>Nice cleanup. Patches 1-3 look good. 

Okay.

>
>>
>> diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
>> index fdcb0dc0..7bea38c0 100644
>> --- a/src/util/virnetlink.c
>> +++ b/src/util/virnetlink.c
>> @@ -353,6 +353,48 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
>>   return 0;
>>   }
>>  
>> +
>> +static int
>> +virNetlinkTalk(const char *ifname,
>> +   virNetlinkMsg *nl_msg,
>> +   uint32_t src_pid,
>> +   uint32_t dst_pid,
>> +   struct nlmsghdr **resp,
>> +   unsigned int *resp_len,
>> +   int *error,
>> +   virNetlinkTalkFallback fallback)
>> +{
>> +    if (virNetlinkCommand(nl_msg, resp, resp_len,
>> +  src_pid, dst_pid, NETLINK_ROUTE, 0) < 0)
>> +    return -1;
>> +
>> +    if (*resp_len < NLMSG_LENGTH(0) || resp == NULL)
>> +    goto malformed_resp;
>> +
>> +    if ((*resp)->nlmsg_type == NLMSG_ERROR) {
>> +    struct nlmsgerr *err = (struct nlmsgerr *) NLMSG_DATA(resp);
>> +    if ((*resp)->nlmsg_len < NLMSG_LENGTH(sizeof(*err)))
>> +    goto malformed_resp;
>> +
>> +    if (-err->error == EOPNOTSUPP && fallback)
>> +    return fallback(ifname);
>> +
>> +    if (err->error < 0) {
>> +    if (error)
>> +    *error = err->error;
>
>I wonder whether we should report an error here. I mean, if err->error
>is set (and not EOPNOTSUPP) then it's stored into *error, good. But -1
>is returned ...
>
>> +    return -1;
>
>.. here and it's indistinguishable to the caller from -1 returned in
>'malformed_resp'. Looking at the usage in next hunks, how about:
>
>   if (error)
> *error = err->error;
>   else
> virReportSystemError(-err->error, ...);
>
>   return -1;
> 

Okay.

>> +    }
>> +    }
>> +
>> +    return 0;
>> +
>> + malformed_resp:
>> +    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +   _("malformed netlink response message"));
>> +    return -1;
>> +}
>> +
>> +
>>   int
>>   virNetlinkDumpCommand(struct nl_msg *nl_msg,
>> virNetlinkDumpCallback callback,
>> @@ -396,6 +438,7 @@ virNetlinkDumpCommand(struct nl_msg *nl_msg,
>>   return 0;
>>   }
>>  
>> +
>>   /**
>>    * virNetlinkDumpLink:
>>    *
>> @@ -420,15 +463,13 @@ virNetlinkDumpLink(const char *ifname, int ifindex,
>>  void **nlData, struct nlattr **tb,
>>  uint32_t src_pid, uint32_t dst_pid)
>>   {
>> -    int rc = -1;
>> -    struct nlmsgerr *err;
>>   struct ifinfomsg ifinfo = {
>>   .ifi_family = AF_UNSPEC,
>>   .ifi_index  = ifindex
>>   };
>> -    unsigned int recvbuflen;
>>   g_autoptr(virNetlinkMsg) nl_msg = NULL;
>>   g_autofree struct nlmsghdr *resp = NULL;
>> +    unsigned int resp_len = 0;
>>  
>>   if (ifname && ifindex <= 0 && virNetDevGetIndex(ifname, ) < 0)
>>   return -1;
>> @@ -459,46 +500,19 @@ virNetlinkDumpLink(const char *ifname, int ifindex,
>>   }
>>   # endif
>>  
>> -    if (virNetlinkCommand(nl_msg, , ,
>> -  src_pid, dst_pid, NETLINK_ROUTE, 0) < 0)
>> +    if (virNetlinkTalk(ifname, nl_msg, src_pid, dst_pid,
>> +   , _len, NULL, NULL) < 0)
>>   return -1;
>>  
>> -    if (recvbuflen < NLMSG_LENGTH(0) || resp == NULL)
>> -    goto malformed_resp;
>> -
>> -    switch (resp->nlmsg_type) {
>> -    case NLMSG_ERROR:
>> -    err = (struct nlmsgerr *)NLMSG_DATA(resp);
>> -    if (resp->nlmsg_len < NLMSG_LENGTH(sizeof(*err)))
>> -    goto malformed_resp;
>> -
>> -    if (err->error) {
>> -    virReportSystemError(-err->error,
>> - _("error dumping %s (%d) interface"),
>> - ifname, ifindex);
>
>
>Here we'd report an error. 

Okay.

>
>> -    return -1;
>> -    }
>> -    break;
>> -
>> -    case GENL_ID_CTRL:
>> -    case NLMSG_DONE:
>> -    rc = nlmsg_parse(resp, sizeof(struct ifinfomsg),
>> - tb, IFLA_MAX, NULL);
>> -    if (rc < 0)
>> -    goto malformed_resp;
>> -    break;
>> -
>> -    default:
>> -    goto malformed_resp;
>> +    if ((resp->nlmsg_type != GENL_ID_CTRL && resp->nlmsg_type != 
>> NLMSG_DONE) ||
>> +    nlmsg_parse(resp, sizeof(struct ifinfomsg), tb, IFLA_MAX, NULL) < 
>> 0) {
>> +    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +   _("malformed netlink response message"));
>> +    return -1;
>>   }

Re: [libvirt PATCH v3 0/8] vmx: Don't error out on missing filename for cdrom

2021-01-05 Thread Martin Kletzander

On Tue, Jan 05, 2021 at 07:19:08PM +0100, Michal Privoznik wrote:

On 1/5/21 4:54 PM, Martin Kletzander wrote:

This is perfectly valid in VMWare and the VM just boots with an empty drive.  We
used to just skip the whole drive before, but since we changed how we parse
empty cdrom drives this now results in an error and the user not being able to
even dump the XML.  Instead of erroring out, just keep the drive empty.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1903953

v3:
  - Fixed the vmware driver
  - Bit of a clean-up
  - Few more tests

v2:
  - Do not report and reset an error, but handle it more nicely.
  - https://www.redhat.com/archives/libvir-list/2020-December/msg00846.html

v1:
  - https://www.redhat.com/archives/libvir-list/2020-December/msg00840.html

Martin Kletzander (8):
   esx: Unindent unnecessary conditional branch
   tests: Use g_autofree in testParseVMXFileName
   vmx: Make virVMXParseFileName return an integer
   tests: Allow testing for parse failures in vmx2xmltest
   vmx: Allow missing cdrom image file in virVMXParseFileName
   tests: Test vmx files with missing images
   esx: Handle missing images in esxParseVMXFileName
   vmx: Treat missing cdrom-image as empty drive

  src/esx/esx_driver.c  | 160 ++
  src/vmware/vmware_conf.c  |  21 ++-
  src/vmware/vmware_conf.h  |  10 +-
  src/vmware/vmware_driver.c|   6 +-
  src/vmx/vmx.c |  27 +--
  src/vmx/vmx.h |   5 +-
  ...x2xml-cdrom-ide-file-missing-datastore.vmx |   6 +
  .../vmx2xml-cdrom-ide-file-missing-file.vmx   |   6 +
  ...ml-harddisk-ide-file-missing-datastore.vmx |   6 +
  ...mx2xml-harddisk-scsi-file-missing-file.vmx |   7 +
  tests/vmx2xmltest.c   |  67 +---
  11 files changed, 203 insertions(+), 118 deletions(-)
  create mode 100644 
tests/vmx2xmldata/vmx2xml-cdrom-ide-file-missing-datastore.vmx
  create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-file-missing-file.vmx
  create mode 100644 
tests/vmx2xmldata/vmx2xml-harddisk-ide-file-missing-datastore.vmx
  create mode 100644 
tests/vmx2xmldata/vmx2xml-harddisk-scsi-file-missing-file.vmx



Reviewed-by: Michal Privoznik 



Thanks, although I *again* forgot to add your R-b to the patches...  I wish
gitlab took care of that.  And I even have a function and a command for it:

function rb --description 'Add Reviewed-by: to current commit' --argument who
set git_cmd (command -s git)
if ! test $git_cmd
begin
set_color red
echo -n "error: "
set_color normal
echo "Command `git` not available"
end >&2
return 1
end
if ! test $who
begin
set_color red
echo -n "error: "
set_color normal
echo "Missing parameter `who`"
end >&2
return 1
end
env VISUAL='git interpret-trailers --in-place --trailer 
"Reviewed-by='$who'"' $git_cmd commit --amend
end

Anyway, thanks, and sorry O:-)


Michal


signature.asc
Description: PGP signature


[libvirt PATCH] qemu: The TSC tolerance interval should be closed

2021-01-05 Thread Jiri Denemark
The kernel refuses to set guest TSC frequency less than a minimum
frequency or greater than maximum frequency (both computed based on the
host TSC frequency). When writing the libvirt code with a reversed logic
(return success when the requested frequency falls within the tolerance
interval) I forgot to include the boundaries.

Fixes: d8e5b4560006590668d4669f54a46b08ec14c1a2
https://bugzilla.redhat.com/show_bug.cgi?id=1839095

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_process.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index e7421b415f..e9802809a5 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5403,7 +5403,7 @@ qemuProcessStartValidateTSC(virQEMUDriverPtr driver,
   tsc->frequency, virTristateBoolTypeToString(tsc->scaling),
   tolerance);
 
-if (freq > minFreq && freq < maxFreq) {
+if (freq >= minFreq && freq <= maxFreq) {
 VIR_DEBUG("Requested TSC frequency is within tolerance interval");
 return 0;
 }
-- 
2.30.0



Plans for the next release

2021-01-05 Thread Jiri Denemark
We are getting close to the first release of libvirt in 2021. To aim for
the release on Jan 15 I suggest entering the freeze on Friday Jan 08 in
the evening or during the weekend and tagging RC2 on Wednesday Jan 13.

I hope this works for everyone.

Jirka



[RFC] exposing 'nodedev assigned to domain' info to users

2021-01-05 Thread Daniel Henrique Barboza

Hi,

This is something I've been giving a thought after working in Gitlab issue
#72 and decided to run through the ML before hitting the code.

We don't have an easy way to retrieve the domain that is using an specific
hostdev.  Let's say that I want to know which domain is using the PCI card
pci__01_00_2. 'nodedev-dumpxml' will return the hardware/driver capabilities
of the device, such as IOMMU group, driver and so on, but will not inform
which domain is using the hostdev, if any. 'nodedev-list' will simply list
all nodedev names known to Libvirt, without outputting any other information.

IIUC, the only existing way I can reliably tell whether a hostdev is being
used by domain, aside from having to register the information by myself
during domain definition of course, is via 'virsh dumpxml ' each
existing running domain and matching the nodedev name with the source.address
element of the XML.

When we consider SR-IOV devices that can have 28+ VFs each (and have lots of
fun caveats, like Github #72 showed us), the capability of hot plug/unplug
hostdevs freely, and lots of running domains, it is clear that we're putting a
considerable pressure in the upper layers (OVirt, or a poor human admin) to
keep track of the nodedevs each running domain is using. An info that we
already have internally and can just expose it.

I have a few ideas to make this happen:

1 - upgrade 'nodedev-list' to add an extra 'assigned to' column

This is the more straightforward way of exposing the info. A simple 
'nodedev-list'
call can retrieve which domain is using which nodedev. To preserve the existing
usage we can add an "--show-assigned-domains" option to control whether we
will display this info.


2 - add an '' element in nodedev XML definition

I'm not a fan of exposing this in this particular XML because we would mix
host/hw related attributes with domain info. But it would be easier to pull
this off comparing to (1), so I'm mentioning it for the record.


I would start by exposing the info for HOSTDEV_SUBSYS_TYPE_PCI hostdevs
(--cap pci in nodedev-list).


Thanks,


DHB












Re: [libvirt PATCH v3 0/8] vmx: Don't error out on missing filename for cdrom

2021-01-05 Thread Michal Privoznik

On 1/5/21 4:54 PM, Martin Kletzander wrote:

This is perfectly valid in VMWare and the VM just boots with an empty drive.  We
used to just skip the whole drive before, but since we changed how we parse
empty cdrom drives this now results in an error and the user not being able to
even dump the XML.  Instead of erroring out, just keep the drive empty.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1903953

v3:
  - Fixed the vmware driver
  - Bit of a clean-up
  - Few more tests

v2:
  - Do not report and reset an error, but handle it more nicely.
  - https://www.redhat.com/archives/libvir-list/2020-December/msg00846.html

v1:
  - https://www.redhat.com/archives/libvir-list/2020-December/msg00840.html

Martin Kletzander (8):
   esx: Unindent unnecessary conditional branch
   tests: Use g_autofree in testParseVMXFileName
   vmx: Make virVMXParseFileName return an integer
   tests: Allow testing for parse failures in vmx2xmltest
   vmx: Allow missing cdrom image file in virVMXParseFileName
   tests: Test vmx files with missing images
   esx: Handle missing images in esxParseVMXFileName
   vmx: Treat missing cdrom-image as empty drive

  src/esx/esx_driver.c  | 160 ++
  src/vmware/vmware_conf.c  |  21 ++-
  src/vmware/vmware_conf.h  |  10 +-
  src/vmware/vmware_driver.c|   6 +-
  src/vmx/vmx.c |  27 +--
  src/vmx/vmx.h |   5 +-
  ...x2xml-cdrom-ide-file-missing-datastore.vmx |   6 +
  .../vmx2xml-cdrom-ide-file-missing-file.vmx   |   6 +
  ...ml-harddisk-ide-file-missing-datastore.vmx |   6 +
  ...mx2xml-harddisk-scsi-file-missing-file.vmx |   7 +
  tests/vmx2xmltest.c   |  67 +---
  11 files changed, 203 insertions(+), 118 deletions(-)
  create mode 100644 
tests/vmx2xmldata/vmx2xml-cdrom-ide-file-missing-datastore.vmx
  create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-file-missing-file.vmx
  create mode 100644 
tests/vmx2xmldata/vmx2xml-harddisk-ide-file-missing-datastore.vmx
  create mode 100644 
tests/vmx2xmldata/vmx2xml-harddisk-scsi-file-missing-file.vmx



Reviewed-by: Michal Privoznik 

Michal



Re: [PATCH RFC v4 13/15] target/riscv: Introduce dynamic time frequency for virt machine

2021-01-05 Thread Alistair Francis
On Mon, Dec 14, 2020 at 11:31 PM Jiangyifei  wrote:
>
>
> > -Original Message-
> > From: Alistair Francis [mailto:alistai...@gmail.com]
> > Sent: Wednesday, December 9, 2020 6:26 AM
> > To: Jiangyifei 
> > Cc: qemu-de...@nongnu.org Developers ; open
> > list:RISC-V ; Zhangxiaofeng (F)
> > ; Sagar Karandikar
> > ; open list:Overall ;
> > libvir-list@redhat.com; Bastian Koppelmann
> > ; Anup Patel ;
> > yinyipeng ; Alistair Francis
> > ; kvm-ri...@lists.infradead.org; Palmer Dabbelt
> > ; dengkai (A) ; Wubin (H)
> > ; Zhanghailiang 
> > Subject: Re: [PATCH RFC v4 13/15] target/riscv: Introduce dynamic time
> > frequency for virt machine
> >
> > On Thu, Dec 3, 2020 at 4:57 AM Yifei Jiang  wrote:
> > >
> > > Currently, time base frequency was fixed as SIFIVE_CLINT_TIMEBASE_FREQ.
> > > Here introduce "time-frequency" property to set time base frequency
> > > dynamically of which default value is still
> > > SIFIVE_CLINT_TIMEBASE_FREQ. The virt machine uses frequency of the first
> > cpu to create clint and fdt.
> > >
> > > Signed-off-by: Yifei Jiang 
> > > Signed-off-by: Yipeng Yin 
> > > ---
> > >  hw/riscv/virt.c| 18 ++
> > >  target/riscv/cpu.c |  3 +++
> > >  target/riscv/cpu.h |  2 ++
> > >  3 files changed, 19 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c index
> > > 47b7018193..788a7237b6 100644
> > > --- a/hw/riscv/virt.c
> > > +++ b/hw/riscv/virt.c
> > > @@ -178,7 +178,7 @@ static void create_pcie_irq_map(void *fdt, char
> > > *nodename,  }
> > >
> > >  static void create_fdt(RISCVVirtState *s, const struct MemmapEntry
> > *memmap,
> > > -uint64_t mem_size, const char *cmdline)
> > > +uint64_t mem_size, const char *cmdline, uint64_t
> > > + timebase_frequency)
> > >  {
> > >  void *fdt;
> > >  int i, cpu, socket;
> > > @@ -225,7 +225,7 @@ static void create_fdt(RISCVVirtState *s, const
> > > struct MemmapEntry *memmap,
> > >
> > >  qemu_fdt_add_subnode(fdt, "/cpus");
> > >  qemu_fdt_setprop_cell(fdt, "/cpus", "timebase-frequency",
> > > -  SIFIVE_CLINT_TIMEBASE_FREQ);
> > > +  timebase_frequency);
> > >  qemu_fdt_setprop_cell(fdt, "/cpus", "#size-cells", 0x0);
> > >  qemu_fdt_setprop_cell(fdt, "/cpus", "#address-cells", 0x1);
> > >  qemu_fdt_add_subnode(fdt, "/cpus/cpu-map"); @@ -510,6 +510,7
> > @@
> > > static void virt_machine_init(MachineState *machine)
> > >  target_ulong firmware_end_addr, kernel_start_addr;
> > >  uint32_t fdt_load_addr;
> > >  uint64_t kernel_entry;
> > > +uint64_t timebase_frequency = 0;
> > >  DeviceState *mmio_plic, *virtio_plic, *pcie_plic;
> > >  int i, j, base_hartid, hart_count;
> > >  CPUState *cs;
> > > @@ -553,12 +554,20 @@ static void virt_machine_init(MachineState
> > *machine)
> > >  hart_count, _abort);
> > >  sysbus_realize(SYS_BUS_DEVICE(>soc[i]), _abort);
> > >
> > > +if (!timebase_frequency) {
> > > +timebase_frequency = RISCV_CPU(first_cpu)->env.frequency;
> > > +}
> > > +/* If vcpu's time frequency is not specified, we use default
> > frequency */
> > > +if (!timebase_frequency) {
> > > +timebase_frequency = SIFIVE_CLINT_TIMEBASE_FREQ;
> > > +}
> > > +
> > >  /* Per-socket CLINT */
> > >  sifive_clint_create(
> > >  memmap[VIRT_CLINT].base + i *
> > memmap[VIRT_CLINT].size,
> > >  memmap[VIRT_CLINT].size, base_hartid, hart_count,
> > >  SIFIVE_SIP_BASE, SIFIVE_TIMECMP_BASE,
> > SIFIVE_TIME_BASE,
> > > -SIFIVE_CLINT_TIMEBASE_FREQ, true);
> > > +timebase_frequency, true);
> > >
> > >  /* Per-socket PLIC hart topology configuration string */
> > >  plic_hart_config_len =
> > > @@ -610,7 +619,8 @@ static void virt_machine_init(MachineState
> > *machine)
> > >  main_mem);
> > >
> > >  /* create device tree */
> > > -create_fdt(s, memmap, machine->ram_size,
> > machine->kernel_cmdline);
> > > +create_fdt(s, memmap, machine->ram_size,
> > machine->kernel_cmdline,
> > > +   timebase_frequency);
> > >
> > >  /* boot rom */
> > >  memory_region_init_rom(mask_rom, NULL, "riscv_virt_board.mrom",
> > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index
> > > 439dc89ee7..66f35bcbbf 100644
> > > --- a/target/riscv/cpu.c
> > > +++ b/target/riscv/cpu.c
> > > @@ -494,6 +494,8 @@ static void riscv_cpu_realize(DeviceState *dev,
> > > Error **errp)
> > >
> > >  riscv_cpu_register_gdb_regs_for_features(cs);
> > >
> > > +env->user_frequency = env->frequency;
> > > +
> > >  qemu_init_vcpu(cs);
> > >  cpu_reset(cs);
> > >
> > > @@ -531,6 +533,7 @@ static Property riscv_cpu_properties[] = {
> > >  DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
> > >  DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
> > >  

[libvirt PATCH v2 06/16] docs: add manpage for virtinterfaced

2021-01-05 Thread Daniel P . Berrangé
This is an adaptation of the libvirtd manpage.

Signed-off-by: Daniel P. Berrangé 
---
 docs/manpages/index.rst  |   1 +
 docs/manpages/meson.build|   1 +
 docs/manpages/virtinterfaced.rst | 215 +++
 3 files changed, 217 insertions(+)
 create mode 100644 docs/manpages/virtinterfaced.rst

diff --git a/docs/manpages/index.rst b/docs/manpages/index.rst
index da835d62ec..18153ac714 100644
--- a/docs/manpages/index.rst
+++ b/docs/manpages/index.rst
@@ -18,6 +18,7 @@ Modular Driver daemons
 These daemons provide functionality to a single libvirt driver
 
 * `virtbhyved(8) `__ - libvirt bhyve management daemon
+* `virtinterfaced(8) `__ - libvirt host network interface 
management daemon
 
 Tools
 =
diff --git a/docs/manpages/meson.build b/docs/manpages/meson.build
index 7c03cb74cf..a46f75d503 100644
--- a/docs/manpages/meson.build
+++ b/docs/manpages/meson.build
@@ -23,6 +23,7 @@ docs_man_files = [
   { 'name': 'libvirtd', 'section': '8', 'install': conf.has('WITH_LIBVIRTD') },
   { 'name': 'virt-sanlock-cleanup', 'section': '8', 'install': 
conf.has('WITH_SANLOCK') },
   { 'name': 'virtbhyved', 'section': '8', 'install': conf.has('WITH_BHYVE') },
+  { 'name': 'virtinterfaced', 'section': '8', 'install': 
conf.has('WITH_INTERFACE') },
   { 'name': 'virtlockd', 'section': '8', 'install': conf.has('WITH_LIBVIRTD') 
},
   { 'name': 'virtlogd', 'section': '8', 'install': conf.has('WITH_LIBVIRTD') },
   { 'name': 'virtproxyd', 'section': '8', 'install': conf.has('WITH_LIBVIRTD') 
},
diff --git a/docs/manpages/virtinterfaced.rst b/docs/manpages/virtinterfaced.rst
new file mode 100644
index 00..5777dba638
--- /dev/null
+++ b/docs/manpages/virtinterfaced.rst
@@ -0,0 +1,215 @@
+==
+virtinterfaced
+==
+
+
+libvirt host network interface management daemon
+
+
+:Manual section: 8
+:Manual group: Virtualization Support
+
+.. contents::
+
+SYNOPSIS
+
+
+``virtinterfaced`` [*OPTION*]...
+
+
+DESCRIPTION
+===
+
+The ``virtinterfaced`` program is a server side daemon component of the libvirt
+virtualization management system.
+
+It is one of a collection of modular daemons that replace functionality
+previously provided by the monolithic ``libvirtd`` daemon.
+
+This daemon runs on virtualization hosts to provide management for host network
+interfaces.
+
+The ``virtinterfaced`` daemon only listens for requests on a local Unix domain
+socket. Remote off-host access and backwards compatibility with legacy
+clients expecting ``libvirtd`` is provided by the ``virtproxy`` daemon.
+
+Restarting ``virtinterfaced`` does not interrupt running guests. Guests 
continue to
+operate and changes in their state will generally be picked up automatically
+during startup. None the less it is recommended to avoid restarting with
+running guests whenever practical.
+
+
+SYSTEM SOCKET ACTIVATION
+
+
+The ``virtinterfaced`` daemon is capable of starting in two modes.
+
+In the traditional mode, it will create and listen on UNIX sockets itself.
+
+In socket activation mode, it will rely on systemd to create and listen
+on the UNIX sockets and pass them as pre-opened file descriptors. In this
+mode most of the socket related config options in
+``/etc/libvirt/virtinterfaced.conf`` will no longer have any effect.
+
+Socket activation mode is generally the default when running on a host
+OS that uses systemd. To revert to the traditional mode, all the socket
+unit files must be masked:
+
+::
+
+   $ systemctl mask virtinterfaced.socket virtinterfaced-ro.socket \
+  virtinterfaced-admin.socket
+
+
+OPTIONS
+===
+
+``-h``, ``--help``
+
+Display command line help usage then exit.
+
+``-d``, ``--daemon``
+
+Run as a daemon & write PID file.
+
+``-f``, ``--config *FILE*``
+
+Use this configuration file, overriding the default value.
+
+``-p``, ``--pid-file *FILE*``
+
+Use this name for the PID file, overriding the default value.
+
+``-t``, ``--timeout *SECONDS*``
+
+Exit after timeout period (in seconds), provided there are neither any client
+connections nor any running domains.
+
+``-v``, ``--verbose``
+
+Enable output of verbose messages.
+
+``--version``
+
+Display version information then exit.
+
+
+SIGNALS
+===
+
+On receipt of ``SIGHUP`` ``virtinterfaced`` will reload its configuration.
+
+
+FILES
+=
+
+When run as *root*
+--
+
+* ``@SYSCONFDIR@/libvirt/virtinterfaced.conf``
+
+The default configuration file used by ``virtinterfaced``, unless overridden 
on the
+command line using the ``-f`` | ``--config`` option.
+
+* ``@RUNSTATEDIR@/libvirt/virtinterfaced-sock``
+* ``@RUNSTATEDIR@/libvirt/virtinterfaced-sock-ro``
+* ``@RUNSTATEDIR@/libvirt/virtinterfaced-admin-sock``
+
+The sockets ``virtinterfaced`` will use.
+
+The TLS **Server** private key ``virtinterfaced`` will use.
+
+* 

[libvirt PATCH v2 14/16] docs: add manpage for virtvboxd

2021-01-05 Thread Daniel P . Berrangé
This is an adaptation of the libvirtd manpage.

Signed-off-by: Daniel P. Berrangé 
---
 docs/manpages/index.rst |   1 +
 docs/manpages/meson.build   |   1 +
 docs/manpages/virtvboxd.rst | 213 
 3 files changed, 215 insertions(+)
 create mode 100644 docs/manpages/virtvboxd.rst

diff --git a/docs/manpages/index.rst b/docs/manpages/index.rst
index 40f1fb849f..fa983c6cbd 100644
--- a/docs/manpages/index.rst
+++ b/docs/manpages/index.rst
@@ -26,6 +26,7 @@ These daemons provide functionality to a single libvirt driver
 * `virtqemud(8) `__ - libvirt QEMU management daemon
 * `virtsecretd(8) `__ - libvirt secret data management daemon
 * `virtstoraged(8) `__ - libvirt storage pool management 
daemon
+* `virtvboxd(8) `__ - libvirt VirtualBox management daemon
 
 Tools
 =
diff --git a/docs/manpages/meson.build b/docs/manpages/meson.build
index 9bc61c56cd..af10d3a2c5 100644
--- a/docs/manpages/meson.build
+++ b/docs/manpages/meson.build
@@ -34,6 +34,7 @@ docs_man_files = [
   { 'name': 'virtqemud', 'section': '8', 'install': conf.has('WITH_QEMU') },
   { 'name': 'virtsecretd', 'section': '8', 'install': conf.has('WITH_SECRETS') 
},
   { 'name': 'virtstoraged', 'section': '8', 'install': 
conf.has('WITH_STORAGE') },
+  { 'name': 'virtvboxd', 'section': '8', 'install': conf.has('WITH_VBOX') },
 ]
 
 foreach name : keycode_list
diff --git a/docs/manpages/virtvboxd.rst b/docs/manpages/virtvboxd.rst
new file mode 100644
index 00..d452766b3e
--- /dev/null
+++ b/docs/manpages/virtvboxd.rst
@@ -0,0 +1,213 @@
+=
+virtvboxd
+=
+
+
+libvirt VirtualBox management daemon
+
+
+:Manual section: 8
+:Manual group: Virtualization Support
+
+.. contents::
+
+SYNOPSIS
+
+
+``virtvboxd`` [*OPTION*]...
+
+
+DESCRIPTION
+===
+
+The ``virtvboxd`` program is a server side daemon component of the libvirt
+virtualization management system.
+
+It is one of a collection of modular daemons that replace functionality
+previously provided by the monolithic ``libvirtd`` daemon.
+
+This daemon runs on virtualization hosts to provide management for VirtualBox
+virtual machines.
+
+The ``virtvboxd`` daemon only listens for requests on a local Unix domain
+socket. Remote off-host access and backwards compatibility with legacy
+clients expecting ``libvirtd`` is provided by the ``virtproxy`` daemon.
+
+Restarting ``virtvboxd`` does not interrupt running guests. Guests continue to
+operate and changes in their state will generally be picked up automatically
+during startup.
+
+SYSTEM SOCKET ACTIVATION
+
+
+The ``virtvboxd`` daemon is capable of starting in two modes.
+
+In the traditional mode, it will create and listen on UNIX sockets itself.
+
+In socket activation mode, it will rely on systemd to create and listen
+on the UNIX sockets and pass them as pre-opened file descriptors. In this
+mode most of the socket related config options in
+``/etc/libvirt/virtvboxd.conf`` will no longer have any effect.
+
+Socket activation mode is generally the default when running on a host
+OS that uses systemd. To revert to the traditional mode, all the socket
+unit files must be masked:
+
+::
+
+   $ systemctl mask virtvboxd.socket virtvboxd-ro.socket \
+  virtvboxd-admin.socket
+
+
+OPTIONS
+===
+
+``-h``, ``--help``
+
+Display command line help usage then exit.
+
+``-d``, ``--daemon``
+
+Run as a daemon & write PID file.
+
+``-f``, ``--config *FILE*``
+
+Use this configuration file, overriding the default value.
+
+``-p``, ``--pid-file *FILE*``
+
+Use this name for the PID file, overriding the default value.
+
+``-t``, ``--timeout *SECONDS*``
+
+Exit after timeout period (in seconds), provided there are neither any client
+connections nor any running domains.
+
+``-v``, ``--verbose``
+
+Enable output of verbose messages.
+
+``--version``
+
+Display version information then exit.
+
+
+SIGNALS
+===
+
+On receipt of ``SIGHUP`` ``virtvboxd`` will reload its configuration.
+
+
+FILES
+=
+
+When run as *root*
+--
+
+* ``@SYSCONFDIR@/libvirt/virtvboxd.conf``
+
+The default configuration file used by ``virtvboxd``, unless overridden on the
+command line using the ``-f`` | ``--config`` option.
+
+* ``@RUNSTATEDIR@/libvirt/virtvboxd-sock``
+* ``@RUNSTATEDIR@/libvirt/virtvboxd-sock-ro``
+* ``@RUNSTATEDIR@/libvirt/virtvboxd-admin-sock``
+
+The sockets ``virtvboxd`` will use.
+
+The TLS **Server** private key ``virtvboxd`` will use.
+
+* ``@RUNSTATEDIR@/virtvboxd.pid``
+
+The PID file to use, unless overridden by the ``-p`` | ``--pid-file`` option.
+
+
+When run as *non-root*
+--
+
+* ``$XDG_CONFIG_HOME/libvirt/virtvboxd.conf``
+
+The default configuration file used by ``virtvboxd``, unless overridden on the
+command line using the ``-f``|``--config`` option.
+
+* ``$XDG_RUNTIME_DIR/libvirt/virtvboxd-sock``
+* 

[libvirt PATCH v2 16/16] docs: add manpage for virtxend

2021-01-05 Thread Daniel P . Berrangé
This is an adaptation of the libvirtd manpage.

Signed-off-by: Daniel P. Berrangé 
---
 docs/manpages/index.rst|   1 +
 docs/manpages/meson.build  |   1 +
 docs/manpages/virtxend.rst | 215 +
 3 files changed, 217 insertions(+)
 create mode 100644 docs/manpages/virtxend.rst

diff --git a/docs/manpages/index.rst b/docs/manpages/index.rst
index 4e351722a5..fb4a36d46a 100644
--- a/docs/manpages/index.rst
+++ b/docs/manpages/index.rst
@@ -28,6 +28,7 @@ These daemons provide functionality to a single libvirt driver
 * `virtstoraged(8) `__ - libvirt storage pool management 
daemon
 * `virtvboxd(8) `__ - libvirt VirtualBox management daemon
 * `virtvzd(8) `__ - libvirt Virtuozzo management daemon
+* `virtxend(8) `__ - libvirt Xen management daemon
 
 Tools
 =
diff --git a/docs/manpages/meson.build b/docs/manpages/meson.build
index 9c3431c95c..800c3692fb 100644
--- a/docs/manpages/meson.build
+++ b/docs/manpages/meson.build
@@ -36,6 +36,7 @@ docs_man_files = [
   { 'name': 'virtstoraged', 'section': '8', 'install': 
conf.has('WITH_STORAGE') },
   { 'name': 'virtvboxd', 'section': '8', 'install': conf.has('WITH_VBOX') },
   { 'name': 'virtvzd', 'section': '8', 'install': conf.has('WITH_VZ') },
+  { 'name': 'virtxend', 'section': '8', 'install': conf.has('WITH_XEN') },
 ]
 
 foreach name : keycode_list
diff --git a/docs/manpages/virtxend.rst b/docs/manpages/virtxend.rst
new file mode 100644
index 00..cf0128d3ff
--- /dev/null
+++ b/docs/manpages/virtxend.rst
@@ -0,0 +1,215 @@
+
+virtxend
+
+
+-
+libvirt Xen management daemon
+-
+
+:Manual section: 8
+:Manual group: Virtualization Support
+
+.. contents::
+
+SYNOPSIS
+
+
+``virtxend`` [*OPTION*]...
+
+
+DESCRIPTION
+===
+
+The ``virtxend`` program is a server side daemon component of the libvirt
+virtualization management system.
+
+It is one of a collection of modular daemons that replace functionality
+previously provided by the monolithic ``libvirtd`` daemon.
+
+This daemon runs on virtualization hosts to provide management for Xen virtual
+machines.
+
+The ``virtxend`` daemon only listens for requests on a local Unix domain
+socket. Remote off-host access and backwards compatibility with legacy
+clients expecting ``libvirtd`` is provided by the ``virtproxy`` daemon.
+
+Restarting ``virtxend`` does not interrupt running guests. Guests continue to
+operate and changes in their state will generally be picked up automatically
+during startup. None the less it is recommended to avoid restarting with
+running guests whenever practical.
+
+
+SYSTEM SOCKET ACTIVATION
+
+
+The ``virtxend`` daemon is capable of starting in two modes.
+
+In the traditional mode, it will create and listen on UNIX sockets itself.
+
+In socket activation mode, it will rely on systemd to create and listen
+on the UNIX sockets and pass them as pre-opened file descriptors. In this
+mode most of the socket related config options in
+``/etc/libvirt/virtxend.conf`` will no longer have any effect.
+
+Socket activation mode is generally the default when running on a host
+OS that uses systemd. To revert to the traditional mode, all the socket
+unit files must be masked:
+
+::
+
+   $ systemctl mask virtxend.socket virtxend-ro.socket \
+  virtxend-admin.socket
+
+
+OPTIONS
+===
+
+``-h``, ``--help``
+
+Display command line help usage then exit.
+
+``-d``, ``--daemon``
+
+Run as a daemon & write PID file.
+
+``-f``, ``--config *FILE*``
+
+Use this configuration file, overriding the default value.
+
+``-p``, ``--pid-file *FILE*``
+
+Use this name for the PID file, overriding the default value.
+
+``-t``, ``--timeout *SECONDS*``
+
+Exit after timeout period (in seconds), provided there are neither any client
+connections nor any running domains.
+
+``-v``, ``--verbose``
+
+Enable output of verbose messages.
+
+``--version``
+
+Display version information then exit.
+
+
+SIGNALS
+===
+
+On receipt of ``SIGHUP`` ``virtxend`` will reload its configuration.
+
+
+FILES
+=
+
+When run as *root*
+--
+
+* ``@SYSCONFDIR@/libvirt/virtxend.conf``
+
+The default configuration file used by ``virtxend``, unless overridden on the
+command line using the ``-f`` | ``--config`` option.
+
+* ``@RUNSTATEDIR@/libvirt/virtxend-sock``
+* ``@RUNSTATEDIR@/libvirt/virtxend-sock-ro``
+* ``@RUNSTATEDIR@/libvirt/virtxend-admin-sock``
+
+The sockets ``virtxend`` will use.
+
+The TLS **Server** private key ``virtxend`` will use.
+
+* ``@RUNSTATEDIR@/virtxend.pid``
+
+The PID file to use, unless overridden by the ``-p`` | ``--pid-file`` option.
+
+
+When run as *non-root*
+--
+
+* ``$XDG_CONFIG_HOME/libvirt/virtxend.conf``
+
+The default configuration file used by ``virtxend``, unless overridden on the
+command line using the ``-f``|``--config`` option.
+
+* ``$XDG_RUNTIME_DIR/libvirt/virtxend-sock``
+* 

[libvirt PATCH v2 15/16] docs: add manpage for virtvzd

2021-01-05 Thread Daniel P . Berrangé
This is an adaptation of the libvirtd manpage.

Signed-off-by: Daniel P. Berrangé 
---
 docs/manpages/index.rst   |   1 +
 docs/manpages/meson.build |   1 +
 docs/manpages/virtvzd.rst | 215 ++
 3 files changed, 217 insertions(+)
 create mode 100644 docs/manpages/virtvzd.rst

diff --git a/docs/manpages/index.rst b/docs/manpages/index.rst
index fa983c6cbd..4e351722a5 100644
--- a/docs/manpages/index.rst
+++ b/docs/manpages/index.rst
@@ -27,6 +27,7 @@ These daemons provide functionality to a single libvirt driver
 * `virtsecretd(8) `__ - libvirt secret data management daemon
 * `virtstoraged(8) `__ - libvirt storage pool management 
daemon
 * `virtvboxd(8) `__ - libvirt VirtualBox management daemon
+* `virtvzd(8) `__ - libvirt Virtuozzo management daemon
 
 Tools
 =
diff --git a/docs/manpages/meson.build b/docs/manpages/meson.build
index af10d3a2c5..9c3431c95c 100644
--- a/docs/manpages/meson.build
+++ b/docs/manpages/meson.build
@@ -35,6 +35,7 @@ docs_man_files = [
   { 'name': 'virtsecretd', 'section': '8', 'install': conf.has('WITH_SECRETS') 
},
   { 'name': 'virtstoraged', 'section': '8', 'install': 
conf.has('WITH_STORAGE') },
   { 'name': 'virtvboxd', 'section': '8', 'install': conf.has('WITH_VBOX') },
+  { 'name': 'virtvzd', 'section': '8', 'install': conf.has('WITH_VZ') },
 ]
 
 foreach name : keycode_list
diff --git a/docs/manpages/virtvzd.rst b/docs/manpages/virtvzd.rst
new file mode 100644
index 00..7e776026f8
--- /dev/null
+++ b/docs/manpages/virtvzd.rst
@@ -0,0 +1,215 @@
+===
+virtvzd
+===
+
+---
+libvirt Virtuozzo management daemon
+---
+
+:Manual section: 8
+:Manual group: Virtualization Support
+
+.. contents::
+
+SYNOPSIS
+
+
+``virtvzd`` [*OPTION*]...
+
+
+DESCRIPTION
+===
+
+The ``virtvzd`` program is a server side daemon component of the libvirt
+virtualization management system.
+
+It is one of a collection of modular daemons that replace functionality
+previously provided by the monolithic ``libvirtd`` daemon.
+
+This daemon runs on virtualization hosts to provide management for Virtuozzo
+virtual machines.
+
+The ``virtvzd`` daemon only listens for requests on a local Unix domain
+socket. Remote off-host access and backwards compatibility with legacy
+clients expecting ``libvirtd`` is provided by the ``virtproxy`` daemon.
+
+Restarting ``virtvzd`` does not interrupt running guests. Guests continue to
+operate and changes in their state will generally be picked up automatically
+during startup. None the less it is recommended to avoid restarting with
+running guests whenever practical.
+
+
+SYSTEM SOCKET ACTIVATION
+
+
+The ``virtvzd`` daemon is capable of starting in two modes.
+
+In the traditional mode, it will create and listen on UNIX sockets itself.
+
+In socket activation mode, it will rely on systemd to create and listen
+on the UNIX sockets and pass them as pre-opened file descriptors. In this
+mode most of the socket related config options in
+``/etc/libvirt/virtvzd.conf`` will no longer have any effect.
+
+Socket activation mode is generally the default when running on a host
+OS that uses systemd. To revert to the traditional mode, all the socket
+unit files must be masked:
+
+::
+
+   $ systemctl mask virtvzd.socket virtvzd-ro.socket \
+  virtvzd-admin.socket
+
+
+OPTIONS
+===
+
+``-h``, ``--help``
+
+Display command line help usage then exit.
+
+``-d``, ``--daemon``
+
+Run as a daemon & write PID file.
+
+``-f``, ``--config *FILE*``
+
+Use this configuration file, overriding the default value.
+
+``-p``, ``--pid-file *FILE*``
+
+Use this name for the PID file, overriding the default value.
+
+``-t``, ``--timeout *SECONDS*``
+
+Exit after timeout period (in seconds), provided there are neither any client
+connections nor any running domains.
+
+``-v``, ``--verbose``
+
+Enable output of verbose messages.
+
+``--version``
+
+Display version information then exit.
+
+
+SIGNALS
+===
+
+On receipt of ``SIGHUP`` ``virtvzd`` will reload its configuration.
+
+
+FILES
+=
+
+When run as *root*
+--
+
+* ``@SYSCONFDIR@/libvirt/virtvzd.conf``
+
+The default configuration file used by ``virtvzd``, unless overridden on the
+command line using the ``-f`` | ``--config`` option.
+
+* ``@RUNSTATEDIR@/libvirt/virtvzd-sock``
+* ``@RUNSTATEDIR@/libvirt/virtvzd-sock-ro``
+* ``@RUNSTATEDIR@/libvirt/virtvzd-admin-sock``
+
+The sockets ``virtvzd`` will use.
+
+The TLS **Server** private key ``virtvzd`` will use.
+
+* ``@RUNSTATEDIR@/virtvzd.pid``
+
+The PID file to use, unless overridden by the ``-p`` | ``--pid-file`` option.
+
+
+When run as *non-root*
+--
+
+* ``$XDG_CONFIG_HOME/libvirt/virtvzd.conf``
+
+The default configuration file used by ``virtvzd``, unless overridden on the
+command line using the ``-f``|``--config`` option.
+
+* ``$XDG_RUNTIME_DIR/libvirt/virtvzd-sock``
+* 

[libvirt PATCH v2 13/16] docs: add manpage for virtstoraged

2021-01-05 Thread Daniel P . Berrangé
This is an adaptation of the libvirtd manpage.

Signed-off-by: Daniel P. Berrangé 
---
 docs/manpages/index.rst|   1 +
 docs/manpages/meson.build  |   1 +
 docs/manpages/virtstoraged.rst | 215 +
 3 files changed, 217 insertions(+)
 create mode 100644 docs/manpages/virtstoraged.rst

diff --git a/docs/manpages/index.rst b/docs/manpages/index.rst
index fb62dc86a2..40f1fb849f 100644
--- a/docs/manpages/index.rst
+++ b/docs/manpages/index.rst
@@ -25,6 +25,7 @@ These daemons provide functionality to a single libvirt driver
 * `virtnwfilterd(8) `__ - libvirt network filter 
management daemon
 * `virtqemud(8) `__ - libvirt QEMU management daemon
 * `virtsecretd(8) `__ - libvirt secret data management daemon
+* `virtstoraged(8) `__ - libvirt storage pool management 
daemon
 
 Tools
 =
diff --git a/docs/manpages/meson.build b/docs/manpages/meson.build
index 1476722bde..9bc61c56cd 100644
--- a/docs/manpages/meson.build
+++ b/docs/manpages/meson.build
@@ -33,6 +33,7 @@ docs_man_files = [
   { 'name': 'virtproxyd', 'section': '8', 'install': conf.has('WITH_LIBVIRTD') 
},
   { 'name': 'virtqemud', 'section': '8', 'install': conf.has('WITH_QEMU') },
   { 'name': 'virtsecretd', 'section': '8', 'install': conf.has('WITH_SECRETS') 
},
+  { 'name': 'virtstoraged', 'section': '8', 'install': 
conf.has('WITH_STORAGE') },
 ]
 
 foreach name : keycode_list
diff --git a/docs/manpages/virtstoraged.rst b/docs/manpages/virtstoraged.rst
new file mode 100644
index 00..1ca0b394bc
--- /dev/null
+++ b/docs/manpages/virtstoraged.rst
@@ -0,0 +1,215 @@
+
+virtstoraged
+
+
+--
+libvirt storage pool management daemon
+--
+
+:Manual section: 8
+:Manual group: Virtualization Support
+
+.. contents::
+
+SYNOPSIS
+
+
+``virtstoraged`` [*OPTION*]...
+
+
+DESCRIPTION
+===
+
+The ``virtstoraged`` program is a server side daemon component of the libvirt
+virtualization management system.
+
+It is one of a collection of modular daemons that replace functionality
+previously provided by the monolithic ``libvirtd`` daemon.
+
+This daemon runs on virtualization hosts to provide management for storage
+pools.
+
+The ``virtstoraged`` daemon only listens for requests on a local Unix domain
+socket. Remote off-host access and backwards compatibility with legacy
+clients expecting ``libvirtd`` is provided by the ``virtproxy`` daemon.
+
+Restarting ``virtstoraged`` does not interrupt running guests. Guests continue 
to
+operate and changes in their state will generally be picked up automatically
+during startup. None the less it is recommended to avoid restarting with
+running guests whenever practical.
+
+
+SYSTEM SOCKET ACTIVATION
+
+
+The ``virtstoraged`` daemon is capable of starting in two modes.
+
+In the traditional mode, it will create and listen on UNIX sockets itself.
+
+In socket activation mode, it will rely on systemd to create and listen
+on the UNIX sockets and pass them as pre-opened file descriptors. In this
+mode most of the socket related config options in
+``/etc/libvirt/virtstoraged.conf`` will no longer have any effect.
+
+Socket activation mode is generally the default when running on a host
+OS that uses systemd. To revert to the traditional mode, all the socket
+unit files must be masked:
+
+::
+
+   $ systemctl mask virtstoraged.socket virtstoraged-ro.socket \
+  virtstoraged-admin.socket
+
+
+OPTIONS
+===
+
+``-h``, ``--help``
+
+Display command line help usage then exit.
+
+``-d``, ``--daemon``
+
+Run as a daemon & write PID file.
+
+``-f``, ``--config *FILE*``
+
+Use this configuration file, overriding the default value.
+
+``-p``, ``--pid-file *FILE*``
+
+Use this name for the PID file, overriding the default value.
+
+``-t``, ``--timeout *SECONDS*``
+
+Exit after timeout period (in seconds), provided there are neither any client
+connections nor any running domains.
+
+``-v``, ``--verbose``
+
+Enable output of verbose messages.
+
+``--version``
+
+Display version information then exit.
+
+
+SIGNALS
+===
+
+On receipt of ``SIGHUP`` ``virtstoraged`` will reload its configuration.
+
+
+FILES
+=
+
+When run as *root*
+--
+
+* ``@SYSCONFDIR@/libvirt/virtstoraged.conf``
+
+The default configuration file used by ``virtstoraged``, unless overridden on 
the
+command line using the ``-f`` | ``--config`` option.
+
+* ``@RUNSTATEDIR@/libvirt/virtstoraged-sock``
+* ``@RUNSTATEDIR@/libvirt/virtstoraged-sock-ro``
+* ``@RUNSTATEDIR@/libvirt/virtstoraged-admin-sock``
+
+The sockets ``virtstoraged`` will use.
+
+The TLS **Server** private key ``virtstoraged`` will use.
+
+* ``@RUNSTATEDIR@/virtstoraged.pid``
+
+The PID file to use, unless overridden by the ``-p`` | ``--pid-file`` option.
+
+
+When run as *non-root*
+--
+
+* ``$XDG_CONFIG_HOME/libvirt/virtstoraged.conf``
+
+The default configuration file 

[libvirt PATCH v2 11/16] docs: add manpage for virtqemud

2021-01-05 Thread Daniel P . Berrangé
This is an adaptation of the libvirtd manpage.

Signed-off-by: Daniel P. Berrangé 
---
 docs/manpages/index.rst |   1 +
 docs/manpages/meson.build   |   1 +
 docs/manpages/virtqemud.rst | 215 
 3 files changed, 217 insertions(+)
 create mode 100644 docs/manpages/virtqemud.rst

diff --git a/docs/manpages/index.rst b/docs/manpages/index.rst
index e70b560a0d..67357419eb 100644
--- a/docs/manpages/index.rst
+++ b/docs/manpages/index.rst
@@ -23,6 +23,7 @@ These daemons provide functionality to a single libvirt driver
 * `virtnetworkd(8) `__ - libvirt virtual network management 
daemon
 * `virtnodedevd(8) `__ - libvirt host device management 
daemon
 * `virtnwfilterd(8) `__ - libvirt network filter 
management daemon
+* `virtqemud(8) `__ - libvirt QEMU management daemon
 
 Tools
 =
diff --git a/docs/manpages/meson.build b/docs/manpages/meson.build
index 019accbca2..e08365b780 100644
--- a/docs/manpages/meson.build
+++ b/docs/manpages/meson.build
@@ -31,6 +31,7 @@ docs_man_files = [
   { 'name': 'virtnodedevd', 'section': '8', 'install': 
conf.has('WITH_NODE_DEVICES') },
   { 'name': 'virtnwfilterd', 'section': '8', 'install': 
conf.has('WITH_NWFILTER') },
   { 'name': 'virtproxyd', 'section': '8', 'install': conf.has('WITH_LIBVIRTD') 
},
+  { 'name': 'virtqemud', 'section': '8', 'install': conf.has('WITH_QEMU') },
 ]
 
 foreach name : keycode_list
diff --git a/docs/manpages/virtqemud.rst b/docs/manpages/virtqemud.rst
new file mode 100644
index 00..78a032caa1
--- /dev/null
+++ b/docs/manpages/virtqemud.rst
@@ -0,0 +1,215 @@
+=
+virtqemud
+=
+
+--
+libvirt QEMU management daemon
+--
+
+:Manual section: 8
+:Manual group: Virtualization Support
+
+.. contents::
+
+SYNOPSIS
+
+
+``virtqemud`` [*OPTION*]...
+
+
+DESCRIPTION
+===
+
+The ``virtqemud`` program is a server side daemon component of the libvirt
+virtualization management system.
+
+It is one of a collection of modular daemons that replace functionality
+previously provided by the monolithic ``libvirtd`` daemon.
+
+This daemon runs on virtualization hosts to provide management for QEMU virtual
+machines.
+
+The ``virtqemud`` daemon only listens for requests on a local Unix domain
+socket. Remote off-host access and backwards compatibility with legacy
+clients expecting ``libvirtd`` is provided by the ``virtproxy`` daemon.
+
+Restarting ``virtqemud`` does not interrupt running guests. Guests continue to
+operate and changes in their state will generally be picked up automatically
+during startup. None the less it is recommended to avoid restarting with
+running guests whenever practical.
+
+
+SYSTEM SOCKET ACTIVATION
+
+
+The ``virtqemud`` daemon is capable of starting in two modes.
+
+In the traditional mode, it will create and listen on UNIX sockets itself.
+
+In socket activation mode, it will rely on systemd to create and listen
+on the UNIX sockets and pass them as pre-opened file descriptors. In this
+mode most of the socket related config options in
+``/etc/libvirt/virtqemud.conf`` will no longer have any effect.
+
+Socket activation mode is generally the default when running on a host
+OS that uses systemd. To revert to the traditional mode, all the socket
+unit files must be masked:
+
+::
+
+   $ systemctl mask virtqemud.socket virtqemud-ro.socket \
+  virtqemud-admin.socket
+
+
+OPTIONS
+===
+
+``-h``, ``--help``
+
+Display command line help usage then exit.
+
+``-d``, ``--daemon``
+
+Run as a daemon & write PID file.
+
+``-f``, ``--config *FILE*``
+
+Use this configuration file, overriding the default value.
+
+``-p``, ``--pid-file *FILE*``
+
+Use this name for the PID file, overriding the default value.
+
+``-t``, ``--timeout *SECONDS*``
+
+Exit after timeout period (in seconds), provided there are neither any client
+connections nor any running domains.
+
+``-v``, ``--verbose``
+
+Enable output of verbose messages.
+
+``--version``
+
+Display version information then exit.
+
+
+SIGNALS
+===
+
+On receipt of ``SIGHUP`` ``virtqemud`` will reload its configuration.
+
+
+FILES
+=
+
+When run as *root*
+--
+
+* ``@SYSCONFDIR@/libvirt/virtqemud.conf``
+
+The default configuration file used by ``virtqemud``, unless overridden on the
+command line using the ``-f`` | ``--config`` option.
+
+* ``@RUNSTATEDIR@/libvirt/virtqemud-sock``
+* ``@RUNSTATEDIR@/libvirt/virtqemud-sock-ro``
+* ``@RUNSTATEDIR@/libvirt/virtqemud-admin-sock``
+
+The sockets ``virtqemud`` will use.
+
+The TLS **Server** private key ``virtqemud`` will use.
+
+* ``@RUNSTATEDIR@/virtqemud.pid``
+
+The PID file to use, unless overridden by the ``-p`` | ``--pid-file`` option.
+
+
+When run as *non-root*
+--
+
+* ``$XDG_CONFIG_HOME/libvirt/virtqemud.conf``
+
+The default configuration file used by ``virtqemud``, unless overridden on the
+command line using the ``-f``|``--config`` 

[libvirt PATCH v2 12/16] docs: add manpage for virtsecretd

2021-01-05 Thread Daniel P . Berrangé
This is an adaptation of the libvirtd manpage.

Signed-off-by: Daniel P. Berrangé 
---
 docs/manpages/index.rst   |   1 +
 docs/manpages/meson.build |   1 +
 docs/manpages/virtsecretd.rst | 214 ++
 3 files changed, 216 insertions(+)
 create mode 100644 docs/manpages/virtsecretd.rst

diff --git a/docs/manpages/index.rst b/docs/manpages/index.rst
index 67357419eb..fb62dc86a2 100644
--- a/docs/manpages/index.rst
+++ b/docs/manpages/index.rst
@@ -24,6 +24,7 @@ These daemons provide functionality to a single libvirt driver
 * `virtnodedevd(8) `__ - libvirt host device management 
daemon
 * `virtnwfilterd(8) `__ - libvirt network filter 
management daemon
 * `virtqemud(8) `__ - libvirt QEMU management daemon
+* `virtsecretd(8) `__ - libvirt secret data management daemon
 
 Tools
 =
diff --git a/docs/manpages/meson.build b/docs/manpages/meson.build
index e08365b780..1476722bde 100644
--- a/docs/manpages/meson.build
+++ b/docs/manpages/meson.build
@@ -32,6 +32,7 @@ docs_man_files = [
   { 'name': 'virtnwfilterd', 'section': '8', 'install': 
conf.has('WITH_NWFILTER') },
   { 'name': 'virtproxyd', 'section': '8', 'install': conf.has('WITH_LIBVIRTD') 
},
   { 'name': 'virtqemud', 'section': '8', 'install': conf.has('WITH_QEMU') },
+  { 'name': 'virtsecretd', 'section': '8', 'install': conf.has('WITH_SECRETS') 
},
 ]
 
 foreach name : keycode_list
diff --git a/docs/manpages/virtsecretd.rst b/docs/manpages/virtsecretd.rst
new file mode 100644
index 00..2fa01ef147
--- /dev/null
+++ b/docs/manpages/virtsecretd.rst
@@ -0,0 +1,214 @@
+===
+virtsecretd
+===
+
+-
+libvirt secret data management daemon
+-
+
+:Manual section: 8
+:Manual group: Virtualization Support
+
+.. contents::
+
+SYNOPSIS
+
+
+``virtsecretd`` [*OPTION*]...
+
+
+DESCRIPTION
+===
+
+The ``virtsecretd`` program is a server side daemon component of the libvirt
+virtualization management system.
+
+It is one of a collection of modular daemons that replace functionality
+previously provided by the monolithic ``libvirtd`` daemon.
+
+This daemon runs on virtualization hosts to provide management for secret data.
+
+The ``virtsecretd`` daemon only listens for requests on a local Unix domain
+socket. Remote off-host access and backwards compatibility with legacy
+clients expecting ``libvirtd`` is provided by the ``virtproxy`` daemon.
+
+Restarting ``virtsecretd`` does not interrupt running guests. Guests continue 
to
+operate and changes in their state will generally be picked up automatically
+during startup. None the less it is recommended to avoid restarting with
+running guests whenever practical.
+
+
+SYSTEM SOCKET ACTIVATION
+
+
+The ``virtsecretd`` daemon is capable of starting in two modes.
+
+In the traditional mode, it will create and listen on UNIX sockets itself.
+
+In socket activation mode, it will rely on systemd to create and listen
+on the UNIX sockets and pass them as pre-opened file descriptors. In this
+mode most of the socket related config options in
+``/etc/libvirt/virtsecretd.conf`` will no longer have any effect.
+
+Socket activation mode is generally the default when running on a host
+OS that uses systemd. To revert to the traditional mode, all the socket
+unit files must be masked:
+
+::
+
+   $ systemctl mask virtsecretd.socket virtsecretd-ro.socket \
+  virtsecretd-admin.socket
+
+
+OPTIONS
+===
+
+``-h``, ``--help``
+
+Display command line help usage then exit.
+
+``-d``, ``--daemon``
+
+Run as a daemon & write PID file.
+
+``-f``, ``--config *FILE*``
+
+Use this configuration file, overriding the default value.
+
+``-p``, ``--pid-file *FILE*``
+
+Use this name for the PID file, overriding the default value.
+
+``-t``, ``--timeout *SECONDS*``
+
+Exit after timeout period (in seconds), provided there are neither any client
+connections nor any running domains.
+
+``-v``, ``--verbose``
+
+Enable output of verbose messages.
+
+``--version``
+
+Display version information then exit.
+
+
+SIGNALS
+===
+
+On receipt of ``SIGHUP`` ``virtsecretd`` will reload its configuration.
+
+
+FILES
+=
+
+When run as *root*
+--
+
+* ``@SYSCONFDIR@/libvirt/virtsecretd.conf``
+
+The default configuration file used by ``virtsecretd``, unless overridden on 
the
+command line using the ``-f`` | ``--config`` option.
+
+* ``@RUNSTATEDIR@/libvirt/virtsecretd-sock``
+* ``@RUNSTATEDIR@/libvirt/virtsecretd-sock-ro``
+* ``@RUNSTATEDIR@/libvirt/virtsecretd-admin-sock``
+
+The sockets ``virtsecretd`` will use.
+
+The TLS **Server** private key ``virtsecretd`` will use.
+
+* ``@RUNSTATEDIR@/virtsecretd.pid``
+
+The PID file to use, unless overridden by the ``-p`` | ``--pid-file`` option.
+
+
+When run as *non-root*
+--
+
+* ``$XDG_CONFIG_HOME/libvirt/virtsecretd.conf``
+
+The default configuration file used by ``virtsecretd``, unless 

[libvirt PATCH v2 10/16] docs: add manpage for virtnwfilterd

2021-01-05 Thread Daniel P . Berrangé
This is an adaptation of the libvirtd manpage.

Signed-off-by: Daniel P. Berrangé 
---
 docs/manpages/index.rst |   1 +
 docs/manpages/meson.build   |   1 +
 docs/manpages/virtnwfilterd.rst | 215 
 3 files changed, 217 insertions(+)
 create mode 100644 docs/manpages/virtnwfilterd.rst

diff --git a/docs/manpages/index.rst b/docs/manpages/index.rst
index 5e87870f4b..e70b560a0d 100644
--- a/docs/manpages/index.rst
+++ b/docs/manpages/index.rst
@@ -22,6 +22,7 @@ These daemons provide functionality to a single libvirt driver
 * `virtlxcd(8) `__ - libvirt LXC management daemon
 * `virtnetworkd(8) `__ - libvirt virtual network management 
daemon
 * `virtnodedevd(8) `__ - libvirt host device management 
daemon
+* `virtnwfilterd(8) `__ - libvirt network filter 
management daemon
 
 Tools
 =
diff --git a/docs/manpages/meson.build b/docs/manpages/meson.build
index 85f45410a0..019accbca2 100644
--- a/docs/manpages/meson.build
+++ b/docs/manpages/meson.build
@@ -29,6 +29,7 @@ docs_man_files = [
   { 'name': 'virtlxcd', 'section': '8', 'install': conf.has('WITH_LXC') },
   { 'name': 'virtnetworkd', 'section': '8', 'install': 
conf.has('WITH_NETWORK') },
   { 'name': 'virtnodedevd', 'section': '8', 'install': 
conf.has('WITH_NODE_DEVICES') },
+  { 'name': 'virtnwfilterd', 'section': '8', 'install': 
conf.has('WITH_NWFILTER') },
   { 'name': 'virtproxyd', 'section': '8', 'install': conf.has('WITH_LIBVIRTD') 
},
 ]
 
diff --git a/docs/manpages/virtnwfilterd.rst b/docs/manpages/virtnwfilterd.rst
new file mode 100644
index 00..47cca7e282
--- /dev/null
+++ b/docs/manpages/virtnwfilterd.rst
@@ -0,0 +1,215 @@
+=
+virtnwfilterd
+=
+
+
+libvirt network filter management daemon
+
+
+:Manual section: 8
+:Manual group: Virtualization Support
+
+.. contents::
+
+SYNOPSIS
+
+
+``virtnwfilterd`` [*OPTION*]...
+
+
+DESCRIPTION
+===
+
+The ``virtnwfilterd`` program is a server side daemon component of the libvirt
+virtualization management system.
+
+It is one of a collection of modular daemons that replace functionality
+previously provided by the monolithic ``libvirtd`` daemon.
+
+This daemon runs on virtualization hosts to provide management for network
+filters.
+
+The ``virtnwfilterd`` daemon only listens for requests on a local Unix domain
+socket. Remote off-host access and backwards compatibility with legacy
+clients expecting ``libvirtd`` is provided by the ``virtproxy`` daemon.
+
+Restarting ``virtnwfilterd`` does not interrupt running guests. Guests 
continue to
+operate and changes in their state will generally be picked up automatically
+during startup. None the less it is recommended to avoid restarting with
+running guests whenever practical.
+
+
+SYSTEM SOCKET ACTIVATION
+
+
+The ``virtnwfilterd`` daemon is capable of starting in two modes.
+
+In the traditional mode, it will create and listen on UNIX sockets itself.
+
+In socket activation mode, it will rely on systemd to create and listen
+on the UNIX sockets and pass them as pre-opened file descriptors. In this
+mode most of the socket related config options in
+``/etc/libvirt/virtnwfilterd.conf`` will no longer have any effect.
+
+Socket activation mode is generally the default when running on a host
+OS that uses systemd. To revert to the traditional mode, all the socket
+unit files must be masked:
+
+::
+
+   $ systemctl mask virtnwfilterd.socket virtnwfilterd-ro.socket \
+  virtnwfilterd-admin.socket
+
+
+OPTIONS
+===
+
+``-h``, ``--help``
+
+Display command line help usage then exit.
+
+``-d``, ``--daemon``
+
+Run as a daemon & write PID file.
+
+``-f``, ``--config *FILE*``
+
+Use this configuration file, overriding the default value.
+
+``-p``, ``--pid-file *FILE*``
+
+Use this name for the PID file, overriding the default value.
+
+``-t``, ``--timeout *SECONDS*``
+
+Exit after timeout period (in seconds), provided there are neither any client
+connections nor any running domains.
+
+``-v``, ``--verbose``
+
+Enable output of verbose messages.
+
+``--version``
+
+Display version information then exit.
+
+
+SIGNALS
+===
+
+On receipt of ``SIGHUP`` ``virtnwfilterd`` will reload its configuration.
+
+
+FILES
+=
+
+When run as *root*
+--
+
+* ``@SYSCONFDIR@/libvirt/virtnwfilterd.conf``
+
+The default configuration file used by ``virtnwfilterd``, unless overridden on 
the
+command line using the ``-f`` | ``--config`` option.
+
+* ``@RUNSTATEDIR@/libvirt/virtnwfilterd-sock``
+* ``@RUNSTATEDIR@/libvirt/virtnwfilterd-sock-ro``
+* ``@RUNSTATEDIR@/libvirt/virtnwfilterd-admin-sock``
+
+The sockets ``virtnwfilterd`` will use.
+
+The TLS **Server** private key ``virtnwfilterd`` will use.
+
+* ``@RUNSTATEDIR@/virtnwfilterd.pid``
+
+The PID file to use, unless overridden by the ``-p`` | ``--pid-file`` option.
+
+
+When run as *non-root*

[libvirt PATCH v2 07/16] docs: add manpage for virtlxcd

2021-01-05 Thread Daniel P . Berrangé
This is an adaptation of the libvirtd manpage.

Signed-off-by: Daniel P. Berrangé 
---
 docs/manpages/index.rst|   1 +
 docs/manpages/meson.build  |   1 +
 docs/manpages/virtlxcd.rst | 215 +
 3 files changed, 217 insertions(+)
 create mode 100644 docs/manpages/virtlxcd.rst

diff --git a/docs/manpages/index.rst b/docs/manpages/index.rst
index 18153ac714..0eab1056e4 100644
--- a/docs/manpages/index.rst
+++ b/docs/manpages/index.rst
@@ -19,6 +19,7 @@ These daemons provide functionality to a single libvirt driver
 
 * `virtbhyved(8) `__ - libvirt bhyve management daemon
 * `virtinterfaced(8) `__ - libvirt host network interface 
management daemon
+* `virtlxcd(8) `__ - libvirt LXC management daemon
 
 Tools
 =
diff --git a/docs/manpages/meson.build b/docs/manpages/meson.build
index a46f75d503..083289642a 100644
--- a/docs/manpages/meson.build
+++ b/docs/manpages/meson.build
@@ -26,6 +26,7 @@ docs_man_files = [
   { 'name': 'virtinterfaced', 'section': '8', 'install': 
conf.has('WITH_INTERFACE') },
   { 'name': 'virtlockd', 'section': '8', 'install': conf.has('WITH_LIBVIRTD') 
},
   { 'name': 'virtlogd', 'section': '8', 'install': conf.has('WITH_LIBVIRTD') },
+  { 'name': 'virtlxcd', 'section': '8', 'install': conf.has('WITH_LXC') },
   { 'name': 'virtproxyd', 'section': '8', 'install': conf.has('WITH_LIBVIRTD') 
},
 ]
 
diff --git a/docs/manpages/virtlxcd.rst b/docs/manpages/virtlxcd.rst
new file mode 100644
index 00..f89d70ba21
--- /dev/null
+++ b/docs/manpages/virtlxcd.rst
@@ -0,0 +1,215 @@
+
+virtlxcd
+
+
+--
+libvirt LXC management daemon
+--
+
+:Manual section: 8
+:Manual group: Virtualization Support
+
+.. contents::
+
+SYNOPSIS
+
+
+``virtlxcd`` [*OPTION*]...
+
+
+DESCRIPTION
+===
+
+The ``virtlxcd`` program is a server side daemon component of the libvirt
+virtualization management system.
+
+It is one of a collection of modular daemons that replace functionality
+previously provided by the monolithic ``libvirtd`` daemon.
+
+This daemon runs on virtualization hosts to provide management for LXC
+containers.
+
+The ``virtlxcd`` daemon only listens for requests on a local Unix domain
+socket. Remote off-host access and backwards compatibility with legacy
+clients expecting ``libvirtd`` is provided by the ``virtproxy`` daemon.
+
+Restarting ``virtlxcd`` does not interrupt running guests. Guests continue to
+operate and changes in their state will generally be picked up automatically
+during startup. None the less it is recommended to avoid restarting with
+running guests whenever practical.
+
+
+SYSTEM SOCKET ACTIVATION
+
+
+The ``virtlxcd`` daemon is capable of starting in two modes.
+
+In the traditional mode, it will create and listen on UNIX sockets itself.
+
+In socket activation mode, it will rely on systemd to create and listen
+on the UNIX sockets and pass them as pre-opened file descriptors. In this
+mode most of the socket related config options in
+``/etc/libvirt/virtlxcd.conf`` will no longer have any effect.
+
+Socket activation mode is generally the default when running on a host
+OS that uses systemd. To revert to the traditional mode, all the socket
+unit files must be masked:
+
+::
+
+   $ systemctl mask virtlxcd.socket virtlxcd-ro.socket \
+  virtlxcd-admin.socket
+
+
+OPTIONS
+===
+
+``-h``, ``--help``
+
+Display command line help usage then exit.
+
+``-d``, ``--daemon``
+
+Run as a daemon & write PID file.
+
+``-f``, ``--config *FILE*``
+
+Use this configuration file, overriding the default value.
+
+``-p``, ``--pid-file *FILE*``
+
+Use this name for the PID file, overriding the default value.
+
+``-t``, ``--timeout *SECONDS*``
+
+Exit after timeout period (in seconds), provided there are neither any client
+connections nor any running domains.
+
+``-v``, ``--verbose``
+
+Enable output of verbose messages.
+
+``--version``
+
+Display version information then exit.
+
+
+SIGNALS
+===
+
+On receipt of ``SIGHUP`` ``virtlxcd`` will reload its configuration.
+
+
+FILES
+=
+
+When run as *root*
+--
+
+* ``@SYSCONFDIR@/libvirt/virtlxcd.conf``
+
+The default configuration file used by ``virtlxcd``, unless overridden on the
+command line using the ``-f`` | ``--config`` option.
+
+* ``@RUNSTATEDIR@/libvirt/virtlxcd-sock``
+* ``@RUNSTATEDIR@/libvirt/virtlxcd-sock-ro``
+* ``@RUNSTATEDIR@/libvirt/virtlxcd-admin-sock``
+
+The sockets ``virtlxcd`` will use.
+
+The TLS **Server** private key ``virtlxcd`` will use.
+
+* ``@RUNSTATEDIR@/virtlxcd.pid``
+
+The PID file to use, unless overridden by the ``-p`` | ``--pid-file`` option.
+
+
+When run as *non-root*
+--
+
+* ``$XDG_CONFIG_HOME/libvirt/virtlxcd.conf``
+
+The default configuration file used by ``virtlxcd``, unless overridden on the
+command line using the ``-f``|``--config`` option.
+
+* ``$XDG_RUNTIME_DIR/libvirt/virtlxcd-sock``
+* 

[libvirt PATCH v2 09/16] docs: add manpage for virtnodedevd

2021-01-05 Thread Daniel P . Berrangé
This is an adaptation of the libvirtd manpage.

Signed-off-by: Daniel P. Berrangé 
---
 docs/manpages/index.rst|   1 +
 docs/manpages/meson.build  |   1 +
 docs/manpages/virtnodedevd.rst | 214 +
 3 files changed, 216 insertions(+)
 create mode 100644 docs/manpages/virtnodedevd.rst

diff --git a/docs/manpages/index.rst b/docs/manpages/index.rst
index 2ce222f952..5e87870f4b 100644
--- a/docs/manpages/index.rst
+++ b/docs/manpages/index.rst
@@ -21,6 +21,7 @@ These daemons provide functionality to a single libvirt driver
 * `virtinterfaced(8) `__ - libvirt host network interface 
management daemon
 * `virtlxcd(8) `__ - libvirt LXC management daemon
 * `virtnetworkd(8) `__ - libvirt virtual network management 
daemon
+* `virtnodedevd(8) `__ - libvirt host device management 
daemon
 
 Tools
 =
diff --git a/docs/manpages/meson.build b/docs/manpages/meson.build
index 4c294909ee..85f45410a0 100644
--- a/docs/manpages/meson.build
+++ b/docs/manpages/meson.build
@@ -28,6 +28,7 @@ docs_man_files = [
   { 'name': 'virtlogd', 'section': '8', 'install': conf.has('WITH_LIBVIRTD') },
   { 'name': 'virtlxcd', 'section': '8', 'install': conf.has('WITH_LXC') },
   { 'name': 'virtnetworkd', 'section': '8', 'install': 
conf.has('WITH_NETWORK') },
+  { 'name': 'virtnodedevd', 'section': '8', 'install': 
conf.has('WITH_NODE_DEVICES') },
   { 'name': 'virtproxyd', 'section': '8', 'install': conf.has('WITH_LIBVIRTD') 
},
 ]
 
diff --git a/docs/manpages/virtnodedevd.rst b/docs/manpages/virtnodedevd.rst
new file mode 100644
index 00..a29c0dbc9e
--- /dev/null
+++ b/docs/manpages/virtnodedevd.rst
@@ -0,0 +1,214 @@
+
+virtnodedevd
+
+
+-
+libvirt host device management daemon
+-
+
+:Manual section: 8
+:Manual group: Virtualization Support
+
+.. contents::
+
+SYNOPSIS
+
+
+``virtnodedevd`` [*OPTION*]...
+
+
+DESCRIPTION
+===
+
+The ``virtnodedevd`` program is a server side daemon component of the libvirt
+virtualization management system.
+
+It is one of a collection of modular daemons that replace functionality
+previously provided by the monolithic ``libvirtd`` daemon.
+
+This daemon runs on virtualization hosts to provide management for host 
devices.
+
+The ``virtnodedevd`` daemon only listens for requests on a local Unix domain
+socket. Remote off-host access and backwards compatibility with legacy
+clients expecting ``libvirtd`` is provided by the ``virtproxy`` daemon.
+
+Restarting ``virtnodedevd`` does not interrupt running guests. Guests continue 
to
+operate and changes in their state will generally be picked up automatically
+during startup. None the less it is recommended to avoid restarting with
+running guests whenever practical.
+
+
+SYSTEM SOCKET ACTIVATION
+
+
+The ``virtnodedevd`` daemon is capable of starting in two modes.
+
+In the traditional mode, it will create and listen on UNIX sockets itself.
+
+In socket activation mode, it will rely on systemd to create and listen
+on the UNIX sockets and pass them as pre-opened file descriptors. In this
+mode most of the socket related config options in
+``/etc/libvirt/virtnodedevd.conf`` will no longer have any effect.
+
+Socket activation mode is generally the default when running on a host
+OS that uses systemd. To revert to the traditional mode, all the socket
+unit files must be masked:
+
+::
+
+   $ systemctl mask virtnodedevd.socket virtnodedevd-ro.socket \
+  virtnodedevd-admin.socket
+
+
+OPTIONS
+===
+
+``-h``, ``--help``
+
+Display command line help usage then exit.
+
+``-d``, ``--daemon``
+
+Run as a daemon & write PID file.
+
+``-f``, ``--config *FILE*``
+
+Use this configuration file, overriding the default value.
+
+``-p``, ``--pid-file *FILE*``
+
+Use this name for the PID file, overriding the default value.
+
+``-t``, ``--timeout *SECONDS*``
+
+Exit after timeout period (in seconds), provided there are neither any client
+connections nor any running domains.
+
+``-v``, ``--verbose``
+
+Enable output of verbose messages.
+
+``--version``
+
+Display version information then exit.
+
+
+SIGNALS
+===
+
+On receipt of ``SIGHUP`` ``virtnodedevd`` will reload its configuration.
+
+
+FILES
+=
+
+When run as *root*
+--
+
+* ``@SYSCONFDIR@/libvirt/virtnodedevd.conf``
+
+The default configuration file used by ``virtnodedevd``, unless overridden on 
the
+command line using the ``-f`` | ``--config`` option.
+
+* ``@RUNSTATEDIR@/libvirt/virtnodedevd-sock``
+* ``@RUNSTATEDIR@/libvirt/virtnodedevd-sock-ro``
+* ``@RUNSTATEDIR@/libvirt/virtnodedevd-admin-sock``
+
+The sockets ``virtnodedevd`` will use.
+
+The TLS **Server** private key ``virtnodedevd`` will use.
+
+* ``@RUNSTATEDIR@/virtnodedevd.pid``
+
+The PID file to use, unless overridden by the ``-p`` | ``--pid-file`` option.
+
+
+When run as *non-root*
+--
+
+* 

[libvirt PATCH v2 03/16] docs: tweak heading for daemon manual pages

2021-01-05 Thread Daniel P . Berrangé
This group will be distinct from the per-driver modular daemon mapages.

Signed-off-by: Daniel P. Berrangé 
---
 docs/manpages/index.rst | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/docs/manpages/index.rst b/docs/manpages/index.rst
index 3624ae4e8f..e116c6f415 100644
--- a/docs/manpages/index.rst
+++ b/docs/manpages/index.rst
@@ -2,8 +2,10 @@
 Libvirt Manual Pages
 
 
-Daemons
-===
+Common Daemons
+==
+
+These daemons provide functionality across multiple libvirt drivers
 
 * `libvirtd(8) `__ - libvirt management daemon
 * `virtlockd(8) `__ - libvirt lock management daemon
-- 
2.29.2



[libvirt PATCH v2 08/16] docs: add manpage for virtnetworkd

2021-01-05 Thread Daniel P . Berrangé
This is an adaptation of the libvirtd manpage.

Signed-off-by: Daniel P. Berrangé 
---
 docs/manpages/index.rst|   1 +
 docs/manpages/meson.build  |   1 +
 docs/manpages/virtnetworkd.rst | 215 +
 3 files changed, 217 insertions(+)
 create mode 100644 docs/manpages/virtnetworkd.rst

diff --git a/docs/manpages/index.rst b/docs/manpages/index.rst
index 0eab1056e4..2ce222f952 100644
--- a/docs/manpages/index.rst
+++ b/docs/manpages/index.rst
@@ -20,6 +20,7 @@ These daemons provide functionality to a single libvirt driver
 * `virtbhyved(8) `__ - libvirt bhyve management daemon
 * `virtinterfaced(8) `__ - libvirt host network interface 
management daemon
 * `virtlxcd(8) `__ - libvirt LXC management daemon
+* `virtnetworkd(8) `__ - libvirt virtual network management 
daemon
 
 Tools
 =
diff --git a/docs/manpages/meson.build b/docs/manpages/meson.build
index 083289642a..4c294909ee 100644
--- a/docs/manpages/meson.build
+++ b/docs/manpages/meson.build
@@ -27,6 +27,7 @@ docs_man_files = [
   { 'name': 'virtlockd', 'section': '8', 'install': conf.has('WITH_LIBVIRTD') 
},
   { 'name': 'virtlogd', 'section': '8', 'install': conf.has('WITH_LIBVIRTD') },
   { 'name': 'virtlxcd', 'section': '8', 'install': conf.has('WITH_LXC') },
+  { 'name': 'virtnetworkd', 'section': '8', 'install': 
conf.has('WITH_NETWORK') },
   { 'name': 'virtproxyd', 'section': '8', 'install': conf.has('WITH_LIBVIRTD') 
},
 ]
 
diff --git a/docs/manpages/virtnetworkd.rst b/docs/manpages/virtnetworkd.rst
new file mode 100644
index 00..1ef9a12210
--- /dev/null
+++ b/docs/manpages/virtnetworkd.rst
@@ -0,0 +1,215 @@
+
+virtnetworkd
+
+
+-
+libvirt virtual network management daemon
+-
+
+:Manual section: 8
+:Manual group: Virtualization Support
+
+.. contents::
+
+SYNOPSIS
+
+
+``virtnetworkd`` [*OPTION*]...
+
+
+DESCRIPTION
+===
+
+The ``virtnetworkd`` program is a server side daemon component of the libvirt
+virtualization management system.
+
+It is one of a collection of modular daemons that replace functionality
+previously provided by the monolithic ``libvirtd`` daemon.
+
+This daemon runs on virtualization hosts to provide management for virtual
+networks.
+
+The ``virtnetworkd`` daemon only listens for requests on a local Unix domain
+socket. Remote off-host access and backwards compatibility with legacy
+clients expecting ``libvirtd`` is provided by the ``virtproxy`` daemon.
+
+Restarting ``virtnetworkd`` does not interrupt running guests. Guests continue 
to
+operate and changes in their state will generally be picked up automatically
+during startup. None the less it is recommended to avoid restarting with
+running guests whenever practical.
+
+
+SYSTEM SOCKET ACTIVATION
+
+
+The ``virtnetworkd`` daemon is capable of starting in two modes.
+
+In the traditional mode, it will create and listen on UNIX sockets itself.
+
+In socket activation mode, it will rely on systemd to create and listen
+on the UNIX sockets and pass them as pre-opened file descriptors. In this
+mode most of the socket related config options in
+``/etc/libvirt/virtnetworkd.conf`` will no longer have any effect.
+
+Socket activation mode is generally the default when running on a host
+OS that uses systemd. To revert to the traditional mode, all the socket
+unit files must be masked:
+
+::
+
+   $ systemctl mask virtnetworkd.socket virtnetworkd-ro.socket \
+  virtnetworkd-admin.socket
+
+
+OPTIONS
+===
+
+``-h``, ``--help``
+
+Display command line help usage then exit.
+
+``-d``, ``--daemon``
+
+Run as a daemon & write PID file.
+
+``-f``, ``--config *FILE*``
+
+Use this configuration file, overriding the default value.
+
+``-p``, ``--pid-file *FILE*``
+
+Use this name for the PID file, overriding the default value.
+
+``-t``, ``--timeout *SECONDS*``
+
+Exit after timeout period (in seconds), provided there are neither any client
+connections nor any running domains.
+
+``-v``, ``--verbose``
+
+Enable output of verbose messages.
+
+``--version``
+
+Display version information then exit.
+
+
+SIGNALS
+===
+
+On receipt of ``SIGHUP`` ``virtnetworkd`` will reload its configuration.
+
+
+FILES
+=
+
+When run as *root*
+--
+
+* ``@SYSCONFDIR@/libvirt/virtnetworkd.conf``
+
+The default configuration file used by ``virtnetworkd``, unless overridden on 
the
+command line using the ``-f`` | ``--config`` option.
+
+* ``@RUNSTATEDIR@/libvirt/virtnetworkd-sock``
+* ``@RUNSTATEDIR@/libvirt/virtnetworkd-sock-ro``
+* ``@RUNSTATEDIR@/libvirt/virtnetworkd-admin-sock``
+
+The sockets ``virtnetworkd`` will use.
+
+The TLS **Server** private key ``virtnetworkd`` will use.
+
+* ``@RUNSTATEDIR@/virtnetworkd.pid``
+
+The PID file to use, unless overridden by the ``-p`` | ``--pid-file`` option.
+
+
+When run as *non-root*
+--
+
+* 

[libvirt PATCH v2 04/16] docs: add manpage for virtproxyd

2021-01-05 Thread Daniel P . Berrangé
This is an adaptation of the libvirtd manpage.

Signed-off-by: Daniel P. Berrangé 
---
 docs/manpages/index.rst  |   1 +
 docs/manpages/meson.build|   1 +
 docs/manpages/virtproxyd.rst | 256 +++
 3 files changed, 258 insertions(+)
 create mode 100644 docs/manpages/virtproxyd.rst

diff --git a/docs/manpages/index.rst b/docs/manpages/index.rst
index e116c6f415..6a2a1e065d 100644
--- a/docs/manpages/index.rst
+++ b/docs/manpages/index.rst
@@ -10,6 +10,7 @@ These daemons provide functionality across multiple libvirt 
drivers
 * `libvirtd(8) `__ - libvirt management daemon
 * `virtlockd(8) `__ - libvirt lock management daemon
 * `virtlogd(8) `__ - libvirt log management daemon
+* `virtproxyd(8) `__ - libvirt proxy daemon
 
 Tools
 =
diff --git a/docs/manpages/meson.build b/docs/manpages/meson.build
index f6388b6262..7d5a81ecd5 100644
--- a/docs/manpages/meson.build
+++ b/docs/manpages/meson.build
@@ -24,6 +24,7 @@ docs_man_files = [
   { 'name': 'virt-sanlock-cleanup', 'section': '8', 'install': 
conf.has('WITH_SANLOCK') },
   { 'name': 'virtlockd', 'section': '8', 'install': conf.has('WITH_LIBVIRTD') 
},
   { 'name': 'virtlogd', 'section': '8', 'install': conf.has('WITH_LIBVIRTD') },
+  { 'name': 'virtproxyd', 'section': '8', 'install': conf.has('WITH_LIBVIRTD') 
},
 ]
 
 foreach name : keycode_list
diff --git a/docs/manpages/virtproxyd.rst b/docs/manpages/virtproxyd.rst
new file mode 100644
index 00..a8a0c044fa
--- /dev/null
+++ b/docs/manpages/virtproxyd.rst
@@ -0,0 +1,256 @@
+==
+virtproxyd
+==
+
+
+libvirt proxy daemon
+
+
+:Manual section: 8
+:Manual group: Virtualization Support
+
+.. contents::
+
+SYNOPSIS
+
+
+``virtproxyd`` [*OPTION*]...
+
+
+DESCRIPTION
+===
+
+The ``virtproxyd`` program is a server side daemon component of the libvirt
+virtualization management system.
+
+It is one of a collection of modular daemons that replace functionality
+previously provided by the monolithic ``libvirtd`` daemon.
+
+This daemon runs on virtualization hosts an
+
+ * Listens on a UNIX socket to provide backwards compatibility for clients
+   that previously connected to the ``libvirtd`` socket.
+
+ * Optionally listens on TCP ports for connections from off-node clients
+
+Upon receiving RPC messages from a client ``virtproxyd`` will transparently
+forward them on to the appropriate modular daemon, and similarly relay back
+any asynchronous events.
+
+By default, the ``virtproxyd`` daemon listens for requests on a local Unix
+domain socket with the same path previously used by ``libvirtd``.  The
+configuration file can be used to instruct it to also listen on TCP socket(s).
+Systemd socket activation is also supported to allow it to receive pre-opened
+listener sockets on startup.
+
+Since ``virtproxyd`` merely forwards RPC mesages, it has no important state,
+and can be restarted at any time. Clients should expect to reconnect after
+the restart.
+
+
+SYSTEM SOCKET ACTIVATION
+
+
+The ``virtproxyd`` daemon is capable of starting in two modes.
+
+In the traditional mode, it will create and listen on UNIX sockets itself.
+It will also listen on TCP/IP socket(s), according to the ``listen_tcp``
+and ``listen_tls`` options in ``/etc/libvirt/virtproxyd.conf``
+
+In socket activation mode, it will rely on systemd to create and listen
+on the UNIX, and optionally TCP/IP, sockets and pass them as pre-opened
+file descriptors. In this mode most of the socket related config options in
+``/etc/libvirt/virtproxyd.conf`` will no longer have any effect. To enable
+TCP or TLS sockets use either
+
+::
+
+   $ systemctl start virtproxyd-tls.socket
+
+Or
+
+::
+
+   $ systemctl start virtproxyd-tcp.socket
+
+Socket activation mode is generally the default when running on a host
+OS that uses systemd. To revert to the traditional mode, all the socket
+unit files must be masked:
+
+::
+
+   $ systemctl mask virtproxyd.socket virtproxyd-ro.socket \
+  virtproxyd-admin.socket virtproxyd-tls.socket virtproxyd-tcp.socket
+
+
+OPTIONS
+===
+
+``-h``, ``--help``
+
+Display command line help usage then exit.
+
+``-d``, ``--daemon``
+
+Run as a daemon & write PID file.
+
+``-f``, ``--config *FILE*``
+
+Use this configuration file, overriding the default value.
+
+``-p``, ``--pid-file *FILE*``
+
+Use this name for the PID file, overriding the default value.
+
+``-t``, ``--timeout *SECONDS*``
+
+Exit after timeout period (in seconds), provided there are neither any client
+connections nor any running domains.
+
+``-v``, ``--verbose``
+
+Enable output of verbose messages.
+
+``--version``
+
+Display version information then exit.
+
+
+SIGNALS
+===
+
+On receipt of ``SIGHUP`` ``virtproxyd`` will reload its configuration.
+
+
+FILES
+=
+
+When run as *root*
+--
+
+* ``@SYSCONFDIR@/libvirt/virtproxyd.conf``
+
+The default configuration file used by ``virtproxyd``, 

[libvirt PATCH v2 05/16] docs: add manpage for virtbhyved

2021-01-05 Thread Daniel P . Berrangé
This is an adaptation of the libvirtd manpage.

Signed-off-by: Daniel P. Berrangé 
---
 docs/manpages/index.rst  |   7 ++
 docs/manpages/meson.build|   1 +
 docs/manpages/virtbhyved.rst | 193 +++
 3 files changed, 201 insertions(+)
 create mode 100644 docs/manpages/virtbhyved.rst

diff --git a/docs/manpages/index.rst b/docs/manpages/index.rst
index 6a2a1e065d..da835d62ec 100644
--- a/docs/manpages/index.rst
+++ b/docs/manpages/index.rst
@@ -12,6 +12,13 @@ These daemons provide functionality across multiple libvirt 
drivers
 * `virtlogd(8) `__ - libvirt log management daemon
 * `virtproxyd(8) `__ - libvirt proxy daemon
 
+Modular Driver daemons
+==
+
+These daemons provide functionality to a single libvirt driver
+
+* `virtbhyved(8) `__ - libvirt bhyve management daemon
+
 Tools
 =
 
diff --git a/docs/manpages/meson.build b/docs/manpages/meson.build
index 7d5a81ecd5..7c03cb74cf 100644
--- a/docs/manpages/meson.build
+++ b/docs/manpages/meson.build
@@ -22,6 +22,7 @@ docs_man_files = [
 
   { 'name': 'libvirtd', 'section': '8', 'install': conf.has('WITH_LIBVIRTD') },
   { 'name': 'virt-sanlock-cleanup', 'section': '8', 'install': 
conf.has('WITH_SANLOCK') },
+  { 'name': 'virtbhyved', 'section': '8', 'install': conf.has('WITH_BHYVE') },
   { 'name': 'virtlockd', 'section': '8', 'install': conf.has('WITH_LIBVIRTD') 
},
   { 'name': 'virtlogd', 'section': '8', 'install': conf.has('WITH_LIBVIRTD') },
   { 'name': 'virtproxyd', 'section': '8', 'install': conf.has('WITH_LIBVIRTD') 
},
diff --git a/docs/manpages/virtbhyved.rst b/docs/manpages/virtbhyved.rst
new file mode 100644
index 00..9fdaca6da2
--- /dev/null
+++ b/docs/manpages/virtbhyved.rst
@@ -0,0 +1,193 @@
+==
+virtbhyved
+==
+
+---
+libvirt bhyve management daemon
+---
+
+:Manual section: 8
+:Manual group: Virtualization Support
+
+.. contents::
+
+SYNOPSIS
+
+
+``virtbhyved`` [*OPTION*]...
+
+
+DESCRIPTION
+===
+
+The ``virtbhyved`` program is a server side daemon component of the libvirt
+virtualization management system.
+
+It is one of a collection of modular daemons that replace functionality
+previously provided by the monolithic ``libvirtd`` daemon.
+
+This daemon runs on virtualization hosts to provide management for bhyve 
virtual
+machines.
+
+The ``virtbhyved`` daemon only listens for requests on a local Unix domain
+socket. Remote off-host access and backwards compatibility with legacy
+clients expecting ``libvirtd`` is provided by the ``virtproxy`` daemon.
+
+Restarting ``virtbhyved`` does not interrupt running guests. Guests continue to
+operate and changes in their state will generally be picked up automatically
+during startup. None the less it is recommended to avoid restarting with
+running guests whenever practical.
+
+
+OPTIONS
+===
+
+``-h``, ``--help``
+
+Display command line help usage then exit.
+
+``-d``, ``--daemon``
+
+Run as a daemon & write PID file.
+
+``-f``, ``--config *FILE*``
+
+Use this configuration file, overriding the default value.
+
+``-p``, ``--pid-file *FILE*``
+
+Use this name for the PID file, overriding the default value.
+
+``-t``, ``--timeout *SECONDS*``
+
+Exit after timeout period (in seconds), provided there are neither any client
+connections nor any running domains.
+
+``-v``, ``--verbose``
+
+Enable output of verbose messages.
+
+``--version``
+
+Display version information then exit.
+
+
+SIGNALS
+===
+
+On receipt of ``SIGHUP`` ``virtbhyved`` will reload its configuration.
+
+
+FILES
+=
+
+When run as *root*
+--
+
+* ``@SYSCONFDIR@/libvirt/virtbhyved.conf``
+
+The default configuration file used by ``virtbhyved``, unless overridden on the
+command line using the ``-f`` | ``--config`` option.
+
+* ``@RUNSTATEDIR@/libvirt/virtbhyved-sock``
+* ``@RUNSTATEDIR@/libvirt/virtbhyved-sock-ro``
+* ``@RUNSTATEDIR@/libvirt/virtbhyved-admin-sock``
+
+The sockets ``virtbhyved`` will use.
+
+The TLS **Server** private key ``virtbhyved`` will use.
+
+* ``@RUNSTATEDIR@/virtbhyved.pid``
+
+The PID file to use, unless overridden by the ``-p`` | ``--pid-file`` option.
+
+
+When run as *non-root*
+--
+
+* ``$XDG_CONFIG_HOME/libvirt/virtbhyved.conf``
+
+The default configuration file used by ``virtbhyved``, unless overridden on the
+command line using the ``-f``|``--config`` option.
+
+* ``$XDG_RUNTIME_DIR/libvirt/virtbhyved-sock``
+* ``$XDG_RUNTIME_DIR/libvirt/virtbhyved-admin-sock``
+
+The sockets ``virtbhyved`` will use.
+
+* ``$XDG_RUNTIME_DIR/libvirt/virtbhyved.pid``
+
+The PID file to use, unless overridden by the ``-p``|``--pid-file`` option.
+
+
+If ``$XDG_CONFIG_HOME`` is not set in your environment, ``virtbhyved`` will use
+``$HOME/.config``
+
+If ``$XDG_RUNTIME_DIR`` is not set in your environment, ``virtbhyved`` will use
+``$HOME/.cache``
+
+
+EXAMPLES
+
+
+To retrieve the version of ``virtbhyved``:

[libvirt PATCH v2 02/16] docs: don't hardcode an ancient version in manpage examples

2021-01-05 Thread Daniel P . Berrangé
Subsitute in the current version so the example always reflect today's
version of reality.

Signed-off-by: Daniel P. Berrangé 
---
 docs/manpages/libvirtd.rst  | 2 +-
 docs/manpages/meson.build   | 1 +
 docs/manpages/virtlockd.rst | 2 +-
 docs/manpages/virtlogd.rst  | 2 +-
 4 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/docs/manpages/libvirtd.rst b/docs/manpages/libvirtd.rst
index 2ce6284b3a..ed591f4778 100644
--- a/docs/manpages/libvirtd.rst
+++ b/docs/manpages/libvirtd.rst
@@ -202,7 +202,7 @@ To retrieve the version of ``libvirtd``:
 ::
 
   # libvirtd --version
-  libvirtd (libvirt) 0.8.2
+  libvirtd (libvirt) @VERSION@
 
 
 To start ``libvirtd``, instructing it to daemonize and create a PID file:
diff --git a/docs/manpages/meson.build b/docs/manpages/meson.build
index 7156c10cc6..f6388b6262 100644
--- a/docs/manpages/meson.build
+++ b/docs/manpages/meson.build
@@ -71,6 +71,7 @@ endforeach
 docs_man_conf = configuration_data()
 docs_man_conf.set('SYSCONFDIR', sysconfdir)
 docs_man_conf.set('RUNSTATEDIR', runstatedir)
+docs_man_conf.set('VERSION', meson.project_version())
 
 foreach data : docs_man_files
   rst_in_file = '@0@.rst'.format(data['name'])
diff --git a/docs/manpages/virtlockd.rst b/docs/manpages/virtlockd.rst
index 8ca231d14f..0bbee5a5f7 100644
--- a/docs/manpages/virtlockd.rst
+++ b/docs/manpages/virtlockd.rst
@@ -123,7 +123,7 @@ To retrieve the version of ``virtlockd``:
 ::
 
   # virtlockd --version
-  virtlockd (libvirt) 1.1.1
+  virtlockd (libvirt) @VERSION@
 
 To start ``virtlockd``, instructing it to daemonize and create a PID file:
 
diff --git a/docs/manpages/virtlogd.rst b/docs/manpages/virtlogd.rst
index 4b749fe18e..1e39ff1b49 100644
--- a/docs/manpages/virtlogd.rst
+++ b/docs/manpages/virtlogd.rst
@@ -124,7 +124,7 @@ To retrieve the version of ``virtlogd``:
 ::
 
   # virtlogd --version
-  virtlogd (libvirt) 1.1.1
+  virtlogd (libvirt) @VERSION@
 
 To start ``virtlogd``, instructing it to daemonize and create a PID file:
 
-- 
2.29.2



[libvirt PATCH v2 01/16] docs: consistently mark libvirtd as preformatted text

2021-01-05 Thread Daniel P . Berrangé
Signed-off-by: Daniel P. Berrangé 
---
 docs/manpages/libvirtd.rst | 48 +++---
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/docs/manpages/libvirtd.rst b/docs/manpages/libvirtd.rst
index cd9976c5e4..2ce6284b3a 100644
--- a/docs/manpages/libvirtd.rst
+++ b/docs/manpages/libvirtd.rst
@@ -32,13 +32,13 @@ The libvirt client libraries and utilities connect to this 
daemon to issue
 tasks and collect information about the configuration and resources of the host
 system and guests.
 
-By default, the libvirtd daemon listens for requests on a local Unix domain
-socket.  Using the ``-l`` | ``--listen`` command line option, the libvirtd 
daemon
-can be instructed to additionally listen on a TCP/IP socket.  The TCP/IP socket
-to use is defined in the libvirtd configuration file.
+By default, the ``libvirtd`` daemon listens for requests on a local Unix domain
+socket.  Using the ``-l`` | ``--listen`` command line option, the ``libvirtd``
+daemon can be instructed to additionally listen on a TCP/IP socket.  The TCP/IP
+socket to use is defined in the ``libvirtd`` configuration file.
 
-Restarting libvirtd does not impact running guests.  Guests continue to operate
-and will be picked up automatically if their XML configuration has been
+Restarting ``libvirtd`` does not impact running guests.  Guests continue to
+operate and will be picked up automatically if their XML configuration has been
 defined.  Any guests whose XML configuration has not been defined will be lost
 from the configuration.
 
@@ -98,8 +98,8 @@ Use this configuration file, overriding the default value.
 ``-l``, ``--listen``
 
 Listen for TCP/IP connections. This should not be set if using systemd
-socket activation. Instead activate the libvirtd-tls.socket or
-libvirtd-tcp.socket unit files.
+socket activation. Instead activate the ``libvirtd-tls.socket`` or
+``libvirtd-tcp.socket`` unit files.
 
 ``-p``, ``--pid-file *FILE*``
 
@@ -122,7 +122,7 @@ Display version information then exit.
 SIGNALS
 ===
 
-On receipt of ``SIGHUP`` libvirtd will reload its configuration.
+On receipt of ``SIGHUP`` ``libvirtd`` will reload its configuration.
 
 
 FILES
@@ -133,25 +133,25 @@ When run as *root*
 
 * ``@SYSCONFDIR@/libvirt/libvirtd.conf``
 
-The default configuration file used by libvirtd, unless overridden on the
+The default configuration file used by ``libvirtd``, unless overridden on the
 command line using the ``-f`` | ``--config`` option.
 
 * ``@RUNSTATEDIR@/libvirt/libvirt-sock``
 * ``@RUNSTATEDIR@/libvirt/libvirt-sock-ro``
 
-The sockets libvirtd will use.
+The sockets ``libvirtd`` will use.
 
 * ``@SYSCONFDIR@/pki/CA/cacert.pem``
 
-The TLS **Certificate Authority** certificate libvirtd will use.
+The TLS **Certificate Authority** certificate ``libvirtd`` will use.
 
 * ``@SYSCONFDIR@/pki/libvirt/servercert.pem``
 
-The TLS **Server** certificate libvirtd will use.
+The TLS **Server** certificate ``libvirtd`` will use.
 
 * ``@SYSCONFDIR@/pki/libvirt/private/serverkey.pem``
 
-The TLS **Server** private key libvirtd will use.
+The TLS **Server** private key ``libvirtd`` will use.
 
 * ``@RUNSTATEDIR@/libvirtd.pid``
 
@@ -163,41 +163,41 @@ When run as *non-root*
 
 * ``$XDG_CONFIG_HOME/libvirt/libvirtd.conf``
 
-The default configuration file used by libvirtd, unless overridden on the
+The default configuration file used by ``libvirtd``, unless overridden on the
 command line using the ``-f``|``--config`` option.
 
 * ``$XDG_RUNTIME_DIR/libvirt/libvirt-sock``
 
-The socket libvirtd will use.
+The socket ``libvirtd`` will use.
 
 * ``$HOME/.pki/libvirt/cacert.pem``
 
-The TLS **Certificate Authority** certificate libvirtd will use.
+The TLS **Certificate Authority** certificate ``libvirtd`` will use.
 
 * ``$HOME/.pki/libvirt/servercert.pem``
 
-The TLS **Server** certificate libvirtd will use.
+The TLS **Server** certificate ``libvirtd`` will use.
 
 * ``$HOME/.pki/libvirt/serverkey.pem``
 
-The TLS **Server** private key libvirtd will use.
+The TLS **Server** private key ``libvirtd`` will use.
 
 * ``$XDG_RUNTIME_DIR/libvirt/libvirtd.pid``
 
 The PID file to use, unless overridden by the ``-p``|``--pid-file`` option.
 
 
-If ``$XDG_CONFIG_HOME`` is not set in your environment, libvirtd will use
+If ``$XDG_CONFIG_HOME`` is not set in your environment, ``libvirtd`` will use
 ``$HOME/.config``
 
-If ``$XDG_RUNTIME_DIR`` is not set in your environment, libvirtd will use
+If ``$XDG_RUNTIME_DIR`` is not set in your environment, ``libvirtd`` will use
 ``$HOME/.cache``
 
 
 EXAMPLES
 
 
-To retrieve the version of libvirtd:
+To retrieve the version of ``libvirtd``:
 
 ::
 
@@ -205,7 +205,7 @@ To retrieve the version of libvirtd:
   libvirtd (libvirt) 0.8.2
 
 
-To start libvirtd, instructing it to daemonize and create a PID file:
+To start ``libvirtd``, instructing it to daemonize and create a PID file:
 
 ::
 
@@ -246,7 +246,7 @@ libvirt AUTHORS file.
 LICENSE
 ===
 
-libvirtd is 

[libvirt PATCH v2 00/16] docs: add manpages for all the modular daemons

2021-01-05 Thread Daniel P . Berrangé
Most of the modular daemon stuff has been done from a single template,
but for the man pages that is more trouble than it is worth, so we
create a separate man page source for each daemon, which makes it easy
to extend with driver specific information.

Changed in v2:

 - BHyve man page has had references to systemd removed.

Daniel P. Berrangé (16):
  docs: consistently mark libvirtd as preformatted text
  docs: don't hardcode an ancient version in manpage examples
  docs: tweak heading for daemon manual pages
  docs: add manpage for virtproxyd
  docs: add manpage for virtbhyved
  docs: add manpage for virtinterfaced
  docs: add manpage for virtlxcd
  docs: add manpage for virtnetworkd
  docs: add manpage for virtnodedevd
  docs: add manpage for virtnwfilterd
  docs: add manpage for virtqemud
  docs: add manpage for virtsecretd
  docs: add manpage for virtstoraged
  docs: add manpage for virtvboxd
  docs: add manpage for virtvzd
  docs: add manpage for virtxend

 docs/manpages/index.rst  |  25 ++-
 docs/manpages/libvirtd.rst   |  50 +++---
 docs/manpages/meson.build|  14 ++
 docs/manpages/virtbhyved.rst | 193 +++
 docs/manpages/virtinterfaced.rst | 215 ++
 docs/manpages/virtlockd.rst  |   2 +-
 docs/manpages/virtlogd.rst   |   2 +-
 docs/manpages/virtlxcd.rst   | 215 ++
 docs/manpages/virtnetworkd.rst   | 215 ++
 docs/manpages/virtnodedevd.rst   | 214 ++
 docs/manpages/virtnwfilterd.rst  | 215 ++
 docs/manpages/virtproxyd.rst | 256 +++
 docs/manpages/virtqemud.rst  | 215 ++
 docs/manpages/virtsecretd.rst| 214 ++
 docs/manpages/virtstoraged.rst   | 215 ++
 docs/manpages/virtvboxd.rst  | 213 +
 docs/manpages/virtvzd.rst| 215 ++
 docs/manpages/virtxend.rst   | 215 ++
 18 files changed, 2874 insertions(+), 29 deletions(-)
 create mode 100644 docs/manpages/virtbhyved.rst
 create mode 100644 docs/manpages/virtinterfaced.rst
 create mode 100644 docs/manpages/virtlxcd.rst
 create mode 100644 docs/manpages/virtnetworkd.rst
 create mode 100644 docs/manpages/virtnodedevd.rst
 create mode 100644 docs/manpages/virtnwfilterd.rst
 create mode 100644 docs/manpages/virtproxyd.rst
 create mode 100644 docs/manpages/virtqemud.rst
 create mode 100644 docs/manpages/virtsecretd.rst
 create mode 100644 docs/manpages/virtstoraged.rst
 create mode 100644 docs/manpages/virtvboxd.rst
 create mode 100644 docs/manpages/virtvzd.rst
 create mode 100644 docs/manpages/virtxend.rst

-- 
2.29.2




Re: [libvirt PATCH v3 08/21] nodedev: add helper functions to remove node devices

2021-01-05 Thread Erik Skultety
On Thu, Dec 24, 2020 at 08:14:32AM -0600, Jonathon Jongsma wrote:
> When a mediated device is stopped or undefined by an application outside
> of libvirt, we need to remove it from our list of node devices within
> libvirt. This patch introduces virNodeDeviceObjListRemoveLocked() and
> virNodeDeviceObjListForEach() (which are analogous to other types of
> object lists in libvirt) to facilitate that. They will be used in coming
> commits.
> 
> Signed-off-by: Jonathon Jongsma 
> ---
Reviewed-by: Erik Skultety 



Re: [PATCH] qemu: backup: Properly delete temporary bitmap after push-mode incremental backup

2021-01-05 Thread Ján Tomko

On a Tuesday in 2021, Peter Krempa wrote:

Refactor in 0316c28a453ac used incorrect source variable to initialize
the variable which holds the name of the bitmap which needs to be
deleted after the backup job finishes. This resulted into deleting the
source bitmap of the backup rather than the temporary one.

Use 'dd->incrementalBitmap' which holds the temporary bitmap name
instead of 'dd->backupdisk->incremental' which holds the name of the
source bitmap which is used by the backup.

Fixes: 0316c28a453ac15f58c61f30359f66ab9a649884
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1908647
Signed-off-by: Peter Krempa 
---
src/qemu/qemu_backup.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature


Re: [libvirt PATCH v3 06/21] nodedev: add STOPPED/STARTED lifecycle events

2021-01-05 Thread Erik Skultety
On Thu, Dec 24, 2020 at 08:14:30AM -0600, Jonathon Jongsma wrote:
> Since a mediated device can be persistently defined by the mdevctl
> backend, we need additional lifecycle events beyond CREATED/DELETED to
> indicate that e.g. the device has been stopped but the device definition
> still exists.
> 
> Signed-off-by: Jonathon Jongsma 
> ---
>  examples/c/misc/event-test.c |  4 
>  include/libvirt/libvirt-nodedev.h|  2 ++
>  src/conf/node_device_conf.h  |  1 +
>  src/node_device/node_device_driver.c |  1 +
>  src/node_device/node_device_udev.c   | 25 +++--
>  tools/virsh-nodedev.c|  4 +++-
>  6 files changed, 34 insertions(+), 3 deletions(-)
> 
> diff --git a/examples/c/misc/event-test.c b/examples/c/misc/event-test.c
> index f164e825e1..d6eec648ec 100644
> --- a/examples/c/misc/event-test.c
> +++ b/examples/c/misc/event-test.c
> @@ -381,6 +381,10 @@ nodeDeviceEventToString(int event)
>  return "Created";
>  case VIR_NODE_DEVICE_EVENT_DELETED:
>  return "Deleted";
> +case VIR_NODE_DEVICE_EVENT_STOPPED:
> +return "Stopped";
> +case VIR_NODE_DEVICE_EVENT_STARTED:
> +return "Started";
>  case VIR_NODE_DEVICE_EVENT_LAST:
>  break;
>  }
> diff --git a/include/libvirt/libvirt-nodedev.h 
> b/include/libvirt/libvirt-nodedev.h
> index d304283871..a473563857 100644
> --- a/include/libvirt/libvirt-nodedev.h
> +++ b/include/libvirt/libvirt-nodedev.h
> @@ -197,6 +197,8 @@ int virConnectNodeDeviceEventDeregisterAny(virConnectPtr 
> conn,
>  typedef enum {
>  VIR_NODE_DEVICE_EVENT_CREATED = 0,
>  VIR_NODE_DEVICE_EVENT_DELETED = 1,
> +VIR_NODE_DEVICE_EVENT_STOPPED = 2,
> +VIR_NODE_DEVICE_EVENT_STARTED = 3,
>  
>  # ifdef VIR_ENUM_SENTINELS
>  VIR_NODE_DEVICE_EVENT_LAST
> diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
> index 3d7872fd6e..bbc28cf2b9 100644
> --- a/src/conf/node_device_conf.h
> +++ b/src/conf/node_device_conf.h
> @@ -157,6 +157,7 @@ struct _virNodeDevCapMdev {
>  char *uuid;
>  virMediatedDeviceAttrPtr *attributes;
>  size_t nattributes;
> +bool persistent;

So, this patch is essentially unchanged since v2. In v2 I suggested ^this
attribute should be bound to the object not the capabability and you haven't
replied to those comments, is there a reason why that is not the case? It's
also consistent with what we do for other objects, like domains and networks.

...

>  
>  if (!(obj = virNodeDeviceObjListFindBySysfsPath(driver->devs, path))) {
>  VIR_DEBUG("Failed to find device to remove that has udev path '%s'",
> @@ -1409,13 +1411,32 @@ udevRemoveOneDeviceSysPath(const char *path)
>  }
>  def = virNodeDeviceObjGetDef(obj);
>  
> +/* If the device is a mediated device that has been 'stopped', it may 
> still
> + * be defined by mdevctl and can therefore be started again. Don't drop 
> it
> + * from the list of node devices */
> +cap = def->caps;
> +while (cap != NULL) {
> +if (cap->data.type == VIR_NODE_DEV_CAP_MDEV) {
> +if (cap->data.mdev.persistent) {
> +VIR_FREE(def->sysfs_path);
> +event_type = VIR_NODE_DEVICE_EVENT_STOPPED;
> +break;
> +}
> +}
> +cap = cap->next;
> +}
> +

...which would simplify ^this while loop to a mere:
if (obj->persitent) {
VIR_FREE(def->sysfs_path);
virNodeDeviceObjSetActive(obj, false);
}

>  event = virNodeDeviceEventLifecycleNew(def->name,
> -   VIR_NODE_DEVICE_EVENT_DELETED,
> +   event_type,
> 0);
>  
>  VIR_DEBUG("Removing device '%s' with sysfs path '%s'",
>def->name, path);
> -virNodeDeviceObjListRemove(driver->devs, obj);
> +
> +if (event_type == VIR_NODE_DEVICE_EVENT_DELETED)
> +virNodeDeviceObjListRemove(driver->devs, obj);
> +else
> +virNodeDeviceObjSetActive(obj, false);

...and this 'else' branch would be unnecessary then.

>  virNodeDeviceObjEndAPI();
>  
>  virObjectEventStateQueue(driver->nodeDeviceEventState, event);

Regards,
Erik



Re: [PATCH v2 0/3] Storage: remove unused variable

2021-01-05 Thread Michal Privoznik

On 1/5/21 3:43 PM, Yi Li wrote:

refactor storageBackendCreateRaw and remove some unused variable.

Yi Li (3):
   createRawFile: remove unused return variable
   virStorageBackendCopyToFD: remove unused return variable
   storageBackendCreateRaw: remove unused created

  src/storage/storage_util.c | 66 +++---
  1 file changed, 25 insertions(+), 41 deletions(-)



Reviewed-by: Michal Privoznik 

and pushed.

Michal



Re: [PATCH v2 3/3] storageBackendCreateRaw: remove unused created

2021-01-05 Thread Michal Privoznik

On 1/5/21 3:43 PM, Yi Li wrote:

refactor and remove unused created variable.

Signed-off-by: Yi Li 
---
  src/storage/storage_util.c | 23 ++-
  1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index c6d0f7a97c..cc8189c3e0 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -384,11 +384,9 @@ storageBackendCreateRaw(virStoragePoolObjPtr pool,
  unsigned int flags)
  {
  virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
-int ret = -1;
  int operation_flags;
  bool reflink_copy = false;
  mode_t open_mode = VIR_STORAGE_DEFAULT_VOL_PERM_MODE;
-bool created = false;
  VIR_AUTOCLOSE fd = -1;
  
  virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA |

@@ -399,13 +397,13 @@ storageBackendCreateRaw(virStoragePoolObjPtr pool,
  virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
 _("metadata preallocation is not supported for raw "
   "volumes"));
-goto cleanup;
+return -1;
  }
  
  if (virStorageSourceHasBacking(>target)) {

  virReportError(VIR_ERR_NO_SUPPORT, "%s",
 _("backing storage not supported for raw volumes"));
-goto cleanup;
+return -1;
  }
  
  if (flags & VIR_STORAGE_VOL_CREATE_REFLINK)

@@ -415,7 +413,7 @@ storageBackendCreateRaw(virStoragePoolObjPtr pool,
  if (vol->target.encryption) {
  virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
 _("storage pool does not support encrypted volumes"));
-goto cleanup;
+return -1;
  }
  
  operation_flags = VIR_FILE_OPEN_FORCE_MODE | VIR_FILE_OPEN_FORCE_OWNER;

@@ -434,26 +432,25 @@ storageBackendCreateRaw(virStoragePoolObjPtr pool,
  virReportSystemError(-fd,
   _("Failed to create file '%s'"),
   vol->target.path);
-goto cleanup;
+return -1;
  }
-created = true;
  
  /* NB, COW flag can only be toggled when the file is zero-size,

   * so must go before the createRawFile call allocates payload */
  if (vol->target.nocow &&
  virFileSetCOW(vol->target.path, VIR_TRISTATE_BOOL_NO) < 0)
-goto cleanup;
+goto error;
  
-if ((ret = createRawFile(fd, vol, inputvol, reflink_copy)) < 0)

+if (createRawFile(fd, vol, inputvol, reflink_copy) < 0)
  /* createRawFile already reported the exact error. */
-ret = -1;
+goto error;


Since you're touching these lines, they deserve to be wrapped in curly 
braces - any body with two or more lines (including comments) must be, 
according to our coding style.



+return 0;
  
- cleanup:

-if (ret < 0 && created)
+ error:
  ignore_value(virFileRemove(vol->target.path,
 vol->target.perms->uid,
 vol->target.perms->gid));


This should be re-aligned. And the ignore_value() is needless - 
virFileRemove() is not declared with G_GNUC_WARN_UNUSED_RESULT and never 
was.



-return ret;
+return -1;
  }
  
  



I'm fixing small nits I've found locally before pushing.

Michal



Re: [libvirt PATCH v3 05/21] nodedev: add ability to list and parse defined mdevs

2021-01-05 Thread Erik Skultety
On Thu, Dec 24, 2020 at 08:14:29AM -0600, Jonathon Jongsma wrote:
> This adds some internal API to query for persistent mediated devices
> that are defined by mdevctl. Following commits will make use of this
> information. This just provides the infrastructure and tests for this
> feature. One test verifies that we are executing mdevctl with the proper
> arguments, and the other test verifies that we can parse the returned
> JSON and convert it to our own internal node device representations.
> 
> Signed-off-by: Jonathon Jongsma 
> ---
>  src/node_device/node_device_driver.c  | 150 ++
>  src/node_device/node_device_driver.h  |   7 +
>  .../mdevctl-list-defined.argv |   1 +
>  .../mdevctl-list-multiple.json|  59 +++
>  .../mdevctl-list-multiple.out.xml |  39 +
>  tests/nodedevmdevctltest.c|  95 ++-
>  6 files changed, 349 insertions(+), 2 deletions(-)
>  create mode 100644 tests/nodedevmdevctldata/mdevctl-list-defined.argv
>  create mode 100644 tests/nodedevmdevctldata/mdevctl-list-multiple.json
>  create mode 100644 tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml
> 
> diff --git a/src/node_device/node_device_driver.c 
> b/src/node_device/node_device_driver.c
> index 6143459618..bbd373e32e 100644
> --- a/src/node_device/node_device_driver.c
> +++ b/src/node_device/node_device_driver.c
> @@ -853,6 +853,156 @@ virMdevctlStop(virNodeDeviceDefPtr def)
>  }
>  
>  
> +virCommandPtr
> +nodeDeviceGetMdevctlListCommand(bool defined,
> +char **output)
> +{
> +virCommandPtr cmd = virCommandNewArgList(MDEVCTL,
> + "list",
> + "--dumpjson",
> + NULL);
> +
> +if (defined)
> +virCommandAddArg(cmd, "--defined");
> +
> +virCommandSetOutputBuffer(cmd, output);
> +
> +return cmd;
> +}
> +

At this point I think we could split this even further and add the parser code
first and then introduce listing support.

...

> +int
> +nodeDeviceParseMdevctlJSON(const char *jsonstring,
> +   virNodeDeviceDefPtr **devs)
> +{
> +int n;
> +g_autoptr(virJSONValue) json_devicelist = NULL;
> +virNodeDeviceDefPtr *outdevs = NULL;
> +size_t noutdevs = 0;
> +size_t i, j;

One declaration per line please (I've seen another one, but not sure if it
was in this patch or some later one).

> +
> +json_devicelist = virJSONValueFromString(jsonstring);
> +
> +if (!json_devicelist)
> +goto parsefailure;
> +
> +if (!virJSONValueIsArray(json_devicelist))
> +goto parsefailure;
> +
> +n = virJSONValueArraySize(json_devicelist);
> +
> +for (i = 0; i < n; i++) {
> +virJSONValuePtr obj = virJSONValueArrayGet(json_devicelist, i);
> +const char *parent;
> +virJSONValuePtr child_array;
> +int nchildren;
> +
> +if (!virJSONValueIsObject(obj))
> +goto parsefailure;
> +
> +/* mdevctl returns an array of objects.  Each object is a parent 
> device
> + * object containing a single key-value pair which maps from the name
> + * of the parent device to an array of child devices */
> +if (virJSONValueObjectKeysNumber(obj) != 1)
> +goto parsefailure;
> +
> +parent = virJSONValueObjectGetKey(obj, 0);
> +child_array = virJSONValueObjectGetValue(obj, 0);
> +
> +if (!virJSONValueIsArray(child_array))
> +goto parsefailure;
> +
> +nchildren = virJSONValueArraySize(child_array);
> +
> +for (j = 0; j < nchildren; j++) {
> +g_autoptr(virNodeDeviceDef) child = NULL;
> +virJSONValuePtr child_obj = virJSONValueArrayGet(child_array, j);
> +
> +if (!(child = nodeDeviceParseMdevctlChildDevice(parent, 
> child_obj)))
> +goto parsefailure;
> +
> +if (VIR_APPEND_ELEMENT(outdevs, noutdevs, child) < 0)
> +goto parsefailure;
> +}
> +}
> +
> +*devs = outdevs;
> +return noutdevs;
> +
> + parsefailure:

As pointed out in v2:

s/parsefailure/error

> +for (i = 0; i < noutdevs; i++)
> +virNodeDeviceDefFree(outdevs[i]);
> +VIR_FREE(outdevs);
> +
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +   _("unable to parse JSON response"));
> +return -1;
> +}
> +
> +

...

> --- /dev/null
> +++ b/tests/nodedevmdevctldata/mdevctl-list-multiple.json
> @@ -0,0 +1,59 @@
> +[
> +  {
> +":00:02.0": [
> +  {
> +"200f228a-c80a-4d50-bfb7-f5a0e4e34045": {
> +  "mdev_type": "i915-GVTg_V5_4",
> +  "start": "manual"
> +}
> +  },
> +  {
> +"de807ffc-1923-4d5f-b6c9-b20ecebc6d4b": {
> +  "mdev_type": "i915-GVTg_V5_4",
> +  "start": "auto"
> +}
> +  },
> +  {
> +

Re: [PATCH 4/4] netlink: Introduce a helper function to simplify netlink functions

2021-01-05 Thread Michal Privoznik

On 12/21/20 4:23 AM, Shi Lei wrote:

Extract common code as helper function virNetlinkTalk, then simplify
the functions virNetlink[DumpLink|NewLink|DelLink|GetNeighbor].

Signed-off-by: Shi Lei 
---
  src/util/virnetlink.c | 210 +++---
  src/util/virnetlink.h |   4 +-
  2 files changed, 78 insertions(+), 136 deletions(-)


Nice cleanup. Patches 1-3 look good.



diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
index fdcb0dc0..7bea38c0 100644
--- a/src/util/virnetlink.c
+++ b/src/util/virnetlink.c
@@ -353,6 +353,48 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
  return 0;
  }
  
+

+static int
+virNetlinkTalk(const char *ifname,
+   virNetlinkMsg *nl_msg,
+   uint32_t src_pid,
+   uint32_t dst_pid,
+   struct nlmsghdr **resp,
+   unsigned int *resp_len,
+   int *error,
+   virNetlinkTalkFallback fallback)
+{
+if (virNetlinkCommand(nl_msg, resp, resp_len,
+  src_pid, dst_pid, NETLINK_ROUTE, 0) < 0)
+return -1;
+
+if (*resp_len < NLMSG_LENGTH(0) || resp == NULL)
+goto malformed_resp;
+
+if ((*resp)->nlmsg_type == NLMSG_ERROR) {
+struct nlmsgerr *err = (struct nlmsgerr *) NLMSG_DATA(resp);
+if ((*resp)->nlmsg_len < NLMSG_LENGTH(sizeof(*err)))
+goto malformed_resp;
+
+if (-err->error == EOPNOTSUPP && fallback)
+return fallback(ifname);
+
+if (err->error < 0) {
+if (error)
+*error = err->error;


I wonder whether we should report an error here. I mean, if err->error 
is set (and not EOPNOTSUPP) then it's stored into *error, good. But -1 
is returned ...



+return -1;


.. here and it's indistinguishable to the caller from -1 returned in 
'malformed_resp'. Looking at the usage in next hunks, how about:


  if (error)
*error = err->error;
  else
virReportSystemError(-err->error, ...);

  return -1;


+}
+}
+
+return 0;
+
+ malformed_resp:
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("malformed netlink response message"));
+return -1;
+}
+
+
  int
  virNetlinkDumpCommand(struct nl_msg *nl_msg,
virNetlinkDumpCallback callback,
@@ -396,6 +438,7 @@ virNetlinkDumpCommand(struct nl_msg *nl_msg,
  return 0;
  }
  
+

  /**
   * virNetlinkDumpLink:
   *
@@ -420,15 +463,13 @@ virNetlinkDumpLink(const char *ifname, int ifindex,
 void **nlData, struct nlattr **tb,
 uint32_t src_pid, uint32_t dst_pid)
  {
-int rc = -1;
-struct nlmsgerr *err;
  struct ifinfomsg ifinfo = {
  .ifi_family = AF_UNSPEC,
  .ifi_index  = ifindex
  };
-unsigned int recvbuflen;
  g_autoptr(virNetlinkMsg) nl_msg = NULL;
  g_autofree struct nlmsghdr *resp = NULL;
+unsigned int resp_len = 0;
  
  if (ifname && ifindex <= 0 && virNetDevGetIndex(ifname, ) < 0)

  return -1;
@@ -459,46 +500,19 @@ virNetlinkDumpLink(const char *ifname, int ifindex,
  }
  # endif
  
-if (virNetlinkCommand(nl_msg, , ,

-  src_pid, dst_pid, NETLINK_ROUTE, 0) < 0)
+if (virNetlinkTalk(ifname, nl_msg, src_pid, dst_pid,
+   , _len, NULL, NULL) < 0)
  return -1;
  
-if (recvbuflen < NLMSG_LENGTH(0) || resp == NULL)

-goto malformed_resp;
-
-switch (resp->nlmsg_type) {
-case NLMSG_ERROR:
-err = (struct nlmsgerr *)NLMSG_DATA(resp);
-if (resp->nlmsg_len < NLMSG_LENGTH(sizeof(*err)))
-goto malformed_resp;
-
-if (err->error) {
-virReportSystemError(-err->error,
- _("error dumping %s (%d) interface"),
- ifname, ifindex);



Here we'd report an error.


-return -1;
-}
-break;
-
-case GENL_ID_CTRL:
-case NLMSG_DONE:
-rc = nlmsg_parse(resp, sizeof(struct ifinfomsg),
- tb, IFLA_MAX, NULL);
-if (rc < 0)
-goto malformed_resp;
-break;
-
-default:
-goto malformed_resp;
+if ((resp->nlmsg_type != GENL_ID_CTRL && resp->nlmsg_type != NLMSG_DONE) ||
+nlmsg_parse(resp, sizeof(struct ifinfomsg), tb, IFLA_MAX, NULL) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("malformed netlink response message"));
+return -1;
  }
  
  *nlData = g_steal_pointer();

  return 0;
-
- malformed_resp:
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("malformed netlink response message"));
-return rc;
  }
  
  
@@ -523,13 +537,12 @@ virNetlinkNewLink(const char *ifname,

virNetlinkNewLinkDataPtr extra_args,
int *error)
  {
-struct nlmsgerr *err;
  struct nlattr *linkinfo = NULL;
  struct nlattr *infodata = NULL;
-

[libvirt PATCH v3 6/8] tests: Test vmx files with missing images

2021-01-05 Thread Martin Kletzander
Signed-off-by: Martin Kletzander 
---
 .../vmx2xml-cdrom-ide-file-missing-datastore.vmx   | 6 ++
 tests/vmx2xmldata/vmx2xml-cdrom-ide-file-missing-file.vmx  | 6 ++
 .../vmx2xml-harddisk-ide-file-missing-datastore.vmx| 6 ++
 .../vmx2xml-harddisk-scsi-file-missing-file.vmx| 7 +++
 tests/vmx2xmltest.c| 6 ++
 5 files changed, 31 insertions(+)
 create mode 100644 
tests/vmx2xmldata/vmx2xml-cdrom-ide-file-missing-datastore.vmx
 create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-file-missing-file.vmx
 create mode 100644 
tests/vmx2xmldata/vmx2xml-harddisk-ide-file-missing-datastore.vmx
 create mode 100644 
tests/vmx2xmldata/vmx2xml-harddisk-scsi-file-missing-file.vmx

diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-ide-file-missing-datastore.vmx 
b/tests/vmx2xmldata/vmx2xml-cdrom-ide-file-missing-datastore.vmx
new file mode 100644
index ..8a8de892c88a
--- /dev/null
+++ b/tests/vmx2xmldata/vmx2xml-cdrom-ide-file-missing-datastore.vmx
@@ -0,0 +1,6 @@
+config.version = "8"
+virtualHW.version = "4"
+ide0:0.present = "true"
+ide0:0.deviceType = "cdrom-image"
+ide0:0.fileName = "/vmfs/volumes/missing/cdrom.iso"
+displayName = "test"
diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-ide-file-missing-file.vmx 
b/tests/vmx2xmldata/vmx2xml-cdrom-ide-file-missing-file.vmx
new file mode 100644
index ..6ee2fb553ae4
--- /dev/null
+++ b/tests/vmx2xmldata/vmx2xml-cdrom-ide-file-missing-file.vmx
@@ -0,0 +1,6 @@
+config.version = "8"
+virtualHW.version = "4"
+ide0:0.present = "true"
+ide0:0.deviceType = "cdrom-image"
+ide0:0.fileName = "/vmfs/volumes/ds/missing.iso"
+displayName = "test"
diff --git a/tests/vmx2xmldata/vmx2xml-harddisk-ide-file-missing-datastore.vmx 
b/tests/vmx2xmldata/vmx2xml-harddisk-ide-file-missing-datastore.vmx
new file mode 100644
index ..9d89ce7158a8
--- /dev/null
+++ b/tests/vmx2xmldata/vmx2xml-harddisk-ide-file-missing-datastore.vmx
@@ -0,0 +1,6 @@
+config.version = "8"
+virtualHW.version = "4"
+ide0:0.present = "true"
+ide0:0.deviceType = "ata-hardDisk"
+ide0:0.fileName = "/vmfs/volumes/missing/harddisk.vmdk"
+displayName = "test"
diff --git a/tests/vmx2xmldata/vmx2xml-harddisk-scsi-file-missing-file.vmx 
b/tests/vmx2xmldata/vmx2xml-harddisk-scsi-file-missing-file.vmx
new file mode 100644
index ..d39f657e437f
--- /dev/null
+++ b/tests/vmx2xmldata/vmx2xml-harddisk-scsi-file-missing-file.vmx
@@ -0,0 +1,7 @@
+config.version = "8"
+virtualHW.version = "4"
+scsi0.present = "true"
+scsi0:0.present = "true"
+scsi0:0.deviceType = "scsi-hardDisk"
+scsi0:0.fileName = "/vmfs/volumes/ds/missing.vmdk"
+displayName = "test"
diff --git a/tests/vmx2xmltest.c b/tests/vmx2xmltest.c
index 3a11dfb41ce8..d622e46fd563 100644
--- a/tests/vmx2xmltest.c
+++ b/tests/vmx2xmltest.c
@@ -247,6 +247,12 @@ mymain(void)
 DO_TEST("cdrom-ide-raw-auto-detect", "cdrom-ide-raw-auto-detect");
 DO_TEST("cdrom-ide-raw-auto-detect", "cdrom-ide-raw-auto-detect");
 
+DO_TEST_FAIL("cdrom-ide-file-missing-datastore", "cdrom-ide-empty");
+DO_TEST_FAIL("cdrom-ide-file-missing-file", "cdrom-ide-empty");
+
+DO_TEST_FAIL("harddisk-ide-file-missing-datastore", "harddisk-ide-file");
+DO_TEST_FAIL("harddisk-scsi-file-missing-file", "harddisk-scsi-file");
+
 DO_TEST("floppy-file", "floppy-file");
 DO_TEST("floppy-device", "floppy-device");
 
-- 
2.30.0



[libvirt PATCH v3 8/8] vmx: Treat missing cdrom-image as empty drive

2021-01-05 Thread Martin Kletzander
This is perfectly valid in VMWare and the VM just boots with an empty drive.  We
used to just skip the whole drive before, but since we changed how we parse
empty cdrom drives this results in an error.  Make it behave more closer to
VMWare.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1903953

Signed-off-by: Martin Kletzander 
---
 src/vmx/vmx.c   | 2 +-
 tests/vmx2xmltest.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index aa5d1d4eedea..56318fc8b285 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -2448,7 +2448,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr 
xmlopt, virConfPtr con
 }
 
 virDomainDiskSetType(*def, VIR_STORAGE_TYPE_FILE);
-if (ctx->parseFileName(fileName, ctx->opaque, , false) < 0)
+if (ctx->parseFileName(fileName, ctx->opaque, , true) < 0)
 goto cleanup;
 virDomainDiskSetSource(*def, tmp);
 VIR_FREE(tmp);
diff --git a/tests/vmx2xmltest.c b/tests/vmx2xmltest.c
index d622e46fd563..11739e6f3f51 100644
--- a/tests/vmx2xmltest.c
+++ b/tests/vmx2xmltest.c
@@ -247,8 +247,8 @@ mymain(void)
 DO_TEST("cdrom-ide-raw-auto-detect", "cdrom-ide-raw-auto-detect");
 DO_TEST("cdrom-ide-raw-auto-detect", "cdrom-ide-raw-auto-detect");
 
-DO_TEST_FAIL("cdrom-ide-file-missing-datastore", "cdrom-ide-empty");
-DO_TEST_FAIL("cdrom-ide-file-missing-file", "cdrom-ide-empty");
+DO_TEST("cdrom-ide-file-missing-datastore", "cdrom-ide-empty");
+DO_TEST("cdrom-ide-file-missing-file", "cdrom-ide-empty");
 
 DO_TEST_FAIL("harddisk-ide-file-missing-datastore", "harddisk-ide-file");
 DO_TEST_FAIL("harddisk-scsi-file-missing-file", "harddisk-scsi-file");
-- 
2.30.0



[libvirt PATCH v3 5/8] vmx: Allow missing cdrom image file in virVMXParseFileName

2021-01-05 Thread Martin Kletzander
This will be used later.

Signed-off-by: Martin Kletzander 
---
 src/esx/esx_driver.c |  3 ++-
 src/vmware/vmware_conf.c |  3 ++-
 src/vmware/vmware_conf.h |  3 ++-
 src/vmx/vmx.c| 12 +++-
 src/vmx/vmx.h|  5 -
 tests/vmx2xmltest.c  | 13 -
 6 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 86d5396147a3..dde51688f72f 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -128,7 +128,8 @@ esxFreePrivate(esxPrivate **priv)
 static int
 esxParseVMXFileName(const char *fileName,
 void *opaque,
-char **out)
+char **out,
+bool allow_missing G_GNUC_UNUSED)
 {
 esxVMX_Data *data = opaque;
 esxVI_String *propertyNameList = NULL;
diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c
index 4f7dc3001d2b..55cd1d6f2d36 100644
--- a/src/vmware/vmware_conf.c
+++ b/src/vmware/vmware_conf.c
@@ -510,7 +510,8 @@ vmwareExtractPid(const char * vmxPath)
 int
 vmwareParseVMXFileName(const char *datastorePath,
void *opaque G_GNUC_UNUSED,
-   char **out)
+   char **out,
+   bool allow_missing G_GNUC_UNUSED)
 {
 *out = g_strdup(datastorePath);
 
diff --git a/src/vmware/vmware_conf.h b/src/vmware/vmware_conf.h
index 6f86983f511f..4974260e68f6 100644
--- a/src/vmware/vmware_conf.h
+++ b/src/vmware/vmware_conf.h
@@ -86,7 +86,8 @@ int vmwareExtractPid(const char * vmxPath);
 int
 vmwareParseVMXFileName(const char *datastorePath,
void *opaque,
-   char **out);
+   char **out,
+   bool allow_missing);
 
 char *
 vmwareFormatVMXFileName(const char *datastorePath,
diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index 97591842f789..aa5d1d4eedea 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -2411,7 +2411,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr 
xmlopt, virConfPtr con
 }
 
 virDomainDiskSetType(*def, VIR_STORAGE_TYPE_FILE);
-if (ctx->parseFileName(fileName, ctx->opaque, ) < 0)
+if (ctx->parseFileName(fileName, ctx->opaque, , false) < 0)
 goto cleanup;
 virDomainDiskSetSource(*def, tmp);
 VIR_FREE(tmp);
@@ -2448,7 +2448,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr 
xmlopt, virConfPtr con
 }
 
 virDomainDiskSetType(*def, VIR_STORAGE_TYPE_FILE);
-if (ctx->parseFileName(fileName, ctx->opaque, ) < 0)
+if (ctx->parseFileName(fileName, ctx->opaque, , false) < 0)
 goto cleanup;
 virDomainDiskSetSource(*def, tmp);
 VIR_FREE(tmp);
@@ -2515,7 +2515,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr 
xmlopt, virConfPtr con
 
 virDomainDiskSetType(*def, VIR_STORAGE_TYPE_FILE);
 if (fileName &&
-ctx->parseFileName(fileName, ctx->opaque, ) < 0)
+ctx->parseFileName(fileName, ctx->opaque, , false) < 0)
 goto cleanup;
 virDomainDiskSetSource(*def, tmp);
 VIR_FREE(tmp);
@@ -2977,7 +2977,8 @@ virVMXParseSerial(virVMXContext *ctx, virConfPtr conf, 
int port,
 (*def)->source->type = VIR_DOMAIN_CHR_TYPE_FILE;
 if (ctx->parseFileName(fileName,
   ctx->opaque,
-  &(*def)->source->data.file.path) < 0)
+  &(*def)->source->data.file.path,
+  false) < 0)
 goto cleanup;
 } else if (STRCASEEQ(fileType, "pipe")) {
 /*
@@ -3142,7 +3143,8 @@ virVMXParseParallel(virVMXContext *ctx, virConfPtr conf, 
int port,
 (*def)->source->type = VIR_DOMAIN_CHR_TYPE_FILE;
 if (ctx->parseFileName(fileName,
   ctx->opaque,
-  &(*def)->source->data.file.path) < 0)
+  &(*def)->source->data.file.path,
+  false) < 0)
 goto cleanup;
 } else {
 virReportError(VIR_ERR_INTERNAL_ERROR,
diff --git a/src/vmx/vmx.h b/src/vmx/vmx.h
index e5420c970a4b..550c1264f3b8 100644
--- a/src/vmx/vmx.h
+++ b/src/vmx/vmx.h
@@ -36,7 +36,10 @@ virDomainXMLOptionPtr virVMXDomainXMLConfInit(virCapsPtr 
caps);
  * Context
  */
 
-typedef int (*virVMXParseFileName)(const char *fileName, void *opaque, char 
**src);
+typedef int (*virVMXParseFileName)(const char *fileName,
+   void *opaque,
+   char **src,
+   bool allow_missing);
 typedef char * (*virVMXFormatFileName)(const char *src, void *opaque);
 typedef int (*virVMXAutodetectSCSIControllerModel)(virDomainDiskDefPtr def,
 

[libvirt PATCH v3 7/8] esx: Handle missing images in esxParseVMXFileName

2021-01-05 Thread Martin Kletzander
Signed-off-by: Martin Kletzander 
---
 src/esx/esx_driver.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index dde51688f72f..0271f81a5655 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -129,7 +129,7 @@ static int
 esxParseVMXFileName(const char *fileName,
 void *opaque,
 char **out,
-bool allow_missing G_GNUC_UNUSED)
+bool allow_missing)
 {
 esxVMX_Data *data = opaque;
 esxVI_String *propertyNameList = NULL;
@@ -223,9 +223,13 @@ esxParseVMXFileName(const char *fileName,
 }
 
 if (!datastoreList) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("File name '%s' refers to non-existing datastore 
'%s'"),
-   fileName, datastoreName);
+if (allow_missing) {
+ret = 0;
+} else {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("File name '%s' refers to non-existing 
datastore '%s'"),
+   fileName, datastoreName);
+}
 goto cleanup;
 }
 
-- 
2.30.0



[libvirt PATCH v3 2/8] tests: Use g_autofree in testParseVMXFileName

2021-01-05 Thread Martin Kletzander
There's only one variable to clean-up, others are just tokens inside that
variable, but it is nicer anyway.  Positive returns have not been converted
because the function will change soon and it would not make much sense.

Signed-off-by: Martin Kletzander 
---
 tests/vmx2xmltest.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/tests/vmx2xmltest.c b/tests/vmx2xmltest.c
index 376116bb750a..8cc227bbedfc 100644
--- a/tests/vmx2xmltest.c
+++ b/tests/vmx2xmltest.c
@@ -130,7 +130,7 @@ testCompareHelper(const void *data)
 static char *
 testParseVMXFileName(const char *fileName, void *opaque G_GNUC_UNUSED)
 {
-char *copyOfFileName = NULL;
+g_autofree char *copyOfFileName = NULL;
 char *tmp = NULL;
 char *saveptr = NULL;
 char *datastoreName = NULL;
@@ -145,7 +145,7 @@ testParseVMXFileName(const char *fileName, void *opaque 
G_GNUC_UNUSED)
 if ((tmp = STRSKIP(copyOfFileName, "/vmfs/volumes/")) == NULL ||
 (datastoreName = strtok_r(tmp, "/", )) == NULL ||
 (directoryAndFileName = strtok_r(NULL, "", )) == NULL) {
-goto cleanup;
+return NULL;
 }
 
 src = g_strdup_printf("[%s] %s", datastoreName, directoryAndFileName);
@@ -154,15 +154,12 @@ testParseVMXFileName(const char *fileName, void *opaque 
G_GNUC_UNUSED)
 src = g_strdup(fileName);
 } else if (strchr(fileName, '/') != NULL) {
 /* Found relative path, this is not supported */
-src = NULL;
+return NULL;
 } else {
 /* Found single file name referencing a file inside a datastore */
 src = g_strdup_printf("[datastore] directory/%s", fileName);
 }
 
- cleanup:
-VIR_FREE(copyOfFileName);
-
 return src;
 }
 
-- 
2.30.0



[libvirt PATCH v3 3/8] vmx: Make virVMXParseFileName return an integer

2021-01-05 Thread Martin Kletzander
And return the actual extracted value in a parameter.  This way we can later
return success even without any extracted value.

Signed-off-by: Martin Kletzander 
---
 src/esx/esx_driver.c   | 31 ++-
 src/vmware/vmware_conf.c   | 20 ++--
 src/vmware/vmware_conf.h   |  9 -
 src/vmware/vmware_driver.c |  6 +++---
 src/vmx/vmx.c  | 25 -
 src/vmx/vmx.h  |  2 +-
 tests/vmx2xmltest.c| 21 -
 7 files changed, 68 insertions(+), 46 deletions(-)

diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 51c26e29c65e..86d5396147a3 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -125,10 +125,11 @@ esxFreePrivate(esxPrivate **priv)
  * exception and need special handling. Parse the datastore name and use it
  * to lookup the datastore by name to verify that it exists.
  */
-static char *
-esxParseVMXFileName(const char *fileName, void *opaque)
+static int
+esxParseVMXFileName(const char *fileName,
+void *opaque,
+char **out)
 {
-char *result = NULL;
 esxVMX_Data *data = opaque;
 esxVI_String *propertyNameList = NULL;
 esxVI_ObjectContent *datastoreList = NULL;
@@ -140,18 +141,22 @@ esxParseVMXFileName(const char *fileName, void *opaque)
 char *strippedFileName = NULL;
 char *copyOfFileName = NULL;
 char *directoryAndFileName;
+int ret = -1;
+
+*out = NULL;
 
 if (!strchr(fileName, '/') && !strchr(fileName, '\\')) {
 /* Plain file name, use same directory as for the .vmx file */
-return g_strdup_printf("%s/%s", data->datastorePathWithoutFileName,
+*out = g_strdup_printf("%s/%s", data->datastorePathWithoutFileName,
fileName);
+return 0;
 }
 
 if (esxVI_String_AppendValueToList(,
"summary.name") < 0 ||
 esxVI_LookupDatastoreList(data->ctx, propertyNameList,
   ) < 0) {
-return NULL;
+return -1;
 }
 
 /* Search for datastore by mount path */
@@ -189,13 +194,13 @@ esxParseVMXFileName(const char *fileName, void *opaque)
 ++tmp;
 }
 
-result = g_strdup_printf("[%s] %s", datastoreName, strippedFileName);
+*out = g_strdup_printf("[%s] %s", datastoreName, strippedFileName);
 
 break;
 }
 
 /* Fallback to direct datastore name match */
-if (!result && STRPREFIX(fileName, "/vmfs/volumes/")) {
+if (!*out && STRPREFIX(fileName, "/vmfs/volumes/")) {
 copyOfFileName = g_strdup(fileName);
 
 /* Expected format: '/vmfs/volumes//' */
@@ -223,22 +228,22 @@ esxParseVMXFileName(const char *fileName, void *opaque)
 goto cleanup;
 }
 
-result = g_strdup_printf("[%s] %s", datastoreName,
- directoryAndFileName);
+*out = g_strdup_printf("[%s] %s", datastoreName, directoryAndFileName);
 }
 
 /* If it's an absolute path outside of a datastore just use it as is */
-if (!result && *fileName == '/') {
+if (!*out && *fileName == '/') {
 /* FIXME: need to deal with Windows paths here too */
-result = g_strdup(fileName);
+*out = g_strdup(fileName);
 }
 
-if (!result) {
+if (!*out) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Could not handle file name '%s'"), fileName);
 goto cleanup;
 }
 
+ret = 0;
  cleanup:
 esxVI_String_Free();
 esxVI_ObjectContent_Free();
@@ -246,7 +251,7 @@ esxParseVMXFileName(const char *fileName, void *opaque)
 VIR_FREE(strippedFileName);
 VIR_FREE(copyOfFileName);
 
-return result;
+return ret;
 }
 
 
diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c
index e44673247ff1..4f7dc3001d2b 100644
--- a/src/vmware/vmware_conf.c
+++ b/src/vmware/vmware_conf.c
@@ -139,7 +139,7 @@ vmwareLoadDomains(struct vmware_driver *driver)
 char *saveptr = NULL;
 virCommandPtr cmd;
 
-ctx.parseFileName = vmwareCopyVMXFileName;
+ctx.parseFileName = vmwareParseVMXFileName;
 ctx.formatFileName = NULL;
 ctx.autodetectSCSIControllerModel = NULL;
 ctx.datacenterPath = NULL;
@@ -507,11 +507,19 @@ vmwareExtractPid(const char * vmxPath)
 return pid_value;
 }
 
-char *
-vmwareCopyVMXFileName(const char *datastorePath, void *opaque G_GNUC_UNUSED)
+int
+vmwareParseVMXFileName(const char *datastorePath,
+   void *opaque G_GNUC_UNUSED,
+   char **out)
 {
-char *path;
+*out = g_strdup(datastorePath);
+
+return *out ? 0 : -1;
+}
 
-path = g_strdup(datastorePath);
-return path;
+char *
+vmwareFormatVMXFileName(const char *datastorePath,
+void *opaque G_GNUC_UNUSED)
+{
+return g_strdup(datastorePath);
 }
diff --git a/src/vmware/vmware_conf.h b/src/vmware/vmware_conf.h
index 

[libvirt PATCH v3 0/8] vmx: Don't error out on missing filename for cdrom

2021-01-05 Thread Martin Kletzander
This is perfectly valid in VMWare and the VM just boots with an empty drive.  We
used to just skip the whole drive before, but since we changed how we parse
empty cdrom drives this now results in an error and the user not being able to
even dump the XML.  Instead of erroring out, just keep the drive empty.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1903953

v3:
 - Fixed the vmware driver
 - Bit of a clean-up
 - Few more tests

v2:
 - Do not report and reset an error, but handle it more nicely.
 - https://www.redhat.com/archives/libvir-list/2020-December/msg00846.html

v1:
 - https://www.redhat.com/archives/libvir-list/2020-December/msg00840.html

Martin Kletzander (8):
  esx: Unindent unnecessary conditional branch
  tests: Use g_autofree in testParseVMXFileName
  vmx: Make virVMXParseFileName return an integer
  tests: Allow testing for parse failures in vmx2xmltest
  vmx: Allow missing cdrom image file in virVMXParseFileName
  tests: Test vmx files with missing images
  esx: Handle missing images in esxParseVMXFileName
  vmx: Treat missing cdrom-image as empty drive

 src/esx/esx_driver.c  | 160 ++
 src/vmware/vmware_conf.c  |  21 ++-
 src/vmware/vmware_conf.h  |  10 +-
 src/vmware/vmware_driver.c|   6 +-
 src/vmx/vmx.c |  27 +--
 src/vmx/vmx.h |   5 +-
 ...x2xml-cdrom-ide-file-missing-datastore.vmx |   6 +
 .../vmx2xml-cdrom-ide-file-missing-file.vmx   |   6 +
 ...ml-harddisk-ide-file-missing-datastore.vmx |   6 +
 ...mx2xml-harddisk-scsi-file-missing-file.vmx |   7 +
 tests/vmx2xmltest.c   |  67 +---
 11 files changed, 203 insertions(+), 118 deletions(-)
 create mode 100644 
tests/vmx2xmldata/vmx2xml-cdrom-ide-file-missing-datastore.vmx
 create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-file-missing-file.vmx
 create mode 100644 
tests/vmx2xmldata/vmx2xml-harddisk-ide-file-missing-datastore.vmx
 create mode 100644 
tests/vmx2xmldata/vmx2xml-harddisk-scsi-file-missing-file.vmx

-- 
2.30.0




[libvirt PATCH v3 4/8] tests: Allow testing for parse failures in vmx2xmltest

2021-01-05 Thread Martin Kletzander
Signed-off-by: Martin Kletzander 
---
 tests/vmx2xmltest.c | 24 ++--
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/tests/vmx2xmltest.c b/tests/vmx2xmltest.c
index bb7c498d1b41..7db2edb12c27 100644
--- a/tests/vmx2xmltest.c
+++ b/tests/vmx2xmltest.c
@@ -66,7 +66,7 @@ testCapsInit(void)
 }
 
 static int
-testCompareFiles(const char *vmx, const char *xml)
+testCompareFiles(const char *vmx, const char *xml, bool should_fail_parse)
 {
 int ret = -1;
 char *vmxData = NULL;
@@ -74,9 +74,17 @@ testCompareFiles(const char *vmx, const char *xml)
 virDomainDefPtr def = NULL;
 
 if (virTestLoadFile(vmx, ) < 0)
-goto cleanup;
+return -1;
 
-if (!(def = virVMXParseConfig(, xmlopt, caps, vmxData)))
+def = virVMXParseConfig(, xmlopt, caps, vmxData);
+if (should_fail_parse) {
+if (!def)
+ret = 0;
+else
+VIR_TEST_DEBUG("passed instead of expected failure");
+goto cleanup;
+}
+if (!def)
 goto cleanup;
 
 if (!virDomainDefCheckABIStability(def, def, xmlopt)) {
@@ -104,6 +112,7 @@ testCompareFiles(const char *vmx, const char *xml)
 struct testInfo {
 const char *input;
 const char *output;
+bool should_fail;
 };
 
 static int
@@ -119,7 +128,7 @@ testCompareHelper(const void *data)
 xml = g_strdup_printf("%s/vmx2xmldata/vmx2xml-%s.xml", abs_srcdir,
   info->output);
 
-ret = testCompareFiles(vmx, xml);
+ret = testCompareFiles(vmx, xml, info->should_fail);
 
 VIR_FREE(vmx);
 VIR_FREE(xml);
@@ -171,9 +180,9 @@ mymain(void)
 {
 int ret = 0;
 
-# define DO_TEST(_in, _out) \
+# define DO_TEST_FULL(_in, _out, _should_fail) \
 do { \
-struct testInfo info = { _in, _out }; \
+struct testInfo info = { _in, _out, _should_fail }; \
 virResetLastError(); \
 if (virTestRun("VMware VMX-2-XML "_in" -> "_out, \
testCompareHelper, ) < 0) { \
@@ -181,6 +190,9 @@ mymain(void)
 } \
 } while (0)
 
+# define DO_TEST(_in, _out) DO_TEST_FULL(_in, _out, false)
+# define DO_TEST_FAIL(_in, _out) DO_TEST_FULL(_in, _out, true)
+
 testCapsInit();
 
 if (caps == NULL)
-- 
2.30.0



[libvirt PATCH v3 1/8] esx: Unindent unnecessary conditional branch

2021-01-05 Thread Martin Kletzander
The positive branch can just return and the huge negative part does not need to
be indented an extra level.  Best viewed with `-w`.

Signed-off-by: Martin Kletzander 
---
 src/esx/esx_driver.c | 144 +--
 1 file changed, 72 insertions(+), 72 deletions(-)

diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index a17bf58a5124..51c26e29c65e 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -143,100 +143,100 @@ esxParseVMXFileName(const char *fileName, void *opaque)
 
 if (!strchr(fileName, '/') && !strchr(fileName, '\\')) {
 /* Plain file name, use same directory as for the .vmx file */
-result = g_strdup_printf("%s/%s", data->datastorePathWithoutFileName,
- fileName);
-} else {
-if (esxVI_String_AppendValueToList(,
-   "summary.name") < 0 ||
-esxVI_LookupDatastoreList(data->ctx, propertyNameList,
-  ) < 0) {
-return NULL;
-}
+return g_strdup_printf("%s/%s", data->datastorePathWithoutFileName,
+   fileName);
+}
 
-/* Search for datastore by mount path */
-for (datastore = datastoreList; datastore;
- datastore = datastore->_next) {
-esxVI_DatastoreHostMount_Free();
-datastoreName = NULL;
-
-if (esxVI_LookupDatastoreHostMount(data->ctx, datastore->obj,
-   ,
-   esxVI_Occurrence_RequiredItem) 
< 0 ||
-esxVI_GetStringValue(datastore, "summary.name", ,
- esxVI_Occurrence_RequiredItem) < 0) {
-goto cleanup;
-}
+if (esxVI_String_AppendValueToList(,
+   "summary.name") < 0 ||
+esxVI_LookupDatastoreList(data->ctx, propertyNameList,
+  ) < 0) {
+return NULL;
+}
 
-tmp = (char *)STRSKIP(fileName, hostMount->mountInfo->path);
+/* Search for datastore by mount path */
+for (datastore = datastoreList; datastore;
+ datastore = datastore->_next) {
+esxVI_DatastoreHostMount_Free();
+datastoreName = NULL;
 
-if (!tmp)
-continue;
+if (esxVI_LookupDatastoreHostMount(data->ctx, datastore->obj,
+   ,
+   esxVI_Occurrence_RequiredItem) < 0 
||
+esxVI_GetStringValue(datastore, "summary.name", ,
+ esxVI_Occurrence_RequiredItem) < 0) {
+goto cleanup;
+}
 
-/* Found a match. Strip leading separators */
-while (*tmp == '/' || *tmp == '\\')
-++tmp;
+tmp = (char *)STRSKIP(fileName, hostMount->mountInfo->path);
 
-strippedFileName = g_strdup(tmp);
+if (!tmp)
+continue;
 
-tmp = strippedFileName;
+/* Found a match. Strip leading separators */
+while (*tmp == '/' || *tmp == '\\')
+++tmp;
 
-/* Convert \ to / */
-while (*tmp != '\0') {
-if (*tmp == '\\')
-*tmp = '/';
+strippedFileName = g_strdup(tmp);
 
-++tmp;
-}
+tmp = strippedFileName;
 
-result = g_strdup_printf("[%s] %s", datastoreName, 
strippedFileName);
+/* Convert \ to / */
+while (*tmp != '\0') {
+if (*tmp == '\\')
+*tmp = '/';
 
-break;
+++tmp;
 }
 
-/* Fallback to direct datastore name match */
-if (!result && STRPREFIX(fileName, "/vmfs/volumes/")) {
-copyOfFileName = g_strdup(fileName);
-
-/* Expected format: '/vmfs/volumes//' */
-if (!(tmp = STRSKIP(copyOfFileName, "/vmfs/volumes/")) ||
-!(datastoreName = strtok_r(tmp, "/", ))||
-!(directoryAndFileName = strtok_r(NULL, "", ))) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("File name '%s' doesn't have expected format "
- "'/vmfs/volumes//'"), 
fileName);
-goto cleanup;
-}
-
-esxVI_ObjectContent_Free();
+result = g_strdup_printf("[%s] %s", datastoreName, strippedFileName);
 
-if (esxVI_LookupDatastoreByName(data->ctx, datastoreName,
-NULL, ,
-esxVI_Occurrence_OptionalItem) < 
0) {
-goto cleanup;
-}
+break;
+}
 
-if (!datastoreList) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("File name '%s' refers to non-existing 

[PATCH] qemu: backup: Properly delete temporary bitmap after push-mode incremental backup

2021-01-05 Thread Peter Krempa
Refactor in 0316c28a453ac used incorrect source variable to initialize
the variable which holds the name of the bitmap which needs to be
deleted after the backup job finishes. This resulted into deleting the
source bitmap of the backup rather than the temporary one.

Use 'dd->incrementalBitmap' which holds the temporary bitmap name
instead of 'dd->backupdisk->incremental' which holds the name of the
source bitmap which is used by the backup.

Fixes: 0316c28a453ac15f58c61f30359f66ab9a649884
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1908647
Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_backup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c
index b2340eb1cf..c444f8aaba 100644
--- a/src/qemu/qemu_backup.c
+++ b/src/qemu/qemu_backup.c
@@ -235,7 +235,7 @@ qemuBackupDiskPrepareOneBitmaps(struct qemuBackupDiskData 
*dd,
  blockNamedNodeData) < 0)
 return -1;

-dd->domdiskIncrementalBitmap = dd->backupdisk->incremental;
+dd->domdiskIncrementalBitmap = dd->incrementalBitmap;
 }

 return 0;
-- 
2.29.2



[PATCH v2 2/3] virStorageBackendCopyToFD: remove unused return variable

2021-01-05 Thread Yi Li
remove unused return variable,
The errno will throw by virReportSystemError

Signed-off-by: Yi Li 
---
 src/storage/storage_util.c | 22 +++---
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index c13abf03af..c6d0f7a97c 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -128,7 +128,6 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol,
   bool reflink_copy)
 {
 int amtread = -1;
-int ret = 0;
 size_t rbytes = READ_BLOCK_SIZE_DEFAULT;
 int wbytes = 0;
 int interval;
@@ -138,11 +137,10 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol,
 VIR_AUTOCLOSE inputfd = -1;
 
 if ((inputfd = open(inputvol->target.path, O_RDONLY)) < 0) {
-ret = -errno;
 virReportSystemError(errno,
  _("could not open input path '%s'"),
  inputvol->target.path);
-return ret;
+return -1;
 }
 
 #ifdef __linux__
@@ -160,11 +158,10 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol,
 
 if (reflink_copy) {
 if (reflinkCloneFile(fd, inputfd) < 0) {
-ret = -errno;
 virReportSystemError(errno,
  _("failed to clone files from '%s'"),
  inputvol->target.path);
-return ret;
+return -1;
 } else {
 VIR_DEBUG("btrfs clone finished.");
 return 0;
@@ -178,11 +175,10 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol,
 rbytes = *total;
 
 if ((amtread = saferead(inputfd, buf, rbytes)) < 0) {
-ret = -errno;
 virReportSystemError(errno,
  _("failed reading from file '%s'"),
  inputvol->target.path);
-return ret;
+return -1;
 }
 *total -= amtread;
 
@@ -195,36 +191,32 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol,
 
 if (want_sparse && memcmp(buf+offset, zerobuf, interval) == 0) {
 if (lseek(fd, interval, SEEK_CUR) < 0) {
-ret = -errno;
 virReportSystemError(errno,
  _("cannot extend file '%s'"),
  vol->target.path);
-return ret;
+return -1;
 }
 } else if (safewrite(fd, buf+offset, interval) < 0) {
-ret = -errno;
 virReportSystemError(errno,
  _("failed writing to file '%s'"),
  vol->target.path);
-return ret;
+return -1;
 
 }
 } while ((amtleft -= interval) > 0);
 }
 
 if (virFileDataSync(fd) < 0) {
-ret = -errno;
 virReportSystemError(errno, _("cannot sync data to file '%s'"),
  vol->target.path);
-return ret;
+return -1;
 }
 
 if (VIR_CLOSE(inputfd) < 0) {
-ret = -errno;
 virReportSystemError(errno,
  _("cannot close file '%s'"),
  inputvol->target.path);
-return ret;
+return -1;
 }
 
 return 0;
-- 
2.25.3






[PATCH v2 3/3] storageBackendCreateRaw: remove unused created

2021-01-05 Thread Yi Li
refactor and remove unused created variable.

Signed-off-by: Yi Li 
---
 src/storage/storage_util.c | 23 ++-
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index c6d0f7a97c..cc8189c3e0 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -384,11 +384,9 @@ storageBackendCreateRaw(virStoragePoolObjPtr pool,
 unsigned int flags)
 {
 virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
-int ret = -1;
 int operation_flags;
 bool reflink_copy = false;
 mode_t open_mode = VIR_STORAGE_DEFAULT_VOL_PERM_MODE;
-bool created = false;
 VIR_AUTOCLOSE fd = -1;
 
 virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA |
@@ -399,13 +397,13 @@ storageBackendCreateRaw(virStoragePoolObjPtr pool,
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("metadata preallocation is not supported for raw "
  "volumes"));
-goto cleanup;
+return -1;
 }
 
 if (virStorageSourceHasBacking(>target)) {
 virReportError(VIR_ERR_NO_SUPPORT, "%s",
_("backing storage not supported for raw volumes"));
-goto cleanup;
+return -1;
 }
 
 if (flags & VIR_STORAGE_VOL_CREATE_REFLINK)
@@ -415,7 +413,7 @@ storageBackendCreateRaw(virStoragePoolObjPtr pool,
 if (vol->target.encryption) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("storage pool does not support encrypted volumes"));
-goto cleanup;
+return -1;
 }
 
 operation_flags = VIR_FILE_OPEN_FORCE_MODE | VIR_FILE_OPEN_FORCE_OWNER;
@@ -434,26 +432,25 @@ storageBackendCreateRaw(virStoragePoolObjPtr pool,
 virReportSystemError(-fd,
  _("Failed to create file '%s'"),
  vol->target.path);
-goto cleanup;
+return -1;
 }
-created = true;
 
 /* NB, COW flag can only be toggled when the file is zero-size,
  * so must go before the createRawFile call allocates payload */
 if (vol->target.nocow &&
 virFileSetCOW(vol->target.path, VIR_TRISTATE_BOOL_NO) < 0)
-goto cleanup;
+goto error;
 
-if ((ret = createRawFile(fd, vol, inputvol, reflink_copy)) < 0)
+if (createRawFile(fd, vol, inputvol, reflink_copy) < 0)
 /* createRawFile already reported the exact error. */
-ret = -1;
+goto error;
+return 0;
 
- cleanup:
-if (ret < 0 && created)
+ error:
 ignore_value(virFileRemove(vol->target.path,
vol->target.perms->uid,
vol->target.perms->gid));
-return ret;
+return -1;
 }
 
 
-- 
2.25.3






[PATCH v2 0/3] Storage: remove unused variable

2021-01-05 Thread Yi Li
refactor storageBackendCreateRaw and remove some unused variable.

Yi Li (3):
  createRawFile: remove unused return variable
  virStorageBackendCopyToFD: remove unused return variable
  storageBackendCreateRaw: remove unused created

 src/storage/storage_util.c | 66 +++---
 1 file changed, 25 insertions(+), 41 deletions(-)

-- 
2.25.3






[PATCH v2 1/3] createRawFile: remove unused return variable

2021-01-05 Thread Yi Li
remove unused return variable,
The errno will throw by virReportSystemError

Signed-off-by: Yi Li 
---
 src/storage/storage_util.c | 21 -
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index cf1f33f177..c13abf03af 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -315,7 +315,6 @@ createRawFile(int fd, virStorageVolDefPtr vol,
   bool reflink_copy)
 {
 bool need_alloc = true;
-int ret = 0;
 unsigned long long pos = 0;
 
 /* If the new allocation is lower than the capacity of the original file,
@@ -327,11 +326,10 @@ createRawFile(int fd, virStorageVolDefPtr vol,
 /* Seek to the final size, so the capacity is available upfront
  * for progress reporting */
 if (ftruncate(fd, vol->target.capacity) < 0) {
-ret = -errno;
 virReportSystemError(errno,
  _("cannot extend file '%s'"),
  vol->target.path);
-return ret;
+return -1;
 }
 
 /* Avoid issues with older kernel's  namespace pollution. */
@@ -347,11 +345,10 @@ createRawFile(int fd, virStorageVolDefPtr vol,
 if (fallocate(fd, 0, 0, vol->target.allocation) == 0) {
 need_alloc = false;
 } else if (errno != ENOSYS && errno != EOPNOTSUPP) {
-ret = -errno;
 virReportSystemError(errno,
  _("cannot allocate %llu bytes in file '%s'"),
  vol->target.allocation, vol->target.path);
-return ret;
+return -1;
 }
 }
 #endif
@@ -361,9 +358,9 @@ createRawFile(int fd, virStorageVolDefPtr vol,
 /* allow zero blocks to be skipped if we've requested sparse
  * allocation (allocation < capacity) or we have already
  * been able to allocate the required space. */
-if ((ret = virStorageBackendCopyToFD(vol, inputvol, fd, ,
- !need_alloc, reflink_copy)) < 0)
-return ret;
+if (virStorageBackendCopyToFD(vol, inputvol, fd, ,
+  !need_alloc, reflink_copy) < 0)
+return -1;
 
 /* If the new allocation is greater than the original capacity,
  * but fallocate failed, fill the rest with zeroes.
@@ -373,21 +370,19 @@ createRawFile(int fd, virStorageVolDefPtr vol,
 
 if (need_alloc && (vol->target.allocation - pos > 0)) {
 if (safezero(fd, pos, vol->target.allocation - pos) < 0) {
-ret = -errno;
 virReportSystemError(errno, _("cannot fill file '%s'"),
  vol->target.path);
-return ret;
+return -1;
 }
 }
 
 if (g_fsync(fd) < 0) {
-ret = -errno;
 virReportSystemError(errno, _("cannot sync data to file '%s'"),
  vol->target.path);
-return ret;
+return -1;
 }
 
-return ret;
+return 0;
 }
 
 static int
-- 
2.25.3






Re: To start multiple KVM guests from one qcow2 image with transient disk option

2021-01-05 Thread Peter Krempa
On Mon, Jan 04, 2021 at 15:30:19 -0500, Masayoshi Mizuma wrote:
> On Sat, Dec 19, 2020 at 11:30:39PM -0500, Masayoshi Mizuma wrote:

[...]

> I think following qemu command line options and QMP commands work for sharing
> the qcow2 disks. The following uses disk hotplug instead of snapshot overlay.
> Does that make sense for libvirt...?
> 
> qemu command line options:

So you are proposing to ...

> 
>   qemu-system-x86_64 \
>   -M q35,accel=kvm,usb=off,vmport=off,smm=on,dump-guest-core=off \
>   -smp 1 \
>   -m 4096 \
>   -blockdev 
> '{"driver":"file","filename":"/home/mmizuma/debug/guest.qcow2","node-name":"storage1","auto-read-only":true,"discard":"unmap"}'
>  \
>   -blockdev 
> '{"node-name":"format1","read-only":true,"driver":"qcow2","file":"storage1","backing":null}'
>  \

... start with the disk already in 'read-only' mode _and_ skip addition
of the disk ...

>   -nographic \
>   -nodefaults \
>   -no-user-config \
>   -serial telnet::1,server,nowait \
>   -qmp tcp::10001,server,nowait \
>   -S \
>   -device pcie-root-port,id=pci.1
> 
> QMP commands:
> 
>   {"execute":"qmp_capabilities"}
>   
> {"execute":"blockdev-add","arguments":{"driver":"file","filename":"/var/lib/libvirt/images/guest.TRANSIENT","node-name":"storage2","auto-read-only":true,"discard":"unmap"}}
>   
> {"execute":"blockdev-create","arguments":{"job-id":"create","options":{"driver":"qcow2","file":"storage2","size":4294967296,"cluster-size":65536,"backing-file":"/var/lib/libvirt/images/guest.TRANSIENT","backing-fmt":"qcow2"}}}
>   
> {"execute":"blockdev-add","arguments":{"node-name":"format2","read-only":false,"driver":"qcow2","file":"storage2"}}

... and then add a writable overlay ...

>   
> {"execute":"device_add","arguments":{"driver":"virtio-blk-pci","drive":"format2","id":"transient-disk","bootindex":1,"bus":"pci.1","addr":0}}

... and hotplug the disk.
>   {"execute":"cont"}

So that is a no-go. Some disk bus-es such as IDE don't support hotplug:

https://gitlab.com/libvirt/libvirt/-/blob/master/src/qemu/qemu_hotplug.c#L1074

You could try to just instantiate the backend of the disk as read-only,
and then create a writable overlay. You just need to make sure that the
disk will be writable and that it works even for IDE/SATA which doesn't
support read-only:

https://gitlab.com/libvirt/libvirt/-/blob/master/src/qemu/qemu_validate.c#L2634



[PATCH] update ci dockerfile from Leap 15.1 to 15.2

2021-01-05 Thread Cho, Yu-Chen
Signed-off-by: Cho, Yu-Chen 
---
 .gitlab-ci.yml | 14 +++---
 ...e-151.Dockerfile => ci-opensuse-152.Dockerfile} |  2 +-
 2 files changed, 8 insertions(+), 8 deletions(-)
 rename ci/containers/{ci-opensuse-151.Dockerfile => 
ci-opensuse-152.Dockerfile} (98%)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index e2d5545f0f..30a0b23381 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -160,10 +160,10 @@ x64-fedora-rawhide-container:
   variables:
 NAME: fedora-rawhide
 
-x64-opensuse-151-container:
+x64-opensuse-152-container:
   <<: *container_job_definition
   variables:
-NAME: opensuse-151
+NAME: opensuse-152
 
 x64-ubuntu-1804-container:
   <<: *container_job_definition
@@ -365,12 +365,12 @@ x64-fedora-rawhide-clang:
 CC: clang
 RPM: skip
 
-x64-opensuse-151:
+x64-opensuse-152:
   <<: *native_build_job_definition
   needs:
-- x64-opensuse-151-container
+- x64-opensuse-152-container
   variables:
-NAME: opensuse-151
+NAME: opensuse-152
 RPM: skip
 
 x64-ubuntu-1804:
@@ -533,9 +533,9 @@ website:
 
 codestyle:
   stage: builds
-  image: $CI_REGISTRY_IMAGE/ci-opensuse-151:latest
+  image: $CI_REGISTRY_IMAGE/ci-opensuse-152:latest
   needs:
-- x64-opensuse-151-container
+- x64-opensuse-152-container
   before_script:
 - *script_variables
   script:
diff --git a/ci/containers/ci-opensuse-151.Dockerfile 
b/ci/containers/ci-opensuse-152.Dockerfile
similarity index 98%
rename from ci/containers/ci-opensuse-151.Dockerfile
rename to ci/containers/ci-opensuse-152.Dockerfile
index 9458d2de0c..def45cbe3f 100644
--- a/ci/containers/ci-opensuse-151.Dockerfile
+++ b/ci/containers/ci-opensuse-152.Dockerfile
@@ -3,7 +3,7 @@
 #  $ lcitool dockerfile opensuse-151 libvirt
 #
 # 
https://gitlab.com/libvirt/libvirt-ci/-/commit/b098ec6631a85880f818f2dd25c437d509e53680
-FROM registry.opensuse.org/opensuse/leap:15.1
+FROM registry.opensuse.org/opensuse/leap:15.2
 
 RUN zypper update -y && \
 zypper install -y \
-- 
2.29.2




Re: Ping: [PATCH] Qemu: migration: Not bind RAM info with active migration status

2021-01-05 Thread Keqian Zhu



On 2021/1/5 21:34, Daniel P. Berrangé wrote:
> On Tue, Jan 05, 2021 at 09:28:27PM +0800, Keqian Zhu wrote:
>> Hi Daniel,
>>
>> Thanks for your reply :-) Please see my words below.
>>
>> On 2021/1/4 19:58, Daniel P. Berrangé wrote:
>>> On Fri, Dec 18, 2020 at 04:38:22PM +0800, Keqian Zhu wrote:
 Hi Daniel and Jiri,

 On 2020/12/8 18:31, Jiri Denemark wrote:
> On Tue, Dec 08, 2020 at 09:27:39 +, Daniel P. Berrangé wrote:
>> On Tue, Dec 08, 2020 at 10:06:25AM +0800, zhukeqian wrote:
>>>
>>> On 2020/12/7 18:38, Daniel P. Berrangé wrote:
 On Mon, Dec 07, 2020 at 09:55:53AM +0800, zhukeqian wrote:
> Hi Daniel,
 [...]

>>>
>>> Hi Daniel,
>>>
>>> The purpose is to remove this failure check for QEMU v2.12.
>>> In QEMU commit 65ace0604551, it decoupled the RAM status from the 
>>> active migration status.
>>>
>>> The usage scenario is querying migration status at destination side, 
>>> which may contain
>>> active migration status, but without RAM status, so we will see that 
>>> libvirt report error here.
>>
>> I'm confused, because AFAIK, libvirt does not need to run
>> query-migrate on the destination, so there shouldn't be anything
>> that needs fixing.
>
> Moreover, you can't even request migration statistics on the destination
> manually because libvirt blocks that:
>
> # virsh domjobinfo nest
> error: Operation not supported: migration statistics are available only
> on the source host
>
> Jirka
>
> .
>
 Sorry for delay reply.

 The purpose of QEMU commit 65ace0604551 (migration: add postcopy total 
 blocktime into query-migrate)
 is to query some postcopy related information on destination side.

 We can call query-migrate on destination side *after* migration complete, 
 thanks.
>>>
>>> But nothing in libvirt ever tries to call query-migrate on the dest
>>> side. 
>> Yes, but the dest side does not always act as dest. After migration 
>> completion, the dest side enters
>> to a normal status and libvirt does not forbid us to query migration status.
>>
>> Before QEMU commit 65ace0604551, we can successfully query the migration 
>> status, which is
>> MIGRATION_STATUS_NONE. But this commit will return valid status 
>> (MIGRATION_STATUS_COMPLETED)
>> without ram info, causing libvirt reports error (migration was active, but 
>> no RAM info was set).
>>
>>>
>>> Do you have more patches that add such calls ? If so, then please send a
>>> patch series that does the full job.
>> I do not add new feature to libvirt, but just manually execute query-migrate 
>> on dest side *after
>> migration completion* will trigger this problem.
> 
> How are you running query-migrate ? If you're doing that with monitor
> command passthrough then we shouldn't need to change the libvirt migration
> code.
OK. I am not very familiar with libvirt code logic, and my work flow is:

 source sidedest side

 virsh start domain_name|
  | |
 virsh migrate --live domain_name   (wait for migrate complete)
  | |
  (migrate complete)|
virsh domjobinfo domain_name
|
 (migration was active, but no RAM info was set)

Hope the above work flow helps.

Thanks,
Keqian

> 
> Please can you provide the exact sequence of libvirt API calls / virsh
> commands you are running on both source and dest  to reproduce the
> problem.
> 
> 
> Regards,
> Daniel
> 




Re: Ping: [PATCH] Qemu: migration: Not bind RAM info with active migration status

2021-01-05 Thread Daniel P . Berrangé
On Tue, Jan 05, 2021 at 09:28:27PM +0800, Keqian Zhu wrote:
> Hi Daniel,
> 
> Thanks for your reply :-) Please see my words below.
> 
> On 2021/1/4 19:58, Daniel P. Berrangé wrote:
> > On Fri, Dec 18, 2020 at 04:38:22PM +0800, Keqian Zhu wrote:
> >> Hi Daniel and Jiri,
> >>
> >> On 2020/12/8 18:31, Jiri Denemark wrote:
> >>> On Tue, Dec 08, 2020 at 09:27:39 +, Daniel P. Berrangé wrote:
>  On Tue, Dec 08, 2020 at 10:06:25AM +0800, zhukeqian wrote:
> >
> > On 2020/12/7 18:38, Daniel P. Berrangé wrote:
> >> On Mon, Dec 07, 2020 at 09:55:53AM +0800, zhukeqian wrote:
> >>> Hi Daniel,
> >> [...]
> >>
> >
> > Hi Daniel,
> >
> > The purpose is to remove this failure check for QEMU v2.12.
> > In QEMU commit 65ace0604551, it decoupled the RAM status from the 
> > active migration status.
> >
> > The usage scenario is querying migration status at destination side, 
> > which may contain
> > active migration status, but without RAM status, so we will see that 
> > libvirt report error here.
> 
>  I'm confused, because AFAIK, libvirt does not need to run
>  query-migrate on the destination, so there shouldn't be anything
>  that needs fixing.
> >>>
> >>> Moreover, you can't even request migration statistics on the destination
> >>> manually because libvirt blocks that:
> >>>
> >>> # virsh domjobinfo nest
> >>> error: Operation not supported: migration statistics are available only
> >>> on the source host
> >>>
> >>> Jirka
> >>>
> >>> .
> >>>
> >> Sorry for delay reply.
> >>
> >> The purpose of QEMU commit 65ace0604551 (migration: add postcopy total 
> >> blocktime into query-migrate)
> >> is to query some postcopy related information on destination side.
> >>
> >> We can call query-migrate on destination side *after* migration complete, 
> >> thanks.
> > 
> > But nothing in libvirt ever tries to call query-migrate on the dest
> > side. 
> Yes, but the dest side does not always act as dest. After migration 
> completion, the dest side enters
> to a normal status and libvirt does not forbid us to query migration status.
> 
> Before QEMU commit 65ace0604551, we can successfully query the migration 
> status, which is
> MIGRATION_STATUS_NONE. But this commit will return valid status 
> (MIGRATION_STATUS_COMPLETED)
> without ram info, causing libvirt reports error (migration was active, but no 
> RAM info was set).
> 
> > 
> > Do you have more patches that add such calls ? If so, then please send a
> > patch series that does the full job.
> I do not add new feature to libvirt, but just manually execute query-migrate 
> on dest side *after
> migration completion* will trigger this problem.

How are you running query-migrate ? If you're doing that with monitor
command passthrough then we shouldn't need to change the libvirt migration
code.

Please can you provide the exact sequence of libvirt API calls / virsh
commands you are running on both source and dest  to reproduce the
problem.


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: Ping: [PATCH] Qemu: migration: Not bind RAM info with active migration status

2021-01-05 Thread Keqian Zhu
Hi Daniel,

Thanks for your reply :-) Please see my words below.

On 2021/1/4 19:58, Daniel P. Berrangé wrote:
> On Fri, Dec 18, 2020 at 04:38:22PM +0800, Keqian Zhu wrote:
>> Hi Daniel and Jiri,
>>
>> On 2020/12/8 18:31, Jiri Denemark wrote:
>>> On Tue, Dec 08, 2020 at 09:27:39 +, Daniel P. Berrangé wrote:
 On Tue, Dec 08, 2020 at 10:06:25AM +0800, zhukeqian wrote:
>
> On 2020/12/7 18:38, Daniel P. Berrangé wrote:
>> On Mon, Dec 07, 2020 at 09:55:53AM +0800, zhukeqian wrote:
>>> Hi Daniel,
>> [...]
>>
>
> Hi Daniel,
>
> The purpose is to remove this failure check for QEMU v2.12.
> In QEMU commit 65ace0604551, it decoupled the RAM status from the active 
> migration status.
>
> The usage scenario is querying migration status at destination side, 
> which may contain
> active migration status, but without RAM status, so we will see that 
> libvirt report error here.

 I'm confused, because AFAIK, libvirt does not need to run
 query-migrate on the destination, so there shouldn't be anything
 that needs fixing.
>>>
>>> Moreover, you can't even request migration statistics on the destination
>>> manually because libvirt blocks that:
>>>
>>> # virsh domjobinfo nest
>>> error: Operation not supported: migration statistics are available only
>>> on the source host
>>>
>>> Jirka
>>>
>>> .
>>>
>> Sorry for delay reply.
>>
>> The purpose of QEMU commit 65ace0604551 (migration: add postcopy total 
>> blocktime into query-migrate)
>> is to query some postcopy related information on destination side.
>>
>> We can call query-migrate on destination side *after* migration complete, 
>> thanks.
> 
> But nothing in libvirt ever tries to call query-migrate on the dest
> side. 
Yes, but the dest side does not always act as dest. After migration completion, 
the dest side enters
to a normal status and libvirt does not forbid us to query migration status.

Before QEMU commit 65ace0604551, we can successfully query the migration 
status, which is
MIGRATION_STATUS_NONE. But this commit will return valid status 
(MIGRATION_STATUS_COMPLETED)
without ram info, causing libvirt reports error (migration was active, but no 
RAM info was set).

> 
> Do you have more patches that add such calls ? If so, then please send a
> patch series that does the full job.
I do not add new feature to libvirt, but just manually execute query-migrate on 
dest side *after
migration completion* will trigger this problem.

Thanks,
Keqian




Re: [PATCH 1/3] virStorageBackendCopyToFD: remove unused return variable

2021-01-05 Thread Michal Privoznik

On 12/29/20 12:29 PM, Yi Li wrote:

remove unused return variable,
The errno will throw by virReportSystemError

Signed-off-by: Yi Li 
---
  src/storage/storage_util.c | 22 +++---
  1 file changed, 7 insertions(+), 15 deletions(-)



Patches look good, but I'd reorder them a bit. 2/3 should go first, then 
this on and 3/3 the last. Also, can you please send v2 with cover 
letter? It's easy as 'git format-patch -v2 --cover-letter ...'


Michal



Re: [PATCH 3/3] storageBackendCreateRaw: remove unused created variable

2021-01-05 Thread Michal Privoznik

On 12/29/20 12:29 PM, Yi Li wrote:

refactor and remove unused created variable

Signed-off-by: Yi Li 
---
  src/storage/storage_util.c | 20 ++--
  1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index c6d0f7a97c..c02ece8253 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -384,11 +384,10 @@ storageBackendCreateRaw(virStoragePoolObjPtr pool,
  unsigned int flags)
  {
  virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
-int ret = -1;
+int ret = 0;


No, please don't initialize this to zero. The pattern we use (and are 
used to) is:


  int ret = -1;

  if (something)
goto cleanup;

  if (something else)
goto cleanup;

  ret = 0;
 cleanup:
  if (ret < 0)
cleanupWhatsNeeded();

  return ret;


Alternatively, we can rename 'cleanup' to 'error' and do the following 
(without even having to use @ret variable):


  if (something)
goto error;

  if (something else)
goto error;

  return 0;
 error:
  cleanupWhatsNeeded();
  return -1;

Michal



Re: [PATCH v2] docs: support qcow2 format in luks encryption volume

2021-01-05 Thread Michal Privoznik

On 12/24/20 10:31 AM, Meina Li wrote:

Signed-off-by: Meina Li 
---
  docs/formatstorageencryption.html.in | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)



Pushed now. Congratulations on your first libvirt contribution!

Michal



Re: [PATCH] virsh-domain: Add quotes around '%s' formatting domain name

2021-01-05 Thread Michal Privoznik

On 1/5/21 1:18 PM, Peter Krempa wrote:

Domain name can contain spaces in which case it's not immediately clear
from virsh messages where the boundary of the name is. Enclose all %s
formatters in apostrophes as delimiters.

Done via the following vim regex:

  %s/omain %s/omain '%s'/g

This patch changes:

  $ virsh undefine --snapshots-metadata 'OWASP Broken Web Apps VM v1.2'
  Domain OWASP Broken Web Apps VM v1.2 has been undefined

to:

  $ virsh undefine --snapshots-metadata 'OWASP Broken Web Apps VM v1.2'
  Domain 'OWASP Broken Web Apps VM v1.2' has been undefined

Signed-off-by: Peter Krempa 
---
  tests/virsh-define-dev-segfault |   2 +-
  tests/virsh-read-bufsiz |   2 +-
  tests/virsh-undefine|   8 +-
  tools/virsh-domain.c| 136 
  4 files changed, 74 insertions(+), 74 deletions(-)


Reviewed-by: Michal Privoznik 

Michal



Re: [PATCH v1] docs: migration: remove xenmigr

2021-01-05 Thread Michal Privoznik

On 1/5/21 11:41 AM, Olaf Hering wrote:

xenmigr was for xend

Fixes commit 1dac5f06a0341e8087dc33af75c8352d77a4

Signed-off-by: Olaf Hering 
---
  docs/migration.html.in | 6 --
  1 file changed, 6 deletions(-)


Tweaked the commit message a bit and pushed.

Reviewed-by: Michal Privoznik 

Michal



Re: [libvirt][PATCH v2 0/3] introduce 'restrictive' mode in numatune

2021-01-05 Thread Daniel Henrique Barboza

Luyao,

I failed to realize, back in the v1 version of your patches, that you didn't
sign them with a "Signed-off-by" tag. This is required to assert that you agree
with the terms of the Developer Certificate of Origin, described here:

https://developercertificate.org/

This is described in detail in this link:

https://libvirt.org/submitting-patches.html


If you agree with the DCO, please re-send the patches with your "Signed-off-by"
tag in the patches. You can do that by adding a "-s" tag in "git commit" when
creating/amending the commit. My code review still stands, so feel free to keep
my "Reviewed-by" tag in all of them.


I apologize for not bringing this up back in the v1 review.


Thanks,


DHB



On 1/3/21 7:39 AM, Luyao Zhong wrote:

Before this patch set, numatune only has three memory modes:
static, interleave and prefered. These memory policies are
ultimately set by mbind() system call.

Memory policy could be 'hard coded' into the kernel, but none of
above policies fit our requirment under this case. mbind() support
default memory policy, but it requires a NULL nodemask. So obviously
setting allowed memory nodes is cgroups' mission under this case.
So we introduce a new option for mode in numatune named 'restrictive'.







The config above means we only use cgroups to restrict the allowed
memory nodes and not setting any specific memory policies explicitly.

RFC discussion:
https://www.redhat.com/archives/libvir-list/2020-November/msg01256.html

Regards,
Luyao

Luyao Zhong (3):
   docs: add docs for 'restrictive' option for mode in numatune
   schema: add 'restrictive' config option for mode in numatune
   qemu: add parser and formatter for 'restrictive' mode in numatune

  docs/formatdomain.rst |  7 +++-
  docs/schemas/domaincommon.rng |  2 +
  include/libvirt/libvirt-domain.h  |  1 +
  src/conf/numa_conf.c  |  9 +
  src/qemu/qemu_command.c   |  6 ++-
  src/qemu/qemu_process.c   | 27 +
  src/util/virnuma.c|  3 ++
  .../numatune-memnode-invalid-mode.err |  1 +
  .../numatune-memnode-invalid-mode.xml | 33 +++
  ...emnode-restrictive-mode.x86_64-latest.args | 40 +++
  .../numatune-memnode-restrictive-mode.xml | 33 +++
  tests/qemuxml2argvtest.c  |  2 +
  ...memnode-restrictive-mode.x86_64-latest.xml | 40 +++
  tests/qemuxml2xmltest.c   |  1 +
  14 files changed, 202 insertions(+), 3 deletions(-)
  create mode 100644 tests/qemuxml2argvdata/numatune-memnode-invalid-mode.err
  create mode 100644 tests/qemuxml2argvdata/numatune-memnode-invalid-mode.xml
  create mode 100644 
tests/qemuxml2argvdata/numatune-memnode-restrictive-mode.x86_64-latest.args
  create mode 100644 
tests/qemuxml2argvdata/numatune-memnode-restrictive-mode.xml
  create mode 100644 
tests/qemuxml2xmloutdata/numatune-memnode-restrictive-mode.x86_64-latest.xml





[PATCH] virsh-domain: Add quotes around '%s' formatting domain name

2021-01-05 Thread Peter Krempa
Domain name can contain spaces in which case it's not immediately clear
from virsh messages where the boundary of the name is. Enclose all %s
formatters in apostrophes as delimiters.

Done via the following vim regex:

 %s/omain %s/omain '%s'/g

This patch changes:

 $ virsh undefine --snapshots-metadata 'OWASP Broken Web Apps VM v1.2'
 Domain OWASP Broken Web Apps VM v1.2 has been undefined

to:

 $ virsh undefine --snapshots-metadata 'OWASP Broken Web Apps VM v1.2'
 Domain 'OWASP Broken Web Apps VM v1.2' has been undefined

Signed-off-by: Peter Krempa 
---
 tests/virsh-define-dev-segfault |   2 +-
 tests/virsh-read-bufsiz |   2 +-
 tests/virsh-undefine|   8 +-
 tools/virsh-domain.c| 136 
 4 files changed, 74 insertions(+), 74 deletions(-)

diff --git a/tests/virsh-define-dev-segfault b/tests/virsh-define-dev-segfault
index a7533ba9b0..cc5b461932 100755
--- a/tests/virsh-define-dev-segfault
+++ b/tests/virsh-define-dev-segfault
@@ -65,7 +65,7 @@ url=test:///default
 $abs_top_builddir/tools/virsh --connect "$url" 'define D.xml; dumpxml D' > out 
2>&1 || fail=1

 cat > exp < $in || fail=1

   $abs_top_builddir/tools/virsh --connect test:///default define $in > out || 
fail=1
-  printf "Domain newtest defined from $in\n\n" > exp || fail=1
+  printf "Domain 'newtest' defined from $in\n\n" > exp || fail=1
   compare exp out || fail=1
 done

diff --git a/tests/virsh-undefine b/tests/virsh-undefine
index 998d4d3268..dbbb367391 100755
--- a/tests/virsh-undefine
+++ b/tests/virsh-undefine
@@ -37,7 +37,7 @@ sed '/^Persistent/n; /:/d' < out1 > out
 cat <<\EOF > exp || fail=1
 Persistent: yes

-Domain test has been undefined
+Domain 'test' has been undefined

 Persistent: no

@@ -52,7 +52,7 @@ sed '/^Persistent/n; /:/d' < out1 > out
 cat <<\EOF > exp || fail=1
 Persistent: yes

-Domain 1 has been undefined
+Domain '1' has been undefined

 Persistent: no

@@ -64,9 +64,9 @@ $abs_top_builddir/tools/virsh -c test:///default \
 'shutdown test; undefine test; dominfo test' > out 2>&1
 test $? = 1 || fail=1
 cat <<\EOF > expout || fail=1
-Domain test is being shutdown
+Domain 'test' is being shutdown

-Domain test has been undefined
+Domain 'test' has been undefined

 error: failed to get domain 'test'

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 1ef9b8d606..2bb136333f 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1117,17 +1117,17 @@ cmdAutostart(vshControl *ctl, const vshCmd *cmd)

 if (virDomainSetAutostart(dom, autostart) < 0) {
 if (autostart)
-vshError(ctl, _("Failed to mark domain %s as autostarted"), name);
+vshError(ctl, _("Failed to mark domain '%s' as autostarted"), 
name);
 else
-vshError(ctl, _("Failed to unmark domain %s as autostarted"), 
name);
+vshError(ctl, _("Failed to unmark domain '%s' as autostarted"), 
name);
 virshDomainFree(dom);
 return false;
 }

 if (autostart)
-vshPrintExtra(ctl, _("Domain %s marked as autostarted\n"), name);
+vshPrintExtra(ctl, _("Domain '%s' marked as autostarted\n"), name);
 else
-vshPrintExtra(ctl, _("Domain %s unmarked as autostarted\n"), name);
+vshPrintExtra(ctl, _("Domain '%s' unmarked as autostarted\n"), name);

 virshDomainFree(dom);
 return true;
@@ -2987,7 +2987,7 @@ cmdRunConsole(vshControl *ctl, virDomainPtr dom,
 return false;
 }

-vshPrintExtra(ctl, _("Connected to domain %s\n"), virDomainGetName(dom));
+vshPrintExtra(ctl, _("Connected to domain '%s'\n"), virDomainGetName(dom));
 vshPrintExtra(ctl, _("Escape character is %s"), priv->escapeChar);
 if (priv->escapeChar[0] == '^')
 vshPrintExtra(ctl, " (Ctrl + %c)", priv->escapeChar[1]);
@@ -3427,9 +3427,9 @@ cmdSuspend(vshControl *ctl, const vshCmd *cmd)
 return false;

 if (virDomainSuspend(dom) == 0) {
-vshPrintExtra(ctl, _("Domain %s suspended\n"), name);
+vshPrintExtra(ctl, _("Domain '%s' suspended\n"), name);
 } else {
-vshError(ctl, _("Failed to suspend domain %s"), name);
+vshError(ctl, _("Failed to suspend domain '%s'"), name);
 ret = false;
 }

@@ -3501,12 +3501,12 @@ cmdDomPMSuspend(vshControl *ctl, const vshCmd *cmd)
 }

 if (virDomainPMSuspendForDuration(dom, suspendTarget, duration, 0) < 0) {
-vshError(ctl, _("Domain %s could not be suspended"),
+vshError(ctl, _("Domain '%s' could not be suspended"),
  virDomainGetName(dom));
 goto cleanup;
 }

-vshPrintExtra(ctl, _("Domain %s successfully suspended"),
+vshPrintExtra(ctl, _("Domain '%s' successfully suspended"),
  virDomainGetName(dom));

 ret = true;
@@ -3548,12 +3548,12 @@ cmdDomPMWakeup(vshControl *ctl, const vshCmd *cmd)
 return false;

 if (virDomainPMWakeup(dom, flags) < 0) {
-vshError(ctl, _("Domain %s 

Re: [PATCH v2] qemu: Don't prealloc mem for real NVDIMMs

2021-01-05 Thread Daniel P . Berrangé
On Tue, Jan 05, 2021 at 12:39:00PM +0100, Michal Privoznik wrote:
> Currently, we configure QEMU to prealloc memory almost by
> default. Well, by default for NVDIMMs, hugepages and if user
> asked us to (via memoryBacking ).
> 
> However, when guest's NVDIMM is backed by real life NVDIMM this
> approach is not the best. In this case users should put 
> into the  device , like this:
> 
>   
> 
>   /dev/pmem0
>   
> 
>   
> 
> Instructing QEMU to do prealloc in this case means that each
> page of the NVDIMM is "touched" (the first byte is read and
> written back - see QEMU commit v2.9.0-rc1~26^2) which cripples
> device wear.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1894053
> Signed-off-by: Michal Privoznik 
> ---
> 
> v2 of:
> 
> https://www.redhat.com/archives/libvir-list/2020-November/msg01568.html
> 
> diff to v1:
> - I've dropped the first hunk of v1 which forbade prealloc even if user
>   requested it explicitly.
> 
>  src/qemu/qemu_command.c  | 5 -
>  .../memory-hotplug-nvdimm-pmem.x86_64-latest.args| 2 +-
>  2 files changed, 5 insertions(+), 2 deletions(-)

Reviewed-by: Daniel P. Berrangé 


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [PATCH] qemu: Properly handle setting of for empty cdrom

2021-01-05 Thread Daniel Henrique Barboza




On 1/5/21 8:22 AM, Peter Krempa wrote:

When starting a VM with an empty cdrom which has  configured the
startup fails as qemu is not happy about setting tuning for an empty
drive:

  error: internal error: unable to execute 'block_set_io_throttle', unexpected 
error: 'Device has no medium'

Resolve this by skipping the setting of throttling for empty drives and
updating the throttling when new medium is inserted into the drive.

Resolves: https://gitlab.com/libvirt/libvirt/-/issues/111
Signed-off-by: Peter Krempa 
---



Reviewed-by: Daniel Henrique Barboza 


  src/qemu/qemu_hotplug.c | 10 ++
  src/qemu/qemu_process.c |  4 
  2 files changed, 14 insertions(+)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 9b93f256e8..57635cd419 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -558,6 +558,16 @@ qemuDomainChangeMediaBlockdev(virQEMUDriverPtr driver,
   nodename);
  }

+/* set throttling for the new image */
+if (rc == 0 &&
+!virStorageSourceIsEmpty(newsrc) &&
+qemuDiskConfigBlkdeviotuneEnabled(disk)) {
+rc = qemuMonitorSetBlockIoThrottle(priv->mon, NULL,
+   diskPriv->qomName,
+   >blkdeviotune,
+   true, true, true);
+}
+
  if (rc == 0)
  rc = qemuMonitorBlockdevTrayClose(priv->mon, diskPriv->qomName);

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index e7421b415f..414e9327d2 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6838,6 +6838,10 @@ qemuProcessSetupDiskThrottlingBlockdev(virQEMUDriverPtr 
driver,
  if (qemuDiskBusIsSD(disk->bus))
  continue;

+/* Setting throttling for empty drives fails */
+if (virStorageSourceIsEmpty(disk->src))
+continue;
+
  if (!qemuDiskConfigBlkdeviotuneEnabled(disk))
  continue;





[PATCH] networkGetDHCPLeases: Don't assign @ipdef_tmp twice

2021-01-05 Thread Michal Privoznik
When rewriting the function, I've mistakenly declared a variable
and assigned it to itself. Let's initialize the variable properly.

Fixes: 5fb6d98c881c42ab41ca72060217b846949a438f
Signed-off-by: Michal Privoznik 
---

Pushed as trivial.

 src/network/bridge_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 22d7d330a3..a7c5aade14 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -4155,7 +4155,7 @@ networkGetDHCPLeases(virNetworkPtr net,
 
 /* Obtain prefix */
 for (j = 0; j < def->nips; j++) {
-virNetworkIPDefPtr ipdef_tmp = ipdef_tmp = >ips[j];
+virNetworkIPDefPtr ipdef_tmp = >ips[j];
 
 if (ipv6 && VIR_SOCKET_ADDR_IS_FAMILY(_tmp->address,
   AF_INET6)) {
-- 
2.26.2



Re: [PATCH 08/10] network: Rework networkGetDHCPLeases()

2021-01-05 Thread Michal Privoznik

On 1/5/21 12:49 PM, John Ferlan wrote:



On 12/18/20 10:09 AM, Michal Privoznik wrote:

Firstly, bring variables that are used only within loops into
their respective loops. Secondly, drop 'error' label which is
redundant since we have @rv which holds the return value.
Thirdly, fix indendation in one case, the rest is indented
properly.

Signed-off-by: Michal Privoznik 
---
  src/network/bridge_driver.c | 47 +
  1 file changed, 22 insertions(+), 25 deletions(-)



[...]


@@ -4145,7 +4146,7 @@ networkGetDHCPLeases(virNetworkPtr net,
  /* A lease without ip-address makes no sense */
  virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
 _("found lease without ip-address"));
-goto error;
+goto cleanup;
  }
  
  /* Unlike IPv4, IPv6 uses ':' instead of '.' as separator */

@@ -4154,7 +4155,7 @@ networkGetDHCPLeases(virNetworkPtr net,
  
  /* Obtain prefix */

  for (j = 0; j < def->nips; j++) {
-ipdef_tmp = >ips[j];
+virNetworkIPDefPtr ipdef_tmp = ipdef_tmp = >ips[j];


Coverity notes... Department of redundancy department

Simple/trivial fix for someone I'm sure


Huh, how I managed to write this? :-)

I'll push the fix as trivial shortly. Thanks for noticing.

Michal



Re: [PATCH 08/10] network: Rework networkGetDHCPLeases()

2021-01-05 Thread John Ferlan



On 12/18/20 10:09 AM, Michal Privoznik wrote:
> Firstly, bring variables that are used only within loops into
> their respective loops. Secondly, drop 'error' label which is
> redundant since we have @rv which holds the return value.
> Thirdly, fix indendation in one case, the rest is indented
> properly.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/network/bridge_driver.c | 47 +
>  1 file changed, 22 insertions(+), 25 deletions(-)
> 

[...]

> @@ -4145,7 +4146,7 @@ networkGetDHCPLeases(virNetworkPtr net,
>  /* A lease without ip-address makes no sense */
>  virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("found lease without ip-address"));
> -goto error;
> +goto cleanup;
>  }
>  
>  /* Unlike IPv4, IPv6 uses ':' instead of '.' as separator */
> @@ -4154,7 +4155,7 @@ networkGetDHCPLeases(virNetworkPtr net,
>  
>  /* Obtain prefix */
>  for (j = 0; j < def->nips; j++) {
> -ipdef_tmp = >ips[j];
> +virNetworkIPDefPtr ipdef_tmp = ipdef_tmp = >ips[j];

Coverity notes... Department of redundancy department

Simple/trivial fix for someone I'm sure

John

>  
>  if (ipv6 && VIR_SOCKET_ADDR_IS_FAMILY(_tmp->address,
>AF_INET6)) {
> @@ -4162,7 +4163,7 @@ networkGetDHCPLeases(virNetworkPtr net,

[...]



Re: [PATCH v2] qemu: Don't prealloc mem for real NVDIMMs

2021-01-05 Thread Daniel Henrique Barboza




On 1/5/21 8:39 AM, Michal Privoznik wrote:

Currently, we configure QEMU to prealloc memory almost by
default. Well, by default for NVDIMMs, hugepages and if user
asked us to (via memoryBacking ).

However, when guest's NVDIMM is backed by real life NVDIMM this
approach is not the best. In this case users should put 
into the  device , like this:

   
 
   /dev/pmem0
   
 
   

Instructing QEMU to do prealloc in this case means that each
page of the NVDIMM is "touched" (the first byte is read and
written back - see QEMU commit v2.9.0-rc1~26^2) which cripples
device wear.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1894053
Signed-off-by: Michal Privoznik 
---

v2 of:

https://www.redhat.com/archives/libvir-list/2020-November/msg01568.html

diff to v1:
- I've dropped the first hunk of v1 which forbade prealloc even if user
   requested it explicitly.

  src/qemu/qemu_command.c  | 5 -
  .../memory-hotplug-nvdimm-pmem.x86_64-latest.args| 2 +-
  2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index b06a086e18..f572ed64c1 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3085,7 +3085,10 @@ qemuBuildMemoryBackendProps(virJSONValuePtr 
*backendProps,
  
  if (mem->nvdimmPath) {

  memPath = g_strdup(mem->nvdimmPath);
-prealloc = true;
+/* If the NVDIMM is a real device then there's nothing to prealloc.
+ * If anyhing, we would be only wearing off the device. */


s/anyhing/anything

Reviewed-by: Daniel Henrique Barboza 


+if (!mem->nvdimmPmem)
+prealloc = true;
  } else if (useHugepage) {
  if (qemuGetDomainHupageMemPath(priv->driver, def, pagesize, 
) < 0)
  return -1;
diff --git 
a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-pmem.x86_64-latest.args 
b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-pmem.x86_64-latest.args
index cac02a6f6d..fb4ae4b518 100644
--- a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-pmem.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-pmem.x86_64-latest.args
@@ -20,7 +20,7 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
  -object memory-backend-ram,id=ram-node0,size=224395264 \
  -numa node,nodeid=0,cpus=0-1,memdev=ram-node0 \
  -object memory-backend-file,id=memnvdimm0,mem-path=/tmp/nvdimm,share=no,\
-prealloc=yes,size=536870912,pmem=yes \
+size=536870912,pmem=yes \
  -device nvdimm,node=0,memdev=memnvdimm0,id=nvdimm0,slot=0 \
  -uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
  -display none \





[PATCH v2] qemu: Don't prealloc mem for real NVDIMMs

2021-01-05 Thread Michal Privoznik
Currently, we configure QEMU to prealloc memory almost by
default. Well, by default for NVDIMMs, hugepages and if user
asked us to (via memoryBacking ).

However, when guest's NVDIMM is backed by real life NVDIMM this
approach is not the best. In this case users should put 
into the  device , like this:

  

  /dev/pmem0
  

  

Instructing QEMU to do prealloc in this case means that each
page of the NVDIMM is "touched" (the first byte is read and
written back - see QEMU commit v2.9.0-rc1~26^2) which cripples
device wear.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1894053
Signed-off-by: Michal Privoznik 
---

v2 of:

https://www.redhat.com/archives/libvir-list/2020-November/msg01568.html

diff to v1:
- I've dropped the first hunk of v1 which forbade prealloc even if user
  requested it explicitly.

 src/qemu/qemu_command.c  | 5 -
 .../memory-hotplug-nvdimm-pmem.x86_64-latest.args| 2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index b06a086e18..f572ed64c1 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3085,7 +3085,10 @@ qemuBuildMemoryBackendProps(virJSONValuePtr 
*backendProps,
 
 if (mem->nvdimmPath) {
 memPath = g_strdup(mem->nvdimmPath);
-prealloc = true;
+/* If the NVDIMM is a real device then there's nothing to prealloc.
+ * If anyhing, we would be only wearing off the device. */
+if (!mem->nvdimmPmem)
+prealloc = true;
 } else if (useHugepage) {
 if (qemuGetDomainHupageMemPath(priv->driver, def, pagesize, 
) < 0)
 return -1;
diff --git 
a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-pmem.x86_64-latest.args 
b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-pmem.x86_64-latest.args
index cac02a6f6d..fb4ae4b518 100644
--- a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-pmem.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-pmem.x86_64-latest.args
@@ -20,7 +20,7 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
 -object memory-backend-ram,id=ram-node0,size=224395264 \
 -numa node,nodeid=0,cpus=0-1,memdev=ram-node0 \
 -object memory-backend-file,id=memnvdimm0,mem-path=/tmp/nvdimm,share=no,\
-prealloc=yes,size=536870912,pmem=yes \
+size=536870912,pmem=yes \
 -device nvdimm,node=0,memdev=memnvdimm0,id=nvdimm0,slot=0 \
 -uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
 -display none \
-- 
2.26.2



Re: [libvirt PATCH v2 3/5] vmx: Make virVMXParseFileName return an integer

2021-01-05 Thread Martin Kletzander

On Tue, Jan 05, 2021 at 10:15:47AM +0100, Michal Privoznik wrote:

On 1/5/21 12:32 AM, Daniel Henrique Barboza wrote:


On 12/21/20 1:19 PM, Martin Kletzander wrote:

And return the actual extracted value in a parameter.  This way we can
later
return success even without any extracted value.

Signed-off-by: Martin Kletzander 
---
  src/esx/esx_driver.c | 31 ++-
  src/vmware/vmware_conf.c | 10 +-
  src/vmx/vmx.c    | 21 ++---
  src/vmx/vmx.h    |  2 +-
  tests/vmx2xmltest.c  | 19 ++-
  5 files changed, 44 insertions(+), 39 deletions(-)




diff --git a/src/vmx/vmx.h b/src/vmx/vmx.h
index df5d39157b99..e5420c970a4b 100644
--- a/src/vmx/vmx.h
+++ b/src/vmx/vmx.h
@@ -36,7 +36,7 @@ virDomainXMLOptionPtr
virVMXDomainXMLConfInit(virCapsPtr caps);
   * Context
   */
-typedef char * (*virVMXParseFileName)(const char *fileName, void
*opaque);
+typedef int (*virVMXParseFileName)(const char *fileName, void
*opaque, char **src);




This change is making the build break in my env:

../src/vmware/vmware_conf.c: In function ‘vmwareLoadDomains’:
../src/vmware/vmware_conf.c:142:23: error: assignment to
‘virVMXParseFileName’ {aka ‘int (*)(const char *, void *, char **)’}
from incompatible pointer type ‘char * (*)(const char *, void *)’
[-Werror=incompatible-pointer-types]
   142 | ctx.parseFileName = vmwareCopyVMXFileName;
   |   ^
../src/vmware/vmware_conf.c: At top level:
../src/vmware/vmware_conf.c:511:1: error: conflicting types for
‘vmwareCopyVMXFileName’
   511 | vmwareCopyVMXFileName(const char *datastorePath, void *opaque
G_GNUC_UNUSED,
   | ^
In file included from ../src/vmware/vmware_conf.c:32:
../src/vmware/vmware_conf.h:86:7: note: previous declaration of
‘vmwareCopyVMXFileName’ was here
    86 | char *vmwareCopyVMXFileName(const char *datastorePath, void
*opaque);
   |   ^
cc1: all warnings being treated as errors

(...)

../src/vmware/vmware_driver.c: In function
‘vmwareConnectDomainXMLFromNative’:
../src/vmware/vmware_driver.c:953:23: error: assignment to
‘virVMXParseFileName’ {aka ‘int (*)(const char *, void *, char **)’}
from incompatible pointer type ‘char * (*)(const char *, void *)’
[-Werror=incompatible-pointer-types]
   953 | ctx.parseFileName = vmwareCopyVMXFileName;
   |   ^
cc1: all warnings being treated as errors



I believe there are a handful of virVMXParseFileName impl instances that
needs
to be changed as well.



Indeed. I think we will need to change virVMXFormatFileName() too at the
same time, because of vmwareCopyVMXFileName() which is passed as
formatFileName callback.



Not only that, but vmware_driver uses the smae function for parsing and
formatting.  They are, however, just g_strdup()s, so I can split them no
problem.  Thanks for noticing.

Anyway, just looking at the commits they should be done a bit differently, I had
some cock-ups there, I guess.  I'll send a v3.


BTW: I've found out that we don't automatically enable vmware driver. I
had to enable it explicitly:

meson -Dsystem=true -Ddriver_vmware=enabled _build

I'm looking into that.



We're not building it downstream even, I think, I managed to fix it for ESX
where the issue was happening actually.  The vmware driver just controls vmware
player/workstation locally.  Are there actually people using that?


Michal


signature.asc
Description: PGP signature


[PATCH] qemu: Properly handle setting of for empty cdrom

2021-01-05 Thread Peter Krempa
When starting a VM with an empty cdrom which has  configured the
startup fails as qemu is not happy about setting tuning for an empty
drive:

 error: internal error: unable to execute 'block_set_io_throttle', unexpected 
error: 'Device has no medium'

Resolve this by skipping the setting of throttling for empty drives and
updating the throttling when new medium is inserted into the drive.

Resolves: https://gitlab.com/libvirt/libvirt/-/issues/111
Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_hotplug.c | 10 ++
 src/qemu/qemu_process.c |  4 
 2 files changed, 14 insertions(+)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 9b93f256e8..57635cd419 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -558,6 +558,16 @@ qemuDomainChangeMediaBlockdev(virQEMUDriverPtr driver,
  nodename);
 }

+/* set throttling for the new image */
+if (rc == 0 &&
+!virStorageSourceIsEmpty(newsrc) &&
+qemuDiskConfigBlkdeviotuneEnabled(disk)) {
+rc = qemuMonitorSetBlockIoThrottle(priv->mon, NULL,
+   diskPriv->qomName,
+   >blkdeviotune,
+   true, true, true);
+}
+
 if (rc == 0)
 rc = qemuMonitorBlockdevTrayClose(priv->mon, diskPriv->qomName);

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index e7421b415f..414e9327d2 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6838,6 +6838,10 @@ qemuProcessSetupDiskThrottlingBlockdev(virQEMUDriverPtr 
driver,
 if (qemuDiskBusIsSD(disk->bus))
 continue;

+/* Setting throttling for empty drives fails */
+if (virStorageSourceIsEmpty(disk->src))
+continue;
+
 if (!qemuDiskConfigBlkdeviotuneEnabled(disk))
 continue;

-- 
2.29.2



Re: [libvirt PATCH v3 04/21] nodedev: expose internal helper for naming devices

2021-01-05 Thread Erik Skultety
On Thu, Dec 24, 2020 at 08:14:28AM -0600, Jonathon Jongsma wrote:
> Expose a helper function that can be used by udev and mdevctl to
> generate device names for node devices.
> 
> Signed-off-by: Jonathon Jongsma 
> ---
Reviewed-by: Erik Skultety 



Re: [libvirt PATCH v3 16/21] api: add virNodeDeviceUndefine()

2021-01-05 Thread Erik Skultety
On Thu, Dec 24, 2020 at 08:14:40AM -0600, Jonathon Jongsma wrote:
> This interface allows you to undefine a persistently defined (but
> inactive) mediated devices. It is implemented via 'mdevctl'
> 
> Signed-off-by: Jonathon Jongsma 
> ---

This patch doesn't pass tests, because
tests/nodedevmdevctldata/mdevctl-undefine.argv is missing.

I'll post the review in a separate reply.

Erik



[PATCH v1] docs: migration: remove xenmigr

2021-01-05 Thread Olaf Hering
xenmigr was for xend

Fixes commit 1dac5f06a0341e8087dc33af75c8352d77a4

Signed-off-by: Olaf Hering 
---
 docs/migration.html.in | 6 --
 1 file changed, 6 deletions(-)

diff --git a/docs/migration.html.in b/docs/migration.html.in
index ac38c6e13d..627200f96a 100644
--- a/docs/migration.html.in
+++ b/docs/migration.html.in
@@ -491,7 +491,6 @@ virsh migrate web1 xen+tls://desthost/system
 eg using secondary network interface
 
 virsh migrate web1 qemu://desthost/system tcp://10.0.0.1/
-virsh migrate web1 xen+tcp://desthost/system  xenmigr:10.0.0.1/
 
 
 
@@ -562,13 +561,8 @@ syntax: virsh migrate GUESTNAME HV-URI
 
 eg using same libvirt URI for all connections
 
-virsh migrate --direct web1 xenmigr://desthost/
 
 
-
-  Supported by Xen driver
-
-
 Native migration, peer2peer between two 
libvirtd servers
 
 



Re: [libvirt PATCH] docs: Fix dead link

2021-01-05 Thread Michal Privoznik

On 12/21/20 12:57 PM, Martin Kletzander wrote:

Reviewed-by: Martin Kletzander 


Pushed now.

Michal



Re: [PATCH] update ci dockerfile from Leap 15.1 to 15.2

2021-01-05 Thread Daniel P . Berrangé
On Tue, Jan 05, 2021 at 05:56:13PM +0800, Cho, Yu-Chen wrote:
> Signed-off-by: Cho, Yu-Chen 


> diff --git a/ci/containers/ci-opensuse-151.Dockerfile 
> b/ci/containers/ci-opensuse-152.Dockerfile
> similarity index 98%
> rename from ci/containers/ci-opensuse-151.Dockerfile
> rename to ci/containers/ci-opensuse-152.Dockerfile
> index 9458d2de0c..def45cbe3f 100644
> --- a/ci/containers/ci-opensuse-151.Dockerfile
> +++ b/ci/containers/ci-opensuse-152.Dockerfile
> @@ -3,7 +3,7 @@
>  #  $ lcitool dockerfile opensuse-151 libvirt
>  #
>  # 
> https://gitlab.com/libvirt/libvirt-ci/-/commit/b098ec6631a85880f818f2dd25c437d509e53680
> -FROM registry.opensuse.org/opensuse/leap:15.1
> +FROM registry.opensuse.org/opensuse/leap:15.2

the comment just above this line says this file is auto-generated
and thus should not be manually changed.

To update to 15.2 requires changing  libvirt-ci.git contents instead.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [libvirt PATCH 2/2] rpm: fix ownership of the swtpm log directory

2021-01-05 Thread Daniel P . Berrangé
On Mon, Jan 04, 2021 at 06:05:17PM +, Daniel P. Berrangé wrote:
> As soon as a guest using a  device is launched, libvirt will change
> the ownership to 'tss' user and group, which will cause RPM verify to
> then fail.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  libvirt.spec.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libvirt.spec.in b/libvirt.spec.in
> index 2e026b0423..c455aa7788 100644
> --- a/libvirt.spec.in
> +++ b/libvirt.spec.in
> @@ -1747,7 +1747,7 @@ exit 0
>  %{_datadir}/augeas/lenses/tests/test_libvirtd_qemu.aug
>  %{_libdir}/%{name}/connection-driver/libvirt_driver_qemu.so
>  %dir %attr(0711, root, root) %{_localstatedir}/lib/libvirt/swtpm/
> -%dir %attr(0711, root, root) %{_localstatedir}/log/swtpm/libvirt/qemu/
> +%dir %attr(0711, tss, tss) %{_localstatedir}/log/swtpm/libvirt/qemu/

Mode should have been changed to 0730 too, since that is what the
code (strangely) uses right now.


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [PATCH] Enable VMware driver by default

2021-01-05 Thread Daniel P . Berrangé
On Tue, Jan 05, 2021 at 10:51:02AM +0100, Michal Privoznik wrote:
> During rewrite to meson it was mistakenly disabled. Originally,
> we had:
> 
>   LIBVIRT_ARG_WITH_FEATURE([VMWARE], [VMware], [yes])
> 
> which enabled the driver by default. But in meson we are checking
> whether the 'driver_vmware' option is enabled without anything
> enabling it automagically.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  meson.build | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Daniel P. Berrangé 

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



[PATCH] Enable VMware driver by default

2021-01-05 Thread Michal Privoznik
During rewrite to meson it was mistakenly disabled. Originally,
we had:

  LIBVIRT_ARG_WITH_FEATURE([VMWARE], [VMware], [yes])

which enabled the driver by default. But in meson we are checking
whether the 'driver_vmware' option is enabled without anything
enabling it automagically.

Signed-off-by: Michal Privoznik 
---
 meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index 3f3412975b..b5164f68ed 100644
--- a/meson.build
+++ b/meson.build
@@ -1790,7 +1790,7 @@ if not get_option('driver_vbox').disabled() and 
conf.has('WITH_LIBVIRTD')
   conf.set_quoted('VBOX_XPCOMC_DIR', get_option('vbox_xpcomc_dir'))
 endif
 
-if get_option('driver_vmware').enabled()
+if not get_option('driver_vmware').disabled()
   conf.set('WITH_VMWARE', 1)
   conf.set('WITH_VMX', 1)
 endif
-- 
2.26.2



回复: [PATCH] Fix wrong use of path variable

2021-01-05 Thread 李亚磊
I also realized this is already fixed.






Thank you



发送自 Windows 10 版邮件应用

发件人: Peter Krempa
发送时间: 2021年1月4日 21:15
收件人: liyalei
抄送: libvir-list@redhat.com; liyalei
主题: Re: [PATCH] Fix wrong use of path variable

On Wed, Dec 30, 2020 at 18:39:35 +0800, liyalei wrote:
> From: liyalei 

Please provide some more description in the commit message, such as
describe the bug that this patch is fixing.

Additionally the libvirt project requires [1] you to certify that you
are in compliance of the 'Developer certificate of origin' [2] by
providing the appropriate tag in the commit message (read the docs to
figure out which).

[1]: https://libvirt.org/hacking.html#developer-certificate-of-origin
[2]: https://developercertificate.org/

> 
> ---
>  src/util/virnetdevopenvswitch.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c
> index d380b0cf22..7eabaa763d 100644
> --- a/src/util/virnetdevopenvswitch.c
> +++ b/src/util/virnetdevopenvswitch.c
> @@ -494,7 +494,7 @@ virNetDevOpenvswitchGetVhostuserIfname(const char *path,
>  if (server) {
>  virCommandAddArgList(cmd, "--no-headings", "--columns=name", "find",
>   "Interface", NULL);
> -virCommandAddArgPair(cmd, "options:vhost-server-path", "path");
> +virCommandAddArgPair(cmd, "options:vhost-server-path", path);

Looks like this is already fixed:

commit 0dd029b7f2e7d9619ca6a22f43857aafb449c3a7
Author: Michal Prívozník 
Date:   Wed Dec 16 18:52:48 2020 +0100

virNetDevOpenvswitchGetVhostuserIfname: Actually use @path to lookup 
interface

In v6.10.0-rc1~221 I wanted to make virNetDevOpenvswitchGetVhostuserIfname()
lookup interface name even for vhostuser interfaces with mode='server'. For
these, we are given a socket path which is then created by QEMU and to which
OpenVSwitch connects to and creates an interface. Because of this, we don't
know the name of the interface upfront (when starting QEMU) and have to use
the path to query OpenVSwitch later (using ovs-vsctl). What I intended to 
use
was:

  ovs-vsctl --no-headings --columns=name find Interface 
options:vhost-server-path=$path

But what my code does is:

  ovs-vsctl --no-headings --columns=name find Interface 
options:vhost-server-path=path

and it's all because the argument to the function is named "path"
which I then enclosed in double quotes while it should have been
used as a variable.

Fixes: e4c29e2904197472919d050c67acfd59f0144bbc
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1767013
Signed-off-by: Michal Privoznik 
Reviewed-by: Laine Stump 

diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c
index d380b0cf22..7eabaa763d 100644
--- a/src/util/virnetdevopenvswitch.c
+++ b/src/util/virnetdevopenvswitch.c
@@ -494,7 +494,7 @@ virNetDevOpenvswitchGetVhostuserIfname(const char *path,
 if (server) {
 virCommandAddArgList(cmd, "--no-headings", "--columns=name", "find",
  "Interface", NULL);
-virCommandAddArgPair(cmd, "options:vhost-server-path", "path");
+virCommandAddArgPair(cmd, "options:vhost-server-path", path);
 } else {
 const char *tmpIfname = NULL;


>  } else {
>  const char *tmpIfname = NULL;
>  
> -- 
> 2.27.0
> 




Re: [libvirt PATCH v2 3/5] vmx: Make virVMXParseFileName return an integer

2021-01-05 Thread Michal Privoznik

On 1/5/21 12:32 AM, Daniel Henrique Barboza wrote:


On 12/21/20 1:19 PM, Martin Kletzander wrote:
And return the actual extracted value in a parameter.  This way we can 
later

return success even without any extracted value.

Signed-off-by: Martin Kletzander 
---
  src/esx/esx_driver.c | 31 ++-
  src/vmware/vmware_conf.c | 10 +-
  src/vmx/vmx.c    | 21 ++---
  src/vmx/vmx.h    |  2 +-
  tests/vmx2xmltest.c  | 19 ++-
  5 files changed, 44 insertions(+), 39 deletions(-)




diff --git a/src/vmx/vmx.h b/src/vmx/vmx.h
index df5d39157b99..e5420c970a4b 100644
--- a/src/vmx/vmx.h
+++ b/src/vmx/vmx.h
@@ -36,7 +36,7 @@ virDomainXMLOptionPtr 
virVMXDomainXMLConfInit(virCapsPtr caps);

   * Context
   */
-typedef char * (*virVMXParseFileName)(const char *fileName, void 
*opaque);
+typedef int (*virVMXParseFileName)(const char *fileName, void 
*opaque, char **src);




This change is making the build break in my env:

../src/vmware/vmware_conf.c: In function ‘vmwareLoadDomains’:
../src/vmware/vmware_conf.c:142:23: error: assignment to 
‘virVMXParseFileName’ {aka ‘int (*)(const char *, void *, char **)’} 
from incompatible pointer type ‘char * (*)(const char *, void *)’ 
[-Werror=incompatible-pointer-types]

   142 | ctx.parseFileName = vmwareCopyVMXFileName;
   |   ^
../src/vmware/vmware_conf.c: At top level:
../src/vmware/vmware_conf.c:511:1: error: conflicting types for 
‘vmwareCopyVMXFileName’
   511 | vmwareCopyVMXFileName(const char *datastorePath, void *opaque 
G_GNUC_UNUSED,

   | ^
In file included from ../src/vmware/vmware_conf.c:32:
../src/vmware/vmware_conf.h:86:7: note: previous declaration of 
‘vmwareCopyVMXFileName’ was here
    86 | char *vmwareCopyVMXFileName(const char *datastorePath, void 
*opaque);

   |   ^
cc1: all warnings being treated as errors

(...)

../src/vmware/vmware_driver.c: In function 
‘vmwareConnectDomainXMLFromNative’:
../src/vmware/vmware_driver.c:953:23: error: assignment to 
‘virVMXParseFileName’ {aka ‘int (*)(const char *, void *, char **)’} 
from incompatible pointer type ‘char * (*)(const char *, void *)’ 
[-Werror=incompatible-pointer-types]

   953 | ctx.parseFileName = vmwareCopyVMXFileName;
   |   ^
cc1: all warnings being treated as errors



I believe there are a handful of virVMXParseFileName impl instances that 
needs

to be changed as well.



Indeed. I think we will need to change virVMXFormatFileName() too at the 
same time, because of vmwareCopyVMXFileName() which is passed as 
formatFileName callback.


BTW: I've found out that we don't automatically enable vmware driver. I 
had to enable it explicitly:


meson -Dsystem=true -Ddriver_vmware=enabled _build

I'm looking into that.

Michal