[PATCH 3/3] qemuTestParseCapabilitiesArch: Free @binary

2020-02-20 Thread Michal Privoznik
The variable is allocated, but never freed.

==119642== 29 bytes in 1 blocks are definitely lost in loss record 409 of 671
==119642==at 0x483579F: malloc (vg_replace_malloc.c:309)
==119642==by 0x5AB075F: __vasprintf_internal (in /lib64/libc-2.29.so)
==119642==by 0x57C1A28: g_vasprintf (in /usr/lib64/libglib-2.0.so.0.6000.7)
==119642==by 0x579A0CC: g_strdup_vprintf (in 
/usr/lib64/libglib-2.0.so.0.6000.7)
==119642==by 0x4AE6D58: vir_g_strdup_printf (glibcompat.c:197)
==119642==by 0x136EEE: qemuTestParseCapabilitiesArch (testutilsqemu.c:291)
==119642==by 0x138506: testQemuInfoSetArgs (testutilsqemu.c:763)
==119642==by 0x135FFF: mymain (qemuxml2argvtest.c:3093)
==119642==by 0x13A60E: virTestMain (testutils.c:839)
==119642==by 0x1368C2: main (qemuxml2argvtest.c:3121)

Signed-off-by: Michal Privoznik 
---
 tests/testutilsqemu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index 0cb9a7456d..4dd5664f7b 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -288,8 +288,8 @@ qemuTestParseCapabilitiesArch(virArch arch,
   const char *capsFile)
 {
 virQEMUCapsPtr qemuCaps = NULL;
-char *binary = g_strdup_printf("/usr/bin/qemu-system-%s",
-   virArchToString(arch));
+g_autofree char *binary = g_strdup_printf("/usr/bin/qemu-system-%s",
+  virArchToString(arch));
 
 if (!(qemuCaps = virQEMUCapsNewBinary(binary)) ||
 virQEMUCapsLoadCache(arch, qemuCaps, capsFile) < 0)
-- 
2.24.1



[PATCH 2/3] virDomainNetDefClear: Free @persistent name

2020-02-20 Thread Michal Privoznik
The persistent alias name @persistent is allocated in
virDomainNetDefParseXML() but never freed.

==119642== 22 bytes in 2 blocks are definitely lost in loss record 178 of 671
==119642==at 0x483579F: malloc (vg_replace_malloc.c:309)
==119642==by 0x58F89F1: xmlStrndup (in /usr/lib64/libxml2.so.2.9.9)
==119642==by 0x4BA3B74: virXMLPropString (virxml.c:520)
==119642==by 0x4BDB0C5: virDomainNetDefParseXML (domain_conf.c:11876)
==119642==by 0x4BF9EF4: virDomainDefParseXML (domain_conf.c:21196)
==119642==by 0x4BFCD5B: virDomainDefParseNode (domain_conf.c:21943)
==119642==by 0x4BFCC36: virDomainDefParse (domain_conf.c:21901)
==119642==by 0x4BFCCCB: virDomainDefParseFile (domain_conf.c:21924)
==119642==by 0x114A9D: testCompareXMLToArgv (qemuxml2argvtest.c:452)
==119642==by 0x13894F: virTestRun (testutils.c:143)
==119642==by 0x11F46E: mymain (qemuxml2argvtest.c:1316)
==119642==by 0x13A60E: virTestMain (testutils.c:839

Signed-off-by: Michal Privoznik 
---
 src/conf/domain_conf.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index bd2ca53f1d..cef49df3f8 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2444,6 +2444,7 @@ virDomainNetDefClear(virDomainNetDefPtr def)
 
 VIR_FREE(def->backend.tap);
 VIR_FREE(def->backend.vhost);
+VIR_FREE(def->teaming.persistent);
 VIR_FREE(def->virtPortProfile);
 VIR_FREE(def->script);
 VIR_FREE(def->domain_name);
-- 
2.24.1



[PATCH 0/3] Couple of memleak fixes

2020-02-20 Thread Michal Privoznik
*** BLURB HERE ***

Michal Prívozník (3):
  virDomainFSDefFree: Unref private data
  virDomainNetDefClear: Free @persistent name
  qemuTestParseCapabilitiesArch: Free @binary

 src/conf/domain_conf.c | 2 ++
 tests/testutilsqemu.c  | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

-- 
2.24.1



[PATCH 1/3] virDomainFSDefFree: Unref private data

2020-02-20 Thread Michal Privoznik
The privateData object is allocated in virDomainFSDefNew() but
never unref'd.

==119642== 480 bytes in 20 blocks are definitely lost in loss record 656 of 671
==119642==at 0x4837B86: calloc (vg_replace_malloc.c:762)
==119642==by 0x57806A0: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.6000.7)
==119642==by 0x4AE7392: virAllocVar (viralloc.c:331)
==119642==by 0x4B64395: virObjectNew (virobject.c:241)
==119642==by 0x48F1464: qemuDomainFSPrivateNew (qemu_domain.c:1427)
==119642==by 0x4BBF004: virDomainFSDefNew (domain_conf.c:2307)
==119642==by 0x4BD859A: virDomainFSDefParseXML (domain_conf.c:11217)
==119642==by 0x4BF9DD1: virDomainDefParseXML (domain_conf.c:21179)
==119642==by 0x4BFCD5B: virDomainDefParseNode (domain_conf.c:21943)
==119642==by 0x4BFCC36: virDomainDefParse (domain_conf.c:21901)
==119642==by 0x4BFCCCB: virDomainDefParseFile (domain_conf.c:21924)
==119642==by 0x114A9D: testCompareXMLToArgv (qemuxml2argvtest.c:452)

Signed-off-by: Michal Privoznik 
---
 src/conf/domain_conf.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a2603f095e..bd2ca53f1d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2324,6 +2324,7 @@ void virDomainFSDefFree(virDomainFSDefPtr def)
 VIR_FREE(def->dst);
 virDomainDeviceInfoClear(>info);
 VIR_FREE(def->virtio);
+virObjectUnref(def->privateData);
 
 VIR_FREE(def);
 }
-- 
2.24.1



[PATCH v1 2/3] qemumonitorjsontest: add tests for cpu comparison

2020-02-20 Thread Collin Walling
Signed-off-by: Collin Walling 
---
 tests/qemumonitorjsontest.c | 50 +
 1 file changed, 50 insertions(+)

diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index cd7fa1f..5568af5 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -2981,6 +2981,48 @@ testQemuMonitorJSONTransaction(const void *opaque)
 
 
 static int
+testQemuMonitorJSONqemuMonitorJSONGetCPUModelComparison(const void *opaque)
+{
+const testGenericData *data = opaque;
+g_autoptr(qemuMonitorTest) test = NULL;
+virCPUDefPtr cpu_a;
+virCPUDefPtr cpu_b = NULL;
+g_autofree char *result = NULL;
+int ret = -1;
+
+if (!(test = qemuMonitorTestNewSchema(data->xmlopt, data->schema)))
+return -1;
+
+if (qemuMonitorTestAddItem(test, "query-cpu-model-comparison",
+   "{\"return\":{\"result\":\"test\"}}") < 0)
+return -1;
+
+if (VIR_ALLOC(cpu_a) < 0 || VIR_ALLOC(cpu_b) < 0)
+goto cleanup;
+
+cpu_a->model = g_strdup("cpu_a");
+cpu_b->model = g_strdup("cpu_b");
+
+if (qemuMonitorJSONGetCPUModelComparison(qemuMonitorTestGetMonitor(test),
+ cpu_a, cpu_b, ) < 0)
+goto cleanup;
+
+if (!result || STRNEQ(result, "test")) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   "Compare result not set");
+goto cleanup;
+}
+
+ret = 0;
+
+ cleanup:
+virCPUDefFree(cpu_a);
+virCPUDefFree(cpu_b);
+return ret;
+}
+
+
+static int
 mymain(void)
 {
 int ret = 0;
@@ -3257,6 +3299,14 @@ mymain(void)
 
 #undef DO_TEST_QUERY_JOBS
 
+if (!(qapiData.schema = testQEMUSchemaLoad("s390x"))) {
+VIR_TEST_VERBOSE("failed to load qapi schema for s390x");
+ret = -1;
+goto cleanup;
+}
+
+DO_TEST(qemuMonitorJSONGetCPUModelComparison);
+
  cleanup:
 VIR_FREE(metaschemastr);
 virJSONValueFree(metaschema);
-- 
2.7.4




[PATCH v1 3/3] qemumonitorjsontest: add test for cpu baseline

2020-02-20 Thread Collin Walling
Signed-off-by: Collin Walling 
---
 tests/qemumonitorjsontest.c | 77 +
 1 file changed, 77 insertions(+)

diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 5568af5..e9f95e3 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -3023,6 +3023,82 @@ 
testQemuMonitorJSONqemuMonitorJSONGetCPUModelComparison(const void *opaque)
 
 
 static int
+testQemuMonitorJSONqemuMonitorJSONGetCPUModelBaseline(const void *opaque)
+{
+const testGenericData *data = opaque;
+g_autoptr(qemuMonitorTest) test = NULL;
+virCPUDefPtr cpu_a;
+virCPUDefPtr cpu_b = NULL;
+qemuMonitorCPUModelInfoPtr baseline = NULL;
+int ret = -1;
+
+if (!(test = qemuMonitorTestNewSchema(data->xmlopt, data->schema)))
+return -1;
+
+if (qemuMonitorTestAddItem(test, "query-cpu-model-baseline",
+   "{ "
+   "   \"return\": { "
+   "   \"model\": { "
+   "   \"name\": \"cpu_c\", "
+   "   \"props\": { "
+   "\"feat_a\": true, "
+   "\"feat_b\": false "
+   "} "
+   "} "
+   "} "
+   "}") < 0)
+return -1;
+
+if (VIR_ALLOC(cpu_a) < 0 || VIR_ALLOC(cpu_b) < 0)
+goto cleanup;
+
+cpu_a->model = g_strdup("cpu_a");
+cpu_b->model = g_strdup("cpu_b");
+
+if (virCPUDefAddFeature(cpu_a, "feat_a", VIR_CPU_FEATURE_REQUIRE) < 0 ||
+virCPUDefAddFeature(cpu_a, "feat_b", VIR_CPU_FEATURE_REQUIRE) < 0 ||
+virCPUDefAddFeature(cpu_a, "feat_c", VIR_CPU_FEATURE_REQUIRE) < 0)
+goto cleanup;
+
+if (qemuMonitorJSONGetCPUModelBaseline(qemuMonitorTestGetMonitor(test),
+   cpu_a, cpu_b, ) < 0)
+goto cleanup;
+
+if (!baseline) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   "Baseline missing result");
+goto cleanup;
+}
+if (!baseline->name) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   "Baseline missing model name");
+goto cleanup;
+}
+if (baseline->nprops != 2) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   "Baseline missing properties");
+goto cleanup;
+}
+if (STRNEQ(baseline->props[0].name, "feat_a") ||
+!baseline->props[0].value.boolean ||
+STRNEQ(baseline->props[1].name, "feat_b") ||
+baseline->props[1].value.boolean) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   "Baseline property error");
+goto cleanup;
+}
+
+ret = 0;
+
+ cleanup:
+virCPUDefFree(cpu_a);
+virCPUDefFree(cpu_b);
+qemuMonitorCPUModelInfoFree(baseline);
+return ret;
+}
+
+
+static int
 mymain(void)
 {
 int ret = 0;
@@ -3306,6 +3382,7 @@ mymain(void)
 }
 
 DO_TEST(qemuMonitorJSONGetCPUModelComparison);
+DO_TEST(qemuMonitorJSONGetCPUModelBaseline);
 
  cleanup:
 VIR_FREE(metaschemastr);
-- 
2.7.4




[PATCH v1 0/3] qemumonitorjson tests for cpu compare and baseline

2020-02-20 Thread Collin Walling
These patches implement tests for the libvirt qemu_monitor_json API for
the hypervisor-cpu-compare and -baseline commands. The input and output
data is mocked with arbitrary values.

A prerequisite patch is included to load the capabilities schema for
a specific architecture. Originally, only the x86 capabilities were
loaded for the qemu_monitor_json tests. By accepting a string denoting
which architecture's QEMU capabilities we'd like to load, we can now
test the comparison and baseline code that is currently only supported
on s390.

Collin Walling (3):
  qemumonitorjsontest: load schema based on specified arch
  qemumonitorjsontest: add tests for cpu comparison
  qemumonitorjsontest: add test for cpu baseline

 tests/qemublocktest.c   |   2 +-
 tests/qemuhotplugtest.c |   2 +-
 tests/qemumonitorjsontest.c | 131 +++-
 tests/testutilsqemuschema.c |   8 +--
 tests/testutilsqemuschema.h |   4 +-
 5 files changed, 137 insertions(+), 10 deletions(-)

-- 
2.7.4




[PATCH v1 1/3] qemumonitorjsontest: load schema based on specified arch

2020-02-20 Thread Collin Walling
There are some architectures that support capabilities that others
do not (e.g. s390x supports cpu comparison and baseline via QEMU).

Let's make testQEMUSchemaLoad accept a string to specify the schema
to load based on the specified arch.

Signed-off-by: Collin Walling 
---
 tests/qemublocktest.c   | 2 +-
 tests/qemuhotplugtest.c | 2 +-
 tests/qemumonitorjsontest.c | 4 ++--
 tests/testutilsqemuschema.c | 8 
 tests/testutilsqemuschema.h | 4 ++--
 5 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 5a564b7..7b7948d 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -981,7 +981,7 @@ mymain(void)
 
 #define TEST_DISK_TO_JSON(nme) TEST_DISK_TO_JSON_FULL(nme, false)
 
-if (!(diskxmljsondata.schema = testQEMUSchemaLoad())) {
+if (!(diskxmljsondata.schema = testQEMUSchemaLoad("x86_64"))) {
 ret = -1;
 goto cleanup;
 }
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 6a3e61c..9dd4266 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -620,7 +620,7 @@ mymain(void)
 if (!(driver.domainEventState = virObjectEventStateNew()))
 return EXIT_FAILURE;
 
-if (!(qmpschema = testQEMUSchemaLoad())) {
+if (!(qmpschema = testQEMUSchemaLoad("x86_64"))) {
 VIR_TEST_VERBOSE("failed to load qapi schema\n");
 return EXIT_FAILURE;
 }
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 24e0ce1..cd7fa1f 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -2995,7 +2995,7 @@ mymain(void)
 
 virEventRegisterDefaultImpl();
 
-if (!(qapiData.schema = testQEMUSchemaLoad())) {
+if (!(qapiData.schema = testQEMUSchemaLoad("x86_64"))) {
 VIR_TEST_VERBOSE("failed to load qapi schema");
 ret = -1;
 goto cleanup;
@@ -3232,7 +3232,7 @@ mymain(void)
 DO_TEST_QAPI_VALIDATE("alternate 2", "blockdev-add/arg-type", false,
   "{\"driver\":\"qcow2\",\"file\": 1234}");
 
-if (!(metaschema = testQEMUSchemaGetLatest()) ||
+if (!(metaschema = testQEMUSchemaGetLatest("x86_64")) ||
 !(metaschemastr = virJSONValueToString(metaschema, false))) {
 VIR_TEST_VERBOSE("failed to load latest qapi schema");
 ret = -1;
diff --git a/tests/testutilsqemuschema.c b/tests/testutilsqemuschema.c
index 9a1b1fe..7b82ff2 100644
--- a/tests/testutilsqemuschema.c
+++ b/tests/testutilsqemuschema.c
@@ -524,7 +524,7 @@ testQEMUSchemaValidate(virJSONValuePtr obj,
  * replies file used for qemucapabilitiestest for the x86_64 architecture.
  */
 virJSONValuePtr
-testQEMUSchemaGetLatest(void)
+testQEMUSchemaGetLatest(const char* arch)
 {
 char *capsLatestFile = NULL;
 char *capsLatest = NULL;
@@ -533,7 +533,7 @@ testQEMUSchemaGetLatest(void)
 virJSONValuePtr reply = NULL;
 virJSONValuePtr schema = NULL;
 
-if (!(capsLatestFile = testQemuGetLatestCapsForArch("x86_64", "replies"))) 
{
+if (!(capsLatestFile = testQemuGetLatestCapsForArch(arch, "replies"))) {
 VIR_TEST_VERBOSE("failed to find latest caps replies");
 return NULL;
 }
@@ -575,11 +575,11 @@ testQEMUSchemaGetLatest(void)
 
 
 virHashTablePtr
-testQEMUSchemaLoad(void)
+testQEMUSchemaLoad(const char* arch)
 {
 virJSONValuePtr schema;
 
-if (!(schema = testQEMUSchemaGetLatest()))
+if (!(schema = testQEMUSchemaGetLatest(arch)))
 return NULL;
 
 return virQEMUQAPISchemaConvert(schema);
diff --git a/tests/testutilsqemuschema.h b/tests/testutilsqemuschema.h
index de9da7c..84ee9a9 100644
--- a/tests/testutilsqemuschema.h
+++ b/tests/testutilsqemuschema.h
@@ -29,7 +29,7 @@ testQEMUSchemaValidate(virJSONValuePtr obj,
virBufferPtr debug);
 
 virJSONValuePtr
-testQEMUSchemaGetLatest(void);
+testQEMUSchemaGetLatest(const char* arch);
 
 virHashTablePtr
-testQEMUSchemaLoad(void);
+testQEMUSchemaLoad(const char* arch);
-- 
2.7.4




Re: [libvirt PATCH] m4: libxl: properly fail when libxl is required

2020-02-20 Thread Andrea Bolognani
On Thu, 2020-02-20 at 16:05 +0100, Ján Tomko wrote:
> We specify "true" as the fail-action for LIBVIRT_CHECK_PKG.
> 
> This was used when we had a fallback to non-pkg-config detection,
> then removed in commit 5bdcef13d13560512c7d6d8c9e8822e456889e0c
> later re-introduced in commit dc3d2c9f8c7678a950abedd227b1587ca62335c4
> and then left in when removing the old detection again in
> commit 18981877d2e20390a79d068861a24e716f8ee422
> 
> Remove it to properly error out when libxl was requested but not
> detected.
> 
> Signed-off-by: Ján Tomko 
> Fixes: 18981877d2e20390a79d068861a24e716f8ee422
> ---
>  m4/virt-driver-libxl.m4 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Andrea Bolognani 

-- 
Andrea Bolognani / Red Hat / Virtualization



Re: [PATCH] storage: Add support to set{uid,gid} and sticky bit

2020-02-20 Thread Ján Tomko

On Wed, Feb 19, 2020 at 05:51:44PM -0300, Julio Faracco wrote:

This commit add more features to storages that supports setuid, setgid
and sticky bit. This extend some permission levels of volumes when you
run an hypervisor using a specific user that can run but cannot delete
volumes for instance.


I'm confused about the use case here - volumes should not be executable
and setuid/setgid only make sense on executable files.


Additionally, when you create a directory without
`pool-build` command, you cannot import those extra permissions.
Example:

   # mkdir /var/lib/libvirt/images/
   # chmod 0755 /var/lib/libvirt/images/
   # chmod u+s /var/lib/libvirt/images/
   # pool-start default
   # pool-dumpxml default

No setuid from `0755`.
Output should expect `4755`.



FYI I proposed a similar patch ~7.5 years ago (and still haven't
bothered to resend it O:-)):
https://www.redhat.com/archives/libvir-list/2012-August/msg00687.html
https://www.redhat.com/archives/libvir-list/2012-August/msg01004.html

The consensus seemed to be
* not wanting to touch the SGID/SUID bits
* reporting the perms should be OK

For regular files, these bits seem to be useless for volumes, I think
we should reject them.
For directories, SGID and sticky might make sense


Signed-off-by: Julio Faracco 
---
src/conf/storage_conf.c| 11 ---
src/storage/storage_util.c | 12 
2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 252d28cbfb..54e4a60ded 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -746,7 +746,7 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt,
if ((mode = virXPathString("string(./mode)", ctxt))) {
int tmp;

-if (virStrToLong_i(mode, NULL, 8, ) < 0 || (tmp & ~0777)) {
+if (virStrToLong_i(mode, NULL, 8, ) < 0 || (tmp & ~0)) {
virReportError(VIR_ERR_XML_ERROR, "%s",
   _("malformed octal mode"));
goto error;


To loosen this check, I'd rather see a new check added in the callers of
this function, to make sure we won't allow creating a suid volume.


@@ -1187,9 +1187,14 @@ virStoragePoolDefFormatBuf(virBufferPtr buf,
def->target.perms.label) {
virBufferAddLit(buf, "\n");
virBufferAdjustIndent(buf, 2);
-if (def->target.perms.mode != (mode_t) -1)
-virBufferAsprintf(buf, "0%o\n",
+if (def->target.perms.mode != (mode_t) -1) {
+if (def->target.perms.mode & (S_ISUID | S_ISGID | S_ISVTX))
+virBufferAsprintf(buf, "%4o\n",


Wouldn't this print it without the leading zero?


  def->target.perms.mode);
+else
+virBufferAsprintf(buf, "0%o\n",
+  def->target.perms.mode);
+}
if (def->target.perms.uid != (uid_t) -1)
virBufferAsprintf(buf, "%d\n",
  (int) def->target.perms.uid);
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index c2754dbb93..5352ab9120 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -82,6 +82,10 @@ VIR_LOG_INIT("storage.storage_util");
# define S_IRWXUGO (S_IRWXU | S_IRWXG | S_IRWXO)
#endif

+#ifndef S_IALLUGO
+# define S_IALLUGO (S_ISUID | S_ISGID | S_ISVTX | S_IRWXUGO)
+#endif
+
/* virStorageBackendNamespaceInit:
 * @poolType: virStoragePoolType
 * @xmlns: Storage Pool specific namespace callback methods
@@ -512,7 +516,7 @@ virStorageBackendCreateExecCommand(virStoragePoolObjPtr 
pool,

virCommandSetUID(cmd, vol->target.perms->uid);
virCommandSetGID(cmd, vol->target.perms->gid);
-virCommandSetUmask(cmd, S_IRWXUGO ^ mode);
+virCommandSetUmask(cmd, S_IALLUGO ^ mode);

if (virCommandRun(cmd, NULL) == 0) {
/* command was successfully run, check if the file was created */
@@ -523,7 +527,7 @@ virStorageBackendCreateExecCommand(virStoragePoolObjPtr 
pool,
 * If that doesn't match what we expect, then let's try to
 * re-open the file and attempt to force the mode change.
 */
-if (mode != (st.st_mode & S_IRWXUGO)) {
+if (mode != (st.st_mode & S_IALLUGO)) {
VIR_AUTOCLOSE fd = -1;
int flags = VIR_FILE_OPEN_FORK | VIR_FILE_OPEN_FORCE_MODE;

@@ -569,7 +573,7 @@ virStorageBackendCreateExecCommand(virStoragePoolObjPtr 
pool,
goto cleanup;
}

-if (mode != (st.st_mode & S_IRWXUGO) &&
+if (mode != (st.st_mode & S_IALLUGO) &&
chmod(vol->target.path, mode) < 0) {
virReportSystemError(errno,
 _("cannot set mode of '%s' to %04o"),


The checks above are for volume creation and IMO should stay.

Jano


@@ -1825,7 +1829,7 @@ 
virStorageBackendUpdateVolTargetInfoFD(virStorageSourcePtr target,

 

[libvirt PATCH] m4: libxl: properly fail when libxl is required

2020-02-20 Thread Ján Tomko
We specify "true" as the fail-action for LIBVIRT_CHECK_PKG.

This was used when we had a fallback to non-pkg-config detection,
then removed in commit 5bdcef13d13560512c7d6d8c9e8822e456889e0c
later re-introduced in commit dc3d2c9f8c7678a950abedd227b1587ca62335c4
and then left in when removing the old detection again in
commit 18981877d2e20390a79d068861a24e716f8ee422

Remove it to properly error out when libxl was requested but not
detected.

Signed-off-by: Ján Tomko 
Fixes: 18981877d2e20390a79d068861a24e716f8ee422
---
 m4/virt-driver-libxl.m4 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/m4/virt-driver-libxl.m4 b/m4/virt-driver-libxl.m4
index 2f3565f1d5..a958cb26fa 100644
--- a/m4/virt-driver-libxl.m4
+++ b/m4/virt-driver-libxl.m4
@@ -30,7 +30,7 @@ AC_DEFUN([LIBVIRT_DRIVER_CHECK_LIBXL], [
 
   dnl search for libxl, aka libxenlight
   old_with_libxl="$with_libxl"
-  LIBVIRT_CHECK_PKG([LIBXL], [xenlight], [4.6.0], [true])
+  LIBVIRT_CHECK_PKG([LIBXL], [xenlight], [4.6.0])
   if test "x$with_libxl" = "xyes" ; then
 LIBXL_FIRMWARE_DIR=$($PKG_CONFIG --variable xenfirmwaredir xenlight)
 LIBXL_EXECBIN_DIR=$($PKG_CONFIG --variable libexec_bin xenlight)
-- 
2.24.1



[libvirt PATCHv4 01/15] docs: reduce excessive spacing in ToC for RST files

2020-02-20 Thread Ján Tomko
From: Daniel P. Berrangé 

The table of contents in the RST based files uses  tags inside the
, which results in 1em's worth of spacing above & below each
entry. This results in way too much whitespace in the ToC.

Signed-off-by: Daniel P. Berrangé 
Reviewed-by: Ján Tomko 
---
 docs/libvirt.css | 4 
 1 file changed, 4 insertions(+)

diff --git a/docs/libvirt.css b/docs/libvirt.css
index 2fe123395c..18e55dac59 100644
--- a/docs/libvirt.css
+++ b/docs/libvirt.css
@@ -579,3 +579,7 @@ ul.news-section-content li dl dd {
 font-family: monospace;
 background: #ee;
 }
+
+.contents li p {
+margin: 2px;
+}
-- 
2.24.1



[libvirt PATCHv4 06/15] conf: qemu: add virtiofs fsdriver type

2020-02-20 Thread Ján Tomko
Introduce a new 'virtiofs' driver type for filesystem.


  
  
  
  


Signed-off-by: Ján Tomko 
Reviewed-by: Daniel P. Berrangé 
---
 docs/formatdomain.html.in | 12 ++-
 docs/schemas/domaincommon.rng |  6 ++
 src/conf/domain_conf.c|  1 +
 src/conf/domain_conf.h|  1 +
 src/qemu/qemu_command.c   |  4 +
 src/qemu/qemu_domain.c|  4 +
 src/qemu/qemu_domain_address.c|  4 +
 .../vhost-user-fs-fd-memory.xml   | 39 ++
 .../vhost-user-fs-hugepages.xml   | 74 +++
 .../vhost-user-fs-fd-memory.x86_64-latest.xml |  1 +
 .../vhost-user-fs-hugepages.x86_64-latest.xml |  1 +
 tests/qemuxml2xmltest.c   |  3 +
 12 files changed, 149 insertions(+), 1 deletion(-)
 create mode 100644 tests/qemuxml2argvdata/vhost-user-fs-fd-memory.xml
 create mode 100644 tests/qemuxml2argvdata/vhost-user-fs-hugepages.xml
 create mode 12 
tests/qemuxml2xmloutdata/vhost-user-fs-fd-memory.x86_64-latest.xml
 create mode 12 
tests/qemuxml2xmloutdata/vhost-user-fs-hugepages.x86_64-latest.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index f4af65f13f..dab8fb8f6b 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3935,6 +3935,11 @@
 target dir='/import/from/host'/
 readonly/
   /filesystem
+  filesystem type='mount' accessmode='passthrough'
+  driver type='virtiofs'/
+  source dir='/path'/
+  target dir='mount_tag'/
+  /filesystem
   ...
 /devices
 ...
@@ -3963,6 +3968,9 @@
 while the value immediate means that a host writeback
 is immediately triggered for all pages touched during a guest file
 write operation (since 0.9.10).
+Since 6.1.0, type='virtiofs'
+is also supported. Using virtiofs requires setting up shared memory,
+see the guide: Virtio-FS
 
 template
 
@@ -3998,7 +4006,9 @@
   The filesystem element has an optional attribute accessmode
   which specifies the security mode for accessing the source
   (since 0.8.5). Currently this only works
-  with type='mount' for the QEMU/KVM driver. The possible
+  with type='mount' for the QEMU/KVM driver.
+  For driver type virtiofs, only passthrough is
+  supported. For other driver types, the possible
   values are:
 
 
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 5b609a4756..f7125e401a 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2645,6 +2645,12 @@
   
   
 
+
+  
+virtiofs
+  
+  
+
 
   
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index dcd070d2ad..d78ea92ead 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -477,6 +477,7 @@ VIR_ENUM_IMPL(virDomainFSDriver,
   "loop",
   "nbd",
   "ploop",
+  "virtiofs",
 );
 
 VIR_ENUM_IMPL(virDomainFSAccessMode,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 867a9c7661..0ef1746e2a 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -773,6 +773,7 @@ typedef enum {
 VIR_DOMAIN_FS_DRIVER_TYPE_LOOP,
 VIR_DOMAIN_FS_DRIVER_TYPE_NBD,
 VIR_DOMAIN_FS_DRIVER_TYPE_PLOOP,
+VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS,
 
 VIR_DOMAIN_FS_DRIVER_TYPE_LAST
 } virDomainFSDriverType;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f69a9e651c..9fcd06f8c3 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2680,6 +2680,10 @@ qemuBuildFilesystemCommandLine(virCommandPtr cmd,
 return -1;
 break;
 
+case VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS:
+/* TODO: vhost-user-fs-pci */
+break;
+
 case VIR_DOMAIN_FS_DRIVER_TYPE_LOOP:
 case VIR_DOMAIN_FS_DRIVER_TYPE_NBD:
 case VIR_DOMAIN_FS_DRIVER_TYPE_PLOOP:
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index af6817cc05..c3fc3fed1c 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8298,6 +8298,10 @@ qemuDomainDeviceDefValidateFS(virDomainFSDefPtr fs,
_("Filesystem driver type not supported"));
 return -1;
 
+case VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS:
+/* TODO: vhost-user-fs-pci */
+return 0;
+
 case VIR_DOMAIN_FS_DRIVER_TYPE_LAST:
 default:
 virReportEnumRangeError(virDomainFSDriverType, fs->fsdriver);
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index c7d8a3ac3b..ab6bce19f4 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -699,6 +699,10 @@ 
qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
 }
 break;
 
+case 

[libvirt PATCHv4 08/15] qemu: add virtiofsd_debug to qemu.conf

2020-02-20 Thread Ján Tomko
Add a 'virtiofsd_debug' option for tuning whether to run virtiofsd
in debug mode.

Signed-off-by: Ján Tomko 
Reviewed-by: Daniel P. Berrangé 
---
 src/qemu/libvirtd_qemu.aug | 1 +
 src/qemu/qemu.conf | 7 +++
 src/qemu/qemu_conf.c   | 2 ++
 src/qemu/qemu_conf.h   | 1 +
 src/qemu/test_libvirtd_qemu.aug.in | 1 +
 5 files changed, 12 insertions(+)

diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index 557b6f38f8..3014fa6b86 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -116,6 +116,7 @@ module Libvirtd_qemu =
let nvram_entry = str_array_entry "nvram"
 
let debug_level_entry = int_entry "gluster_debug_level"
+ | bool_entry "virtiofsd_debug"
 
let memory_entry = str_entry "memory_backing_dir"
 
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index b6805ffc41..e82c1b5bd5 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -809,6 +809,13 @@
 #
 #gluster_debug_level = 9
 
+# virtiofsd debug
+#
+# Whether to enable the debugging output of the virtiofsd daemon.
+# Possible values are 0 or 1.
+#
+#virtiofsd_debug = 1
+
 # To enhance security, QEMU driver is capable of creating private namespaces
 # for each domain started. Well, so far only "mount" namespace is supported. If
 # enabled it means qemu process is unable to see all the devices on the system,
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 0357501dc6..08c2fffce3 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -835,6 +835,8 @@ virQEMUDriverConfigLoadDebugEntry(virQEMUDriverConfigPtr 
cfg,
 {
 if (virConfGetValueUInt(conf, "gluster_debug_level", 
>glusterDebugLevel) < 0)
 return -1;
+if (virConfGetValueBool(conf, "virtiofsd_debug", >virtiofsdDebug) < 0)
+return -1;
 
 return 0;
 }
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index cedf232223..3ce9566b71 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -202,6 +202,7 @@ struct _virQEMUDriverConfig {
 virFirmwarePtr *firmwares;
 size_t nfirmwares;
 unsigned int glusterDebugLevel;
+bool virtiofsdDebug;
 
 char *memoryBackingDir;
 
diff --git a/src/qemu/test_libvirtd_qemu.aug.in 
b/src/qemu/test_libvirtd_qemu.aug.in
index dd90edf687..fca9a942c9 100644
--- a/src/qemu/test_libvirtd_qemu.aug.in
+++ b/src/qemu/test_libvirtd_qemu.aug.in
@@ -98,6 +98,7 @@ module Test_libvirtd_qemu =
 }
 { "stdio_handler" = "logd" }
 { "gluster_debug_level" = "9" }
+{ "virtiofsd_debug" = "1" }
 { "namespaces"
 { "1" = "mount" }
 }
-- 
2.24.1



[libvirt PATCHv4 09/15] qemu: validate virtiofs filesystems

2020-02-20 Thread Ján Tomko
Reject unsupported configurations.

Signed-off-by: Ján Tomko 
---
 src/qemu/qemu_domain.c | 61 +++---
 1 file changed, 58 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index c3fc3fed1c..7cb283123d 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8266,10 +8266,44 @@ qemuDomainDeviceDefValidateIOMMU(const 
virDomainIOMMUDef *iommu,
 return 0;
 }
 
+static int
+qemuDomainDefValidateVirtioFSSharedMemory(const virDomainDef *def)
+{
+size_t numa_nodes = virDomainNumaGetNodeCount(def->numa);
+size_t i;
+
+for (i = 0; i < numa_nodes; i++) {
+virDomainMemoryAccess node_access =
+virDomainNumaGetNodeMemoryAccessMode(def->numa, i);
+
+switch (node_access) {
+case VIR_DOMAIN_MEMORY_ACCESS_DEFAULT:
+if (def->mem.access != VIR_DOMAIN_MEMORY_ACCESS_SHARED) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("virtiofs requires shared memory"));
+return -1;
+}
+break;
+case VIR_DOMAIN_MEMORY_ACCESS_SHARED:
+break;
+case VIR_DOMAIN_MEMORY_ACCESS_PRIVATE:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("virtiofs requires shared memory"));
+return -1;
+
+case VIR_DOMAIN_MEMORY_ACCESS_LAST:
+default:
+virReportEnumRangeError(virDomainMemoryAccess, node_access);
+return -1;
+
+}
+}
+return 0;
+}
 
 static int
 qemuDomainDeviceDefValidateFS(virDomainFSDefPtr fs,
-  const virDomainDef *def G_GNUC_UNUSED,
+  const virDomainDef *def,
   virQEMUCapsPtr qemuCaps G_GNUC_UNUSED)
 {
 if (fs->type != VIR_DOMAIN_FS_TYPE_MOUNT) {
@@ -8299,8 +8333,29 @@ qemuDomainDeviceDefValidateFS(virDomainFSDefPtr fs,
 return -1;
 
 case VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS:
-/* TODO: vhost-user-fs-pci */
-return 0;
+if (fs->accessmode != VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("virtiofs only supports passthrough accessmode"));
+return -1;
+}
+if (fs->wrpolicy != VIR_DOMAIN_FS_WRPOLICY_DEFAULT) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("virtiofs does not support wrpolicy"));
+return -1;
+}
+if (fs->model != VIR_DOMAIN_FS_MODEL_DEFAULT) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("virtiofs does not support model"));
+return -1;
+}
+if (fs->format != VIR_STORAGE_FILE_NONE) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("virtiofs does not support format"));
+return -1;
+}
+if (qemuDomainDefValidateVirtioFSSharedMemory(def) < 0)
+return -1;
+break;
 
 case VIR_DOMAIN_FS_DRIVER_TYPE_LAST:
 default:
-- 
2.24.1



[libvirt PATCHv4 10/15] qemu: forbid migration with vhost-user-fs device

2020-02-20 Thread Ján Tomko
This is not yet supported.

Signed-off-by: Ján Tomko 
Reviewed-by: Daniel P. Berrangé 
---
 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 a307c5ebe2..2624a7388c 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1280,6 +1280,16 @@ qemuMigrationSrcIsAllowed(virQEMUDriverPtr driver,
 return false;
 }
 }
+
+for (i = 0; i < vm->def->nfss; i++) {
+virDomainFSDefPtr fs = vm->def->fss[i];
+
+if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("migration with virtiofs device is not 
supported"));
+return false;
+}
+}
 }
 
 return true;
-- 
2.24.1



[libvirt PATCHv4 13/15] qemu: use the vhost-user schemas to find binary

2020-02-20 Thread Ján Tomko
Look into /usr/share/qemu/vhost-user to see whether we can find
a suitable virtiofsd binary, in case the user did not provide one
in the domain XML.

Signed-off-by: Ján Tomko 
Reviewed-by: Daniel P. Berrangé 
---
 src/qemu/qemu_extdevice.c  |  9 +
 src/qemu/qemu_vhost_user.c | 40 ++
 src/qemu/qemu_vhost_user.h |  4 
 src/qemu/qemu_virtiofs.c   | 11 +++
 src/qemu/qemu_virtiofs.h   |  4 
 5 files changed, 68 insertions(+)

diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c
index 31655b7f0a..699b4daade 100644
--- a/src/qemu/qemu_extdevice.c
+++ b/src/qemu/qemu_extdevice.c
@@ -105,6 +105,15 @@ qemuExtDevicesPrepareDomain(virQEMUDriverPtr driver,
 }
 }
 
+for (i = 0; i < vm->def->nfss; i++) {
+virDomainFSDefPtr fs = vm->def->fss[i];
+
+if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS) {
+if (qemuVirtioFSPrepareDomain(driver, fs) < 0)
+return -1;
+}
+}
+
 return ret;
 }
 
diff --git a/src/qemu/qemu_vhost_user.c b/src/qemu/qemu_vhost_user.c
index 4c25b30664..d437fd1bd5 100644
--- a/src/qemu/qemu_vhost_user.c
+++ b/src/qemu/qemu_vhost_user.c
@@ -416,3 +416,43 @@ qemuVhostUserFillDomainGPU(virQEMUDriverPtr driver,
 VIR_FREE(vus);
 return ret;
 }
+
+
+int
+qemuVhostUserFillDomainFS(virQEMUDriverPtr driver,
+  virDomainFSDefPtr fs)
+{
+qemuVhostUserPtr *vus = NULL;
+ssize_t nvus = 0;
+ssize_t i;
+int ret = -1;
+
+if ((nvus = qemuVhostUserFetchParsedConfigs(driver->privileged,
+, NULL)) < 0)
+goto end;
+
+for (i = 0; i < nvus; i++) {
+qemuVhostUserPtr vu = vus[i];
+
+if (vu->type != QEMU_VHOST_USER_TYPE_FS)
+continue;
+
+g_free(fs->binary);
+fs->binary = g_strdup(vu->binary);
+break;
+}
+
+if (i == nvus) {
+virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+   _("Unable to find a satisfying virtiofsd"));
+goto end;
+}
+
+ret = 0;
+
+ end:
+for (i = 0; i < nvus; i++)
+qemuVhostUserFree(vus[i]);
+g_free(vus);
+return ret;
+}
diff --git a/src/qemu/qemu_vhost_user.h b/src/qemu/qemu_vhost_user.h
index 369ba00caa..e505c8a473 100644
--- a/src/qemu/qemu_vhost_user.h
+++ b/src/qemu/qemu_vhost_user.h
@@ -45,3 +45,7 @@ qemuVhostUserFetchConfigs(char ***configs,
 int
 qemuVhostUserFillDomainGPU(virQEMUDriverPtr driver,
virDomainVideoDefPtr video);
+
+int
+qemuVhostUserFillDomainFS(virQEMUDriverPtr driver,
+  virDomainFSDefPtr fs);
diff --git a/src/qemu/qemu_virtiofs.c b/src/qemu/qemu_virtiofs.c
index 9e354a30c6..b53e5b0806 100644
--- a/src/qemu/qemu_virtiofs.c
+++ b/src/qemu/qemu_virtiofs.c
@@ -28,6 +28,7 @@
 #include "qemu_conf.h"
 #include "qemu_extdevice.h"
 #include "qemu_security.h"
+#include "qemu_vhost_user.h"
 #include "qemu_virtiofs.h"
 #include "virpidfile.h"
 
@@ -326,3 +327,13 @@ qemuVirtioFSSetupCgroup(virQEMUDriverPtr driver,
 
 return 0;
 }
+
+int
+qemuVirtioFSPrepareDomain(virQEMUDriverPtr driver,
+  virDomainFSDefPtr fs)
+{
+if (fs->binary)
+return 0;
+
+return qemuVhostUserFillDomainFS(driver, fs);
+}
diff --git a/src/qemu/qemu_virtiofs.h b/src/qemu/qemu_virtiofs.h
index 341fed1def..b70b2f1b6a 100644
--- a/src/qemu/qemu_virtiofs.h
+++ b/src/qemu/qemu_virtiofs.h
@@ -42,3 +42,7 @@ qemuVirtioFSSetupCgroup(virQEMUDriverPtr driver,
 virDomainDefPtr def,
 virDomainFSDefPtr fs,
 virCgroupPtr cgroup);
+
+int
+qemuVirtioFSPrepareDomain(virQEMUDriverPtr driver,
+  virDomainFSDefPtr fs);
-- 
2.24.1



[libvirt PATCHv4 03/15] qemuExtDevicesStart: pass logManager

2020-02-20 Thread Ján Tomko
Pass logManager to qemuExtDevicesStart for future usage.

Signed-off-by: Ján Tomko 
Reviewed-by: Daniel P. Berrangé 
---
 src/qemu/qemu_extdevice.c | 1 +
 src/qemu/qemu_extdevice.h | 1 +
 src/qemu/qemu_process.c   | 4 +++-
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c
index 9c0c0fd573..7f3bb104d9 100644
--- a/src/qemu/qemu_extdevice.c
+++ b/src/qemu/qemu_extdevice.c
@@ -153,6 +153,7 @@ qemuExtDevicesCleanupHost(virQEMUDriverPtr driver,
 int
 qemuExtDevicesStart(virQEMUDriverPtr driver,
 virDomainObjPtr vm,
+virLogManagerPtr logManager G_GNUC_UNUSED,
 bool incomingMigration)
 {
 virDomainDefPtr def = vm->def;
diff --git a/src/qemu/qemu_extdevice.h b/src/qemu/qemu_extdevice.h
index 5cf777ab4b..df29968e16 100644
--- a/src/qemu/qemu_extdevice.h
+++ b/src/qemu/qemu_extdevice.h
@@ -46,6 +46,7 @@ void qemuExtDevicesCleanupHost(virQEMUDriverPtr driver,
 
 int qemuExtDevicesStart(virQEMUDriverPtr driver,
 virDomainObjPtr vm,
+virLogManagerPtr logManager,
 bool incomingMigration)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
 G_GNUC_WARN_UNUSED_RESULT;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 8c1ed76677..5d24dc1645 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6734,7 +6734,9 @@ qemuProcessLaunch(virConnectPtr conn,
 if (qemuProcessGenID(vm, flags) < 0)
 goto cleanup;
 
-if (qemuExtDevicesStart(driver, vm, incoming != NULL) < 0)
+if (qemuExtDevicesStart(driver, vm,
+qemuDomainLogContextGetManager(logCtxt),
+incoming != NULL) < 0)
 goto cleanup;
 
 VIR_DEBUG("Building emulator command line");
-- 
2.24.1



[libvirt PATCHv4 15/15] wip: SELinux integration for virtiofsd

2020-02-20 Thread Ján Tomko
This patch attempts to run virtiofsd with libvirtd's label and the
category of the currently ran domain, which obviously does nothing
without a corresponding policy.

Per:
https://www.redhat.com/archives/libvir-list/2020-February/msg00108.html
a new type might be needed.

TODO: file a SELinux policy bug
---
 src/libvirt_private.syms|  1 +
 src/qemu/qemu_security.c| 40 +++
 src/qemu/qemu_security.h|  7 
 src/qemu/qemu_virtiofs.c|  4 +-
 src/security/security_dac.c | 20 ++
 src/security/security_driver.h  |  2 +
 src/security/security_manager.c | 12 ++
 src/security/security_manager.h |  4 ++
 src/security/security_nop.c |  1 +
 src/security/security_selinux.c | 69 +
 src/security/security_stack.c   | 19 +
 11 files changed, 178 insertions(+), 1 deletion(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 49eb8ab7cb..aaba15297a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1504,6 +1504,7 @@ virSecurityManagerSetSavedStateLabel;
 virSecurityManagerSetSocketLabel;
 virSecurityManagerSetTapFDLabel;
 virSecurityManagerSetTPMLabels;
+virSecurityManagerSetVirtioFSProcessLabel;
 virSecurityManagerStackAddNested;
 virSecurityManagerTransactionAbort;
 virSecurityManagerTransactionCommit;
diff --git a/src/qemu/qemu_security.c b/src/qemu/qemu_security.c
index 2aa2b5b9c6..834556c910 100644
--- a/src/qemu/qemu_security.c
+++ b/src/qemu/qemu_security.c
@@ -475,6 +475,46 @@ qemuSecurityStartVhostUserGPU(virQEMUDriverPtr driver,
 }
 
 
+/*
+ * qemuSecurityStartVhostUserGPU:
+ *
+ * @driver: the QEMU driver
+ * @vm: the domain object
+ * @cmd: the command to run
+ * @existstatus: pointer to int returning exit status of process
+ * @cmdret: pointer to int returning result of virCommandRun
+ *
+ * Start the vhost-user-gpu process with approriate labels.
+ * This function returns -1 on security setup error, 0 if all the
+ * setup was done properly. In case the virCommand failed to run
+ * 0 is returned but cmdret is set appropriately with the process
+ * exitstatus also set.
+ */
+int
+qemuSecurityStartVirtioFS(virQEMUDriverPtr driver,
+  virDomainObjPtr vm,
+  virCommandPtr cmd,
+  int *exitstatus,
+  int *cmdret)
+{
+if (virSecurityManagerSetVirtioFSProcessLabel(driver->securityManager,
+  vm->def, cmd) < 0)
+return -1;
+
+if (virSecurityManagerPreFork(driver->securityManager) < 0)
+return -1;
+
+*cmdret = virCommandRun(cmd, exitstatus);
+
+virSecurityManagerPostFork(driver->securityManager);
+
+if (*cmdret < 0)
+return -1;
+
+return 0;
+}
+
+
 /*
  * qemuSecurityStartTPMEmulator:
  *
diff --git a/src/qemu/qemu_security.h b/src/qemu/qemu_security.h
index a8c648ece1..dbb3bdbc9b 100644
--- a/src/qemu/qemu_security.h
+++ b/src/qemu/qemu_security.h
@@ -84,6 +84,13 @@ int qemuSecurityStartVhostUserGPU(virQEMUDriverPtr driver,
   int *exitstatus,
   int *cmdret);
 
+int qemuSecurityStartVirtioFS(virQEMUDriverPtr driver,
+  virDomainObjPtr vm,
+  virCommandPtr cmd,
+  int *exitstatus,
+  int *cmdret);
+
+
 int qemuSecurityStartTPMEmulator(virQEMUDriverPtr driver,
  virDomainObjPtr vm,
  virCommandPtr cmd,
diff --git a/src/qemu/qemu_virtiofs.c b/src/qemu/qemu_virtiofs.c
index b53e5b0806..17afe27535 100644
--- a/src/qemu/qemu_virtiofs.c
+++ b/src/qemu/qemu_virtiofs.c
@@ -34,6 +34,7 @@
 
 #define VIR_FROM_THIS VIR_FROM_QEMU
 
+VIR_LOG_INIT("qemu.virtiofs");
 
 char *
 qemuVirtioFSCreatePidFilename(virQEMUDriverConfigPtr cfg,
@@ -227,7 +228,8 @@ qemuVirtioFSStart(virLogManagerPtr logManager,
 if (qemuExtDeviceLogCommand(driver, vm, cmd, "virtiofsd") < 0)
 goto cleanup;
 
-rc = virCommandRun(cmd, NULL);
+if (qemuSecurityStartVirtioFS(driver, vm, cmd, NULL, ) < 0)
+goto error;
 
 if (rc < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index d75b18170b..64e00b0127 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -2263,6 +2263,24 @@ virSecurityDACSetChildProcessLabel(virSecurityManagerPtr 
mgr,
 if (virSecurityDACGetIds(secdef, priv, , , NULL, NULL) < 0)
 return -1;
 
+ VIR_DEBUG("Setting child to drop privileges to %u:%u",
+   (unsigned int)user, (unsigned int)group);
+
+virCommandSetUID(cmd, user);
+virCommandSetGID(cmd, group);
+return 0;
+}
+
+
+static int
+virSecurityDACSetVirtioFSProcessLabel(virSecurityManagerPtr mgr G_GNUC_UNUSED,
+  

[libvirt PATCHv4 05/15] docs: add virtiofs kbase

2020-02-20 Thread Ján Tomko
Add a document describing the usage of virtiofs.
---
 docs/kbase.html.in  |   3 +
 docs/kbase/virtiofs.rst | 152 
 2 files changed, 155 insertions(+)
 create mode 100644 docs/kbase/virtiofs.rst

diff --git a/docs/kbase.html.in b/docs/kbase.html.in
index c156414c41..7d6caf3cb1 100644
--- a/docs/kbase.html.in
+++ b/docs/kbase.html.in
@@ -29,6 +29,9 @@
 Backing chain 
management
 Explanation of how disk backing chain specification impacts 
libvirt's
   behaviour and basic troubleshooting steps of disk problems.
+
+Virtio-FS
+Share a filesystem between the guest and the host
   
 
 
diff --git a/docs/kbase/virtiofs.rst b/docs/kbase/virtiofs.rst
new file mode 100644
index 00..fe6885d139
--- /dev/null
+++ b/docs/kbase/virtiofs.rst
@@ -0,0 +1,152 @@
+
+Sharing files with Virtio-FS
+
+
+=== 8< delete before merging 8< ===
+NOTE: if you're looking at this note, this is just a proposal.
+See the up-to-date version on: https://libvirt.org/kbase/virtiofs.html
+=== 8< - 8< ===
+
+.. contents::
+
+=
+Virtio-FS
+=
+
+Virtio-FS is a shared file system that lets virtual machines access
+a directory tree on the host. Unlike existing approaches, it
+is designed to offer local file system semantics and performance.
+
+See https://virtio-fs.gitlab.io/
+
+==
+Host setup
+==
+
+The host-side virtiofsd daemon, like other vhost-user backed devices,
+requires shared memory between the host and the guest. As of QEMU 4.2, this
+requires specifying a NUMA topology for the guest and explicitly specifying
+a memory backend. Multiple options are available:
+
+Either of the following:
+
+* Use file-backed memory
+
+  Configure the directory where the files backing the memory will be stored
+  with the ``memory_backing_dir`` option in ``/etc/libvirt/qemu.conf``
+
+  ::
+
+# This directory is used for memoryBacking source if configured as file.
+# NOTE: big files will be stored here
+memory_backing_dir = "/dev/shm/"
+
+* Use hugepage-backed memory
+
+  Make sure there are enough huge pages allocated for the requested guest 
memory.
+  For example, for one guest with 2 GiB of RAM backed by 2 MiB hugepages:
+
+  ::
+
+  # virsh allocpages 2M 1024
+
+===
+Guest setup
+===
+
+#. Specify the NUMA topology
+
+   in the domain XML of the guest.
+   For the simplest one-node topology for a guest with 2GiB of RAM and 8 vCPUs:
+
+   ::
+
+  
+...
+
+  
+
+  
+
+   ...
+  
+
+   Note that the CPU element might already be specified and only one is 
allowed.
+
+#. Specify the memory backend
+
+   Either of the following:
+
+   * File-backed memory
+
+ ::
+
+
+  ...
+  
+
+  
+  ...
+
+
+ This will create a file in the directory specified in ``qemu.conf``
+
+   * Hugepage-backed memory
+
+ ::
+
+
+  ...
+  
+
+  
+
+
+  
+  ...
+
+
+#. Add the ``vhost-user-fs`` QEMU device via the ``filesystem`` element
+
+   ::
+
+  
+...
+
+  ...
+  
+
+
+
+  
+  ...
+
+  
+
+   Note that despite its name, the ``target dir`` is actually a mount tag and 
does
+   not have to correspond to the desired mount point in the guest.
+
+   So far, ``passthrough`` is the only supported access mode and it requires
+   running the ``virtiofsd`` daemon as root.
+
+#. Boot the guest and mount the filesystem
+
+   ::
+
+  guest# mount -t virtiofs mount_tag /mnt/mount/path
+
+   Note: this requires virtiofs support in the guest kernel (Linux v5.4 or 
later)
+
+===
+Optional parameters
+===
+
+More optional elements can be specified
+
+::
+
+  
+  
+
+
+  
-- 
2.24.1



[libvirt PATCHv4 11/15] qemu: add code for handling virtiofsd

2020-02-20 Thread Ján Tomko
Start virtiofsd for each  device using it.

Pre-create the socket for communication with QEMU and pass it
to virtiofsd.

Note that virtiofsd needs to run as root.

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

Introduced by QEMU commit a43efa34c7d7b628cbf1ec0fe60043e5c91043ea

Signed-off-by: Ján Tomko 
---
 po/POTFILES.in|   1 +
 src/qemu/Makefile.inc.am  |   2 +
 src/qemu/qemu_domain.c|   5 +-
 src/qemu/qemu_domain.h|   2 +-
 src/qemu/qemu_extdevice.c |  20 ++-
 src/qemu/qemu_virtiofs.c  | 300 ++
 src/qemu/qemu_virtiofs.h  |  38 +
 tests/qemuxml2argvtest.c  |  11 ++
 8 files changed, 376 insertions(+), 3 deletions(-)
 create mode 100644 src/qemu/qemu_virtiofs.c
 create mode 100644 src/qemu/qemu_virtiofs.h

diff --git a/po/POTFILES.in b/po/POTFILES.in
index dba0d3a12e..bb97110e83 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -168,6 +168,7 @@
 @SRCDIR@/src/qemu/qemu_tpm.c
 @SRCDIR@/src/qemu/qemu_vhost_user.c
 @SRCDIR@/src/qemu/qemu_vhost_user_gpu.c
+@SRCDIR@/src/qemu/qemu_virtiofs.c
 @SRCDIR@/src/remote/remote_daemon.c
 @SRCDIR@/src/remote/remote_daemon_config.c
 @SRCDIR@/src/remote/remote_daemon_dispatch.c
diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am
index b9c0c6ea9c..baa324dc4b 100644
--- a/src/qemu/Makefile.inc.am
+++ b/src/qemu/Makefile.inc.am
@@ -67,6 +67,8 @@ QEMU_DRIVER_SOURCES = \
qemu/qemu_vhost_user.h \
qemu/qemu_vhost_user_gpu.c \
qemu/qemu_vhost_user_gpu.h \
+   qemu/qemu_virtiofs.c \
+   qemu/qemu_virtiofs.h \
qemu/qemu_checkpoint.c \
qemu/qemu_checkpoint.h \
qemu/qemu_backup.c \
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 7cb283123d..d0b2bff775 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1432,8 +1432,11 @@ qemuDomainFSPrivateNew(void)
 
 
 static void
-qemuDomainFSPrivateDispose(void *obj G_GNUC_UNUSED)
+qemuDomainFSPrivateDispose(void *obj)
 {
+qemuDomainFSPrivatePtr priv = obj;
+
+g_free(priv->vhostuser_fs_sock);
 }
 
 static virClassPtr qemuDomainVideoPrivateClass;
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index f8fb48f2ff..476056c73f 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -568,7 +568,7 @@ typedef qemuDomainFSPrivate *qemuDomainFSPrivatePtr;
 struct _qemuDomainFSPrivate {
 virObject parent;
 
-int dummy;
+char *vhostuser_fs_sock;
 };
 
 
diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c
index 7f3bb104d9..5103d4921c 100644
--- a/src/qemu/qemu_extdevice.c
+++ b/src/qemu/qemu_extdevice.c
@@ -20,11 +20,13 @@
 
 #include 
 
+#include "qemu_command.h"
 #include "qemu_extdevice.h"
 #include "qemu_vhost_user_gpu.h"
 #include "qemu_domain.h"
 #include "qemu_tpm.h"
 #include "qemu_slirp.h"
+#include "qemu_virtiofs.h"
 
 #include "viralloc.h"
 #include "virlog.h"
@@ -153,7 +155,7 @@ qemuExtDevicesCleanupHost(virQEMUDriverPtr driver,
 int
 qemuExtDevicesStart(virQEMUDriverPtr driver,
 virDomainObjPtr vm,
-virLogManagerPtr logManager G_GNUC_UNUSED,
+virLogManagerPtr logManager,
 bool incomingMigration)
 {
 virDomainDefPtr def = vm->def;
@@ -183,6 +185,15 @@ qemuExtDevicesStart(virQEMUDriverPtr driver,
 return -1;
 }
 
+for (i = 0; i < def->nfss; i++) {
+virDomainFSDefPtr fs = def->fss[i];
+
+if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS) {
+if (qemuVirtioFSStart(logManager, driver, vm, fs) < 0)
+return -1;
+}
+}
+
 return 0;
 }
 
@@ -214,6 +225,13 @@ qemuExtDevicesStop(virQEMUDriverPtr driver,
 if (slirp)
 qemuSlirpStop(slirp, vm, driver, net, false);
 }
+
+for (i = 0; i < def->nfss; i++) {
+virDomainFSDefPtr fs = def->fss[i];
+
+if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS)
+qemuVirtioFSStop(driver, vm, fs);
+}
 }
 
 
diff --git a/src/qemu/qemu_virtiofs.c b/src/qemu/qemu_virtiofs.c
new file mode 100644
index 00..600a3644bd
--- /dev/null
+++ b/src/qemu/qemu_virtiofs.c
@@ -0,0 +1,300 @@
+/*
+ * qemu_virtiofs.c: virtiofs support
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * .
+ */
+
+#include 
+
+#include 
+#include 
+#include 
+
+#include 

[libvirt PATCHv4 14/15] qemu: build vhost-user-fs device command line

2020-02-20 Thread Ján Tomko
Format the 'vhost-user-fs' device on the QEMU command line.

This device provides shared file system access using the FUSE protocol
carried over virtio.
The actual file server is implemented in an external vhost-user-fs device
backend process.

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

Signed-off-by: Ján Tomko 
Reviewed-by: Daniel P. Berrangé 
---
 src/qemu/qemu_command.c   | 45 +-
 ...vhost-user-fs-fd-memory.x86_64-latest.args | 39 +++
 ...vhost-user-fs-hugepages.x86_64-latest.args | 47 +++
 tests/qemuxml2argvtest.c  |  3 ++
 4 files changed, 132 insertions(+), 2 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/vhost-user-fs-fd-memory.x86_64-latest.args
 create mode 100644 
tests/qemuxml2argvdata/vhost-user-fs-hugepages.x86_64-latest.args

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 9fcd06f8c3..272a0b8d44 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2574,6 +2574,45 @@ qemuBuildDisksCommandLine(virCommandPtr cmd,
 }
 
 
+static int
+qemuBuildVHostUserFsCommandLine(virCommandPtr cmd,
+virDomainFSDef *fs,
+const virDomainDef *def,
+qemuDomainObjPrivatePtr priv)
+{
+g_autofree char *chardev_alias = NULL;
+g_auto(virBuffer) opt = VIR_BUFFER_INITIALIZER;
+
+chardev_alias = g_strdup_printf("chr-vu-%s", fs->info.alias);
+
+virCommandAddArg(cmd, "-chardev");
+virBufferAddLit(, "socket");
+virBufferAsprintf(, ",id=%s", chardev_alias);
+virBufferEscapeString(, ",path=%s", 
QEMU_DOMAIN_FS_PRIVATE(fs)->vhostuser_fs_sock);
+virCommandAddArg(cmd, virBufferContentAndReset());
+
+virCommandAddArg(cmd, "-device");
+
+if (qemuBuildVirtioDevStr(, "vhost-user-fs", priv->qemuCaps,
+  VIR_DOMAIN_DEVICE_FS, fs) < 0)
+return -1;
+
+virBufferAsprintf(, ",chardev=%s", chardev_alias);
+if (fs->queue_size)
+virBufferAsprintf(, ",queue-size=%llu", fs->queue_size);
+virBufferAddLit(, ",tag=");
+virQEMUBuildBufferEscapeComma(, fs->dst);
+if (qemuBuildVirtioOptionsStr(, fs->virtio, priv->qemuCaps) < 0)
+return -1;
+
+if (qemuBuildDeviceAddressStr(, def, >info, priv->qemuCaps) < 0)
+return -1;
+
+virCommandAddArg(cmd, virBufferContentAndReset());
+return 0;
+}
+
+
 static char *
 qemuBuildFSStr(virDomainFSDefPtr fs)
 {
@@ -2666,7 +2705,7 @@ static int
 qemuBuildFilesystemCommandLine(virCommandPtr cmd,
const virDomainDef *def,
virQEMUCapsPtr qemuCaps,
-   qemuDomainObjPrivatePtr priv G_GNUC_UNUSED)
+   qemuDomainObjPrivatePtr priv)
 {
 size_t i;
 
@@ -2681,7 +2720,9 @@ qemuBuildFilesystemCommandLine(virCommandPtr cmd,
 break;
 
 case VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS:
-/* TODO: vhost-user-fs-pci */
+/* vhost-user-fs-pci */
+if (qemuBuildVHostUserFsCommandLine(cmd, def->fss[i], def, priv) < 
0)
+return -1;
 break;
 
 case VIR_DOMAIN_FS_DRIVER_TYPE_LOOP:
diff --git a/tests/qemuxml2argvdata/vhost-user-fs-fd-memory.x86_64-latest.args 
b/tests/qemuxml2argvdata/vhost-user-fs-fd-memory.x86_64-latest.args
new file mode 100644
index 00..a7df45a7f0
--- /dev/null
+++ b/tests/qemuxml2argvdata/vhost-user-fs-fd-memory.x86_64-latest.args
@@ -0,0 +1,39 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-guest \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-guest/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-guest/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-guest/.config \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-x86_64 \
+-name guest=guest,debug-threads=on \
+-S \
+-object secret,id=masterKey0,format=raw,\
+file=/tmp/lib/domain--1-guest/master-key.aes \
+-machine pc,accel=kvm,usb=off,dump-guest-core=off \
+-cpu qemu64 \
+-m 14336 \
+-overcommit mem-lock=off \
+-smp 2,sockets=2,cores=1,threads=1 \
+-object memory-backend-file,id=ram-node0,\
+mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-guest/ram-node0,share=yes,\
+size=15032385536 \
+-numa node,nodeid=0,cpus=0-1,memdev=ram-node0 \
+-uuid 126f2720-6f8e-45ab-a886-ec9277079a67 \
+-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 \
+-chardev socket,id=chr-vu-fs0,path=/tmp/lib/domain--1-guest/fs0.vhost-fs.sock \
+-device vhost-user-fs-pci,chardev=chr-vu-fs0,queue-size=1024,tag=mount_tag,\
+bus=pci.0,addr=0x2 \
+-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
+resourcecontrol=deny \
+-msg timestamp=on
diff --git a/tests/qemuxml2argvdata/vhost-user-fs-hugepages.x86_64-latest.args 

[libvirt PATCHv4 12/15] qemu: put virtiofsd in the emulator cgroup

2020-02-20 Thread Ján Tomko
Wire up the code to put virtiofsd in the emulator cgroup on domain
startup.

Signed-off-by: Ján Tomko 
---
 src/qemu/qemu_extdevice.c | 15 +++
 src/qemu/qemu_virtiofs.c  | 28 
 src/qemu/qemu_virtiofs.h  |  6 ++
 3 files changed, 49 insertions(+)

diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c
index 5103d4921c..31655b7f0a 100644
--- a/src/qemu/qemu_extdevice.c
+++ b/src/qemu/qemu_extdevice.c
@@ -248,6 +248,13 @@ qemuExtDevicesHasDevice(virDomainDefPtr def)
 if (def->tpm && def->tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR)
 return true;
 
+for (i = 0; i < def->nfss; i++) {
+virDomainFSDefPtr fs = def->fss[i];
+
+if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS)
+return true;
+}
+
 return false;
 }
 
@@ -271,5 +278,13 @@ qemuExtDevicesSetupCgroup(virQEMUDriverPtr driver,
 qemuExtTPMSetupCgroup(driver, def, cgroup) < 0)
 return -1;
 
+for (i = 0; i < def->nfss; i++) {
+virDomainFSDefPtr fs = def->fss[i];
+
+if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS &&
+qemuVirtioFSSetupCgroup(driver, def, fs, cgroup) < 0)
+return -1;
+}
+
 return 0;
 }
diff --git a/src/qemu/qemu_virtiofs.c b/src/qemu/qemu_virtiofs.c
index 600a3644bd..9e354a30c6 100644
--- a/src/qemu/qemu_virtiofs.c
+++ b/src/qemu/qemu_virtiofs.c
@@ -298,3 +298,31 @@ qemuVirtioFSStop(virQEMUDriverPtr driver,
  cleanup:
 virErrorRestore(_err);
 }
+
+
+int
+qemuVirtioFSSetupCgroup(virQEMUDriverPtr driver,
+virDomainDefPtr def,
+virDomainFSDefPtr fs,
+virCgroupPtr cgroup)
+{
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
+g_autofree char *pidfile = NULL;
+pid_t pid = -1;
+int rc;
+
+if (!(pidfile = qemuVirtioFSCreatePidFilename(cfg, def, fs->info.alias)))
+return -1;
+
+rc = virPidFileReadPathIfAlive(pidfile, , NULL);
+if (rc < 0 || pid == (pid_t) -1) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("virtiofsd died unexpectedly"));
+return -1;
+}
+
+if (virCgroupAddProcess(cgroup, pid) < 0)
+return -1;
+
+return 0;
+}
diff --git a/src/qemu/qemu_virtiofs.h b/src/qemu/qemu_virtiofs.h
index 49db807b19..341fed1def 100644
--- a/src/qemu/qemu_virtiofs.h
+++ b/src/qemu/qemu_virtiofs.h
@@ -36,3 +36,9 @@ void
 qemuVirtioFSStop(virQEMUDriverPtr driver,
  virDomainObjPtr vm,
  virDomainFSDefPtr fs);
+
+int
+qemuVirtioFSSetupCgroup(virQEMUDriverPtr driver,
+virDomainDefPtr def,
+virDomainFSDefPtr fs,
+virCgroupPtr cgroup);
-- 
2.24.1



[libvirt PATCHv4 07/15] conf: add virtiofs-related elements and attributes

2020-02-20 Thread Ján Tomko
Add more elements for tuning the virtiofsd daemon
and the vhost-user-fs device:

  

  
  

  

Signed-off-by: Ján Tomko 
Reviewed-by: Daniel P. Berrangé 
Reviewed-by: Masayoshi Mizuma 
---
 docs/formatdomain.html.in |  25 +++-
 docs/schemas/domaincommon.rng |  48 
 src/conf/domain_conf.c| 107 +-
 src/conf/domain_conf.h|  15 +++
 src/libvirt_private.syms  |   1 +
 .../vhost-user-fs-fd-memory.xml   |   6 +-
 .../vhost-user-fs-hugepages.xml   |   1 +
 7 files changed, 200 insertions(+), 3 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index dab8fb8f6b..7c4153c7ce 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3936,10 +3936,15 @@
 readonly/
   /filesystem
   filesystem type='mount' accessmode='passthrough'
-  driver type='virtiofs'/
+  driver type='virtiofs queue='1024'/
+  binary path='/usr/libexec/virtiofsd' xattr='on'
+ cache mode='always'/
+ lock posix='on' flock='on'/
+  /binary
   source dir='/path'/
   target dir='mount_tag'/
   /filesystem
+
   ...
 /devices
 ...
@@ -4063,9 +4068,27 @@
   Virtio-specific options can also be
   set. (Since 3.5.0)
   
+  
+For virtiofs, the queue attribute can be 
used
+to specify the queue size (i.e. how many requests can the queue 
fit).
+(Since 6.1.0)
+  
 
   
 
+  binary
+  
+The optional binary element can tune the options for 
virtiofsd.
+The attribute path can be used to override the path to 
the daemon.
+Attribute xattr enables the use of filesystem extended 
attributes.
+Caching can be tuned via the cache element, possible 
mode
+values being none and always.
+Locking can be controlled via the lock
+element - attributes posix and flock both 
accepting
+values yes or no.
+(Since 6.1.0)
+  
+
   source
   
 The resource on the host that is being accessed in the guest. The
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index f7125e401a..a3e60a6430 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2500,6 +2500,9 @@
   
 
   
+  
+
+  
   
 
   
@@ -2649,12 +2652,57 @@
   
 virtiofs
   
+  
+
+  
+
+  
   
 
 
   
 
   
+  
+
+  
+
+  
+
+  
+  
+
+  
+
+  
+  
+
+  
+
+  
+none
+always
+  
+
+  
+
+  
+  
+
+  
+
+  
+
+  
+  
+
+  
+
+  
+
+  
+
+  
 
   
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d78ea92ead..8e7400294b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -501,6 +501,14 @@ VIR_ENUM_IMPL(virDomainFSModel,
   "virtio-non-transitional",
 );
 
+VIR_ENUM_IMPL(virDomainFSCacheMode,
+  VIR_DOMAIN_FS_CACHE_MODE_LAST,
+  "default",
+  "none",
+  "always",
+);
+
+
 VIR_ENUM_IMPL(virDomainNet,
   VIR_DOMAIN_NET_TYPE_LAST,
   "user",
@@ -2325,6 +2333,7 @@ void virDomainFSDefFree(virDomainFSDefPtr def)
 VIR_FREE(def->dst);
 virDomainDeviceInfoClear(>info);
 VIR_FREE(def->virtio);
+VIR_FREE(def->binary);
 
 VIR_FREE(def);
 }
@@ -11311,6 +11320,64 @@ virDomainFSDefParseXML(virDomainXMLOptionPtr xmlopt,
 }
 }
 
+if (def->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS) {
+g_autofree char *queue_size = 
virXPathString("string(./driver/@queue)", ctxt);
+g_autofree char *binary = virXPathString("string(./binary/@path)", 
ctxt);
+g_autofree char *xattr = virXPathString("string(./binary/@xattr)", 
ctxt);
+g_autofree char *cache = 
virXPathString("string(./binary/cache/@mode)", ctxt);
+g_autofree char *posix_lock = 
virXPathString("string(./binary/lock/@posix)", ctxt);
+g_autofree char *flock = 
virXPathString("string(./binary/lock/@flock)", ctxt);
+int val;
+
+
+if (queue_size && virStrToLong_ull(queue_size, NULL, 10, 
>queue_size) < 0) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("cannot parse queue size '%s' for virtiofs"),
+   queue_size);
+goto error;
+}
+
+if (binary)
+def->binary = virFileSanitizePath(binary);
+
+ 

[libvirt PATCHv4 04/15] qemu: add QEMU_CAPS_VHOST_USER_FS

2020-02-20 Thread Ján Tomko
Introduced by QEMU commit 98fc1ada4cf70af0f1df1a2d7183cf786fc7da05
virtio: add vhost-user-fs base device

Released in QEMU v4.2.0.

Signed-off-by: Ján Tomko 
Reviewed-by: Peter Krempa 
Acked-by: Stefan Hajnoczi 
Reviewed-by: Daniel P. Berrangé 
---
 src/qemu/qemu_capabilities.c  | 4 
 src/qemu/qemu_capabilities.h  | 3 +++
 tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml | 1 +
 tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml   | 1 +
 tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml  | 1 +
 tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml | 1 +
 tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml  | 1 +
 7 files changed, 12 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 0e727093bc..13f9a08eb2 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -560,6 +560,9 @@ VIR_ENUM_IMPL(virQEMUCaps,
   "virtio-net.failover",
   "tpm-spapr",
   "cpu.kvm-no-adjvtime",
+
+  /* 355 */
+  "vhost-user-fs",
 );
 
 
@@ -1277,6 +1280,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
 { "i8042", QEMU_CAPS_DEVICE_I8042 },
 { "rng-builtin", QEMU_CAPS_OBJECT_RNG_BUILTIN },
 { "tpm-spapr", QEMU_CAPS_DEVICE_TPM_SPAPR },
+{ "vhost-user-fs-device", QEMU_CAPS_DEVICE_VHOST_USER_FS },
 };
 
 static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsVirtioBalloon[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 7b6ed53863..5b483a2419 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -543,6 +543,9 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 QEMU_CAPS_DEVICE_TPM_SPAPR, /* -device tpm-spapr */
 QEMU_CAPS_CPU_KVM_NO_ADJVTIME, /* cpu.kvm-no-adjvtime */
 
+/* 355 */
+QEMU_CAPS_DEVICE_VHOST_USER_FS, /* -device vhost-user-fs */
+
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
 
diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml 
b/tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml
index d0faa4c471..640ce29c8c 100644
--- a/tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml
@@ -178,6 +178,7 @@
   
   
   
+  
   4001050
   0
   61700242
diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml 
b/tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml
index 9275bf36fa..37776e1bbe 100644
--- a/tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml
@@ -137,6 +137,7 @@
   
   
   
+  
   4001050
   0
   39100242
diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml
index faed23f5c1..83e804ea36 100644
--- a/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml
@@ -222,6 +222,7 @@
   
   
   
+  
   4002000
   0
   43100242
diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml 
b/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml
index c05cea2eb7..e52c60607d 100644
--- a/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml
@@ -180,6 +180,7 @@
   
   
   
+  
   4002050
   0
   61700241
diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml
index 1eef7197ef..6902cffd17 100644
--- a/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml
@@ -223,6 +223,7 @@
   
   
   
+  
   4002050
   0
   43100241
-- 
2.24.1



[libvirt PATCHv4 00/15] add virtiofs support (virtio-fs epopee)

2020-02-20 Thread Ján Tomko
For:
  https://bugzilla.redhat.com/show_bug.cgi?id=1694166

v3:
https://www.redhat.com/archives/libvir-list/2020-January/msg01401.html
v4:
* place virtiofsd into the emulator cgroup
* do not leak the log file descriptor
* better validation of the path existence and shared memory support
* run as root:root explicitly
* correctly use listification in RST document

bonus:
15/15 wip for SELinux integration

Avaliable on my repo (except for the bonus round)
git fetch https://repo.or.cz/libvirt/jtomko.git virtiofs-v4

TODO:
* a bug against selinux-policy
* address the inconsistency of some downstreams wrt placing the json
  files into /usr/share/qemu vs. /usr/share/qemu-kvm:
  https://bugzilla.redhat.com/show_bug.cgi?id=1804196

Daniel P. Berrangé (1):
  docs: reduce excessive spacing in ToC for RST files

Ján Tomko (14):
  schema: wrap fsDriver in a choice group
  qemuExtDevicesStart: pass logManager
  qemu: add QEMU_CAPS_VHOST_USER_FS
  docs: add virtiofs kbase
  conf: qemu: add virtiofs fsdriver type
  conf: add virtiofs-related elements and attributes
  qemu: add virtiofsd_debug to qemu.conf
  qemu: validate virtiofs filesystems
  qemu: forbid migration with vhost-user-fs device
  qemu: add code for handling virtiofsd
  qemu: put virtiofsd in the emulator cgroup
  qemu: use the vhost-user schemas to find binary
  qemu: build vhost-user-fs device command line
  wip: SELinux integration for virtiofsd

 docs/formatdomain.html.in |  35 +-
 docs/kbase.html.in|   3 +
 docs/kbase/virtiofs.rst   | 152 
 docs/libvirt.css  |   4 +
 docs/schemas/domaincommon.rng |  88 -
 po/POTFILES.in|   1 +
 src/conf/domain_conf.c| 108 +-
 src/conf/domain_conf.h|  16 +
 src/libvirt_private.syms  |   2 +
 src/qemu/Makefile.inc.am  |   2 +
 src/qemu/libvirtd_qemu.aug|   1 +
 src/qemu/qemu.conf|   7 +
 src/qemu/qemu_capabilities.c  |   4 +
 src/qemu/qemu_capabilities.h  |   3 +
 src/qemu/qemu_command.c   |  47 ++-
 src/qemu/qemu_conf.c  |   2 +
 src/qemu/qemu_conf.h  |   1 +
 src/qemu/qemu_domain.c|  66 +++-
 src/qemu/qemu_domain.h|   2 +-
 src/qemu/qemu_domain_address.c|   4 +
 src/qemu/qemu_extdevice.c |  43 +++
 src/qemu/qemu_extdevice.h |   1 +
 src/qemu/qemu_migration.c |  10 +
 src/qemu/qemu_process.c   |   4 +-
 src/qemu/qemu_security.c  |  40 ++
 src/qemu/qemu_security.h  |   7 +
 src/qemu/qemu_vhost_user.c|  40 ++
 src/qemu/qemu_vhost_user.h|   4 +
 src/qemu/qemu_virtiofs.c  | 341 ++
 src/qemu/qemu_virtiofs.h  |  48 +++
 src/qemu/test_libvirtd_qemu.aug.in|   1 +
 src/security/security_dac.c   |  20 +
 src/security/security_driver.h|   2 +
 src/security/security_manager.c   |  12 +
 src/security/security_manager.h   |   4 +
 src/security/security_nop.c   |   1 +
 src/security/security_selinux.c   |  69 
 src/security/security_stack.c |  19 +
 .../caps_4.2.0.aarch64.xml|   1 +
 .../qemucapabilitiesdata/caps_4.2.0.s390x.xml |   1 +
 .../caps_4.2.0.x86_64.xml |   1 +
 .../caps_5.0.0.aarch64.xml|   1 +
 .../caps_5.0.0.x86_64.xml |   1 +
 ...vhost-user-fs-fd-memory.x86_64-latest.args |  39 ++
 .../vhost-user-fs-fd-memory.xml   |  43 +++
 ...vhost-user-fs-hugepages.x86_64-latest.args |  47 +++
 .../vhost-user-fs-hugepages.xml   |  75 
 tests/qemuxml2argvtest.c  |  14 +
 .../vhost-user-fs-fd-memory.x86_64-latest.xml |   1 +
 .../vhost-user-fs-hugepages.x86_64-latest.xml |   1 +
 tests/qemuxml2xmltest.c   |   3 +
 51 files changed, 1420 insertions(+), 22 deletions(-)
 create mode 100644 docs/kbase/virtiofs.rst
 create mode 100644 src/qemu/qemu_virtiofs.c
 create mode 100644 src/qemu/qemu_virtiofs.h
 create mode 100644 
tests/qemuxml2argvdata/vhost-user-fs-fd-memory.x86_64-latest.args
 create mode 100644 tests/qemuxml2argvdata/vhost-user-fs-fd-memory.xml
 create mode 100644 
tests/qemuxml2argvdata/vhost-user-fs-hugepages.x86_64-latest.args
 create mode 100644 tests/qemuxml2argvdata/vhost-user-fs-hugepages.xml
 create mode 12 
tests/qemuxml2xmloutdata/vhost-user-fs-fd-memory.x86_64-latest.xml
 create mode 12 
tests/qemuxml2xmloutdata/vhost-user-fs-hugepages.x86_64-latest.xml

-- 
2.24.1



[libvirt PATCHv4 02/15] schema: wrap fsDriver in a choice group

2020-02-20 Thread Ján Tomko
Allow adding new groups without changing indentation.

Signed-off-by: Ján Tomko 
Reviewed-by: Peter Krempa 
Acked-by: Stefan Hajnoczi 
Reviewed-by: Daniel P. Berrangé 
---
 docs/schemas/domaincommon.rng | 50 +++
 1 file changed, 27 insertions(+), 23 deletions(-)

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 4be751461d..5b609a4756 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2620,29 +2620,33 @@
for this kind of info, and 'type' for the
storage format. We need the latter too, so
had to invent a new attribute name -->
-  
-
-  
-path
-handle
-loop
-nbd
-ploop
-  
-
-  
-  
-
-  
-
-  
-  
-
-  immediate
-
-  
-  
-  
+  
+
+  
+
+  
+path
+handle
+loop
+nbd
+ploop
+  
+
+  
+  
+
+  
+
+  
+  
+
+  immediate
+
+  
+  
+
+
+  
 
   
 
-- 
2.24.1



Re: [jenkins-ci PATCH 0/8] lcitool: Support MinGW cross-build Dockerfiles

2020-02-20 Thread Andrea Bolognani
On Mon, 2020-02-10 at 18:18 +0100, Andrea Bolognani wrote:
> More details in the commit message for patch 7/8.
> 
> Pavel pointed out today that the current method of triggering MinGW
> builds using our CI scaffolding, eg.
> 
>   $ make ci-build@fedora-30 CI_CONFIGURE=mingw64-configure
> 
> is easy to get wrong and not very discoverable, so I took that as
> motivation to implement a change that I had been thinking about for
> a long time anyway. The new usage will be
> 
>   $ make ci-build@fedora-30-cross-mingw64
> 
> which aligns with how we're already doing cross-builds for other
> architectures and is discoverable via 'make ci-list-images'.
> 
> The implementation is not the prettiest, but the Dockerfile
> generator in general could use some love so I don't think this
> improvement should be blocked because of that; I'll try to spend
> some time refactoring and cleaning up once this has been merged.
> 
> Andrea Bolognani (8):
>   lcitool: Introduce cross_arch local variable
>   lcitool: Change check for pip_pkgs formatting
>   lcitool: Separate computation and formatting
>   lcitool: Introduce _dockerfile_format()
>   lcitool: Introduce _dockerfile_build_varmap()
>   lcitool: Add RPM-specific _dockerfile_build_varmap() variant
>   lcitool: Support MinGW cross-build Dockerfiles on Fedora
>   lcitool: Add more checks to _action_dockerfile()
> 
>  guests/lcitool | 219 ++---
>  1 file changed, 172 insertions(+), 47 deletions(-)

Ping? :)

-- 
Andrea Bolognani / Red Hat / Virtualization



Re: [PATCH 0/3] Couple of almost trivial patches

2020-02-20 Thread Ján Tomko

On Thu, Feb 20, 2020 at 12:02:34PM +0100, Michal Privoznik wrote:

These stem out from my review of Marc-André's patches:

https://www.redhat.com/archives/libvir-list/2020-January/msg00648.html

Michal Prívozník (3):
 virpidfile: Set correct retval in virPidFileReadPath()
 qemu: Don't explicitly remove pidfile after
   virPidFileForceCleanupPath()
 qemu_migration: Rearrange some checks in qemuMigrationSrcIsAllowed()

src/qemu/qemu_migration.c  | 66 +-
src/qemu/qemu_process.c|  9 +
src/qemu/qemu_vhost_user_gpu.c | 10 +-
src/util/virpidfile.c  |  2 +-
4 files changed, 36 insertions(+), 51 deletions(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature


[PATCH 1/3] virpidfile: Set correct retval in virPidFileReadPath()

2020-02-20 Thread Michal Privoznik
The virPidFileReadPath() function is supposed to return 0 on
success or a negative value on failure. But the negative value
has a special meaning - it's negated errno. Therefore, when
converting string to int we shouldn't return -1 which translates
to EPERM. Returning EINVAL looks closer to the truth.

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

diff --git a/src/util/virpidfile.c b/src/util/virpidfile.c
index a8a743504d..d5aa5f4f84 100644
--- a/src/util/virpidfile.c
+++ b/src/util/virpidfile.c
@@ -130,7 +130,7 @@ int virPidFileReadPath(const char *path,
 if (virStrToLong_ll(pidstr, , 10, _value) < 0 ||
 !(*endptr == '\0' || g_ascii_isspace(*endptr)) ||
 (pid_t) pid_value != pid_value) {
-rc = -1;
+rc = -EINVAL;
 goto cleanup;
 }
 
-- 
2.24.1



[PATCH 3/3] qemu_migration: Rearrange some checks in qemuMigrationSrcIsAllowed()

2020-02-20 Thread Michal Privoznik
Firstly, the check for disk I/O error can be moved into 'if
(!offline)' section a few lines below.
Secondly, checks for vmstate and slirp should be moved under the
same section because they reflect live state of a domain. For
offline migration no QEMU is involved and thus these restrictions
are not valid.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_migration.c | 66 +++
 1 file changed, 33 insertions(+), 33 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index ceac81c960..a307c5ebe2 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1185,43 +1185,25 @@ qemuMigrationSrcIsAllowed(virQEMUDriverPtr driver,
nsnapshots);
 return false;
 }
-
-/* cancel migration if disk I/O error is emitted while migrating */
-if (flags & VIR_MIGRATE_ABORT_ON_ERROR &&
-!(flags & VIR_MIGRATE_OFFLINE) &&
-virDomainObjGetState(vm, ) == VIR_DOMAIN_PAUSED &&
-pauseReason == VIR_DOMAIN_PAUSED_IOERROR) {
-virReportError(VIR_ERR_OPERATION_INVALID, "%s",
-   _("cannot migrate domain with I/O error"));
-return false;
-}
-}
-
-if (virHashSize(priv->dbusVMStates) > 0 &&
-!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DBUS_VMSTATE)) {
-virReportError(VIR_ERR_OPERATION_INVALID, "%s",
-   _("domain requires dbus-vmstate support"));
-return false;
-}
-
-for (i = 0; i < vm->def->nnets; i++) {
-virDomainNetDefPtr net = vm->def->nets[i];
-qemuSlirpPtr slirp = QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp;
-
-if (slirp && !qemuSlirpHasFeature(slirp, QEMU_SLIRP_FEATURE_MIGRATE)) {
-virReportError(VIR_ERR_OPERATION_INVALID, "%s",
-   _("a slirp-helper cannot be migrated"));
-return false;
-}
 }
 
 /* following checks don't make sense for offline migration */
 if (!(flags & VIR_MIGRATE_OFFLINE)) {
-if (remote &&
-qemuProcessAutoDestroyActive(driver, vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("domain is marked for auto destroy"));
-return false;
+if (remote) {
+/* cancel migration if disk I/O error is emitted while migrating */
+if (flags & VIR_MIGRATE_ABORT_ON_ERROR &&
+virDomainObjGetState(vm, ) == VIR_DOMAIN_PAUSED &&
+pauseReason == VIR_DOMAIN_PAUSED_IOERROR) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("cannot migrate domain with I/O error"));
+return false;
+}
+
+if (qemuProcessAutoDestroyActive(driver, vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   "%s", _("domain is marked for auto destroy"));
+return false;
+}
 }
 
 
@@ -1280,6 +1262,24 @@ qemuMigrationSrcIsAllowed(virQEMUDriverPtr driver,
_("migration with shmem device is not supported"));
 return false;
 }
+
+if (virHashSize(priv->dbusVMStates) > 0 &&
+!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DBUS_VMSTATE)) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("domain requires dbus-vmstate support"));
+return false;
+}
+
+for (i = 0; i < vm->def->nnets; i++) {
+virDomainNetDefPtr net = vm->def->nets[i];
+qemuSlirpPtr slirp = QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp;
+
+if (slirp && !qemuSlirpHasFeature(slirp, 
QEMU_SLIRP_FEATURE_MIGRATE)) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("a slirp-helper cannot be migrated"));
+return false;
+}
+}
 }
 
 return true;
-- 
2.24.1



[PATCH 2/3] qemu: Don't explicitly remove pidfile after virPidFileForceCleanupPath()

2020-02-20 Thread Michal Privoznik
In two places where virPidFileForceCleanupPath() is called, we
try to unlink() the pidfile again. This is needless because
virPidFileForceCleanupPath() has done just that.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_process.c|  9 +
 src/qemu/qemu_vhost_user_gpu.c | 10 +-
 2 files changed, 2 insertions(+), 17 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index bf987a3bc3..8c1ed76677 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2802,14 +2802,7 @@ qemuProcessKillManagedPRDaemon(virDomainObjPtr vm)
 if (virPidFileForceCleanupPath(pidfile) < 0) {
 VIR_WARN("Unable to kill pr-helper process");
 } else {
-if (unlink(pidfile) < 0 &&
-errno != ENOENT) {
-virReportSystemError(errno,
- _("Unable to remove stale pidfile %s"),
- pidfile);
-} else {
-priv->prDaemonRunning = false;
-}
+priv->prDaemonRunning = false;
 }
 virErrorRestore(_err);
 }
diff --git a/src/qemu/qemu_vhost_user_gpu.c b/src/qemu/qemu_vhost_user_gpu.c
index 51244f9b35..ae1f530338 100644
--- a/src/qemu/qemu_vhost_user_gpu.c
+++ b/src/qemu/qemu_vhost_user_gpu.c
@@ -223,16 +223,8 @@ void qemuExtVhostUserGPUStop(virQEMUDriverPtr driver,
 }
 
 virErrorPreserveLast(_err);
-if (virPidFileForceCleanupPath(pidfile) < 0) {
+if (virPidFileForceCleanupPath(pidfile) < 0)
 VIR_WARN("Unable to kill vhost-user-gpu process");
-} else {
-if (unlink(pidfile) < 0 &&
-errno != ENOENT) {
-virReportSystemError(errno,
- _("Unable to remove stale pidfile %s"),
- pidfile);
-}
-}
 virErrorRestore(_err);
 }
 
-- 
2.24.1



[PATCH 0/3] Couple of almost trivial patches

2020-02-20 Thread Michal Privoznik
These stem out from my review of Marc-André's patches:

https://www.redhat.com/archives/libvir-list/2020-January/msg00648.html

Michal Prívozník (3):
  virpidfile: Set correct retval in virPidFileReadPath()
  qemu: Don't explicitly remove pidfile after
virPidFileForceCleanupPath()
  qemu_migration: Rearrange some checks in qemuMigrationSrcIsAllowed()

 src/qemu/qemu_migration.c  | 66 +-
 src/qemu/qemu_process.c|  9 +
 src/qemu/qemu_vhost_user_gpu.c | 10 +-
 src/util/virpidfile.c  |  2 +-
 4 files changed, 36 insertions(+), 51 deletions(-)

-- 
2.24.1



[ruby PATCH] Fix cpumap allocation for virDomainGetVcpus and use return value

2020-02-20 Thread Charlie Smurthwaite
This patch fixes a bug in which only enough memory for one cpumap is 
allocated
for virDomainGetVcpus instead of one per virtual CPU. This Fixes an 
overflow.

Additionally, it uses the return value of virDomainGetVcpus to determine how
many cpuinfo structs were actually populated rather than assuming they 
all are.
Finally, it uses the logical CPU number from the cpuinfo struct in the 
retrurn
data instead of assuming CPU numbers are sequential. This should handle 
cases

where arbitrary CPUs are offline.

Signed-off-by: Charlie Smurthwaite 
---
ext/libvirt/domain.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/ext/libvirt/domain.c b/ext/libvirt/domain.c
index d665907..c6de1bf 100644
--- a/ext/libvirt/domain.c
+++ b/ext/libvirt/domain.c
@@ -803,7 +803,7 @@ static VALUE libvirt_domain_vcpus(VALUE d)
cpumaplen = VIR_CPU_MAPLEN(maxcpus);
- cpumap = alloca(sizeof(unsigned char) * cpumaplen);
+ cpumap = alloca(sizeof(unsigned char) * cpumaplen * dominfo.nrVirtCpu);
r = virDomainGetVcpus(ruby_libvirt_domain_get(d), cpuinfo,
dominfo.nrVirtCpu, cpumap, cpumaplen);
@@ -832,15 +832,16 @@ static VALUE libvirt_domain_vcpus(VALUE d)
result = rb_ary_new();
- for (i = 0; i < dominfo.nrVirtCpu; i++) {
+ for (i = 0; i < r; i++) {
vcpuinfo = rb_class_new_instance(0, NULL, c_domain_vcpuinfo);
- rb_iv_set(vcpuinfo, "@number", UINT2NUM(i));
if (cpuinfo != NULL) {
+ rb_iv_set(vcpuinfo, "@number", INT2NUM(cpuinfo[i].number));
rb_iv_set(vcpuinfo, "@state", INT2NUM(cpuinfo[i].state));
rb_iv_set(vcpuinfo, "@cpu_time", ULL2NUM(cpuinfo[i].cpuTime));
rb_iv_set(vcpuinfo, "@cpu", INT2NUM(cpuinfo[i].cpu));
}
else {
+ rb_iv_set(vcpuinfo, "@number", Qnil);
rb_iv_set(vcpuinfo, "@state", Qnil);
rb_iv_set(vcpuinfo, "@cpu_time", Qnil);
rb_iv_set(vcpuinfo, "@cpu", Qnil);

--
2.25.0



Re: [PATCH 7/8] qemu: add dbus-vmstate helper migration support

2020-02-20 Thread Michal Privoznik

On 1/14/20 2:46 PM, marcandre.lur...@redhat.com wrote:

From: Marc-André Lureau 

Helper processes may have their state migrated with QEMU data stream
thanks to the QEMU "dbus-vmstate".

libvirt maintains the list of helpers to be migrated. The
"dbus-vmstate" is added when required, and given the list of helper
Ids that must be migrated, on save & load sides.

See also:
https://git.qemu.org/?p=qemu.git;a=blob;f=docs/interop/dbus-vmstate.rst

Signed-off-by: Marc-André Lureau 
---
  src/qemu/qemu_alias.c|  7 +++
  src/qemu/qemu_alias.h|  2 +
  src/qemu/qemu_command.c  | 62 +++
  src/qemu/qemu_command.h  |  3 ++
  src/qemu/qemu_dbus.c | 14 ++
  src/qemu/qemu_dbus.h |  4 ++
  src/qemu/qemu_domain.c   | 10 +
  src/qemu/qemu_domain.h   |  5 +++
  src/qemu/qemu_hotplug.c  | 82 
  src/qemu/qemu_hotplug.h  |  8 
  src/qemu/qemu_migration.c| 51 ++
  src/qemu/qemu_monitor.c  | 21 +
  src/qemu/qemu_monitor.h  |  3 ++
  src/qemu/qemu_monitor_json.c | 15 +++
  src/qemu/qemu_monitor_json.h |  5 +++
  15 files changed, 292 insertions(+)





diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 71d0bb0879..8c281f3a70 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1125,10 +1125,18 @@ qemuMigrationSrcIsAllowed(virQEMUDriverPtr driver,
bool remote,
unsigned int flags)
  {
+qemuDomainObjPrivatePtr priv = vm->privateData;
  int nsnapshots;
  int pauseReason;
  size_t i;
  
+if (virStringListLength((const char **)priv->dbusVMStateIds) &&

+!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DBUS_VMSTATE)) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("cannot migrate this domain without dbus-vmstate 
support"));
+return false;
+}
+


This should be done in the if(!OFFLINE) a few lines below. IIUC, vmstate 
is runtime thing, and when doing offline migration (e.g. just copying 
over disks and XMLs), no qemu is involved and thus no vmstate matters.



  /* perform these checks only when migrating to remote hosts */
  if (remote) {
  nsnapshots = virDomainSnapshotObjListNum(vm->snapshots, NULL, 0);


Michal



Re: [PATCH 3/8] qemu-conf: add dbusStateDir

2020-02-20 Thread Michal Privoznik

On 1/14/20 2:46 PM, marcandre.lur...@redhat.com wrote:

From: Marc-André Lureau 

Location of DBus daemon state configuration, socket, pid...

Signed-off-by: Marc-André Lureau 
---
  src/qemu/qemu_conf.c | 4 
  src/qemu/qemu_conf.h | 1 +
  2 files changed, 5 insertions(+)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index e1fea390c7..ade12dac6c 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -144,6 +144,8 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool 
privileged)
  
  cfg->cacheDir = g_strdup_printf("%s/cache/libvirt/qemu", LOCALSTATEDIR);
  
+cfg->dbusStateDir = g_strdup_printf("%s/run/libvirt/qemu/dbus", LOCALSTATEDIR);

+
  cfg->libDir = g_strdup_printf("%s/lib/libvirt/qemu", LOCALSTATEDIR);
  cfg->saveDir = g_strdup_printf("%s/save", cfg->libDir);
  cfg->snapshotDir = g_strdup_printf("%s/snapshot", cfg->libDir);
@@ -174,6 +176,7 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool 
privileged)
  cfg->stateDir = g_strdup_printf("%s/qemu/run", rundir);
  
  cfg->swtpmStateDir = g_strdup_printf("%s/swtpm", cfg->stateDir);

+cfg->dbusStateDir = g_strdup_printf("%s/dbus", cfg->stateDir);
  
  cfg->configBaseDir = virGetUserConfigDirectory();


Instead of doing practically the same in two branches, you can achieve 
the same result with just one line if you put the call just below 
cfg->slirpStateDir init.


However, do we need this to be in a special directory instead of per VM 
private directory? The way I implemented PR helper was that the socket 
it creates and to which qemu connects is stored under vm->priv->libDir 
which is initialized in qemuDomainSetPrivatePaths() to:


  $cfg->libDir/domain-$shortName/

This way you wouldn't need to care about domain's shortname in the next 
patch.


Michal



Re: [PATCH 4/8] qemu: add a DBus daemon helper unit

2020-02-20 Thread Michal Privoznik

On 1/14/20 2:46 PM, marcandre.lur...@redhat.com wrote:

From: Marc-André Lureau 

Add a unit to start & stop a private dbus-daemon.

The daemon is meant to be started on demand, and associated with a
QEMU process. It should be stopped when the QEMU process is stopped.

The current policy is permissive like a session bus. Stricter
policies can be added later, following recommendations from:
https://git.qemu.org/?p=qemu.git;a=blob;f=docs/interop/dbus.rst

Signed-off-by: Marc-André Lureau 
---
  src/qemu/Makefile.inc.am |   4 +
  src/qemu/qemu_dbus.c | 299 +++
  src/qemu/qemu_dbus.h |  40 ++
  src/qemu/qemu_domain.c   |   2 +
  src/qemu/qemu_domain.h   |   3 +
  tests/Makefile.am|   1 +
  6 files changed, 349 insertions(+)
  create mode 100644 src/qemu/qemu_dbus.c
  create mode 100644 src/qemu/qemu_dbus.h

diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am
index 028ab9043c..833638ec3c 100644
--- a/src/qemu/Makefile.inc.am
+++ b/src/qemu/Makefile.inc.am
@@ -69,6 +69,8 @@ QEMU_DRIVER_SOURCES = \
qemu/qemu_checkpoint.h \
qemu/qemu_backup.c \
qemu/qemu_backup.h \
+   qemu/qemu_dbus.c \
+   qemu/qemu_dbus.h \


These can be added where they were just a moment ago ;-)


$(NULL)
  
  
@@ -93,6 +95,7 @@ libvirt_driver_qemu_impl_la_CFLAGS = \

$(LIBNL_CFLAGS) \
$(SELINUX_CFLAGS) \
$(XDR_CFLAGS) \
+   $(DBUS_CFLAGS) \
-I$(srcdir)/access \
-I$(builddir)/access \
-I$(srcdir)/conf \
@@ -105,6 +108,7 @@ libvirt_driver_qemu_impl_la_LIBADD = \
$(GNUTLS_LIBS) \
$(LIBNL_LIBS) \
$(SELINUX_LIBS) \
+   $(DBUS_LIBS) \
$(LIBXML_LIBS) \
$(NULL)
  libvirt_driver_qemu_impl_la_SOURCES = $(QEMU_DRIVER_SOURCES)
diff --git a/src/qemu/qemu_dbus.c b/src/qemu/qemu_dbus.c
new file mode 100644
index 00..9c8a03c947
--- /dev/null
+++ b/src/qemu/qemu_dbus.c
@@ -0,0 +1,299 @@
+/*
+ * qemu_dbus.c: QEMU dbus daemon
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * .
+ */
+
+#include 
+
+#include "qemu_extdevice.h"
+#include "qemu_dbus.h"
+#include "qemu_security.h"
+
+#include "viralloc.h"
+#include "virlog.h"
+#include "virstring.h"
+#include "virtime.h"
+#include "virpidfile.h"
+
+#define VIR_FROM_THIS VIR_FROM_NONE
+
+VIR_LOG_INIT("qemu.dbus");
+
+
+int
+qemuDBusPrepareHost(virQEMUDriverPtr driver)
+{
+virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+
+return virDirCreate(cfg->dbusStateDir, 0770, cfg->user, cfg->group,
+VIR_DIR_CREATE_ALLOW_EXIST);
+}
+
+
+static char *
+qemuDBusCreatePidFilename(const char *stateDir,
+  const char *shortName)
+{
+g_autofree char *name = g_strdup_printf("%s-dbus", shortName);
+
+return virPidFileBuildPath(stateDir, name);


Instead of having each caller pass cfg->dbusStateDir, we can accept cfg 
here and deref ourselves.



+}
+
+
+static char *
+qemuDBusCreateFilename(const char *stateDir,
+   const char *shortName,
+   const char *ext)
+{
+g_autofree char *name = g_strdup_printf("%s-dbus", shortName);
+
+return virFileBuildPath(stateDir, name,  ext);
+}
+
+
+static char *
+qemuDBusCreateSocketPath(virQEMUDriverConfigPtr cfg,
+ const char *shortName)
+{
+return qemuDBusCreateFilename(cfg->dbusStateDir, shortName, ".sock");
+}
+


I'd introduce qemuDBusCreateConfPath() so that nobody else has to call 
qemuDBusCreateFilename() again.



+
+char *
+qemuDBusGetAddress(virQEMUDriverPtr driver,
+   virDomainObjPtr vm)
+{
+virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+g_autofree char *shortName = virDomainDefGetShortName(vm->def);
+g_autofree char *path = qemuDBusCreateSocketPath(cfg, shortName);
+
+return g_strdup_printf("unix:path=%s", path);
+}
+
+
+static int
+qemuDBusGetPid(const char *binPath,
+   const char *stateDir,
+   const char *shortName,
+   pid_t *pid)
+{
+g_autofree char *pidfile = qemuDBusCreatePidFilename(stateDir, shortName);
+
+return virPidFileReadPathIfAlive(pidfile, pid, binPath);
+}


After the daemon startup is reworked, this function is no longer needed.


+
+
+static int
+qemuDBusWriteConfig(const char *filename, 

Re: [PATCH 1/8] qemu: remove dbus-vmstate code

2020-02-20 Thread Michal Privoznik

On 1/14/20 2:46 PM, marcandre.lur...@redhat.com wrote:

From: Marc-André Lureau 

This code was based on a per-helper instance and peer-to-peer
connections. The code that landed in qemu master for v5.0 is relying
on a single instance and DBus bus.

Instead of trying to adapt the existing dbus-vmstate code, let's
remove it and resubmit. That should make reviewing easier.

Signed-off-by: Marc-André Lureau 
---
  src/qemu/Makefile.inc.am  |   2 -
  src/qemu/qemu_alias.c |  16 -
  src/qemu/qemu_alias.h |   3 -
  src/qemu/qemu_command.c   |  83 -
  src/qemu/qemu_command.h   |   3 -
  src/qemu/qemu_dbus.c  |  94 
  src/qemu/qemu_dbus.h  |  42 -
  src/qemu/qemu_domain.c|  13 
  src/qemu/qemu_domain.h|   1 -
  src/qemu/qemu_extdevice.c |   4 +-
  src/qemu/qemu_hotplug.c   |  83 +
  src/qemu/qemu_hotplug.h   |  11 
  src/qemu/qemu_migration.c |   8 ---
  src/qemu/qemu_slirp.c | 125 +-
  src/qemu/qemu_slirp.h |   4 +-
  15 files changed, 7 insertions(+), 485 deletions(-)
  delete mode 100644 src/qemu/qemu_dbus.c
  delete mode 100644 src/qemu/qemu_dbus.h


You missed po/POTFILES.in:

@SRCDIR@/src/qemu/qemu_dbus.c

Michal



Re: [PATCH 5/8] domain: save/restore the state of dbus-daemon running

2020-02-20 Thread Michal Privoznik

On 1/14/20 2:46 PM, marcandre.lur...@redhat.com wrote:

From: Marc-André Lureau 

This avoids trying to start a dbus-daemon when its already running.

Signed-off-by: Marc-André Lureau 
---
  src/qemu/qemu_domain.c | 5 +
  1 file changed, 5 insertions(+)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 7722a53c62..dda3cb781f 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2916,6 +2916,9 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf,
virDomainChrTypeToString(priv->monConfig->type));
  }
  
+if (priv->dbusDaemonRunning)

+virBufferAddLit(buf, "\n");
+
  if (priv->namespaces) {
  ssize_t ns = -1;
  
@@ -3697,6 +3700,8 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt,

  goto error;
  }
  
+priv->dbusDaemonRunning = virXPathBoolean("boolean(./dbusDaemon)", ctxt) > 0;

+
  if ((node = virXPathNode("./namespaces", ctxt))) {
  xmlNodePtr next;
  



I'd push these a bit down - closer to PR daemon and slirp so that they 
are grouped together.


Michal



Re: [PATCH 6/8] qemu: prepare and stop the dbus daemon

2020-02-20 Thread Michal Privoznik

On 1/14/20 2:46 PM, marcandre.lur...@redhat.com wrote:

From: Marc-André Lureau 

Signed-off-by: Marc-André Lureau 
---
  src/qemu/qemu_process.c | 6 ++
  1 file changed, 6 insertions(+)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 4195042194..845a7caa55 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -58,6 +58,7 @@
  #include "qemu_extdevice.h"
  #include "qemu_firmware.h"
  #include "qemu_backup.h"
+#include "qemu_dbus.h"
  
  #include "cpu/cpu.h"

  #include "cpu/cpu_x86.h"
@@ -6457,6 +6458,9 @@ qemuProcessPrepareHost(virQEMUDriverPtr driver,
  qemuDomainObjPrivatePtr priv = vm->privateData;
  g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
  
+if (qemuDBusPrepareHost(driver) < 0)

+return -1;
+


Again, put this closer to qemuExtDevicesPrepareHost() so that it's close 
to each other.



  if (qemuPrepareNVRAM(cfg, vm) < 0)
  return -1;
  
@@ -7378,6 +7382,8 @@ void qemuProcessStop(virQEMUDriverPtr driver,
  
  qemuExtDevicesStop(driver, vm);
  
+qemuDBusStop(driver, vm);

+
  vm->def->id = -1;
  
  /* Stop autodestroy in case guest is restarted */




Michal



Re: [PATCH 8/8] qemu-slirp: register helper for migration

2020-02-20 Thread Michal Privoznik

On 1/14/20 2:46 PM, marcandre.lur...@redhat.com wrote:

From: Marc-André Lureau 

When the helper supports DBus, connect it to the bus and set its ID.

If the helper supports migration, register its ID to the list of
dbus-vmstate ID to migrate, and specify --dbus-incoming when
restoring the VM.

Signed-off-by: Marc-André Lureau 
---
  src/qemu/qemu_slirp.c | 37 +
  1 file changed, 37 insertions(+)

diff --git a/src/qemu/qemu_slirp.c b/src/qemu/qemu_slirp.c
index 8e001f0d10..48fc0a68c2 100644
--- a/src/qemu/qemu_slirp.c
+++ b/src/qemu/qemu_slirp.c
@@ -18,6 +18,7 @@
  
  #include 
  
+#include "qemu_dbus.h"

  #include "qemu_extdevice.h"
  #include "qemu_security.h"
  #include "qemu_slirp.h"
@@ -202,6 +203,16 @@ qemuSlirpGetFD(qemuSlirpPtr slirp)
  }
  
  
+static char *

+qemuSlirpGetDBusVMStateId(virDomainNetDefPtr net)
+{
+char macstr[VIR_MAC_STRING_BUFLEN] = "";
+
+/* can't use alias, because it's not stable across restarts */
+return g_strdup_printf("slirp-%s", virMacAddrFormat(>mac, macstr));
+}
+
+
  void
  qemuSlirpStop(qemuSlirpPtr slirp,
virDomainObjPtr vm,
@@ -209,11 +220,14 @@ qemuSlirpStop(qemuSlirpPtr slirp,
virDomainNetDefPtr net)
  {
  g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
+g_autofree char *id = qemuSlirpGetDBusVMStateId(net);
  g_autofree char *pidfile = NULL;
  virErrorPtr orig_err;
  pid_t pid;
  int rc;
  
+qemuDBusVMStateRemove(vm, id);

+
  if (!(pidfile = qemuSlirpCreatePidFilename(cfg, vm->def, 
net->info.alias))) {
  VIR_WARN("Unable to construct slirp pidfile path");
  return;
@@ -310,6 +324,29 @@ qemuSlirpStart(qemuSlirpPtr slirp,
  }
  }
  
+if (qemuSlirpHasFeature(slirp, QEMU_SLIRP_FEATURE_DBUS_ADDRESS)) {

+g_autofree char *id = qemuSlirpGetDBusVMStateId(net);
+g_autofree char *dbus_addr = qemuDBusGetAddress(driver, vm);
+
+if (qemuDBusStart(driver, vm) < 0)
+return -1;
+
+virCommandAddArgFormat(cmd, "--dbus-id=%s", id);
+
+virCommandAddArgFormat(cmd, "--dbus-address=%s", dbus_addr);
+
+if (qemuSlirpHasFeature(slirp, QEMU_SLIRP_FEATURE_MIGRATE)) {
+if (qemuDBusVMStateAdd(vm, id) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Failed to register slirp migration"));
+return -1;
+}
+if (incoming) {
+virCommandAddArg(cmd, "--dbus-incoming");
+}


'make syntax-check' complains here.

Michal



Re: [PATCH 2/8] qemu-conf: add configurable dbus-daemon location

2020-02-20 Thread Michal Privoznik

On 1/14/20 2:46 PM, marcandre.lur...@redhat.com wrote:

From: Marc-André Lureau 

Signed-off-by: Marc-André Lureau 
---
  m4/virt-driver-qemu.m4 | 6 ++
  src/qemu/libvirtd_qemu.aug | 1 +
  src/qemu/qemu.conf | 3 +++
  src/qemu/qemu_conf.c   | 5 +
  src/qemu/qemu_conf.h   | 1 +
  src/qemu/test_libvirtd_qemu.aug.in | 1 +
  6 files changed, 17 insertions(+)




diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index b62dd1df52..e1fea390c7 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -228,6 +228,7 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool 
privileged)
  cfg->bridgeHelperName = g_strdup(QEMU_BRIDGE_HELPER);
  cfg->prHelperName = g_strdup(QEMU_PR_HELPER);
  cfg->slirpHelperName = g_strdup(QEMU_SLIRP_HELPER);
+cfg->slirpHelperName = g_strdup(QEMU_DBUS_DAEMON);


Oops, s/slirpHelperName/dbusDaemonName/  :-D

Michal



Re: [libvirt] [PATCH 0/8] Second take on slirp-helper & dbus-vmstate

2020-02-20 Thread Michal Privoznik

On 1/14/20 2:46 PM, marcandre.lur...@redhat.com wrote:

From: Marc-André Lureau 



Sorry for late review. I will reply to individual patches with suggested 
changes. I have them locally as a fixup patches, so I can squash them 
and resend (keeping your authorship of course). You can also find them 
on my branch (if you want to test them):


  https://github.com/zippy2/libvirt/commits/qemu_dbus_review

Michal