Re: recommendations for handling Hyper-V version number

[Matt Coleman]

>> For example, the Windows Server 2016 Hyper-V version is 10.0.14393.0,
>> so its “micro” is over 14 times larger than the encoding allows.
>> 
>> My current workaround is to truncate it to something that works (E.G.
>> 10.0.143), but that's clearly less than ideal.
> The problem here is the virConnectGetVersion public API - we cannot
> alter its signature.
I hadn’t realized this was exposed by the public API.

Since the API can’t change, should I update my code to only output 
major.minor instead of truncating micro to an incorrect/misleading 
value?

The downside of this is that Microsoft has referred to Windows 10 as 
“the last version of Windows” and Hyper-V versions are the same as the 
OS’s kernel version. So, this would result in libvirt’s Hyper-V driver 
reporting version 10.0 until Microsoft reconsiders that approach.

> One solution would be to introduce a new API (that possibly returns a
> string), but it will take time until apps start using it.
That would be necessary to be able to convey the full Hyper-V version,
but I’m not sure if that’s needed anywhere at the moment. Also, I’d
like to flesh out Hyper-V support within the current API before adding
new functionality.

Matt

Re: Help on Meson build Error

[Wei Wang]

On Fri, Sep 25, 2020 at 10:03 AM Wei Wang 
wrote:

> On Thu, Sep 24, 2020 at 2:58 PM Ján Tomko  wrote:
>
>> On a Thursday in 2020, Wei Wang wrote:
>> >Seems it didn't appear on the mailing list, resent it.
>> >
>> > Hi folks,
>> >
>> >I'm trying to build libvirt using meson with the latest upstream libvirt,
>> >but the compilation fails:
>> >(followed on https://libvirt.org/compiling.html, not sure if any
>> >dependencies are missed. I checked, src/util/libvirt_util.a.p does not
>> >exist.)
>> >
>> >FAILED: src/util/libvirt_util.a.p/glibcompat.c.o
>> >
>> >In file included from ../src/util/glibcompat.c:19:
>> >./config.h:1026:10: fatal error: config-post.h: No such file or directory
>> >FAILED: src/util/libvirt_util.a.p/virfirmware.c.o
>> >In file included from ../src/util/virarch.c:22:
>> >./config.h:1026:10: fatal error: config-post.h: No such file or directory
>> >FAILED: src/util/libvirt_util.a.p/viraudit.c.o
>> >In file included from ../src/util/viraudit.c:22:
>> >./config.h:1026:10: fatal error: config-post.h: No such file or directory
>> >
>>
>> config-post.h was moved to config.h when we converted to Meson.
>>
>> There are probably some leftovers from a previous build in your build
>> directory. Can you try it with an empty build directory?
>>
>>
> Thanks, just done cloning a new one from the official repo, but meson
> build reports an error:
> src/util/meson.build:138:0: ERROR: Program
> '/opt/projects/libvirt/src/keycodemapdb/tools/keymap-gen' not found
>
> Is there anything I should do before "meson build"?
>

OK, solved this one with network proxy (seems that meson needs to download
it).
 ninja -C build fails with

ninja: Entering directory `build'
[6/908] Compiling C object src/util/libvirt_util.a.p/virfile.c.o
FAILED: src/util/libvirt_util.a.p/virfile.c.o
cc -Isrc/util/libvirt_util.a.p -Isrc/util -I../src/util -Iinclude
-I../include -Isrc -I../src -I. -I.. -I/usr/include/p11-kit-1
-I/usr/include/libnl3 -I/usr/include/glib-2.0
-I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/include/gio-unix-2.0
-I/usr/include/libmount -I/usr/include/blkid -I/usr/include/libxml2
-I/usr/include/yajl -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64
-Wall -Winvalid-pch -Wextra -std=gnu99 -O2 -g -Werror -fno-common -W
-Wabsolute-value -Waddress -Waddress-of-packed-member
-Waggressive-loop-optimizations -Wall -Wattribute-warning -Wattributes
-Wbool-compare -Wbool-operation -Wbuiltin-declaration-mismatch
-Wbuiltin-macro-redefined -Wcannot-profile -Wcast-align -Wcast-align=strict
-Wcast-function-type -Wchar-subscripts -Wclobbered -Wcomment -Wcomments
-Wcoverage-mismatch -Wcpp -Wdangling-else -Wdate-time
-Wdeclaration-after-statement -Wdeprecated-declarations -Wdesignated-init
-Wdiscarded-array-qualifiers -Wdiscarded-qualifiers -Wdiv-by-zero
-Wduplicated-cond -Wduplicate-decl-specifier -Wempty-body -Wendif-labels
-Wexpansion-to-defined -Wextra -Wformat-contains-nul -Wformat-extra-args
-Wformat-nonliteral -Wformat-security -Wformat-y2k -Wformat-zero-length
-Wframe-address -Wfree-nonheap-object -Whsa -Wif-not-aligned
-Wignored-attributes -Wignored-qualifiers -Wimplicit
-Wimplicit-function-declaration -Wimplicit-int -Wincompatible-pointer-types
-Winit-self -Winline -Wint-conversion -Wint-in-bool-context
-Wint-to-pointer-cast -Winvalid-memory-model -Winvalid-pch
-Wlogical-not-parentheses -Wlogical-op -Wmain -Wmaybe-uninitialized
-Wmemset-elt-size -Wmemset-transposed-args -Wmisleading-indentation
-Wmissing-attributes -Wmissing-braces -Wmissing-declarations
-Wmissing-field-initializers -Wmissing-include-dirs
-Wmissing-parameter-type -Wmissing-profile -Wmissing-prototypes -Wmultichar
-Wmultistatement-macros -Wnarrowing -Wnested-externs -Wnonnull
-Wnonnull-compare -Wnull-dereference -Wodr -Wold-style-declaration
-Wold-style-definition -Wopenmp-simd -Woverflow -Woverride-init
-Wpacked-bitfield-compat -Wpacked-not-aligned -Wparentheses -Wpointer-arith
-Wpointer-compare -Wpointer-sign -Wpointer-to-int-cast -Wpragmas -Wpsabi
-Wrestrict -Wreturn-local-addr -Wreturn-type -Wscalar-storage-order
-Wsequence-point -Wshadow -Wshift-count-negative -Wshift-count-overflow
-Wshift-negative-value -Wsizeof-array-argument -Wsizeof-pointer-div
-Wsizeof-pointer-memaccess -Wstrict-aliasing -Wstrict-prototypes
-Wstringop-truncation -Wsuggest-attribute=cold -Wsuggest-attribute=const
-Wsuggest-attribute=format -Wsuggest-attribute=noreturn
-Wsuggest-attribute=pure -Wsuggest-final-methods -Wsuggest-final-types
-Wswitch -Wswitch-bool -Wswitch-unreachable -Wsync-nand
-Wtautological-compare -Wtrampolines -Wtrigraphs -Wtype-limits
-Wuninitialized -Wunknown-pragmas -Wunused -Wunused-but-set-parameter
-Wunused-but-set-variable -Wunused-function -Wunused-label
-Wunused-local-typedefs -Wunused-parameter -Wunused-result -Wunused-value
-Wunused-variable -Wvarargs -Wvariadic-macros
-Wvector-operation-performance -Wvla -Wvolatile-register-var
-Wwrite-strings -Walloc-size-larger-than=9223372036854775807
-Warray-bounds=2 -Wattribute-alias=2 

Re: Help on Meson build Error

[Wei Wang]

On Thu, Sep 24, 2020 at 2:58 PM Ján Tomko  wrote:

> On a Thursday in 2020, Wei Wang wrote:
> >Seems it didn't appear on the mailing list, resent it.
> >
> > Hi folks,
> >
> >I'm trying to build libvirt using meson with the latest upstream libvirt,
> >but the compilation fails:
> >(followed on https://libvirt.org/compiling.html, not sure if any
> >dependencies are missed. I checked, src/util/libvirt_util.a.p does not
> >exist.)
> >
> >FAILED: src/util/libvirt_util.a.p/glibcompat.c.o
> >
> >In file included from ../src/util/glibcompat.c:19:
> >./config.h:1026:10: fatal error: config-post.h: No such file or directory
> >FAILED: src/util/libvirt_util.a.p/virfirmware.c.o
> >In file included from ../src/util/virarch.c:22:
> >./config.h:1026:10: fatal error: config-post.h: No such file or directory
> >FAILED: src/util/libvirt_util.a.p/viraudit.c.o
> >In file included from ../src/util/viraudit.c:22:
> >./config.h:1026:10: fatal error: config-post.h: No such file or directory
> >
>
> config-post.h was moved to config.h when we converted to Meson.
>
> There are probably some leftovers from a previous build in your build
> directory. Can you try it with an empty build directory?
>
>
Thanks, just done cloning a new one from the official repo, but meson build
reports an error:
src/util/meson.build:138:0: ERROR: Program
'/opt/projects/libvirt/src/keycodemapdb/tools/keymap-gen' not found

Is there anything I should do before "meson build"?

Thanks,
Wei

Re: [PATCH v2 5/5] qemu: fix error message when baselining with a single cpu

[Collin Walling]

Change the commit message to:

qemu: allow hypervisor-cpu-baseline with single cpu

When executing the hypervisor-cpu-baseline command and if there is
only a single CPU definition present in the XML file, then the
baseline handler will exit early and libvirt will print an unhelpful
message:

"error: An error occurred, but the cause is unknown"

This is due to no CPU definition ever being "baselined", since the
handler expects at least two CPU models.

Let's fix this by performing a CPU model expansion on the single CPU
definition and returning the result to the caller. This will also
ensure the CPU model's feature set is sane if any were provided in
the file.

Signed-off-by: Collin Walling 

On 9/24/20 8:22 PM, Collin Walling wrote:
> When executing the hypervisor-cpu-baseline command and if there is
> only a single CPU definition present in the XML file, then libvirt
> will print an unhelpful message:
> 
> "error: An error occurred, but the cause is unknown"
> 
> This is due to no CPU definition ever being "baselined", since the
> API expects at least two CPU models.
> 
> Let's fix this by performing a CPU model expansion on the single CPU
> definition and returning the result to the caller. This will also
> ensure the CPU model is sane.
> 
> Signed-off-by: Collin Walling 
> ---
>  src/qemu/qemu_driver.c | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index fe572b13e1..bc823fc585 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -12459,6 +12459,7 @@ qemuConnectCPUModelBaseline(virQEMUCapsPtr qemuCaps,
>  g_autoptr(qemuProcessQMP) proc = NULL;
>  g_autoptr(virCPUDef) baseline = NULL;
>  qemuMonitorCPUModelInfoPtr result = NULL;
> +qemuMonitorCPUModelExpansionType expansion_type;
>  size_t i, j;
>  
>  for (i = 0; i < ncpus; i++) {
> @@ -12506,9 +12507,11 @@ qemuConnectCPUModelBaseline(virQEMUCapsPtr qemuCaps,
>  return NULL;
>  }
>  
> -if (expand_features) {
> -if (qemuMonitorGetCPUModelExpansion(proc->mon,
> -
> QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL,
> +if (expand_features || ncpus == 1) {
> +expansion_type = expand_features ? 
> QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL
> + : 
> QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC;
> +
> +if (qemuMonitorGetCPUModelExpansion(proc->mon, expansion_type,
>  baseline, true, false, ) 
> < 0)
>  return NULL;
>  
> 


-- 
Regards,
Collin

Stay safe and stay healthy

[PATCH v2 3/5] qemu: report error if missing model name when baselining

[Collin Walling]

When executing the hypervisor-cpu-baseline command and the
XML file contains a CPU definition without a model name, or
an invalid CPU definition, then the commands will fail and
return an error message from the QMP response.

Let's clean this up by checking for a valid definition and
presence of a model name.

This code is copied from virCPUBaseline.

Signed-off-by: Collin Walling 
---
 src/qemu/qemu_driver.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 92684a9876..1c5b1dcfee 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12460,6 +12460,19 @@ qemuConnectCPUModelBaseline(virQEMUCapsPtr qemuCaps,
 qemuMonitorCPUModelInfoPtr result = NULL;
 size_t i;
 
+for (i = 0; i < ncpus; i++) {
+if (!cpus[i]) {
+virReportError(VIR_ERR_INVALID_ARG,
+   _("invalid CPU definition at index %zu"), i);
+return NULL;
+}
+if (!cpus[i]->model) {
+virReportError(VIR_ERR_INVALID_ARG,
+   _("no CPU model specified at index %zu"), i);
+return NULL;
+}
+}
+
 if (!(proc = qemuProcessQMPNew(virQEMUCapsGetBinary(qemuCaps),
libDir, runUid, runGid, false)))
 return NULL;
-- 
2.26.2

[PATCH v2 2/5] qemu: fix one instance of rc check styling in baseline

[Collin Walling]

Signed-off-by: Collin Walling 
---
 src/qemu/qemu_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0b68b33af8..92684a9876 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12470,7 +12470,7 @@ qemuConnectCPUModelBaseline(virQEMUCapsPtr qemuCaps,
 if (VIR_ALLOC(baseline) < 0)
 return NULL;
 
-if (virCPUDefCopyModel(baseline, cpus[0], false))
+if (virCPUDefCopyModel(baseline, cpus[0], false) < 0)
 return NULL;
 
 for (i = 1; i < ncpus; i++) {
-- 
2.26.2

[PATCH v2 5/5] qemu: fix error message when baselining with a single cpu

[Collin Walling]

When executing the hypervisor-cpu-baseline command and if there is
only a single CPU definition present in the XML file, then libvirt
will print an unhelpful message:

"error: An error occurred, but the cause is unknown"

This is due to no CPU definition ever being "baselined", since the
API expects at least two CPU models.

Let's fix this by performing a CPU model expansion on the single CPU
definition and returning the result to the caller. This will also
ensure the CPU model is sane.

Signed-off-by: Collin Walling 
---
 src/qemu/qemu_driver.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index fe572b13e1..bc823fc585 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12459,6 +12459,7 @@ qemuConnectCPUModelBaseline(virQEMUCapsPtr qemuCaps,
 g_autoptr(qemuProcessQMP) proc = NULL;
 g_autoptr(virCPUDef) baseline = NULL;
 qemuMonitorCPUModelInfoPtr result = NULL;
+qemuMonitorCPUModelExpansionType expansion_type;
 size_t i, j;
 
 for (i = 0; i < ncpus; i++) {
@@ -12506,9 +12507,11 @@ qemuConnectCPUModelBaseline(virQEMUCapsPtr qemuCaps,
 return NULL;
 }
 
-if (expand_features) {
-if (qemuMonitorGetCPUModelExpansion(proc->mon,
-
QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL,
+if (expand_features || ncpus == 1) {
+expansion_type = expand_features ? 
QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL
+ : 
QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC;
+
+if (qemuMonitorGetCPUModelExpansion(proc->mon, expansion_type,
 baseline, true, false, ) < 
0)
 return NULL;
 
-- 
2.26.2

[PATCH v2 0/5] Hypervisor CPU Baseline Cleanups and Fixes

[Collin Walling]

The following patches provide some TLC to the hypervisor CPU baseline
handler within the qemu_driver code.

#1 checks for the cpu-model-expansion capability before
executing the baseline handler since it is used for feature expansion.

#2 fixes a styling where a < 0 condition was missing from one of the 
if (function()) lines for consistency's sake.

#3 will check if the cpu definition(s) are valid and contain a model
name.

#4 checks the cpu definition(s) model names against the model names
known by the hypervisor. This patch must come before #5.

#5 will allow the baseline command to be ran with a single cpu
definition, whereas before the command would simply fail with an
unhelpful error message. A CPU model expansion will be performed in
this case, which will produce the same result as if the model were
actually baselined.

Note: without patch #4, #5 can result in a segfault in the case where
a single CPU model is provided and the model is not recognized by the
hypervisor. This is because cpu model expansion will return 0 and the
result will be NULL. 

Since the QMP response returns "GenericError" in the case of a missing
CPU model, or if the command is not supported (and perhaps for other
reasons I am unsure of -- the response does not explicitly detail that
the CPU model provided was erroneous), we cannot rely on this 
response always meaning there was a missing CPU model.

So, to be safe-and-sure, the CPU model is checked against the list of
CPU models known to the hypervisor prior to baselining / expanding
(which were retrieved at some point previously during libvirt init).

Collin Walling (5):
  qemu: check for model-expansion cap before baselining
  qemu: fix one instance of rc check styling in baseline
  qemu: report error if missing model name when baselining
  qemu: check if cpu model is supported before baselining
  qemu: fix error message when baselining with a single cpu

 src/qemu/qemu_driver.c | 45 ++
 1 file changed, 37 insertions(+), 8 deletions(-)

-- 
2.26.2

[PATCH v2 1/5] qemu: check for model-expansion cap before baselining

[Collin Walling]

Hypervisor-cpu-baseline requires the cpu-model-expansion
capability when expanding CPU model features if the
--features flag is provided.

Signed-off-by: Collin Walling 
---
 src/qemu/qemu_driver.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1cecef01f7..0b68b33af8 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12563,7 +12563,8 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn,
(const char **)features, migratable)))
 goto cleanup;
 } else if (ARCH_IS_S390(arch) &&
-   virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_BASELINE)) {
+   virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_BASELINE) &&
+   virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION)) {
 bool expand_features = (flags & 
VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES);
 
 if (!(cpu = qemuConnectCPUModelBaseline(qemuCaps, cfg->libDir,
-- 
2.26.2

[PATCH v2 4/5] qemu: check if cpu model is supported before baselining

[Collin Walling]

Check the provided CPU models against the CPU models
known by the hypervisor before baselining and print
an error if an unrecognized model is found.

Signed-off-by: Collin Walling 
---
 src/qemu/qemu_driver.c | 18 +++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1c5b1dcfee..fe572b13e1 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12453,12 +12453,13 @@ qemuConnectCPUModelBaseline(virQEMUCapsPtr qemuCaps,
 gid_t runGid,
 bool expand_features,
 virCPUDefPtr *cpus,
-int ncpus)
+int ncpus,
+virDomainCapsCPUModelsPtr cpuModels)
 {
 g_autoptr(qemuProcessQMP) proc = NULL;
 g_autoptr(virCPUDef) baseline = NULL;
 qemuMonitorCPUModelInfoPtr result = NULL;
-size_t i;
+size_t i, j;
 
 for (i = 0; i < ncpus; i++) {
 if (!cpus[i]) {
@@ -12471,6 +12472,16 @@ qemuConnectCPUModelBaseline(virQEMUCapsPtr qemuCaps,
_("no CPU model specified at index %zu"), i);
 return NULL;
 }
+for (j = 0; j < cpuModels->nmodels; j++) {
+if (STREQ(cpus[i]->model, cpuModels->models[j].name))
+break;
+}
+if (j == cpuModels->nmodels) {
+virReportError(VIR_ERR_INVALID_ARG,
+   _("CPU model '%s' not supported by hypervisor"),
+   cpus[i]->model);
+return NULL;
+}
 }
 
 if (!(proc = qemuProcessQMPNew(virQEMUCapsGetBinary(qemuCaps),
@@ -12582,7 +12593,8 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn,
 
 if (!(cpu = qemuConnectCPUModelBaseline(qemuCaps, cfg->libDir,
 cfg->user, cfg->group,
-expand_features, cpus, ncpus)))
+expand_features, cpus, ncpus,
+cpuModels)))
 goto cleanup;
 } else {
 virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
-- 
2.26.2

[libvirt PATCH v4 2/6] qemu: add vhost-vdpa capability

[Jonathon Jongsma]

Recent versions of qemu added the -netdev vhost-vdpa device. This
capability allows libvirt to know whether this is supported.

Signed-off-by: Jonathon Jongsma 
---
 src/qemu/qemu_capabilities.c | 4 
 src/qemu/qemu_capabilities.h | 3 +++
 tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml | 1 +
 tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml | 1 +
 4 files changed, 9 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 2cc0c61588..b19928f68d 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -597,6 +597,9 @@ VIR_ENUM_IMPL(virQEMUCaps,
   "spapr-tpm-proxy",
   "numa.hmat",
   "blockdev-hostdev-scsi",
+
+  /* 380 */
+  "netdev.vhost-vdpa",
 );
 
 
@@ -1526,6 +1529,7 @@ static struct virQEMUCapsStringFlags 
virQEMUCapsQMPSchemaQueries[] = {
 { "migrate-set-parameters/arg-type/downtime-limit", 
QEMU_CAPS_MIGRATION_PARAM_DOWNTIME },
 { "migrate-set-parameters/arg-type/xbzrle-cache-size", 
QEMU_CAPS_MIGRATION_PARAM_XBZRLE_CACHE_SIZE },
 { "set-numa-node/arg-type/+hmat-lb", QEMU_CAPS_NUMA_HMAT },
+{ "netdev_add/arg-type/+vhost-vdpa", QEMU_CAPS_NETDEV_VHOST_VDPA },
 };
 
 typedef struct _virQEMUCapsObjectTypeProps virQEMUCapsObjectTypeProps;
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 5d08941538..b6110f1c34 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -578,6 +578,9 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 QEMU_CAPS_NUMA_HMAT, /* -numa hmat */
 QEMU_CAPS_BLOCKDEV_HOSTDEV_SCSI, /* -blockdev used for (i)SCSI hostdevs */
 
+/* 380 */
+QEMU_CAPS_NETDEV_VHOST_VDPA, /* -netdev vhost-vdpa*/
+
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
 
diff --git a/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml
index 7496ff1379..0fd2f3b816 100644
--- a/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml
@@ -242,6 +242,7 @@
   
   
   
+  
   5001000
   0
   43100242
diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml
index 151bd18137..e4686000a9 100644
--- a/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml
@@ -242,6 +242,7 @@
   
   
   
+  
   5001050
   0
   43100243
-- 
2.26.2

[libvirt PATCH v4 0/6] Add support for vDPA network devices

[Jonathon Jongsma]

vDPA network devices allow high-performance networking in a virtual machine by
providing a wire-speed data path. These devices require a vendor-specific host
driver but the data path follows the virtio specification.

The support for vDPA devices was recently added to qemu. This allows
libvirt to support these devices. This patchset requires that the device is
configured on the host with the appropriate vendor-specific driver.  This will
create a chardev on the host at e.g. /dev/vhost-vdpa-0. That chardev path can
then be used to define a new interface with type=3D'vdpa'.

Changes in v4:
 - rebased to latest master
 - added hotplug support
 - report vdpa devices in node device list

Jonathon Jongsma (6):
  conf: Add support for vDPA network devices
  qemu: add vhost-vdpa capability
  qemu: add vdpa support
  qemu: add monitor functions for handling file descriptors
  qemu: support hotplug of vdpa devices
  Include vdpa devices in node device list

 docs/formatdomain.rst |  24 +++
 docs/schemas/domaincommon.rng |  15 ++
 include/libvirt/libvirt-nodedev.h |   1 +
 src/conf/domain_conf.c|  31 
 src/conf/domain_conf.h|   4 +
 src/conf/netdev_bandwidth_conf.c  |   1 +
 src/conf/node_device_conf.c   |   5 +
 src/conf/node_device_conf.h   |   4 +-
 src/conf/virnodedeviceobj.c   |   4 +-
 src/libxl/libxl_conf.c|   1 +
 src/libxl/xen_common.c|   1 +
 src/lxc/lxc_controller.c  |   1 +
 src/lxc/lxc_driver.c  |   3 +
 src/lxc/lxc_process.c |   1 +
 src/node_device/node_device_udev.c|  16 ++
 src/qemu/qemu_capabilities.c  |   4 +
 src/qemu/qemu_capabilities.h  |   3 +
 src/qemu/qemu_command.c   |  36 +++-
 src/qemu/qemu_command.h   |   3 +-
 src/qemu/qemu_domain.c|   6 +-
 src/qemu/qemu_hotplug.c   |  73 +++-
 src/qemu/qemu_interface.c |  25 +++
 src/qemu/qemu_interface.h |   2 +
 src/qemu/qemu_migration.c |  10 +-
 src/qemu/qemu_monitor.c   |  93 ++
 src/qemu/qemu_monitor.h   |  41 +
 src/qemu/qemu_monitor_json.c  | 173 ++
 src/qemu/qemu_monitor_json.h  |  12 ++
 src/qemu/qemu_process.c   |   2 +
 src/qemu/qemu_validate.c  |  15 ++
 src/vmx/vmx.c |   1 +
 .../caps_5.1.0.x86_64.xml |   1 +
 .../caps_5.2.0.x86_64.xml |   1 +
 tests/qemuhotplugmock.c   |   9 +
 tests/qemuhotplugtest.c   |  16 ++
 .../qemuhotplug-interface-vdpa.xml|   4 +
 .../qemuhotplug-base-live+interface-vdpa.xml  |  57 ++
 .../net-vdpa.x86_64-latest.args   |  37 
 tests/qemuxml2argvdata/net-vdpa.xml   |  28 +++
 tests/qemuxml2argvmock.c  |  11 +-
 tests/qemuxml2argvtest.c  |   1 +
 tests/qemuxml2xmloutdata/net-vdpa.xml |  34 
 tests/qemuxml2xmltest.c   |   1 +
 tools/virsh-domain.c  |   1 +
 tools/virsh-nodedev.c |   3 +
 45 files changed, 799 insertions(+), 16 deletions(-)
 create mode 100644 tests/qemuhotplugtestdevices/qemuhotplug-interface-vdpa.x=
ml
 create mode 100644 tests/qemuhotplugtestdomains/qemuhotplug-base-live+interf=
ace-vdpa.xml
 create mode 100644 tests/qemuxml2argvdata/net-vdpa.x86_64-latest.args
 create mode 100644 tests/qemuxml2argvdata/net-vdpa.xml
 create mode 100644 tests/qemuxml2xmloutdata/net-vdpa.xml

--=20
2.26.2

[libvirt PATCH v4 4/6] qemu: add monitor functions for handling file descriptors

[Jonathon Jongsma]

add-fd, remove-fd, and query-fdsets provide functionality that can be
used for passing fds to qemu and closing fdsets that are no longer
necessary.

Signed-off-by: Jonathon Jongsma 
---
 src/qemu/qemu_monitor.c  |  93 +++
 src/qemu/qemu_monitor.h  |  41 +
 src/qemu/qemu_monitor_json.c | 173 +++
 src/qemu/qemu_monitor_json.h |  12 +++
 4 files changed, 319 insertions(+)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index ab3bcc761e..b33f2eec0a 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2651,6 +2651,99 @@ qemuMonitorGraphicsRelocate(qemuMonitorPtr mon,
 }
 
 
+/**
+ * qemuMonitorAddFileHandleToSet:
+ * @mon: monitor object
+ * @fd: file descriptor to pass to qemu
+ * @fdset: the fdset to register this fd with, -1 to create a new fdset
+ * @opaque: opaque data to associated with this fd
+ * @info: structure that will be updated with the fd and fdset returned by qemu
+ *
+ * Attempts to register a file descriptor with qemu that can then be referenced
+ * via the file path /dev/fdset/$FDSETID
+ * Returns 0 if ok, and -1 on failure */
+int
+qemuMonitorAddFileHandleToSet(qemuMonitorPtr mon,
+  int fd,
+  int fdset,
+  const char *opaque,
+  qemuMonitorAddFdInfoPtr info)
+{
+VIR_DEBUG("fd=%d,fdset=%i,opaque=%s", fd, fdset, opaque);
+
+QEMU_CHECK_MONITOR(mon);
+
+if (fd < 0) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+   _("fd must be valid"));
+return -1;
+}
+
+return qemuMonitorJSONAddFileHandleToSet(mon, fd, fdset, opaque, info);
+}
+
+
+/**
+ * qemuMonitorRemoveFdset:
+ * @mon: monitor object
+ * @fdset: the fdset to remove
+ *
+ * Attempts to remove a fdset from qemu and close associated file descriptors
+ * Returns 0 if ok, and -1 on failure */
+int
+qemuMonitorRemoveFdset(qemuMonitorPtr mon,
+   int fdset)
+{
+VIR_DEBUG("fdset=%d", fdset);
+
+QEMU_CHECK_MONITOR(mon);
+
+if (fdset < 0) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+   _("fdset must be valid"));
+return -1;
+}
+
+return qemuMonitorJSONRemoveFdset(mon, fdset);
+}
+
+
+void qemuMonitorFdsetsFree(qemuMonitorFdsetsPtr fdsets)
+{
+size_t i;
+
+for (i = 0; i < fdsets->nfdsets; i++) {
+size_t j;
+qemuMonitorFdsetInfoPtr set = >fdsets[i];
+
+for (j = 0; j < set->nfds; j++)
+g_free(set->fds[j].opaque);
+}
+g_free(fdsets->fdsets);
+g_free(fdsets);
+}
+
+
+/**
+ * qemuMonitorQueryFdsets:
+ * @mon: monitor object
+ * @fdsets: a pointer that is filled with a new qemuMonitorFdsets struct
+ *
+ * Queries qemu for the fdsets that are registered with that instance, and
+ * returns a structure describing those fdsets. The returned struct should be
+ * freed with qemuMonitorFdsetsFree();
+ *
+ * Returns 0 if ok, and -1 on failure */
+int
+qemuMonitorQueryFdsets(qemuMonitorPtr mon,
+   qemuMonitorFdsetsPtr *fdsets)
+{
+QEMU_CHECK_MONITOR(mon);
+
+return qemuMonitorJSONQueryFdsets(mon, fdsets);
+}
+
+
 int
 qemuMonitorSendFileHandle(qemuMonitorPtr mon,
   const char *fdname,
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index d3f7797085..ac97aedf8a 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -880,6 +880,47 @@ int qemuMonitorGraphicsRelocate(qemuMonitorPtr mon,
 int tlsPort,
 const char *tlsSubject);
 
+typedef struct _qemuMonitorAddFdInfo qemuMonitorAddFdInfo;
+typedef qemuMonitorAddFdInfo *qemuMonitorAddFdInfoPtr;
+struct _qemuMonitorAddFdInfo {
+int fd;
+int fdset;
+};
+int
+qemuMonitorAddFileHandleToSet(qemuMonitorPtr mon,
+  int fd,
+  int fdset,
+  const char *opaque,
+  qemuMonitorAddFdInfoPtr info);
+
+int
+qemuMonitorRemoveFdset(qemuMonitorPtr mon,
+   int fdset);
+
+typedef struct _qemuMonitorFdsetFdInfo qemuMonitorFdsetFdInfo;
+typedef qemuMonitorFdsetFdInfo *qemuMonitorFdsetFdInfoPtr;
+struct _qemuMonitorFdsetFdInfo {
+int fd;
+char *opaque;
+};
+typedef struct _qemuMonitorFdsetInfo qemuMonitorFdsetInfo;
+typedef qemuMonitorFdsetInfo *qemuMonitorFdsetInfoPtr;
+struct _qemuMonitorFdsetInfo {
+int id;
+qemuMonitorFdsetFdInfoPtr fds;
+int nfds;
+};
+typedef struct _qemuMonitorFdsets qemuMonitorFdsets;
+typedef qemuMonitorFdsets *qemuMonitorFdsetsPtr;
+struct _qemuMonitorFdsets {
+qemuMonitorFdsetInfoPtr fdsets;
+int nfdsets;
+};
+void qemuMonitorFdsetsFree(qemuMonitorFdsetsPtr fdsets);
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuMonitorFdsets, qemuMonitorFdsetsFree);
+int qemuMonitorQueryFdsets(qemuMonitorPtr mon,
+  

[libvirt PATCH v4 1/6] conf: Add support for vDPA network devices

[Jonathon Jongsma]

This patch adds new schema and adds support for parsing and formatting
domain configurations that include vdpa devices.

vDPA network devices allow high-performance networking in a virtual
machine by providing a wire-speed data path. These devices require a
vendor-specific host driver but the data path follows the virtio
specification.

When a device on the host is bound to an appropriate vendor-specific
driver, it will create a chardev on the host at e.g.  /dev/vhost-vdpa-0.
That chardev path can then be used to define a new interface with
type='vdpa'.

Signed-off-by: Jonathon Jongsma 
---
 docs/formatdomain.rst| 24 
 docs/schemas/domaincommon.rng| 15 +++
 src/conf/domain_conf.c   | 31 +++
 src/conf/domain_conf.h   |  4 
 src/conf/netdev_bandwidth_conf.c |  1 +
 src/libxl/libxl_conf.c   |  1 +
 src/libxl/xen_common.c   |  1 +
 src/lxc/lxc_controller.c |  1 +
 src/lxc/lxc_driver.c |  3 +++
 src/lxc/lxc_process.c|  1 +
 src/qemu/qemu_command.c  |  3 +++
 src/qemu/qemu_domain.c   |  4 +++-
 src/qemu/qemu_hotplug.c  |  3 +++
 src/qemu/qemu_interface.c|  2 ++
 src/qemu/qemu_process.c  |  2 ++
 src/qemu/qemu_validate.c |  1 +
 src/vmx/vmx.c|  1 +
 tools/virsh-domain.c |  1 +
 18 files changed, 98 insertions(+), 1 deletion(-)

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 888db5ea29..b0c2b43dc2 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -4643,6 +4643,30 @@ or stopping the guest.

...
 
+:anchor:``
+
+vDPA devices
+
+
+A vDPA network device can be used to provide wire speed network performance
+within a domain. A vDPA device is a specialized type of network device that
+uses a datapath that complies with the virtio specification but has a
+vendor-specific control path.  To use such a device with libvirt, the host
+device must already be bound to the appropriate device-specific vDPA driver.
+This creates a vDPA char device (e.g. /dev/vhost-vdpa-0) that can be used to
+assign the device to a libvirt domain.  :since:`Since 6.8.0 (QEMU only,
+requires QEMU 5.1.0 or newer)`
+
+::
+
+   ...
+   
+ 
+   
+ 
+   
+   ...
+
 :anchor:``
 
 Teaming a virtio/hostdev NIC pair
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 4b7e460148..4ce485a762 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3117,6 +3117,21 @@
 
   
 
+
+
+  
+vdpa
+  
+  
+
+  
+
+  
+
+
+  
+
+
   
   
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a91dbd4aa9..0b78ff7c70 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -554,6 +554,7 @@ VIR_ENUM_IMPL(virDomainNet,
   "direct",
   "hostdev",
   "udp",
+  "vdpa",
 );
 
 VIR_ENUM_IMPL(virDomainNetModel,
@@ -2505,6 +2506,10 @@ virDomainNetDefClear(virDomainNetDefPtr def)
 def->data.vhostuser = NULL;
 break;
 
+case VIR_DOMAIN_NET_TYPE_VDPA:
+VIR_FREE(def->data.vdpa.devicepath);
+break;
+
 case VIR_DOMAIN_NET_TYPE_SERVER:
 case VIR_DOMAIN_NET_TYPE_CLIENT:
 case VIR_DOMAIN_NET_TYPE_MCAST:
@@ -12133,6 +12138,10 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
 if (virDomainChrSourceReconnectDefParseXML(, cur, 
ctxt) < 0)
 goto error;
 
+} else if (!dev
+   && def->type == VIR_DOMAIN_NET_TYPE_VDPA
+   && virXMLNodeNameEqual(cur, "source")) {
+dev = virXMLPropString(cur, "dev");
 } else if (!def->virtPortProfile
&& virXMLNodeNameEqual(cur, "virtualport")) {
 if (def->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
@@ -12390,6 +12399,16 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
 }
 break;
 
+case VIR_DOMAIN_NET_TYPE_VDPA:
+if (dev == NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("No  'dev' attribute "
+ "specified with "));
+goto error;
+}
+def->data.vdpa.devicepath = g_steal_pointer();
+break;
+
 case VIR_DOMAIN_NET_TYPE_BRIDGE:
 if (bridge == NULL) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -12779,6 +12798,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
 case VIR_DOMAIN_NET_TYPE_DIRECT:
 case VIR_DOMAIN_NET_TYPE_HOSTDEV:
 case VIR_DOMAIN_NET_TYPE_UDP:
+case VIR_DOMAIN_NET_TYPE_VDPA:
 break;
 case VIR_DOMAIN_NET_TYPE_LAST:
 default:

[libvirt PATCH v4 3/6] qemu: add vdpa support

[Jonathon Jongsma]

Enable  for qemu domains. This provides basic
support and does not support hotplug or migration.

Signed-off-by: Jonathon Jongsma 
---
 src/qemu/qemu_command.c   | 35 --
 src/qemu/qemu_command.h   |  3 +-
 src/qemu/qemu_domain.c|  6 ++-
 src/qemu/qemu_hotplug.c   | 14 ---
 src/qemu/qemu_interface.c | 23 
 src/qemu/qemu_interface.h |  2 +
 src/qemu/qemu_migration.c | 10 -
 src/qemu/qemu_validate.c  | 14 +++
 .../net-vdpa.x86_64-latest.args   | 37 +++
 tests/qemuxml2argvdata/net-vdpa.xml   | 28 ++
 tests/qemuxml2argvmock.c  | 11 +-
 tests/qemuxml2argvtest.c  |  1 +
 tests/qemuxml2xmloutdata/net-vdpa.xml | 34 +
 tests/qemuxml2xmltest.c   |  1 +
 14 files changed, 205 insertions(+), 14 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/net-vdpa.x86_64-latest.args
 create mode 100644 tests/qemuxml2argvdata/net-vdpa.xml
 create mode 100644 tests/qemuxml2xmloutdata/net-vdpa.xml

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index a9367f7758..bc1d4bfd90 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3554,7 +3554,8 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
 size_t tapfdSize,
 char **vhostfd,
 size_t vhostfdSize,
-const char *slirpfd)
+const char *slirpfd,
+const char *vdpadev)
 {
 bool is_tap = false;
 virDomainNetType netType = virDomainNetGetActualType(net);
@@ -3693,6 +3694,12 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
 break;
 
 case VIR_DOMAIN_NET_TYPE_VDPA:
+/* Caller will pass the fd to qemu with add-fd */
+if (virJSONValueObjectCreate(, "s:type", "vhost-vdpa", NULL) 
< 0 ||
+virJSONValueObjectAppendString(netprops, "vhostdev", vdpadev) < 0)
+return NULL;
+break;
+
 case VIR_DOMAIN_NET_TYPE_HOSTDEV:
 /* Should have been handled earlier via PCI/USB hotplug code. */
 case VIR_DOMAIN_NET_TYPE_LAST:
@@ -8042,6 +8049,8 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
 char **tapfdName = NULL;
 char **vhostfdName = NULL;
 g_autofree char *slirpfdName = NULL;
+g_autofree char *vdpafdName = NULL;
+int vdpafd = -1;
 virDomainNetType actualType = virDomainNetGetActualType(net);
 const virNetDevBandwidth *actualBandwidth;
 bool requireNicdev = false;
@@ -8127,13 +8136,17 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
 
 break;
 
+case VIR_DOMAIN_NET_TYPE_VDPA:
+if ((vdpafd = qemuInterfaceVDPAConnect(net)) < 0)
+goto cleanup;
+break;
+
 case VIR_DOMAIN_NET_TYPE_USER:
 case VIR_DOMAIN_NET_TYPE_SERVER:
 case VIR_DOMAIN_NET_TYPE_CLIENT:
 case VIR_DOMAIN_NET_TYPE_MCAST:
 case VIR_DOMAIN_NET_TYPE_INTERNAL:
 case VIR_DOMAIN_NET_TYPE_UDP:
-case VIR_DOMAIN_NET_TYPE_VDPA:
 case VIR_DOMAIN_NET_TYPE_LAST:
 /* nada */
 break;
@@ -8250,13 +8263,29 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
 vhostfd[i] = -1;
 }
 
+if (vdpafd > 0) {
+g_autofree char *fdset = NULL;
+g_autofree char *addfdarg = NULL;
+
+virCommandPassFD(cmd, vdpafd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
+fdset = qemuVirCommandGetFDSet(cmd, vdpafd);
+if (!fdset)
+goto cleanup;
+vdpafdName = qemuVirCommandGetDevSet(cmd, vdpafd);
+/* set opaque to the devicepath so that we can look up the fdset later
+ * if necessary */
+addfdarg = g_strdup_printf("%s,opaque=%s", fdset,
+   net->data.vdpa.devicepath);
+virCommandAddArgList(cmd, "-add-fd", addfdarg, NULL);
+}
+
 if (chardev)
 virCommandAddArgList(cmd, "-chardev", chardev, NULL);
 
 if (!(hostnetprops = qemuBuildHostNetStr(net,
  tapfdName, tapfdSize,
  vhostfdName, vhostfdSize,
- slirpfdName)))
+ slirpfdName, vdpafdName)))
 goto cleanup;
 
 if (!(host = virQEMUBuildNetdevCommandlineFromJSON(hostnetprops,
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 89d99b111f..8db51f93b1 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -99,7 +99,8 @@ virJSONValuePtr qemuBuildHostNetStr(virDomainNetDefPtr net,
 size_t tapfdSize,
 char **vhostfd,
 size_t vhostfdSize,
-const char 

[libvirt PATCH v4 6/6] Include vdpa devices in node device list

[Jonathon Jongsma]

The current udev node device driver ignores all events related to vdpa
devices. Since libvirt now supports vDPA network devices, include these
devices in the device list.

Signed-off-by: Jonathon Jongsma 
---
 include/libvirt/libvirt-nodedev.h  |  1 +
 src/conf/node_device_conf.c|  5 +
 src/conf/node_device_conf.h|  4 +++-
 src/conf/virnodedeviceobj.c|  4 +++-
 src/node_device/node_device_udev.c | 16 
 tools/virsh-nodedev.c  |  3 +++
 6 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/include/libvirt/libvirt-nodedev.h 
b/include/libvirt/libvirt-nodedev.h
index dd2ffd5782..b73b076f14 100644
--- a/include/libvirt/libvirt-nodedev.h
+++ b/include/libvirt/libvirt-nodedev.h
@@ -82,6 +82,7 @@ typedef enum {
 VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV  = 1 << 14, /* Mediated 
device */
 VIR_CONNECT_LIST_NODE_DEVICES_CAP_CCW_DEV   = 1 << 15, /* CCW device */
 VIR_CONNECT_LIST_NODE_DEVICES_CAP_CSS_DEV   = 1 << 16, /* CSS device */
+VIR_CONNECT_LIST_NODE_DEVICES_CAP_VDPA  = 1 << 17, /* vDPA device 
*/
 } virConnectListAllNodeDeviceFlags;
 
 int virConnectListAllNodeDevices (virConnectPtr conn,
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index a9a03ad6c2..3eab1cda75 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -66,6 +66,7 @@ VIR_ENUM_IMPL(virNodeDevCap,
   "mdev",
   "ccw",
   "css",
+  "vdpa",
 );
 
 VIR_ENUM_IMPL(virNodeDevNetCap,
@@ -614,6 +615,7 @@ virNodeDeviceDefFormat(const virNodeDeviceDef *def)
 case VIR_NODE_DEV_CAP_MDEV_TYPES:
 case VIR_NODE_DEV_CAP_FC_HOST:
 case VIR_NODE_DEV_CAP_VPORTS:
+case VIR_NODE_DEV_CAP_VDPA:
 case VIR_NODE_DEV_CAP_LAST:
 break;
 }
@@ -1913,6 +1915,7 @@ virNodeDevCapsDefParseXML(xmlXPathContextPtr ctxt,
 case VIR_NODE_DEV_CAP_FC_HOST:
 case VIR_NODE_DEV_CAP_VPORTS:
 case VIR_NODE_DEV_CAP_SCSI_GENERIC:
+case VIR_NODE_DEV_CAP_VDPA:
 case VIR_NODE_DEV_CAP_LAST:
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("unknown capability type '%d' for '%s'"),
@@ -2232,6 +2235,7 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps)
 case VIR_NODE_DEV_CAP_VPORTS:
 case VIR_NODE_DEV_CAP_CCW_DEV:
 case VIR_NODE_DEV_CAP_CSS_DEV:
+case VIR_NODE_DEV_CAP_VDPA:
 case VIR_NODE_DEV_CAP_LAST:
 /* This case is here to shutup the compiler */
 break;
@@ -2286,6 +2290,7 @@ virNodeDeviceUpdateCaps(virNodeDeviceDefPtr def)
 case VIR_NODE_DEV_CAP_MDEV:
 case VIR_NODE_DEV_CAP_CCW_DEV:
 case VIR_NODE_DEV_CAP_CSS_DEV:
+case VIR_NODE_DEV_CAP_VDPA:
 case VIR_NODE_DEV_CAP_LAST:
 break;
 }
diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index 5484bc340f..4f8e47a068 100644
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h
@@ -65,6 +65,7 @@ typedef enum {
 VIR_NODE_DEV_CAP_MDEV,  /* Mediated device */
 VIR_NODE_DEV_CAP_CCW_DEV,   /* s390 CCW device */
 VIR_NODE_DEV_CAP_CSS_DEV,   /* s390 channel subsystem device */
+VIR_NODE_DEV_CAP_VDPA,  /* vDPA device */
 
 VIR_NODE_DEV_CAP_LAST
 } virNodeDevCapType;
@@ -369,7 +370,8 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps);
  VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV_TYPES| \
  VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV  | \
  VIR_CONNECT_LIST_NODE_DEVICES_CAP_CCW_DEV   | \
- VIR_CONNECT_LIST_NODE_DEVICES_CAP_CSS_DEV)
+ VIR_CONNECT_LIST_NODE_DEVICES_CAP_CSS_DEV   | \
+ VIR_CONNECT_LIST_NODE_DEVICES_CAP_VDPA)
 
 int
 virNodeDeviceGetSCSIHostCaps(virNodeDevCapSCSIHostPtr scsi_host);
diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
index 8aefd15e94..83c58ebe91 100644
--- a/src/conf/virnodedeviceobj.c
+++ b/src/conf/virnodedeviceobj.c
@@ -711,6 +711,7 @@ virNodeDeviceObjHasCap(const virNodeDeviceObj *obj,
 case VIR_NODE_DEV_CAP_MDEV:
 case VIR_NODE_DEV_CAP_CCW_DEV:
 case VIR_NODE_DEV_CAP_CSS_DEV:
+case VIR_NODE_DEV_CAP_VDPA:
 case VIR_NODE_DEV_CAP_LAST:
 break;
 }
@@ -862,7 +863,8 @@ virNodeDeviceObjMatch(virNodeDeviceObjPtr obj,
   MATCH(MDEV_TYPES)||
   MATCH(MDEV)  ||
   MATCH(CCW_DEV)   ||
-  MATCH(CSS_DEV)))
+  MATCH(CSS_DEV)   ||
+  MATCH(VDPA)))
 return false;
 }
 
diff --git a/src/node_device/node_device_udev.c 
b/src/node_device/node_device_udev.c
index 12e3f30bad..fda72f9071 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1144,6 +1144,18 @@ udevProcessCSS(struct udev_device *device,
 return 0;
 }
 
+

[libvirt PATCH v4 5/6] qemu: support hotplug of vdpa devices

[Jonathon Jongsma]

By using the new qemu monitor functions to handle passing and removing
file descriptors, we can support hotplug of vdpa devices.

Signed-off-by: Jonathon Jongsma 
---
 src/qemu/qemu_hotplug.c   | 60 +--
 tests/qemuhotplugmock.c   |  9 +++
 tests/qemuhotplugtest.c   | 16 +
 .../qemuhotplug-interface-vdpa.xml|  4 ++
 .../qemuhotplug-base-live+interface-vdpa.xml  | 57 ++
 5 files changed, 142 insertions(+), 4 deletions(-)
 create mode 100644 tests/qemuhotplugtestdevices/qemuhotplug-interface-vdpa.xml
 create mode 100644 
tests/qemuhotplugtestdomains/qemuhotplug-base-live+interface-vdpa.xml

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 0582b78f97..3a2aff607c 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1152,6 +1152,8 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
 virErrorPtr originalError = NULL;
 g_autofree char *slirpfdName = NULL;
 int slirpfd = -1;
+g_autofree char *vdpafdName = NULL;
+int vdpafd = -1;
 char **tapfdName = NULL;
 int *tapfd = NULL;
 size_t tapfdSize = 0;
@@ -1335,12 +1337,16 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
 /* hostdev interfaces were handled earlier in this function */
 break;
 
+case VIR_DOMAIN_NET_TYPE_VDPA:
+if ((vdpafd = qemuInterfaceVDPAConnect(net)) < 0)
+goto cleanup;
+break;
+
 case VIR_DOMAIN_NET_TYPE_SERVER:
 case VIR_DOMAIN_NET_TYPE_CLIENT:
 case VIR_DOMAIN_NET_TYPE_MCAST:
 case VIR_DOMAIN_NET_TYPE_INTERNAL:
 case VIR_DOMAIN_NET_TYPE_UDP:
-case VIR_DOMAIN_NET_TYPE_VDPA:
 case VIR_DOMAIN_NET_TYPE_LAST:
 virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
_("hotplug of interface type of %s is not implemented 
yet"),
@@ -1386,14 +1392,28 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
 for (i = 0; i < vhostfdSize; i++)
 vhostfdName[i] = g_strdup_printf("vhostfd-%s%zu", net->info.alias, i);
 
+qemuDomainObjEnterMonitor(driver, vm);
+
+if (vdpafd > 0) {
+/* vhost-vdpa only takes a filename for the dev, but we want to pass an
+ * open fd to qemu. Passing -1 as the fdset-id will create a new fdset
+ * and return the id of that fdset */
+qemuMonitorAddFdInfo fdinfo;
+if (qemuMonitorAddFileHandleToSet(priv->mon, vdpafd, -1,
+  net->data.vdpa.devicepath,
+  ) < 0) {
+ignore_value(qemuDomainObjExitMonitor(driver, vm));
+goto cleanup;
+}
+vdpafdName = g_strdup_printf("/dev/fdset/%d", fdinfo.fdset);
+}
+
 if (!(netprops = qemuBuildHostNetStr(net,
  tapfdName, tapfdSize,
  vhostfdName, vhostfdSize,
- slirpfdName, NULL)))
+ slirpfdName, vdpafdName)))
 goto cleanup;
 
-qemuDomainObjEnterMonitor(driver, vm);
-
 if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER) {
 if (qemuMonitorAttachCharDev(priv->mon, charDevAlias, 
net->data.vhostuser) < 0) {
 ignore_value(qemuDomainObjExitMonitor(driver, vm));
@@ -1518,6 +1538,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
 VIR_FREE(vhostfdName);
 virDomainCCWAddressSetFree(ccwaddrs);
 VIR_FORCE_CLOSE(slirpfd);
+VIR_FORCE_CLOSE(vdpafd);
 
 return ret;
 
@@ -4586,8 +4607,39 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver,
  * to just ignore the error and carry on.
  */
 }
+} else if (actualType == VIR_DOMAIN_NET_TYPE_VDPA) {
+int vdpafdset = -1;
+g_autoptr(qemuMonitorFdsets) fdsets = NULL;
+
+/* query qemu for which fdset is associated with the fd that we passed
+ * to qemu via 'add-fd' for this vdpa device. If we don't remove the
+ * fd, qemu will keep it open */
+if (qemuMonitorQueryFdsets(priv->mon, ) == 0) {
+for (i = 0; i < fdsets->nfdsets && vdpafdset < 0; i++) {
+size_t j;
+qemuMonitorFdsetInfoPtr set = >fdsets[i];
+
+for (j = 0; j < set->nfds; j++) {
+qemuMonitorFdsetFdInfoPtr fdinfo = >fds[j];
+if (STREQ_NULLABLE(fdinfo->opaque, 
net->data.vdpa.devicepath)) {
+vdpafdset = set->id;
+break;
+}
+}
+}
+}
+
+if (vdpafdset < 0) {
+VIR_WARN("Cannot determine fdset for vdpa device");
+} else {
+if (qemuMonitorRemoveFdset(priv->mon, vdpafdset) < 0) {
+/* if it fails, there's not much we can do... just carry on */
+VIR_WARN("failed to close vdpa device");
+}
+   

Re: [libvirt PATCH 5/5] tools: use g_new0 instead of VIR_ALLOC*

[Daniel Henrique Barboza]

On 9/23/20 5:11 PM, Ján Tomko wrote:

With the exception of vsh*alloc.

Signed-off-by: Ján Tomko 
---


The 'error' label removal down there in virt-admin-completer.c was
really nice. 2 'for' loops and several VIR_FREE() calls were needed
solely because of that VIR_ALLOC_N().

Reviewed-by: Daniel Henrique Barboza 





  tools/virsh-domain-monitor.c| 11 +++
  tools/virsh-domain.c| 26 --
  tools/virt-admin-completer.c| 12 +---
  tools/virt-login-shell-helper.c | 13 -
  tools/vsh-table.c   | 21 +++--
  tools/vsh.c |  5 +
  6 files changed, 24 insertions(+), 64 deletions(-)

diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 848efc8aa3..c8a7c0f1b7 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -1234,8 +1234,7 @@ cmdDomBlkError(vshControl *ctl, const vshCmd *cmd)
  ndisks = count;
  
  if (ndisks) {

-if (VIR_ALLOC_N(disks, ndisks) < 0)
-goto cleanup;
+disks = g_new0(virDomainDiskError, ndisks);
  
  if ((count = virDomainGetDiskErrors(dom, disks, ndisks, 0)) == -1)

  goto cleanup;
@@ -1378,10 +1377,7 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd)
  vshPrint(ctl, "%-15s %s\n", _("Security DOI:"), secmodel.doi);
  
  /* Security labels are only valid for active domains */

-if (VIR_ALLOC(seclabel) < 0) {
-virshDomainFree(dom);
-return false;
-}
+seclabel = g_new0(virSecurityLabel, 1);
  
  if (virDomainGetSecurityLabel(dom, seclabel) == -1) {

  virshDomainFree(dom);
@@ -2280,8 +2276,7 @@ cmdDomstats(vshControl *ctl, const vshCmd *cmd)
  flags |= VIR_CONNECT_GET_ALL_DOMAINS_STATS_NOWAIT;
  
  if (vshCommandOptBool(cmd, "domain")) {

-if (VIR_ALLOC_N(domlist, 1) < 0)
-goto cleanup;
+domlist = g_new0(virDomainPtr, 1);
  ndoms = 1;
  
  while ((opt = vshCommandOptArgv(ctl, cmd, opt))) {

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 32edfc0398..dfcba04682 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1762,8 +1762,7 @@ virshBlockJobWaitInit(vshControl *ctl,
  virshBlockJobWaitDataPtr ret;
  virshControlPtr priv = ctl->privData;
  
-if (VIR_ALLOC(ret) < 0)

-return NULL;
+ret = g_new0(virshBlockJobWaitData, 1);
  
  ret->ctl = ctl;

  ret->dom = dom;
@@ -8177,8 +8176,7 @@ cmdCPUStats(vshControl *ctl, const vshCmd *cmd)
  goto cleanup;
  }
  
-if (VIR_ALLOC_N(params, nparams * MIN(show_count, 128)) < 0)

-goto cleanup;
+params = g_new0(virTypedParameter, nparams * MIN(show_count, 128));
  
  while (show_count) {

  int ncpus = MIN(show_count, 128);
@@ -8215,8 +8213,7 @@ cmdCPUStats(vshControl *ctl, const vshCmd *cmd)
  goto cleanup;
  }
  
-if (VIR_ALLOC_N(params, nparams) < 0)

-goto cleanup;
+params = g_new0(virTypedParameter, nparams);
  
  /* passing start_cpu == -1 gives us domain's total status */

  if ((stats_per_cpu = virDomainGetCPUStats(dom, params, nparams,
@@ -10086,14 +10083,9 @@ cmdLxcEnterNamespace(vshControl *ctl, const vshCmd 
*cmd)
  goto cleanup;
  
  if (setlabel) {

-if (VIR_ALLOC(secmodel) < 0) {
-vshError(ctl, "%s", _("Failed to allocate security model"));
-goto cleanup;
-}
-if (VIR_ALLOC(seclabel) < 0) {
-vshError(ctl, "%s", _("Failed to allocate security label"));
-goto cleanup;
-}
+secmodel = g_new0(virSecurityModel, 1);
+seclabel = g_new0(virSecurityLabel, 1);
+
  if (virNodeGetSecurityModel(priv->conn, secmodel) < 0)
  goto cleanup;
  if (virDomainGetSecurityLabel(dom, seclabel) < 0)
@@ -13737,8 +13729,7 @@ cmdEvent(vshControl *ctl, const vshCmd *cmd)
  }
  
  if (all) {

-if (VIR_ALLOC_N(data, VIR_DOMAIN_EVENT_ID_LAST) < 0)
-goto cleanup;
+data = g_new0(virshDomEventData, VIR_DOMAIN_EVENT_ID_LAST);
  for (i = 0; i < VIR_DOMAIN_EVENT_ID_LAST; i++) {
  data[i].ctl = ctl;
  data[i].loop = loop;
@@ -13748,8 +13739,7 @@ cmdEvent(vshControl *ctl, const vshCmd *cmd)
  data[i].id = -1;
  }
  } else {
-if (VIR_ALLOC_N(data, 1) < 0)
-goto cleanup;
+data = g_new0(virshDomEventData, 1);
  data[0].ctl = ctl;
  data[0].loop = vshCommandOptBool(cmd, "loop");
  data[0].count = 
diff --git a/tools/virt-admin-completer.c b/tools/virt-admin-completer.c
index 7201bb71eb..61004beeb5 100644
--- a/tools/virt-admin-completer.c
+++ b/tools/virt-admin-completer.c
@@ -46,8 +46,7 @@ vshAdmServerCompleter(vshControl *ctl,
  if ((nsrvs = virAdmConnectListServers(priv->conn, , 0)) 

Re: [libvirt PATCH 4/5] security: use g_new0 instead of VIR_ALLOC*

[Daniel Henrique Barboza]

On 9/23/20 5:11 PM, Ján Tomko wrote:

Signed-off-by: Ján Tomko 
---


Reviewed-by: Daniel Henrique Barboza 


  src/security/security_apparmor.c |  3 +--
  src/security/security_dac.c  |  9 +++--
  src/security/security_manager.c  | 14 +-
  src/security/security_selinux.c  |  9 +++--
  src/security/security_stack.c|  6 ++
  5 files changed, 14 insertions(+), 27 deletions(-)

diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
index eea37dca83..c2d86c6940 100644
--- a/src/security/security_apparmor.c
+++ b/src/security/security_apparmor.c
@@ -867,8 +867,7 @@ AppArmorSetSecurityHostdevLabel(virSecurityManagerPtr mgr,
  if (profile_loaded(secdef->imagelabel) < 0)
  return 0;
  
-if (VIR_ALLOC(ptr) < 0)

-return -1;
+ptr = g_new0(struct SDPDOP, 1);
  ptr->mgr = mgr;
  ptr->def = def;
  
diff --git a/src/security/security_dac.c b/src/security/security_dac.c

index d9d4cda159..258d246659 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -108,8 +108,7 @@ virSecurityDACChownListAppend(virSecurityDACChownListPtr 
list,
  char *tmp = NULL;
  virSecurityDACChownItemPtr item = NULL;
  
-if (VIR_ALLOC(item) < 0)

-return -1;
+item = g_new0(virSecurityDACChownItem, 1);
  
  tmp = g_strdup(path);
  
@@ -227,8 +226,7 @@ virSecurityDACTransactionRun(pid_t pid G_GNUC_UNUSED,

  int ret = -1;
  
  if (list->lock) {

-if (VIR_ALLOC_N(paths, list->nItems) < 0)
-return -1;
+paths = g_new0(const char *, list->nItems);
  
  for (i = 0; i < list->nItems; i++) {

  virSecurityDACChownItemPtr item = list->items[i];
@@ -580,8 +578,7 @@ virSecurityDACTransactionStart(virSecurityManagerPtr mgr)
  return -1;
  }
  
-if (VIR_ALLOC(list) < 0)

-return -1;
+list = g_new0(virSecurityDACChownList, 1);
  
  list->manager = virObjectRef(mgr);
  
diff --git a/src/security/security_manager.c b/src/security/security_manager.c

index 17b565cc12..be81ee5e44 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -87,8 +87,7 @@ virSecurityManagerNewDriver(virSecurityDriverPtr drv,
  
  virCheckFlags(VIR_SECURITY_MANAGER_NEW_MASK, NULL);
  
-if (VIR_ALLOC_N(privateData, drv->privateDataLen) < 0)

-return NULL;
+privateData = g_new0(char, drv->privateDataLen);
  
  if (!(mgr = virObjectLockableNew(virSecurityManagerClass)))

  goto error;
@@ -1034,8 +1033,7 @@ virSecurityManagerGetNested(virSecurityManagerPtr mgr)
  if (STREQ("stack", mgr->drv->name))
  return virSecurityStackGetNested(mgr);
  
-if (VIR_ALLOC_N(list, 2) < 0)

-return NULL;
+list = g_new0(virSecurityManagerPtr, 2);
  
  list[0] = mgr;

  list[1] = NULL;
@@ -1346,9 +1344,8 @@ virSecurityManagerMetadataLock(virSecurityManagerPtr mgr 
G_GNUC_UNUSED,
  const char **locked_paths = NULL;
  virSecurityManagerMetadataLockStatePtr ret = NULL;
  
-if (VIR_ALLOC_N(fds, npaths) < 0 ||

-VIR_ALLOC_N(locked_paths, npaths) < 0)
-return NULL;
+fds = g_new0(int, npaths);
+locked_paths = g_new0(const char *, npaths);
  
  /* Sort paths to lock in order to avoid deadlocks with other

   * processes. For instance, if one process wants to lock
@@ -1441,8 +1438,7 @@ virSecurityManagerMetadataLock(virSecurityManagerPtr mgr 
G_GNUC_UNUSED,
  VIR_APPEND_ELEMENT_COPY_INPLACE(fds, nfds, fd);
  }
  
-if (VIR_ALLOC(ret) < 0)

-goto cleanup;
+ret = g_new0(virSecurityManagerMetadataLockState, 1);
  
  ret->paths = g_steal_pointer(_paths);

  ret->fds = g_steal_pointer();
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 87741d6dad..e40d670e97 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -123,8 +123,7 @@ 
virSecuritySELinuxContextListAppend(virSecuritySELinuxContextListPtr list,
  int ret = -1;
  virSecuritySELinuxContextItemPtr item = NULL;
  
-if (VIR_ALLOC(item) < 0)

-return -1;
+item = g_new0(virSecuritySELinuxContextItem, 1);
  
  item->path = g_strdup(path);

  item->tcon = g_strdup(tcon);
@@ -258,8 +257,7 @@ virSecuritySELinuxTransactionRun(pid_t pid G_GNUC_UNUSED,
  int ret = -1;
  
  if (list->lock) {

-if (VIR_ALLOC_N(paths, list->nItems) < 0)
-return -1;
+paths = g_new0(const char *, list->nItems);
  
  for (i = 0; i < list->nItems; i++) {

  virSecuritySELinuxContextItemPtr item = list->items[i];
@@ -1088,8 +1086,7 @@ virSecuritySELinuxTransactionStart(virSecurityManagerPtr 
mgr)
  return -1;
  }
  
-if (VIR_ALLOC(list) < 0)

-return -1;
+list = g_new0(virSecuritySELinuxContextList, 1);
  
  list->manager = virObjectRef(mgr);
  
diff --git a/src/security/security_stack.c 

Re: [libvirt PATCH 2/5] openvz: use g_new0 instead of VIR_ALLOC*

[Daniel Henrique Barboza]

On 9/23/20 5:11 PM, Ján Tomko wrote:

Signed-off-by: Ján Tomko 
---


Reviewed-by: Daniel Henrique Barboza 


  src/openvz/openvz_conf.c   | 12 
  src/openvz/openvz_driver.c |  3 +--
  2 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c
index 6d54123a35..8f1740863c 100644
--- a/src/openvz/openvz_conf.c
+++ b/src/openvz/openvz_conf.c
@@ -205,8 +205,7 @@ openvzReadNetworkConf(virDomainDefPtr def,
  } else if (ret > 0) {
  token = strtok_r(temp, " ", );
  while (token != NULL) {
-if (VIR_ALLOC(net) < 0)
-goto error;
+net = g_new0(virDomainNetDef, 1);
  
  net->type = VIR_DOMAIN_NET_TYPE_ETHERNET;

  if (virDomainNetAppendIPAddress(net, token, AF_UNSPEC, 0) < 0)
@@ -238,8 +237,7 @@ openvzReadNetworkConf(virDomainDefPtr def,
  int len;
  
  /* add new device to list */

-if (VIR_ALLOC(net) < 0)
-goto error;
+net = g_new0(virDomainNetDef, 1);
  
  net->type = VIR_DOMAIN_NET_TYPE_BRIDGE;
  
@@ -259,8 +257,7 @@ openvzReadNetworkConf(virDomainDefPtr def,

  goto error;
  }
  
-if (VIR_ALLOC_N(net->ifname, len+1) < 0)

-goto error;
+net->ifname = g_new0(char, len+1);
  
  if (virStrncpy(net->ifname, p, len, len+1) < 0) {

  virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -276,8 +273,7 @@ openvzReadNetworkConf(virDomainDefPtr def,
  goto error;
  }
  
-if (VIR_ALLOC_N(net->data.bridge.brname, len+1) < 0)

-goto error;
+net->data.bridge.brname = g_new0(char, len+1);
  
  if (virStrncpy(net->data.bridge.brname, p, len, len+1) < 0) {

  virReportError(VIR_ERR_INTERNAL_ERROR,
diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index 71e270ea09..f90ae4fe69 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -1287,8 +1287,7 @@ static virDrvOpenStatus openvzConnectOpen(virConnectPtr 
conn,
  /* We now know the URI is definitely for this driver, so beyond
   * here, don't return DECLINED, always use ERROR */
  
-if (VIR_ALLOC(driver) < 0)

-return VIR_DRV_OPEN_ERROR;
+driver = g_new0(struct openvz_driver, 1);
  
  if (!(driver->domains = virDomainObjListNew()))

  goto cleanup;

Re: [libvirt PATCH 3/5] secret: use g_new0 instead of VIR_ALLOC*

[Daniel Henrique Barboza]

On 9/23/20 5:11 PM, Ján Tomko wrote:

Signed-off-by: Ján Tomko 
---


Reviewed-by: Daniel Henrique Barboza 


  src/secret/secret_driver.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c
index 1cb342878f..45da09322b 100644
--- a/src/secret/secret_driver.c
+++ b/src/secret/secret_driver.c
@@ -459,8 +459,7 @@ secretStateInitialize(bool privileged,
virStateInhibitCallback callback G_GNUC_UNUSED,
void *opaque G_GNUC_UNUSED)
  {
-if (VIR_ALLOC(driver) < 0)
-return VIR_DRV_STATE_INIT_ERROR;
+driver = g_new0(virSecretDriverState, 1);
  
  driver->lockFD = -1;

  if (virMutexInit(>lock) < 0) {

Re: [libvirt PATCH 1/5] node_device: use g_new0 instead of VIR_ALLOC*

[Daniel Henrique Barboza]

On 9/23/20 5:11 PM, Ján Tomko wrote:

Signed-off-by: Ján Tomko 
---


Reviewed-by: Daniel Henrique Barboza 


  src/node_device/node_device_udev.c | 27 +--
  1 file changed, 9 insertions(+), 18 deletions(-)

diff --git a/src/node_device/node_device_udev.c 
b/src/node_device/node_device_udev.c
index 12e3f30bad..2d0ca27fc6 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -413,13 +413,11 @@ udevProcessPCI(struct udev_device *device,
  goto cleanup;
  
  if (virPCIDeviceIsPCIExpress(pciDev) > 0) {

-if (VIR_ALLOC(pci_express) < 0)
-goto cleanup;
+pci_express = g_new0(virPCIEDeviceInfo, 1);
  
  if (virPCIDeviceHasPCIExpressLink(pciDev) > 0) {

-if (VIR_ALLOC(pci_express->link_cap) < 0 ||
-VIR_ALLOC(pci_express->link_sta) < 0)
-goto cleanup;
+pci_express->link_cap = g_new0(virPCIELink, 1);
+pci_express->link_sta = g_new0(virPCIELink, 1);
  
  if (virPCIDeviceGetLinkCapSta(pciDev,

_express->link_cap->port,
@@ -1159,8 +1157,7 @@ udevGetDeviceNodes(struct udev_device *device,
  udev_list_entry_foreach(list_entry, 
udev_device_get_devlinks_list_entry(device))
  n++;
  
-if (VIR_ALLOC_N(def->devlinks, n + 1) < 0)

-return -1;
+def->devlinks = g_new0(char *, n + 1);
  
  n = 0;

  udev_list_entry_foreach(list_entry, 
udev_device_get_devlinks_list_entry(device)) {
@@ -1371,16 +1368,14 @@ udevAddOneDevice(struct udev_device *device)
  bool new_device = true;
  int ret = -1;
  
-if (VIR_ALLOC(def) != 0)

-goto cleanup;
+def = g_new0(virNodeDeviceDef, 1);
  
  def->sysfs_path = g_strdup(udev_device_get_syspath(device));
  
  if (udevGetStringProperty(device, "DRIVER", >driver) < 0)

  goto cleanup;
  
-if (VIR_ALLOC(def->caps) != 0)

-goto cleanup;
+def->caps = g_new0(virNodeDevCapsDef, 1);
  
  if (udevGetDeviceType(device, >caps->data.type) != 0)

  goto cleanup;
@@ -1788,13 +1783,10 @@ udevSetupSystemDev(void)
  virNodeDeviceObjPtr obj = NULL;
  int ret = -1;
  
-if (VIR_ALLOC(def) < 0)

-return -1;
+def = g_new0(virNodeDeviceDef, 1);
  
  def->name = g_strdup("computer");

-
-if (VIR_ALLOC(def->caps) != 0)
-goto cleanup;
+def->caps = g_new0(virNodeDevCapsDef, 1);
  
  #if defined(__x86_64__) || defined(__i386__) || defined(__amd64__)

  udevGetDMIData(>caps->data.system);
@@ -1882,8 +1874,7 @@ nodeStateInitialize(bool privileged,
  return -1;
  }
  
-if (VIR_ALLOC(driver) < 0)

-return VIR_DRV_STATE_INIT_ERROR;
+driver = g_new0(virNodeDeviceDriverState, 1);
  
  driver->lockFD = -1;

  if (virMutexInit(>lock) < 0) {

Re: [PATCH FOR 6.8.0] libxl: Don't free def member of virDomainObj

[Neal Gompa]

On Thu, Sep 24, 2020 at 1:03 PM Jim Fehlig  wrote:
>
> The refactoring in commit de49d5bad3 accidentally dropped the statement
> setting def to NULL after successfully adding it to the virDomainObjList,
> causing it to be freed while still in use. The resulting memory
> corruption caused unpredictable behavior, often resulting in a libvirtd
> crash.
>
> Signed-off-by: Jim Fehlig 
> ---
>
> Unpredictable is an understatement! When running monolithic libvirtd with
> both qemu and xen drviers enabled, qemu crashed while initializing. Recall
> it is initialized after xen.
>
> Thread 17 "daemon-init" received signal SIGSEGV, Segmentation fault.
> #0  0x7f32e5fbe9e3 in _int_malloc () at /lib64/libc.so.6
> #1  0x7f32e5fbf6e0 in _int_realloc () at /lib64/libc.so.6
> #2  0x7f32e5fc0729 in realloc () at /lib64/libc.so.6
> #3  0x7f32e6dc21b8 in g_realloc () at /usr/lib64/libglib-2.0.so.0
> #4  0x7f32e7532090 in virReallocN (ptrptr=0x7f329affcad8, size=1, 
> count=1403)
> at ../src/util/viralloc.c:91
> #5  0x7f32e75530c7 in virCommandProcessIO (cmd=0x7f328807ff40) at 
> ../src/util/vircommand.c:2271
> #6  0x7f32e7553a6a in virCommandRun (cmd=0x7f328807ff40, exitstatus=0x0)
> at ../src/util/vircommand.c:2451
> #7  0x7f32e75dde73 in virSysinfoReadDMI () at 
> ../src/util/virsysinfo.c:1237
> #8  0x7f32e75de0cb in virSysinfoRead () at ../src/util/virsysinfo.c:1294
> #9  0x7f32a240b69d in qemuStateInitialize
> (privileged=true, root=0x0, callback=0x56453a0b3e97 
> , opaque=0x56453b30) at 
> ../src/qemu/qemu_driver.c:658
> #10 0x7f32e7832350 in virStateInitialize
> (privileged=true, mandatory=false, root=0x0, callback=0x56453a0b3e97 
> , opaque=0x56453b30) at ../src/libvirt.c:656
> #11 0x56453a0b4175 in daemonRunStateInit (opaque=0x56453b30)
> at ../src/remote/remote_daemon.c:596
>
>  src/libxl/libxl_driver.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 083738871d..571b70f982 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -627,6 +627,7 @@ libxlAddDom0(libxlDriverPrivatePtr driver)
> NULL)))
>  goto cleanup;
>
> +def = NULL;
>  vm->persistent = 1;
>  virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, 
> VIR_DOMAIN_RUNNING_BOOTED);
>  }
> --
> 2.28.0
>
>

Reviewed-by: Neal Gompa 


-- 
真実はいつも一つ！/ Always, there's only one truth!

[PATCH] rpm: Simplify qemu+kvm conditionals and eliminate duplication

[Neal Gompa]

The conditionals for enabling qemu+kvm were unnecessarily complex.
In practice, there were far fewer cases where the functionality would
be disabled than what the conditional logic expressed, and this change
simplifies it to the realistic set of options.

Signed-off-by: Neal Gompa 
---
 libvirt.spec.in | 49 +++--
 1 file changed, 23 insertions(+), 26 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index c4a7c30737..6940066de9 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -20,31 +20,12 @@
 
 %define with_qemu_tcg  %{with_qemu}
 
-%define qemu_kvm_arches %{ix86} x86_64
-
-%if 0%{?fedora}
-%define qemu_kvm_arches %{ix86} x86_64 %{power64} s390x %{arm} aarch64
-%endif
-
-%if 0%{?rhel}
-%define with_qemu_tcg 0
-%define qemu_kvm_arches x86_64 %{power64} aarch64 s390x
-%endif
-
 # On RHEL 7 and older macro _vpath_builddir is not defined.
 %if 0%{?rhel} && 0%{?rhel} <= 7
 %define _vpath_builddir %{_target_platform}
 %endif
 
-%ifarch %{qemu_kvm_arches}
-%define with_qemu_kvm  %{with_qemu}
-%else
-%define with_qemu_kvm  0
-%endif
-
-%if ! %{with_qemu_tcg} && ! %{with_qemu_kvm}
-%define with_qemu 0
-%endif
+%define with_qemu_kvm  %{with_qemu}
 
 # Then the hypervisor drivers that run outside libvirtd, in libvirt.so
 %define with_openvz0%{!?_without_openvz:1}
@@ -61,12 +42,6 @@
 %endif
 
 %define with_storage_gluster 0%{!?_without_storage_gluster:1}
-%ifnarch %{qemu_kvm_arches}
-# gluster is only built where qemu driver is enabled on RHEL 8
-%if 0%{?rhel} >= 8
-%define with_storage_gluster 0
-%endif
-%endif
 
 %define with_numactl  0%{!?_without_numactl:1}
 
@@ -97,6 +72,11 @@
 
 # Finally set the OS / architecture specific special cases
 
+# KVM is available on most architectures
+%ifnarch %{ix86} x86_64 %{power64} s390x %{arm} aarch64
+%define with_qemu_kvm 0
+%endif
+
 # Xen is available only on i386 x86_64 ia64
 %ifnarch %{ix86} x86_64 ia64
 %define with_libxl 0
@@ -122,6 +102,23 @@
 %define with_storage_rbd 0
 %endif
 
+# RHEL does not ship qemu-tcg
+%if 0%{?rhel}
+%define with_qemu_tcg 0
+%endif
+
+# In the event that qemu-tcg and qemu-kvm are unavailable, don't ship qemu
+%if ! %{with_qemu_tcg} && ! %{with_qemu_kvm}
+%define with_qemu 0
+%endif
+
+%if ! %{with_qemu_kvm}
+# gluster is only built where qemu driver is enabled on RHEL 8
+%if 0%{?rhel} >= 8
+%define with_storage_gluster 0
+%endif
+%endif
+
 # RHEL doesn't ship OpenVZ, VBox, PowerHypervisor,
 # VMware, libxenlight (Xen 4.1 and newer),
 # or HyperV.
-- 
2.26.2

Re: Entering freeze for libvirt-6.8.0

[Neal Gompa]

On Thu, Sep 24, 2020 at 3:00 PM Jiri Denemark  wrote:
>
> On Thu, Sep 24, 2020 at 18:38:18 +0200, Andrea Bolognani wrote:
> > On Thu, 2020-09-24 at 15:13 +0200, Jiri Denemark wrote:
> > > I have just tagged v6.8.0-rc1 in the repository and pushed signed
> > > tarballs and source RPMs to https://libvirt.org/sources/
> >
> > Can we please stop generating and publishing the source RPMs? They're
> > of little use considering that you can easily run
> >
> >   $ rpmbuild -ta libvirt-x.y.z.tar.xz
> >
> > to obtain RPMs from a release archive, so all they really do is
> > clutter the directory listing.
>
> The nice thing about the source RPMs is that they are internally signed
> (in contrast to an external signature in *.asc for tarballs). Personally
> I don't find them useful either, but some people may think otherwise and
> I'm not sure they will raise their voice here. We could just stop
> creating source RPMs and wait if anyone complains. Or we could be a
> little bit more conservative and just move them to a subdirectory
> perhaps. I don't mind either way.
>

I like that Source RPMs are produced, because I can trivially use them
in my import->build->test pipeline, since my build system accepts
SRPMs as input and not random tarballs. :)

So at least here's one voice for keeping them. :)


-- 
真実はいつも一つ！/ Always, there's only one truth!

Re: [PATCH FOR 6.8.0] libxl: Don't free def member of virDomainObj

[Jiri Denemark]

On Thu, Sep 24, 2020 at 10:57:59 -0600, Jim Fehlig wrote:
> The refactoring in commit de49d5bad3 accidentally dropped the statement
> setting def to NULL after successfully adding it to the virDomainObjList,
> causing it to be freed while still in use. The resulting memory
> corruption caused unpredictable behavior, often resulting in a libvirtd
> crash.
> 
> Signed-off-by: Jim Fehlig 

For 6.8.0:
Reviewed-by: Jiri Denemark 

Re: Entering freeze for libvirt-6.8.0

[Jiri Denemark]

On Thu, Sep 24, 2020 at 18:38:18 +0200, Andrea Bolognani wrote:
> On Thu, 2020-09-24 at 15:13 +0200, Jiri Denemark wrote:
> > I have just tagged v6.8.0-rc1 in the repository and pushed signed
> > tarballs and source RPMs to https://libvirt.org/sources/
> 
> Can we please stop generating and publishing the source RPMs? They're
> of little use considering that you can easily run
> 
>   $ rpmbuild -ta libvirt-x.y.z.tar.xz
> 
> to obtain RPMs from a release archive, so all they really do is
> clutter the directory listing.

The nice thing about the source RPMs is that they are internally signed
(in contrast to an external signature in *.asc for tarballs). Personally
I don't find them useful either, but some people may think otherwise and
I'm not sure they will raise their voice here. We could just stop
creating source RPMs and wait if anyone complains. Or we could be a
little bit more conservative and just move them to a subdirectory
perhaps. I don't mind either way.

Jirka

Re: [RFC DOCUMENT 00/12] kubevirt-and-kvm: Add documents

[Andrea Bolognani]

On Tue, 2020-09-22 at 11:29 +0200, Philippe Mathieu-Daudé wrote:
> Hi Andrea,

Hi Philippe, and sorry for the delay in answering!

First of all, thanks for taking the time to go through the documents
and posting your thoughts. More comments below.

> Thanks a lot for this documentation, I could learn new things,
> use cases out of my interest area. Useful as a developer to
> better understand how are used the areas I'm coding. This
> shorten a bit that gap between developers and users.
> 
> What would be more valuable than a developer review/feedback is
> having feedback from users and technical writers.
> Suggestion: also share it on qemu-disc...@nongnu.org which is
> less technical (maybe simply repost the cover and link to the
> Wiki).

More eyes would obviously be good, but note that these are really
intended to improve the interactions between QEMU/libvirt and
KubeVirt, so the audience is ultimately developers. Of course you
could say that KubeVirt developers *are* users when it comes to
QEMU/libvirt, and you wouldn't be wrong ;) Still, qemu-devel seems
like the proper venue.

> What is not obvious in this cover (and the documents pasted on
> the list) is there are schema pictures on the Wiki pages which
> are not viewable and appreciable via an email post.

You're right! I was pretty sure I had a line about that somewhere in
there but I guess it got lost during editing. Hopefully the URL at
the very beginning of each document caused people to browse the HTML
version.

> I had zero knowledge on Kubernetes. I have been confused by their
> use in the introduction...
> 
> From Index:
> 
> "The intended audience is people who are familiar with the traditional
> virtualization stack (QEMU plus libvirt), and in order to make it
> more approachable to them comparisons, are included and little to no
> knowledge of KubeVirt or Kubernetes is assumed."
> 
> Then in Architecture's {Goals and Components} there is an assumption
> Kubernetes is known. Entering in Components, Kubernetes is briefly
> but enough explained.
> 
> Then KubeVirt is very well explained.

I guess the sections in the Index you're referring to assume that you
know that Kubernetes is somehow connected to containers, and that
it's a clustered environment. Anything else I missed?

Perhaps we could move the contents of

  
https://gitlab.cee.redhat.com/abologna/kubevirt-and-kvm/-/blob/master/Components.md#kubernetes

to a small document that's linked to near the very top. Would that
improve things, in your opinion?

> Sometimes the "Other topics" category is confusing, it seems out
> of the scope of the "better understanding and documenting the
> interactions between KubeVirt and KVM" and looks like left over
> notes.

That's probably because they absolutely are O:-)

> Maybe renaming the "Other topics" section would help.
> "Unanswered questions", "Other possibilities to investigate",...

This sounds sensible :)

Thanks again for your feedback!

-- 
Andrea Bolognani / Red Hat / Virtualization

[PATCH 2/3] logging: read all bytes on EOF in event handler

[Nikolay Shirokovskiy]

If writing side writes enough bytes to the pipe and closes writing
end then we got both VIR_EVENT_HANDLE_HANGUP and VIR_EVENT_HANDLE_READ
in handler. Currently in this situation handler reads 1024 bytes
and finish reading leaving unread data in pipe.

Signed-off-by: Nikolay Shirokovskiy 
---
 src/logging/log_handler.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/logging/log_handler.c b/src/logging/log_handler.c
index cfadc69..c04f341 100644
--- a/src/logging/log_handler.c
+++ b/src/logging/log_handler.c
@@ -138,7 +138,7 @@ virLogHandlerGetLogFileFromWatch(virLogHandlerPtr handler,
 static void
 virLogHandlerDomainLogFileEvent(int watch,
 int fd,
-int events,
+int events G_GNUC_UNUSED,
 void *opaque)
 {
 virLogHandlerPtr handler = opaque;
@@ -169,14 +169,13 @@ virLogHandlerDomainLogFileEvent(int watch,
 virReportSystemError(errno, "%s",
  _("Unable to read from log pipe"));
 goto error;
+} else if (len == 0) {
+goto error;
 }
 
 if (virRotatingFileWriterAppend(logfile->file, buf, len) != len)
 goto error;
 
-if (events & VIR_EVENT_HANDLE_HANGUP)
-goto error;
-
  cleanup:
 virObjectUnlock(handler);
 return;
-- 
1.8.3.1

[PATCH 3/3] logging: fix endless loop on EOF

[Nikolay Shirokovskiy]

On EOF condition we always have POLLHUP event and read returns
0 thus we never break loop in virLogHandlerDomainLogFileDrain.

Signed-off-by: Nikolay Shirokovskiy 
---
 src/logging/log_handler.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/logging/log_handler.c b/src/logging/log_handler.c
index c04f341..152ca24 100644
--- a/src/logging/log_handler.c
+++ b/src/logging/log_handler.c
@@ -464,6 +464,8 @@ virLogHandlerDomainLogFileDrain(virLogHandlerLogFilePtr 
file)
 if (errno == EINTR)
 continue;
 return;
+} else if (len == 0) {
+return;
 }
 
 if (virRotatingFileWriterAppend(file->file, buf, len) != len)
-- 
1.8.3.1

[PATCH 0/3] logging: fix a couple of issues

[Nikolay Shirokovskiy]

The code fixed in the first actual patch is quite venerable so I made up
a synthetic test to show issue/test it is fixed. Test covers both issues.
Initially I got /tmp/virtlogd-test size of only 2048 and after the first
fix I saw actual endless loop.

Nikolay Shirokovskiy (3):
  DO NOT APPLY: code to show logging issues
  logging: read all bytes on EOF in event handler
  logging: fix endless loop on EOF

 src/logging/log_handler.c | 10 ++
 src/qemu/qemu_process.c   | 35 +++
 2 files changed, 41 insertions(+), 4 deletions(-)

-- 
1.8.3.1

[PATCH 1/3] DO NOT APPLY: code to show logging issues

[Nikolay Shirokovskiy]

---
 src/logging/log_handler.c |  1 +
 src/qemu/qemu_process.c   | 35 +++
 2 files changed, 36 insertions(+)

diff --git a/src/logging/log_handler.c b/src/logging/log_handler.c
index 87748d9..cfadc69 100644
--- a/src/logging/log_handler.c
+++ b/src/logging/log_handler.c
@@ -160,6 +160,7 @@ virLogHandlerDomainLogFileEvent(int watch,
 }
 
  reread:
+g_usleep(200 * 1000);
 len = read(fd, buf, sizeof(buf));
 if (len < 0) {
 if (errno == EINTR)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index f21b8f1..22219f8 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -93,6 +93,8 @@
 #include "viridentity.h"
 #include "virthreadjob.h"
 #include "virutil.h"
+#include "logging/log_manager.h"
+#include "logging/log_protocol.h"
 
 #define VIR_FROM_THIS VIR_FROM_QEMU
 
@@ -6829,6 +6831,39 @@ qemuProcessLaunch(virConnectPtr conn,
 incoming != NULL) < 0)
 goto cleanup;
 
+if (qemuDomainLogContextGetManager(logCtxt)) {
+int fd = -1;
+char *buf;
+int rc;
+ino_t inode;
+off_t offset;
+
+if ((fd = virLogManagerDomainOpenLogFile(
+qemuDomainLogContextGetManager(logCtxt),
+"qemu",
+vm->def->uuid,
+vm->def->name,
+"/tmp/virtlogd-test",
+VIR_LOG_MANAGER_PROTOCOL_DOMAIN_OPEN_LOG_FILE_TRUNCATE,
+NULL, NULL)) < 0)
+goto cleanup;
+
+buf = g_new0(char, 1);
+rc = safewrite(fd, buf, 1);
+VIR_FORCE_CLOSE(fd);
+
+if (rc < 1) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   "can't write all bytes to test file");
+goto cleanup;
+}
+
+if (virLogManagerDomainGetLogFilePosition(
+qemuDomainLogContextGetManager(logCtxt),
+"/tmp/virtlogd-test", 0, , ) < 0)
+goto cleanup;
+}
+
 VIR_DEBUG("Building emulator command line");
 if (!(cmd = qemuBuildCommandLine(driver,
  qemuDomainLogContextGetManager(logCtxt),
-- 
1.8.3.1

[PATCH FOR 6.8.0] libxl: Don't free def member of virDomainObj

[Jim Fehlig]

The refactoring in commit de49d5bad3 accidentally dropped the statement
setting def to NULL after successfully adding it to the virDomainObjList,
causing it to be freed while still in use. The resulting memory
corruption caused unpredictable behavior, often resulting in a libvirtd
crash.

Signed-off-by: Jim Fehlig 
---

Unpredictable is an understatement! When running monolithic libvirtd with
both qemu and xen drviers enabled, qemu crashed while initializing. Recall
it is initialized after xen.

Thread 17 "daemon-init" received signal SIGSEGV, Segmentation fault.
#0  0x7f32e5fbe9e3 in _int_malloc () at /lib64/libc.so.6
#1  0x7f32e5fbf6e0 in _int_realloc () at /lib64/libc.so.6
#2  0x7f32e5fc0729 in realloc () at /lib64/libc.so.6
#3  0x7f32e6dc21b8 in g_realloc () at /usr/lib64/libglib-2.0.so.0
#4  0x7f32e7532090 in virReallocN (ptrptr=0x7f329affcad8, size=1, 
count=1403)
at ../src/util/viralloc.c:91
#5  0x7f32e75530c7 in virCommandProcessIO (cmd=0x7f328807ff40) at 
../src/util/vircommand.c:2271
#6  0x7f32e7553a6a in virCommandRun (cmd=0x7f328807ff40, exitstatus=0x0)
at ../src/util/vircommand.c:2451
#7  0x7f32e75dde73 in virSysinfoReadDMI () at ../src/util/virsysinfo.c:1237
#8  0x7f32e75de0cb in virSysinfoRead () at ../src/util/virsysinfo.c:1294
#9  0x7f32a240b69d in qemuStateInitialize
(privileged=true, root=0x0, callback=0x56453a0b3e97 
, opaque=0x56453b30) at ../src/qemu/qemu_driver.c:658
#10 0x7f32e7832350 in virStateInitialize
(privileged=true, mandatory=false, root=0x0, callback=0x56453a0b3e97 
, opaque=0x56453b30) at ../src/libvirt.c:656
#11 0x56453a0b4175 in daemonRunStateInit (opaque=0x56453b30)
at ../src/remote/remote_daemon.c:596

 src/libxl/libxl_driver.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 083738871d..571b70f982 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -627,6 +627,7 @@ libxlAddDom0(libxlDriverPrivatePtr driver)
NULL)))
 goto cleanup;
 
+def = NULL;
 vm->persistent = 1;
 virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, 
VIR_DOMAIN_RUNNING_BOOTED);
 }
-- 
2.28.0

Re: Entering freeze for libvirt-6.8.0

[Andrea Bolognani]

On Thu, 2020-09-24 at 15:13 +0200, Jiri Denemark wrote:
> I have just tagged v6.8.0-rc1 in the repository and pushed signed
> tarballs and source RPMs to https://libvirt.org/sources/

Can we please stop generating and publishing the source RPMs? They're
of little use considering that you can easily run

  $ rpmbuild -ta libvirt-x.y.z.tar.xz

to obtain RPMs from a release archive, so all they really do is
clutter the directory listing.

> If you have not done so yet, please update NEWS.rst to document any
> significant change you made since the last release.

Yes, please do that everybody :)

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [libvirt PATCH] news: document recent esx API implementations

[Andrea Bolognani]

On Thu, 2020-09-24 at 17:24 +0200, Pino Toscano wrote:
> Signed-off-by: Pino Toscano 
> ---
>  NEWS.rst | 7 +++
>  1 file changed, 7 insertions(+)

Reviewed-by: Andrea Bolognani 

-- 
Andrea Bolognani / Red Hat / Virtualization

[PATCH] docs: bhyve: document sound device and VNC bits

[Roman Bogorodskiy]

 * Document sound device support,
 * Document VNC password configuration and framebuffer resolution.

Signed-off-by: Roman Bogorodskiy 
---
 docs/drvbhyve.html.in | 47 +++
 1 file changed, 47 insertions(+)

diff --git a/docs/drvbhyve.html.in b/docs/drvbhyve.html.in
index ca511eeccd..cffb63f1ad 100644
--- a/docs/drvbhyve.html.in
+++ b/docs/drvbhyve.html.in
@@ -389,6 +389,30 @@ it with the port attribute):
 graphics type='vnc' autoport='yes'
 
 
+Since 6.8.0, it's possible to set framebuffer 
resolution
+using the resolution sub-element:
+
+
+   video
+ model type='gop' heads='1' primary='yes'
+   resolution x='800' y='600'/
+ /model
+   /video
+
+
+Since 6.8.0, VNC server can be configured to use
+password based authentication:
+
+
+  graphics type='vnc' port='5904' passwd='foobar'
+listen type='address' address='127.0.0.1'/
+  /graphics
+
+
+Note: VNC password authentication is known to be cryptographically weak.
+Additionally, the password is passed as a command line argument in clear text.
+Make sure you understand the risks associated with this feature before using 
it.
+
 Clock configuration
 
 Originally bhyve supported only localtime for RTC. Support for UTC time was 
introduced in
@@ -432,6 +456,29 @@ supports Intel e1000 network adapter emulation. It's 
supported in libvirt
 ...
 
 
+Sound device
+
+As of https://svnweb.freebsd.org/changeset/base/349355;>r349355 bhyve
+supports sound device emulation. It's supported in libvirt
+since 6.7.0.
+
+
+...
+  sound model='ich7'
+audio id='1'/
+  /sound
+  audio id='1' type='oss'
+input dev='/dev/dsp0'/
+output dev='/dev/dsp0'/
+  /audio
+...
+
+
+Here, the sound element specifies the sound device as it's 
exposed
+to the guest, with ich7 being the only supported model now,
+and the audio element specifies how the guest device is mapped
+to the host sound device.
+
 Wiring guest memory
 
 Since 4.4.0, it's possible to specify that guest 
memory should
-- 
2.27.0

Re: [libvirt PATCH] news: add note about new virt-ssh-helper binary

[Andrea Bolognani]

On Thu, 2020-09-24 at 16:00 +0100, Daniel P. Berrangé wrote:
> +  * remote: ``virt-ssh-helper`` replaces ``nc`` for SSH tunnelling
> +
> +Libvirt now provides a ``virt-ssh-helper`` binary on the server
> +side. The libvirt remote client will use this binary for setting
> +up an SSH tunnelled connection to hosts. If not present, it will
> +transparently fallback to the traditional ``nc`` tunnel. The new
> +binary makes it possible for libvirt to transparently connect
> +across hosts even if libvirt is built with a different installation
> +prefix on the client vs server. It also enables remote access to
> +the unprivileged per-user libvirt daemons(eg using a URI such as

s/daemons(eg/daemons (eg./

> +``qemu+ssh://hostname/session``. The only requirement is that

s/session``/session``)/

> +``virt-ssh-helper`` is present in $PATH of the remote host.

s/$PATH/``$PATH``/

With these nits addressed,

  Reviewed-by: Andrea Bolognani 

and safe for freeze.

-- 
Andrea Bolognani / Red Hat / Virtualization

[libvirt PATCH] news: document recent esx API implementations

[Pino Toscano]

Signed-off-by: Pino Toscano 
---
 NEWS.rst | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/NEWS.rst b/NEWS.rst
index e992fbe471..7c723e6610 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -32,6 +32,13 @@ v6.8.0 (unreleased)
 can now be passed using the ``passwd`` attribute on
 the  element.
 
+  * esx: implement few APIs
+
+The ``virConnectListAllNetworks()``, ``virDomainGetHostname()``, and
+``virDomainInterfaceAddresses()`` (only for
+``VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT`` source) APIs were implemented
+in the esx driver.
+
 * **Improvements**
 
   * qemu: Allow migration over UNIX sockets
-- 
2.26.2

[libvirt PATCH] news: add note about new virt-ssh-helper binary

[Daniel P . Berrangé]

Signed-off-by: Daniel P. Berrangé 
---
 NEWS.rst | 13 +
 1 file changed, 13 insertions(+)

diff --git a/NEWS.rst b/NEWS.rst
index e992fbe471..13d2ff1d2f 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -32,6 +32,19 @@ v6.8.0 (unreleased)
 can now be passed using the ``passwd`` attribute on
 the  element.
 
+  * remote: ``virt-ssh-helper`` replaces ``nc`` for SSH tunnelling
+
+Libvirt now provides a ``virt-ssh-helper`` binary on the server
+side. The libvirt remote client will use this binary for setting
+up an SSH tunnelled connection to hosts. If not present, it will
+transparently fallback to the traditional ``nc`` tunnel. The new
+binary makes it possible for libvirt to transparently connect
+across hosts even if libvirt is built with a different installation
+prefix on the client vs server. It also enables remote access to
+the unprivileged per-user libvirt daemons(eg using a URI such as
+``qemu+ssh://hostname/session``. The only requirement is that
+``virt-ssh-helper`` is present in $PATH of the remote host.
+
 * **Improvements**
 
   * qemu: Allow migration over UNIX sockets
-- 
2.26.2

Re: [libvirt PATCH v4 00/11] remote: introduce a custom netcat impl for ssh tunnelling

[Andrea Bolognani]

On Fri, 2020-08-07 at 18:40 +0100, Daniel P. Berrangé wrote:
> Daniel P. Berrangé (11):
>   rpc: merge logic for generating remote SSH shell script
>   remote: push logic for default netcat binary into common helper
>   remote: split off enums into separate source file
>   remote: split out function for parsing URI scheme
>   remote: parse the remote transport string earlier
>   remote: split out function for constructing socket path
>   remote: extract logic for determining daemon to connect to
>   remote: introduce virt-ssh-helper binary
>   rpc: switch order of args in virNetClientNewSSH
>   rpc: use new virt-ssh-helper binary for remote tunnelling
>   remote: fix error reporting for invalid daemon mode

Can you please mention this in the release notes? It's quite an
important change, although hopefully it will be completely
transparent to users.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH V3] Modify virCPUarmCompare to perform compare actions

[Zhenyu Zheng]

Thanks alot, I've addressed the comments and updated the patch to V4,
please have a look.

BR

On Thu, Sep 24, 2020 at 1:30 AM Jiri Denemark  wrote:

> On Wed, Sep 16, 2020 at 16:58:03 +0800, Zhenyu Zheng wrote:
> > Modify virCPUarmCompare in cpu_arm.c to perform compare action.
> > This patch only adds host to host CPU compare, the rest cases
> > remains the same. This is useful for source and destination host
> > compare during migrations to avoid migration between different
> > CPU models that have different CPU freatures.
> >
> > Signed-off-by: Zhenyu Zheng 
> > ---
> >  src/cpu/cpu_arm.c | 43 +++
> >  1 file changed, 39 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c
> > index 939a3b8390..b420b14e86 100644
> > --- a/src/cpu/cpu_arm.c
> > +++ b/src/cpu/cpu_arm.c
> > @@ -463,11 +463,46 @@ virCPUarmBaseline(virCPUDefPtr *cpus,
> >  }
> >
> >  static virCPUCompareResult
> > -virCPUarmCompare(virCPUDefPtr host G_GNUC_UNUSED,
> > - virCPUDefPtr cpu G_GNUC_UNUSED,
> > - bool failMessages G_GNUC_UNUSED)
> > +virCPUarmCompare(virCPUDefPtr host,
> > + virCPUDefPtr cpu,
> > + bool failIncompatible
> > +)
> >  {
> > -return VIR_CPU_COMPARE_IDENTICAL;
> > +virCPUCompareResult ret = VIR_CPU_COMPARE_IDENTICAL;
>
> This looks a bit too complicated as there's no common code to be
> executed for several ret values. Just drop this variable and return
> directly wherever you set it.
>
> > +
> > +/* Only support host to host CPU compare for ARM*/
> > +if (cpu->type != VIR_CPU_TYPE_HOST)
> > +return ret;
> > +
> > +if (!host || !host->model) {
> > +if (failIncompatible) {
> > +virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s",
> > +   _("unknown host CPU"));
> > +ret = VIR_CPU_COMPARE_ERROR;
> > +} else {
> > +VIR_WARN("unknown host CPU");
> > +ret = VIR_CPU_COMPARE_INCOMPATIBLE;
> > +}
> > +return ret;
> > +}
> > +
> > +/* Compare vendor and model to check if CPUs are identical */
> > +if (STRNEQ(host->vendor, cpu->vendor) ||
> > +STRNEQ(host->model, cpu->model)) {
>
> Use STRNEQ_NULLABLE instead.
>
> > +VIR_DEBUG("Host CPU model does not match required CPU model %s",
> > +  cpu->model);
>
> NULLSTR(cpu->model)
>
> and I would also add NULLSTR(cpu->vendor) in the message just in case
> the CPUs differ only in vendor.
>
> > +
> > +if (failIncompatible) {
> > +ret = VIR_CPU_COMPARE_ERROR;
> > +virReportError(VIR_ERR_CPU_INCOMPATIBLE,
> > +   _("Host CPU model does not match required
> CPU model %s"),
> > +   cpu->model);
> > +} else {
> > +ret = VIR_CPU_COMPARE_INCOMPATIBLE;
> > +}
> > +}
> > +
> > +return ret;
> >  }
> >
> >  static int
>
> Jirka
>
>

[PATCH V4] Modify virCPUarmCompare to perform compare actions

[Zhenyu Zheng]

Modify virCPUarmCompare in cpu_arm.c to perform compare action.
This patch only adds host to host CPU compare, the rest cases
remains the same. This is useful for source and destination host
compare during migrations to avoid migration between different
CPU models that have different CPU freatures.

Signed-off-by: Zhenyu Zheng 
---
 src/cpu/cpu_arm.c | 39 ---
 1 file changed, 36 insertions(+), 3 deletions(-)

diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c
index 939a3b8390..64bd0f03c2 100644
--- a/src/cpu/cpu_arm.c
+++ b/src/cpu/cpu_arm.c
@@ -463,10 +463,43 @@ virCPUarmBaseline(virCPUDefPtr *cpus,
 }
 
 static virCPUCompareResult
-virCPUarmCompare(virCPUDefPtr host G_GNUC_UNUSED,
- virCPUDefPtr cpu G_GNUC_UNUSED,
- bool failMessages G_GNUC_UNUSED)
+virCPUarmCompare(virCPUDefPtr host,
+ virCPUDefPtr cpu,
+ bool failIncompatible
+)
 {
+/* Only support host to host CPU compare for ARM*/
+if (cpu->type != VIR_CPU_TYPE_HOST)
+return VIR_CPU_COMPARE_IDENTICAL;
+
+if (!host || !host->model) {
+if (failIncompatible) {
+virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s",
+   _("unknown host CPU"));
+return VIR_CPU_COMPARE_ERROR;
+} else {
+VIR_WARN("unknown host CPU");
+return VIR_CPU_COMPARE_INCOMPATIBLE;
+}
+}
+
+/* Compare vendor and model to check if CPUs are identical */
+if (STRNEQ_NULLABLE(host->vendor, cpu->vendor) ||
+STRNEQ_NULLABLE(host->model, cpu->model)) {
+VIR_DEBUG("Host CPU model does not match required CPU "
+  "vendor %s or(and) model %s",
+  NULLSTR(cpu->vendor), NULLSTR(cpu->model));
+
+if (failIncompatible) {
+virReportError(VIR_ERR_CPU_INCOMPATIBLE,
+   _("Host CPU model does not match required CPU "
+ "vendor %s or(and) model %s"),
+   NULLSTR(cpu->vendor), NULLSTR(cpu->model));
+return VIR_CPU_COMPARE_ERROR;
+} else {
+return VIR_CPU_COMPARE_INCOMPATIBLE;
+}
+}
 return VIR_CPU_COMPARE_IDENTICAL;
 }
 
-- 
2.20.1

Re: [PATCH v1 2/2] qemu: driver: perform CPU model expansion on single CPU during baseline

[Collin Walling]

On 9/24/20 3:30 AM, Jiri Denemark wrote:
> On Wed, Sep 23, 2020 at 22:26:29 -0400, Collin Walling wrote:
>> When executing the hypervisor-cpu-baseline command and if there is only
>> a single CPU definition present in the XML file, then libvirt will
>> print an unhelpful message:
>>
>> "error: An error occurred, but the cause is unknown"
>>
>> This is due to no CPU definition ever being "baselined", since the
>> API expects at least two CPU models.
>>
>> Let's fix this by performing a CPU model expansion on the single CPU
>> definition and returning the result to the caller.
> 
> Ah, so when host-passthrough CPU is passed to the baseline API, we
> should report an error. So this patch is actually trying to handle
> single CPU definition with a non-empty  element specified. Good,
> as the API is of course supposed to work in this case. It should
> basically return the CPU definition passed to it with unsupported
> features disabled.
> 
> Normally, expansion should only be done when requested by the
> corresponding API flag. The simplest fix would be just returning the
> original CPU definition if only one was passed to baseline. But the
> result might not be the correct one as it could include unsupported

Right. In fact, if the model cannot be baselined (due to only a single
CPU provided to the API), and we DON'T call expansion here, the result
will just be the same verbatim XML that the user provided... which may
or may not be correct...

Calling expansion on the model will guarantee a sane response.

> features. So is static expansion the thing that will return the expected
> result? If so, this patch is mostly correct...

Precisely. A static expansion will return the CPU model and a
non-exhaustive list of features associated with the model.

> 
>> Signed-off-by: Collin Walling 
>> ---
>>  src/qemu/qemu_driver.c | 17 +
>>  1 file changed, 9 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 427d2419f3..97a960a769 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -12472,6 +12472,7 @@ qemuConnectCPUModelBaseline(virQEMUCapsPtr qemuCaps,
>>  g_autoptr(qemuProcessQMP) proc = NULL;
>>  g_autoptr(virCPUDef) baseline = NULL;
>>  qemuMonitorCPUModelInfoPtr result = NULL;
>> +qemuMonitorCPUModelExpansionType expansion_type;
>>  size_t i;
>>  
>>  if (!(proc = qemuProcessQMPNew(virQEMUCapsGetBinary(qemuCaps),
>> @@ -12503,15 +12504,15 @@ qemuConnectCPUModelBaseline(virQEMUCapsPtr 
>> qemuCaps,
>>  return NULL;
>>  }
>>  
>> -if (expand_features) {
> 
> While full expansion has to always be performed on the baseline result,
> static expansion should only be called when ncpus == 1. In other words,
> rather than removing this condition, you should change it to
> 
>if (expand_features || ncpus == 1) {
> 

Sounds good. I'll replace patch #1 with something that simply checks for
a missing model name within baseline and prints a helpful error, and
make the appropriate changes to this one.

>> -if (qemuMonitorGetCPUModelExpansion(proc->mon,
>> -
>> QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL,
>> -baseline, true, false, ) 
>> < 0)
>> -return NULL;
>> +expansion_type = expand_features ? QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL
>> + : 
>> QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC;
>>  
>> -if (qemuConnectStealCPUModelFromInfo(baseline, ) < 0)
>> -return NULL;
>> -}
>> +if (qemuMonitorGetCPUModelExpansion(proc->mon, expansion_type,
>> +baseline, true, false, ) < 0)
>> +return NULL;
>> +
>> +if (qemuConnectStealCPUModelFromInfo(baseline, ) < 0)
>> +return NULL;
>>  
>>  return g_steal_pointer();
>>  }
> 
> Jirka
> 

Thanks!

-- 
Regards,
Collin

Stay safe and stay healthy

Entering freeze for libvirt-6.8.0

[Jiri Denemark]

I have just tagged v6.8.0-rc1 in the repository and pushed signed
tarballs and source RPMs to https://libvirt.org/sources/

Please give the release candidate some testing and in case you find a
serious issue which should have a fix in the upcoming release, feel
free to reply to this thread to make sure the issue is more visible.

If you have not done so yet, please update NEWS.rst to document any
significant change you made since the last release.

Thanks,

Jirka

[PATCH] NEWS: Mention qcow2 cluster size being preserved accross snapshots and iSCSI hostdev fixes

[Peter Krempa]

Signed-off-by: Peter Krempa 
---
 NEWS.rst | 12 
 1 file changed, 12 insertions(+)

diff --git a/NEWS.rst b/NEWS.rst
index e992fbe471..97417033f4 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -55,8 +55,20 @@ v6.8.0 (unreleased)
 the previous behavior with an improvement: it also reflects the 
auto-aligned
 value in the domain XML.

+  * qemu: Preserve qcow2 cluster size after external snapshots
+
+   The new overlay image which is installed on top of the current chain when
+   taking an external snapshot now preserves the cluser size of the original
+   top image to preserve any performance tuning done on the original image.
+
 * **Bug fixes**

+  * qemu: Various (i)SCSI backed hostdev fixes
+
+   (i)SCSI backed hostdevs now work again with an arbitrarily long
+   user-specified device alias and also honor the 'readonly' property after a
+   recent rewrite.
+
 * **Removed features**

   * node_device: Remove HAL node device backend
-- 
2.26.2

[RFC PATCH v2] fix error message in virMigrate3 if connection is broken

[Nikolay Shirokovskiy]

Changes from v1 [1]:
- return error value from VIR_DRV_SUPPORTS_FEATURE instead
  of setting error to out argument (decrease over enginering
  level on patch generator in other words :)

Currently virDomainMigrate3 reports "this function is not supported by the
connection driver: virDomainMigrate3" if connection to destination for example
is broken. This is a bit misleading.

This is a result of we treat errors as unsupported feature in
VIR_DRV_SUPPORTS_FEATURE macro. Let's return error from macro as is.

I guess all the other occurences of VIR_DRV_SUPPORTS_FEATURE need to be updated
as well so that we detect errors early and not have issues similar to
virDomainMigrate3. I'm going to fix the other places if this RFC is approved.

[1] https://www.redhat.com/archives/libvir-list/2020-September/msg01056.html
---
 src/driver.h | 14 
 src/libvirt-domain.c | 91 +---
 2 files changed, 79 insertions(+), 26 deletions(-)

diff --git a/src/driver.h b/src/driver.h
index 6278aa0..c29b2fa 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -60,6 +60,20 @@ typedef enum {
 (drv)->connectSupportsFeature((conn), (feature)) > 0 : 0)
 
 
+/*
+ * A little wrapper for connectSupportsFeature API to test that the API
+ * itself is available first. Return values are same as for API.
+ *
+ * Returns:
+ *  -1  on error
+ *   0  feature is not supported
+ *   1  feature is supported
+ */
+#define VIR_DRV_SUPPORTS_FEATURE2(drv, conn, feature) \
+((drv)->connectSupportsFeature ? \
+(drv)->connectSupportsFeature((conn), (feature)) : 0)
+
+
 #define __VIR_DRIVER_H_INCLUDES___
 
 #include "driver-hypervisor.h"
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index cde86c7..03c357f 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -3846,6 +3846,9 @@ virDomainMigrate3(virDomainPtr domain,
 const char *dname = NULL;
 const char *dxml = NULL;
 unsigned long long bandwidth = 0;
+int rc_src;
+int rc_dst;
+int rc;
 
 VIR_DOMAIN_DEBUG(domain, "dconn=%p, params=%p, nparms=%u flags=0x%x",
  dconn, params, nparams, flags);
@@ -3878,15 +3881,21 @@ virDomainMigrate3(virDomainPtr domain,
 }
 
 if (flags & VIR_MIGRATE_OFFLINE) {
-if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
-  VIR_DRV_FEATURE_MIGRATION_OFFLINE)) {
+rc = VIR_DRV_SUPPORTS_FEATURE2(domain->conn->driver, domain->conn,
+   VIR_DRV_FEATURE_MIGRATION_OFFLINE);
+if (rc < 0) {
+goto error;
+} else if (rc == 0) {
 virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
_("offline migration is not supported by "
  "the source host"));
 goto error;
 }
-if (!VIR_DRV_SUPPORTS_FEATURE(dconn->driver, dconn,
-  VIR_DRV_FEATURE_MIGRATION_OFFLINE)) {
+rc = VIR_DRV_SUPPORTS_FEATURE2(dconn->driver, dconn,
+   VIR_DRV_FEATURE_MIGRATION_OFFLINE);
+if (rc < 0) {
+goto error;
+} else if (rc == 0) {
 virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
_("offline migration is not supported by "
  "the destination host"));
@@ -3899,21 +3908,30 @@ virDomainMigrate3(virDomainPtr domain,
  * the flag for just the source side.  We mask it out to allow
  * migration from newer source to an older destination that
  * rejects the flag.  */
-if (flags & VIR_MIGRATE_CHANGE_PROTECTION &&
-!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
-  VIR_DRV_FEATURE_MIGRATE_CHANGE_PROTECTION)) {
-virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
-   _("cannot enforce change protection"));
-goto error;
+if (flags & VIR_MIGRATE_CHANGE_PROTECTION) {
+rc = VIR_DRV_SUPPORTS_FEATURE2(domain->conn->driver, domain->conn,
+   
VIR_DRV_FEATURE_MIGRATE_CHANGE_PROTECTION);
+if (rc < 0) {
+goto error;
+} else if (rc == 0) {
+virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+   _("cannot enforce change protection"));
+goto error;
+}
 }
 flags &= ~VIR_MIGRATE_CHANGE_PROTECTION;
 
 /* Prefer extensible API but fall back to older migration APIs if params
  * only contains parameters which were supported by the older API. */
-if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
- VIR_DRV_FEATURE_MIGRATION_PARAMS) &&
-VIR_DRV_SUPPORTS_FEATURE(dconn->driver, dconn,
- VIR_DRV_FEATURE_MIGRATION_PARAMS)) {
+rc_src = VIR_DRV_SUPPORTS_FEATURE2(domain->conn->driver, 

[libvirt PATCH 5/6] docs: glib-adoption: add links to GLib documentation

[Ján Tomko]

Make life a bit easier for people unfamiliar with GLib.

Signed-off-by: Ján Tomko 
---
 docs/glib-adoption.rst | 4 
 1 file changed, 4 insertions(+)

diff --git a/docs/glib-adoption.rst b/docs/glib-adoption.rst
index fc0a8cfb08..6dcd4bc14e 100644
--- a/docs/glib-adoption.rst
+++ b/docs/glib-adoption.rst
@@ -18,6 +18,8 @@ Memory allocation
``VIR_ALLOC``, ``VIR_REALLOC``, ``VIR_RESIZE_N``,
``VIR_EXPAND_N``, ``VIR_SHRINK_N``, ``VIR_FREE``
 
+   https://developer.gnome.org/glib/stable/glib-Memory-Allocation.html
+
Prefer the GLib APIs ``g_new0``/``g_renew``/ ``g_free`` in most
cases.  There should rarely be a need to use
``g_malloc``/``g_realloc``.  **NEVER MIX** use of the classic
@@ -28,6 +30,8 @@ Memory allocation
 Array operations
``VIR_APPEND_ELEMENT``, ``VIR_INSERT_ELEMENT``, ``VIR_DELETE_ELEMENT``
 
+   https://developer.gnome.org/glib/stable/glib-Arrays.html
+
Instead of using plain C arrays, it is preferrable to use one of
the GLib types, ``GArray``, ``GPtrArray`` or ``GByteArray``.
These all use a struct to track the array memory and size
-- 
2.26.2

[libvirt PATCH 4/6] docs: glib-adoption: split into sections

[Ján Tomko]

Although all the mentioned functions deal with
allocation, replacing the pure allocation
functions is easier than converting code to
use GArrays.

Split them out to encourage usage of GLib
allocation APIs even at the cost of them
being combined with VIR_*ELEMENT APIs.

Signed-off-by: Ján Tomko 
---
 docs/glib-adoption.rst | 27 +--
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/docs/glib-adoption.rst b/docs/glib-adoption.rst
index 91d0c66d91..fc0a8cfb08 100644
--- a/docs/glib-adoption.rst
+++ b/docs/glib-adoption.rst
@@ -14,14 +14,21 @@ the GLib APIs straight away where possible.
 The following is a list of libvirt APIs that should no longer be
 used in new code, and their suggested GLib replacements:
 
-``VIR_ALLOC``, ``VIR_REALLOC``, ``VIR_RESIZE_N``, ``VIR_EXPAND_N``, 
``VIR_SHRINK_N``, ``VIR_FREE``, ``VIR_APPEND_ELEMENT``, ``VIR_INSERT_ELEMENT``, 
``VIR_DELETE_ELEMENT``
+Memory allocation
+   ``VIR_ALLOC``, ``VIR_REALLOC``, ``VIR_RESIZE_N``,
+   ``VIR_EXPAND_N``, ``VIR_SHRINK_N``, ``VIR_FREE``
+
Prefer the GLib APIs ``g_new0``/``g_renew``/ ``g_free`` in most
-   cases. There should rarely be a need to use
-   ``g_malloc``/``g_realloc``. Instead of using plain C arrays, it
-   is preferrable to use one of the GLib types, ``GArray``,
-   ``GPtrArray`` or ``GByteArray``. These all use a struct to
-   track the array memory and size together and efficiently
-   resize. **NEVER MIX** use of the classic libvirt memory
-   allocation APIs and GLib APIs within a single method. Keep the
-   style consistent, converting existing code to GLib style in a
-   separate, prior commit.
+   cases.  There should rarely be a need to use
+   ``g_malloc``/``g_realloc``.  **NEVER MIX** use of the classic
+   libvirt memory allocation APIs and GLib APIs within a single
+   method. Keep the style consistent, converting existing code to
+   GLib style in a separate, prior commit.
+
+Array operations
+   ``VIR_APPEND_ELEMENT``, ``VIR_INSERT_ELEMENT``, ``VIR_DELETE_ELEMENT``
+
+   Instead of using plain C arrays, it is preferrable to use one of
+   the GLib types, ``GArray``, ``GPtrArray`` or ``GByteArray``.
+   These all use a struct to track the array memory and size
+   together and efficiently resize.
-- 
2.26.2

[libvirt PATCH 0/6] Resurrect glib-adoption.rst

[Ján Tomko]

Ján Tomko (6):
  Revert "docs: Drop glib-adoption.rst"
  docs: build glib-adoption.html
  docs: glib-adoption: remove stuff we alredy removed
  docs: glib-adoption: split into sections
  docs: glib-adoption: add links to GLib documentation
  docs: glib-adoption: add string arrays and objects

 docs/glib-adoption.rst | 54 ++
 docs/hacking.rst   |  1 +
 docs/meson.build   |  1 +
 3 files changed, 56 insertions(+)
 create mode 100644 docs/glib-adoption.rst

-- 
2.26.2

[libvirt PATCH 3/6] docs: glib-adoption: remove stuff we alredy removed

[Ján Tomko]

https://www.redhat.com/archives/libvir-list/2020-May/msg00299.html

Suggested-by: Daniel P. Berrangé 
Signed-off-by: Ján Tomko 
---
 docs/glib-adoption.rst | 69 --
 1 file changed, 69 deletions(-)

diff --git a/docs/glib-adoption.rst b/docs/glib-adoption.rst
index 62ddd7c61d..91d0c66d91 100644
--- a/docs/glib-adoption.rst
+++ b/docs/glib-adoption.rst
@@ -25,72 +25,3 @@ used in new code, and their suggested GLib replacements:
allocation APIs and GLib APIs within a single method. Keep the
style consistent, converting existing code to GLib style in a
separate, prior commit.
-``virStrerror``
-   The GLib ``g_strerror()`` function should be used instead,
-   which has a simpler calling convention as an added benefit.
-
-The following libvirt APIs have been deleted already:
-
-``VIR_AUTOPTR``, ``VIR_AUTOCLEAN``, ``VIR_AUTOFREE``
-   The GLib macros ``g_autoptr``, ``g_auto`` and ``g_autofree``
-   must be used instead in all new code. In existing code, the
-   GLib macros must never be mixed with libvirt macros within a
-   method, nor should they be mixed with ``VIR_FREE``. If
-   introducing GLib macros to an existing method, any use of
-   libvirt macros must be converted in an independent commit.
-``VIR_DEFINE_AUTOPTR_FUNC``, ``VIR_DEFINE_AUTOCLEAN_FUNC``
-   The GLib macros ``G_DEFINE_AUTOPTR_CLEANUP_FUNC`` and
-   ``G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC`` must be used in all new
-   code. Existing code should be converted to the new macros where
-   relevant. It is permissible to use ``g_autoptr``, ``g_auto`` on
-   an object whose cleanup function is declared with the libvirt
-   macros and vice-versa.
-``VIR_AUTOUNREF``
-   The GLib macros ``g_autoptr`` and
-   ``G_DEFINE_AUTOPTR_CLEANUP_FUNC`` should be used to manage
-   autoclean of virObject classes. This matches usage with GObject
-   classes.
-``VIR_STRDUP``, ``VIR_STRNDUP``
-   Prefer the GLib APIs ``g_strdup`` and ``g_strndup``.
-
-+---+--+---+
-| deleted version   | GLib version | Notes 
|
-+===+==+===+
-| ``VIR_AUTOPTR``   | ``g_autoptr``|   
|
-+---+--+---+
-| ``VIR_AUTOCLEAN`` | ``g_auto``   |   
|
-+---+--+---+
-| ``VIR_AUTOFREE``  | ``g_autofree``   | The 
GLib version does not use parentheses |
-+---+--+---+
-| ``VIR_AUTOUNREF`` | ``g_autoptr``| The 
cleanup function needs to be defined  |
-+---+--+---+
-| ``VIR_DEFINE_AUTOPTR_FUNC``   | ``G_DEFINE_AUTOPTR_CLEANUP_FUNC``|   
|
-+---+--+---+
-| ``VIR_DEFINE_AUTOCLEAN_FUNC`` | ``G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC`` |   
|
-+---+--+---+
-| ``VIR_STEAL_PTR`` | ``g_steal_pointer``  | ``a = 
f()`` instead of ``f(a, b)``  |
-+---+--+---+
-| ``VIR_RETURN_PTR``| ``return g_steal_pointer``   |   
|
-+---+--+---+
-| ``ARRAY_CARDINALITY`` | ``G_N_ELEMENTS`` |   
|
-+---+--+---+
-| ``ATTRIBUTE_FALLTHROUGH`` | ``G_GNUC_FALLTHROUGH``   |   
|
-+---+--+---+
-| ``ATTRIBUTE_FMT_PRINTF``  | ``G_GNUC_PRINTF``|   
|
-+---+--+---+
-| ``ATTRIBUTE_NOINLINE``| ``G_GNUC_NO_INLINE``  

[libvirt PATCH 2/6] docs: build glib-adoption.html

[Ján Tomko]

We switched to meson in the meantime so the conversion
to HTML has to be explicitly requested.

Signed-off-by: Ján Tomko 
---
 docs/meson.build | 1 +
 1 file changed, 1 insertion(+)

diff --git a/docs/meson.build b/docs/meson.build
index 7397d6409b..400c1ca955 100644
--- a/docs/meson.build
+++ b/docs/meson.build
@@ -117,6 +117,7 @@ docs_rst_files = [
   'formatbackup',
   'formatcheckpoint',
   'formatdomain',
+  'glib-adoption',
   'hacking',
   'kbase',
   'libvirt-go',
-- 
2.26.2

[libvirt PATCH 1/6] Revert "docs: Drop glib-adoption.rst"

[Ján Tomko]

Cleaning up after Andrea as he requested:
https://www.redhat.com/archives/libvir-list/2020-May/msg00405.html

This reverts commit 842d3712ed612f213b454e610cc7ff9db2321803
---
 docs/glib-adoption.rst | 96 ++
 docs/hacking.rst   |  1 +
 2 files changed, 97 insertions(+)
 create mode 100644 docs/glib-adoption.rst

diff --git a/docs/glib-adoption.rst b/docs/glib-adoption.rst
new file mode 100644
index 00..62ddd7c61d
--- /dev/null
+++ b/docs/glib-adoption.rst
@@ -0,0 +1,96 @@
+=
+Adoption of GLib APIs
+=
+
+Libvirt has adopted use of the `GLib
+library `__. Due to
+libvirt's long history of development, there are many APIs in
+libvirt, for which GLib provides an alternative solution. The
+general rule to follow is that the standard GLib solution will be
+preferred over historical libvirt APIs. Existing code will be
+ported over to use GLib APIs over time, but new code should use
+the GLib APIs straight away where possible.
+
+The following is a list of libvirt APIs that should no longer be
+used in new code, and their suggested GLib replacements:
+
+``VIR_ALLOC``, ``VIR_REALLOC``, ``VIR_RESIZE_N``, ``VIR_EXPAND_N``, 
``VIR_SHRINK_N``, ``VIR_FREE``, ``VIR_APPEND_ELEMENT``, ``VIR_INSERT_ELEMENT``, 
``VIR_DELETE_ELEMENT``
+   Prefer the GLib APIs ``g_new0``/``g_renew``/ ``g_free`` in most
+   cases. There should rarely be a need to use
+   ``g_malloc``/``g_realloc``. Instead of using plain C arrays, it
+   is preferrable to use one of the GLib types, ``GArray``,
+   ``GPtrArray`` or ``GByteArray``. These all use a struct to
+   track the array memory and size together and efficiently
+   resize. **NEVER MIX** use of the classic libvirt memory
+   allocation APIs and GLib APIs within a single method. Keep the
+   style consistent, converting existing code to GLib style in a
+   separate, prior commit.
+``virStrerror``
+   The GLib ``g_strerror()`` function should be used instead,
+   which has a simpler calling convention as an added benefit.
+
+The following libvirt APIs have been deleted already:
+
+``VIR_AUTOPTR``, ``VIR_AUTOCLEAN``, ``VIR_AUTOFREE``
+   The GLib macros ``g_autoptr``, ``g_auto`` and ``g_autofree``
+   must be used instead in all new code. In existing code, the
+   GLib macros must never be mixed with libvirt macros within a
+   method, nor should they be mixed with ``VIR_FREE``. If
+   introducing GLib macros to an existing method, any use of
+   libvirt macros must be converted in an independent commit.
+``VIR_DEFINE_AUTOPTR_FUNC``, ``VIR_DEFINE_AUTOCLEAN_FUNC``
+   The GLib macros ``G_DEFINE_AUTOPTR_CLEANUP_FUNC`` and
+   ``G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC`` must be used in all new
+   code. Existing code should be converted to the new macros where
+   relevant. It is permissible to use ``g_autoptr``, ``g_auto`` on
+   an object whose cleanup function is declared with the libvirt
+   macros and vice-versa.
+``VIR_AUTOUNREF``
+   The GLib macros ``g_autoptr`` and
+   ``G_DEFINE_AUTOPTR_CLEANUP_FUNC`` should be used to manage
+   autoclean of virObject classes. This matches usage with GObject
+   classes.
+``VIR_STRDUP``, ``VIR_STRNDUP``
+   Prefer the GLib APIs ``g_strdup`` and ``g_strndup``.
+
++---+--+---+
+| deleted version   | GLib version | Notes 
|
++===+==+===+
+| ``VIR_AUTOPTR``   | ``g_autoptr``|   
|
++---+--+---+
+| ``VIR_AUTOCLEAN`` | ``g_auto``   |   
|
++---+--+---+
+| ``VIR_AUTOFREE``  | ``g_autofree``   | The 
GLib version does not use parentheses |
++---+--+---+
+| ``VIR_AUTOUNREF`` | ``g_autoptr``| The 
cleanup function needs to be defined  |
++---+--+---+
+| ``VIR_DEFINE_AUTOPTR_FUNC``   | ``G_DEFINE_AUTOPTR_CLEANUP_FUNC``|   
|
++---+--+---+
+| ``VIR_DEFINE_AUTOCLEAN_FUNC`` | ``G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC`` |   
|

[libvirt PATCH 6/6] docs: glib-adoption: add string arrays and objects

[Ján Tomko]

Signed-off-by: Ján Tomko 
---
 docs/glib-adoption.rst | 16 
 1 file changed, 16 insertions(+)

diff --git a/docs/glib-adoption.rst b/docs/glib-adoption.rst
index 6dcd4bc14e..96bea98e87 100644
--- a/docs/glib-adoption.rst
+++ b/docs/glib-adoption.rst
@@ -36,3 +36,19 @@ Array operations
the GLib types, ``GArray``, ``GPtrArray`` or ``GByteArray``.
These all use a struct to track the array memory and size
together and efficiently resize.
+
+String arrays
+   ``virStringList*``, ``virStringListCount*``
+
+   https://developer.gnome.org/glib/stable/glib-String-Utility-Functions.html
+
+   Prefer the NULL-terminated variant instead of storing the count
+   separately. Prefer ``g_str*v`` functions instead of their ``vir*``
+   counterparts. For use with ``g_auto`` GLib provides the ``GStrv`` type.
+
+Objects
+   ``virObject``
+
+   https://developer.gnome.org/gobject/stable/gobject-The-Base-Object-Type.html
+
+   Prefer ``GObject`` instead.
-- 
2.26.2

[PATCH] util: remove compile time tests for IFF_VNET_HDR/IFF_MULTI_QUEUE

[Daniel P . Berrangé]

The former has been present since

  commit f43798c27684ab925adde7d8acc34c78c6e50df8
  Author: Rusty Russell 
  Date:   Thu Jul 3 03:48:02 2008 -0700

tun: Allow GSO using virtio_net_hdr

and the latter since

  commit bbb009941efaece3898910a862f6d23aa55d6ba8
  Author: Jason Wang 
  Date:   Wed Oct 31 19:45:59 2012 +

tuntap: introduce multiqueue flags

these are old enough that they can be assumed present in all Linux
platforms we support. The tap device creation code changed is specific
to Linux, with a separate impl for non-Linux platforms.

Signed-off-by: Daniel P. Berrangé 
---
 src/util/virnetdevtap.c | 11 +--
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
index 77c4d1c52c..1738f48a5f 100644
--- a/src/util/virnetdevtap.c
+++ b/src/util/virnetdevtap.c
@@ -292,20 +292,11 @@ int virNetDevTapCreate(char **ifname,
 
 ifr.ifr_flags = IFF_TAP | IFF_NO_PI;
 /* If tapfdSize is greater than one, request multiqueue */
-if (tapfdSize > 1) {
-# ifdef IFF_MULTI_QUEUE
+if (tapfdSize > 1)
 ifr.ifr_flags |= IFF_MULTI_QUEUE;
-# else
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("Multiqueue devices are not supported on this 
system"));
-goto cleanup;
-# endif
-}
 
-# ifdef IFF_VNET_HDR
 if (flags &  VIR_NETDEV_TAP_CREATE_VNET_HDR)
 ifr.ifr_flags |= IFF_VNET_HDR;
-# endif
 
 if (virStrcpyStatic(ifr.ifr_name, *ifname) < 0) {
 virReportSystemError(ERANGE,
-- 
2.26.2

Re: [PATCH] virDomainNetFindIdx: add support for CCW addresses

[Ján Tomko]

On a Thursday in 2020, Cornelia Huck wrote:

Allow to match with CCW addresses in addition to PCI addresses
(and MAC addresses).

Signed-off-by: Cornelia Huck 
---
src/conf/device_conf.c   | 12 
src/conf/device_conf.h   |  3 +++
src/conf/domain_conf.c   | 23 ++-
src/libvirt_private.syms |  1 +
4 files changed, 38 insertions(+), 1 deletion(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature

Re: [PATCH] qemuxml2argvtest: Append newline to tested error messages

[Ján Tomko]

On a Thursday in 2020, Peter Krempa wrote:

'virTestCompareToFile' automatically fixes newline if it is not present
in the input string but is present in the file. In this case we need to
append the erorr messages with a newline so that
VIR_TEST_REGENERATE_OUTPUT produces files which will pass syntax-check.

Fixes: 9ec77eef2df
Signed-off-by: Peter Krempa 
---
tests/qemuxml2argvtest.c | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature

[PATCH v4 06/11] qemu: Block migration when transient disk option is enabled

[Peter Krempa]

From: Masayoshi Mizuma 

Block migration when transient disk option is enabled to simplify the
handling of the overlay files.

Signed-off-by: Masayoshi Mizuma 
---
 src/qemu/qemu_migration.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 5708334d2f..2f24d56312 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1394,6 +1394,16 @@ qemuMigrationSrcIsAllowed(virQEMUDriverPtr driver,
_("cannot migrate this domain without dbus-vmstate 
support"));
 return false;
 }
+
+for (i = 0; i < vm->def->ndisks; i++) {
+virDomainDiskDefPtr disk = vm->def->disks[i];
+
+if (disk->transient) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("migration with transient disk is not 
supported"));
+return false;
+}
+}
 }

 return true;
-- 
2.26.2

[PATCH v4 00/11] qemu: Add disk support

[Peter Krempa]

This version:
- is rebased on top of current refactors
- has more strict rules in the validation
- adds tests
- fixes bug with disk-unplug

Masayoshi Mizuma (5):
  qemu: Block blockjobs when transient disk option is enabled
  qemu: Block disk hotplug when transient disk option is enabled
  qemu: Block migration when transient disk option is enabled
  qemu: process: Handle transient disks on VM startup
  qemu: validate: Allow  disks

Peter Krempa (6):
  virDomainSnapshotDiskDefFree: Export and register as autoptr func
  qemu: prepare cleanup for  disk overlays
  qemu: snapshot: Introduce helpers for creating overlays on
 disks
  qemu: hotplug: Remove overlay of  disk on disk unplug
  tests: qemuxml2argv: Fix and enable 'disk-transient' case
  TODO: Add news entry

 TODO  |  0
 docs/formatdomain.rst |  5 +-
 src/conf/snapshot_conf.h  |  5 +
 src/conf/snapshot_conf_priv.h |  3 -
 src/qemu/qemu_domain.c|  9 ++
 src/qemu/qemu_domain.h|  4 +
 src/qemu/qemu_hotplug.c   | 15 +++
 src/qemu/qemu_migration.c | 10 ++
 src/qemu/qemu_process.c   | 27 ++
 src/qemu/qemu_snapshot.c  | 91 ++-
 src/qemu/qemu_snapshot.h  |  5 +
 src/qemu/qemu_validate.c  | 56 ++--
 .../disk-transient.x86_64-4.1.0.err   |  1 +
 .../disk-transient.x86_64-latest.args | 42 +
 tests/qemuxml2argvdata/disk-transient.xml |  4 +-
 tests/qemuxml2argvtest.c  |  2 +
 16 files changed, 265 insertions(+), 14 deletions(-)
 create mode 100644 TODO
 create mode 100644 tests/qemuxml2argvdata/disk-transient.x86_64-4.1.0.err
 create mode 100644 tests/qemuxml2argvdata/disk-transient.x86_64-latest.args

-- 
2.26.2

[PATCH v4 10/11] tests: qemuxml2argv: Fix and enable 'disk-transient' case

[Peter Krempa]

We didn't actually use this file. Change the disk type to 'file' so that
it works in qemu and add pre and post-blockdev invocations.

Signed-off-by: Peter Krempa 
---
 .../disk-transient.x86_64-4.1.0.err   |  1 +
 .../disk-transient.x86_64-latest.args | 42 +++
 tests/qemuxml2argvdata/disk-transient.xml |  4 +-
 tests/qemuxml2argvtest.c  |  2 +
 4 files changed, 47 insertions(+), 2 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/disk-transient.x86_64-4.1.0.err
 create mode 100644 tests/qemuxml2argvdata/disk-transient.x86_64-latest.args

diff --git a/tests/qemuxml2argvdata/disk-transient.x86_64-4.1.0.err 
b/tests/qemuxml2argvdata/disk-transient.x86_64-4.1.0.err
new file mode 100644
index 00..341902e144
--- /dev/null
+++ b/tests/qemuxml2argvdata/disk-transient.x86_64-4.1.0.err
@@ -0,0 +1 @@
+unsupported configuration: transient disk not supported by this qemu binary 
(hda)
diff --git a/tests/qemuxml2argvdata/disk-transient.x86_64-latest.args 
b/tests/qemuxml2argvdata/disk-transient.x86_64-latest.args
new file mode 100644
index 00..1b9c64c96a
--- /dev/null
+++ b/tests/qemuxml2argvdata/disk-transient.x86_64-latest.args
@@ -0,0 +1,42 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-QEMUGuest1 \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-i386 \
+-name guest=QEMUGuest1,debug-threads=on \
+-S \
+-object secret,id=masterKey0,format=raw,\
+file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
+-machine pc,accel=tcg,usb=off,dump-guest-core=off \
+-cpu qemu64 \
+-m 214 \
+-overcommit mem-lock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,fd=1729,server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-no-acpi \
+-boot strict=on \
+-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \
+-blockdev '{"driver":"file","filename":"/tmp/QEMUGuest1.img",\
+"node-name":"libvirt-1-storage","cache":{"direct":true,"no-flush":false},\
+"auto-read-only":true,"discard":"unmap"}' \
+-blockdev '{"node-name":"libvirt-1-format","read-only":false,\
+"cache":{"direct":true,"no-flush":false},"driver":"qcow2",\
+"file":"libvirt-1-storage"}' \
+-device 
ide-hd,bus=ide.0,unit=0,drive=libvirt-1-format,id=ide0-0-0,bootindex=1,\
+write-cache=on \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 \
+-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
+resourcecontrol=deny \
+-msg timestamp=on
diff --git a/tests/qemuxml2argvdata/disk-transient.xml 
b/tests/qemuxml2argvdata/disk-transient.xml
index 00ddb6487a..cffb325336 100644
--- a/tests/qemuxml2argvdata/disk-transient.xml
+++ b/tests/qemuxml2argvdata/disk-transient.xml
@@ -14,9 +14,9 @@
   destroy
   
 /usr/bin/qemu-system-i386
-
+
   
-  
+  
   
   
   
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index a3c91fd5de..9da74adb60 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1171,6 +1171,8 @@ mymain(void)
 DO_TEST_CAPS_VER("disk-cache", "2.7.0");
 DO_TEST_CAPS_VER("disk-cache", "2.12.0");
 DO_TEST_CAPS_LATEST("disk-cache");
+DO_TEST_CAPS_ARCH_VER_PARSE_ERROR("disk-transient", "x86_64", "4.1.0");
+DO_TEST_CAPS_LATEST("disk-transient");
 DO_TEST("disk-network-nbd", NONE);
 DO_TEST_CAPS_VER("disk-network-nbd", "2.12.0");
 DO_TEST_CAPS_LATEST("disk-network-nbd");
-- 
2.26.2

[PATCH v4 08/11] qemu: process: Handle transient disks on VM startup

[Peter Krempa]

From: Masayoshi Mizuma 

Add overlays after the VM starts before we start executing guest code.

Signed-off-by: Masayoshi Mizuma 
Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_process.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index ffb3afa9c5..9122069cc9 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -60,6 +60,7 @@
 #include "qemu_firmware.h"
 #include "qemu_backup.h"
 #include "qemu_dbus.h"
+#include "qemu_snapshot.h"

 #include "cpu/cpu.h"
 #include "cpu/cpu_x86.h"
@@ -7077,6 +7078,10 @@ qemuProcessLaunch(virConnectPtr conn,
 qemuProcessAutoDestroyAdd(driver, vm, conn) < 0)
 goto cleanup;

+VIR_DEBUG("Setting up transient disk");
+if (qemuSnapshotCreateDisksTransient(vm, asyncJob) < 0)
+goto cleanup;
+
 ret = 0;

  cleanup:
-- 
2.26.2

[PATCH v4 07/11] qemu: hotplug: Remove overlay of disk on disk unplug

[Peter Krempa]

Remove the overlay if the disk was . Note that even if we'd
forbid unplug of such a disk through the API, the disk can still be
ejected from the guest.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_hotplug.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index ed4d1580fa..11b549b12b 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -4313,6 +4313,15 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver,
 qemuHotplugRemoveManagedPR(driver, vm, QEMU_ASYNC_JOB_NONE) < 0)
 goto cleanup;

+if (disk->transient) {
+VIR_DEBUG("Removing transient overlay '%s' of disk '%s'",
+  disk->src->path, disk->dst);
+if (qemuDomainStorageFileInit(driver, vm, disk->src, NULL) >= 0) {
+virStorageFileUnlink(disk->src);
+virStorageFileDeinit(disk->src);
+}
+}
+
 ret = 0;

  cleanup:
-- 
2.26.2

[PATCH v4 05/11] qemu: Block disk hotplug when transient disk option is enabled

[Peter Krempa]

From: Masayoshi Mizuma 

For now we disable disk hotplug of transient disk as it requires
creating an overlay prior to adding the frontend.

Signed-off-by: Masayoshi Mizuma 
---
 src/qemu/qemu_hotplug.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index fc0866c011..ed4d1580fa 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1031,6 +1031,12 @@ qemuDomainAttachDeviceDiskLiveInternal(virQEMUDriverPtr 
driver,
 return -1;
 }

+if (disk->transient) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+   _("transient disk hotplug isn't supported"));
+return -1;
+}
+
 if (virDomainDiskTranslateSourcePool(disk) < 0)
 goto cleanup;

-- 
2.26.2

[PATCH v4 09/11] qemu: validate: Allow disks

[Peter Krempa]

From: Masayoshi Mizuma 

Extract the validation of transient disk option. We support transient
disks in qemu under the following conditions:

 - -blockdev is used
 - the disk source is a local file
 - the disk type is 'disk'
 - the disk is not readonly

Signed-off-by: Masayoshi Mizuma 
Signed-off-by: Peter Krempa 
---
 docs/formatdomain.rst|  5 ++--
 src/qemu/qemu_validate.c | 56 +++-
 2 files changed, 53 insertions(+), 8 deletions(-)

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 888db5ea29..cc1467c0e6 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -2974,8 +2974,9 @@ paravirtualized driver is specified via the ``disk`` 
element.
 ``transient``
If present, this indicates that changes to the device contents should be
reverted automatically when the guest exits. With some hypervisors, marking 
a
-   disk transient prevents the domain from participating in migration or
-   snapshots. Only suppported in vmx hypervisor. :since:`Since 0.9.5`
+   disk transient prevents the domain from participating in migration,
+   snapshots, or blockjobs. Only suppported in vmx hypervisor
+   (:since:`Since 0.9.5`) and ``qemu`` hypervisor (:since:`Since 6.9.0`).
 ``serial``
If present, this specify serial number of virtual hard drive. For example, 
it
may look like ``WD-WMAP9A966149``. Not supported for
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 3ed4039cdf..3196814aca 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -2186,12 +2186,6 @@ qemuValidateDomainDeviceDefDiskFrontend(const 
virDomainDiskDef *disk,
 }
 }

-if (disk->transient) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("transient disks not supported yet"));
-return -1;
-}
-
 if (disk->iomode == VIR_DOMAIN_DISK_IO_NATIVE &&
 disk->cachemode != VIR_DOMAIN_DISK_CACHE_DIRECTSYNC &&
 disk->cachemode != VIR_DOMAIN_DISK_CACHE_DISABLE) {
@@ -2340,6 +2334,53 @@ qemuValidateDomainDeviceDefDiskBlkdeviotune(const 
virDomainDiskDef *disk,
 }


+static int
+qemuValidateDomainDeviceDefDiskTransient(const virDomainDiskDef *disk,
+ virQEMUCapsPtr qemuCaps)
+
+{
+virStorageType actualType = virStorageSourceGetActualType(disk->src);
+
+if (!disk->transient)
+return 0;
+
+if (virStorageSourceIsEmpty(disk->src)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("transient disk '%s' must not be empty"), disk->dst);
+return -1;
+}
+
+if (disk->src->readonly) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("transient disk '%s' must not be read-only"), 
disk->dst);
+return -1;
+}
+
+if (actualType != VIR_STORAGE_TYPE_FILE) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("transient disk supported only with 'file' type 
(%s)"),
+   disk->dst);
+return -1;
+}
+
+if (disk->device != VIR_DOMAIN_DISK_DEVICE_DISK) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("transient disk supported only with 'disk' device 
(%s)"),
+   disk->dst);
+return -1;
+}
+
+if (qemuCaps && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("transient disk not supported by this qemu binary 
(%s)"),
+   disk->dst);
+return -1;
+}
+
+return 0;
+}
+
+
 int
 qemuValidateDomainDeviceDefDisk(const virDomainDiskDef *disk,
 const virDomainDef *def,
@@ -2357,6 +2398,9 @@ qemuValidateDomainDeviceDefDisk(const virDomainDiskDef 
*disk,
 if (qemuValidateDomainDeviceDefDiskBlkdeviotune(disk, def, qemuCaps) < 0)
 return -1;

+if (qemuValidateDomainDeviceDefDiskTransient(disk, qemuCaps) < 0)
+return -1;
+
 if (disk->src->shared && !disk->src->readonly &&
 !qemuBlockStorageSourceSupportsConcurrentAccess(disk->src)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-- 
2.26.2

[PATCH v4 03/11] qemu: snapshot: Introduce helpers for creating overlays on disks

[Peter Krempa]

To implement  disks we'll need to install an overlay on top
of the original disk image which will be discarded after the VM is
turned off. This was initially implemented by qemu but libvirt never
picked up this option. With blockdev the qemu feature became unsupported
so we need to do this via the snapshot code anyways.

The helpers introduced in this patch prepare a fake snapshot disk
definition for a disk which is configured as  and use it to
create a snapshot (without actually modifying metada or persistent def).

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_snapshot.c | 91 +++-
 src/qemu/qemu_snapshot.h |  5 +++
 2 files changed, 95 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index ca051071aa..d10e7b6b3a 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -986,6 +986,7 @@ qemuSnapshotDiskPrepareOne(virDomainObjPtr vm,
qemuSnapshotDiskDataPtr dd,
virHashTablePtr blockNamedNodeData,
bool reuse,
+   bool updateConfig,
qemuDomainAsyncJob asyncJob,
virJSONValuePtr actions)
 {
@@ -1008,7 +1009,8 @@ qemuSnapshotDiskPrepareOne(virDomainObjPtr vm,
 return -1;

 /* modify disk in persistent definition only when the source is the same */
-if (vm->newDef &&
+if (updateConfig &&
+vm->newDef &&
 (persistdisk = virDomainDiskByTarget(vm->newDef, dd->disk->dst)) &&
 virStorageSourceIsSameLocation(dd->disk->src, persistdisk->src)) {

@@ -1116,6 +1118,54 @@ qemuSnapshotDiskPrepareActiveExternal(virDomainObjPtr vm,
snapctxt->dd + snapctxt->ndd++,
blockNamedNodeData,
reuse,
+   true,
+   asyncJob,
+   snapctxt->actions) < 0)
+return NULL;
+}
+
+return g_steal_pointer();
+}
+
+
+static qemuSnapshotDiskContextPtr
+qemuSnapshotDiskPrepareDisksTransient(virDomainObjPtr vm,
+  virQEMUDriverConfigPtr cfg,
+  virHashTablePtr blockNamedNodeData,
+  qemuDomainAsyncJob asyncJob)
+{
+g_autoptr(qemuSnapshotDiskContext) snapctxt = NULL;
+size_t i;
+
+snapctxt = qemuSnapshotDiskContextNew(vm->def->ndisks, vm, asyncJob);
+
+for (i = 0; i < vm->def->ndisks; i++) {
+virDomainDiskDefPtr domdisk = vm->def->disks[i];
+g_autoptr(virDomainSnapshotDiskDef) snapdisk = 
g_new0(virDomainSnapshotDiskDef, 1);
+
+if (!domdisk->transient)
+continue;
+
+/* validation code makes sure that we do this only for local disks
+ * with a file source */
+snapdisk->name = g_strdup(domdisk->dst);
+snapdisk->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
+snapdisk->src = virStorageSourceNew();
+snapdisk->src->type = VIR_STORAGE_TYPE_FILE;
+snapdisk->src->format = VIR_STORAGE_FILE_QCOW2;
+snapdisk->src->path = g_strdup_printf("%s.TRANSIENT", 
domdisk->src->path);
+if (virFileExists(snapdisk->src->path)) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+   _("Overlay file '%s' for transient disk '%s' 
already exists"),
+   snapdisk->src->path, domdisk->dst);
+return NULL;
+}
+
+if (qemuSnapshotDiskPrepareOne(vm, cfg, domdisk, snapdisk,
+   snapctxt->dd + snapctxt->ndd++,
+   blockNamedNodeData,
+   false,
+   false,
asyncJob,
snapctxt->actions) < 0)
 return NULL;
@@ -1253,6 +1303,45 @@ qemuSnapshotCreateActiveExternalDisks(virDomainObjPtr vm,
 }


+/**
+ * qemuSnapshotCreateDisksTransient:
+ * @vm: domain object
+ * @asyncJob: qemu async job type
+ *
+ * Creates overlays on top of disks which are configured as . Note
+ * that the validation code ensures that  disks have appropriate
+ * configuration.
+ */
+int
+qemuSnapshotCreateDisksTransient(virDomainObjPtr vm,
+ qemuDomainAsyncJob asyncJob)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+virQEMUDriverPtr driver = priv->driver;
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
+g_autoptr(qemuSnapshotDiskContext) snapctxt = NULL;
+g_autoptr(virHashTable) blockNamedNodeData = NULL;
+
+if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) {
+if (!(blockNamedNodeData = qemuBlockGetNamedNodeData(vm, asyncJob)))
+

[PATCH v4 02/11] qemu: prepare cleanup for disk overlays

[Peter Krempa]

Later patches will implement support for  disks in libvirt
by installing an overlay on top of the configured image. This will
require cleanup after the VM will be stopped so that the state is
correctly discarded.

Since the overlay will be installed only during the startup phase of the
VM we need to ensure that qemuProcessStop doesn't delete the original
file on some previous failure. This is solved by adding
'inhibitDiskTransientDelete' VM private data member which is set prior
to any startup step and will be cleared once transient disk overlays are
established.

Based on that we can then delete the overlays for any  disk.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_domain.c  |  2 ++
 src/qemu/qemu_domain.h  |  4 
 src/qemu/qemu_process.c | 22 ++
 3 files changed, 28 insertions(+)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 279de2997d..dc5949edfa 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1792,6 +1792,8 @@ qemuDomainObjPrivateDataClear(qemuDomainObjPrivatePtr 
priv)
 priv->dbusVMStateIds = NULL;

 priv->dbusVMState = false;
+
+priv->inhibitDiskTransientDelete = false;
 }


diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index c7c3c5c073..ec776ced72 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -263,6 +263,10 @@ struct _qemuDomainObjPrivate {
 char **dbusVMStateIds;
 /* true if -object dbus-vmstate was added */
 bool dbusVMState;
+
+/* prevent deletion of  disk overlay files between startup and
+ * succesful setup of the overlays */
+bool inhibitDiskTransientDelete;
 };

 #define QEMU_DOMAIN_PRIVATE(vm) \
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index f21b8f1585..ffb3afa9c5 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5616,6 +5616,9 @@ qemuProcessInit(virQEMUDriverPtr driver,
 if (virDomainObjSetDefTransient(driver->xmlopt, vm, priv->qemuCaps) < 0)
 goto cleanup;

+/* don't clean up files for  disks until we set them up */
+priv->inhibitDiskTransientDelete = true;
+
 if (flags & VIR_QEMU_PROCESS_START_PRETEND) {
 if (qemuDomainSetPrivatePaths(driver, vm) < 0) {
 virDomainObjRemoveTransientDef(vm);
@@ -7710,6 +7713,18 @@ void qemuProcessStop(virQEMUDriverPtr driver,
 }

 qemuBlockRemoveImageMetadata(driver, vm, disk->dst, disk->src);
+
+/* for now transient disks are forbidden with migration so they
+ * can be handled here */
+if (disk->transient &&
+!priv->inhibitDiskTransientDelete) {
+VIR_DEBUG("Removing transient overlay '%s' of disk '%s'",
+  disk->src->path, disk->dst);
+if (qemuDomainStorageFileInit(driver, vm, disk->src, NULL) >= 
0) {
+virStorageFileUnlink(disk->src);
+virStorageFileDeinit(disk->src);
+}
+}
 }
 }

@@ -8125,6 +8140,10 @@ qemuProcessReconnect(void *opaque)
 cfg = virQEMUDriverGetConfig(driver);
 priv = obj->privateData;

+/* expect that libvirt might have crashed during VM start, so prevent
+ * cleanup of transient disks */
+priv->inhibitDiskTransientDelete = true;
+
 if (qemuDomainObjBeginJob(driver, obj, QEMU_JOB_MODIFY) < 0)
 goto error;
 jobStarted = true;
@@ -8228,6 +8247,9 @@ qemuProcessReconnect(void *opaque)
 goto error;
 }

+/* vm startup complete, we can remove transient disks if required */
+priv->inhibitDiskTransientDelete = false;
+
 /* In case the domain shutdown while we were not running,
  * we need to finish the shutdown process. And we need to do it after
  * we have virQEMUCaps filled in.
-- 
2.26.2

[PATCH v4 01/11] virDomainSnapshotDiskDefFree: Export and register as autoptr func

[Peter Krempa]

Allow using the function for creating temporary snapshot disk
definitions for creating  disk overlays.

Signed-off-by: Peter Krempa 
---
 src/conf/snapshot_conf.h  | 5 +
 src/conf/snapshot_conf_priv.h | 3 ---
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index fbc9b17c54..0f3987fc80 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -70,6 +70,11 @@ struct _virDomainSnapshotDiskDef {
 virStorageSourcePtr src;
 };

+void
+virDomainSnapshotDiskDefFree(virDomainSnapshotDiskDefPtr disk);
+
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainSnapshotDiskDef, 
virDomainSnapshotDiskDefFree);
+
 /* Stores the complete snapshot metadata */
 struct _virDomainSnapshotDef {
 virDomainMomentDef parent;
diff --git a/src/conf/snapshot_conf_priv.h b/src/conf/snapshot_conf_priv.h
index b721a57c4b..369a023881 100644
--- a/src/conf/snapshot_conf_priv.h
+++ b/src/conf/snapshot_conf_priv.h
@@ -30,6 +30,3 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
  virDomainSnapshotDiskDefPtr def,
  unsigned int flags,
  virDomainXMLOptionPtr xmlopt);
-
-void
-virDomainSnapshotDiskDefFree(virDomainSnapshotDiskDefPtr disk);
-- 
2.26.2

[PATCH v4 11/11] TODO: Add news entry

[Peter Krempa]

---
 TODO | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 TODO

diff --git a/TODO b/TODO
new file mode 100644
index 00..e69de29bb2
-- 
2.26.2

[PATCH v4 04/11] qemu: Block blockjobs when transient disk option is enabled

[Peter Krempa]

From: Masayoshi Mizuma 

For now we disallow blockjobs with transient disks to avoid dealing with
obsoleted overlays.

Signed-off-by: Masayoshi Mizuma 
---
 src/qemu/qemu_domain.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index dc5949edfa..0331fd55e0 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -10859,6 +10859,13 @@ qemuDomainDiskBlockJobIsSupported(virDomainObjPtr vm,
 return false;
 }

+if (disk->transient) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+   _("block jobs are not supported on transient disk 
'%s'"),
+   disk->dst);
+return false;
+}
+
 return true;
 }

-- 
2.26.2

Re: [PATCH] cpu_map: Add EPYC-Rome model

[Jiri Denemark]

On Thu, Sep 24, 2020 at 12:25:23 +0200, Markus Schade wrote:
> Hi everyone,
> 
> I'd like to add support for the EPYC-Rome CPU model from qemu.
> 
> The following patch adds the model to cpu_map. While this allows to use
> the model, I am aware that some tests are required as well.
> 
> However I am not certain what exactly is needed. I am happy to provide
> CPU flags or any required information from an EPYC 7502 system

Great. First, make sure you have cpuid and current qemu installed. Then
go to tests/cputestdata and run ./cpu-gather.sh script there. It should
print CPUID dump followed by a lot of JSON from QEMU. If that works, you
can pipe it to another script:

./cpu-gather.sh | ./cpu-parse.sh

This will create new data files for cputest. They will be called
x86_64-cpuid-*{.xml,.json,-disabled.xml,-enabled.xml}. Got back to the
top source directory, edit tests/cputest.c and add a new test case:

DO_TEST_CPUID(VIR_ARCH_X86_64, , JSON_MODELS);

where  is the part between "x86_64-cpuid-" and ".json" from the
new data file. If you rebuild libvirt and run tests now, cputest should
fail complaining about missing files. Run

VIR_TEST_REGENERATE_OUTPUT=1 tests/cputest

from the build directory to create those files. Running "meson test"
should succeed now. If it does, you can just add all the new files and
tests/cputest changes to git and send it for review.

Jirka

[PATCH] qemuxml2argvtest: Append newline to tested error messages

[Peter Krempa]

'virTestCompareToFile' automatically fixes newline if it is not present
in the input string but is present in the file. In this case we need to
append the erorr messages with a newline so that
VIR_TEST_REGENERATE_OUTPUT produces files which will pass syntax-check.

Fixes: 9ec77eef2df
Signed-off-by: Peter Krempa 
---
 tests/qemuxml2argvtest.c | 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index a3c91fd5de..2b97eb80a4 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -621,9 +621,11 @@ testCompareXMLToArgv(const void *data)
 VIR_TEST_DEBUG("no error was reported for expected parse error");
 goto cleanup;
 }
-if (flags & FLAG_EXPECT_PARSE_ERROR &&
-virTestCompareToFile(err->message, info->errfile) >= 0)
-goto ok;
+if (flags & FLAG_EXPECT_PARSE_ERROR) {
+g_autofree char *tmperr = g_strdup_printf("%s\n", 
NULLSTR(err->message));
+if (virTestCompareToFile(tmperr, info->errfile) >= 0)
+goto ok;
+}
 goto cleanup;
 }
 if (flags & FLAG_EXPECT_PARSE_ERROR) {
@@ -663,9 +665,11 @@ testCompareXMLToArgv(const void *data)
 VIR_TEST_DEBUG("no error was reported for expected failure");
 goto cleanup;
 }
-if (flags & FLAG_EXPECT_FAILURE &&
-virTestCompareToFile(err->message, info->errfile) >= 0)
-goto ok;
+if (flags & FLAG_EXPECT_FAILURE) {
+g_autofree char *tmperr = g_strdup_printf("%s\n", 
NULLSTR(err->message));
+if (virTestCompareToFile(tmperr, info->errfile) >= 0)
+goto ok;
+}
 goto cleanup;
 }
 if (flags & FLAG_EXPECT_FAILURE) {
-- 
2.26.2

[PATCH] virDomainNetFindIdx: add support for CCW addresses

[Cornelia Huck]

Allow to match with CCW addresses in addition to PCI addresses
(and MAC addresses).

Signed-off-by: Cornelia Huck 
---
 src/conf/device_conf.c   | 12 
 src/conf/device_conf.h   |  3 +++
 src/conf/domain_conf.c   | 23 ++-
 src/libvirt_private.syms |  1 +
 4 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
index 9398191dfd84..87bf32bbc685 100644
--- a/src/conf/device_conf.c
+++ b/src/conf/device_conf.c
@@ -341,6 +341,18 @@ virDomainDeviceCCWAddressParseXML(xmlNodePtr node,
 return 0;
 }
 
+bool
+virDomainDeviceCCWAddressEqual(virDomainDeviceCCWAddressPtr addr1,
+   virDomainDeviceCCWAddressPtr addr2)
+{
+if (addr1->cssid == addr2->cssid &&
+addr1->ssid == addr2->ssid &&
+addr1->devno == addr2->devno) {
+return true;
+}
+return false;
+}
+
 int
 virDomainDeviceDriveAddressParseXML(xmlNodePtr node,
 virDomainDeviceDriveAddressPtr addr)
diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
index d7395f220174..a51bdf10ee6e 100644
--- a/src/conf/device_conf.h
+++ b/src/conf/device_conf.h
@@ -207,6 +207,9 @@ void virPCIDeviceAddressFormat(virBufferPtr buf,
 bool virDomainDeviceCCWAddressIsValid(virDomainDeviceCCWAddressPtr addr);
 int virDomainDeviceCCWAddressParseXML(xmlNodePtr node,
   virDomainDeviceCCWAddressPtr addr);
+bool virDomainDeviceCCWAddressEqual(virDomainDeviceCCWAddressPtr addr1,
+virDomainDeviceCCWAddressPtr addr2);
+#define VIR_CCW_DEVICE_ADDRESS_FMT "%x.%x.%04x"
 
 int virDomainDeviceDriveAddressParseXML(xmlNodePtr node,
 virDomainDeviceDriveAddressPtr addr);
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8d30557bdcbe..a91dbd4aa95b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17816,6 +17816,8 @@ virDomainNetFindIdx(virDomainDefPtr def, 
virDomainNetDefPtr net)
 bool MACAddrSpecified = !net->mac_generated;
 bool PCIAddrSpecified = virDomainDeviceAddressIsValid(>info,
   
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI);
+bool CCWAddrSpecified = virDomainDeviceAddressIsValid(>info,
+  
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW);
 
 for (i = 0; i < def->nnets; i++) {
 if (MACAddrSpecified &&
@@ -17827,9 +17829,14 @@ virDomainNetFindIdx(virDomainDefPtr def, 
virDomainNetDefPtr net)
   >info.addr.pci))
 continue;
 
+if (CCWAddrSpecified &&
+!virDomainDeviceCCWAddressEqual(>nets[i]->info.addr.ccw,
+>info.addr.ccw))
+continue;
+
 if (matchidx >= 0) {
 /* there were multiple matches on mac address, and no
- * qualifying guest-side PCI address was given, so we must
+ * qualifying guest-side PCI/CCW address was given, so we must
  * fail (NB: a USB address isn't adequate, since it may
  * specify only vendor and product ID, and there may be
  * multiples of those.
@@ -17859,6 +17866,14 @@ virDomainNetFindIdx(virDomainDefPtr def, 
virDomainNetDefPtr net)
net->info.addr.pci.bus,
net->info.addr.pci.slot,
net->info.addr.pci.function);
+} else if (MACAddrSpecified && CCWAddrSpecified) {
+virReportError(VIR_ERR_DEVICE_MISSING,
+   _("no device matching MAC address %s found on "
+ VIR_CCW_DEVICE_ADDRESS_FMT),
+   virMacAddrFormat(>mac, mac),
+   net->info.addr.ccw.cssid,
+   net->info.addr.ccw.ssid,
+   net->info.addr.ccw.devno);
 } else if (PCIAddrSpecified) {
 virReportError(VIR_ERR_DEVICE_MISSING,
_("no device found on " VIR_PCI_DEVICE_ADDRESS_FMT),
@@ -17866,6 +17881,12 @@ virDomainNetFindIdx(virDomainDefPtr def, 
virDomainNetDefPtr net)
net->info.addr.pci.bus,
net->info.addr.pci.slot,
net->info.addr.pci.function);
+} else if (CCWAddrSpecified) {
+virReportError(VIR_ERR_DEVICE_MISSING,
+   _("no device found on " VIR_CCW_DEVICE_ADDRESS_FMT),
+   net->info.addr.ccw.cssid,
+   net->info.addr.ccw.ssid,
+   net->info.addr.ccw.devno);
 } else if (MACAddrSpecified) {
 virReportError(VIR_ERR_DEVICE_MISSING,
_("no device matching MAC address %s found"),
diff --git a/src/libvirt_private.syms 

Re: [PATCH] util: stop probing for IFF_VNET_HDR

[Daniel P . Berrangé]

On Thu, Sep 24, 2020 at 01:05:48PM +0200, Ján Tomko wrote:
> On a Thursday in 2020, Daniel P. Berrangé wrote:
> > This flag was added by Linux with:
> > 
> >  commit f43798c27684ab925adde7d8acc34c78c6e50df8
> >  Author: Rusty Russell 
> >  Date:   Thu Jul 3 03:48:02 2008 -0700
> > 
> >tun: Allow GSO using virtio_net_hdr
> > 
> > so we can assume all Linux distros we support have this flag available
> > and thus the compile time check is sufficient.
> > 
> > Signed-off-by: Daniel P. Berrangé 
> > ---
> > src/util/virnetdevtap.c | 63 +
> > 1 file changed, 1 insertion(+), 62 deletions(-)
> > 
> > diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
> > index cbce5c71b7..77c4d1c52c 100644
> > --- a/src/util/virnetdevtap.c
> > +++ b/src/util/virnetdevtap.c
> > @@ -183,66 +183,6 @@ virNetDevTapGetRealDeviceName(char *ifname 
> > G_GNUC_UNUSED)
> > }
> > 
> > 
> > -/**
> > - * virNetDevProbeVnetHdr:
> > - * @tapfd: a tun/tap file descriptor
> > - *
> > - * Check whether it is safe to enable the IFF_VNET_HDR flag on the
> > - * tap interface.
> > - *
> > - * Setting IFF_VNET_HDR enables QEMU's virtio_net driver to allow
> > - * guests to pass larger (GSO) packets, with partial checksums, to
> > - * the host. This greatly increases the achievable throughput.
> > - *
> > - * It is only useful to enable this when we're setting up a virtio
> > - * interface. And it is only *safe* to enable it when we know for
> > - * sure that a) qemu has support for IFF_VNET_HDR and b) the running
> > - * kernel implements the TUNGETIFF ioctl(), which qemu needs to query
> > - * the supplied tapfd.
> > - *
> > - * Returns 1 if VnetHdr is supported, 0 if not supported
> > - */
> > -#ifdef IFF_VNET_HDR
> > -static int
> > -virNetDevProbeVnetHdr(int tapfd)
> > -{
> > -# if defined(IFF_VNET_HDR) && defined(TUNGETFEATURES) && defined(TUNGETIFF)
> > -unsigned int features;
> > -struct ifreq dummy;
> > -
> > -if (ioctl(tapfd, TUNGETFEATURES, ) != 0) {
> > -VIR_INFO("Not enabling IFF_VNET_HDR; "
> > - "TUNGETFEATURES ioctl() not implemented");
> > -return 0;
> > -}
> > -
> > -if (!(features & IFF_VNET_HDR)) {
> > -VIR_INFO("Not enabling IFF_VNET_HDR; "
> > - "TUNGETFEATURES ioctl() reports no IFF_VNET_HDR");
> > -return 0;
> > -}
> > -
> > -/* The kernel will always return -1 at this point.
> > - * If TUNGETIFF is not implemented then errno == EBADFD.
> > - */
> > -if (ioctl(tapfd, TUNGETIFF, ) != -1 || errno != EBADFD) {
> > -VIR_INFO("Not enabling IFF_VNET_HDR; "
> > - "TUNGETIFF ioctl() not implemented");
> > -return 0;
> > -}
> > -
> > -VIR_INFO("Enabling IFF_VNET_HDR");
> > -
> > -return 1;
> > -# else
> > -(void) tapfd;
> > -VIR_INFO("Not enabling IFF_VNET_HDR; disabled at build time");
> > -return 0;
> > -# endif
> > -}
> > -#endif
> > -
> > -
> > #ifdef TUNSETIFF
> > /**
> >  * virNetDevTapGenerateName:
> > @@ -363,8 +303,7 @@ int virNetDevTapCreate(char **ifname,
> > }
> > 
> > # ifdef IFF_VNET_HDR
> 
> The TUNSETIFF guard above (which was introduced in Linux eons ago)
> seems to be enough according to our CI:
> https://gitlab.com/janotomko/libvirt/-/pipelines/193942161
> 
> It builds even with the IFF_MULTI_QUEUE guard removed.
> commit bbb009941efaece3898910a862f6d23aa55d6ba8
> CommitDate: 2012-11-01 11:14:08 -0400
> tuntap: introduce multiqueue flags
> 
> But #ifdef removal is out of scope of this patch.

Oh right, I was thinking this code was used on BSD/MacOS, but
I forget there was a competely seperate impl. I'll do a followup
for the ifdefs.

> 
> Reviewed-by: Ján Tomko 
> 
> Jano
> 
> > -if ((flags &  VIR_NETDEV_TAP_CREATE_VNET_HDR) &&
> > -virNetDevProbeVnetHdr(fd))
> > +if (flags &  VIR_NETDEV_TAP_CREATE_VNET_HDR)
> > ifr.ifr_flags |= IFF_VNET_HDR;
> > # endif
> > 
> > -- 
> > 2.26.2
> > 



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 0/2] qemuSnapshotDiskContext(Cleanup|New): fix few bugs

[Ján Tomko]

On a Thursday in 2020, Peter Krempa wrote:

Oops.



How did this get past review? O:-)


Peter Krempa (2):
 qemuSnapshotDiskContextCleanup: Don't leak snapctxt
 qemuSnapshotDiskContextNew: Don't set 'ndd'

src/qemu/qemu_snapshot.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature

Re: [PATCH] util: stop probing for IFF_VNET_HDR

[Ján Tomko]

On a Thursday in 2020, Daniel P. Berrangé wrote:

This flag was added by Linux with:

 commit f43798c27684ab925adde7d8acc34c78c6e50df8
 Author: Rusty Russell 
 Date:   Thu Jul 3 03:48:02 2008 -0700

   tun: Allow GSO using virtio_net_hdr

so we can assume all Linux distros we support have this flag available
and thus the compile time check is sufficient.

Signed-off-by: Daniel P. Berrangé 
---
src/util/virnetdevtap.c | 63 +
1 file changed, 1 insertion(+), 62 deletions(-)

diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
index cbce5c71b7..77c4d1c52c 100644
--- a/src/util/virnetdevtap.c
+++ b/src/util/virnetdevtap.c
@@ -183,66 +183,6 @@ virNetDevTapGetRealDeviceName(char *ifname G_GNUC_UNUSED)
}


-/**
- * virNetDevProbeVnetHdr:
- * @tapfd: a tun/tap file descriptor
- *
- * Check whether it is safe to enable the IFF_VNET_HDR flag on the
- * tap interface.
- *
- * Setting IFF_VNET_HDR enables QEMU's virtio_net driver to allow
- * guests to pass larger (GSO) packets, with partial checksums, to
- * the host. This greatly increases the achievable throughput.
- *
- * It is only useful to enable this when we're setting up a virtio
- * interface. And it is only *safe* to enable it when we know for
- * sure that a) qemu has support for IFF_VNET_HDR and b) the running
- * kernel implements the TUNGETIFF ioctl(), which qemu needs to query
- * the supplied tapfd.
- *
- * Returns 1 if VnetHdr is supported, 0 if not supported
- */
-#ifdef IFF_VNET_HDR
-static int
-virNetDevProbeVnetHdr(int tapfd)
-{
-# if defined(IFF_VNET_HDR) && defined(TUNGETFEATURES) && defined(TUNGETIFF)
-unsigned int features;
-struct ifreq dummy;
-
-if (ioctl(tapfd, TUNGETFEATURES, ) != 0) {
-VIR_INFO("Not enabling IFF_VNET_HDR; "
- "TUNGETFEATURES ioctl() not implemented");
-return 0;
-}
-
-if (!(features & IFF_VNET_HDR)) {
-VIR_INFO("Not enabling IFF_VNET_HDR; "
- "TUNGETFEATURES ioctl() reports no IFF_VNET_HDR");
-return 0;
-}
-
-/* The kernel will always return -1 at this point.
- * If TUNGETIFF is not implemented then errno == EBADFD.
- */
-if (ioctl(tapfd, TUNGETIFF, ) != -1 || errno != EBADFD) {
-VIR_INFO("Not enabling IFF_VNET_HDR; "
- "TUNGETIFF ioctl() not implemented");
-return 0;
-}
-
-VIR_INFO("Enabling IFF_VNET_HDR");
-
-return 1;
-# else
-(void) tapfd;
-VIR_INFO("Not enabling IFF_VNET_HDR; disabled at build time");
-return 0;
-# endif
-}
-#endif
-
-
#ifdef TUNSETIFF
/**
 * virNetDevTapGenerateName:
@@ -363,8 +303,7 @@ int virNetDevTapCreate(char **ifname,
}

# ifdef IFF_VNET_HDR


The TUNSETIFF guard above (which was introduced in Linux eons ago)
seems to be enough according to our CI:
https://gitlab.com/janotomko/libvirt/-/pipelines/193942161

It builds even with the IFF_MULTI_QUEUE guard removed.
commit bbb009941efaece3898910a862f6d23aa55d6ba8
CommitDate: 2012-11-01 11:14:08 -0400
tuntap: introduce multiqueue flags

But #ifdef removal is out of scope of this patch.

Reviewed-by: Ján Tomko 

Jano


-if ((flags &  VIR_NETDEV_TAP_CREATE_VNET_HDR) &&
-virNetDevProbeVnetHdr(fd))
+if (flags &  VIR_NETDEV_TAP_CREATE_VNET_HDR)
ifr.ifr_flags |= IFF_VNET_HDR;
# endif

--
2.26.2



signature.asc
Description: PGP signature

[PATCH 1/2] qemuSnapshotDiskContextCleanup: Don't leak snapctxt

[Peter Krempa]

The container itself needs to be freed too.

Fixes: 8c2ecdf131c
Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_snapshot.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 0a97d3643f..63401db2a9 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -883,6 +883,8 @@ qemuSnapshotDiskContextCleanup(qemuSnapshotDiskContextPtr 
snapctxt)
 virJSONValueFree(snapctxt->actions);

 qemuSnapshotDiskCleanup(snapctxt->dd, snapctxt->ndd, snapctxt->vm, 
snapctxt->asyncJob);
+
+g_free(snapctxt);
 }

 G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuSnapshotDiskContext, 
qemuSnapshotDiskContextCleanup);
-- 
2.26.2

[PATCH 2/2] qemuSnapshotDiskContextNew: Don't set 'ndd'

[Peter Krempa]

'ndd' tracks the actual number of snapshot disks filled into the
structure and is incremented by the functions filling the context, thus
it must not be set when initializing the context.

Fixes: 8c2ecdf131c
Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_snapshot.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 63401db2a9..ca051071aa 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -865,7 +865,6 @@ qemuSnapshotDiskContextNew(size_t ndisks,
 qemuSnapshotDiskContextPtr ret = g_new0(qemuSnapshotDiskContext, 1);

 ret->dd = g_new0(qemuSnapshotDiskData, ndisks);
-ret->ndd = ndisks;
 ret->actions = virJSONValueNewArray();
 ret->vm = vm;
 ret->asyncJob = asyncJob;
-- 
2.26.2

[PATCH 0/2] qemuSnapshotDiskContext(Cleanup|New): fix few bugs

[Peter Krempa]

Oops.

Peter Krempa (2):
  qemuSnapshotDiskContextCleanup: Don't leak snapctxt
  qemuSnapshotDiskContextNew: Don't set 'ndd'

 src/qemu/qemu_snapshot.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

-- 
2.26.2

[PATCH] cpu_map: Add EPYC-Rome model

[Markus Schade]

Hi everyone,

I'd like to add support for the EPYC-Rome CPU model from qemu.

The following patch adds the model to cpu_map. While this allows to use
the model, I am aware that some tests are required as well.

However I am not certain what exactly is needed. I am happy to provide
CPU flags or any required information from an EPYC 7502 system


Signed-off-by: Markus Schade 

diff --git a/src/cpu_map/index.xml b/src/cpu_map/index.xml
index 1486a29c65..fec01f324c 100644
--- a/src/cpu_map/index.xml
+++ b/src/cpu_map/index.xml
@@ -66,6 +66,7 @@
 
 
 
+

 
 
diff --git a/src/cpu_map/meson.build b/src/cpu_map/meson.build
index 19daa7157b..b86612b6e0 100644
--- a/src/cpu_map/meson.build
+++ b/src/cpu_map/meson.build
@@ -32,6 +32,7 @@ cpumap_data = [
   'x86_Dhyana.xml',
   'x86_EPYC-IBPB.xml',
   'x86_EPYC.xml',
+  'x86_EPYC-Rome.xml',
   'x86_features.xml',
   'x86_Haswell-IBRS.xml',
   'x86_Haswell-noTSX-IBRS.xml',
diff --git a/src/cpu_map/x86_EPYC-Rome.xml b/src/cpu_map/x86_EPYC-Rome.xml
new file mode 100644
index 00..41d4123917
--- /dev/null
+++ b/src/cpu_map/x86_EPYC-Rome.xml
@@ -0,0 +1,81 @@
+
+  
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+  
+




signature.asc
Description: OpenPGP digital signature

Re: [PATCH] RFC: fix error message in virMigrate3 if connection is broken

[Nikolay Shirokovskiy]

On 23.09.2020 22:50, Jiri Denemark wrote:
> On Mon, Sep 21, 2020 at 10:51:16 +0300, Nikolay Shirokovskiy wrote:
>> Currently virDomainMigrate3 reports "this function is not supported by the
>> connection driver: virDomainMigrate3" if connection to destination for 
>> example
>> is broken. This is a bit misleading. 
>>
>> This is a result of we treat errors as unsupported feature in
>> VIR_DRV_SUPPORTS_FEATURE macro. So let's handle errors instead. In order to
>> keep logic clean as before there are new helper functions
>> virDrvSupportsFeature/virDrvNSupportsFeature. They allow to keep if/else if
>> structure of feature tests.
>>
>> I guess all the other occurences of VIR_DRV_SUPPORTS_FEATURE need to be
>> replaced with one these new helper functions so that we detect error early 
>> and
>> not have issues similar to virDomainMigrate3. I'm going to fix the other 
>> places if this RFC is approved.
> 
> To be honest, I think this is quite ugly.
> 
>>
>> ---
>>  src/datatypes.c  | 70 +++
>>  src/datatypes.h  |  7 +
>>  src/libvirt-domain.c | 76 
>> +++-
>>  3 files changed, 123 insertions(+), 30 deletions(-)
>>
>> diff --git a/src/datatypes.c b/src/datatypes.c
>> index 1db38c5..3fb71f4 100644
>> --- a/src/datatypes.c
>> +++ b/src/datatypes.c
>> @@ -1257,3 +1257,73 @@ virAdmClientDispose(void *obj)
>>  
>>  virObjectUnref(clt->srv);
>>  }
>> +
>> +
>> +/*
>> + * Tests if feature is supported by connection. If testing failed
>> + * due to error then function returns true as well and set @err flag
>> + * to true. Thus positive result should be checked for an error.
>> + * If @err is already set to true then no checking is done and
>> + * the function returns true immediately so that previous error
>> + * is not overwritten.
>> + *
>> + * Returns:
>> + *  truefeature is supported or testing hit error
>> + *  false   feature is not supported
>> + */
>> +bool
>> +virDrvSupportsFeature(virConnectPtr conn,
>> +  virDrvFeature feature,
>> +  bool *err)
>> +{
>> +int rc;
>> +
>> +if (*err)
>> +return true;
>> +
>> +if (!conn->driver->connectSupportsFeature)
>> +return false;
>> +
>> +if ((rc = conn->driver->connectSupportsFeature(conn, feature)) < 0) {
>> +*err = true;
>> +return true;
>> +}
>> +
>> +return rc > 0 ? true : false;
>> +}
> 
> I would just make virDrvSupportsFeature a tiny wrapper around
> conn->driver->connectSupportsFeature checking
> conn->driver->connectSupportsFeature != NULL and that's it. It could
> break the if/elseif flow, but with much cleaner semantics and reduced
> black magic.
> 

Ok, thanx. I'll send next version.

Nikolay

[PATCH] util: stop probing for IFF_VNET_HDR

[Daniel P . Berrangé]

This flag was added by Linux with:

  commit f43798c27684ab925adde7d8acc34c78c6e50df8
  Author: Rusty Russell 
  Date:   Thu Jul 3 03:48:02 2008 -0700

tun: Allow GSO using virtio_net_hdr

so we can assume all Linux distros we support have this flag available
and thus the compile time check is sufficient.

Signed-off-by: Daniel P. Berrangé 
---
 src/util/virnetdevtap.c | 63 +
 1 file changed, 1 insertion(+), 62 deletions(-)

diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
index cbce5c71b7..77c4d1c52c 100644
--- a/src/util/virnetdevtap.c
+++ b/src/util/virnetdevtap.c
@@ -183,66 +183,6 @@ virNetDevTapGetRealDeviceName(char *ifname G_GNUC_UNUSED)
 }
 
 
-/**
- * virNetDevProbeVnetHdr:
- * @tapfd: a tun/tap file descriptor
- *
- * Check whether it is safe to enable the IFF_VNET_HDR flag on the
- * tap interface.
- *
- * Setting IFF_VNET_HDR enables QEMU's virtio_net driver to allow
- * guests to pass larger (GSO) packets, with partial checksums, to
- * the host. This greatly increases the achievable throughput.
- *
- * It is only useful to enable this when we're setting up a virtio
- * interface. And it is only *safe* to enable it when we know for
- * sure that a) qemu has support for IFF_VNET_HDR and b) the running
- * kernel implements the TUNGETIFF ioctl(), which qemu needs to query
- * the supplied tapfd.
- *
- * Returns 1 if VnetHdr is supported, 0 if not supported
- */
-#ifdef IFF_VNET_HDR
-static int
-virNetDevProbeVnetHdr(int tapfd)
-{
-# if defined(IFF_VNET_HDR) && defined(TUNGETFEATURES) && defined(TUNGETIFF)
-unsigned int features;
-struct ifreq dummy;
-
-if (ioctl(tapfd, TUNGETFEATURES, ) != 0) {
-VIR_INFO("Not enabling IFF_VNET_HDR; "
- "TUNGETFEATURES ioctl() not implemented");
-return 0;
-}
-
-if (!(features & IFF_VNET_HDR)) {
-VIR_INFO("Not enabling IFF_VNET_HDR; "
- "TUNGETFEATURES ioctl() reports no IFF_VNET_HDR");
-return 0;
-}
-
-/* The kernel will always return -1 at this point.
- * If TUNGETIFF is not implemented then errno == EBADFD.
- */
-if (ioctl(tapfd, TUNGETIFF, ) != -1 || errno != EBADFD) {
-VIR_INFO("Not enabling IFF_VNET_HDR; "
- "TUNGETIFF ioctl() not implemented");
-return 0;
-}
-
-VIR_INFO("Enabling IFF_VNET_HDR");
-
-return 1;
-# else
-(void) tapfd;
-VIR_INFO("Not enabling IFF_VNET_HDR; disabled at build time");
-return 0;
-# endif
-}
-#endif
-
-
 #ifdef TUNSETIFF
 /**
  * virNetDevTapGenerateName:
@@ -363,8 +303,7 @@ int virNetDevTapCreate(char **ifname,
 }
 
 # ifdef IFF_VNET_HDR
-if ((flags &  VIR_NETDEV_TAP_CREATE_VNET_HDR) &&
-virNetDevProbeVnetHdr(fd))
+if (flags &  VIR_NETDEV_TAP_CREATE_VNET_HDR)
 ifr.ifr_flags |= IFF_VNET_HDR;
 # endif
 
-- 
2.26.2

Re: [PATCH 5/6] qemuSnapshotCreateActiveExternalDisks: Extract actual snapshot creation to 'qemuSnapshotDiskCreate'

[Ján Tomko]

On a Thursday in 2020, Peter Krempa wrote:

Extract the code which invokes the monitor and finalizes the snapshot
into a separate function for easier reuse.

Signed-off-by: Peter Krempa 
---
src/qemu/qemu_snapshot.c | 67 
1 file changed, 40 insertions(+), 27 deletions(-)

diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index b9b640058c..6fe925763e 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -1182,57 +1182,42 @@ qemuSnapshotDiskUpdateSource(virDomainObjPtr vm,
}


-/* The domain is expected to be locked and active. */
static int
-qemuSnapshotCreateActiveExternalDisks(virQEMUDriverPtr driver,
-  virDomainObjPtr vm,
-  virDomainMomentObjPtr snap,
-  virHashTablePtr blockNamedNodeData,
-  unsigned int flags,
-  virQEMUDriverConfigPtr cfg,
-  qemuDomainAsyncJob asyncJob)
+qemuSnapshotDiskCreate(qemuSnapshotDiskContextPtr snapctxt,
+   virQEMUDriverConfigPtr cfg)
{
-qemuDomainObjPrivatePtr priv = vm->privateData;
-int rc;
+qemuDomainObjPrivatePtr priv = snapctxt->vm->privateData;
+virQEMUDriverPtr driver = priv->driver;
size_t i;
-bool reuse = (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT) != 0;
-g_autoptr(qemuSnapshotDiskContext) snapctxt = NULL;
-
-if (virDomainObjCheckActive(vm) < 0)
-return -1;
+int rc;

-/* prepare a list of objects to use in the vm definition so that we don't
- * have to roll back later */
-if (!(snapctxt = qemuSnapshotDiskPrepare(vm, snap, cfg, reuse,
- blockNamedNodeData, asyncJob)))
-return -1;



This leaves a double space here.

Jano


/* check whether there's anything to do */
if (snapctxt->ndd == 0)
return 0;

-if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
+if (qemuDomainObjEnterMonitorAsync(driver, snapctxt->vm, snapctxt->asyncJob) 
< 0)
return -1;

rc = qemuMonitorTransaction(priv->mon, >actions);

-if (qemuDomainObjExitMonitor(driver, vm) < 0)
+if (qemuDomainObjExitMonitor(driver, snapctxt->vm) < 0)
rc = -1;

for (i = 0; i < snapctxt->ndd; i++) {
qemuSnapshotDiskDataPtr dd = snapctxt->dd + i;

-virDomainAuditDisk(vm, dd->disk->src, dd->src, "snapshot", rc >= 0);
+virDomainAuditDisk(snapctxt->vm, dd->disk->src, dd->src, "snapshot", rc 
>= 0);

if (rc == 0)
-qemuSnapshotDiskUpdateSource(vm, dd);
+qemuSnapshotDiskUpdateSource(snapctxt->vm, dd);
}

if (rc < 0)
return -1;

-if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0 ||
-(vm->newDef && virDomainDefSave(vm->newDef, driver->xmlopt,
+if (virDomainObjSave(snapctxt->vm, driver->xmlopt, cfg->stateDir) < 0 ||
+(snapctxt->vm->newDef && virDomainDefSave(snapctxt->vm->newDef, 
driver->xmlopt,
cfg->configDir) < 0))
return -1;

@@ -1240,6 +1225,34 @@ qemuSnapshotCreateActiveExternalDisks(virQEMUDriverPtr 
driver,
}


+/* The domain is expected to be locked and active. */
+static int
+qemuSnapshotCreateActiveExternalDisks(virDomainObjPtr vm,
+  virDomainMomentObjPtr snap,
+  virHashTablePtr blockNamedNodeData,
+  unsigned int flags,
+  virQEMUDriverConfigPtr cfg,
+  qemuDomainAsyncJob asyncJob)
+{
+bool reuse = (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT) != 0;
+g_autoptr(qemuSnapshotDiskContext) snapctxt = NULL;
+
+if (virDomainObjCheckActive(vm) < 0)
+return -1;
+
+/* prepare a list of objects to use in the vm definition so that we don't
+ * have to roll back later */
+if (!(snapctxt = qemuSnapshotDiskPrepare(vm, snap, cfg, reuse,
+ blockNamedNodeData, asyncJob)))
+return -1;
+
+if (qemuSnapshotDiskCreate(snapctxt, cfg) < 0)
+return -1;
+
+return 0;
+}
+
+
static int
qemuSnapshotCreateActiveExternal(virQEMUDriverPtr driver,
 virDomainObjPtr vm,
@@ -1366,7 +1379,7 @@ qemuSnapshotCreateActiveExternal(virQEMUDriverPtr driver,

/* the domain is now paused if a memory snapshot was requested */

-if ((ret = qemuSnapshotCreateActiveExternalDisks(driver, vm, snap,
+if ((ret = qemuSnapshotCreateActiveExternalDisks(vm, snap,
 blockNamedNodeData, flags, 
cfg,
 QEMU_ASYNC_JOB_SNAPSHOT)) 
< 0)
goto cleanup;
--
2.26.2



signature.asc
Description: PGP signature

Re: [PATCH 0/6] qemu: snapshot: Refactors prior to introduction of disk support

[Ján Tomko]

On a Thursday in 2020, Peter Krempa wrote:

We'll be installing overlays on top of  disks via the
snapshot code. Prepare some aspects.


Peter Krempa (6):
 qemu: snapshot: Rename 'qemuSnapshotCreateDiskActive' to
   'qemuSnapshotCreateActiveExternalDisks'
 qemuSnapshotDiskUpdateSource: Extract 'driver' and 'blockdev' from
   'vm'
 qemuSnapshotDiskPrepare/Cleanup: simplify passing of 'driver' and
   'blockdev'
 qemu: snapshot: Introduce qemuSnapshotDiskContext
 qemuSnapshotCreateActiveExternalDisks: Extract actual snapshot
   creation to 'qemuSnapshotDiskCreate'
 qemuSnapshotDiskPrepare: rename to
   qemuSnapshotDiskPrepareActiveExternal

src/qemu/qemu_snapshot.c | 215 ++-
1 file changed, 124 insertions(+), 91 deletions(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature

Re: [libvirt PATCH 11/14] storage: createFileDir: use less ternary operators

[Martin Kletzander]

On Wed, Sep 23, 2020 at 08:15:00PM +0200, Ján Tomko wrote:

Introduce separate variables and if conditions
with spaces around them to make the function call
easier to read.

Signed-off-by: Ján Tomko 
---
src/storage/storage_util.c | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 93c24ab6bc..49ecbc5344 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -1997,6 +1997,8 @@ createFileDir(virStoragePoolObjPtr pool,
  unsigned int flags)
{
virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
+mode_t permmode = VIR_STORAGE_DEFAULT_VOL_PERM_MODE;
+unsigned int flags = 0;

virCheckFlags(0, -1);

@@ -2013,15 +2015,17 @@ createFileDir(virStoragePoolObjPtr pool,
return -1;
}

+if (vol->target.perms->mode != (mode_t)-1)
+permmode = vol->target.perms->mode;
+


It would be even less repetitive if you did:

mode_t permmode = vol->target.perms->mode;
if (permmode == -1)
permmode = VIR_STORAGE_DEFAULT_VOL_PERM_MODE;

But

Reviewed-by: Martin Kletzander 

with your squash, of course.


+if (def->type == VIR_STORAGE_POOL_NETFS)
+flags |= VIR_DIR_CREATE_AS_UID;

if (virDirCreate(vol->target.path,
- (vol->target.perms->mode == (mode_t)-1 ?
-  VIR_STORAGE_DEFAULT_VOL_PERM_MODE :
-  vol->target.perms->mode),
+ permmode,
 vol->target.perms->uid,
 vol->target.perms->gid,
- (def->type == VIR_STORAGE_POOL_NETFS
-  ? VIR_DIR_CREATE_AS_UID : 0)) < 0) {
+ flags) < 0) {
return -1;
}

--
2.26.2



signature.asc
Description: PGP signature

Re: [libvirt PATCH 14/14] Reduce scope of some variables

[Martin Kletzander]

On Wed, Sep 23, 2020 at 08:15:03PM +0200, Ján Tomko wrote:

Signed-off-by: Ján Tomko 


Reviewed-by: Martin Kletzander 


signature.asc
Description: PGP signature

Re: [libvirt PATCH 13/14] storage: storageBackendWipeLocal: reduce variable scope

[Martin Kletzander]

On Wed, Sep 23, 2020 at 08:15:02PM +0200, Ján Tomko wrote:

Also use MIN instead of open-coding it.

Signed-off-by: Ján Tomko 


Reviewed-by: Martin Kletzander 


---
src/storage/storage_util.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 49ecbc5344..23632fc884 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -2526,10 +2526,8 @@ storageBackendWipeLocal(const char *path,
size_t writebuf_length,
bool zero_end)
{
-int written = 0;
unsigned long long remaining = 0;
off_t size;
-size_t write_size = 0;
g_autofree char *writebuf = NULL;

if (VIR_ALLOC_N(writebuf, writebuf_length) < 0)
@@ -2557,9 +2555,9 @@ storageBackendWipeLocal(const char *path,

remaining = wipe_len;
while (remaining > 0) {
+size_t write_size = MIN(writebuf_length, remaining);
+int written = safewrite(fd, writebuf, write_size);

-write_size = (writebuf_length < remaining) ? writebuf_length : 
remaining;
-written = safewrite(fd, writebuf, write_size);
if (written < 0) {
virReportSystemError(errno,
 _("Failed to write %zu bytes to "
--
2.26.2



signature.asc
Description: PGP signature

Re: [libvirt PATCH 12/14] vbox: reduce variable scope in vboxDumpStorageControllers

[Martin Kletzander]

On Wed, Sep 23, 2020 at 08:15:01PM +0200, Ján Tomko wrote:

Most of the variables were reinitialized on every iteration.

Also remove the pointless initialization of 'i'.



Same reply about the 'i' (), for the rest:

Reviewed-by: Martin Kletzander 


Signed-off-by: Ján Tomko 
---
src/vbox/vbox_common.c | 23 +--
1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index b9b72e2e02..6a517ff96c 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3079,23 +3079,18 @@ static int
vboxDumpStorageControllers(virDomainDefPtr def, IMachine *machine)
{
vboxArray storageControllers = VBOX_ARRAY_INITIALIZER;
-IStorageController *controller = NULL;
-PRUint32 storageBus = StorageBus_Null;
-PRUint32 controllerType = StorageControllerType_Null;
-virDomainControllerDefPtr cont = NULL;
-size_t i = 0;
-int model = -1, ret = -1;
-virDomainControllerType type = VIR_DOMAIN_CONTROLLER_TYPE_LAST;
+int ret = -1;
+size_t i;

gVBoxAPI.UArray.vboxArrayGet(, machine,
 gVBoxAPI.UArray.handleMachineGetStorageControllers(machine));

for (i = 0; i < storageControllers.count; i++) {
-controller = storageControllers.items[i];
-storageBus = StorageBus_Null;
-controllerType = StorageControllerType_Null;
-type = VIR_DOMAIN_CONTROLLER_TYPE_LAST;
-model = -1;
+IStorageController *controller = storageControllers.items[i];
+PRUint32 storageBus = StorageBus_Null;
+PRUint32 controllerType = StorageControllerType_Null;
+virDomainControllerType type = VIR_DOMAIN_CONTROLLER_TYPE_LAST;
+int model = -1;

if (!controller)
continue;
@@ -3133,8 +3128,6 @@ vboxDumpStorageControllers(virDomainDefPtr def, IMachine 
*machine)
case StorageControllerType_IntelAhci:
case StorageControllerType_I82078:
case StorageControllerType_Null:
-model = -1;
-
break;
}

@@ -3165,6 +3158,8 @@ vboxDumpStorageControllers(virDomainDefPtr def, IMachine 
*machine)
}

if (type != VIR_DOMAIN_CONTROLLER_TYPE_LAST) {
+virDomainControllerDefPtr cont;
+
cont = virDomainDefAddController(def, type, -1, model);
if (!cont) {
virReportError(VIR_ERR_INTERNAL_ERROR,
--
2.26.2



signature.asc
Description: PGP signature

[PATCH] the leading space in volmode check will never match the leading tab output from zfs get

[richardburakowski]

Signed-off-by: Richard Burakowski 
---
 src/storage/storage_backend_zfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/storage/storage_backend_zfs.c 
b/src/storage/storage_backend_zfs.c
index dc692f47ed..439f5b2fd5 100644
--- a/src/storage/storage_backend_zfs.c
+++ b/src/storage/storage_backend_zfs.c
@@ -71,7 +71,7 @@ virStorageBackendZFSVolModeNeeded(void)
 return ret;
 }
 
-if (strstr(error, " volmode "))
+if (strstr(error, "volmode "))
 return 1;
 else
 return 0;
-- 
2.26.2

Help on meson build

[Wei Wang]

Hi folks,

I'm trying to build libvirt using meson with the latest upstream libvirt,
but the compilation fails:
(followed on https://libvirt.org/compiling.html, not sure if any
dependencies are missed)

FAILED: src/util/libvirt_util.a.p/glibcompat.c.o

In file included from ../src/util/glibcompat.c:19:
./config.h:1026:10: fatal error: config-post.h: No such file or directory
FAILED: src/util/libvirt_util.a.p/virfirmware.c.o
In file included from ../src/util/virarch.c:22:
./config.h:1026:10: fatal error: config-post.h: No such file or directory
FAILED: src/util/libvirt_util.a.p/viraudit.c.o
In file included from ../src/util/viraudit.c:22:
./config.h:1026:10: fatal error: config-post.h: No such file or directory

Any help is appreciated.

Thanks,
Wei

Re: [libvirt PATCH 05/14] Remove pointless initializations

[Daniel P . Berrangé]

On Thu, Sep 24, 2020 at 10:30:21AM +0200, Martin Kletzander wrote:
> On Wed, Sep 23, 2020 at 08:14:54PM +0200, Ján Tomko wrote:
> > Signed-off-by: Ján Tomko 
> > ---
> > src/test/test_driver.c | 2 +-
> > tests/virnumamock.c| 2 +-
> > tests/virrandommock.c  | 2 +-
> > 3 files changed, 3 insertions(+), 3 deletions(-)
> > 
> 
> As mentioned in the previous patch I do not think they are pointless in the 
> long
> run.  I prefer ret, rc, rv and similar to be initialised to error/negative 
> value
> and others to their null values (numbers to zero, pointers to NULL etc.) even
> though I know this is a very subjective opinion.

Yeah, I tend to agree. From a defensive coding POV it is preferrable to
initialize every single variable at time of declaration to a known value.

While compilers and coverity can check for use of uninitialized variables,
I don't think their checks have ever been perfect, because new versions of
compilers tend to discover things periodically.


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 10/14] storage: createFileDir: remove useless 'err' variable

[Martin Kletzander]

On Wed, Sep 23, 2020 at 08:14:59PM +0200, Ján Tomko wrote:

Signed-off-by: Ján Tomko 


Reviewed-by: Martin Kletzander 


---
src/storage/storage_util.c | 17 -
1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 9171cb084f..93c24ab6bc 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -1997,7 +1997,6 @@ createFileDir(virStoragePoolObjPtr pool,
  unsigned int flags)
{
virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
-int err;

virCheckFlags(0, -1);

@@ -2015,14 +2014,14 @@ createFileDir(virStoragePoolObjPtr pool,
}


-if ((err = virDirCreate(vol->target.path,
-(vol->target.perms->mode == (mode_t)-1 ?
- VIR_STORAGE_DEFAULT_VOL_PERM_MODE :
- vol->target.perms->mode),
-vol->target.perms->uid,
-vol->target.perms->gid,
-(def->type == VIR_STORAGE_POOL_NETFS
- ? VIR_DIR_CREATE_AS_UID : 0))) < 0) {
+if (virDirCreate(vol->target.path,
+ (vol->target.perms->mode == (mode_t)-1 ?
+  VIR_STORAGE_DEFAULT_VOL_PERM_MODE :
+  vol->target.perms->mode),
+ vol->target.perms->uid,
+ vol->target.perms->gid,
+ (def->type == VIR_STORAGE_POOL_NETFS
+  ? VIR_DIR_CREATE_AS_UID : 0)) < 0) {
return -1;
}

--
2.26.2



signature.asc
Description: PGP signature

Re: [libvirt PATCH 09/14] api: virDomainMemoryStats: use 'ret' variable

[Martin Kletzander]

On Wed, Sep 23, 2020 at 08:14:58PM +0200, Ján Tomko wrote:

Instead of 'nr_stats_ret'. Also reduce its scope.

Signed-off-by: Ján Tomko 


Reviewed-by: Martin Kletzander 


---
src/libvirt-domain.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index cde86c77e8..415482a526 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -5741,7 +5741,6 @@ virDomainMemoryStats(virDomainPtr dom, 
virDomainMemoryStatPtr stats,
 unsigned int nr_stats, unsigned int flags)
{
virConnectPtr conn;
-unsigned long nr_stats_ret = 0;

VIR_DOMAIN_DEBUG(dom, "stats=%p, nr_stats=%u, flags=0x%x",
 stats, nr_stats, flags);
@@ -5758,11 +5757,10 @@ virDomainMemoryStats(virDomainPtr dom, 
virDomainMemoryStatPtr stats,

conn = dom->conn;
if (conn->driver->domainMemoryStats) {
-nr_stats_ret = conn->driver->domainMemoryStats(dom, stats, nr_stats,
-   flags);
-if (nr_stats_ret == -1)
+int ret = conn->driver->domainMemoryStats(dom, stats, nr_stats, flags);
+if (ret == -1)
goto error;
-return nr_stats_ret;
+return ret;
}

virReportUnsupportedError();
--
2.26.2



signature.asc
Description: PGP signature

Re: [libvirt PATCH 08/14] libxl: remove unused 'bits' from struct guest_arch

[Martin Kletzander]

On Wed, Sep 23, 2020 at 08:14:57PM +0200, Ján Tomko wrote:

It was made pointless by:
commit c25c18f71bdc43a1305be4ad1a2ca91b25cf13f3
   Convert capabilities / domain_conf to use virArch

and unused by:
commit 8db1f2d228bb2f27a729a873dcdb81ce3c7c38fd
   Fix libxl driver for virArch changes

Signed-off-by: Ján Tomko 


Reviewed-by: Martin Kletzander 


---
src/libxl/libxl_capabilities.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c
index 8284ea3d53..e5b144ea1d 100644
--- a/src/libxl/libxl_capabilities.c
+++ b/src/libxl/libxl_capabilities.c
@@ -52,7 +52,6 @@ enum libxlHwcapVersion {

struct guest_arch {
virArch arch;
-int bits;
int hvm;
int pvh;
int pae;
--
2.26.2



signature.asc
Description: PGP signature

Re: [libvirt PATCH 07/14] Do not check whether unsigned variables are negative

[Martin Kletzander]

On Wed, Sep 23, 2020 at 08:14:56PM +0200, Ján Tomko wrote:

Signed-off-by: Ján Tomko 
---
src/openvz/openvz_driver.c | 2 +-
src/vbox/vbox_common.c | 9 -
2 files changed, 5 insertions(+), 6 deletions(-)



Reviewed-by: Martin Kletzander 


signature.asc
Description: PGP signature

Re: [libvirt PATCH 06/14] virsh: virshStreamSourceSkip: remove unused 'off'

[Martin Kletzander]

On Wed, Sep 23, 2020 at 08:14:55PM +0200, Ján Tomko wrote:

Signed-off-by: Ján Tomko 
---
tools/virsh-util.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)



Reviewed-by: Martin Kletzander 


diff --git a/tools/virsh-util.c b/tools/virsh-util.c
index b8b0f98066..af7ed55348 100644
--- a/tools/virsh-util.c
+++ b/tools/virsh-util.c
@@ -173,9 +173,8 @@ virshStreamSourceSkip(virStreamPtr st G_GNUC_UNUSED,
{
virshStreamCallbackDataPtr cbData = opaque;
int fd = cbData->fd;
-off_t cur;

-if ((cur = lseek(fd, offset, SEEK_CUR)) == (off_t) -1)
+if (lseek(fd, offset, SEEK_CUR) == (off_t) -1)
return -1;

return 0;
--
2.26.2



signature.asc
Description: PGP signature

Re: [libvirt PATCH 05/14] Remove pointless initializations

[Martin Kletzander]

On Wed, Sep 23, 2020 at 08:14:54PM +0200, Ján Tomko wrote:

Signed-off-by: Ján Tomko 
---
src/test/test_driver.c | 2 +-
tests/virnumamock.c| 2 +-
tests/virrandommock.c  | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)



As mentioned in the previous patch I do not think they are pointless in the long
run.  I prefer ret, rc, rv and similar to be initialised to error/negative value
and others to their null values (numbers to zero, pointers to NULL etc.) even
though I know this is a very subjective opinion.

If you really want to push for this then it would require making

Of course the compiler can check if the value is used before initialisation.
And I hope all current compilers do that with our warning options.  Except they
cannot check for that if you only pass the address of the value.  I guess these
are fine, but I would not like to create a precedent.

What I think we should do instead is make an exception for defining variables
for iteration in the for loop itself.  I know it's only 21 years since it was
standardised and we want to make sure all the compilers and distros can handle
it... /s

OK, I understand that defining variables in the middle of the function most of
the time clutters the code, but for 'i' and 'j' I, for one, see simply no reason
to disallow that.


diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index d582c207f4..cbbfea6665 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4354,7 +4354,7 @@ testNodeGetFreePages(virConnectPtr conn G_GNUC_UNUSED,
 unsigned long long *counts,
 unsigned int flags)
{
-size_t i = 0, j = 0;
+size_t i, j;
int x = 6;

virCheckFlags(0, -1);
diff --git a/tests/virnumamock.c b/tests/virnumamock.c
index 40e18e646e..d39c264a3f 100644
--- a/tests/virnumamock.c
+++ b/tests/virnumamock.c
@@ -130,7 +130,7 @@ virNumaGetPages(int node,
{
const int pages_def[] = { 4, 2 * 1024, 1 * 1024 * 1024};
const int npages_def = G_N_ELEMENTS(pages_def);
-size_t i = 0;
+size_t i;

if (pages_size)
*pages_size = g_new0(unsigned int, npages_def);
diff --git a/tests/virrandommock.c b/tests/virrandommock.c
index 6dd15213e3..ca0520a5a3 100644
--- a/tests/virrandommock.c
+++ b/tests/virrandommock.c
@@ -69,7 +69,7 @@ int
gnutls_dh_params_generate2(gnutls_dh_params_t dparams,
   unsigned int bits)
{
-int rc = 0;
+int rc;

VIR_MOCK_REAL_INIT(gnutls_dh_params_generate2);

--
2.26.2



signature.asc
Description: PGP signature

[PATCH v3] util: support PCI passthrough net device stats collection

[zhenwei pi]

Collect PCI passthrough net device stats from kernel by netlink
API.

Currently, libvirt can not get PCI passthrough net device stats,
run command:
 #virsh domifstat instance --interface=52:54:00:2d:b2:35
 error: Failed to get interface stats instance 52:54:00:2d:b2:35
 error: internal error: Interface name not provided

The PCI device(usually SR-IOV virtual function device) is detached
while it's used in PCI passthrough mode. And we can not parse this
device from /proc/net/dev any more.

In this patch, libvirt check net device is VF of not firstly, then
query virNetDevVFInterfaceStats(new API).
virNetDevVFInterfaceStats parses VFs info of all PFs, compares MAC
address until the two MAC addresses match.
'#ip -s link show' can get the same result. Instead of parsing the
output result, implement this feature by libnl API.

Notice that this feature deponds on driver of PF.
Test on Mellanox ConnectX-4 Lx, it works well.
Also test on Intel Corporation 82599ES, it works, but only get 0.
(ip-link command get the same result).

IFLA_VF_STATS is supported since Linux-4.2, suggested by Laine,
just using defined(__linux__) && WITH_LIBNL is enough.

Signed-off-by: zhenwei pi 
---
 src/libvirt_private.syms |   1 +
 src/qemu/qemu_driver.c   |   3 ++
 src/util/virnetdev.c | 121 +++
 src/util/virnetdev.h |   5 ++
 4 files changed, 130 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index bdbe3431b8..bcc40b8d69 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2585,6 +2585,7 @@ virNetDevSetRcvMulti;
 virNetDevSetupControl;
 virNetDevSysfsFile;
 virNetDevValidateConfig;
+virNetDevVFInterfaceStats;
 
 
 # util/virnetdevbandwidth.h
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ae715c01d7..f554010c40 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10196,6 +10196,9 @@ qemuDomainInterfaceStats(virDomainPtr dom,
 if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_VHOSTUSER) {
 if (virNetDevOpenvswitchInterfaceStats(net->ifname, stats) < 0)
 goto cleanup;
+} else if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
+if (virNetDevVFInterfaceStats(>mac, stats) < 0)
+goto cleanup;
 } else {
 if (virNetDevTapInterfaceStats(net->ifname, stats,
!virDomainNetTypeSharesHostView(net)) < 
0)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index e1a4cc2bef..3d54d07606 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -1489,6 +1489,7 @@ static struct nla_policy ifla_vf_policy[IFLA_VF_MAX+1] = {
 .maxlen = sizeof(struct ifla_vf_mac) },
 [IFLA_VF_VLAN]  = { .type = NLA_UNSPEC,
 .maxlen = sizeof(struct ifla_vf_vlan) },
+[IFLA_VF_STATS] = { .type = NLA_NESTED },
 };
 
 
@@ -2265,6 +2266,116 @@ virNetDevSetNetConfig(const char *linkdev, int vf,
 return 0;
 }
 
+static struct nla_policy ifla_vfstats_policy[IFLA_VF_STATS_MAX+1] = {
+[IFLA_VF_STATS_RX_PACKETS]  = { .type = NLA_U64 },
+[IFLA_VF_STATS_TX_PACKETS]  = { .type = NLA_U64 },
+[IFLA_VF_STATS_RX_BYTES]= { .type = NLA_U64 },
+[IFLA_VF_STATS_TX_BYTES]= { .type = NLA_U64 },
+[IFLA_VF_STATS_BROADCAST]   = { .type = NLA_U64 },
+[IFLA_VF_STATS_MULTICAST]   = { .type = NLA_U64 },
+};
+
+static int
+virNetDevParseVfStats(struct nlattr **tb, virMacAddrPtr mac,
+  virDomainInterfaceStatsPtr stats)
+{
+int ret = -1, len;
+struct ifla_vf_mac *vf_lladdr;
+struct nlattr *nla, *t[IFLA_VF_MAX+1];
+struct nlattr *stb[IFLA_VF_STATS_MAX+1];
+
+if (tb == NULL || mac == NULL || stats == NULL) {
+return -1;
+}
+
+if (!tb[IFLA_VFINFO_LIST])
+return -1;
+
+len = nla_len(tb[IFLA_VFINFO_LIST]);
+
+for (nla = nla_data(tb[IFLA_VFINFO_LIST]); nla_ok(nla, len);
+nla = nla_next(nla, )) {
+ret = nla_parse(t, IFLA_VF_MAX, nla_data(nla), nla_len(nla),
+ifla_vf_policy);
+if (ret < 0)
+return -1;
+
+if (t[IFLA_VF_MAC] == NULL) {
+continue;
+}
+
+vf_lladdr = nla_data(t[IFLA_VF_MAC]);
+if (virMacAddrCmpRaw(mac, vf_lladdr->mac)) {
+continue;
+}
+
+if (t[IFLA_VF_STATS]) {
+ret = nla_parse_nested(stb, IFLA_VF_STATS_MAX,
+t[IFLA_VF_STATS],
+ifla_vfstats_policy);
+if (ret < 0)
+return -1;
+
+stats->rx_bytes = nla_get_u64(stb[IFLA_VF_STATS_RX_BYTES]);
+stats->tx_bytes = nla_get_u64(stb[IFLA_VF_STATS_TX_BYTES]);
+stats->rx_packets = nla_get_u64(stb[IFLA_VF_STATS_RX_PACKETS]);
+stats->tx_packets = nla_get_u64(stb[IFLA_VF_STATS_TX_PACKETS]);
+}
+return 0;
+}
+
+return ret;
+}
+
+/**
+ * 

Re: recommendations for handling Hyper-V version number

[Ján Tomko]

On a Wednesday in 2020, Matt Coleman wrote:

Hello again,

Hyper-V version numbers are not compatible with the encoding used in
virParseVersionString:
https://gitlab.com/libvirt/libvirt/-/blob/master/src/util/virutil.c#L246

For example, the Windows Server 2016 Hyper-V version is 10.0.14393.0,
so its “micro” is over 14 times larger than the encoding allows.

My current workaround is to truncate it to something that works (E.G.
10.0.143), but that's clearly less than ideal.

Is the output of virParseVersionString stored on disk at any point?
Would changing its output type from unsigned long to unsigned long long
cause any compatibility issues? That would allow “minor” and “micro” to
be up to 6 digits each and would be a quick, easy change.



virParseVersionString is just an internal helper, those can be changed
at will if you don't break existing users.

The problem here is the virConnectGetVersion public API - we cannot
alter its signature.

One solution would be to introduce a new API (that possibly returns a
string), but it will take time until apps start using it.

Jano


Thanks,
Matt




signature.asc
Description: PGP signature

Re: [libvirt PATCH 04/14] Remove pointless assignments

[Martin Kletzander]

On Wed, Sep 23, 2020 at 08:14:53PM +0200, Ján Tomko wrote:

There's no need to initialize every variable,


Automotive and some other industries would wholeheartedly disagree with you, but
that's not the point here.  I would argue that initializing by default is a very
good practice in larger codebases if only for error-proofing against assumptions
in future changes.  But what you are fixing are just assignments which are
mostly pointless.


especially not if it's overwritten two lines later.



That's true.  Or if it is unused (e.g. the last hunk).


Signed-off-by: Ján Tomko 
---
examples/c/admin/logging.c | 2 +-
src/libxl/libxl_driver.c   | 2 +-
src/vbox/vbox_common.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/examples/c/admin/logging.c b/examples/c/admin/logging.c
index 730ae40d9d..c183c4867d 100644
--- a/examples/c/admin/logging.c
+++ b/examples/c/admin/logging.c
@@ -29,7 +29,7 @@ int main(int argc, char **argv)
const char *set_outputs = NULL;
const char *set_filters = NULL;

-ret = c = -1;
+ret = -1;
opterr = 0;

while ((c = getopt(argc, argv, ":hpo:f:")) > 0) {
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 161a6882f3..ea7d08cd11 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -5423,7 +5423,7 @@ libxlDiskSectorSize(int domid, int devno)
return ret;
}

-path = val = NULL;
+val = NULL;


Why not get rid of the `val = NULL` as well?


path = g_strdup_printf("/local/domain/%d/device/vbd/%d/backend", domid, 
devno);

if ((val = xs_read(handle, XBT_NULL, path, )) == NULL)


It's rewritten right here.


diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 99c7fbd117..2783827012 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -4618,7 +4618,7 @@ vboxSnapshotRedefine(virDomainPtr dom,
if (openSessionForMachine(data, dom->uuid, , ) < 0)
goto cleanup;

-rc = gVBoxAPI.UIMachine.SaveSettings(machine);
+gVBoxAPI.UIMachine.SaveSettings(machine);


Maybe this is missing a check for an error instead.  Impossible to find out from
the docs that I quickly tried to look up though.

Anyway,

Reviewed-by: Martin Kletzander 


/* It may failed when the machine is not mutable. */
rc = gVBoxAPI.UIMachine.GetSettingsFilePath(machine, );
if (NS_FAILED(rc)) {
--
2.26.2



signature.asc
Description: PGP signature

Re: [libvirt PATCH 02/14] vbox: remove VBoxCGlueTerm

[Martin Kletzander]

On Wed, Sep 23, 2020 at 08:14:51PM +0200, Ján Tomko wrote:

cppcheck reports:
 src/vbox/vbox_XPCOMCGlue.c:226:21: style:
 The statement 'if (hVBoxXPCOMC!=NULL) hVBoxXPCOMC=NULL' is
 logically equivalent to 'hVBoxXPCOMC=NULL'.
 [duplicateConditionalAssign]

It does not matter anyway because this function
is never called.

Fixes: e1506cb4eb7eab96e7ded27a23f0d8ac9697ac2a
Signed-off-by: Ján Tomko 


Reviewed-by: Martin Kletzander 


signature.asc
Description: PGP signature

Re: [libvirt PATCH 03/14] vbox: StartMachine: overwrite ret less often

[Martin Kletzander]

On Wed, Sep 23, 2020 at 08:14:52PM +0200, Ján Tomko wrote:

Use goto to jump over the ret = 0 assignment
as is usual in rest of the code.

Signed-off-by: Ján Tomko 
---
src/vbox/vbox_common.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 9978741a64..99c7fbd117 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -2090,7 +2090,7 @@ vboxStartMachine(virDomainPtr dom, int maxDomID, IMachine 
*machine, vboxIID *iid
int ret = -1;

if (!data->vboxObj)
-return ret;
+return -1;

VBOX_UTF8_TO_UTF16("FRONTEND/Type", );
gVBoxAPI.UIMachine.GetExtraData(machine, keyTypeUtf16, );
@@ -2177,7 +2177,7 @@ vboxStartMachine(virDomainPtr dom, int maxDomID, IMachine 
*machine, vboxIID *iid
if (NS_FAILED(rc)) {
virReportError(VIR_ERR_OPERATION_FAILED, "%s",
   _("OpenRemoteSession/LaunchVMProcess failed, domain can't be 
started"));
-ret = -1;
+goto cleanup;
} else {
PRBool completed = 0;
resultCodeUnion resultCode;
@@ -2186,19 +2186,21 @@ vboxStartMachine(virDomainPtr dom, int maxDomID, 
IMachine *machine, vboxIID *iid
rc = gVBoxAPI.UIProgress.GetCompleted(progress, );
if (NS_FAILED(rc)) {
/* error */
-ret = -1;
+goto cleanup;


This one is not semantically equivalent.  But since I do not know enough about
the vbox driver I cannot tell whether it makes sense to keep the current
behaviour.  It looks like it also fixes it but I cannot be sure enough.

Let's wait for a couple of days if someone knows more about that and if not,
then consider this

Reviewed-by: Martin Kletzander 

and let's see if someone experiences issues after that.


}
gVBoxAPI.UIProgress.GetResultCode(progress, );
if (RC_FAILED(resultCode)) {
/* error */
-ret = -1;
+goto cleanup;
} else {
/* all ok set the domid */
dom->id = maxDomID + 1;
-ret = 0;
}
}

+ret = 0;
+
+ cleanup:
VBOX_RELEASE(progress);

gVBoxAPI.UISession.Close(data->vboxSession);
--
2.26.2



signature.asc
Description: PGP signature