[libvirt] [PATCH v5 3/4] docs: Add QEMU-GA get hostname feature into news.xml
QEMU-GA supports get geust hostname command. This commit includes a specific entry to inform this new feature for QEMU driver to 4.8.0 release. Signed-off-by: Julio Faracco --- docs/news.xml | 10 ++ 1 file changed, 10 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 9a17b2f612..d67327699d 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -37,6 +37,16 @@ + + + qemu: Retrieve guest hostname through QEMU Guest Agent command + + + QEMU is now able to retrieve the guest hostname using a new QEMU-GA + command called 'guest-get-host-name'. Virsh users can execute + 'domhostname' for QEMU driver. + + -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v5 0/4] Add .domainGetHostname() support for QEMU driver.
This serie adds a new function into QEMU Guest Agent handler to use the QEMU command 'guest-get-host-name' to retrieve the domain hostname. This approach requires QEMU-GA running inside the guest, but it is the fastest and easiest way to get this info. This serie has some suggestion made by John Ferlan for v3. Specially, some improvements to error handling. v5: Contains news suggested by Han Han on v2. Contains improvements suggested by John on v3. Removes some tabs reported on v4. Julio Faracco (4): qemu: implementing qemuAgentGetHostname() function. qemu: adding domainGetHostname support for QEMU docs: Add QEMU-GA get hostname feature into news.xml qemu: unlink the error report from VIR_STRDUP. docs/news.xml | 10 +++ src/qemu/qemu_agent.c | 66 ++ src/qemu/qemu_agent.h | 4 +++ src/qemu/qemu_driver.c | 42 +++ 4 files changed, 116 insertions(+), 6 deletions(-) -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v5 1/4] qemu: implementing qemuAgentGetHostname() function.
This commit implements the function qemuAgentGetHostname() that uses
the QEMU guest agent command 'guest-get-host-name' to retrieve the
guest hostname of virtual machine running the QEMU-GA.
Signed-off-by: Julio Faracco
---
src/qemu/qemu_agent.c | 47 +++
src/qemu/qemu_agent.h | 4
2 files changed, 51 insertions(+)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index bf08871f18..ac728becef 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1683,6 +1683,53 @@ qemuAgentUpdateCPUInfo(unsigned int nvcpus,
}
+int
+qemuAgentGetHostname(qemuAgentPtr mon,
+ char **hostname)
+{
+int ret = -1;
+virJSONValuePtr cmd;
+virJSONValuePtr reply = NULL;
+virJSONValuePtr data = NULL;
+const char *result = NULL;
+
+cmd = qemuAgentMakeCommand("guest-get-host-name",
+ NULL);
+
+if (!cmd)
+return ret;
+
+if (qemuAgentCommand(mon, cmd, , true,
+ VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0)
+goto cleanup;
+
+if (qemuAgentCheckError(cmd, reply) < 0)
+goto cleanup;
+
+if (!(data = virJSONValueObjectGet(reply, "return"))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("malformed return value"));
+goto cleanup;
+}
+
+if (!(result = virJSONValueObjectGetString(data, "host-name"))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("'host-name' missing in guest-get-host-name reply"));
+goto cleanup;
+}
+
+if (VIR_STRDUP(*hostname, result) < 0)
+goto cleanup;
+
+ret = 0;
+
+ cleanup:
+virJSONValueFree(cmd);
+virJSONValueFree(reply);
+return ret;
+}
+
+
int
qemuAgentGetTime(qemuAgentPtr mon,
long long *seconds,
diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h
index 6dd9c702dd..4354b7e0cf 100644
--- a/src/qemu/qemu_agent.h
+++ b/src/qemu/qemu_agent.h
@@ -105,6 +105,10 @@ int qemuAgentUpdateCPUInfo(unsigned int nvcpus,
qemuAgentCPUInfoPtr cpuinfo,
int ncpuinfo);
+int
+qemuAgentGetHostname(qemuAgentPtr mon,
+ char **hostname);
+
int qemuAgentGetTime(qemuAgentPtr mon,
long long *seconds,
unsigned int *nseconds);
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v5 2/4] qemu: adding domainGetHostname support for QEMU
This commit adds support to use the function qemuAgentGetHostname()
for obtain the domain hostname using QEMU-GA command.
Signed-off-by: Julio Faracco
---
src/qemu/qemu_driver.c | 42 ++
1 file changed, 42 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 07ea5473b6..2f8d6915e1 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19353,6 +19353,46 @@ qemuConnectGetCPUModelNames(virConnectPtr conn,
return virCPUGetModels(arch, models);
}
+
+static char *
+qemuDomainGetHostname(virDomainPtr dom,
+ unsigned int flags)
+{
+virQEMUDriverPtr driver = dom->conn->privateData;
+virDomainObjPtr vm = NULL;
+qemuAgentPtr agent;
+char *hostname = NULL;
+
+virCheckFlags(0, NULL);
+
+if (!(vm = qemuDomObjFromDomain(dom)))
+return NULL;
+
+if (virDomainGetHostnameEnsureACL(dom->conn, vm->def) < 0)
+goto cleanup;
+
+if (qemuDomainObjBeginAgentJob(driver, vm, QEMU_AGENT_JOB_QUERY) < 0)
+goto cleanup;
+
+if (virDomainObjCheckActive(vm) < 0)
+goto endjob;
+
+if (!qemuDomainAgentAvailable(vm, true))
+goto endjob;
+
+agent = qemuDomainObjEnterAgent(vm);
+ignore_value(qemuAgentGetHostname(agent, ));
+qemuDomainObjExitAgent(vm, agent);
+
+ endjob:
+qemuDomainObjEndAgentJob(vm);
+
+ cleanup:
+virDomainObjEndAPI();
+return hostname;
+}
+
+
static int
qemuDomainGetTime(virDomainPtr dom,
long long *seconds,
@@ -19399,6 +19439,7 @@ qemuDomainGetTime(virDomainPtr dom,
return ret;
}
+
static int
qemuDomainSetTime(virDomainPtr dom,
long long seconds,
@@ -21957,6 +21998,7 @@ static virHypervisorDriver qemuHypervisorDriver = {
.connectGetCPUModelNames = qemuConnectGetCPUModelNames, /* 1.1.3 */
.domainFSFreeze = qemuDomainFSFreeze, /* 1.2.5 */
.domainFSThaw = qemuDomainFSThaw, /* 1.2.5 */
+.domainGetHostname = qemuDomainGetHostname, /* 4.8.0 */
.domainGetTime = qemuDomainGetTime, /* 1.2.5 */
.domainSetTime = qemuDomainSetTime, /* 1.2.5 */
.nodeGetFreePages = qemuNodeGetFreePages, /* 1.2.6 */
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v5 4/4] qemu: unlink the error report from VIR_STRDUP.
The function to retrieve the file system info using QEMU-GA is using
some conditionals to retrieve the info. This is wrong because the error
of some conditionals will be raised if VIR_STRDUP return errors and not
if some problem occurred with JSON.
Signed-off-by: Julio Faracco
---
src/qemu/qemu_agent.c | 19 +--
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index ac728becef..36dc18d72f 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1836,6 +1836,7 @@ qemuAgentGetFSInfo(qemuAgentPtr mon, virDomainFSInfoPtr
**info,
virJSONValuePtr data;
virDomainFSInfoPtr *info_ret = NULL;
virPCIDeviceAddress pci_address;
+const char *result = NULL;
cmd = qemuAgentMakeCommand("guest-get-fsinfo", NULL);
if (!cmd)
@@ -1881,28 +1882,34 @@ qemuAgentGetFSInfo(qemuAgentPtr mon, virDomainFSInfoPtr
**info,
if (VIR_ALLOC(info_ret[i]) < 0)
goto cleanup;
-if (VIR_STRDUP(info_ret[i]->mountpoint,
- virJSONValueObjectGetString(entry, "mountpoint")) < 0) {
+if (!(result = virJSONValueObjectGetString(entry, "mountpoint"))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("'mountpoint' missing in reply of "
"guest-get-fsinfo"));
goto cleanup;
}
-if (VIR_STRDUP(info_ret[i]->name,
- virJSONValueObjectGetString(entry, "name")) < 0) {
+if (VIR_STRDUP(info_ret[i]->mountpoint, result) < 0)
+goto cleanup;
+
+if (!(result = virJSONValueObjectGetString(entry, "name"))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("'name' missing in reply of guest-get-fsinfo"));
goto cleanup;
}
-if (VIR_STRDUP(info_ret[i]->fstype,
- virJSONValueObjectGetString(entry, "type")) < 0) {
+if (VIR_STRDUP(info_ret[i]->name, result) < 0)
+goto cleanup;
+
+if (!(result = virJSONValueObjectGetString(entry, "type"))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("'type' missing in reply of guest-get-fsinfo"));
goto cleanup;
}
+if (VIR_STRDUP(info_ret[i]->fstype, result) < 0)
+goto cleanup;
+
if (!(entry = virJSONValueObjectGet(entry, "disk"))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("'disk' missing in reply of guest-get-fsinfo"));
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 0/4] Add .domainGetHostname() support for QEMU driver.
Hi, This series was run against 'syntax-check' test by patchew.org, which failed, please find the details below: Type: series Message-id: 20180905023833.4867-1-jcfara...@gmail.com Subject: [libvirt] [PATCH v4 0/4] Add .domainGetHostname() support for QEMU driver. === TEST SCRIPT BEGIN === #!/bin/bash # Testing script will be invoked under the git checkout with # HEAD pointing to a commit that has the patches applied on top of "base" # branch time bash -c './autogen.sh && make syntax-check' === TEST SCRIPT END === Updating bcb55ab053bc79561b55d0394490f4b64e0f2d01 >From https://github.com/patchew-project/libvirt * [new tag] patchew/20180905023833.4867-1-jcfara...@gmail.com -> patchew/20180905023833.4867-1-jcfara...@gmail.com Switched to a new branch 'test' fc0bcb4834 qemu: unlink the error report from VIR_STRDUP. 93561f8260 docs: Add QEMU-GA get hostname feature into news.xml d90c4b130d qemu: adding domainGetHostname support for QEMU d3e7a5c16a qemu: implementing qemuAgentGetHostname() function. === OUTPUT BEGIN === Updating submodules... Submodule 'gnulib' (https://git.savannah.gnu.org/git/gnulib.git/) registered for path '.gnulib' Submodule 'keycodemapdb' (https://gitlab.com/keycodemap/keycodemapdb.git) registered for path 'src/keycodemapdb' Cloning into '/var/tmp/patchew-tester-tmp-5vt_esz3/src/.gnulib'... Cloning into '/var/tmp/patchew-tester-tmp-5vt_esz3/src/src/keycodemapdb'... Submodule path '.gnulib': checked out '68df637b5f1b5c10370f6981d2a43a5cf74368df' Submodule path 'src/keycodemapdb': checked out '16e5b0787687d8904dad2c026107409eb9bfcb95' Running bootstrap... ./bootstrap: Bootstrapping from checked-out libvirt sources... ./bootstrap: consider installing git-merge-changelog from gnulib ./bootstrap: getting gnulib files... running: libtoolize --install --copy libtoolize: putting auxiliary files in AC_CONFIG_AUX_DIR, 'build-aux'. libtoolize: copying file 'build-aux/config.guess' libtoolize: copying file 'build-aux/config.sub' libtoolize: copying file 'build-aux/install-sh' libtoolize: copying file 'build-aux/ltmain.sh' libtoolize: putting macros in AC_CONFIG_MACRO_DIRS, 'm4'. libtoolize: copying file 'm4/libtool.m4' libtoolize: copying file 'm4/ltoptions.m4' libtoolize: copying file 'm4/ltsugar.m4' libtoolize: copying file 'm4/ltversion.m4' libtoolize: copying file 'm4/lt~obsolete.m4' ./bootstrap: .gnulib/gnulib-tool--no-changelog --aux-dir=build-aux --doc-base=doc --lib=libgnu --m4-base=m4/ --source-base=gnulib/lib/ --tests-base=gnulib/tests --local-dir=gnulib/local--lgpl=2 --with-tests --makefile-name=gnulib.mk --avoid=pt_chown --avoid=lock-tests --libtool --import ... Module list with included dependencies (indented): absolute-header accept accept-tests alloca alloca-opt alloca-opt-tests allocator areadlink areadlink-tests arpa_inet arpa_inet-tests assure autobuild base64 base64-tests binary-io binary-io-tests bind bind-tests bitrotate bitrotate-tests btowc btowc-tests builtin-expect byteswap byteswap-tests c-ctype c-ctype-tests c-strcase c-strcase-tests c-strcasestr c-strcasestr-tests calloc-posix canonicalize-lgpl canonicalize-lgpl-tests careadlinkat chown chown-tests clock-time cloexec cloexec-tests close close-tests configmake connect connect-tests count-leading-zeros count-leading-zeros-tests count-one-bits count-one-bits-tests ctype ctype-tests dirname-lgpl dosname double-slash-root dup dup-tests dup2 dup2-tests environ environ-tests errno errno-tests error execinfo exitfail extensions extern-inline fatal-signal fclose fclose-tests fcntl fcntl-h fcntl-h-tests fcntl-tests fd-hook fdatasync fdatasync-tests fdopen fdopen-tests fflush fflush-tests ffs ffs-tests ffsl ffsl-tests fgetc-tests filename flexmember float float-tests fnmatch fnmatch-tests fpieee fpucw fpurge fpurge-tests fputc-tests fread-tests freading freading-tests fseek fseek-tests fseeko fseeko-tests fstat fstat-tests fsync fsync-tests ftell ftell-tests ftello ftello-tests ftruncate ftruncate-tests func func-tests fwrite-tests getaddrinfo getaddrinfo-tests getcwd-lgpl getcwd-lgpl-tests getdelim getdelim-tests getdtablesize getdtablesize-tests getgroups getgroups-tests gethostname gethostname-tests getline getline-tests getopt-posix getopt-posix-tests getpagesize getpass getpeername getpeername-tests getprogname getprogname-tests getsockname getsockname-tests getsockopt getsockopt-tests gettext-h gettimeofday gettimeofday-tests getugroups gitlog-to-changelog
[libvirt] [PATCH v4 4/4] qemu: unlink the error report from VIR_STRDUP.
The function to retrieve the file system info using QEMU-GA is using
some conditionals to retrieve the info. This is wrong because the error
of some conditionals will be raised if VIR_STRDUP return errors and not
if some problem occurred with JSON.
Signed-off-by: Julio Faracco
---
src/qemu/qemu_agent.c | 19 +--
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index ac728becef..cf27f53437 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1836,6 +1836,7 @@ qemuAgentGetFSInfo(qemuAgentPtr mon, virDomainFSInfoPtr
**info,
virJSONValuePtr data;
virDomainFSInfoPtr *info_ret = NULL;
virPCIDeviceAddress pci_address;
+const char *result = NULL;
cmd = qemuAgentMakeCommand("guest-get-fsinfo", NULL);
if (!cmd)
@@ -1881,28 +1882,34 @@ qemuAgentGetFSInfo(qemuAgentPtr mon, virDomainFSInfoPtr
**info,
if (VIR_ALLOC(info_ret[i]) < 0)
goto cleanup;
-if (VIR_STRDUP(info_ret[i]->mountpoint,
- virJSONValueObjectGetString(entry, "mountpoint")) < 0) {
+if (!(result = virJSONValueObjectGetString(entry, "mountpoint"))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("'mountpoint' missing in reply of "
"guest-get-fsinfo"));
goto cleanup;
}
-if (VIR_STRDUP(info_ret[i]->name,
- virJSONValueObjectGetString(entry, "name")) < 0) {
+if (VIR_STRDUP(info_ret[i]->mountpoint, result) < 0)
+goto cleanup;
+
+if (!(result = virJSONValueObjectGetString(entry, "name"))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("'name' missing in reply of guest-get-fsinfo"));
goto cleanup;
}
-if (VIR_STRDUP(info_ret[i]->fstype,
- virJSONValueObjectGetString(entry, "type")) < 0) {
+ if (VIR_STRDUP(info_ret[i]->name, result) < 0)
+goto cleanup;
+
+if (!(result = virJSONValueObjectGetString(entry, "type"))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("'type' missing in reply of guest-get-fsinfo"));
goto cleanup;
}
+ if (VIR_STRDUP(info_ret[i]->fstype, result) < 0)
+goto cleanup;
+
if (!(entry = virJSONValueObjectGet(entry, "disk"))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("'disk' missing in reply of guest-get-fsinfo"));
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 2/4] qemu: adding domainGetHostname support for QEMU
This commit adds support to use the function qemuAgentGetHostname()
for obtain the domain hostname using QEMU-GA command.
Signed-off-by: Julio Faracco
---
src/qemu/qemu_driver.c | 42 ++
1 file changed, 42 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 07ea5473b6..2f8d6915e1 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19353,6 +19353,46 @@ qemuConnectGetCPUModelNames(virConnectPtr conn,
return virCPUGetModels(arch, models);
}
+
+static char *
+qemuDomainGetHostname(virDomainPtr dom,
+ unsigned int flags)
+{
+virQEMUDriverPtr driver = dom->conn->privateData;
+virDomainObjPtr vm = NULL;
+qemuAgentPtr agent;
+char *hostname = NULL;
+
+virCheckFlags(0, NULL);
+
+if (!(vm = qemuDomObjFromDomain(dom)))
+return NULL;
+
+if (virDomainGetHostnameEnsureACL(dom->conn, vm->def) < 0)
+goto cleanup;
+
+if (qemuDomainObjBeginAgentJob(driver, vm, QEMU_AGENT_JOB_QUERY) < 0)
+goto cleanup;
+
+if (virDomainObjCheckActive(vm) < 0)
+goto endjob;
+
+if (!qemuDomainAgentAvailable(vm, true))
+goto endjob;
+
+agent = qemuDomainObjEnterAgent(vm);
+ignore_value(qemuAgentGetHostname(agent, ));
+qemuDomainObjExitAgent(vm, agent);
+
+ endjob:
+qemuDomainObjEndAgentJob(vm);
+
+ cleanup:
+virDomainObjEndAPI();
+return hostname;
+}
+
+
static int
qemuDomainGetTime(virDomainPtr dom,
long long *seconds,
@@ -19399,6 +19439,7 @@ qemuDomainGetTime(virDomainPtr dom,
return ret;
}
+
static int
qemuDomainSetTime(virDomainPtr dom,
long long seconds,
@@ -21957,6 +21998,7 @@ static virHypervisorDriver qemuHypervisorDriver = {
.connectGetCPUModelNames = qemuConnectGetCPUModelNames, /* 1.1.3 */
.domainFSFreeze = qemuDomainFSFreeze, /* 1.2.5 */
.domainFSThaw = qemuDomainFSThaw, /* 1.2.5 */
+.domainGetHostname = qemuDomainGetHostname, /* 4.8.0 */
.domainGetTime = qemuDomainGetTime, /* 1.2.5 */
.domainSetTime = qemuDomainSetTime, /* 1.2.5 */
.nodeGetFreePages = qemuNodeGetFreePages, /* 1.2.6 */
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 0/4] Add .domainGetHostname() support for QEMU driver.
This serie adds a new function into QEMU Guest Agent handler to use the QEMU command 'guest-get-host-name' to retrieve the domain hostname. This approach requires QEMU-GA running inside the guest, but it is the fastest and easiest way to get this info. This serie has some suggestion made by John Ferlan for v3. Specially, some improvements to error handling. Julio Faracco (4): qemu: implementing qemuAgentGetHostname() function. qemu: adding domainGetHostname support for QEMU docs: Add QEMU-GA get hostname feature into news.xml qemu: unlink the error report from VIR_STRDUP. docs/news.xml | 9 ++ src/qemu/qemu_agent.c | 66 ++ src/qemu/qemu_agent.h | 4 +++ src/qemu/qemu_driver.c | 42 +++ 4 files changed, 115 insertions(+), 6 deletions(-) -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 1/4] qemu: implementing qemuAgentGetHostname() function.
This commit implements the function qemuAgentGetHostname() that uses
the QEMU guest agent command 'guest-get-host-name' to retrieve the
guest hostname of virtual machine running the QEMU-GA.
Signed-off-by: Julio Faracco
---
src/qemu/qemu_agent.c | 47 +++
src/qemu/qemu_agent.h | 4
2 files changed, 51 insertions(+)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index bf08871f18..ac728becef 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1683,6 +1683,53 @@ qemuAgentUpdateCPUInfo(unsigned int nvcpus,
}
+int
+qemuAgentGetHostname(qemuAgentPtr mon,
+ char **hostname)
+{
+int ret = -1;
+virJSONValuePtr cmd;
+virJSONValuePtr reply = NULL;
+virJSONValuePtr data = NULL;
+const char *result = NULL;
+
+cmd = qemuAgentMakeCommand("guest-get-host-name",
+ NULL);
+
+if (!cmd)
+return ret;
+
+if (qemuAgentCommand(mon, cmd, , true,
+ VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0)
+goto cleanup;
+
+if (qemuAgentCheckError(cmd, reply) < 0)
+goto cleanup;
+
+if (!(data = virJSONValueObjectGet(reply, "return"))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("malformed return value"));
+goto cleanup;
+}
+
+if (!(result = virJSONValueObjectGetString(data, "host-name"))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("'host-name' missing in guest-get-host-name reply"));
+goto cleanup;
+}
+
+if (VIR_STRDUP(*hostname, result) < 0)
+goto cleanup;
+
+ret = 0;
+
+ cleanup:
+virJSONValueFree(cmd);
+virJSONValueFree(reply);
+return ret;
+}
+
+
int
qemuAgentGetTime(qemuAgentPtr mon,
long long *seconds,
diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h
index 6dd9c702dd..4354b7e0cf 100644
--- a/src/qemu/qemu_agent.h
+++ b/src/qemu/qemu_agent.h
@@ -105,6 +105,10 @@ int qemuAgentUpdateCPUInfo(unsigned int nvcpus,
qemuAgentCPUInfoPtr cpuinfo,
int ncpuinfo);
+int
+qemuAgentGetHostname(qemuAgentPtr mon,
+ char **hostname);
+
int qemuAgentGetTime(qemuAgentPtr mon,
long long *seconds,
unsigned int *nseconds);
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 3/4] docs: Add QEMU-GA get hostname feature into news.xml
QEMU-GA supports get geust hostname command. This commit includes a specific entry to inform this new feature for QEMU driver to 4.8.0 release. Signed-off-by: Julio Faracco --- docs/news.xml | 9 + 1 file changed, 9 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 9a17b2f612..5ea1bd285d 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -35,6 +35,15 @@ + + + qemu: Retrieve guest hostname through QEMU Guest Agent command + + + QEMU is now able to retrieve the guest hostname using a new QEMU-GA + command called 'guest-get-host-name'. + + -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 1/2] vl.c deprecate incorrect CPUs topology
On Tue, Sep 04, 2018 at 03:22:35PM +0200, Igor Mammedov wrote:
> -smp [cpus],sockets/cores/threads[,maxcpus] should describe topology
> so that total number of logical CPUs [sockets * cores * threads]
> would be equal to [maxcpus], however historically we didn't have
> such check in QEMU and it is possible to start VM with an invalid
> topology.
> Deprecate invalid options combination so we can make sure that
> the topology VM started with is always correct in the future.
> Users with an invalid sockets/cores/threads/maxcpus values should
> fix their CLI to make sure that
>[sockets * cores * threads] == [maxcpus]
>
> Signed-off-by: Igor Mammedov
> ---
> v5:
> - extend deprecation doc, adding that maxcpus should be multiple of
> present on CLI [sockets/cores/threads] options
> (Eduardo Habkost )
> v4:
> - missed dot comment, fix it with s/./,/ (Andrew Jones )
> v3:
> - more spelling fixes (Andrew Jones )
> - place deprecation check after (sockets * cores * threads > max_cpus) check
> (Eduardo Habkost )
> v2:
> - spelling& fixes (Andrew Jones )
> ---
> qemu-deprecated.texi | 19 +++
> vl.c | 7 +++
> 2 files changed, 26 insertions(+)
>
> diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
> index 87212b6..827c3ce 100644
> --- a/qemu-deprecated.texi
> +++ b/qemu-deprecated.texi
> @@ -159,6 +159,25 @@ The 'file' driver for drives is no longer appropriate
> for character or host
> devices and will only accept regular files (S_IFREG). The correct driver
> for these file types is 'host_cdrom' or 'host_device' as appropriate.
>
> +@subsection -smp X,[socket=a,core=b,thread=c],maxcpus=Y (since 3.1)
Minor: I suggest using @var markup, or maybe just use
"-smp (invalid topologies) (since 3.1)" as subsection title for simplicity?
> +
> +CPU topology properties should describe whole machine topology including
> +possible CPUs, but historically it was possible to start QEMU with
> +an incorrect topology where
> + sockets * cores * threads >= X && X < maxcpus
Minor: this line formatting is lost on the HTML output. I
suggest using @var and/or @math.
Minor: I suggest not using C syntax.
i.e. use something like:
@math{@var{n} <= @var{sockets} * @var{cores} * @var{threads} < @var{maxcpus}},
> +which could lead to an incorrect topology enumeration by the guest.
> +Support for invalid topologies will be removed, the user must ensure
> +topologies described with -smp include all possible cpus, i.e.
> + sockets * cores * threads == maxcpus
Minor: same as above. I suggest:
@math{@var{sockets} * @var{cores} * @var{threads} = @var{maxcpus}}.
> +Note: it's assumed that maxcpus value must be multiple of the topology
> +options present on command line to avoid creating an invalid topology.
> +If maxcpus isn't be multiple of present topology options then the condition
> +(sockets * cores * threads == maxcpus) can't be satisfied and it will
> +trigger deprecation warning which later will be converted to a error.
> +If you get deprecation warning it's recommended to explicitly specify
> +a correct topology to make warning go away and ensure that it will
> +continue working in the future.
I don't understand the purpose of the "Note:" section. It seems to duplicate
what was already said in the lines above it. Can we just remove it?
These are just suggestions in case you are going to respin the series, not
blockers.
--
Eduardo
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v5 2/2] vl:c: make sure that sockets are calculated correctly in '-smp X' case
On Tue, Sep 04, 2018 at 05:18:48PM +0200, Andrew Jones wrote:
> On Tue, Sep 04, 2018 at 03:22:36PM +0200, Igor Mammedov wrote:
> > commit
> > (5cdc9b76e3 vl.c: Remove dead assignment)
> > removed sockets calculation when 'sockets' weren't provided on CLI
> > since there wasn't any users for it back then. Exiting checks
> > are neither reachable
> >} else if (sockets * cores * threads < cpus) {
> > or nor triggable
> >if (sockets * cores * threads > max_cpus)
> > so we weren't noticing wrong topology since then, since users
> > recalculate sockets adhoc on their own.
> >
> > However with deprecation check it becomes noticable, for example
> > -smp 2
> > will start printing warning:
> > "warning: Invalid CPU topology deprecated: sockets (1) * cores (1) *
> > threads (1) != maxcpus (2)"
> > calculating sockets if they weren't specified.
> >
> > Fix it by returning back sockets calculation if it's omited on CLI.
> >
> > Signed-off-by: Igor Mammedov
> > ---
> > vl.c | 5 -
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/vl.c b/vl.c
> > index 7fd700e..333d638 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -1210,11 +1210,14 @@ static void smp_parse(QemuOpts *opts)
> >
> > /* compute missing values, prefer sockets over cores over threads
> > */
> > if (cpus == 0 || sockets == 0) {
> > -sockets = sockets > 0 ? sockets : 1;
> > cores = cores > 0 ? cores : 1;
> > threads = threads > 0 ? threads : 1;
> > if (cpus == 0) {
> > +sockets = sockets > 0 ? sockets : 1;
> > cpus = cores * threads * sockets;
> > +} else {
> > +max_cpus = qemu_opt_get_number(opts, "maxcpus", cpus);
> > +sockets = !sockets ? max_cpus / (cores * threads) :
> > sockets;
Why the !sockets check here? If we have reached this statement
we know that (cpus == 0 || sockets == 0) is true and
(cpus == 0) is false, so sockets is guaranteed to be 0.
Sorry for not spotting this earlier.
> > }
> > } else if (cores == 0) {
> > threads = threads > 0 ? threads : 1;
> > --
> > 2.7.4
> >
> >
>
> Reviewed-by: Andrew Jones
--
Eduardo
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/10] Introduce x86 Cache Monitoring Technology (CMT)
hi reviewer, I understand libvirt community is quite active and you are quite busy. I am written here to know if you ever noticed this patch series, and welcome your comment. BR On 2018年08月27日 19:23, Wang Huaqiang wrote: This series of patches introduced the x86 Cache Monitoring Technology (CMT) to libvirt by interacting with kernel resource control (resctrl) interface. CMT is one of the Intel(R) x86 CPU feature which belongs to the Resource Director Technology (RDT). CMT reports the occupancy of the last level cache, which is shared by all CPU cores. We have serval discussion about the enabling of CMT, please refer to following links for the RFCs. RFCv3 https://www.redhat.com/archives/libvir-list/2018-August/msg01213.html RFCv2 https://www.redhat.com/archives/libvir-list/2018-July/msg00409.html https://www.redhat.com/archives/libvir-list/2018-July/msg01241.html RFCv1 https://www.redhat.com/archives/libvir-list/2018-June/msg00674.html 1. About reason why CMT is necessary in libvirt? The perf events of 'CMT, MBML, MBMT' have been phased out since Linux kernel commit c39a0e2c8850f08249383f2425dbd8dbe4baad69, in libvirt the perf based cmt,mbm will not work with the latest linux kernel. These patches add CMT feature to libvirt through kernel resctrlfs interface. 2. Interfaces for CMT from the high level. 2.1 Query the host capability of CMT. The element 'monitor' represents the host capabilities of CMT. The explanations of involved CMT attributes: - 'maxAllocs' denotes the maximum monitoring groups could be created, which is limited by the number of hardware 'RMID'. - 'threshold' denotes the upper bound of cache occupancy for current group, in bytes, to determine if an RMID can be reused. - element 'feature' denotes the monitoring feature supported. - 'llc_occupancy' is the feature for reporting the last level cache occupancy information. # virsh capabilities ... + + + + + + ... 2.2 Create cache monitoring group (cache monitor). The main interface for creating monitoring group is through XML file. The proposed configuration is like: + + In above XML, created 2 cache resctrl allocation groups and 2 resctrl monitoring groups. The changes of cache monitor will be effective in next booting of VM. 2.3 Show CMT result through command 'domstats' Adding the interface in qemu to report this information for resource monitor group through command 'virsh domstats --cpu-total'. Below is a typical output: # virsh domstats 1 --cpu-total Domain: 'ubuntu16.04-base' ... cpu.cache.monitor.count=2 cpu.cache.0.name=vcpus_1 cpu.cache.0.vcpus=1 cpu.cache.0.bank.count=2 cpu.cache.0.bank.0.id=0 cpu.cache.0.bank.0.bytes=4505600 cpu.cache.0.bank.1.id=1 cpu.cache.0.bank.1.bytes=5586944 cpu.cache.1.name=vcpus_4-6 cpu.cache.1.vcpus=4,5,6 cpu.cache.1.bank.count=2 cpu.cache.1.bank.0.id=0 cpu.cache.1.bank.0.bytes=17571840 cpu.cache.1.bank.1.id=1 cpu.cache.1.bank.1.bytes=29106176 **Changes Since RFCv3** In the output of 'domstats', added 'cpu.cache..bank..id' to tell the OS assigned cache bank id of current cache. Changes is prefixed with a '+': # virsh domstats 1 --cpu-total Domain: 'ubuntu16.04-base' ... cpu.cache.monitor.count=2 cpu.cache.0.name=vcpus_1 cpu.cache.0.vcpus=1 cpu.cache.0.bank.count=2 + cpu.cache.0.bank.0.id=0 cpu.cache.0.bank.0.bytes=4505600 + cpu.cache.0.bank.1.id=1 cpu.cache.0.bank.1.bytes=5586944 cpu.cache.1.name=vcpus_4-6 cpu.cache.1.vcpus=4,5,6 cpu.cache.1.bank.count=2 + cpu.cache.1.bank.0.id=0 cpu.cache.1.bank.0.bytes=17571840 + cpu.cache.1.bank.1.id=1 cpu.cache.1.bank.1.bytes=29106176 Wang Huaqiang (10): conf: Renamed 'controlBuf' to 'childrenBuf' util: add interface retrieving CMT capability conf: Add CMT capability to host test: add test case for resctrl monitor util: resctrl: refactoring some functions util: Introduce resctrl monitor for CMT conf: refactor virDomainResctrlAppend conf: introduce resctrl monitor group in domain qemu: Introduce resctrl monitoring group qemu: Report cache occupancy (CMT) with domstats .gnulib| 1 - docs/formatdomain.html.in | 14 +- docs/schemas/capability.rng| 28 + docs/schemas/domaincommon.rng | 11 +- src/conf/capabilities.c| 51 +-
Re: [libvirt] Virsh reference is out of date, should it be updated from the man page?
On 09/03/2018 01:09 PM, Ján Tomko wrote: > On Mon, Sep 03, 2018 at 10:04:16AM +0200, Michal Prívozník wrote: >> On 09/03/2018 09:38 AM, Povilas Kanapickas wrote: >>> Hi! >>> >>> The online virsh command reference at [1] seems to be very out of date >>> according to [2]. There's much more recent documentation at >>> tools/virsh.pod in the main libvirt repository. >> >> Yes, this is because of lacking manpower. As usual O:-) >> >>> >>> I believe it would be great to update the web docs as they are much more >>> accessible and convenient to use (especially the one-command-per-page >>> version). >>> > > AFAIK the "Virsh Command Reference" was intended to provide more > information than the man page, especially practical usage of the > commands, not just to have the manpage in HTML. > > So providing this extra content with examples would be more worthwhile > than just a copy of the manpage. > >>> What would be the best way to approach that? I suppose we could look >>> into automatically converting the current manpages into docbook and >>> using that to update the website? What do you think? >> > > Generating parts of the document automatically would help it stay > current, but it should not get in the way of adding more 'human touch' > to it. But a manpage copy might be better than nothing. > > We already generate the API documentation automatically, we have a > custom C parser in docs/apibuild.py and docs/hvsupport.pl > that generate some intermediate files - similar approach could be used > to generate a table of options and the availablity sections. > >> This sounds like the best way to go. virsh is changed virtually with >> each release so an automated tool that would regenerate the docs which >> could run at every release is certainly desired.> > > libvirt development goes much faster than that - it can be generated > hourly just like the rest of the autogenerated docs. > > Jano > >> However, I'm no docbook/manpages master, so I'll let somebody else look >> into it (perhaps yourself? ;-)) >> >> Michal >> Thanks for replies, I will come back again asking more questions when I start working in this. Regards, Povilas signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] domain XML for tracking libosinfo ID
Right now in virt-manager we only track a VM's OS name (win10, fedora28, etc.) during the VM install phase. This piece of data is important post-install though: if the user adds a new disk to the VM later, we want to be able to ask libosinfo about what devices the installed OS supports, so we can set optimal defaults, like enabling virtio. There isn't any standard libvirt XML field to track this kind of info though, so apps have to invent their own schema. nova and rhev do it indirectly AFAICT. gnome-boxes does it directly with XML like this: https://wiki.gnome.org/Apps/Boxes;> http://fedoraproject.org/fedora/28 I want to add something similar to virt-manager but it seems a shame to invent our own private schema for something that most non-trivial virt apps will want to know about. I was thinking a schema we could document with libosinfo, something like xmlns:libosinfo="http://libosinfo.org/xmlns/libvirt/domain/1.0;> http://fedoraproject.org/fedora/28 FWIW there's an ld bug about possible tracking something like this in the domain XML as a first class citizen: https://bugzilla.redhat.com/show_bug.cgi?id=509164 But I think nowadays that's a bad fit and is likely off the table Thoughts? - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 21/28] security_manager: Introduce virSecurityManagerLockCloseConn
On 09/04/2018 07:53 AM, Michal Privoznik wrote:
> On 08/31/2018 08:42 PM, John Ferlan wrote:
>>
>>
>> On 08/27/2018 04:08 AM, Michal Privoznik wrote:
>>> This is basically just a wrapper over virLockManagerCloseConn()
>>> so that no connection is left open when it shouldn't be.
>>>
>>> Signed-off-by: Michal Privoznik
>>> ---
>>> src/security/security_manager.c | 75
>>> +
>>> 1 file changed, 75 insertions(+)
>>>
>>> diff --git a/src/security/security_manager.c
>>> b/src/security/security_manager.c
>>> index caaff1f703..2238c75a5c 100644
>>> --- a/src/security/security_manager.c
>>> +++ b/src/security/security_manager.c
>>> @@ -91,6 +91,56 @@ virSecurityManagerLockDispose(void *obj)
>>> }
>>>
>>>
>>> +static void
>>> +virSecurityManagerLockCloseConnLocked(virSecurityManagerLockPtr lock,
>>> + bool force)
>>> +{
>>> +int rc;
>>> +
>>> +if (!lock)
>>> +return;
>>
>> Not possible - if this is true you may just want to abort or let the
>> following code fail miserably. The definition of the API is that it's
>> called when @lock is locked!
>>
>>> +
>>> +while (!force &&
>>> + lock->pathLocked) {
>>
>> No need to split && across 2 lines. Still waiting for @pathLocked =
>> true ;-)
>>
>>
>>> +if (virCondWait(>cond, >parent.lock) < 0) {
>>> +VIR_WARN("Unable to wait on metadata condition");
>>> +return;
>>> +}
>>> +}
>>> +
>>> +rc = virLockManagerCloseConn(lock->lock, 0);
>>
>> still haven't filled in lock->lock either
>>
>>> +if (rc < 0)
>>> +return;
>>> +if (rc > 0)
>>> +lock->lock = NULL;
>>> +
>>> +if (force) {
>>> +/* We've closed the connection. Wake up anybody who might be
>>
>> s/the/our/
>>
>>> + * waiting. */
>>> +lock->pathLocked = false;
>>> +virCondSignal(>cond);
>>> +}
>>> +}
>>> +
>>> +
>>> +static void
>>> +virSecurityManagerLockCloseConn(virSecurityManagerLockPtr lock)
>>> +{
>>> +if (!lock)
>>> +return;
>>
>> So this is lock->lock from a few patches ago.
>>
>>> +
>>> +virObjectLock(lock);
>>> +
>>> +if (!lock->lock)
>>
>> Still not seeing lock->lock, but maybe the next patch exposes that part.
>>
>>> +goto cleanup;
>>> +
>>> +virSecurityManagerLockCloseConnLocked(lock, false);
>>> +
>>> + cleanup:
>>> +virObjectUnlock(lock);
>>> +}
>>> +
>>> +
>>
>> I'm staring blankly at the rest of changes wondering, h... what am I
>> missing.
>>
>> Beyond that if the mgr->drv->*(mgr) isn't called, then should we call
>> the CloseConn. I'm not seeing why it's called in the first place!
>
> Because of the connection sharing (KEEP_OPEN flags) both
> AcquireResources and ReleaseResources will either use previously opened
> connection OR open a new one AND leave it open. This is important - that
> after the last ReleaseResource there is still a connection open to
> virtlockd. ReleaseResouce can't close the connection because it can't
> possibly know whether there is new acquire/release call pair waiting or
> not. It's security driver who knows that. And thus, the last one should
> close the connection.
>
> View from another angle, when relabelling a device we may end up
> relabelling one, two or even more paths. For instance:
>
> virSecurityManagerDomainSetPathLabel() relabels just one path,
>
> virSecurityManagerSetChardevLabel() or
> virSecurityManagerSetHostdevLabel() can relabel one, or many paths,
>
> and finally, virSecurityManagerSetAllLabel() will definitely relabel
> plenty of paths.
>
> Having said that, it's clear now that ReleaseResouce can't know if the
> path its releasing is the last one in the queue and thus if the
> connection can be closed. But the API knows that. So after security
> manager API has called the callback it knows no other path needs
> relabelling (i.e. acquire+release) and thus the connection can be closed.
>
> Except if there is not another thread that is already talking to
> virlockd and locking/unlocking paths for some other domain. Hence the
> connection refcounting in one of the previous patches.
>
> Yes, this is very complicated design. And if possible I'd like to
> simplify it. But I'm not successful on that front, sorry.
>
No need to apologize. The problem is complicated, tough to design, and
presents some interesting challenges with the various interactions.
Hopefully between your chat w/ mkletzan and this review you got feedback
that was helpful. Going forward when there is some assumed knowledge,
don't be shy in noting that in code comments.
In thinking more about a path related transaction solution - I begin to
wonder about interactions for/from iSCSI or LVM "devices" on a local
system that in the long run resolve to the same file. Is the fnctl using
one or the other path "hold true" and get managed somewhere underneath
libvirt in such a way that we don't have to "know" or "check" that
Re: [libvirt] [PATCH 2/2] qemu: Unify generation of command line for virtio devices
On Tue, Sep 04, 2018 at 05:32:51PM +0200, Andrea Bolognani wrote:
On Tue, 2018-09-04 at 16:09 +0200, Ján Tomko wrote:
On Fri, Aug 31, 2018 at 04:03:10PM +0200, Andrea Bolognani wrote:
> +static char*
> +qemuBuildVirtioDevStr(const virDomainDeviceInfo *info,
> + const char *baseName)
[...]
> +virBufferAsprintf(, "%s-%s", baseName, implName);
buf is used exactly once in this function, could have been just
virAsprintf.
Or, even better, since all the calls are followed by adding the string
to a buffer, just pass the buffer as the function argument.
I did it that way initially, but then I changed it to return
a char* to be consistent with other qemuBuild*DevStr(). I can
definitely change it back, but perhaps a different name would
be more appropriate at that point.
OTOH, many qemuBuild.*Str which only build a repetitive part of the
string have a virBuffer as the first argument.
If the DevStr inconsistency bothers you, maybe
'qemuBuildVirtioDeviceStr' or qemuBuildVirtioDeviceSuffix{,Str}?
Jano
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 20/28] security_manager: Load lock plugin on init
On 09/03/2018 11:13 AM, Michal Privoznik wrote:
> On 08/31/2018 07:35 PM, John Ferlan wrote:
>>
>>
>> On 08/27/2018 04:08 AM, Michal Privoznik wrote:
>>> When creating the security managers stack load the lock plugin
>>> too. This is done by creating a single object that all secdrivers
>>> take a reference to. We have to have one shared object so that
>>> the connection to virlockd can be shared between individual
>>> secdrivers. It is important that the connection is shared because
>>> if the connection is closed from one driver while other has a
>>> file locked, then virtlockd does its job and kills libvirtd.
>>>
>>> The cfg.mk change is needed in order to allow syntax-check
>>> to include lock_manager.h. This is generally safe thing to do as
>>> this APIs defined there will always exist. However, instead of
>>> allowing the include for all other drivers (like cpu, network,
>>> and so on) allow it only for security driver. This will still
>>> trigger the error if including from other drivers.
>>>
>>> Signed-off-by: Michal Privoznik
>>> ---
>>> cfg.mk | 4 +-
>>> src/qemu/qemu_driver.c | 12 --
>>> src/security/security_manager.c | 81
>>> -
>>> src/security/security_manager.h | 3 +-
>>> tests/testutilsqemu.c | 2 +-
>>> 5 files changed, 94 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/cfg.mk b/cfg.mk
>>> index 609ae869c2..e0a7b5105a 100644
>>> --- a/cfg.mk
>>> +++ b/cfg.mk
>>> @@ -787,8 +787,10 @@ sc_prohibit_cross_inclusion:
>>> case $$dir in \
>>> util/) safe="util";; \
>>> access/ | conf/) safe="($$dir|conf|util)";; \
>>> - cpu/| network/| node_device/| rpc/| security/| storage/) \
>>> + cpu/| network/| node_device/| rpc/| storage/) \
>>> safe="($$dir|util|conf|storage)";; \
>>> + security/) \
>>> + safe="($$dir|util|conf|storage|locking)";; \
>>> xenapi/ | xenconfig/ ) safe="($$dir|util|conf|xen|cpu)";; \
>>> *) safe="($$dir|$(mid_dirs)|util)";; \
>>> esac; \
>>
>> This I don't really understand - black magic, voodoo stuff ;-)
>
> This is a simple bash version of switch-case. Except in bash you'd
> s/switch/case/ (because why not, right?). So what this says is: if $dir
> is "util/" then safe="util"; or if $dir is either of "cpu/", "network/",
> "node_device/" then safe is "($dir|util|conf|storage)", of if $dir
> is "security/" then safe is "($dir|util|conf|storage|locking)".
>
> Long story short, this shuts up 'syntax-check' because without it it
> complains about "unsafe" cross-directory include. Hmpf.
>
Still black magic... essentially though you've added "locking" to safe
list of tree's that security/* files can access.
>>
>>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>>> index da8c4e8991..e06dee8dfb 100644
>>> --- a/src/qemu/qemu_driver.c
>>> +++ b/src/qemu/qemu_driver.c
>>> @@ -358,7 +358,9 @@ qemuSecurityInit(virQEMUDriverPtr driver)
>>> flags)))
>>> goto error;
>>> if (!stack) {
>>> -if (!(stack = qemuSecurityNewStack(mgr)))
>>> +if (!(stack = qemuSecurityNewStack(mgr,
>>> +
>>> cfg->metadataLockManagerName ?
>>> +
>>> cfg->metadataLockManagerName : "nop")))
>>> goto error;
>>> } else {
>>> if (qemuSecurityStackAddNested(stack, mgr) < 0)
>>> @@ -372,7 +374,9 @@ qemuSecurityInit(virQEMUDriverPtr driver)
>>> QEMU_DRIVER_NAME,
>>> flags)))
>>> goto error;
>>> -if (!(stack = qemuSecurityNewStack(mgr)))
>>> +if (!(stack = qemuSecurityNewStack(mgr,
>>> + cfg->metadataLockManagerName ?
>>> + cfg->metadataLockManagerName :
>>> "nop")))
>>> goto error;
>>> mgr = NULL;
>>> }
>>> @@ -389,7 +393,9 @@ qemuSecurityInit(virQEMUDriverPtr driver)
>>> qemuSecurityChownCallback)))
>>> goto error;
>>> if (!stack) {
>>> -if (!(stack = qemuSecurityNewStack(mgr)))
>>> +if (!(stack = qemuSecurityNewStack(mgr,
>>> +
>>> cfg->metadataLockManagerName ?
>>> +
>>> cfg->metadataLockManagerName : "nop")))
>>
>> This essentially gets called through the driver state initialize
>> function, right? But, wouldn't daemon locks be at a different level?
>> The domain locks would be managed by whatever is managing the domain,
>> the the daemon locks would be managed by whatever is managing the daemon
>> locks, wouldn't they?
>>
>> This also assumes that security_driver is
[libvirt] [PATCH v6 1/2] vl.c deprecate incorrect CPUs topology
-smp [cpus],sockets/cores/threads[,maxcpus] should describe topology
so that total number of logical CPUs [sockets * cores * threads]
would be equal to [maxcpus], however historically we didn't have
such check in QEMU and it is possible to start VM with an invalid
topology.
Deprecate invalid options combination so we can make sure that
the topology VM started with is always correct in the future.
Users with an invalid sockets/cores/threads/maxcpus values should
fix their CLI to make sure that
[sockets * cores * threads] == [maxcpus]
Signed-off-by: Igor Mammedov
Reviewed-by: Andrew Jones
---
v6:
- s/socket/sockets/, the same for core, thread in subsection
(Andrew Jones )
v5:
- extend deprecation doc, adding that maxcpus should be multiple of
present on CLI [sockets/cores/threads] options
(Eduardo Habkost )
v4:
- missed dot comment, fix it with s/./,/ (Andrew Jones )
v3:
- more spelling fixes (Andrew Jones )
- place deprecation check after (sockets * cores * threads > max_cpus) check
(Eduardo Habkost )
v2:
- spelling& fixes (Andrew Jones )
---
qemu-deprecated.texi | 19 +++
vl.c | 7 +++
2 files changed, 26 insertions(+)
diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
index 87212b6..1fac52f 100644
--- a/qemu-deprecated.texi
+++ b/qemu-deprecated.texi
@@ -159,6 +159,25 @@ The 'file' driver for drives is no longer appropriate for
character or host
devices and will only accept regular files (S_IFREG). The correct driver
for these file types is 'host_cdrom' or 'host_device' as appropriate.
+@subsection -smp X,[sockets=a,cores=b,threads=c],maxcpus=Y (since 3.1)
+
+CPU topology properties should describe whole machine topology including
+possible CPUs, but historically it was possible to start QEMU with
+an incorrect topology where
+ sockets * cores * threads >= X && X < maxcpus
+which could lead to an incorrect topology enumeration by the guest.
+Support for invalid topologies will be removed, the user must ensure
+topologies described with -smp include all possible cpus, i.e.
+ sockets * cores * threads == maxcpus
+Note: it's assumed that maxcpus value must be multiple of the topology
+options present on command line to avoid creating an invalid topology.
+If maxcpus isn't be multiple of present topology options then the condition
+(sockets * cores * threads == maxcpus) can't be satisfied and it will
+trigger deprecation warning which later will be converted to a error.
+If you get deprecation warning it's recommended to explicitly specify
+a correct topology to make warning go away and ensure that it will
+continue working in the future.
+
@section QEMU Machine Protocol (QMP) commands
@subsection block-dirty-bitmap-add "autoload" parameter (since 2.12.0)
diff --git a/vl.c b/vl.c
index 5ba06ad..7fd700e 100644
--- a/vl.c
+++ b/vl.c
@@ -1246,6 +1246,13 @@ static void smp_parse(QemuOpts *opts)
exit(1);
}
+if (sockets * cores * threads != max_cpus) {
+warn_report("Invalid CPU topology deprecated: "
+"sockets (%u) * cores (%u) * threads (%u) "
+"!= maxcpus (%u)",
+sockets, cores, threads, max_cpus);
+}
+
smp_cpus = cpus;
smp_cores = cores;
smp_threads = threads;
--
2.7.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] qemu: Unify generation of command line for virtio devices
On Tue, 2018-09-04 at 16:09 +0200, Ján Tomko wrote:
> On Fri, Aug 31, 2018 at 04:03:10PM +0200, Andrea Bolognani wrote:
> > +static char*
> > +qemuBuildVirtioDevStr(const virDomainDeviceInfo *info,
> > + const char *baseName)
[...]
> > +virBufferAsprintf(, "%s-%s", baseName, implName);
>
> buf is used exactly once in this function, could have been just
> virAsprintf.
>
> Or, even better, since all the calls are followed by adding the string
> to a buffer, just pass the buffer as the function argument.
I did it that way initially, but then I changed it to return
a char* to be consistent with other qemuBuild*DevStr(). I can
definitely change it back, but perhaps a different name would
be more appropriate at that point.
[...]
> > +if (!(devStr = qemuBuildVirtioDevStr(&(disk->info), "virtio-blk")))
>
> >info is enough.
>
> Or even simpler:
> disk->info.type
Okay, I'll go with the latter.
[...]
> > +if (disk->iothread &&
> > +(disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW ||
> > + disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) {
> > +virBufferAsprintf(, ",iothread=iothread%u",
> > disk->iothread);
>
> The iothread change could be separated for an easier-to-read patch.
Agreed, it should have been that way to begin with.
> Also, formatting it unconditionally would be nicer - do we even need to
> check for the address type? We format it unconditionally for the
> virtio-scsi controller.
Not sure. I don't really know anything about iothreads,
so I purposefully steered clear of changing that part :)
[...]
> > +virBufferAddStr(, devStr);
> > +VIR_FREE(devStr);
> > +} else {
> > +virBufferAddStr(, nic);
>
> virBufferAddStr(, net->model);
>
> Since we no longer use 'nic' unconditionally.
Okay.
[...]
> > case VIR_DOMAIN_INPUT_TYPE_TABLET:
> > -if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_TABLET) ||
> > -(dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW &&
> > - !virQEMUCapsGet(qemuCaps,
> > QEMU_CAPS_DEVICE_VIRTIO_TABLET_CCW))) {
> > -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> > - _("virtio-tablet is not supported by this QEMU
> > binary"));
> > -goto error;
> > -}
>
> The capability check refactor is out of scope of this patch.
> Also, they should be in the *Validate functions, not the command line
> formatter.
Agreed, I'll split that part off.
[...]
> > case VIR_DOMAIN_INPUT_TYPE_LAST:
> > -break;
> > +default:
> > +virReportEnumRangeError(virDomainInputType,
> > +dev->type);
> > +goto error;
> > +}
>
> Unrelated virReportEnumRangeError addition.
I'll make that a separate patch too.
[...]
> > +if (dev->type == VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH) {
> > +virBufferAddLit(, ",evdev=");
> > +virQEMUBuildBufferEscapeComma(, dev->source.evdev);
>
> Personally, I'd also separate all the changes that remove the additional
> attributes from the device name.
Agreed.
[...]
> > -if (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)
> > -virBufferAsprintf(, "virtio-rng-ccw,rng=obj%s,id=%s",
> > - dev->info.alias, dev->info.alias);
> > -else if (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390)
> > -virBufferAsprintf(, "virtio-rng-s390,rng=obj%s,id=%s",
> > - dev->info.alias, dev->info.alias);
> > -else if (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO)
> > -virBufferAsprintf(, "virtio-rng-device,rng=obj%s,id=%s",
> > - dev->info.alias, dev->info.alias);
>
> Oh, fun. Not only we merged this copy & paste rng= attribute addition,
> it was later amended to add the id= attribute. Then broken into separate
> lines by a separate commit.
Well, at least now we're going to get rid of the duplication :)
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-snmp][PATCH] rpm: simplify applying of patches
The distros we support for RPM builds all have %autosetup support. Use that instead of what we currently have (nothing). Signed-off-by: Michal Privoznik --- Pushed under trivial rule and also 'who cares about snmp' rules ;-) libvirt-snmp.spec.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libvirt-snmp.spec.in b/libvirt-snmp.spec.in index c5a1a41..21a9885 100644 --- a/libvirt-snmp.spec.in +++ b/libvirt-snmp.spec.in @@ -15,7 +15,7 @@ BuildRequires: net-snmp-perl net-snmp net-snmp-utils net-snmp-devel libvirt-deve Provides a way to control libvirt through SNMP protocol. %prep -%setup -q +%autosetup -S git_am %build -- 2.16.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v5 2/2] vl:c: make sure that sockets are calculated correctly in '-smp X' case
On Tue, Sep 04, 2018 at 03:22:36PM +0200, Igor Mammedov wrote:
> commit
> (5cdc9b76e3 vl.c: Remove dead assignment)
> removed sockets calculation when 'sockets' weren't provided on CLI
> since there wasn't any users for it back then. Exiting checks
> are neither reachable
>} else if (sockets * cores * threads < cpus) {
> or nor triggable
>if (sockets * cores * threads > max_cpus)
> so we weren't noticing wrong topology since then, since users
> recalculate sockets adhoc on their own.
>
> However with deprecation check it becomes noticable, for example
> -smp 2
> will start printing warning:
> "warning: Invalid CPU topology deprecated: sockets (1) * cores (1) *
> threads (1) != maxcpus (2)"
> calculating sockets if they weren't specified.
>
> Fix it by returning back sockets calculation if it's omited on CLI.
>
> Signed-off-by: Igor Mammedov
> ---
> vl.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/vl.c b/vl.c
> index 7fd700e..333d638 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1210,11 +1210,14 @@ static void smp_parse(QemuOpts *opts)
>
> /* compute missing values, prefer sockets over cores over threads */
> if (cpus == 0 || sockets == 0) {
> -sockets = sockets > 0 ? sockets : 1;
> cores = cores > 0 ? cores : 1;
> threads = threads > 0 ? threads : 1;
> if (cpus == 0) {
> +sockets = sockets > 0 ? sockets : 1;
> cpus = cores * threads * sockets;
> +} else {
> +max_cpus = qemu_opt_get_number(opts, "maxcpus", cpus);
> +sockets = !sockets ? max_cpus / (cores * threads) : sockets;
> }
> } else if (cores == 0) {
> threads = threads > 0 ? threads : 1;
> --
> 2.7.4
>
>
Reviewed-by: Andrew Jones
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH] guests: Special-case fedora-gpg-keys updates on Rawhide
On Tue, Sep 04, 2018 at 03:59:23PM +0200, Andrea Bolognani wrote: > During each Rawhide development cycle there is a point > at which packages start being signed with new keys, which > causes updates to fail. > > To work around the problem, make sure fedora-gpg-keys is > updated before attempting to update all other packages; > updating fedora-gpg-keys itself requires gpg signature > checking to be disabled. > > Signed-off-by: Andrea Bolognani > --- > I am actually not 100% sure we need to disable gpg > signature checking in order to update fedora-gpg-keys: > it would make sense for that one package to be signed > with the old key to make the update possible without > breaking trust at any point in time. Unfortunately I > updated my Rawhide guest without taking a snapshot > first, and I can't figure out a way to get it back to > a state suitable for checking whether the above makes > sense :( Perhaps someone with deeper understanding of > the Fedora release process will confirm or deny. > guests/lcitool | 24 +--- > guests/playbooks/update/tasks/base.yml | 9 + > 2 files changed, 26 insertions(+), 7 deletions(-) After chatting with one of the Fedora team about this, we came to conclusion there's no nicer option right now, so Reviewed-by: Daniel P. Berrangé Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: skip some unicode tests if expected output won't match
On Tue, Sep 04, 2018 at 04:57:42PM +0200, Simon Kobyda wrote:
> On Tue, 2018-09-04 at 11:30 +0100, Daniel P. Berrangé wrote:
> > The expected output strings from the vshtabletest.c are created on a
> > modern Linux host where unicode printing support is very good. On
> > older
> > Linux platforms, or non-Linux platforms, some unicode characters will
> > not be considered printable. While the vsh table alignment code will
> > stil do the right thing with escaping & aligning in this case, the
> > result will not match the test's expected output.
> >
> > Since we know the code is working correctly, do a check with
> > iswprint()
> > to validate the platform's quality and skip the test if it fails.
> > This
> > fixes the test on FreeBSD platforms.
> >
> > Signed-off-by: Daniel P. Berrangé
> > ---
> >
> > Pushed as a build fix
> >
> > tests/vshtabletest.c | 19 +++
> > 1 file changed, 19 insertions(+)
> >
> > diff --git a/tests/vshtabletest.c b/tests/vshtabletest.c
> > index 9e9c045226..1138e34161 100644
> > --- a/tests/vshtabletest.c
> > +++ b/tests/vshtabletest.c
> > @@ -21,6 +21,7 @@
> > #include
> > #include
> > #include
> > +#include
> >
> > #include "internal.h"
> > #include "testutils.h"
> > @@ -158,6 +159,15 @@ testUnicodeArabic(const void *opaque
> > ATTRIBUTE_UNUSED)
> > " 1 ﻉﺪﻴﻟ ﺎﻠﺜﻘﻴﻟ ﻕﺎﻣ ﻊﻧ, ٣٠ ﻎﻴﻨﻳﺍ ﻮﺘﻧﺎﻤﺗ ﺎﻠﺛﺎﻠﺛ، ﺄﺳﺭ,
> > ﺩﻮﻟ ﺩﻮﻟ. ﺄﻣﺎﻣ ﺍ ﺎﻧ ﻲﻜﻧ \n"
> > " ﺺﻔﺣﺓ ﺖﻜﺘﻴﻛﺍً ﻊﻟ, ﺎﻠﺠﻧﻭﺩ
> > ﻭﺎﻠﻌﺗﺍﺩ ﺵﺭ \n";
> > vshTablePtr table;
> > +wchar_t wc;
> > +
> > +/* If this char is not classed as printable, the actual
> > + * output won't match what this test expects. The code
> > + * is still operating correctly, but we have different
> > + * layout */
> > +mbrtowc(, "،", MB_CUR_MAX, NULL);
> > +if (!iswprint(wc))
> > +return EXIT_AM_SKIP;
> >
> > table = vshTableNew("ﻡﺍ ﻢﻣﺍ ﻕﺎﺌﻣﺓ", "ﺓ ﺎﻠﺼﻋ", "ﺍﻸﺜﻧﺎﻧ", NULL);
> > if (!table)
> > @@ -192,6 +202,15 @@ testUnicodeZeroWidthChar(const void *opaque
> > ATTRIBUTE_UNUSED)
> > " 1\u200Bfedora28 run\u200Bning \n"
> > " 2rhel7.5running \n";
> > char *act = NULL;
> > +wchar_t wc;
> > +
> > +/* If this char is not classed as printable, the actual
> > + * output won't match what this test expects. The code
> > + * is still operating correctly, but we have different
> > + * layout */
> > +mbrtowc(, "\u200B", MB_CUR_MAX, NULL);
> > +if (!iswprint(wc))
> > +return EXIT_AM_SKIP;
>
> Sidenote: This test case with zero-width characters would pass without
> any problems if we implement environment variable
> "gl_cv_func_wcwidth_works=no" as suggested here:
I don't really consider setting magic env vars before configure to
be a satisfactory solution, as it requires the person building
libvirt to somehow find out that they need this obscure env var.
We need to be success out of the box with a normal run of configure.
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v5 1/2] vl.c deprecate incorrect CPUs topology
On Tue, Sep 04, 2018 at 03:22:35PM +0200, Igor Mammedov wrote:
> -smp [cpus],sockets/cores/threads[,maxcpus] should describe topology
> so that total number of logical CPUs [sockets * cores * threads]
> would be equal to [maxcpus], however historically we didn't have
> such check in QEMU and it is possible to start VM with an invalid
> topology.
> Deprecate invalid options combination so we can make sure that
> the topology VM started with is always correct in the future.
> Users with an invalid sockets/cores/threads/maxcpus values should
> fix their CLI to make sure that
>[sockets * cores * threads] == [maxcpus]
>
> Signed-off-by: Igor Mammedov
> ---
> v5:
> - extend deprecation doc, adding that maxcpus should be multiple of
> present on CLI [sockets/cores/threads] options
> (Eduardo Habkost )
> v4:
> - missed dot comment, fix it with s/./,/ (Andrew Jones )
> v3:
> - more spelling fixes (Andrew Jones )
> - place deprecation check after (sockets * cores * threads > max_cpus) check
> (Eduardo Habkost )
> v2:
> - spelling& fixes (Andrew Jones )
> ---
> qemu-deprecated.texi | 19 +++
> vl.c | 7 +++
> 2 files changed, 26 insertions(+)
>
> diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
> index 87212b6..827c3ce 100644
> --- a/qemu-deprecated.texi
> +++ b/qemu-deprecated.texi
> @@ -159,6 +159,25 @@ The 'file' driver for drives is no longer appropriate
> for character or host
> devices and will only accept regular files (S_IFREG). The correct driver
> for these file types is 'host_cdrom' or 'host_device' as appropriate.
>
> +@subsection -smp X,[socket=a,core=b,thread=c],maxcpus=Y (since 3.1)
sockets,cores,threads -- they should all be plural
> +
> +CPU topology properties should describe whole machine topology including
> +possible CPUs, but historically it was possible to start QEMU with
> +an incorrect topology where
> + sockets * cores * threads >= X && X < maxcpus
> +which could lead to an incorrect topology enumeration by the guest.
> +Support for invalid topologies will be removed, the user must ensure
> +topologies described with -smp include all possible cpus, i.e.
> + sockets * cores * threads == maxcpus
> +Note: it's assumed that maxcpus value must be multiple of the topology
> +options present on command line to avoid creating an invalid topology.
> +If maxcpus isn't be multiple of present topology options then the condition
> +(sockets * cores * threads == maxcpus) can't be satisfied and it will
> +trigger deprecation warning which later will be converted to a error.
> +If you get deprecation warning it's recommended to explicitly specify
> +a correct topology to make warning go away and ensure that it will
> +continue working in the future.
> +
> @section QEMU Machine Protocol (QMP) commands
>
> @subsection block-dirty-bitmap-add "autoload" parameter (since 2.12.0)
> diff --git a/vl.c b/vl.c
> index 5ba06ad..7fd700e 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1246,6 +1246,13 @@ static void smp_parse(QemuOpts *opts)
> exit(1);
> }
>
> +if (sockets * cores * threads != max_cpus) {
> +warn_report("Invalid CPU topology deprecated: "
> +"sockets (%u) * cores (%u) * threads (%u) "
> +"!= maxcpus (%u)",
> +sockets, cores, threads, max_cpus);
> +}
> +
> smp_cpus = cpus;
> smp_cores = cores;
> smp_threads = threads;
> --
> 2.7.4
>
>
Otherwise
Reviewed-by: Andrew Jones
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: skip some unicode tests if expected output won't match
On Tue, 2018-09-04 at 11:30 +0100, Daniel P. Berrangé wrote:
> The expected output strings from the vshtabletest.c are created on a
> modern Linux host where unicode printing support is very good. On
> older
> Linux platforms, or non-Linux platforms, some unicode characters will
> not be considered printable. While the vsh table alignment code will
> stil do the right thing with escaping & aligning in this case, the
> result will not match the test's expected output.
>
> Since we know the code is working correctly, do a check with
> iswprint()
> to validate the platform's quality and skip the test if it fails.
> This
> fixes the test on FreeBSD platforms.
>
> Signed-off-by: Daniel P. Berrangé
> ---
>
> Pushed as a build fix
>
> tests/vshtabletest.c | 19 +++
> 1 file changed, 19 insertions(+)
>
> diff --git a/tests/vshtabletest.c b/tests/vshtabletest.c
> index 9e9c045226..1138e34161 100644
> --- a/tests/vshtabletest.c
> +++ b/tests/vshtabletest.c
> @@ -21,6 +21,7 @@
> #include
> #include
> #include
> +#include
>
> #include "internal.h"
> #include "testutils.h"
> @@ -158,6 +159,15 @@ testUnicodeArabic(const void *opaque
> ATTRIBUTE_UNUSED)
> " 1 ﻉﺪﻴﻟ ﺎﻠﺜﻘﻴﻟ ﻕﺎﻣ ﻊﻧ, ٣٠ ﻎﻴﻨﻳﺍ ﻮﺘﻧﺎﻤﺗ ﺎﻠﺛﺎﻠﺛ، ﺄﺳﺭ,
> ﺩﻮﻟ ﺩﻮﻟ. ﺄﻣﺎﻣ ﺍ ﺎﻧ ﻲﻜﻧ \n"
> " ﺺﻔﺣﺓ ﺖﻜﺘﻴﻛﺍً ﻊﻟ, ﺎﻠﺠﻧﻭﺩ
> ﻭﺎﻠﻌﺗﺍﺩ ﺵﺭ \n";
> vshTablePtr table;
> +wchar_t wc;
> +
> +/* If this char is not classed as printable, the actual
> + * output won't match what this test expects. The code
> + * is still operating correctly, but we have different
> + * layout */
> +mbrtowc(, "،", MB_CUR_MAX, NULL);
> +if (!iswprint(wc))
> +return EXIT_AM_SKIP;
>
> table = vshTableNew("ﻡﺍ ﻢﻣﺍ ﻕﺎﺌﻣﺓ", "ﺓ ﺎﻠﺼﻋ", "ﺍﻸﺜﻧﺎﻧ", NULL);
> if (!table)
> @@ -192,6 +202,15 @@ testUnicodeZeroWidthChar(const void *opaque
> ATTRIBUTE_UNUSED)
> " 1\u200Bfedora28 run\u200Bning \n"
> " 2rhel7.5running \n";
> char *act = NULL;
> +wchar_t wc;
> +
> +/* If this char is not classed as printable, the actual
> + * output won't match what this test expects. The code
> + * is still operating correctly, but we have different
> + * layout */
> +mbrtowc(, "\u200B", MB_CUR_MAX, NULL);
> +if (!iswprint(wc))
> +return EXIT_AM_SKIP;
Sidenote: This test case with zero-width characters would pass without
any problems if we implement environment variable
"gl_cv_func_wcwidth_works=no" as suggested here:
https://lists.gnu.org/archive/html/bug-gnulib/2018-08/msg00165.html.
But that's not something important.
I'm happy that tests on freebsd pass :).
Simon.
>
> table = vshTableNew("I\u200Bd", "Name", "\u200BStatus", NULL);
> if (!table)
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 2/3] Split off MinGW builds
On Tue, 2018-09-04 at 16:11 +0200, Erik Skultety wrote: > On Tue, Sep 04, 2018 at 01:01:34PM +0200, Andrea Bolognani wrote: > > Up until now, we've been considering MinGW builds as > > part of the respective project, at least when it > > comes to grouping them. This, however, does not quite > > work for a number of reasons: > > > > * MinGW builds have their own workspace, separate > > from the native one. It goes further than that: > > even the 32-bit and 64-bit builds use a workspace > > each, which goes to show that grouping all three > > together is inaccurate; > > Well, I'd say that if someone cares enough about libvirt mingw builds, then > they > probably care about both ABI versions in which case they'd most probably > prefer > not having to specify for which x86 arch build they're interested in, > therefore > having both of these in the same project playbook, e.g. libvirt+mingw. Fair enough, but that's still supported quite naturally by using 'libvirt+mingw*', whereas the opposite wouldn't be true if we grouped them together; moreover, grouping them together ignores the fact, mentioned above, that they already use separate workspaces, and would additionally mean we'd have to keep the concept of 'variant' around instead of getting rid of it and wouldn't be able to have the separation cleanly mirrored in every aspect from project names (in the vars/projects/ and host_vars/*/ sense) to Jenkins job names. All in all, it looks to me like a partial split has several disadvantages over a complete split and zero advantages, so I'd rather not go down that road. -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [snmp PATCH] Fix wrong object OIDs sent by libvirtMib_subagent
On 09/03/2018 02:44 PM, Miguel Martin wrote: > The objects OIDs sent by the guest agent are: > libvirtGuestName.0 > libvirtGuestUUID.1 > libvirtGuestState.2 > libvirtGuestRowStatus.3 > > The expected libvirtGuestNotif objects OID would be: > libvirtGuestName.0 > libvirtGuestUUID.0 > libvirtGuestState.0 > libvirtGuestRowStatus.0 > > Signed-off-by: Miguel Martin > Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1624839 > --- > src/libvirtNotifications.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) ACKed and pushed. Congratulations on your first libvirt-snmp contribution! Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 14/28] lock_daemon_dispatch: Check for ownerPid rather than ownerId
On 09/03/2018 11:13 AM, Michal Privoznik wrote: > On 08/30/2018 11:47 PM, John Ferlan wrote: >> >> >> On 08/27/2018 04:08 AM, Michal Privoznik wrote: >>> At the beginning of each dispatch function we check if owner >>> attributes were registered (these consist of ID, UUID, PID and >>> name). The check then consists of checking if ID is not zero. >>> This is not going to work with >>> VIR_LOCK_MANAGER_OBJECT_TYPE_DAEMON which doesn't set ID. Switch >>> to setting PID which is available for both cases. >>> >>> Signed-off-by: Michal Privoznik >>> --- >>> src/locking/lock_daemon_dispatch.c | 14 +++--- >>> 1 file changed, 7 insertions(+), 7 deletions(-) >>> >> >> BTW: My idea of setting id == -1 for deamon still works without any >> change required in/for this patch. >> >> So what would be concerning about using ownerPid would be that over the >> lifetime of the host the @pid can recycle; whereas, during the lifetime >> of the daemon don't we guarantee that the @id will be ever increasing? >> But right now I'm too lazy to go look and see if getting the next id is >> through libvirtd or virtlockd. >> >> Not against this, but I need to get feedback from earlier patches and of >> course your thoughts on the @id vs. @pid rotation. Plus I need see how >> this plays out in future patches. > > The fact that @id always increases plays no role here and also is not > true. It's only qemu driver that has increasing IDs. For instance, libxl > driver which also uses virtlockd generates 'random' domain IDs. I saw your query on #virt ;-) > > Pid rotation is no problem here. What my patches currently implement is: > libvirtd registers itself as owner of the lock (with its PID). It has to > keep the connection to virtlockd open since the first lock() to the very > last unlock(), otherwise virtlockd sees EOF on the connection, and > starts releasing locks associated with the PID (=libvirtd) killing the > PID too [1]. So there is no way another process can emerge with the same > PID and magically keep the locks or something. At the moment libvirtd > dies the connection is closed and virlockd releases the locks. There's > no window for PID wrap. Well, we know our consumer libvirtd is well behaved. Of course recalling offhand while reviewing whether the magic of properly handling what to do on EOF or closure is a challenge. Still based on the fact that @id are random w/ libxl means that while unlikely it is possible for reuse too and at least with PID's we have assurances that the lock manager is handling the "removal" of locks when the EOF is seen, so going with PID's is fine. Maybe a blurb that indicates what lock manager magic allows us to consider why usage of PID's even with the knowledge they can turn over makes it more desirable to use. John > > And since for OBJECT_TYPE_DAEMON there is no 'id' (becuase it doesn't > make sense to have one) we have to meet at common ground => PID. Every > process has one (except kernel jobs, but we do not care about those). > We might as well check for owner name. > > Michal > > 1 - this is correct thing to do. We want virlockd to release all the > resources owned by PID when it sees EOF because it acts like regular > file locks. When a process is dying all file locks it has are also > released. More on that in later patches. > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 2/3] Split off MinGW builds
On Tue, Sep 04, 2018 at 01:01:34PM +0200, Andrea Bolognani wrote: > Up until now, we've been considering MinGW builds as > part of the respective project, at least when it > comes to grouping them. This, however, does not quite > work for a number of reasons: > > * MinGW builds have their own workspace, separate > from the native one. It goes further than that: > even the 32-bit and 64-bit builds use a workspace > each, which goes to show that grouping all three > together is inaccurate; Well, I'd say that if someone cares enough about libvirt mingw builds, then they probably care about both ABI versions in which case they'd most probably prefer not having to specify for which x86 arch build they're interested in, therefore having both of these in the same project playbook, e.g. libvirt+mingw. Other than that, I'm completely in favour of splitting mingw into separate playbooks. Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] qemu: Unify generation of command line for virtio devices
On Fri, Aug 31, 2018 at 04:03:10PM +0200, Andrea Bolognani wrote:
A virtio device such as
will be translated to one of four different QEMU devices
based on the address type. This behavior is the same for
all virtio devices, but unfortunately we have separate
ad-hoc code dealing with each and every one of them: not
only this is pointless duplication, but it turns out
that most of that code is not robust against new address
types being introduced and some of it is outright buggy.
Introduce a new function, qemuBuildVirtioDevStr(), which
deals with the issue in a generic fashion, and rewrite
all existing code to use it.
This fixes a bunch of issues such as virtio-serial-pci
being used with virtio-mmio addresses and virtio-gpu
not being usable at all with virtio-mmio addresses.
It also introduces a couple of minor regressions,
namely no longer erroring out when attempting to
use virtio-balloon and virtio-input devices with
virtio-s390 addresses; that said, virtio-s390 has
been superseded by virtio-ccw such a long time ago
that recent QEMU releases have dropped support for
the former entirely, so re-implementing such
device-specific validation is not worth it.
Signed-off-by: Andrea Bolognani
---
src/qemu/qemu_command.c | 299 ++--
1 file changed, 163 insertions(+), 136 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 8aa20496bc..04554d958b 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1812,6 +1812,62 @@ qemuBuildDriveDevCacheStr(virDomainDiskDefPtr disk,
}
+static char*
+qemuBuildVirtioDevStr(const virDomainDeviceInfo *info,
+ const char *baseName)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+const char *implName = NULL;
+
+switch ((virDomainDeviceAddressType)info->type) {
+case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
+implName = "pci";
+break;
+
+case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO:
+implName = "device";
+break;
+
+case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW:
+implName = "ccw";
+break;
+
+case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390:
+implName = "s390";
+break;
+
+case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE:
+case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL:
+case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCID:
+case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB:
+case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO:
+case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA:
+case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM:
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unexpected address type for %s"), baseName);
'%s'
+goto error;
+
+case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE:
+case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST:
+default:
+virReportEnumRangeError(virDomainDeviceAddressType,
+info->type);
+goto error;
+}
+
+virBufferAsprintf(, "%s-%s", baseName, implName);
+
buf is used exactly once in this function, could have been just
virAsprintf.
Or, even better, since all the calls are followed by adding the string
to a buffer, just pass the buffer as the function argument.
+if (virBufferCheckError() < 0)
+goto error;
+
+return virBufferContentAndReset();
+
+ error:
+virBufferFreeAndReset();
+return NULL;
+}
+
+
char *
qemuBuildDiskDeviceStr(const virDomainDef *def,
virDomainDiskDefPtr disk,
@@ -1822,6 +1878,7 @@ qemuBuildDiskDeviceStr(const virDomainDef *def,
const char *bus = virDomainDiskQEMUBusTypeToString(disk->bus);
const char *contAlias;
char *backendAlias = NULL;
+char *devStr = NULL;
int controllerModel;
if (qemuCheckDiskConfig(disk, qemuCaps) < 0)
@@ -2002,21 +2059,19 @@ qemuBuildDiskDeviceStr(const virDomainDef *def,
break;
case VIR_DOMAIN_DISK_BUS_VIRTIO:
-if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
-virBufferAddLit(, "virtio-blk-ccw");
-if (disk->iothread)
-virBufferAsprintf(, ",iothread=iothread%u",
disk->iothread);
-} else if (disk->info.type ==
- VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) {
-virBufferAddLit(, "virtio-blk-s390");
-} else if (disk->info.type ==
- VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO) {
-virBufferAddLit(, "virtio-blk-device");
-} else {
-virBufferAddLit(, "virtio-blk-pci");
-if (disk->iothread)
-virBufferAsprintf(, ",iothread=iothread%u",
disk->iothread);
+
+if (!(devStr = qemuBuildVirtioDevStr(&(disk->info), "virtio-blk")))
>info is enough.
Or even simpler:
disk->info.type
+goto error;
+
+virBufferAddStr(, devStr);
+VIR_FREE(devStr);
+
+if (disk->iothread &&
+(disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW ||
+
[libvirt] [jenkins-ci PATCH] guests: Special-case fedora-gpg-keys updates on Rawhide
During each Rawhide development cycle there is a point
at which packages start being signed with new keys, which
causes updates to fail.
To work around the problem, make sure fedora-gpg-keys is
updated before attempting to update all other packages;
updating fedora-gpg-keys itself requires gpg signature
checking to be disabled.
Signed-off-by: Andrea Bolognani
---
I am actually not 100% sure we need to disable gpg
signature checking in order to update fedora-gpg-keys:
it would make sense for that one package to be signed
with the old key to make the update possible without
breaking trust at any point in time. Unfortunately I
updated my Rawhide guest without taking a snapshot
first, and I can't figure out a way to get it back to
a state suitable for checking whether the above makes
sense :( Perhaps someone with deeper understanding of
the Fedora release process will confirm or deny.
guests/lcitool | 24 +---
guests/playbooks/update/tasks/base.yml | 9 +
2 files changed, 26 insertions(+), 7 deletions(-)
diff --git a/guests/lcitool b/guests/lcitool
index 821cafc..ddeee6a 100755
--- a/guests/lcitool
+++ b/guests/lcitool
@@ -511,7 +511,8 @@ class Application:
facts = self._inventory.get_facts(host)
package_format = facts["package_format"]
os_name = facts["os_name"]
-os_full = os_name + str(facts["os_version"])
+os_version = str(facts["os_version"])
+os_full = os_name + os_version
if package_format not in ["deb", "rpm"]:
raise Error("Host {} doesn't support Dockerfiles".format(host))
@@ -560,12 +561,21 @@ class Application:
apt-get autoclean -y
"""))
elif package_format == "rpm":
-sys.stdout.write(textwrap.dedent("""
-RUN yum update -y && \\
-yum install -y ${PACKAGES} && \\
-yum autoremove -y && \\
-yum clean all -y
-"""))
+if os_name == "Fedora" and os_version == "Rawhide":
+sys.stdout.write(textwrap.dedent("""
+RUN yum update -y --nogpgcheck fedora-gpg-keys && \\
+yum update -y && \\
+yum install -y ${PACKAGES} && \\
+yum autoremove -y && \\
+yum clean all -y
+"""))
+else:
+sys.stdout.write(textwrap.dedent("""
+RUN yum update -y && \\
+yum install -y ${PACKAGES} && \\
+yum autoremove -y && \\
+yum clean all -y
+"""))
def run(self):
cmdline = self._parser.parse_args()
diff --git a/guests/playbooks/update/tasks/base.yml
b/guests/playbooks/update/tasks/base.yml
index 11f600f..cc16eb0 100644
--- a/guests/playbooks/update/tasks/base.yml
+++ b/guests/playbooks/update/tasks/base.yml
@@ -64,6 +64,15 @@
- not ( os_name == 'Fedora' and
os_version == 'Rawhide' )
+- name: Update installed packages
+ package:
+name: fedora-gpg-keys
+state: latest
+disable_gpg_check: yes
+ when:
+- os_name == 'Fedora'
+- os_version == 'Rawhide'
+
- name: Update installed packages
command: dnf update --refresh --exclude 'kernel*' -y
args:
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 13/28] lock_driver: Introduce VIR_LOCK_MANAGER_RESOURCE_TYPE_METADATA
On 09/03/2018 11:13 AM, Michal Privoznik wrote:
> On 08/30/2018 11:34 PM, John Ferlan wrote:
>>
>>
>> On 08/27/2018 04:08 AM, Michal Privoznik wrote:
>>> This is a new type of object that lock drivers can handle.
>>> Currently, it is supported by lockd driver only.
>>>
>>> Signed-off-by: Michal Privoznik
>>> ---
>>> src/locking/lock_driver.h | 2 ++
>>> src/locking/lock_driver_lockd.c | 43
>>> +++
>>> src/locking/lock_driver_sanlock.c | 3 ++-
>>> 3 files changed, 38 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/src/locking/lock_driver.h b/src/locking/lock_driver.h
>>> index a9d2041c30..9be0abcfba 100644
>>> --- a/src/locking/lock_driver.h
>>> +++ b/src/locking/lock_driver.h
>>> @@ -51,6 +51,8 @@ typedef enum {
>>> VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK = 0,
>>> /* A lease against an arbitrary resource */
>>> VIR_LOCK_MANAGER_RESOURCE_TYPE_LEASE = 1,
>>> +/* The resource to be locked is a metadata */
>>> +VIR_LOCK_MANAGER_RESOURCE_TYPE_METADATA = 2,
>>> } virLockManagerResourceType;
>>>
>>> typedef enum {
>>> diff --git a/src/locking/lock_driver_lockd.c
>>> b/src/locking/lock_driver_lockd.c
>>> index 98953500b7..d7cb183d7a 100644
>>> --- a/src/locking/lock_driver_lockd.c
>>> +++ b/src/locking/lock_driver_lockd.c
>>> @@ -557,6 +557,7 @@ static int
>>> virLockManagerLockDaemonAddResource(virLockManagerPtr lock,
>>> virLockManagerLockDaemonPrivatePtr priv = lock->privateData;
>>> char *newName = NULL;
>>> char *newLockspace = NULL;
>>> +int newFlags = 0;
>>> bool autoCreate = false;
>>> int ret = -1;
>>>
>>> @@ -569,7 +570,7 @@ static int
>>> virLockManagerLockDaemonAddResource(virLockManagerPtr lock,
>>> switch (priv->type) {
>>> case VIR_LOCK_MANAGER_OBJECT_TYPE_DOMAIN:
>>>
>>> -switch (type) {
>>> +switch ((virLockManagerResourceType) type) {
>>> case VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK:
>>> if (params || nparams) {
>>> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>>> @@ -670,6 +671,8 @@ static int
>>> virLockManagerLockDaemonAddResource(virLockManagerPtr lock,
>>> goto cleanup;
>>>
>>> } break;
>>> +
>>> +case VIR_LOCK_MANAGER_RESOURCE_TYPE_METADATA:
>>
>> I'm still conflicted with Unknown and Unsupported.
>>
>>> default:
>
> As I explain in one of my previous replies, users are not really
> expected to see this message. Is merely for us to avoid broken code
> pattern. Even if it so happens that broken code slips through review,
> what difference does it make for users to see "Unsupported lock manager
> object type" vs "Unknown lock manager object type"? They'll file a bug
> and we will notice immediately what is the problem when looking into the
> code (we will notice it because the error message logs type number).
>
Consider my your early warning bz then ;-). It doesn't really matter
that much... Maybe it's more of an "Unsupported" message regardless of
whether it's META or 'default'. At first I considered noting the Enum
range error, but it's not the same here.
I'll leave it as a design decision and move on.
John
>>> virReportError(VIR_ERR_INTERNAL_ERROR,
>>> _("Unknown lock manager object type %d for
>>> domain lock object"),
>>> @@ -679,6 +682,29 @@ static int
>>> virLockManagerLockDaemonAddResource(virLockManagerPtr lock,
>>> break;
>>>
>>> case VIR_LOCK_MANAGER_OBJECT_TYPE_DAEMON:
>>> +switch ((virLockManagerResourceType) type) {
>>> +case VIR_LOCK_MANAGER_RESOURCE_TYPE_METADATA:
>>> +if (params || nparams) {
>>> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>>> + _("Unexpected parameters for metadata
>>> resource"));
>>> +goto cleanup;
>>> +}
>>> +if (VIR_STRDUP(newLockspace, "") < 0 ||
>>> +VIR_STRDUP(newName, name) < 0)
>>> +goto cleanup;
>>> +newFlags |= VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_METADATA;
>>> +break;
>>> +
>>> +case VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK:
>>> +case VIR_LOCK_MANAGER_RESOURCE_TYPE_LEASE:
>>
>> Again Unknown and Unsupported...
>>
>>> +default:
>>> +virReportError(VIR_ERR_INTERNAL_ERROR,
>>> + _("Unknown lock manager object type %d for
>>> daemon lock object"),
>>> + type);
>>> +goto cleanup;
>>> +}
>>> +break;
>>> +
>>> default:
>>> virReportError(VIR_ERR_INTERNAL_ERROR,
>>> _("Unknown lock manager object type %d"),
>>> @@ -686,19 +712,18 @@ static int
>>> virLockManagerLockDaemonAddResource(virLockManagerPtr lock,
>>> goto cleanup;
>>> }
>>>
>>> +if (flags & VIR_LOCK_MANAGER_RESOURCE_SHARED)
>>> +newFlags |=
Re: [libvirt] [PATCH v3 11/28] lock_driver: Introduce new VIR_LOCK_MANAGER_OBJECT_TYPE_DAEMON
On 09/03/2018 11:13 AM, Michal Privoznik wrote:
> On 08/30/2018 10:40 PM, John Ferlan wrote:
>>
>>
>> On 08/27/2018 04:08 AM, Michal Privoznik wrote:
>>> We will want virtlockd to lock files on behalf of libvirtd and
>>> not qemu process, because it is libvirtd that needs an exclusive
>>> access not qemu. This requires new lock context.
>>>
>>> Signed-off-by: Michal Privoznik
>>> ---
>>> src/locking/lock_driver.h | 2 +
>>> src/locking/lock_driver_lockd.c | 110
>>> +++---
>>> src/locking/lock_driver_sanlock.c | 37 -
>>> 3 files changed, 117 insertions(+), 32 deletions(-)
>>>
>>
>> Caveat to my comments - I didn't read all the conversations in the
>> previous series... So if using unions was something agreed upon, then
>> mea culpa for being too lazy to go read ;-)
>>
>> [...]
>>
>>> diff --git a/src/locking/lock_driver_lockd.c
>>> b/src/locking/lock_driver_lockd.c
>>> index ca825e6026..8ca0cf5426 100644
>>> --- a/src/locking/lock_driver_lockd.c
>>> +++ b/src/locking/lock_driver_lockd.c
>>> @@ -56,10 +56,21 @@ struct _virLockManagerLockDaemonResource {
>>> };
>>>
>>> struct _virLockManagerLockDaemonPrivate {
>>> -unsigned char uuid[VIR_UUID_BUFLEN];
>>> -char *name;
>>> -int id;
>>> -pid_t pid;
>>> +virLockManagerObjectType type;
>>> +union {
>>> +struct {
>>> +unsigned char uuid[VIR_UUID_BUFLEN];
>>> +char *name;
>>> +int id;
>>> +pid_t pid;
>>> +} dom;
>>> +
>>> +struct {
>>> +unsigned char uuid[VIR_UUID_BUFLEN];
>>> +char *name;
>>> +pid_t pid;
>>> +} daemon;
>>> +} t;
>>
>> Something tells me it'd be better if @dom and @daemon were typedef'd types.
>
> I don't see much benefit to that but I can do that change.
>
Remember the caveat - I was going patch by patch. I see a union like
this I'm thinking something in the future would then be using a pointer
from or rather than typing the whole path.
>>
>> Still unless the lock can be shared why separate things? An @id == -1
>> could signify a lock using @uuid, @name, and @pid is not a @dom lock.
>> using @type is fine as well.
>>
>> I see nothing by the end of the series adding a new type and since the
>> members essentially overlap, it's really not clear why a union should be
>> used.
>
> The idea is that in virLockManagerLockNew() one specifies which type of
> lock they want (whether it's a domain or daemon type of lock) and any
> subsequent call to virLockManager*() will either succeed or fail if they
> want to work with wrong object. So this can be viewed as namespacing.
>
Ok - so I'm not opposed to the design decision of namespacing, only
indicating it really didn't seem necessary. Perhaps "common" members
between the structs can be common to the @priv, while "specific" members
would be in a union like this. Of course, doing that leaves nothing
specific for daemon, does it. Although come to think of it, wouldn't
eventually the clientRefs, client, program, and counter be more
typically associated with daemon instead of dom? IOW: Those are
metadata lock concepts and not disk/lease concepts.
Still it's a design decision and while I think it's perhaps a bit of
overkill, there is some value vis-a-vis namespace and usage of the
virLockManagerObjectType.
> Basically I want to avoid this code pattern:
>
> lock = virLockManagerNew(virLockManagerPluginGetDriver(plugin),
> VIR_LOCK_MANAGER_OBJECT_TYPE_DOMAIN,
> ARRAY_CARDINALITY(params),
> params,
> flags)));
>
> virLockManagerAddResource(lock,
> VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK,
> src->path,
> 0,
> NULL,
> 0);
>
> virLockManagerAddResource(lock,
> VIR_LOCK_MANAGER_RESOURCE_TYPE_METADATA,
> src->path,
> 0,
> NULL,
> 0);
>
> This is a buggy pattern, because OBJECT_TYPE_DOMAIN is associated with
> the domain (the owner of the lock is the domain, the ownerPID is set
> from domain, etc.). However, we want RESOURCE_TYPE_METADATA to be
> associated with the libvirtd and not the domain [1].
One would think that either this or the subsequent patch wouldn't allow
a "domain" type lock to use "metadata", so it would fail because
@lock->priv->type doesn't support that mixed usage.
>
> 1 - the reason for the metadata lock to be associated with the libvirtd
> and not the domain is very simple: it's libvirtd who is changing the
> metadata so it should grab the lock too.
> Also, if registered owner of a lock disappears (e.g. due to a crash)
> virlockd releases all the locks the owner had. And if you view metadata
> locking from this
[libvirt] [PATCH v5 1/2] vl.c deprecate incorrect CPUs topology
-smp [cpus],sockets/cores/threads[,maxcpus] should describe topology
so that total number of logical CPUs [sockets * cores * threads]
would be equal to [maxcpus], however historically we didn't have
such check in QEMU and it is possible to start VM with an invalid
topology.
Deprecate invalid options combination so we can make sure that
the topology VM started with is always correct in the future.
Users with an invalid sockets/cores/threads/maxcpus values should
fix their CLI to make sure that
[sockets * cores * threads] == [maxcpus]
Signed-off-by: Igor Mammedov
---
v5:
- extend deprecation doc, adding that maxcpus should be multiple of
present on CLI [sockets/cores/threads] options
(Eduardo Habkost )
v4:
- missed dot comment, fix it with s/./,/ (Andrew Jones )
v3:
- more spelling fixes (Andrew Jones )
- place deprecation check after (sockets * cores * threads > max_cpus) check
(Eduardo Habkost )
v2:
- spelling& fixes (Andrew Jones )
---
qemu-deprecated.texi | 19 +++
vl.c | 7 +++
2 files changed, 26 insertions(+)
diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
index 87212b6..827c3ce 100644
--- a/qemu-deprecated.texi
+++ b/qemu-deprecated.texi
@@ -159,6 +159,25 @@ The 'file' driver for drives is no longer appropriate for
character or host
devices and will only accept regular files (S_IFREG). The correct driver
for these file types is 'host_cdrom' or 'host_device' as appropriate.
+@subsection -smp X,[socket=a,core=b,thread=c],maxcpus=Y (since 3.1)
+
+CPU topology properties should describe whole machine topology including
+possible CPUs, but historically it was possible to start QEMU with
+an incorrect topology where
+ sockets * cores * threads >= X && X < maxcpus
+which could lead to an incorrect topology enumeration by the guest.
+Support for invalid topologies will be removed, the user must ensure
+topologies described with -smp include all possible cpus, i.e.
+ sockets * cores * threads == maxcpus
+Note: it's assumed that maxcpus value must be multiple of the topology
+options present on command line to avoid creating an invalid topology.
+If maxcpus isn't be multiple of present topology options then the condition
+(sockets * cores * threads == maxcpus) can't be satisfied and it will
+trigger deprecation warning which later will be converted to a error.
+If you get deprecation warning it's recommended to explicitly specify
+a correct topology to make warning go away and ensure that it will
+continue working in the future.
+
@section QEMU Machine Protocol (QMP) commands
@subsection block-dirty-bitmap-add "autoload" parameter (since 2.12.0)
diff --git a/vl.c b/vl.c
index 5ba06ad..7fd700e 100644
--- a/vl.c
+++ b/vl.c
@@ -1246,6 +1246,13 @@ static void smp_parse(QemuOpts *opts)
exit(1);
}
+if (sockets * cores * threads != max_cpus) {
+warn_report("Invalid CPU topology deprecated: "
+"sockets (%u) * cores (%u) * threads (%u) "
+"!= maxcpus (%u)",
+sockets, cores, threads, max_cpus);
+}
+
smp_cpus = cpus;
smp_cores = cores;
smp_threads = threads;
--
2.7.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v5 0/2] deprecate incorrect CPUs topology
Changelog since v4: * extend deprication doc, adding that maxcpus should be multiple of present on CLI [sockets/cores/threads] options (Eduardo Habkost ) series bundles together 2 related patches posted separately earlier: vl.c deprecate incorrect CPUs topology vl:c: make sure that sockets are calculated correctly in '-smp X' case Goal is to depricate invalid topologies so we could make sure that topology configuration is correct by forbidding invalid input once deprecation period ends. --- CC: libvir-list@redhat.com CC: drjo...@redhat.com CC: ehabk...@redhat.com Igor Mammedov (2): vl.c deprecate incorrect CPUs topology vl:c: make sure that sockets are calculated correctly in '-smp X' case qemu-deprecated.texi | 19 +++ vl.c | 12 +++- 2 files changed, 30 insertions(+), 1 deletion(-) -- 2.7.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [perl PATCH] Avoid using /default-pool for test storage pool
The path /default-pool is used by a predefined storage pool in the test driver, and libvirt now enforces pool source uniqueness. Signed-off-by: Daniel P. Berrangé --- Pushed as a build fix t/400-storage-pools.t | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/400-storage-pools.t b/t/400-storage-pools.t index 319a21e..c78507d 100644 --- a/t/400-storage-pools.t +++ b/t/400-storage-pools.t @@ -60,7 +60,7 @@ my $xml = " wibble 12341234-5678-5678-5678-123412341234 -/default-pool +/wibble "; -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v5 2/2] vl:c: make sure that sockets are calculated correctly in '-smp X' case
commit
(5cdc9b76e3 vl.c: Remove dead assignment)
removed sockets calculation when 'sockets' weren't provided on CLI
since there wasn't any users for it back then. Exiting checks
are neither reachable
} else if (sockets * cores * threads < cpus) {
or nor triggable
if (sockets * cores * threads > max_cpus)
so we weren't noticing wrong topology since then, since users
recalculate sockets adhoc on their own.
However with deprecation check it becomes noticable, for example
-smp 2
will start printing warning:
"warning: Invalid CPU topology deprecated: sockets (1) * cores (1) * threads
(1) != maxcpus (2)"
calculating sockets if they weren't specified.
Fix it by returning back sockets calculation if it's omited on CLI.
Signed-off-by: Igor Mammedov
---
vl.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/vl.c b/vl.c
index 7fd700e..333d638 100644
--- a/vl.c
+++ b/vl.c
@@ -1210,11 +1210,14 @@ static void smp_parse(QemuOpts *opts)
/* compute missing values, prefer sockets over cores over threads */
if (cpus == 0 || sockets == 0) {
-sockets = sockets > 0 ? sockets : 1;
cores = cores > 0 ? cores : 1;
threads = threads > 0 ? threads : 1;
if (cpus == 0) {
+sockets = sockets > 0 ? sockets : 1;
cpus = cores * threads * sockets;
+} else {
+max_cpus = qemu_opt_get_number(opts, "maxcpus", cpus);
+sockets = !sockets ? max_cpus / (cores * threads) : sockets;
}
} else if (cores == 0) {
threads = threads > 0 ? threads : 1;
--
2.7.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [go PATCH] Avoid clashing path with default storage pool
The test driver provides a default storage pool at
/default-pool. Libvirt now enforces source path uniqueness, so we must
pick a different path for our test pools.
Signed-off-by: Daniel P. Berrangé
---
Pushed as a build fix
connect_test.go | 6 +++---
storage_pool_test.go | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/connect_test.go b/connect_test.go
index 29c274d..da24448 100644
--- a/connect_test.go
+++ b/connect_test.go
@@ -789,7 +789,7 @@ func TestStoragePoolDefineXML(t *testing.T) {
}()
defName := "default-pool-test-0"
xml := `default-pool-test-0
-/default-pool`
+/default-pool-test-0`
pool, err := conn.StoragePoolDefineXML(xml, 0)
if err != nil {
t.Fatal(err)
@@ -900,7 +900,7 @@ func TestLookupStorageVolByKey(t *testing.T) {
return
}
defer pool.Destroy()
- defPoolPath := "default-pool"
+ defPoolPath := "default-pool-test-1"
defVolName := time.Now().String()
defVolKey := "/" + defPoolPath + "/" + defVolName
vol, err := pool.StorageVolCreateXML(testStorageVolXML(defVolName,
defPoolPath), 0)
@@ -942,7 +942,7 @@ func TestLookupStorageVolByPath(t *testing.T) {
return
}
defer pool.Destroy()
- defPoolPath := "default-pool"
+ defPoolPath := "default-pool-test-1"
defVolName := time.Now().String()
defVolPath := "/" + defPoolPath + "/" + defVolName
vol, err := pool.StorageVolCreateXML(testStorageVolXML(defVolName,
defPoolPath), 0)
diff --git a/storage_pool_test.go b/storage_pool_test.go
index 18cdb3a..7e93b17 100644
--- a/storage_pool_test.go
+++ b/storage_pool_test.go
@@ -42,7 +42,7 @@ func buildTestStoragePool(poolName string) (*StoragePool,
*Connect) {
pool, err := conn.StoragePoolDefineXML(`
`+name+`
- /default-pool
+ /default-pool-test-1
`, 0)
if err != nil {
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] tests: Fix use of virtio-serial for aarch64/virt
On Fri, Aug 31, 2018 at 04:03:09PM +0200, Andrea Bolognani wrote: virtio-serial is an alias for virtio-serial-pci, which should not have been used for a PCIe-less aarch64/virt guest but it ended up being used anyway because the I tried using the XML file with DO_TEST_CAPS_ARCH_LATEST and it resulted in a pcie-root-port being added. It seems all the capability files in our tests/qemucapabilities already support gpex-pcihost, so adding a test using contemporary capabilites might be worthwhile. virtio-mmio capability was missing and the algorithm is buggy. Fix the test case so that we can fix the algorithm next. Signed-off-by: Andrea Bolognani --- tests/qemuxml2argvdata/mach-virt-console-virtio.args | 2 +- tests/qemuxml2argvtest.c | 3 ++- tests/qemuxml2xmloutdata/mach-virt-console-virtio.xml | 4 +++- tests/qemuxml2xmltest.c | 6 -- 4 files changed, 10 insertions(+), 5 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH] guests: Hash test user password dynamically
Current versions of Ansible support the password_hash()
filter, so we can avoid hardcoding a pre-computed hash
and make what's happening a bit clearer.
Signed-off-by: Andrea Bolognani
---
guests/playbooks/update/tasks/users.yml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/guests/playbooks/update/tasks/users.yml
b/guests/playbooks/update/tasks/users.yml
index 0a930d6..abaeaa4 100644
--- a/guests/playbooks/update/tasks/users.yml
+++ b/guests/playbooks/update/tasks/users.yml
@@ -28,7 +28,7 @@
- name: '{{ flavor }}: Set password'
user:
name: '{{ flavor }}'
-password:
'$6$xSlfvkcsDgPmRAMX$mFh9qRmFFW9cyW1n5/jeHvq4OmJA8WzSD70Mfis3VHc3Z5imZeiQAg9VNL4sFEtmDy/siU3nJL.QeAapCgfL20'
+password: '{{ "test"|password_hash("sha512") }}'
when:
- flavor == 'test'
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: skip qemublocktest if building without YAJL
On Tue, Sep 04, 2018 at 02:10:56PM +0200, Andrea Bolognani wrote: > On Tue, 2018-09-04 at 13:05 +0100, Daniel P. Berrangé wrote: > > > > Pushed as a build fix for platforms without JSON parser installed > > > > > > Which platforms? All machines in our CI environments should have > > > yajl installed. > > > > It wasn't CI - it was my own FreeBSD machine without yajl > > I see someone hasn't been using lcitool :P Note the CI machines are just a representative set of build targets, they are not providing exhaustive coverage. If we have a --without-yajl option, then we should be able to sucessfully build without yajl, even if the OS is capable of supporting yajl. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: skip qemublocktest if building without YAJL
On Tue, Sep 04, 2018 at 02:00:21PM +0200, Andrea Bolognani wrote: On Tue, 2018-09-04 at 12:01 +0100, Daniel P. Berrangé wrote: If no JSON parser is available qemublocktest fails, so skip its execution. Signed-off-by: Daniel P. Berrangé --- Pushed as a build fix for platforms without JSON parser installed Which platforms? All machines in our CI environments should have yajl installed. Either way, IIRC during the switch to Jansson we also made the JSON parser mandatory when building the QEMU driver, a change which I believe we might have reverted when switching back to yajl: can we just have that back? Or are there some issues preventing us from doing so? Yes, as a part of the Jansson revert I also reverted the build defaults: commit 5a58b5ed6803e71e32e5d6f8c6e3b68874d085fb Revert "build: switch --with-qemu default from yes to check" commit f204cf51035f51b979dec18ee526e418139fa874 Revert "build: require Jansson if QEMU driver is enabled" Because I unwisely dropped the WITH_YAJL->WITH_JSON rename that was present in earlier series and they used 'WITH_JANSSON'. At runtime, we require JSON for all the supported QEMUs, so the only thing stopping us from re-adding the check is that we need to change the QEMU driver default from 'yes' to 'check': https://www.redhat.com/archives/libvir-list/2018-May/msg01060.html (which, in combination with a change of the required library left some developers confused when the QEMU driver quietly stopped building) Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: skip qemublocktest if building without YAJL
On Tue, 2018-09-04 at 13:05 +0100, Daniel P. Berrangé wrote: > > > Pushed as a build fix for platforms without JSON parser installed > > > > Which platforms? All machines in our CI environments should have > > yajl installed. > > It wasn't CI - it was my own FreeBSD machine without yajl I see someone hasn't been using lcitool :P > > Either way, IIRC during the switch to Jansson we also made the > > JSON parser mandatory when building the QEMU driver, a change > > which I believe we might have reverted when switching back to > > yajl: can we just have that back? Or are there some issues > > preventing us from doing so? > > All our other tests have WITH_YAJL checks in them, this one was the > exception. Possibly they could all be changed with WITH_QEMU > instead but that's more than I want todo as a build fix. Fair enough. I still think we want to re-introduce the behavior described above. -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: skip qemublocktest if building without YAJL
On Tue, Sep 04, 2018 at 02:00:21PM +0200, Andrea Bolognani wrote: > On Tue, 2018-09-04 at 12:01 +0100, Daniel P. Berrangé wrote: > > If no JSON parser is available qemublocktest fails, so skip its execution. > > > > Signed-off-by: Daniel P. Berrangé > > --- > > > > Pushed as a build fix for platforms without JSON parser installed > > Which platforms? All machines in our CI environments should have > yajl installed. It wasn't CI - it was my own FreeBSD machine without yajl > Either way, IIRC during the switch to Jansson we also made the > JSON parser mandatory when building the QEMU driver, a change > which I believe we might have reverted when switching back to > yajl: can we just have that back? Or are there some issues > preventing us from doing so? All our other tests have WITH_YAJL checks in them, this one was the exception. Possibly they could all be changed with WITH_QEMU instead but that's more than I want todo as a build fix. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: skip qemublocktest if building without YAJL
On Tue, 2018-09-04 at 12:01 +0100, Daniel P. Berrangé wrote: > If no JSON parser is available qemublocktest fails, so skip its execution. > > Signed-off-by: Daniel P. Berrangé > --- > > Pushed as a build fix for platforms without JSON parser installed Which platforms? All machines in our CI environments should have yajl installed. Either way, IIRC during the switch to Jansson we also made the JSON parser mandatory when building the QEMU driver, a change which I believe we might have reverted when switching back to yajl: can we just have that back? Or are there some issues preventing us from doing so? -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 21/28] security_manager: Introduce virSecurityManagerLockCloseConn
On 08/31/2018 08:42 PM, John Ferlan wrote:
>
>
> On 08/27/2018 04:08 AM, Michal Privoznik wrote:
>> This is basically just a wrapper over virLockManagerCloseConn()
>> so that no connection is left open when it shouldn't be.
>>
>> Signed-off-by: Michal Privoznik
>> ---
>> src/security/security_manager.c | 75
>> +
>> 1 file changed, 75 insertions(+)
>>
>> diff --git a/src/security/security_manager.c
>> b/src/security/security_manager.c
>> index caaff1f703..2238c75a5c 100644
>> --- a/src/security/security_manager.c
>> +++ b/src/security/security_manager.c
>> @@ -91,6 +91,56 @@ virSecurityManagerLockDispose(void *obj)
>> }
>>
>>
>> +static void
>> +virSecurityManagerLockCloseConnLocked(virSecurityManagerLockPtr lock,
>> + bool force)
>> +{
>> +int rc;
>> +
>> +if (!lock)
>> +return;
>
> Not possible - if this is true you may just want to abort or let the
> following code fail miserably. The definition of the API is that it's
> called when @lock is locked!
>
>> +
>> +while (!force &&
>> + lock->pathLocked) {
>
> No need to split && across 2 lines. Still waiting for @pathLocked =
> true ;-)
>
>
>> +if (virCondWait(>cond, >parent.lock) < 0) {
>> +VIR_WARN("Unable to wait on metadata condition");
>> +return;
>> +}
>> +}
>> +
>> +rc = virLockManagerCloseConn(lock->lock, 0);
>
> still haven't filled in lock->lock either
>
>> +if (rc < 0)
>> +return;
>> +if (rc > 0)
>> +lock->lock = NULL;
>> +
>> +if (force) {
>> +/* We've closed the connection. Wake up anybody who might be
>
> s/the/our/
>
>> + * waiting. */
>> +lock->pathLocked = false;
>> +virCondSignal(>cond);
>> +}
>> +}
>> +
>> +
>> +static void
>> +virSecurityManagerLockCloseConn(virSecurityManagerLockPtr lock)
>> +{
>> +if (!lock)
>> +return;
>
> So this is lock->lock from a few patches ago.
>
>> +
>> +virObjectLock(lock);
>> +
>> +if (!lock->lock)
>
> Still not seeing lock->lock, but maybe the next patch exposes that part.
>
>> +goto cleanup;
>> +
>> +virSecurityManagerLockCloseConnLocked(lock, false);
>> +
>> + cleanup:
>> +virObjectUnlock(lock);
>> +}
>> +
>> +
>
> I'm staring blankly at the rest of changes wondering, h... what am I
> missing.
>
> Beyond that if the mgr->drv->*(mgr) isn't called, then should we call
> the CloseConn. I'm not seeing why it's called in the first place!
Because of the connection sharing (KEEP_OPEN flags) both
AcquireResources and ReleaseResources will either use previously opened
connection OR open a new one AND leave it open. This is important - that
after the last ReleaseResource there is still a connection open to
virtlockd. ReleaseResouce can't close the connection because it can't
possibly know whether there is new acquire/release call pair waiting or
not. It's security driver who knows that. And thus, the last one should
close the connection.
View from another angle, when relabelling a device we may end up
relabelling one, two or even more paths. For instance:
virSecurityManagerDomainSetPathLabel() relabels just one path,
virSecurityManagerSetChardevLabel() or
virSecurityManagerSetHostdevLabel() can relabel one, or many paths,
and finally, virSecurityManagerSetAllLabel() will definitely relabel
plenty of paths.
Having said that, it's clear now that ReleaseResouce can't know if the
path its releasing is the last one in the queue and thus if the
connection can be closed. But the API knows that. So after security
manager API has called the callback it knows no other path needs
relabelling (i.e. acquire+release) and thus the connection can be closed.
Except if there is not another thread that is already talking to
virlockd and locking/unlocking paths for some other domain. Hence the
connection refcounting in one of the previous patches.
Yes, this is very complicated design. And if possible I'd like to
simplify it. But I'm not successful on that front, sorry.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH] lcitool: Don't encrypt password manually
On Tue, 2018-09-04 at 10:49 +0200, Martin Kletzander wrote:
s/manually/ourselves/ in the subject.
[...]
> def get_root_password_file(self):
> -root_pass_file = self._get_config_file("root-password")
> -root_hash_file = self._get_config_file(".root-password.hash")
> -
> -try:
> -with open(root_pass_file, "r") as infile:
> -root_pass = infile.readline().strip()
> -except Exception:
> -raise Error(
> -"Missing or invalid root password file ({})".format(
> -root_pass_file,
> -)
> -)
> -
> -# The hash will be different every time we run, but that doesn't
> -# matter - it will still validate the correct root password
> -root_hash = crypt.crypt(root_pass, Util.mksalt())
> -
> -try:
> -with open(root_hash_file, "w") as infile:
> -infile.write("{}\n".format(root_hash))
> -except Exception:
> -raise Error(
> -"Can't write hashed root password file ({})".format(
> -root_hash_file,
> -)
> -)
> -
> -return root_hash_file
> +return self._get_config_file("root-password")
This is a really nice improvement overall, but we can't quite get
rid of the entire function: we still need to try and open the file,
or at least stat() it, like we do in get_vault_password_file(), so
that we can error out early instead of having Ansible bail out on
us really late in the game.
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4] qemu: Introduce state_lock_timeout to qemu.conf
On Tue, Aug 28, 2018 at 04:40:16PM +0800, Yi Wang wrote:
When doing some job holding state lock for a long time,
we may come across error:
"Timed out during operation: cannot acquire state change lock"
Well, sometimes it's not a problem and users wanner continue
s/wanner/want to/
to wait, and this patch allow users decide how long time they
can wait the state lock.
Signed-off-by: Yi Wang
Reviewed-by: Xi Xu
---
changes in v4:
- fox syntax-check error
changes in v3:
- add user-friendly description and nb of state lock
- check validity of stateLockTimeout
changes in v2:
- change default value to 30 in qemu.conf
- set the default value in virQEMUDriverConfigNew()
v4
Signed-off-by: Yi Wang
---
src/qemu/libvirtd_qemu.aug | 1 +
src/qemu/qemu.conf | 10 ++
src/qemu/qemu_conf.c | 14 ++
src/qemu/qemu_conf.h | 2 ++
src/qemu/qemu_domain.c | 5 +
src/qemu/test_libvirtd_qemu.aug.in | 1 +
6 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index ddc4bbf..f7287ae 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -93,6 +93,7 @@ module Libvirtd_qemu =
| limits_entry "max_core"
| bool_entry "dump_guest_core"
| str_entry "stdio_handler"
+ | int_entry "state_lock_timeout"
here you add the option at the end of the 'process_entry' group
let device_entry = bool_entry "mac_filter"
| bool_entry "relaxed_acs_check"
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index cd57b3c..8920a1a 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -667,6 +667,16 @@
#
#max_queued = 0
+
+# When two or more threads want to work with the same domain they use a
+# job lock to mutually exclude each other. However, waiting for the lock
+# is limited up to state_lock_timeout seconds.
+# NB, strong recommendation to set the timeout longer than 30 seconds.
+#
+# Default is 30
+#
+#state_lock_timeout = 60
But here in qemu.conf, you add it between the rpc entries.
It seems we did not follow the structure with 'stdio_handler',
but adding it either right after 'dump_guest_core' or at the end of file
would be better than squeezing it between rpc entries.
+
###
# Keepalive protocol:
# This allows qemu driver to detect broken connections to remote
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index a4f545e..c761cae 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -129,6 +129,9 @@ void qemuDomainCmdlineDefFree(qemuDomainCmdlineDefPtr def)
#endif
+/* Give up waiting for mutex after 30 seconds */
+#define QEMU_JOB_WAIT_TIME (1000ull * 30)
+
Here the constant is multiplied by 1000 to convert from seconds to
milliseconds.
virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)
{
virQEMUDriverConfigPtr cfg;
@@ -346,6 +349,8 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool
privileged)
cfg->glusterDebugLevel = 4;
cfg->stdioLogD = true;
+cfg->stateLockTimeout = QEMU_JOB_WAIT_TIME;
+
if (!(cfg->namespaces = virBitmapNew(QEMU_DOMAIN_NS_LAST)))
goto error;
@@ -863,6 +868,9 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
if (virConfGetValueUInt(conf, "keepalive_count", >keepAliveCount) < 0)
goto cleanup;
+if (virConfGetValueInt(conf, "state_lock_timeout", >stateLockTimeout)
< 0)
+goto cleanup;
+
But the parsed value is passed directly here, so '60' in the config file
would result in 60 ms.
if (virConfGetValueInt(conf, "seccomp_sandbox", >seccompSandbox) < 0)
goto cleanup;
@@ -1055,6 +1063,12 @@ virQEMUDriverConfigValidate(virQEMUDriverConfigPtr cfg)
return -1;
}
+if (cfg->stateLockTimeout <= 0) {
+virReportError(VIR_ERR_CONF_SYNTAX, "%s",
+ _("state_lock_timeout should larger than zero"));
+return -1;
+}
+
return 0;
}
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index a8d84ef..97cf2e1 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -190,6 +190,8 @@ struct _virQEMUDriverConfig {
int keepAliveInterval;
unsigned int keepAliveCount;
+int stateLockTimeout;
+
int seccompSandbox;
char *migrateHost;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 886e3fb..5a2ca52 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -6652,9 +6652,6 @@ qemuDomainObjCanSetJob(qemuDomainObjPrivatePtr priv,
priv->job.agentActive == QEMU_AGENT_JOB_NONE));
}
-/* Give up waiting for mutex after 30 seconds */
-#define QEMU_JOB_WAIT_TIME (1000ull * 30)
-
/**
* qemuDomainObjBeginJobInternal:
* @driver: qemu driver
@@ -6714,7 +6711,7 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver,
}
priv->jobs_queued++;
-then = now +
[libvirt] [jenkins-ci PATCH 2/3] Split off MinGW builds
Up until now, we've been considering MinGW builds as
part of the respective project, at least when it
comes to grouping them. This, however, does not quite
work for a number of reasons:
* MinGW builds have their own workspace, separate
from the native one. It goes further than that:
even the 32-bit and 64-bit builds use a workspace
each, which goes to show that grouping all three
together is inaccurate;
* when using lcitool to perform builds, grouping
all variants together has the annoying side-effect
of potentially causing extra computation to
happen: even if you only care about the native
build, the MinGW builds will be automatically
started after that, and even if you only care
about the MinGW build you still have to sit
through the native before you can get to it;
* it causes asymmetry in lcitool usage, since you
have to use eg. '-p libvirt+mingw64' when updating
but '-p libvirt' when building.
Making MinGW builds their own pseudo-projects solves
all of the above.
Signed-off-by: Andrea Bolognani
---
.../build/projects/libosinfo+mingw32.yml | 12 ++
.../build/projects/libosinfo+mingw64.yml | 12 ++
guests/playbooks/build/projects/libosinfo.yml | 22 ---
.../build/projects/libvirt+mingw32.yml| 12 ++
.../build/projects/libvirt+mingw64.yml| 12 ++
.../build/projects/libvirt-glib+mingw32.yml | 12 ++
.../build/projects/libvirt-glib+mingw64.yml | 12 ++
.../playbooks/build/projects/libvirt-glib.yml | 22 ---
guests/playbooks/build/projects/libvirt.yml | 22 ---
.../projects/osinfo-db-tools+mingw32.yml | 12 ++
.../projects/osinfo-db-tools+mingw64.yml | 13 +++
.../build/projects/osinfo-db-tools.yml| 22 ---
.../build/projects/virt-viewer+mingw32.yml| 12 ++
.../build/projects/virt-viewer+mingw64.yml| 12 ++
.../playbooks/build/projects/virt-viewer.yml | 22 ---
projects/libosinfo+mingw32.yaml | 12 ++
projects/libosinfo+mingw64.yaml | 12 ++
projects/libosinfo.yaml | 12 --
projects/libvirt+mingw32.yaml | 12 ++
projects/libvirt+mingw64.yaml | 12 ++
projects/libvirt-glib+mingw32.yaml| 12 ++
projects/libvirt-glib+mingw64.yaml| 12 ++
projects/libvirt-glib.yaml| 12 --
projects/libvirt.yaml | 12 --
projects/osinfo-db-tools+mingw32.yaml | 12 ++
projects/osinfo-db-tools+mingw64.yaml | 12 ++
projects/osinfo-db-tools.yaml | 12 --
projects/virt-viewer+mingw32.yaml | 12 ++
projects/virt-viewer+mingw64.yaml | 12 ++
projects/virt-viewer.yaml | 12 --
30 files changed, 241 insertions(+), 170 deletions(-)
create mode 100644 guests/playbooks/build/projects/libosinfo+mingw32.yml
create mode 100644 guests/playbooks/build/projects/libosinfo+mingw64.yml
create mode 100644 guests/playbooks/build/projects/libvirt+mingw32.yml
create mode 100644 guests/playbooks/build/projects/libvirt+mingw64.yml
create mode 100644 guests/playbooks/build/projects/libvirt-glib+mingw32.yml
create mode 100644 guests/playbooks/build/projects/libvirt-glib+mingw64.yml
create mode 100644 guests/playbooks/build/projects/osinfo-db-tools+mingw32.yml
create mode 100644 guests/playbooks/build/projects/osinfo-db-tools+mingw64.yml
create mode 100644 guests/playbooks/build/projects/virt-viewer+mingw32.yml
create mode 100644 guests/playbooks/build/projects/virt-viewer+mingw64.yml
create mode 100644 projects/libosinfo+mingw32.yaml
create mode 100644 projects/libosinfo+mingw64.yaml
create mode 100644 projects/libvirt+mingw32.yaml
create mode 100644 projects/libvirt+mingw64.yaml
create mode 100644 projects/libvirt-glib+mingw32.yaml
create mode 100644 projects/libvirt-glib+mingw64.yaml
create mode 100644 projects/osinfo-db-tools+mingw32.yaml
create mode 100644 projects/osinfo-db-tools+mingw64.yaml
create mode 100644 projects/virt-viewer+mingw32.yaml
create mode 100644 projects/virt-viewer+mingw64.yaml
diff --git a/guests/playbooks/build/projects/libosinfo+mingw32.yml
b/guests/playbooks/build/projects/libosinfo+mingw32.yml
new file mode 100644
index 000..4979f5f
--- /dev/null
+++ b/guests/playbooks/build/projects/libosinfo+mingw32.yml
@@ -0,0 +1,12 @@
+---
+- set_fact:
+name: libosinfo+mingw32
+machines: '{{ mingw_machines }}'
+archive_format: gz
+git_url: '{{ git_urls["libosinfo"][git_remote] }}'
+
+- include: '{{ playbook_base }}/jobs/prepare.yml'
+- include: '{{ playbook_base }}/jobs/autotools-build-job.yml'
+ vars:
+local_env: '{{ mingw32_local_env }}'
+autogen_args: '{{
[libvirt] [jenkins-ci PATCH 3/3] Remove 'variant'
It's no longer used anywhere.
Signed-off-by: Andrea Bolognani
---
.../playbooks/build/jobs/autotools-build-job.yml | 4 ++--
.../playbooks/build/jobs/autotools-check-job.yml | 4 ++--
.../playbooks/build/jobs/autotools-rpm-job.yml | 4 ++--
.../build/jobs/autotools-syntax-check-job.yml| 4 ++--
guests/playbooks/build/jobs/defaults.yml | 1 -
.../playbooks/build/jobs/generic-build-job.yml | 4 ++--
.../playbooks/build/jobs/generic-check-job.yml | 4 ++--
guests/playbooks/build/jobs/generic-rpm-job.yml | 4 ++--
.../build/jobs/generic-syntax-check-job.yml | 4 ++--
guests/playbooks/build/jobs/go-build-job.yml | 4 ++--
guests/playbooks/build/jobs/go-check-job.yml | 4 ++--
.../build/jobs/perl-modulebuild-build-job.yml| 4 ++--
.../build/jobs/perl-modulebuild-check-job.yml| 4 ++--
.../build/jobs/perl-modulebuild-rpm-job.yml | 4 ++--
guests/playbooks/build/jobs/prepare.yml | 8
.../build/jobs/python-distutils-build-job.yml| 4 ++--
.../build/jobs/python-distutils-check-job.yml| 4 ++--
.../build/jobs/python-distutils-rpm-job.yml | 4 ++--
jobs/autotools.yaml | 16
jobs/defaults.yaml | 1 -
jobs/generic.yaml| 16
jobs/go.yaml | 8
jobs/perl-modulebuild.yaml | 12 ++--
jobs/python-distutils.yaml | 12 ++--
24 files changed, 68 insertions(+), 70 deletions(-)
diff --git a/guests/playbooks/build/jobs/autotools-build-job.yml
b/guests/playbooks/build/jobs/autotools-build-job.yml
index 70751bb..bf7a616 100644
--- a/guests/playbooks/build/jobs/autotools-build-job.yml
+++ b/guests/playbooks/build/jobs/autotools-build-job.yml
@@ -1,8 +1,8 @@
---
-- name: '{{ name }}-build{{ variant }}'
+- name: '{{ name }}-build'
shell: |
set -e
-cd {{ name }}{{ variant }}
+cd {{ name }}
{{ global_env }}
{{ local_env }}
diff --git a/guests/playbooks/build/jobs/autotools-check-job.yml
b/guests/playbooks/build/jobs/autotools-check-job.yml
index 24a3ad4..bcb6a85 100644
--- a/guests/playbooks/build/jobs/autotools-check-job.yml
+++ b/guests/playbooks/build/jobs/autotools-check-job.yml
@@ -1,8 +1,8 @@
---
-- name: '{{ name }}-check{{ variant }}'
+- name: '{{ name }}-check'
shell: |
set -e
-cd {{ name }}{{ variant }}
+cd {{ name }}
{{ global_env }}
{{ local_env }}
diff --git a/guests/playbooks/build/jobs/autotools-rpm-job.yml
b/guests/playbooks/build/jobs/autotools-rpm-job.yml
index 1778211..5927d68 100644
--- a/guests/playbooks/build/jobs/autotools-rpm-job.yml
+++ b/guests/playbooks/build/jobs/autotools-rpm-job.yml
@@ -1,8 +1,8 @@
---
-- name: '{{ name }}-rpm{{ variant }}'
+- name: '{{ name }}-rpm'
shell: |
set -e
-cd {{ name }}{{ variant }}
+cd {{ name }}
{{ global_env }}
{{ local_env }}
diff --git a/guests/playbooks/build/jobs/autotools-syntax-check-job.yml
b/guests/playbooks/build/jobs/autotools-syntax-check-job.yml
index e101c6d..4906fdf 100644
--- a/guests/playbooks/build/jobs/autotools-syntax-check-job.yml
+++ b/guests/playbooks/build/jobs/autotools-syntax-check-job.yml
@@ -1,8 +1,8 @@
---
-- name: '{{ name }}-syntax-check{{ variant }}'
+- name: '{{ name }}-syntax-check'
shell: |
set -e
-cd {{ name }}{{ variant }}
+cd {{ name }}
{{ global_env }}
{{ local_env }}
diff --git a/guests/playbooks/build/jobs/defaults.yml
b/guests/playbooks/build/jobs/defaults.yml
index 7ea7ab8..522dd83 100644
--- a/guests/playbooks/build/jobs/defaults.yml
+++ b/guests/playbooks/build/jobs/defaults.yml
@@ -1,5 +1,4 @@
---
-variant: ''
all_machines:
- libvirt-centos-7
- libvirt-debian-8
diff --git a/guests/playbooks/build/jobs/generic-build-job.yml
b/guests/playbooks/build/jobs/generic-build-job.yml
index b651952..5639f7c 100644
--- a/guests/playbooks/build/jobs/generic-build-job.yml
+++ b/guests/playbooks/build/jobs/generic-build-job.yml
@@ -1,8 +1,8 @@
---
-- name: '{{ name }}-build{{ variant }}'
+- name: '{{ name }}-build'
shell: |
set -e
-cd {{ name }}{{ variant }}
+cd {{ name }}
{{ global_env }}
{{ local_env }}
diff --git a/guests/playbooks/build/jobs/generic-check-job.yml
b/guests/playbooks/build/jobs/generic-check-job.yml
index c133890..15717ec 100644
--- a/guests/playbooks/build/jobs/generic-check-job.yml
+++ b/guests/playbooks/build/jobs/generic-check-job.yml
@@ -1,8 +1,8 @@
---
-- name: '{{ name }}-check{{ variant }}'
+- name: '{{ name }}-check'
shell: |
set -e
-cd {{ name }}{{ variant }}
+cd {{ name }}
{{ global_env }}
{{ local_env }}
diff --git a/guests/playbooks/build/jobs/generic-rpm-job.yml
b/guests/playbooks/build/jobs/generic-rpm-job.yml
index 879d9f4..502a91e 100644
--- a/guests/playbooks/build/jobs/generic-rpm-job.yml
+++
[libvirt] [jenkins-ci PATCH 1/3] guests: Split MinGW projects
The 32-bit and 64-bit MinGW builds require almost
completely different sets of packages, so it makes
sense to split them off into separate pseudo-projects
in order to have more control over what gets
installed on each host.
Signed-off-by: Andrea Bolognani
---
guests/host_vars/libvirt-fedora-rawhide/main.yml | 15 ++-
...{libosinfo+mingw.yml => libosinfo+mingw32.yml} | 4
...{libosinfo+mingw.yml => libosinfo+mingw64.yml} | 4
.../{libvirt+mingw.yml => libvirt+mingw32.yml}| 12
.../{libvirt+mingw.yml => libvirt+mingw64.yml}| 12
...rt-glib+mingw.yml => libvirt-glib+mingw32.yml} | 2 --
...rt-glib+mingw.yml => libvirt-glib+mingw64.yml} | 2 --
...ools+mingw.yml => osinfo-db-tools+mingw32.yml} | 4
...glib+mingw.yml => osinfo-db-tools+mingw64.yml} | 4 ++--
...t-viewer+mingw.yml => virt-viewer+mingw32.yml} | 11 ---
...t-viewer+mingw.yml => virt-viewer+mingw64.yml} | 11 ---
11 files changed, 12 insertions(+), 69 deletions(-)
copy guests/vars/projects/{libosinfo+mingw.yml => libosinfo+mingw32.yml} (56%)
rename guests/vars/projects/{libosinfo+mingw.yml => libosinfo+mingw64.yml}
(56%)
copy guests/vars/projects/{libvirt+mingw.yml => libvirt+mingw32.yml} (51%)
rename guests/vars/projects/{libvirt+mingw.yml => libvirt+mingw64.yml} (51%)
copy guests/vars/projects/{libvirt-glib+mingw.yml => libvirt-glib+mingw32.yml}
(57%)
copy guests/vars/projects/{libvirt-glib+mingw.yml => libvirt-glib+mingw64.yml}
(57%)
rename guests/vars/projects/{osinfo-db-tools+mingw.yml =>
osinfo-db-tools+mingw32.yml} (53%)
rename guests/vars/projects/{libvirt-glib+mingw.yml =>
osinfo-db-tools+mingw64.yml} (54%)
copy guests/vars/projects/{virt-viewer+mingw.yml => virt-viewer+mingw32.yml}
(52%)
rename guests/vars/projects/{virt-viewer+mingw.yml => virt-viewer+mingw64.yml}
(52%)
diff --git a/guests/host_vars/libvirt-fedora-rawhide/main.yml
b/guests/host_vars/libvirt-fedora-rawhide/main.yml
index 1bd3332..4318653 100644
--- a/guests/host_vars/libvirt-fedora-rawhide/main.yml
+++ b/guests/host_vars/libvirt-fedora-rawhide/main.yml
@@ -1,13 +1,16 @@
---
projects:
- libosinfo
- - libosinfo+mingw
+ - libosinfo+mingw32
+ - libosinfo+mingw64
- libvirt
- - libvirt+mingw
+ - libvirt+mingw32
+ - libvirt+mingw64
- libvirt-cim
- libvirt-dbus
- libvirt-glib
- - libvirt-glib+mingw
+ - libvirt-glib+mingw32
+ - libvirt-glib+mingw64
- libvirt-go
- libvirt-go-xml
- libvirt-perl
@@ -16,10 +19,12 @@ projects:
- libvirt-tck
- osinfo-db
- osinfo-db-tools
- - osinfo-db-tools+mingw
+ - osinfo-db-tools+mingw32
+ - osinfo-db-tools+mingw64
- virt-manager
- virt-viewer
- - virt-viewer+mingw
+ - virt-viewer+mingw32
+ - virt-viewer+mingw64
package_format: rpm
os_name: Fedora
diff --git a/guests/vars/projects/libosinfo+mingw.yml
b/guests/vars/projects/libosinfo+mingw32.yml
similarity index 56%
copy from guests/vars/projects/libosinfo+mingw.yml
copy to guests/vars/projects/libosinfo+mingw32.yml
index 0eb27af..9435d1f 100644
--- a/guests/vars/projects/libosinfo+mingw.yml
+++ b/guests/vars/projects/libosinfo+mingw32.yml
@@ -4,8 +4,4 @@ packages:
- mingw32-glib2
- mingw32-libxml2
- mingw32-libxslt
- - mingw64-curl
- - mingw64-glib2
- - mingw64-libxml2
- - mingw64-libxslt
- wget
diff --git a/guests/vars/projects/libosinfo+mingw.yml
b/guests/vars/projects/libosinfo+mingw64.yml
similarity index 56%
rename from guests/vars/projects/libosinfo+mingw.yml
rename to guests/vars/projects/libosinfo+mingw64.yml
index 0eb27af..b71664e 100644
--- a/guests/vars/projects/libosinfo+mingw.yml
+++ b/guests/vars/projects/libosinfo+mingw64.yml
@@ -1,9 +1,5 @@
---
packages:
- - mingw32-curl
- - mingw32-glib2
- - mingw32-libxml2
- - mingw32-libxslt
- mingw64-curl
- mingw64-glib2
- mingw64-libxml2
diff --git a/guests/vars/projects/libvirt+mingw.yml
b/guests/vars/projects/libvirt+mingw32.yml
similarity index 51%
copy from guests/vars/projects/libvirt+mingw.yml
copy to guests/vars/projects/libvirt+mingw32.yml
index 063e6a4..2fc6603 100644
--- a/guests/vars/projects/libvirt+mingw.yml
+++ b/guests/vars/projects/libvirt+mingw32.yml
@@ -12,15 +12,3 @@ packages:
- mingw32-pkg-config
- mingw32-portablexdr
- mingw32-readline
- - mingw64-curl
- - mingw64-dbus
- - mingw64-dlfcn
- - mingw64-gcc
- - mingw64-gettext
- - mingw64-gnutls
- - mingw64-libssh2
- - mingw64-libxml2
- - mingw64-openssl
- - mingw64-pkg-config
- - mingw64-portablexdr
- - mingw64-readline
diff --git a/guests/vars/projects/libvirt+mingw.yml
b/guests/vars/projects/libvirt+mingw64.yml
similarity index 51%
rename from guests/vars/projects/libvirt+mingw.yml
rename to guests/vars/projects/libvirt+mingw64.yml
index 063e6a4..615ecec 100644
--- a/guests/vars/projects/libvirt+mingw.yml
+++ b/guests/vars/projects/libvirt+mingw64.yml
@@ -1,17 +1,5 @@
---
packages:
- - mingw32-curl
- - mingw32-dbus
- - mingw32-dlfcn
- - mingw32-gcc
- -
[libvirt] [PATCH] tests: skip qemublocktest if building without YAJL
If no JSON parser is available qemublocktest fails, so skip its execution.
Signed-off-by: Daniel P. Berrangé
---
Pushed as a build fix for platforms without JSON parser installed
tests/qemublocktest.c | 36 +++-
1 file changed, 23 insertions(+), 13 deletions(-)
diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 0c335abc5b..8e8773e6a8 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -19,17 +19,19 @@
#include
#include "testutils.h"
-#include "testutilsqemu.h"
-#include "testutilsqemuschema.h"
-#include "virstoragefile.h"
-#include "virstring.h"
-#include "virlog.h"
-#include "qemu/qemu_block.h"
-#include "qemu/qemu_qapi.h"
-#include "qemu/qemu_command.h"
+#if WITH_YAJL
+# include "testutilsqemu.h"
+# include "testutilsqemuschema.h"
+# include "virstoragefile.h"
+# include "virstring.h"
+# include "virlog.h"
+# include "qemu/qemu_block.h"
+# include "qemu/qemu_qapi.h"
-#define VIR_FROM_THIS VIR_FROM_NONE
+# include "qemu/qemu_command.h"
+
+# define VIR_FROM_THIS VIR_FROM_NONE
VIR_LOG_INIT("tests.storagetest");
@@ -355,7 +357,7 @@ mymain(void)
virTestCounterReset("qemu storage source xml->json->xml ");
-#define TEST_JSON_FORMAT(tpe, xmlstr) \
+# define TEST_JSON_FORMAT(tpe, xmlstr) \
do { \
xmljsonxmldata.type = tpe; \
xmljsonxmldata.xml = xmlstr; \
@@ -364,7 +366,7 @@ mymain(void)
ret = -1; \
} while (0)
-#define TEST_JSON_FORMAT_NET(xmlstr)\
+# define TEST_JSON_FORMAT_NET(xmlstr) \
TEST_JSON_FORMAT(VIR_STORAGE_TYPE_NETWORK, xmlstr)
TEST_JSON_FORMAT(VIR_STORAGE_TYPE_FILE, "\n");
@@ -417,7 +419,7 @@ mymain(void)
" \n"
"\n");
-#define TEST_DISK_TO_JSON_FULL(nme, fl) \
+# define TEST_DISK_TO_JSON_FULL(nme, fl) \
do { \
diskxmljsondata.name = nme; \
diskxmljsondata.props = NULL; \
@@ -435,7 +437,7 @@ mymain(void)
testQemuDiskXMLToPropsClear(); \
} while (0)
-#define TEST_DISK_TO_JSON(nme) TEST_DISK_TO_JSON_FULL(nme, false)
+# define TEST_DISK_TO_JSON(nme) TEST_DISK_TO_JSON_FULL(nme, false)
if (!(diskxmljsondata.schema = testQEMUSchemaLoad())) {
ret = -1;
@@ -500,4 +502,12 @@ mymain(void)
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
}
+#else
+static int
+mymain(void)
+{
+return EXIT_AM_SKIP;
+}
+#endif
+
VIR_TEST_MAIN(mymain)
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH 0/3] Split off MinGW builds
See patch 2/3 for the rationale.
Andrea Bolognani (3):
guests: Split MinGW projects
Split off MinGW builds
Remove 'variant'
.../host_vars/libvirt-fedora-rawhide/main.yml | 15 -
.../build/jobs/autotools-build-job.yml| 4 ++--
.../build/jobs/autotools-check-job.yml| 4 ++--
.../build/jobs/autotools-rpm-job.yml | 4 ++--
.../build/jobs/autotools-syntax-check-job.yml | 4 ++--
guests/playbooks/build/jobs/defaults.yml | 1 -
.../build/jobs/generic-build-job.yml | 4 ++--
.../build/jobs/generic-check-job.yml | 4 ++--
.../playbooks/build/jobs/generic-rpm-job.yml | 4 ++--
.../build/jobs/generic-syntax-check-job.yml | 4 ++--
guests/playbooks/build/jobs/go-build-job.yml | 4 ++--
guests/playbooks/build/jobs/go-check-job.yml | 4 ++--
.../build/jobs/perl-modulebuild-build-job.yml | 4 ++--
.../build/jobs/perl-modulebuild-check-job.yml | 4 ++--
.../build/jobs/perl-modulebuild-rpm-job.yml | 4 ++--
guests/playbooks/build/jobs/prepare.yml | 8 +++
.../build/jobs/python-distutils-build-job.yml | 4 ++--
.../build/jobs/python-distutils-check-job.yml | 4 ++--
.../build/jobs/python-distutils-rpm-job.yml | 4 ++--
.../build/projects/libosinfo+mingw32.yml | 12 ++
.../build/projects/libosinfo+mingw64.yml | 12 ++
guests/playbooks/build/projects/libosinfo.yml | 22 ---
.../build/projects/libvirt+mingw32.yml| 12 ++
.../build/projects/libvirt+mingw64.yml| 12 ++
.../build/projects/libvirt-glib+mingw32.yml | 12 ++
.../build/projects/libvirt-glib+mingw64.yml | 12 ++
.../playbooks/build/projects/libvirt-glib.yml | 22 ---
guests/playbooks/build/projects/libvirt.yml | 22 ---
.../projects/osinfo-db-tools+mingw32.yml | 12 ++
.../projects/osinfo-db-tools+mingw64.yml | 13 +++
.../build/projects/osinfo-db-tools.yml| 22 ---
.../build/projects/virt-viewer+mingw32.yml| 12 ++
.../build/projects/virt-viewer+mingw64.yml| 12 ++
.../playbooks/build/projects/virt-viewer.yml | 22 ---
...osinfo+mingw.yml => libosinfo+mingw32.yml} | 4
...osinfo+mingw.yml => libosinfo+mingw64.yml} | 4
...{libvirt+mingw.yml => libvirt+mingw32.yml} | 12 --
...{libvirt+mingw.yml => libvirt+mingw64.yml} | 12 --
...lib+mingw.yml => libvirt-glib+mingw32.yml} | 2 --
...lib+mingw.yml => libvirt-glib+mingw64.yml} | 2 --
...+mingw.yml => osinfo-db-tools+mingw32.yml} | 4
...+mingw.yml => osinfo-db-tools+mingw64.yml} | 4 ++--
...ewer+mingw.yml => virt-viewer+mingw32.yml} | 11 --
...ewer+mingw.yml => virt-viewer+mingw64.yml} | 11 --
jobs/autotools.yaml | 16 +++---
jobs/defaults.yaml| 1 -
jobs/generic.yaml | 16 +++---
jobs/go.yaml | 8 +++
jobs/perl-modulebuild.yaml| 12 +-
jobs/python-distutils.yaml| 12 +-
projects/libosinfo+mingw32.yaml | 12 ++
projects/libosinfo+mingw64.yaml | 12 ++
projects/libosinfo.yaml | 12 --
projects/libvirt+mingw32.yaml | 12 ++
projects/libvirt+mingw64.yaml | 12 ++
projects/libvirt-glib+mingw32.yaml| 12 ++
projects/libvirt-glib+mingw64.yaml| 12 ++
projects/libvirt-glib.yaml| 12 --
projects/libvirt.yaml | 12 --
projects/osinfo-db-tools+mingw32.yaml | 12 ++
projects/osinfo-db-tools+mingw64.yaml | 12 ++
projects/osinfo-db-tools.yaml | 12 --
projects/virt-viewer+mingw32.yaml | 12 ++
projects/virt-viewer+mingw64.yaml | 12 ++
projects/virt-viewer.yaml | 12 --
65 files changed, 321 insertions(+), 309 deletions(-)
create mode 100644 guests/playbooks/build/projects/libosinfo+mingw32.yml
create mode 100644 guests/playbooks/build/projects/libosinfo+mingw64.yml
create mode 100644 guests/playbooks/build/projects/libvirt+mingw32.yml
create mode 100644 guests/playbooks/build/projects/libvirt+mingw64.yml
create mode 100644 guests/playbooks/build/projects/libvirt-glib+mingw32.yml
create mode 100644 guests/playbooks/build/projects/libvirt-glib+mingw64.yml
create mode 100644 guests/playbooks/build/projects/osinfo-db-tools+mingw32.yml
create mode 100644 guests/playbooks/build/projects/osinfo-db-tools+mingw64.yml
create mode 100644 guests/playbooks/build/projects/virt-viewer+mingw32.yml
create mode 100644 guests/playbooks/build/projects/virt-viewer+mingw64.yml
copy guests/vars/projects/{libosinfo+mingw.yml =>
[libvirt] [PATCH] tests: skip some unicode tests if expected output won't match
The expected output strings from the vshtabletest.c are created on a
modern Linux host where unicode printing support is very good. On older
Linux platforms, or non-Linux platforms, some unicode characters will
not be considered printable. While the vsh table alignment code will
stil do the right thing with escaping & aligning in this case, the
result will not match the test's expected output.
Since we know the code is working correctly, do a check with iswprint()
to validate the platform's quality and skip the test if it fails. This
fixes the test on FreeBSD platforms.
Signed-off-by: Daniel P. Berrangé
---
Pushed as a build fix
tests/vshtabletest.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/tests/vshtabletest.c b/tests/vshtabletest.c
index 9e9c045226..1138e34161 100644
--- a/tests/vshtabletest.c
+++ b/tests/vshtabletest.c
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#include "internal.h"
#include "testutils.h"
@@ -158,6 +159,15 @@ testUnicodeArabic(const void *opaque ATTRIBUTE_UNUSED)
" 1 ﻉﺪﻴﻟ ﺎﻠﺜﻘﻴﻟ ﻕﺎﻣ ﻊﻧ, ٣٠ ﻎﻴﻨﻳﺍ ﻮﺘﻧﺎﻤﺗ ﺎﻠﺛﺎﻠﺛ، ﺄﺳﺭ, ﺩﻮﻟ ﺩﻮﻟ.
ﺄﻣﺎﻣ ﺍ ﺎﻧ ﻲﻜﻧ \n"
" ﺺﻔﺣﺓ ﺖﻜﺘﻴﻛﺍً ﻊﻟ, ﺎﻠﺠﻧﻭﺩ ﻭﺎﻠﻌﺗﺍﺩ ﺵﺭ
\n";
vshTablePtr table;
+wchar_t wc;
+
+/* If this char is not classed as printable, the actual
+ * output won't match what this test expects. The code
+ * is still operating correctly, but we have different
+ * layout */
+mbrtowc(, "،", MB_CUR_MAX, NULL);
+if (!iswprint(wc))
+return EXIT_AM_SKIP;
table = vshTableNew("ﻡﺍ ﻢﻣﺍ ﻕﺎﺌﻣﺓ", "ﺓ ﺎﻠﺼﻋ", "ﺍﻸﺜﻧﺎﻧ", NULL);
if (!table)
@@ -192,6 +202,15 @@ testUnicodeZeroWidthChar(const void *opaque
ATTRIBUTE_UNUSED)
" 1\u200Bfedora28 run\u200Bning \n"
" 2rhel7.5running \n";
char *act = NULL;
+wchar_t wc;
+
+/* If this char is not classed as printable, the actual
+ * output won't match what this test expects. The code
+ * is still operating correctly, but we have different
+ * layout */
+mbrtowc(, "\u200B", MB_CUR_MAX, NULL);
+if (!iswprint(wc))
+return EXIT_AM_SKIP;
table = vshTableNew("I\u200Bd", "Name", "\u200BStatus", NULL);
if (!table)
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: clear seccomp capability if TSYNC is not supported by host
Hi, This series was run against 'syntax-check' test by patchew.org, which failed, please find the details below: Type: series Message-id: 20180830120941.22155-1-marcandre.lur...@redhat.com Subject: [libvirt] [PATCH] qemu: clear seccomp capability if TSYNC is not supported by host === TEST SCRIPT BEGIN === #!/bin/bash # Testing script will be invoked under the git checkout with # HEAD pointing to a commit that has the patches applied on top of "base" # branch time bash -c './autogen.sh && make syntax-check' === TEST SCRIPT END === Updating bcb55ab053bc79561b55d0394490f4b64e0f2d01 >From https://github.com/patchew-project/libvirt 2864b4cd1c..b899726faa master -> master * [new tag] patchew/20180904083955.5162-1-zyi...@linux.ibm.com -> patchew/20180904083955.5162-1-zyi...@linux.ibm.com Switched to a new branch 'test' 4105abd90a qemu: clear seccomp capability if TSYNC is not supported by host === OUTPUT BEGIN === Updating submodules... Submodule 'gnulib' (https://git.savannah.gnu.org/git/gnulib.git/) registered for path '.gnulib' Submodule 'keycodemapdb' (https://gitlab.com/keycodemap/keycodemapdb.git) registered for path 'src/keycodemapdb' Cloning into '/var/tmp/patchew-tester-tmp-dugjf31o/src/.gnulib'... Cloning into '/var/tmp/patchew-tester-tmp-dugjf31o/src/src/keycodemapdb'... Submodule path '.gnulib': checked out '68df637b5f1b5c10370f6981d2a43a5cf74368df' Submodule path 'src/keycodemapdb': checked out '16e5b0787687d8904dad2c026107409eb9bfcb95' Running bootstrap... ./bootstrap: Bootstrapping from checked-out libvirt sources... ./bootstrap: consider installing git-merge-changelog from gnulib ./bootstrap: getting gnulib files... running: libtoolize --install --copy libtoolize: putting auxiliary files in AC_CONFIG_AUX_DIR, 'build-aux'. libtoolize: copying file 'build-aux/config.guess' libtoolize: copying file 'build-aux/config.sub' libtoolize: copying file 'build-aux/install-sh' libtoolize: copying file 'build-aux/ltmain.sh' libtoolize: putting macros in AC_CONFIG_MACRO_DIRS, 'm4'. libtoolize: copying file 'm4/libtool.m4' libtoolize: copying file 'm4/ltoptions.m4' libtoolize: copying file 'm4/ltsugar.m4' libtoolize: copying file 'm4/ltversion.m4' libtoolize: copying file 'm4/lt~obsolete.m4' ./bootstrap: .gnulib/gnulib-tool--no-changelog --aux-dir=build-aux --doc-base=doc --lib=libgnu --m4-base=m4/ --source-base=gnulib/lib/ --tests-base=gnulib/tests --local-dir=gnulib/local--lgpl=2 --with-tests --makefile-name=gnulib.mk --avoid=pt_chown --avoid=lock-tests --libtool --import ... Module list with included dependencies (indented): absolute-header accept accept-tests alloca alloca-opt alloca-opt-tests allocator areadlink areadlink-tests arpa_inet arpa_inet-tests assure autobuild base64 base64-tests binary-io binary-io-tests bind bind-tests bitrotate bitrotate-tests btowc btowc-tests builtin-expect byteswap byteswap-tests c-ctype c-ctype-tests c-strcase c-strcase-tests c-strcasestr c-strcasestr-tests calloc-posix canonicalize-lgpl canonicalize-lgpl-tests careadlinkat chown chown-tests clock-time cloexec cloexec-tests close close-tests configmake connect connect-tests count-leading-zeros count-leading-zeros-tests count-one-bits count-one-bits-tests ctype ctype-tests dirname-lgpl dosname double-slash-root dup dup-tests dup2 dup2-tests environ environ-tests errno errno-tests error execinfo exitfail extensions extern-inline fatal-signal fclose fclose-tests fcntl fcntl-h fcntl-h-tests fcntl-tests fd-hook fdatasync fdatasync-tests fdopen fdopen-tests fflush fflush-tests ffs ffs-tests ffsl ffsl-tests fgetc-tests filename flexmember float float-tests fnmatch fnmatch-tests fpieee fpucw fpurge fpurge-tests fputc-tests fread-tests freading freading-tests fseek fseek-tests fseeko fseeko-tests fstat fstat-tests fsync fsync-tests ftell ftell-tests ftello ftello-tests ftruncate ftruncate-tests func func-tests fwrite-tests getaddrinfo getaddrinfo-tests getcwd-lgpl getcwd-lgpl-tests getdelim getdelim-tests getdtablesize getdtablesize-tests getgroups getgroups-tests gethostname gethostname-tests getline getline-tests getopt-posix getopt-posix-tests getpagesize getpass getpeername getpeername-tests getprogname getprogname-tests getsockname getsockname-tests getsockopt getsockopt-tests gettext-h gettimeofday gettimeofday-tests getugroups gitlog-to-changelog gnumakefile grantpt grantpt-tests hard-locale havelib host-cpu-c-abi hostent
Re: [libvirt] [jenkins-ci PATCH 8/8] guests: Update documentation
On Wed, Aug 29, 2018 at 05:09:05PM +0200, Andrea Bolognani wrote: > Provide instructions on how to build from non-default > git repositories and branches. > > Signed-off-by: Andrea Bolognani > --- With the necessary updates on '-g' being optional: Reviewed-by: Erik Skultety > guests/README.markdown | 8 +++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/guests/README.markdown b/guests/README.markdown > index 68a6af7..f5d1219 100644 > --- a/guests/README.markdown > +++ b/guests/README.markdown > @@ -49,11 +49,17 @@ library and, where supported, as a Windows library using > MinGW. > Once hosts have been prepared following the steps above, you can use > `lcitool` to perform builds as well: for example, running > > -lcitool -a build -h '*debian*' -p libvirt-python > +lcitool -a build -h '*debian*' -p libvirt-python -r upstream/master > > will fetch libvirt-python's `master` branch from the upstream repository > and build it on all Debian hosts. > > +You can add more git repositories by tweaking the `git_urls` dictionary > +defined in `playbooks/build/jobs/defaults.yml` and then build arbitrary > +branches out of those with > + > +lcitool -a build -h all -p libvirt -r github/cool-feature > + > > Host setup > -- > -- > 2.17.1 > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 6/8] Add "git_urls" dictionary to defaults
On Wed, Aug 29, 2018 at 05:09:03PM +0200, Andrea Bolognani wrote: > Out of the box, it contains the upstream repository for > all projects; additionally, the user will be able to > store information about their own repositories, making > it possible to test-build in-progress branches before > submitting the code upstream. > > Despite all the pieces being now in place for us to get > rid of the per-project "git_url" variable entirely, we > can't actually do that because Jenkins Job Builder, > unlike Ansible, doesn't support dictionary access with > other variables being used as keys. That's fairly okay, > though, because the Jenkins part is less dynamic than > the Ansible one anyway. > > Signed-off-by: Andrea Bolognani > --- With the change discussed in patch 1: Reviewed-by: Erik Skultety -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 7/8] guests: Use "git_remote" when building
On Wed, Aug 29, 2018 at 05:09:04PM +0200, Andrea Bolognani wrote: > This makes the build process use the value provided > by the user through lcitool instead of the default. > > With this change, building arbitrary branches from > arbitrary git repository through lcitool is fully > working. > > Signed-off-by: Andrea Bolognani > --- Reviewed-by: Erik Skultety -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH] lcitool: Don't encrypt password manually
Since version 1.9 ansible supports password_hash filter that can do that for us.
Signed-off-by: Martin Kletzander
---
guests/lcitool | 29 +
guests/playbooks/update/tasks/users.yml | 2 +-
2 files changed, 2 insertions(+), 29 deletions(-)
diff --git a/guests/lcitool b/guests/lcitool
index 2901a92c507b..ad1eee288620 100755
--- a/guests/lcitool
+++ b/guests/lcitool
@@ -151,34 +151,7 @@ class Config:
return vault_pass_file
def get_root_password_file(self):
-root_pass_file = self._get_config_file("root-password")
-root_hash_file = self._get_config_file(".root-password.hash")
-
-try:
-with open(root_pass_file, "r") as infile:
-root_pass = infile.readline().strip()
-except Exception:
-raise Error(
-"Missing or invalid root password file ({})".format(
-root_pass_file,
-)
-)
-
-# The hash will be different every time we run, but that doesn't
-# matter - it will still validate the correct root password
-root_hash = crypt.crypt(root_pass, Util.mksalt())
-
-try:
-with open(root_hash_file, "w") as infile:
-infile.write("{}\n".format(root_hash))
-except Exception:
-raise Error(
-"Can't write hashed root password file ({})".format(
-root_hash_file,
-)
-)
-
-return root_hash_file
+return self._get_config_file("root-password")
class Inventory:
diff --git a/guests/playbooks/update/tasks/users.yml
b/guests/playbooks/update/tasks/users.yml
index ec7f798a9c00..0a930d6c382c 100644
--- a/guests/playbooks/update/tasks/users.yml
+++ b/guests/playbooks/update/tasks/users.yml
@@ -2,7 +2,7 @@
- name: 'root: Set password'
user:
name: root
-password: '{{ lookup("file", root_password_file) }}'
+password: '{{ lookup("file", root_password_file)|password_hash("sha512")
}}'
shell: '{{ bash }}'
- name: 'root: Configure ssh access'
--
2.18.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 4/8] guests: Use "git_branch" when building
On Wed, Aug 29, 2018 at 05:09:01PM +0200, Andrea Bolognani wrote: > This makes the build process use the value provided by > the user through lcitool instead of the default. > > Signed-off-by: Andrea Bolognani > --- Reviewed-by: Erik Skultety -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 2/8] Don't use "branch" in paths and job names
On Wed, Aug 29, 2018 at 05:08:59PM +0200, Andrea Bolognani wrote: > We'll soon to make it possible to build arbitrary branches > from arbitrary git repositories with lcitool: sticking with > the current scheme for on-disk storage would mean that we > would create a separate git clone for each branch that was > ever built this way, which is clearly extremely wasteful > not to mention slow. To prevent that from happening we drop > the branch name from the directory name, which makes it > possible to have a single git clone per project and allows > to quickly and cheaply switching repositories and branches. > > After dropping the branch name from the directory name, > there is very little reason to keep it in the job name: > on the CentOS CI environment we only ever build master, > and with lcitool the users themselves provide the git > configuration including the branch name, so it doesn't > really provide any useful information in either case. > Let's drop it from there as well. > > Signed-off-by: Andrea Bolognani > --- Reviewed-by: Erik Skultety -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 1/8] lcitool: Add "-r REVISION" argument for build
On Tue, Sep 04, 2018 at 10:33:18AM +0200, Andrea Bolognani wrote:
> On Tue, 2018-09-04 at 09:55 +0200, Erik Skultety wrote:
> > Initially, I wanted to point out that "revision" is probably not the best
> > name
> [...]
>
> I was thinking yesterday that perhaps it would be better to
> use '-g GITREVISION' here instead, to leave '-r' available for
> future extensions, eg. when/if I get around to adding support
> for dependencies between projects that could be used to mean
> 'recursive', as in: build this one project and also all those
> that depend on it, the same way CentOS CI does it. Does that
> sound reasonable?
Yep, I've been playing with the same thought too, so '-g' sounds fine to me.
>
> [...]
> > > def _action_build(self, hosts, projects, revision):
> > > +if revision is None:
> > > +raise Error("Missing git revision")
> >
> > see above...as I said, I still don't see a reason for requiring ^this, if
> > there
> > truly is a compelling reason to keep it, then I'd suggest changing the
> > default
> > remote name to "origin" from "upstream", because it feels more natural and
> > intuitive to me. Even though you document this in the README and I
> > understand
> > the reasoning - you can name your origin whatever you want + you can clone
> > your
> > fork and then add an upstream remote, but I wouldn't expect that to be very
> > common practice.
>
> Let's just use "default", that one doesn't have any charged
> meaning in the git world and it's pretty accurate, too.
Fair enough.
>
> I'll also make the whole thing optional. I still have a slightly
> uncomfortable feeling about it, though I can't quite put it to
> words... Plus unlike with libvirt going one way or another won't
> quite lock us into that decision forever, so I'm more willing to
> just try stuff ;)
Reviewed-by: Erik Skultety
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v5 11/13] qemu: Add hotpluging support for PCI devices on S390 guests
This commit adds hotplug support for PCI devices on S390 guests.
There's no need to implement hot unplug for zPCI as QEMU implements
an unplug callback which will unplug both PCI and zPCI device in a
cascaded way.
Currently, the following PCI devices are supported:
virtio-blk-pci
virtio-net-pci
virtio-rng-pci
virtio-input-host-pci
virtio-keyboard-pci
virtio-mouse-pci
virtio-tablet-pci
vfio-pci
SCSIVhost device
Signed-off-by: Yi Min Zhao
Reviewed-by: Boris Fiuczynski
Reviewed-by: Stefan Zimmermann
Reviewed-by: Bjoern Walk
Reviewed-by: Ján Tomko
---
src/qemu/qemu_hotplug.c | 151
1 file changed, 141 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 4f290b5648..65d25d776b 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -154,6 +154,70 @@ qemuHotplugPrepareDiskAccess(virQEMUDriverPtr driver,
}
+static int
+qemuDomainAttachZPCIDevice(qemuMonitorPtr mon,
+ virDomainDeviceInfoPtr info)
+{
+int ret = -1;
+char *devstr_zpci = NULL;
+
+if (!(devstr_zpci = qemuBuildZPCIDevStr(info)))
+goto cleanup;
+
+if (qemuMonitorAddDevice(mon, devstr_zpci) < 0)
+goto cleanup;
+
+ret = 0;
+
+ cleanup:
+VIR_FREE(devstr_zpci);
+return ret;
+}
+
+
+static int
+qemuDomainDetachZPCIDevice(qemuMonitorPtr mon,
+ virDomainDeviceInfoPtr info)
+{
+char *zpciAlias = NULL;
+int ret = -1;
+
+if (virAsprintf(, "zpci%d", info->addr.pci.zpci.uid) < 0)
+goto cleanup;
+
+if (qemuMonitorDelDevice(mon, zpciAlias) < 0)
+goto cleanup;
+
+ret = 0;
+
+ cleanup:
+VIR_FREE(zpciAlias);
+return ret;
+}
+
+
+static int
+qemuDomainAttachExtensionDevice(qemuMonitorPtr mon,
+virDomainDeviceInfoPtr info)
+{
+if (!virZPCIDeviceAddressIsEmpty(>addr.pci.zpci))
+return qemuDomainAttachZPCIDevice(mon, info);
+
+return 0;
+}
+
+
+static int
+qemuDomainDetachExtensionDevice(qemuMonitorPtr mon,
+virDomainDeviceInfoPtr info)
+{
+if (!virZPCIDeviceAddressIsEmpty(>addr.pci.zpci))
+return qemuDomainDetachZPCIDevice(mon, info);
+
+return 0;
+}
+
+
static int
qemuHotplugWaitForTrayEject(virDomainObjPtr vm,
virDomainDiskDefPtr disk)
@@ -805,8 +869,13 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
if (qemuHotplugDiskSourceAttach(priv->mon, diskdata) < 0)
goto exit_monitor;
-if (qemuMonitorAddDevice(priv->mon, devstr) < 0)
+if (qemuDomainAttachExtensionDevice(priv->mon, >info) < 0)
+goto exit_monitor;
+
+if (qemuMonitorAddDevice(priv->mon, devstr) < 0) {
+ignore_value(qemuDomainDetachExtensionDevice(priv->mon, >info));
goto exit_monitor;
+}
if (qemuDomainObjExitMonitor(driver, vm) < 0) {
ret = -2;
@@ -913,7 +982,15 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr
driver,
goto cleanup;
qemuDomainObjEnterMonitor(driver, vm);
-ret = qemuMonitorAddDevice(priv->mon, devstr);
+
+if (qemuDomainAttachExtensionDevice(priv->mon, >info) < 0) {
+ignore_value(qemuDomainObjExitMonitor(driver, vm));
+goto cleanup;
+}
+
+if ((ret = qemuMonitorAddDevice(priv->mon, devstr)) < 0)
+ignore_value(qemuDomainDetachExtensionDevice(priv->mon,
>info));
+
if (qemuDomainObjExitMonitor(driver, vm) < 0) {
releaseaddr = false;
ret = -1;
@@ -1377,7 +1454,8 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
goto cleanup;
}
-if (qemuDomainIsS390CCW(vm->def) &&
+if (net->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI &&
+qemuDomainIsS390CCW(vm->def) &&
virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_CCW)) {
net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW;
if (!(ccwaddrs = virDomainCCWAddressSetCreateFromDomain(vm->def)))
@@ -1447,7 +1525,15 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
goto try_remove;
qemuDomainObjEnterMonitor(driver, vm);
+
+if (qemuDomainAttachExtensionDevice(priv->mon, >info) < 0) {
+ignore_value(qemuDomainObjExitMonitor(driver, vm));
+virDomainAuditNet(vm, NULL, net, "attach", false);
+goto try_remove;
+}
+
if (qemuMonitorAddDevice(priv->mon, nicstr) < 0) {
+ignore_value(qemuDomainDetachExtensionDevice(priv->mon, >info));
ignore_value(qemuDomainObjExitMonitor(driver, vm));
virDomainAuditNet(vm, NULL, net, "attach", false);
goto try_remove;
@@ -1665,8 +1751,15 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver,
goto error;
qemuDomainObjEnterMonitor(driver, vm);
-ret = qemuMonitorAddDeviceWithFd(priv->mon, devstr,
- configfd, configfd_name);
+
+if ((ret =
[libvirt] [PATCH v5 08/13] qemu: Add zPCI address definition check
We should ensure that the Qemu should support zPCI when zPCI address is
defined in XML. Otherwise the error should be reported. So this patch
introduces the validation of zPCI address definition for
qemuDomainDeviceDefValidate().
Signed-off-by: Yi Min Zhao
---
src/qemu/qemu_domain.c | 26 ++
1 file changed, 26 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index aebd58c49c..7d7cd3cfdc 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5728,6 +5728,27 @@ qemuDomainDeviceDefValidateGraphics(const
virDomainGraphicsDef *graphics,
}
+static int
+qemuDomainZPCIAddressDefValidate(virDomainDeviceDef *dev,
+ virQEMUCapsPtr qemuCaps)
+{
+virDomainDeviceInfoPtr info = virDomainDeviceGetInfo(dev);
+
+if (!info || (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI))
+return 0;
+
+if (!virZPCIDeviceAddressIsEmpty(>addr.pci.zpci) &&
+!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ZPCI)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ "%s",
+ _("This QEMU binary doesn't support zPCI."));
+return -1;
+}
+
+return 0;
+}
+
+
static int
qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev,
const virDomainDef *def,
@@ -5741,6 +5762,10 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef
*dev,
def->emulator)))
return -1;
+ret = qemuDomainZPCIAddressDefValidate((virDomainDeviceDef *)dev,
qemuCaps);
+if (ret < 0)
+goto out;
+
switch ((virDomainDeviceType)dev->type) {
case VIR_DOMAIN_DEVICE_NET:
ret = qemuDomainDeviceDefValidateNetwork(dev->data.net);
@@ -5813,6 +5838,7 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev,
break;
}
+ out:
virObjectUnref(qemuCaps);
return ret;
}
--
Yi Min
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v5 04/13] qemu: Enable PCI multi bus for S390 guests
QEMU on s390 supports PCI multibus since forever. Signed-off-by: Yi Min Zhao Reviewed-by: Boris Fiuczynski Reviewed-by: Stefan Zimmermann Reviewed-by: Bjoern Walk Reviewed-by: Ján Tomko Reviewed-by: Andrea Bolognani --- src/qemu/qemu_capabilities.c | 4 1 file changed, 4 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 62d56db9de..c307a8d571 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1793,6 +1793,10 @@ bool virQEMUCapsHasPCIMultiBus(virQEMUCapsPtr qemuCaps, return false; } +/* S390 supports PCI-multibus. */ +if (ARCH_IS_S390(def->os.arch)) +return true; + /* If ARM 'virt' supports PCI, it supports multibus. * No extra conditions here for simplicity. */ -- Yi Min -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v5 05/13] qemu: Auto add pci-root for s390/s390x guests
The pci-root depends on zpci capability. So autogenerate pci-root if zpci exists. Signed-off-by: Yi Min Zhao Reviewed-by: Boris Fiuczynski Reviewed-by: Stefan Zimmermann Reviewed-by: Bjoern Walk Reviewed-by: Ján Tomko Reviewed-by: Andrea Bolognani --- src/qemu/qemu_domain.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index f161cf6c84..aebd58c49c 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3289,6 +3289,7 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def, case VIR_ARCH_S390X: addDefaultUSB = false; addPanicDevice = true; +addPCIRoot = virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ZPCI); break; case VIR_ARCH_SPARC: -- Yi Min -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v5 03/13] conf: Introduce a new PCI address extension flag
This patch introduces a new attribute PCI address extension flag
to deal with the extension PCI attributes such as 'uid' and 'fid'
on the S390 platform.
Signed-off-by: Yi Min Zhao
Reviewed-by: Boris Fiuczynski
Reviewed-by: Ján Tomko
Reviewed-by: Andrea Bolognani
---
src/conf/device_conf.h | 1 +
src/conf/domain_addr.h | 5 ++
src/qemu/qemu_domain_address.c | 142 -
3 files changed, 146 insertions(+), 2 deletions(-)
diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
index ff7d6c9d5f..11baf0c1e3 100644
--- a/src/conf/device_conf.h
+++ b/src/conf/device_conf.h
@@ -164,6 +164,7 @@ struct _virDomainDeviceInfo {
* assignment, never saved and never reported.
*/
int pciConnectFlags; /* enum virDomainPCIConnectFlags */
+int pciAddressExtFlags; /* enum virDomainPCIAddressExtensionFlags */
char *loadparm;
/* PCI devices will only be automatically placed on a PCI bus
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index 5ad9d8ef3d..5219d2f208 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -29,6 +29,11 @@
# define VIR_PCI_ADDRESS_SLOT_LAST 31
# define VIR_PCI_ADDRESS_FUNCTION_LAST 7
+typedef enum {
+VIR_PCI_ADDRESS_EXTENSION_NONE = 0, /* no extension */
+VIR_PCI_ADDRESS_EXTENSION_ZPCI = 1 << 0, /* zpci support */
+} virDomainPCIAddressExtensionFlags;
+
typedef enum {
VIR_PCI_CONNECT_HOTPLUGGABLE = 1 << 0, /* is hotplug needed/supported */
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index dda14adeb3..6fbe4a9644 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -509,6 +509,64 @@ qemuDomainAssignVirtioMMIOAddresses(virDomainDefPtr def,
}
+static bool
+qemuDomainDeviceSupportZPCI(virDomainDeviceDefPtr device)
+{
+switch ((virDomainDeviceType) device->type) {
+case VIR_DOMAIN_DEVICE_CHR:
+return false;
+
+case VIR_DOMAIN_DEVICE_CONTROLLER:
+case VIR_DOMAIN_DEVICE_DISK:
+case VIR_DOMAIN_DEVICE_LEASE:
+case VIR_DOMAIN_DEVICE_FS:
+case VIR_DOMAIN_DEVICE_NET:
+case VIR_DOMAIN_DEVICE_INPUT:
+case VIR_DOMAIN_DEVICE_SOUND:
+case VIR_DOMAIN_DEVICE_VIDEO:
+case VIR_DOMAIN_DEVICE_HOSTDEV:
+case VIR_DOMAIN_DEVICE_WATCHDOG:
+case VIR_DOMAIN_DEVICE_GRAPHICS:
+case VIR_DOMAIN_DEVICE_HUB:
+case VIR_DOMAIN_DEVICE_REDIRDEV:
+case VIR_DOMAIN_DEVICE_SMARTCARD:
+case VIR_DOMAIN_DEVICE_MEMBALLOON:
+case VIR_DOMAIN_DEVICE_NVRAM:
+case VIR_DOMAIN_DEVICE_RNG:
+case VIR_DOMAIN_DEVICE_SHMEM:
+case VIR_DOMAIN_DEVICE_TPM:
+case VIR_DOMAIN_DEVICE_PANIC:
+case VIR_DOMAIN_DEVICE_MEMORY:
+case VIR_DOMAIN_DEVICE_IOMMU:
+case VIR_DOMAIN_DEVICE_VSOCK:
+break;
+
+case VIR_DOMAIN_DEVICE_NONE:
+case VIR_DOMAIN_DEVICE_LAST:
+default:
+virReportEnumRangeError(virDomainDeviceType, device->type);
+return false;
+}
+
+return true;
+}
+
+
+static virDomainPCIAddressExtensionFlags
+qemuDomainDeviceCalculatePCIAddressExtensionFlags(virQEMUCapsPtr qemuCaps,
+ virDomainDeviceDefPtr dev)
+{
+virDomainPCIAddressExtensionFlags extFlags = 0;
+
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ZPCI) &&
+qemuDomainDeviceSupportZPCI(dev)) {
+extFlags |= VIR_PCI_ADDRESS_EXTENSION_ZPCI;
+}
+
+return extFlags;
+}
+
+
/**
* qemuDomainDeviceCalculatePCIConnectFlags:
*
@@ -991,6 +1049,56 @@ qemuDomainFillAllPCIConnectFlags(virDomainDefPtr def,
}
+/**
+ * qemuDomainFillDevicePCIExtensionFlagsIter:
+ *
+ * @def: the entire DomainDef
+ * @dev: The device to be checked
+ * @info: virDomainDeviceInfo within the device
+ * @opaque: qemu capabilities
+ *
+ * Sets the pciAddressExtFlags for a single device's info. Has properly
+ * formatted arguments to be called by virDomainDeviceInfoIterate().
+ *
+ * Always returns 0 - there is no failure.
+ */
+static int
+qemuDomainFillDevicePCIExtensionFlagsIter(virDomainDefPtr def ATTRIBUTE_UNUSED,
+ virDomainDeviceDefPtr dev,
+ virDomainDeviceInfoPtr info,
+ void *opaque)
+{
+virQEMUCapsPtr qemuCaps = opaque;
+
+info->pciAddressExtFlags
+= qemuDomainDeviceCalculatePCIAddressExtensionFlags(qemuCaps, dev);
+
+return 0;
+}
+
+
+/**
+ * qemuDomainFillAllPCIExtensionFlags:
+ *
+ * @def: the entire DomainDef
+ * @qemuCaps: as you'd expect
+ *
+ * Set the info->pciAddressExtFlags for all devices in the domain.
+ *
+ * Returns 0 on success or -1 on failure (the only possibility of
+ * failure would be some internal problem with
+ * virDomainDeviceInfoIterate())
+ */
+static int
+qemuDomainFillAllPCIExtensionFlags(virDomainDefPtr def,
+ virQEMUCapsPtr qemuCaps)
+{
+return virDomainDeviceInfoIterate(def,
+
[libvirt] [PATCH v5 02/13] qemu: Introduce zPCI capability
Let's introduce zPCI capability.
Signed-off-by: Yi Min Zhao
Reviewed-by: Boris Fiuczynski
Reviewed-by: Stefan Zimmermann
Reviewed-by: Bjoern Walk
Reviewed-by: Ján Tomko
Reviewed-by: Andrea Bolognani
---
src/qemu/qemu_capabilities.c | 2 ++
src/qemu/qemu_capabilities.h | 1 +
tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 +
8 files changed, 9 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index a075677421..62d56db9de 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -508,6 +508,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
/* 315 */
"vfio-pci.display",
"blockdev",
+ "zpci",
);
@@ -1148,6 +1149,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
{ "vhost-vsock-device", QEMU_CAPS_DEVICE_VHOST_VSOCK },
{ "mch", QEMU_CAPS_DEVICE_MCH },
{ "sev-guest", QEMU_CAPS_SEV_GUEST },
+{ "zpci", QEMU_CAPS_DEVICE_ZPCI },
};
static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsVirtioBalloon[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 3d3a978759..83b21b3763 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -492,6 +492,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for
syntax-check */
/* 315 */
QEMU_CAPS_VFIO_PCI_DISPLAY, /* -device vfio-pci.display */
QEMU_CAPS_BLOCKDEV, /* -blockdev and blockdev-add are supported */
+QEMU_CAPS_DEVICE_ZPCI, /* -device zpci */
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml
index e9ccc63402..efb8c56354 100644
--- a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml
@@ -118,6 +118,7 @@
+
201
0
307493
diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
index ec8330211c..cd3b0188c2 100644
--- a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
@@ -125,6 +125,7 @@
+
2011000
0
346345
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
index b8e46a970a..2f2e7da663 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
@@ -133,6 +133,7 @@
+
2012000
0
375593
diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml
index e8939667b3..06d451daec 100644
--- a/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml
@@ -105,6 +105,7 @@
+
2007000
0
220386
diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
index d91182ee84..d2c71c8163 100644
--- a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
@@ -108,6 +108,7 @@
+
2007093
0
245800
diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml
index e336cb1950..57251525b2 100644
--- a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml
@@ -112,6 +112,7 @@
+
2009000
0
269219
--
Yi Min
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v5 00/13] PCI passthrough support on s390
Abstract The PCI representation in QEMU has recently been extended for S390 allowing configuration of zPCI attributes like uid (user-defined identifier) and fid (PCI function identifier). The details can be found here: https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg07262.html To support the new zPCI feature of the S390 platform, two new XML attributes, @uid and @fid, are introduced for device addresses of type 'pci', i.e.: uid and fid are optional attributes. If they are defined by the user, unique values within the guest domain must be used. If they are not specified and the architecture requires them, they are automatically generated with non-conflicting values. Current implementation is the most seamless one for the user as it unites the address specific data of a PCI device on one XML element. It could accommodate both specifying our special parameters (uid and fid) and re-using standard statements (domain, bus, slot and function) for PCI devices. User can still specify bus/slot/function for the virtualized PCI devices in the XML. Thus uid/fid act as an extension to the PCI address and are stored in a new structure 'virZPCIDeviceAddress' which is a member of common PCI Address structure. Additionally, two hashtables are used for assignment and reservation of uid/fid. In support of extending the PCI address, a new PCI address extension flag is introduced. This extension flag allows is not only dedicated for the S390 platform but also other architectures needing certain extensions to PCI address space. Code Base = commit in master: 2f6ff0da5b qemu: Don't overwrite stats in qemuDomainBlocksStatsGather Change Log == v4->v5: 1. Update the version number. 2. Fixup code style error. 3. Separate qemu code into single patch. 4. Rebase the patches to the new code of master branch. v3->v4: 1. Update docs. 2. Format code style. 3. Optimize zPCI support check. 4. Move the check of zPCI defined in xml but unsupported by Qemu to qemuDomainDeviceDefValidate(). 5. Change zpci address member of PCI address struct from pointer to instance. 6. Modify zpci address definition principle. Currently the user must either define both of uid and fid or not. v2->v3: 1. Revise code style. 2. Update test cases. 3. Introduce qemuDomainCollectPCIAddressExtension() to collect PCI extension addresses. 4. Introduce virDeviceInfoPCIAddressExtensionPresent() to check if zPCI address exists. 5. Optimize zPCI address check logic. 6. Optimize passed parameters of zPCI addr alloc/release/reserve functions. 7. Report enum range error in qemuDomainDeviceSupportZPCI(). 8. Update commit messages. v1->v2: 1. Separate test commit and merge testcases into corresponding commits that introduce the functionalities firstly. 2. Spare some checks for zpci device. 3. Add vsock and controller support. 4. Add uin32 type schema. 5. Rename zpciuid and zpcifid to zpci_uid and zpci_fid. 6. Always return multibus support on S390. Yi Min Zhao (13): conf: Add definitions for 'uid' and 'fid' PCI address attributes qemu: Introduce zPCI capability conf: Introduce a new PCI address extension flag qemu: Enable PCI multi bus for S390 guests qemu: Auto add pci-root for s390/s390x guests conf: Introduce address caching for PCI extensions conf: Introduce parser, formatter for uid and fid qemu: Add zPCI address definition check conf: Allocate/release 'uid' and 'fid' in PCI address qemu: Generate and use zPCI device in QEMU command line qemu: Add hotpluging support for PCI devices on S390 guests docs: Add 'uid' and 'fid' information news: Update news for PCI address extension attributes cfg.mk | 1 + docs/formatdomain.html.in | 9 +- docs/news.xml | 10 + docs/schemas/basictypes.rng| 23 ++ docs/schemas/domaincommon.rng | 1 + src/conf/device_conf.c | 76 + src/conf/device_conf.h | 4 + src/conf/domain_addr.c | 330 + src/conf/domain_addr.h | 29 ++ src/conf/domain_conf.c | 6 + src/libvirt_private.syms | 6 + src/qemu/qemu_capabilities.c | 6 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c| 95 ++ src/qemu/qemu_command.h| 2 + src/qemu/qemu_domain.c | 27 ++ src/qemu/qemu_domain_address.c | 205 - src/qemu/qemu_hotplug.c| 151 +- src/util/virpci.h | 11 + tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 1 +
[libvirt] [PATCH v5 12/13] docs: Add 'uid' and 'fid' information
Update 'Device address' section to describe the 'uid' and 'fid' attributes. Signed-off-by: Yi Min Zhao Reviewed-by: Boris Fiuczynski Reviewed-by: Stefan Zimmermann Reviewed-by: Ján Tomko Reviewed-by: Andrea Bolognani --- docs/formatdomain.html.in | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index eb619a1656..52dc3f688f 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3921,7 +3921,14 @@ (since 0.9.7, requires QEMU 0.13). multifunction defaults to 'off', but should be set to 'on' for function 0 of a slot that will -have multiple functions used. +have multiple functions used. (Since 4.8.0 +), PCI address extensions depending on the architecture +are supported. E.g. for S390 guests, PCI addresses have +additional attributes: uid (a hex value between +0x0001 and 0x, inclusive), and fid (a hex +value between 0x and 0x, inclusive), which are +used by PCI devices on S390 for User-defined Identifiers and +Function Identifiers. Since 1.3.5, some hypervisor drivers may accept an address type='pci'/ element with no other attributes as an explicit request to -- Yi Min -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v5 10/13] qemu: Generate and use zPCI device in QEMU command line
Add new functions to generate zPCI command string and append it to
QEMU command line. And the related tests are added.
Signed-off-by: Yi Min Zhao
Reviewed-by: Boris Fiuczynski
Reviewed-by: Stefan Zimmermann
Reviewed-by: Bjoern Walk
Reviewed-by: Ján Tomko
---
src/qemu/qemu_command.c| 95 ++
src/qemu/qemu_command.h| 2 +
tests/qemuxml2argvdata/disk-virtio-s390-zpci.args | 1 +
.../hostdev-vfio-zpci-autogenerate.args| 25 ++
.../hostdev-vfio-zpci-autogenerate.xml | 18
.../hostdev-vfio-zpci-boundaries.args | 29 +++
.../hostdev-vfio-zpci-boundaries.xml | 26 ++
.../hostdev-vfio-zpci-multidomain-many.args| 39 +
.../hostdev-vfio-zpci-multidomain-many.xml | 67 +++
tests/qemuxml2argvdata/hostdev-vfio-zpci.args | 2 +
tests/qemuxml2argvtest.c | 13 +++
.../hostdev-vfio-zpci-autogenerate.xml | 30 +++
.../hostdev-vfio-zpci-boundaries.xml | 42 ++
.../hostdev-vfio-zpci-multidomain-many.xml | 79 ++
tests/qemuxml2xmltest.c| 11 +++
15 files changed, 479 insertions(+)
create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-autogenerate.args
create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-autogenerate.xml
create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-boundaries.args
create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-boundaries.xml
create mode 100644
tests/qemuxml2argvdata/hostdev-vfio-zpci-multidomain-many.args
create mode 100644
tests/qemuxml2argvdata/hostdev-vfio-zpci-multidomain-many.xml
create mode 100644 tests/qemuxml2xmloutdata/hostdev-vfio-zpci-autogenerate.xml
create mode 100644 tests/qemuxml2xmloutdata/hostdev-vfio-zpci-boundaries.xml
create mode 100644
tests/qemuxml2xmloutdata/hostdev-vfio-zpci-multidomain-many.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 8aa20496bc..afb25b1976 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2155,6 +2155,50 @@ qemuBuildDiskDeviceStr(const virDomainDef *def,
return NULL;
}
+char *
+qemuBuildZPCIDevStr(virDomainDeviceInfoPtr dev)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+virBufferAddLit(, "zpci");
+virBufferAsprintf(, ",uid=%u", dev->addr.pci.zpci.uid);
+virBufferAsprintf(, ",fid=%u", dev->addr.pci.zpci.fid);
+virBufferAsprintf(, ",target=%s", dev->alias);
+virBufferAsprintf(, ",id=zpci%u", dev->addr.pci.zpci.uid);
+
+if (virBufferCheckError() < 0) {
+virBufferFreeAndReset();
+return NULL;
+}
+
+return virBufferContentAndReset();
+}
+
+static int
+qemuAppendZPCIDevStr(virCommandPtr cmd,
+ virDomainDeviceInfoPtr dev)
+{
+char *devstr = NULL;
+
+virCommandAddArg(cmd, "-device");
+if (!(devstr = qemuBuildZPCIDevStr(dev)))
+return -1;
+
+virCommandAddArg(cmd, devstr);
+
+VIR_FREE(devstr);
+return 0;
+}
+
+static int
+qemuBuildExtensionCommandLine(virCommandPtr cmd,
+ virDomainDeviceInfoPtr dev)
+{
+if (!virZPCIDeviceAddressIsEmpty(>addr.pci.zpci))
+return qemuAppendZPCIDevStr(cmd, dev);
+
+return 0;
+}
static int
qemuBuildFloppyCommandLineControllerOptions(virCommandPtr cmd,
@@ -2391,6 +2435,9 @@ qemuBuildDiskCommandLine(virCommandPtr cmd,
if (!qemuDiskBusNeedsDriveArg(disk->bus)) {
if (disk->bus != VIR_DOMAIN_DISK_BUS_FDC ||
virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV)) {
+if (qemuBuildExtensionCommandLine(cmd, >info) < 0)
+return -1;
+
virCommandAddArg(cmd, "-device");
if (!(optstr = qemuBuildDiskDeviceStr(def, disk, bootindex,
@@ -2591,6 +2638,9 @@ qemuBuildFSDevCommandLine(virCommandPtr cmd,
virCommandAddArg(cmd, optstr);
VIR_FREE(optstr);
+if (qemuBuildExtensionCommandLine(cmd, >info) < 0)
+return -1;
+
virCommandAddArg(cmd, "-device");
if (!(optstr = qemuBuildFSDevStr(def, fs, qemuCaps)))
return -1;
@@ -3076,6 +3126,10 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd,
goto cleanup;
if (devstr) {
+if (qemuBuildExtensionCommandLine(cmd, >info) < 0) {
+VIR_FREE(devstr);
+goto cleanup;
+}
virCommandAddArg(cmd, "-device");
virCommandAddArg(cmd, devstr);
VIR_FREE(devstr);
@@ -3880,6 +3934,9 @@ qemuBuildWatchdogCommandLine(virCommandPtr cmd,
if (!def->watchdog)
return 0;
+if (qemuBuildExtensionCommandLine(cmd, >watchdog->info) < 0)
+return -1;
+
virCommandAddArg(cmd, "-device");
optstr = qemuBuildWatchdogDevStr(def, watchdog, qemuCaps);
@@ -3964,6 +4021,9 @@
[libvirt] [PATCH v5 09/13] conf: Allocate/release 'uid' and 'fid' in PCI address
This patch adds new functions for reservation, assignment and release
to handle the uid/fid. If the uid/fid is defined in the domain XML,
they will be reserved directly in collecting phase. If any of them is
not defined, we will find out an available value for it from zPCI
address hashtable, and reserve it. For hotplug case, there might be or
not zPCI definition. So allocate and reserve uid/fid for undefined
case. Assign if needed and reserve uid/fid for defined case. If the user
define zPCI extension address but zPCI capability doesn't exist, an
error will be reported.
Signed-off-by: Yi Min Zhao
Reviewed-by: Boris Fiuczynski
Reviewed-by: Ján Tomko
---
src/conf/device_conf.c | 7 ++
src/conf/device_conf.h | 1 +
src/conf/domain_addr.c | 242 +
src/conf/domain_addr.h | 15 +++
src/libvirt_private.syms | 4 +
src/qemu/qemu_domain_address.c | 56 +-
6 files changed, 324 insertions(+), 1 deletion(-)
diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
index 5ae990afdb..692d4c31ea 100644
--- a/src/conf/device_conf.c
+++ b/src/conf/device_conf.c
@@ -278,6 +278,13 @@ virZPCIDeviceAddressIsEmpty(const virZPCIDeviceAddress
*addr)
return !(addr->uid || addr->fid);
}
+bool
+virDeviceInfoPCIAddressExtensionIsPresent(const virDomainDeviceInfo *info)
+{
+return info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI &&
+!virZPCIDeviceAddressIsEmpty(>addr.pci.zpci);
+}
+
int
virPCIDeviceAddressParseXML(xmlNodePtr node,
diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
index 7e98b4ace0..c5629dc26b 100644
--- a/src/conf/device_conf.h
+++ b/src/conf/device_conf.h
@@ -195,6 +195,7 @@ bool virDeviceInfoPCIAddressIsWanted(const
virDomainDeviceInfo *info);
bool virDeviceInfoPCIAddressIsPresent(const virDomainDeviceInfo *info);
bool virZPCIDeviceAddressIsEmpty(const virZPCIDeviceAddress *addr);
+bool virDeviceInfoPCIAddressExtensionIsPresent(const virDomainDeviceInfo
*info);
int virPCIDeviceAddressParseXML(xmlNodePtr node,
virPCIDeviceAddressPtr addr);
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 62932e4e62..1c3248f04b 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -33,6 +33,152 @@
VIR_LOG_INIT("conf.domain_addr");
+static int
+virDomainZPCIAddressReserveId(virHashTablePtr set,
+ unsigned int id,
+ const char *name)
+{
+if (virHashLookup(set, )) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("zPCI %s %u is already reserved"),
+ name, id);
+return -1;
+}
+
+if (virHashAddEntry(set, , (void*)1) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to reserve %s %u"),
+ name, id);
+return -1;
+}
+
+return 0;
+}
+
+
+static int
+virDomainZPCIAddressReserveUid(virHashTablePtr set,
+ virZPCIDeviceAddressPtr addr)
+{
+return virDomainZPCIAddressReserveId(set, addr->uid, "uid");
+}
+
+
+static int
+virDomainZPCIAddressReserveFid(virHashTablePtr set,
+ virZPCIDeviceAddressPtr addr)
+{
+return virDomainZPCIAddressReserveId(set, addr->fid, "fid");
+}
+
+
+static int
+virDomainZPCIAddressAssignId(virHashTablePtr set,
+ unsigned int *id,
+ unsigned int min,
+ unsigned int max,
+ const char *name)
+{
+while (virHashLookup(set, )) {
+if (min == max) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("There is no more free %s."),
+ name);
+return -1;
+}
+++min;
+}
+*id = min;
+
+return 0;
+}
+
+
+static int
+virDomainZPCIAddressAssignUid(virHashTablePtr set,
+ virZPCIDeviceAddressPtr addr)
+{
+return virDomainZPCIAddressAssignId(set, >uid, 1,
+VIR_DOMAIN_DEVICE_ZPCI_MAX_UID, "uid");
+}
+
+
+static int
+virDomainZPCIAddressAssignFid(virHashTablePtr set,
+ virZPCIDeviceAddressPtr addr)
+{
+return virDomainZPCIAddressAssignId(set, >fid, 0,
+VIR_DOMAIN_DEVICE_ZPCI_MAX_FID, "fid");
+}
+
+
+static void
+virDomainZPCIAddressReleaseUid(virHashTablePtr set,
+ virZPCIDeviceAddressPtr addr)
+{
+if (virHashRemoveEntry(set, >uid) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Release uid %u failed"), addr->uid);
+}
+
+addr->uid = 0;
+}
+
+
+static void
+virDomainZPCIAddressReleaseFid(virHashTablePtr set,
+ virZPCIDeviceAddressPtr addr)
+{
+if (virHashRemoveEntry(set, >fid) < 0) {
+
[libvirt] [PATCH v5 06/13] conf: Introduce address caching for PCI extensions
This patch provides a caching mechanism for the device address
extensions uid and fid on S390. For efficient sparse address allocation,
we introduce two hash tables for uid/fid which hold the address set
information per domain. Also in order to improve performance of
searching available value, we introduce our own callbacks for the two
hashtables. In this way, uid/fid is saved in hash key and hash value
could be any non-NULL pointer due to no operation on hash value. That is
also the reason why we don't introduce hash value free callback.
Signed-off-by: Yi Min Zhao
Reviewed-by: Boris Fiuczynski
Reviewed-by: Bjoern Walk
Reviewed-by: Ján Tomko
---
src/conf/domain_addr.c | 85 ++
src/conf/domain_addr.h | 9 +
src/libvirt_private.syms | 1 +
src/qemu/qemu_domain_address.c | 7
4 files changed, 102 insertions(+)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index b62fd53c66..363e5b21dc 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -27,11 +27,23 @@
#include "virlog.h"
#include "virstring.h"
#include "domain_addr.h"
+#include "virhashcode.h"
#define VIR_FROM_THIS VIR_FROM_DOMAIN
VIR_LOG_INIT("conf.domain_addr");
+static void
+virDomainPCIAddressSetExtensionFree(virDomainPCIAddressSetPtr addrs)
+{
+if (!addrs || !addrs->zpciIds)
+return;
+
+virHashFree(addrs->zpciIds->uids);
+virHashFree(addrs->zpciIds->fids);
+VIR_FREE(addrs->zpciIds);
+}
+
virDomainPCIConnectFlags
virDomainPCIControllerModelToConnectType(virDomainControllerModelPCI model)
{
@@ -741,6 +753,78 @@ virDomainPCIAddressReleaseAddr(virDomainPCIAddressSetPtr
addrs,
addrs->buses[addr->bus].slot[addr->slot].functions &= ~(1 <<
addr->function);
}
+
+static uint32_t
+virZPCIAddrCode(const void *name,
+uint32_t seed)
+{
+unsigned int value = *((unsigned int *)name);
+return virHashCodeGen(, sizeof(value), seed);
+}
+
+
+static bool
+virZPCIAddrEqual(const void *namea,
+ const void *nameb)
+{
+return *((unsigned int *)namea) == *((unsigned int *)nameb);
+}
+
+
+static void *
+virZPCIAddrCopy(const void *name)
+{
+unsigned int *copy;
+
+if (VIR_ALLOC(copy) < 0)
+return NULL;
+
+*copy = *((unsigned int *)name);
+return (void *)copy;
+}
+
+
+static void
+virZPCIAddrKeyFree(void *name)
+{
+VIR_FREE(name);
+}
+
+
+int
+virDomainPCIAddressSetExtensionAlloc(virDomainPCIAddressSetPtr addrs,
+ virDomainPCIAddressExtensionFlags
extFlags)
+{
+if (extFlags & VIR_PCI_ADDRESS_EXTENSION_ZPCI) {
+if (addrs->zpciIds)
+return 0;
+
+if (VIR_ALLOC(addrs->zpciIds) < 0)
+return -1;
+
+if (!(addrs->zpciIds->uids = virHashCreateFull(10, NULL,
+ virZPCIAddrCode,
+ virZPCIAddrEqual,
+ virZPCIAddrCopy,
+ virZPCIAddrKeyFree)))
+goto error;
+
+if (!(addrs->zpciIds->fids = virHashCreateFull(10, NULL,
+ virZPCIAddrCode,
+ virZPCIAddrEqual,
+ virZPCIAddrCopy,
+ virZPCIAddrKeyFree)))
+goto error;
+}
+
+return 0;
+
+ error:
+virDomainPCIAddressSetExtensionFree(addrs);
+return -1;
+}
+
+
virDomainPCIAddressSetPtr
virDomainPCIAddressSetAlloc(unsigned int nbuses)
{
@@ -767,6 +851,7 @@ virDomainPCIAddressSetFree(virDomainPCIAddressSetPtr addrs)
if (!addrs)
return;
+virDomainPCIAddressSetExtensionFree(addrs);
VIR_FREE(addrs->buses);
VIR_FREE(addrs);
}
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index 5219d2f208..d1ec869da4 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -116,6 +116,11 @@ typedef struct {
} virDomainPCIAddressBus;
typedef virDomainPCIAddressBus *virDomainPCIAddressBusPtr;
+typedef struct {
+virHashTablePtr uids, fids;
+} virDomainZPCIAddressIds;
+typedef virDomainZPCIAddressIds *virDomainZPCIAddressIdsPtr;
+
struct _virDomainPCIAddressSet {
virDomainPCIAddressBus *buses;
size_t nbuses;
@@ -125,6 +130,7 @@ struct _virDomainPCIAddressSet {
bool areMultipleRootsSupported;
/* If true, the guest can use the pcie-to-pci-bridge controller */
bool isPCIeToPCIBridgeSupported;
+virDomainZPCIAddressIdsPtr zpciIds;
};
typedef struct _virDomainPCIAddressSet virDomainPCIAddressSet;
typedef virDomainPCIAddressSet *virDomainPCIAddressSetPtr;
@@ -132,6 +138,9 @@ typedef virDomainPCIAddressSet *virDomainPCIAddressSetPtr;
char *virDomainPCIAddressAsString(virPCIDeviceAddressPtr addr)
[libvirt] [PATCH v5 13/13] news: Update news for PCI address extension attributes
Signed-off-by: Yi Min Zhao Reviewed-by: Boris Fiuczynski Reviewed-by: Stefan Zimmermann Reviewed-by: Ján Tomko --- docs/news.xml | 10 ++ 1 file changed, 10 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 9a17b2f612..fc54a3b6ff 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -35,6 +35,16 @@ + + + qemu: Added support for PCI device passthrough on S390 + + + PCI addresses can now include the new uid (user-defined identifier) + and fid (PCI function identifier) attributes, which make the + corresponding devices usable by S390 guests. + + -- Yi Min -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v5 01/13] conf: Add definitions for 'uid' and 'fid' PCI address attributes
Add zPCI definitions in preparation of extending the PCI address
with parameters uid (user-defined identifier) and fid (PCI function
identifier).
Signed-off-by: Yi Min Zhao
Reviewed-by: Boris Fiuczynski
Reviewed-by: Stefan Zimmermann
Reviewed-by: Bjoern Walk
Reviewed-by: Ján Tomko
---
cfg.mk| 1 +
src/util/virpci.h | 8
2 files changed, 9 insertions(+)
diff --git a/cfg.mk b/cfg.mk
index 609ae869c2..1116feb299 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -472,6 +472,7 @@ sc_prohibit_canonicalize_file_name:
# Insist on correct types for [pug]id.
sc_correct_id_types:
@prohibit='\<(int|long) *[pug]id\>' \
+ exclude='exempt from syntax-check' \
halt='use pid_t for pid, uid_t for uid, gid_t for gid' \
$(_sc_search_regexp)
diff --git a/src/util/virpci.h b/src/util/virpci.h
index 2ac87694df..b7bcfa6d9f 100644
--- a/src/util/virpci.h
+++ b/src/util/virpci.h
@@ -37,12 +37,20 @@ typedef virPCIDeviceAddress *virPCIDeviceAddressPtr;
typedef struct _virPCIDeviceList virPCIDeviceList;
typedef virPCIDeviceList *virPCIDeviceListPtr;
+typedef struct _virZPCIDeviceAddress virZPCIDeviceAddress;
+typedef virZPCIDeviceAddress *virZPCIDeviceAddressPtr;
+struct _virZPCIDeviceAddress {
+unsigned int uid; /* exempt from syntax-check */
+unsigned int fid;
+};
+
struct _virPCIDeviceAddress {
unsigned int domain;
unsigned int bus;
unsigned int slot;
unsigned int function;
int multi; /* virTristateSwitch */
+virZPCIDeviceAddress zpci;
};
typedef enum {
--
Yi Min
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v5 07/13] conf: Introduce parser, formatter for uid and fid
This patch introduces new XML parser/formatter functions. Uid is
16-bit and non-zero. Fid is 32-bit. They are added as two new
attributes of PCI address, and parsed/formatted along with PCI
address parser/formatter. The related test is also added.
Signed-off-by: Yi Min Zhao
Reviewed-by: Boris Fiuczynski
Reviewed-by: Stefan Zimmermann
Reviewed-by: Bjoern Walk
Reviewed-by: Ján Tomko
---
docs/schemas/basictypes.rng| 23
docs/schemas/domaincommon.rng | 1 +
src/conf/device_conf.c | 69 ++
src/conf/device_conf.h | 2 +
src/conf/domain_addr.c | 3 +
src/conf/domain_conf.c | 6 ++
src/libvirt_private.syms | 1 +
src/util/virpci.h | 3 +
tests/qemuxml2argvdata/disk-virtio-s390-zpci.args | 25
tests/qemuxml2argvdata/disk-virtio-s390-zpci.xml | 17 ++
tests/qemuxml2argvdata/hostdev-vfio-zpci.args | 23
tests/qemuxml2argvdata/hostdev-vfio-zpci.xml | 19 ++
tests/qemuxml2argvtest.c | 7 +++
tests/qemuxml2xmloutdata/disk-virtio-s390-zpci.xml | 29 +
tests/qemuxml2xmloutdata/hostdev-vfio-zpci.xml | 30 ++
tests/qemuxml2xmltest.c| 6 ++
16 files changed, 264 insertions(+)
create mode 100644 tests/qemuxml2argvdata/disk-virtio-s390-zpci.args
create mode 100644 tests/qemuxml2argvdata/disk-virtio-s390-zpci.xml
create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci.args
create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci.xml
create mode 100644 tests/qemuxml2xmloutdata/disk-virtio-s390-zpci.xml
create mode 100644 tests/qemuxml2xmloutdata/hostdev-vfio-zpci.xml
diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng
index 14a3670e5c..3defb56316 100644
--- a/docs/schemas/basictypes.rng
+++ b/docs/schemas/basictypes.rng
@@ -65,6 +65,17 @@
+
+
+
+(0x)?[0-9a-fA-F]{1,8}
+
+
+0
+4294967295
+
+
+
@@ -111,6 +122,18 @@
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 3796eb4b5e..2ccf777d20 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -5220,6 +5220,7 @@
pci
+
diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
index 7a8f84e036..5ae990afdb 100644
--- a/src/conf/device_conf.c
+++ b/src/conf/device_conf.c
@@ -32,6 +32,65 @@
#define VIR_FROM_THIS VIR_FROM_DEVICE
+static int
+virZPCIDeviceAddressIsValid(virZPCIDeviceAddressPtr zpci)
+{
+/* We don't need to check fid because fid covers
+ * all range of uint32 type.
+ */
+if (zpci->uid > VIR_DOMAIN_DEVICE_ZPCI_MAX_UID ||
+zpci->uid == 0) {
+virReportError(VIR_ERR_XML_ERROR,
+ _("Invalid PCI address uid='0x%x', "
+ "must be > 0x0 and <= 0x%x"),
+ zpci->uid,
+ VIR_DOMAIN_DEVICE_ZPCI_MAX_UID);
+return 0;
+}
+
+return 1;
+}
+
+static int
+virZPCIDeviceAddressParseXML(xmlNodePtr node,
+ virPCIDeviceAddressPtr addr)
+{
+char *uid, *fid;
+int ret = -1;
+virZPCIDeviceAddress def = { 0 };
+
+uid = virXMLPropString(node, "uid");
+fid = virXMLPropString(node, "fid");
+
+if (uid) {
+if (virStrToLong_uip(uid, NULL, 0, ) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Cannot parse 'uid' attribute"));
+goto cleanup;
+}
+}
+
+if (fid) {
+if (virStrToLong_uip(fid, NULL, 0, ) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Cannot parse 'fid' attribute"));
+goto cleanup;
+}
+}
+
+if (!virZPCIDeviceAddressIsEmpty() &&
+!virZPCIDeviceAddressIsValid())
+goto cleanup;
+
+addr->zpci = def;
+ret = 0;
+
+ cleanup:
+VIR_FREE(uid);
+VIR_FREE(fid);
+return ret;
+}
+
int
virDomainDeviceInfoCopy(virDomainDeviceInfoPtr dst,
virDomainDeviceInfoPtr src)
@@ -213,6 +272,13 @@ virDeviceInfoPCIAddressIsPresent(const virDomainDeviceInfo
*info)
}
+bool
+virZPCIDeviceAddressIsEmpty(const virZPCIDeviceAddress *addr)
+{
+return !(addr->uid || addr->fid);
+}
+
+
int
virPCIDeviceAddressParseXML(xmlNodePtr node,
virPCIDeviceAddressPtr addr)
@@ -267,6 +333,9 @@ virPCIDeviceAddressParseXML(xmlNodePtr node,
if (!virPCIDeviceAddressIsEmpty(addr) && !virPCIDeviceAddressIsValid(addr,
true))
goto cleanup;
+if
Re: [libvirt] [jenkins-ci PATCH 1/8] lcitool: Add "-r REVISION" argument for build
On Tue, 2018-09-04 at 09:55 +0200, Erik Skultety wrote:
> Initially, I wanted to point out that "revision" is probably not the best name
[...]
I was thinking yesterday that perhaps it would be better to
use '-g GITREVISION' here instead, to leave '-r' available for
future extensions, eg. when/if I get around to adding support
for dependencies between projects that could be used to mean
'recursive', as in: build this one project and also all those
that depend on it, the same way CentOS CI does it. Does that
sound reasonable?
[...]
> > def _action_build(self, hosts, projects, revision):
> > +if revision is None:
> > +raise Error("Missing git revision")
>
> see above...as I said, I still don't see a reason for requiring ^this, if
> there
> truly is a compelling reason to keep it, then I'd suggest changing the default
> remote name to "origin" from "upstream", because it feels more natural and
> intuitive to me. Even though you document this in the README and I understand
> the reasoning - you can name your origin whatever you want + you can clone
> your
> fork and then add an upstream remote, but I wouldn't expect that to be very
> common practice.
Let's just use "default", that one doesn't have any charged
meaning in the git world and it's pretty accurate, too.
I'll also make the whole thing optional. I still have a slightly
uncomfortable feeling about it, though I can't quite put it to
words... Plus unlike with libvirt going one way or another won't
quite lock us into that decision forever, so I'm more willing to
just try stuff ;)
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 1/8] lcitool: Add "-r REVISION" argument for build
On Fri, Aug 31, 2018 at 12:16:14PM +0200, Andrea Bolognani wrote:
> On Wed, 2018-08-29 at 17:08 +0200, Andrea Bolognani wrote:
> > This will allow users to build arbitrary branches from
> > arbitrary git repositories, but for the moment all it
> > does is parse the argument and pass it down to the
> > Ansible playbook.
> >
> > Signed-off-by: Andrea Bolognani
> > ---
> > guests/lcitool | 36 ++--
> > 1 file changed, 26 insertions(+), 10 deletions(-)
>
> As helpfully pointed out by Erik, this change has the unintended
> consequence of making '-r REVISION' mandatory every time a playbook
> is invoked, including when using '-a update'.
>
> The patch below fixes the issue; consider it squashed in.
>
> diff --git a/guests/lcitool b/guests/lcitool
> index c12143d..0729d55 100755
> --- a/guests/lcitool
> +++ b/guests/lcitool
> @@ -367,13 +367,15 @@ class Application:
> ansible_hosts = ",".join(self._inventory.expand_pattern(hosts))
> selected_projects = self._projects.expand_pattern(projects)
>
> -if revision is None:
> -raise Error("Missing git revision")
> -tokens = revision.split('/')
> -if len(tokens) < 2:
> -raise Error("Invalid git revision '{}'".format(revision))
> -git_remote = tokens[0]
> -git_branch = '/'.join(tokens[1:])
> +if revision is not None:
> +tokens = revision.split('/')
> +if len(tokens) < 2:
> +raise Error("Invalid git revision '{}'".format(revision))
> +git_remote = tokens[0]
> +git_branch = '/'.join(tokens[1:])
> +else:
> +git_remote = None
> +git_branch = None
Sorry for the delay. ^This should rather be:
git_remote = "upstream"
git_branch = "master"
... and the check below should go away, I still don't think we should mandate
specifying revisions at all times.
Initially, I wanted to point out that "revision" is probably not the best name
given the information in man gitrevisions(7) and I wanted to suggest "refname"
instead, but then I tried referencing both with as well as
and it worked as expected, sweet. I also tried referencing
with @{}, however ansible documents that kind of limitation.
>
> ansible_cfg_path = os.path.join(base, "ansible.cfg")
> playbook_base = os.path.join(base, "playbooks", playbook)
> @@ -494,6 +496,9 @@ class Application:
> self._execute_playbook("update", hosts, projects, revision)
>
> def _action_build(self, hosts, projects, revision):
> +if revision is None:
> +raise Error("Missing git revision")
see above...as I said, I still don't see a reason for requiring ^this, if there
truly is a compelling reason to keep it, then I'd suggest changing the default
remote name to "origin" from "upstream", because it feels more natural and
intuitive to me. Even though you document this in the README and I understand
the reasoning - you can name your origin whatever you want + you can clone your
fork and then add an upstream remote, but I wouldn't expect that to be very
common practice.
Other than that, the patch works.
Erik
> +
> self._execute_playbook("build", hosts, projects, revision)
>
> def _action_dockerfile(self, hosts, projects, _revision):
> --
> Andrea Bolognani / Red Hat / Virtualization
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] api,qemu: add block latency histogram
Hi, Peter. I have questions to several of your comments:
On 03.09.2018 14:59, Peter Krempa wrote:
> On Mon, Sep 03, 2018 at 13:58:31 +0300, Nikolay Shirokovskiy wrote:
>> This patch adds option to configure/read latency histogram of
>> block device IO operations. The corresponding functionality
>> in qemu still has x- prefix AFAIK. So this patch should
>> help qemu functionality to become non experimental.
>
> This can be used as a proof of concept for this but commiting this
> feature will be possible only when qemu removes the experimental prefix.
>
> Until then they are free to change the API and that would result in
> multiple implementations in libvirt or they can even drop it.
>
>> In short histogram is configured thru new API
>> virDomainSetBlockLatencyHistogram and
>> histogram itself is available as new fields of virConnectGetAllDomainStats
>> output.
>>
...
>> static int
>> +qemuDomainGetBlockLatency(virDomainStatsRecordPtr record,
>> + int *maxparams,
>> + size_t block_idx,
>> + const char *op,
>> + qemuBlockLatencyStatsPtr latency)
>> +{
>> +char param_name[VIR_TYPED_PARAM_FIELD_LENGTH];
>> +size_t i;
>> +int ret = -1;
>> +
>> +if (!latency->nbins)
>> +return 0;
>> +
>> +snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
>> + "block.%zu.latency.%s.bincount", block_idx, op);
>> +if (virTypedParamsAddUInt(>params, >nparams, maxparams,
>> + param_name, latency->nbins) < 0)
>> +goto cleanup;
>> +
>> +for (i = 0; i < latency->nbins; i++) {
>> +snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
>> + "block.%zu.latency.%s.bin.%zu", block_idx, op, i);
>> +if (virTypedParamsAddULLong(>params, >nparams,
>> maxparams,
>> +param_name, latency->bins[i]) < 0)
>> +goto cleanup;
>> +}
>> +
>> +for (i = 0; i < latency->nbins - 1; i++) {
>> +snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
>> + "block.%zu.latency.%s.boundary.%zu", block_idx, op, i);
>> +if (virTypedParamsAddULLong(>params, >nparams,
>> maxparams,
>> +param_name, latency->boundaries[i]) < 0)
>> +goto cleanup;
>
> The two loops can be merged, so that the bin and value are together.
These loops counts differ by 1. I can either add check inside loop to skip
last iteration for boundaries or add record for last bin outside of the loop.
What is preferable?
...
>> +
>> +static
>> +int qemuDomainSetBlockLatencyHistogram(virDomainPtr dom,
>> + const char *dev,
>> + unsigned int op,
>> + unsigned long long *boundaries,
>> + int nboundaries,
>> + unsigned int flags)
>
> The setting and getting impl should be separated.
>
This API is only for setting bonudaries. After setting boundaries
are available in output of virConnectGetAllDomainStats. Example output:
> virsh block-set-latency-histogram vm sda "1, 5"
> virsh domstats vm | grep latency
block.0.latency.rd.bincount=3
block.0.latency.rd.bin.0=0
block.0.latency.rd.bin.1=0
block.0.latency.rd.bin.2=0
block.0.latency.rd.boundary.0=1
block.0.latency.rd.boundary.1=5
block.0.latency.wr.bincount=3
block.0.latency.wr.bin.0=0
block.0.latency.wr.bin.1=0
block.0.latency.wr.bin.2=0
block.0.latency.wr.boundary.0=1
block.0.latency.wr.boundary.1=5
block.0.latency.fl.bincount=3
block.0.latency.fl.bin.0=0
block.0.latency.fl.bin.1=0
block.0.latency.fl.bin.2=0
block.0.latency.fl.boundary.0=1
block.0.latency.fl.boundary.1=5
>
...
>> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
>> index 48b142a..9295ecb 100644
>> --- a/src/qemu/qemu_monitor.h
>> +++ b/src/qemu/qemu_monitor.h
>> @@ -569,6 +569,14 @@ virHashTablePtr qemuMonitorGetBlockInfo(qemuMonitorPtr
>> mon);
>>
>> virJSONValuePtr qemuMonitorQueryBlockstats(qemuMonitorPtr mon);
>>
>> +typedef struct _qemuBlockLatencyStats qemuBlockLatencyStats;
>> +typedef qemuBlockLatencyStats *qemuBlockLatencyStatsPtr;
>> +struct _qemuBlockLatencyStats {
>> +unsigned long long *boundaries;
>> +unsigned long long *bins;
>
> Since they are connected, you should use an array of structs containing
> both, so that they can't be interpreted otherwise.
Unfortunately these arrays sizes differ by 1. nboundaries = nbins + 1.
That's why I don't introduce another structure here, it would be inconvinient.
>
>> +unsigned int nbins;
>> +};
>> +
...
>> +static int
>> +qemuMonitorJSONGetBlockLatencyStats(virJSONValuePtr stats,
>> +const char *name,
>> +
