Re: [libvirt] [PATCH] rpc: fixing compilation error due to deprecated ssh_get_publickey().

[Eric Blake]

On 05/04/2018 04:01 PM, Julio Faracco wrote:

IMHO:
- The first approach is simple to remove in the future.


No, both approaches are equally easy to trim down in the future (true, 
the second approach leaves a temporary variable that could possibly be 
deleted, but it's not a prerequisite to remove the temporary variable 
when trimming the ifdefs).



- The second one is easy to read and understand.


Furthermore, the second one does not have unbalanced { vs. }, which 
makes it better for some editors.



+#if LIBSSH_VERSION_INT > 0x0705 /* 0.7.5 */
+if (ssh_get_server_publickey(sess->session, ) != SSH_OK) {
+#else
  if (ssh_get_publickey(sess->session, ) != SSH_OK) {
+#endif
  virReportError(VIR_ERR_LIBSSH, "%s",
 _("failed to get the key of the current "
   "session"));


How about making it easier to read and harder to mess up:

 #if LIBSSH_VERSION_INT > 0x0705 /* 0.7.5 */
 rc = ssh_get_server_publickey(sess->session, );
 #else
 rc = ssh_get_publickey(sess->session, );
 #endif

 if (rc != SSH_OK) {
 ...
 }


Furthermore, top-posting on technical lists is harder to read.

If you want a third approach, there is:

#if LIBSSH_VERSION_INT <= 0x0705 /* 0.7.5 */
# define ssh_get_server_publickey ssh_get_publickey
#endif

if (ssh_get_server_publickey(sess->session, ) != SSH_OK) {
virReportError(...

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] rpc: fixing compilation error due to deprecated ssh_get_publickey().

[Julio Faracco]

IMHO:
- The first approach is simple to remove in the future.
- The second one is easy to read and understand.

--
Julio Cesar Faracco

2018-05-04 16:10 GMT-03:00 Jiri Denemark :
> On Tue, May 01, 2018 at 13:21:15 -0300, Julio Faracco wrote:
>> After 0.7.5 release, libssh deprecated ssh_get_publickey() method to
>> introduce ssh_get_server_publickey(). This commit check the current
>> version of libssh and use the proper method during the compilation.
>> See the error:
>>
>> make[3]: Entering directory '/home/julio/Desktop/virt/libvirt/src'
>>   CC   rpc/libvirt_net_rpc_la-virnetlibsshsession.lo
>> rpc/virnetlibsshsession.c:217:9: error: 'ssh_get_publickey' is deprecated 
>> [-Werror,-Wdeprecated-declarations]
>> if (ssh_get_publickey(sess->session, ) != SSH_OK) {
>> ^
>> /usr/include/libssh/libssh.h:489:1: note: 'ssh_get_publickey' has been 
>> explicitly marked deprecated here
>> SSH_DEPRECATED LIBSSH_API int ssh_get_publickey(ssh_session session, ssh_key 
>> *key);
>> ^
>> /usr/include/libssh/libssh.h:99:40: note: expanded from macro 
>> 'SSH_DEPRECATED'
>>^
>> 1 error generated.
>> Makefile:8604: recipe for target 
>> 'rpc/libvirt_net_rpc_la-virnetlibsshsession.lo' failed
>>
>> Signed-off-by: Julio Faracco 
>> ---
>>  src/rpc/virnetlibsshsession.c | 4 
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/src/rpc/virnetlibsshsession.c b/src/rpc/virnetlibsshsession.c
>> index 309e8a9340..96c5bc0882 100644
>> --- a/src/rpc/virnetlibsshsession.c
>> +++ b/src/rpc/virnetlibsshsession.c
>> @@ -214,7 +214,11 @@ virLibsshServerKeyAsString(virNetLibsshSessionPtr sess)
>>  size_t keyhashlen;
>>  char *str;
>>
>> +#if LIBSSH_VERSION_INT > 0x0705 /* 0.7.5 */
>> +if (ssh_get_server_publickey(sess->session, ) != SSH_OK) {
>> +#else
>>  if (ssh_get_publickey(sess->session, ) != SSH_OK) {
>> +#endif
>>  virReportError(VIR_ERR_LIBSSH, "%s",
>> _("failed to get the key of the current "
>>   "session"));
>
> How about making it easier to read and harder to mess up:
>
> #if LIBSSH_VERSION_INT > 0x0705 /* 0.7.5 */
> rc = ssh_get_server_publickey(sess->session, );
> #else
> rc = ssh_get_publickey(sess->session, );
> #endif
>
> if (rc != SSH_OK) {
> ...
> }
>
> Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 07/14] util: Extend virtpm.c with tpm-emulator support

[Stefan Berger]

Add functions for managing the storage of the external swtpm as well
as starting and stopping it. Also implement functions to use swtpm_setup,
which simulates the manufacturing of a TPM which includes creation of
certificates for the device.

Signed-off-by: Stefan Berger 
---
 src/libvirt_private.syms |   5 +
 src/util/virtpm.c| 536 ++-
 src/util/virtpm.h|  33 ++-
 3 files changed, 572 insertions(+), 2 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 33fe75b..eebfc72 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2984,6 +2984,11 @@ virTimeStringThenRaw;
 
 # util/virtpm.h
 virTPMCreateCancelPath;
+virTPMDeleteEmulatorStorage;
+virTPMEmulatorBuildCommand;
+virTPMEmulatorInitPaths;
+virTPMEmulatorPrepareHost;
+virTPMEmulatorStop;
 
 
 # util/virtypedparam.h
diff --git a/src/util/virtpm.c b/src/util/virtpm.c
index d5c10da..76bbb21 100644
--- a/src/util/virtpm.c
+++ b/src/util/virtpm.c
@@ -1,7 +1,7 @@
 /*
  * virtpm.c: TPM support
  *
- * Copyright (C) 2013 IBM Corporation
+ * Copyright (C) 2013,2018 IBM Corporation
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -22,16 +22,36 @@
 
 #include 
 
+#include 
 #include 
+#include 
+#include 
+#include 
 
+#include "conf/domain_conf.h"
+#include "viralloc.h"
+#include "vircommand.h"
 #include "virstring.h"
 #include "virerror.h"
 #include "viralloc.h"
 #include "virfile.h"
+#include "virkmod.h"
+#include "virlog.h"
 #include "virtpm.h"
+#include "virutil.h"
+#include "configmake.h"
 
 #define VIR_FROM_THIS VIR_FROM_NONE
 
+VIR_LOG_INIT("util.tpm")
+
+/*
+ * executables for the swtpm; to be found on the host
+ */
+static char *swtpm_path;
+static char *swtpm_setup;
+static char *swtpm_ioctl;
+
 /**
  * virTPMCreateCancelPath:
  * @devpath: Path to the TPM device
@@ -74,3 +94,517 @@ virTPMCreateCancelPath(const char *devpath)
  cleanup:
 return path;
 }
+
+/*
+ * virTPMEmulatorInit
+ *
+ * Initialize the Emulator functions by searching for necessary
+ * executables that we will use to start and setup the swtpm
+ */
+static int
+virTPMEmulatorInit(void)
+{
+if (!swtpm_path) {
+swtpm_path = virFindFileInPath("swtpm");
+if (!swtpm_path) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Could not find swtpm 'swtpm' in PATH"));
+return -1;
+}
+if (!virFileIsExecutable(swtpm_path)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("TPM emulator %s is not an executable"),
+   swtpm_path);
+VIR_FREE(swtpm_path);
+return -1;
+}
+}
+
+if (!swtpm_setup) {
+swtpm_setup = virFindFileInPath("swtpm_setup");
+if (!swtpm_setup) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Could not find 'swtpm_setup' in PATH"));
+return -1;
+}
+if (!virFileIsExecutable(swtpm_setup)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("'%s' is not an executable"),
+   swtpm_setup);
+VIR_FREE(swtpm_setup);
+return -1;
+}
+}
+
+if (!swtpm_ioctl) {
+swtpm_ioctl = virFindFileInPath("swtpm_ioctl");
+if (!swtpm_ioctl) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Could not find swtpm_ioctl in PATH"));
+return -1;
+}
+if (!virFileIsExecutable(swtpm_ioctl)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("swtpm_ioctl program %s is not an executable"),
+   swtpm_ioctl);
+VIR_FREE(swtpm_ioctl);
+return -1;
+}
+}
+
+return 0;
+}
+
+/*
+ * virTPMCreateEmulatorStoragePath
+ *
+ * @swtpmStorageDir: directory for swtpm persistent state
+ * @vmname: The name of the VM for which to create the storage
+ *
+ * Create the swtpm's storage path
+ */
+static char *
+virTPMCreateEmulatorStoragePath(const char *swtpmStorageDir,
+const char *vmname)
+{
+char *path = NULL;
+
+ignore_value(virAsprintf(, "%s/%s/tpm1.2", swtpmStorageDir, vmname));
+
+return path;
+}
+
+/*
+ * virtTPMGetTPMStorageDir:
+ *
+ * @storagepath: directory for swtpm's pesistent state
+ *
+ * Derive the 'TPMStorageDir' from the storagepath by searching
+ * for the last '/'.
+ */
+static char *
+virTPMGetTPMStorageDir(const char *storagepath)
+{
+const char *tail = strrchr(storagepath, '/');
+char *path = NULL;
+
+if (!tail) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Could not get tail of storagedir %s"),
+   storagepath);
+return NULL;
+}
+

[libvirt] [PATCH v3 05/14] util: Implement virFileChownFiles()

[Stefan Berger]

Implement virFileChownFiles() which changes file ownership of all
files in a given directory.

Signed-off-by: Stefan Berger 
---
 src/libvirt_private.syms |  1 +
 src/util/virfile.c   | 49 
 src/util/virfile.h   |  3 +++
 3 files changed, 53 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f2a4921..33fe75b 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1761,6 +1761,7 @@ virFileActivateDirOverride;
 virFileBindMountDevice;
 virFileBuildPath;
 virFileCanonicalizePath;
+virFileChownFiles;
 virFileClose;
 virFileComparePaths;
 virFileCopyACLs;
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 526b9ad..b6aaf2c 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -38,6 +38,7 @@
 #include 
 #include 
 #include 
+#include 
 #if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R
 # include 
 #endif
@@ -2949,6 +2950,54 @@ void virDirClose(DIR **dirp)
 *dirp = NULL;
 }
 
+/*
+ * virFileChownFiles:
+ * @name: name of the directory
+ * @uid: uid
+ * @gid: gid
+ *
+ * Change ownership of all regular files in a directory.
+ *
+ * Returns -1 on error, with error already reported, 0 on success.
+ */
+int virFileChownFiles(const char *name, uid_t uid, gid_t gid)
+{
+struct dirent *ent;
+int ret;
+DIR *dir;
+char *path;
+
+if (virDirOpen(, name) < 0)
+return -1;
+
+while ((ret = virDirRead(dir, , name)) > 0) {
+if (ent->d_type != DT_REG)
+continue;
+
+if (virAsprintf(, "%s/%s", name, ent->d_name) < 0) {
+ret = -1;
+break;
+}
+if (chown(path, uid, gid) < 0) {
+ret = -1;
+virReportSystemError(errno,
+ _("cannot chown '%s' to (%u, %u)"),
+ ent->d_name, (unsigned int) uid,
+ (unsigned int) gid);
+}
+VIR_FREE(path);
+if (ret < 0)
+break;
+}
+
+virDirClose();
+
+if (ret < 0)
+return -1;
+
+return 0;
+}
+
 static int
 virFileMakePathHelper(char *path, mode_t mode)
 {
diff --git a/src/util/virfile.h b/src/util/virfile.h
index 13d3cf6..f0d99a0 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -239,6 +239,9 @@ int virFileOpenAs(const char *path, int openflags, mode_t 
mode,
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
 int virFileRemove(const char *path, uid_t uid, gid_t gid);
 
+int virFileChownFiles(const char *name, uid_t uid, gid_t gid)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
+
 enum {
 VIR_DIR_CREATE_NONE= 0,
 VIR_DIR_CREATE_AS_UID  = (1 << 0),
-- 
2.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 14/14] qemu: Add swtpm to emulator cgroup

[Stefan Berger]

Add the external swtpm to the emulator cgroup so that upper limits of CPU
usage can be enforced on the emulated TPM.

To enable this we need to have the swtpm write its process id (pid) into a
file. We then read it from the file to configure the emulator cgroup.

The PID file is created in /var/run/libvirt/qemu/swtpm:

[root@localhost swtpm]# ls -lZ /var/run/libvirt/qemu/swtpm/
total 4
-rw-r--r--. 1 tss  tss  system_u:object_r:qemu_var_run_t:s0  5 Apr 10 
12:26 1-testvm-swtpm.pid
srw-rw. 1 qemu qemu system_u:object_r:svirt_image_t:s0:c597,c632 0 Apr 10 
12:26 1-testvm-swtpm.sock

The swtpm command line now looks as follows:

root@localhost testvm]# ps auxZ | grep swtpm | grep socket | grep -v grep
system_u:system_r:virtd_t:s0:c597,c632 tss 18697 0.0  0.0 28172 3892 ?   Ss 
  16:46   0:00 /usr/bin/swtpm socket --daemon --ctrl 
type=unixio,path=/var/run/libvirt/qemu/swtpm/1-testvm-swtpm.sock,mode=0600 
--tpmstate 
dir=/var/lib/libvirt/swtpm/485d0004-a48f-436a-8457-8a3b73e28568/tpm1.2/ --log 
file=/var/log/swtpm/libvirt/qemu/testvm-swtpm.log --pid 
file=/var/run/libvirt/qemu/swtpm/1-testvm-swtpm.pid

Signed-off-by: Stefan Berger 
---
 src/conf/domain_conf.c|  1 +
 src/conf/domain_conf.h|  1 +
 src/qemu/qemu_cgroup.c| 53 +++
 src/qemu/qemu_cgroup.h|  1 +
 src/qemu/qemu_extdevice.c | 19 +
 src/qemu/qemu_process.c   |  4 
 src/util/virtpm.c | 33 +
 7 files changed, 112 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c98d26a..f542c8e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2624,6 +2624,7 @@ void virDomainTPMDefFree(virDomainTPMDefPtr def)
 VIR_FREE(def->data.emulator.source.data.nix.path);
 VIR_FREE(def->data.emulator.storagepath);
 VIR_FREE(def->data.emulator.logfile);
+VIR_FREE(def->data.emulator.pidfile);
 break;
 case VIR_DOMAIN_TPM_TYPE_LAST:
 break;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 826ff26..49c77f7 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1311,6 +1311,7 @@ struct _virDomainTPMDef {
 virDomainChrSourceDef source;
 char *storagepath;
 char *logfile;
+char *pidfile;
 } emulator;
 } data;
 };
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 1a5adca..f86ac3f 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -38,6 +38,7 @@
 #include "virnuma.h"
 #include "virsystemd.h"
 #include "virdevmapper.h"
+#include "virpidfile.h"
 
 #define VIR_FROM_THIS VIR_FROM_QEMU
 
@@ -1146,6 +1147,58 @@ qemuSetupCgroupCpusetCpus(virCgroupPtr cgroup,
 
 
 int
+qemuSetupCgroupForExtDevices(virDomainObjPtr vm)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+virDomainTPMDefPtr tpm = vm->def->tpm;
+virCgroupPtr cgroup_temp = NULL;
+pid_t pid;
+int ret = -1;
+
+if (priv->cgroup == NULL)
+return 0; /* Not supported, so claim success */
+
+/*
+ * If CPU cgroup controller is not initialized here, then we need
+ * neither period nor quota settings.  And if CPUSET controller is
+ * not initialized either, then there's nothing to do anyway.
+ */
+if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU) &&
+!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET))
+return 0;
+
+if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR, 0,
+   false, _temp) < 0)
+goto cleanup;
+
+if (tpm) {
+switch (tpm->type) {
+case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+if (virPidFileReadPath(tpm->data.emulator.pidfile, ) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Could not read swtpm's pidfile %s"),
+   tpm->data.emulator.pidfile);
+goto cleanup;
+}
+if (virCgroupAddTask(cgroup_temp, pid) < 0)
+goto cleanup;
+break;
+case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
+case VIR_DOMAIN_TPM_TYPE_LAST:
+break;
+}
+}
+
+ret = 0;
+
+cleanup:
+virCgroupFree(_temp);
+
+return ret;
+}
+
+
+int
 qemuSetupGlobalCpuCgroup(virDomainObjPtr vm)
 {
 qemuDomainObjPrivatePtr priv = vm->privateData;
diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h
index 3b8ff60..478bf7e 100644
--- a/src/qemu/qemu_cgroup.h
+++ b/src/qemu/qemu_cgroup.h
@@ -69,6 +69,7 @@ int qemuSetupCgroupVcpuBW(virCgroupPtr cgroup,
   long long quota);
 int qemuSetupCgroupCpusetCpus(virCgroupPtr cgroup, virBitmapPtr cpumask);
 int qemuSetupGlobalCpuCgroup(virDomainObjPtr vm);
+int qemuSetupCgroupForExtDevices(virDomainObjPtr vm);
 int qemuRemoveCgroup(virDomainObjPtr vm);
 
 typedef struct 

[libvirt] [PATCH v3 02/14] util: Implement virStringFilterLines()

[Stefan Berger]

Implement virStringFilterLines() that takes as an input a buffer with text
and extracts each line that contains a given needle. The size of each re-
turned line can be restricted and if it is restricted '...' will automa-
tically be appended.

Signed-off-by: Stefan Berger 
---
 src/util/virstring.c | 62 
 src/util/virstring.h |  3 +++
 2 files changed, 65 insertions(+)

diff --git a/src/util/virstring.c b/src/util/virstring.c
index 15f367a..f1d91c7 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -1499,3 +1499,65 @@ virStringParsePort(const char *str,
 
 return 0;
 }
+
+/**
+ * virStringFilterLines:
+ * @input: input buffer with text
+ * @needle: the needle to search in each line
+ * @maxlinelen: maximum line length of each line in output buffer;
+ *  0 to not restrict
+ *
+ * Search for a given needle in each line of the input buffer and create
+ * an output buffer that contains only these line.
+ */
+char *
+virStringFilterLines(char *input, const char *needle, size_t maxlinelen)
+{
+char *sol = input;
+char *eol;
+char *buf = NULL;
+size_t buflen = 1, llen;
+const char *dots = "...";
+
+while (*sol) {
+eol = strchr(sol, '\n');
+if (eol)
+*eol = 0;
+
+if (strstr(sol, needle)) {
+size_t additional = 0;
+
+llen = strlen(sol);
+if (maxlinelen && llen > maxlinelen) {
+llen = maxlinelen;
+additional = strlen(dots);
+}
+
+if (VIR_REALLOC_N(buf, buflen + llen + additional + 1) < 0) {
+VIR_FREE(buf);
+if (*eol)
+*eol = '\n';
+return NULL;
+}
+strncpy([buflen - 1], sol, llen);
+buflen += llen;
+
+if (additional) {
+strncpy([buflen - 1], dots, additional);
+buflen += additional;
+}
+
+strcpy([buflen - 1], "\n");
+buflen += 1;
+}
+
+if (eol)
+*eol = '\n';
+else
+break;
+
+sol = eol + 1;
+}
+
+return buf;
+}
diff --git a/src/util/virstring.h b/src/util/virstring.h
index fa2ec1d..1fb9851 100644
--- a/src/util/virstring.h
+++ b/src/util/virstring.h
@@ -309,4 +309,7 @@ int virStringParsePort(const char *str,
unsigned int *port)
 ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
 
+char *virStringFilterLines(char *input, const char *needle, size_t maxlinelen)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+
 #endif /* __VIR_STRING_H__ */
-- 
2.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 06/14] security: Add DAC and SELinux security for tpm-emulator

[Stefan Berger]

Extend the DAC and SELinux modules with support for the
tpm-emulator.

Signed-off-by: Stefan Berger 
---
 src/security/security_dac.c | 4 
 src/security/security_selinux.c | 5 +
 2 files changed, 9 insertions(+)

diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 5efbc27..351f6f4 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -1373,6 +1373,10 @@ virSecurityDACSetTPMFileLabel(virSecurityManagerPtr mgr,
 false);
 break;
 case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+ret = virSecurityDACSetChardevLabel(mgr, def,
+>data.emulator.source,
+false);
+break;
 case VIR_DOMAIN_TPM_TYPE_LAST:
 break;
 }
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index f5ba877..17bc07a 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1473,6 +1473,11 @@ virSecuritySELinuxSetTPMFileLabel(virSecurityManagerPtr 
mgr,
 }
 break;
 case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+tpmdev = tpm->data.emulator.source.data.nix.path;
+rc = virSecuritySELinuxSetFilecon(mgr, tpmdev, seclabel->imagelabel);
+if (rc < 0)
+return -1;
+break;
 case VIR_DOMAIN_TPM_TYPE_LAST:
 break;
 }
-- 
2.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 00/14] Add support for TPM emulator

[Stefan Berger]

This series of patches adds support for the TPM emulator backend that
is available in QEMU and based on swtpm + libtpms. It allows to attach a
TPM 1.2 or 2 to a QEMU VM. sVirt labels are used for labeling the swtpm
process, its Unix socket, and log file with the same label that the
QEMU process gets. Besides that swtpm is added to the emulator cgroup to
restrict its CPU usage.

The device XML can be changed from a TPM 1.2 to a TPM 2 and back to a
TPM 1.2. The device state is not removed during those changes but only
when the domain is undefined.

The swtpm needs persistent storage to store its state. For that I am
using the uuid of the VM as part of the path since the name of the VM
can be changed. Logfiles, PID files, and socket names are based on the
name of the VM, though.

  Stefan

Stefan Berger (14):
  util: implement virFileReadOffsetQuiet()
  util: Implement virStringFilterLines()
  conf: Add support for external swtpm TPM emulator to domain XML
  qemu: Extend QEMU capabilities with 'tpm-emulator'
  util: Implement virFileChownFiles()
  security: Add DAC and SELinux security for tpm-emulator
  util: Extend virtpm.c with tpm-emulator support
  qemu: Extend qemu_conf with tpm-emulator support
  qemu: Implement a layer for external devices like tpm-emulator
  qemu: Add support for external swtpm TPM emulator
  tests: Add test cases for external swtpm TPM emulator
  security: Label the external swtpm with SELinux labels
  tpm: Add support for choosing emulation of a TPM 2
  qemu: Add swtpm to emulator cgroup

 docs/formatdomain.html.in  |  45 ++
 docs/schemas/domaincommon.rng  |  17 +
 src/conf/domain_audit.c|   2 +
 src/conf/domain_conf.c |  70 ++-
 src/conf/domain_conf.h |  14 +
 src/libvirt_private.syms   |   9 +
 src/qemu/Makefile.inc.am   |   2 +
 src/qemu/libvirtd_qemu.aug |   5 +
 src/qemu/qemu.conf |   8 +
 src/qemu/qemu_capabilities.c   |   5 +
 src/qemu/qemu_capabilities.h   |   1 +
 src/qemu/qemu_cgroup.c |  54 ++
 src/qemu/qemu_cgroup.h |   1 +
 src/qemu/qemu_command.c|  40 +-
 src/qemu/qemu_conf.c   |  43 ++
 src/qemu/qemu_conf.h   |   6 +
 src/qemu/qemu_domain.c |   4 +
 src/qemu/qemu_driver.c |   7 +
 src/qemu/qemu_extdevice.c  | 339 +++
 src/qemu/qemu_extdevice.h  |  43 ++
 src/qemu/qemu_process.c|  17 +
 src/qemu/test_libvirtd_qemu.aug.in |   2 +
 src/security/security_dac.c|   6 +
 src/security/security_driver.h |   4 +
 src/security/security_manager.c|  17 +
 src/security/security_manager.h|   3 +
 src/security/security_selinux.c|  89 +++
 src/security/security_stack.c  |  19 +
 src/util/virfile.c |  63 +-
 src/util/virfile.h |   6 +
 src/util/virstring.c   |  62 ++
 src/util/virstring.h   |   3 +
 src/util/virtpm.c  | 638 -
 src/util/virtpm.h  |  33 +-
 tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml   |   1 +
 tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml |   1 +
 tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml   |   1 +
 tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml   |   1 +
 tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml  |   1 +
 tests/qemuxml2argvdata/tpm-emulator-tpm2.args  |  27 +
 tests/qemuxml2argvdata/tpm-emulator-tpm2.xml   |  30 +
 tests/qemuxml2argvdata/tpm-emulator.args   |  27 +
 tests/qemuxml2argvdata/tpm-emulator.xml|  30 +
 tests/qemuxml2argvtest.c   |  17 +
 tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml |  34 ++
 tests/qemuxml2xmloutdata/tpm-emulator.xml  |  34 ++
 tests/qemuxml2xmltest.c|   1 +
 47 files changed, 1866 insertions(+), 16 deletions(-)
 create mode 100644 src/qemu/qemu_extdevice.c
 create mode 100644 src/qemu/qemu_extdevice.h
 create mode 100644 tests/qemuxml2argvdata/tpm-emulator-tpm2.args
 create mode 100644 tests/qemuxml2argvdata/tpm-emulator-tpm2.xml
 create mode 100644 tests/qemuxml2argvdata/tpm-emulator.args
 create mode 100644 tests/qemuxml2argvdata/tpm-emulator.xml
 create mode 100644 tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml
 create mode 100644 tests/qemuxml2xmloutdata/tpm-emulator.xml

-- 
2.5.5

--
libvir-list mailing list
libvir-list@redhat.com

[libvirt] [PATCH v3 11/14] tests: Add test cases for external swtpm TPM emulator

[Stefan Berger]

This patch adds extensions to existing test cases and specific test cases
for the tpm-emulator.

Signed-off-by: Stefan Berger 
---
 tests/qemuxml2argvdata/tpm-emulator.args | 27 +++
 tests/qemuxml2argvtest.c | 15 +++
 2 files changed, 42 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/tpm-emulator.args

diff --git a/tests/qemuxml2argvdata/tpm-emulator.args 
b/tests/qemuxml2argvdata/tpm-emulator.args
new file mode 100644
index 000..5970928
--- /dev/null
+++ b/tests/qemuxml2argvdata/tpm-emulator.args
@@ -0,0 +1,27 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-x86_64 \
+-name TPM-VM \
+-S \
+-machine pc-i440fx-2.12,accel=tcg,usb=off,dump-guest-core=off \
+-m 2048 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid 11d7cd22-da89-3094-6212-079a48a309a1 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,\
+path=/tmp/lib/domain--1-TPM-VM/monitor.sock,server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-boot order=c,menu=on \
+-usb \
+-tpmdev emulator,id=tpm-tpm0,chardev=chrtpm \
+-chardev socket,id=chrtpm,path=/dev/test \
+-device tpm-tis,tpmdev=tpm-tpm0,id=tpm0 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 8ef7701..a80e3f2 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -532,6 +532,19 @@ testCompareXMLToArgv(const void *data)
 }
 }
 
+if (vm->def->tpm) {
+   switch (vm->def->tpm->type) {
+   case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+   if (VIR_STRDUP(vm->def->tpm->data.emulator.source.data.file.path,
+  "/dev/test") < 0)
+   goto cleanup;
+   break;
+   case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
+   case VIR_DOMAIN_TPM_TYPE_LAST:
+   break;
+   }
+}
+
 if (!(cmd = qemuProcessCreatePretendCmd(, vm, migrateURI,
 (flags & FLAG_FIPS), false,
 VIR_QEMU_PROCESS_START_COLD))) {
@@ -1989,6 +2002,8 @@ mymain(void)
 QEMU_CAPS_DEVICE_TPM_PASSTHROUGH, QEMU_CAPS_DEVICE_TPM_CRB);
 DO_TEST_PARSE_ERROR("tpm-no-backend-invalid",
 QEMU_CAPS_DEVICE_TPM_PASSTHROUGH, 
QEMU_CAPS_DEVICE_TPM_TIS);
+DO_TEST("tpm-emulator",
+QEMU_CAPS_DEVICE_TPM_EMULATOR, QEMU_CAPS_DEVICE_TPM_TIS);
 
 
 DO_TEST_PARSE_ERROR("pci-domain-invalid", NONE);
-- 
2.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 10/14] qemu: Add support for external swtpm TPM emulator

[Stefan Berger]

This patch adds support for an external swtpm TPM emulator. The XML for
this type of TPM looks as follows:

 
   
 

The XML will currently only start a TPM 1.2.

Upon first start, libvirt will run `swtpm_setup`, which will simulate the
manufacturing of a TPM and create certificates for it and write them into
NVRAM locations of the emulated TPM.

After that libvirt starts the swtpm TPM emulator using the `swtpm` executable.

Once the VM terminates, libvirt uses the swtpm_ioctl executable to gracefully
shut down the `swtpm` in case it is still running (QEMU did not send shutdown)
or clean up the socket file.

The above mentioned executables must be found in the PATH.

The executables can either be run as root or started as root and switch to
the tss user. The requirement for the tss user comes through 'tcsd', which
is used for the simulation of the manufacturing. Which user is used can be
configured through qemu.conf. By default 'tss' is used.

The swtpm writes out state into files. The state is kept in 
/var/lib/libvirt/swtpm:

[root@localhost libvirt]# ls -lZ | grep swtpm

drwx--x--x. 7 root root unconfined_u:object_r:virt_var_lib_t:s0 4096 Apr  5 
16:22 swtpm

The directory /var/lib/libvirt/swtpm maintains per-TPM state directories.
(Using the uuid of the VM for that since the name can change per VM renaming but
 we need a stable directory name.)

[root@localhost swtpm]# ls -lZ
total 4
drwx--. 2 tss  tss  system_u:object_r:virt_var_lib_t:s0  4096 Apr  
5 16:46 485d0004-a48f-436a-8457-8a3b73e28568

[root@localhost 485d0004-a48f-436a-8457-8a3b73e28568]# ls -lZ
total 4
drwx--. 2 tss tss system_u:object_r:virt_var_lib_t:s0 4096 Apr 10 21:34 
tpm1.2

[root@localhost tpm1.2]# ls -lZ
total 8
-rw-r--r--. 1 tss tss system_u:object_r:virt_var_lib_t:s0 3648 Apr  5 16:46 
tpm-00.permall

The directory /var/run/libvirt/qemu/swtpm/ hosts the swtpm.sock that
QEMU uses to communicate with the swtpm:

root@localhost domain-1-testvm]# ls -lZ
total 0
srw---. 1 qemu qemu system_u:object_r:svirt_image_t:s0:c597,c632  0 Apr  6 
10:24 1-testvm-swtpm.sock

The logfile for the swtpm is in /var/log/swtpm/libvirt/qemu:

[root@localhost-3 qemu]# ls -lZ
total 4
-rw---. 1 tss tss unconfined_u:object_r:var_log_t:s0 2199 Apr  6 14:01 
testvm-swtpm.log

The processes are labeled as follows:

[root@localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep swtpm | 
grep socket | grep -v grep
system_u:system_r:virtd_t:s0-s0:c0.c1023 tss 18697 0.0  0.0 28172 3892 ?   
Ss   16:46   0:00 /usr/bin/swtpm socket --daemon --ctrl 
type=unixio,path=/var/run/libvirt/qemu/swtpm/1-testvm-swtpm.sock,mode=0600 
--tpmstate 
dir=/var/lib/libvirt/swtpm/485d0004-a48f-436a-8457-8a3b73e28568/tpm1.2 --log 
file=/var/log/swtpm/libvirt/qemu/testvm-swtpm.log

[root@localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep qemu | 
grep tpm | grep -v grep
system_u:system_r:svirt_t:s0:c413,c430 qemu 18702 2.5  0.0 3036052 48676 ? 
Sl   16:46   0:08 /bin/qemu-system-x86_64 [...]

Signed-off-by: Stefan Berger 
---
 src/conf/domain_conf.c   | 22 ++
 src/libvirt_private.syms |  1 +
 src/qemu/qemu_command.c  | 39 +--
 src/qemu/qemu_domain.c   |  3 +++
 src/qemu/qemu_driver.c   |  7 +++
 5 files changed, 66 insertions(+), 6 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d9945dd..a42574a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2593,6 +2593,24 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr def)
 }
 }
 
+void virDomainTPMDelete(virDomainDefPtr def)
+{
+virDomainTPMDefPtr tpm = def->tpm;
+
+if (!tpm)
+return;
+
+switch (tpm->type) {
+case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+virTPMDeleteEmulatorStorage(tpm);
+break;
+case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
+case VIR_DOMAIN_TPM_TYPE_LAST:
+/* nothing to do */
+break;
+}
+}
+
 void virDomainTPMDefFree(virDomainTPMDefPtr def)
 {
 if (!def)
@@ -27614,6 +27632,10 @@ virDomainDeleteConfig(const char *configDir,
 goto cleanup;
 }
 
+/* in case domain is NOT running, remove any TPM storage */
+if (!dom->persistent)
+virDomainTPMDelete(dom->def);
+
 ret = 0;
 
  cleanup:
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index eebfc72..e533b95 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -559,6 +559,7 @@ virDomainTimerTrackTypeToString;
 virDomainTPMBackendTypeFromString;
 virDomainTPMBackendTypeToString;
 virDomainTPMDefFree;
+virDomainTPMDelete;
 virDomainTPMModelTypeFromString;
 virDomainTPMModelTypeToString;
 virDomainUSBDeviceDefForeach;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index bb330bf..c02b783 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9425,21 +9425,31 @@ qemuBuildTPMDevStr(const virDomainDef *def,
 
 
 static char *

[libvirt] [PATCH v3 08/14] qemu: Extend qemu_conf with tpm-emulator support

[Stefan Berger]

Extend qemu_conf with user and group for running the tpm-emulator
and add directories to the configuration for the locations of the
log, state, and socket of the tpm-emulator.

Signed-off-by: Stefan Berger 
---
 src/qemu/libvirtd_qemu.aug |  5 +
 src/qemu/qemu.conf |  8 +++
 src/qemu/qemu_conf.c   | 43 ++
 src/qemu/qemu_conf.h   |  6 ++
 src/qemu/test_libvirtd_qemu.aug.in |  2 ++
 5 files changed, 64 insertions(+)

diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index c19bf3a..23bfe67 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -118,6 +118,9 @@ module Libvirtd_qemu =
let vxhs_entry = bool_entry "vxhs_tls"
  | str_entry "vxhs_tls_x509_cert_dir"
 
+   let swtpm_user_entry = str_entry "swtpm_user"
+   let swtpm_group_entry = str_entry "swtpm_group"
+
(* Each entry in the config is one of the following ... *)
let entry = default_tls_entry
  | vnc_entry
@@ -137,6 +140,8 @@ module Libvirtd_qemu =
  | gluster_debug_level_entry
  | memory_entry
  | vxhs_entry
+ | swtpm_user_entry
+ | swtpm_group_entry
 
let comment = [ label "#comment" . del /#[ \t]*/ "# " .  store /([^ 
\t\n][^\n]*)?/ . del /\n/ "\n" ]
let empty = [ label "#empty" . eol ]
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index 3444185..26a6dc7 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -779,3 +779,11 @@
 # This directory is used for memoryBacking source if configured as file.
 # NOTE: big files will be stored here
 #memory_backing_dir = "/var/lib/libvirt/qemu/ram"
+
+# User for the swtpm TPM Emulator
+#
+# Default is 'tss'; this is the same user that tcsd (TrouSerS) installs
+# and uses; alternative is 'root'
+#
+#swtpm_user = "tss"
+#swtpm_group = "tss"
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index bfbb572..99c37c6 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -159,6 +159,10 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool 
privileged)
 "%s/log/libvirt/qemu", LOCALSTATEDIR) < 0)
 goto error;
 
+if (virAsprintf(>swtpmLogDir,
+"%s/log/swtpm/libvirt/qemu", LOCALSTATEDIR) < 0)
+goto error;
+
 if (VIR_STRDUP(cfg->configBaseDir, SYSCONFDIR "/libvirt") < 0)
 goto error;
 
@@ -166,6 +170,10 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool 
privileged)
   "%s/run/libvirt/qemu", LOCALSTATEDIR) < 0)
 goto error;
 
+if (virAsprintf(>swtpmStateDir,
+   "%s/run/libvirt/qemu/swtpm", LOCALSTATEDIR) < 0)
+goto error;
+
 if (virAsprintf(>cacheDir,
   "%s/cache/libvirt/qemu", LOCALSTATEDIR) < 0)
 goto error;
@@ -186,6 +194,13 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool 
privileged)
 goto error;
 if (virAsprintf(>memoryBackingDir, "%s/ram", cfg->libDir) < 0)
 goto error;
+if (virAsprintf(>swtpmStorageDir, "%s/lib/libvirt/swtpm",
+LOCALSTATEDIR) < 0)
+goto error;
+if (virGetUserID("tss", >swtpm_user) < 0)
+cfg->swtpm_user = 0; /* fall back to root */
+if (virGetGroupID("tss", >swtpm_group) < 0)
+cfg->swtpm_group = 0; /* fall back to root */
 } else {
 char *rundir;
 char *cachedir;
@@ -199,6 +214,11 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool 
privileged)
 VIR_FREE(cachedir);
 goto error;
 }
+if (virAsprintf(>swtpmLogDir,
+"%s/qemu/log", cachedir) < 0) {
+VIR_FREE(cachedir);
+goto error;
+}
 if (virAsprintf(>cacheDir, "%s/qemu/cache", cachedir) < 0) {
 VIR_FREE(cachedir);
 goto error;
@@ -214,6 +234,9 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool 
privileged)
 }
 VIR_FREE(rundir);
 
+if (virAsprintf(>swtpmStateDir, "%s/swtpm", cfg->stateDir) < 0)
+goto error;
+
 if (!(cfg->configBaseDir = virGetUserConfigDirectory()))
 goto error;
 
@@ -233,6 +256,10 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool 
privileged)
 goto error;
 if (virAsprintf(>memoryBackingDir, "%s/qemu/ram", 
cfg->configBaseDir) < 0)
 goto error;
+if (virAsprintf(>swtpmStorageDir, "%s/qemu/swtpm", 
cfg->configBaseDir) < 0)
+goto error;
+cfg->swtpm_user = -1;
+cfg->swtpm_group = -1;
 }
 
 if (virAsprintf(>configDir, "%s/qemu", cfg->configBaseDir) < 0)
@@ -351,7 +378,9 @@ static void virQEMUDriverConfigDispose(void *obj)
 VIR_FREE(cfg->configDir);
 VIR_FREE(cfg->autostartDir);
 VIR_FREE(cfg->logDir);
+

[libvirt] [PATCH v3 03/14] conf: Add support for external swtpm TPM emulator to domain XML

[Stefan Berger]

This patch adds support for an external swtpm TPM emulator. The XML for
this type of TPM looks as follows:

 
   
 

The XML will currently only define a TPM 1.2.

Extend the documentation.

Add a test case testing the XML parser and formatter.

Signed-off-by: Stefan Berger 
---
 docs/formatdomain.html.in | 30 +++
 docs/schemas/domaincommon.rng |  5 +
 src/conf/domain_audit.c   |  2 ++
 src/conf/domain_conf.c| 28 ++---
 src/conf/domain_conf.h|  7 +++
 src/qemu/qemu_cgroup.c|  1 +
 src/qemu/qemu_command.c   |  1 +
 src/qemu/qemu_domain.c|  1 +
 src/security/security_dac.c   |  2 ++
 src/security/security_selinux.c   |  2 ++
 tests/qemuxml2argvdata/tpm-emulator.xml   | 30 +++
 tests/qemuxml2xmloutdata/tpm-emulator.xml | 34 +++
 tests/qemuxml2xmltest.c   |  1 +
 13 files changed, 137 insertions(+), 7 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/tpm-emulator.xml
 create mode 100644 tests/qemuxml2xmloutdata/tpm-emulator.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 6a0110e..2a8912f 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -7649,6 +7649,26 @@ qemu-kvm -net nic,model=? /dev/null
 /devices
 ...
 
+
+
+  The emulator device type gives access to a TPM emulator providing
+  TPM functionlity for each VM. QEMU talks to it over a Unix socket. With
+  the emulator device type each guest gets its own private TPM.
+  'emulator' since 4.4.0
+
+
+ Example: usage of the TPM Emulator
+
+
+  ...
+  devices
+tpm model='tpm-tis'
+  backend type='emulator'
+  /backend
+/tpm
+  /devices
+  ...
+
 
   model
   
@@ -7682,6 +7702,16 @@ qemu-kvm -net nic,model=? /dev/null
 
   
 
+
+  emulator
+  
+
+  For this backend type the 'swtpm' TPM Emulator must be installed 
on the
+  host. Libvirt will automatically start an independent TPM 
emulator
+  for each QEMU guest requesting access to it.
+
+  
+
   
 
 
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 7bad7dd..c65a9a3 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4137,6 +4137,11 @@
   
   
 
+
+  
+ emulator
+  
+
   
 
   
diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
index 82868bc..25cccdd 100644
--- a/src/conf/domain_audit.c
+++ b/src/conf/domain_audit.c
@@ -586,6 +586,8 @@ virDomainAuditTPM(virDomainObjPtr vm, virDomainTPMDefPtr 
tpm,
   "virt=%s resrc=dev reason=%s %s uuid=%s %s",
   virt, reason, vmname, uuidstr, device);
 break;
+case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+break;
 case VIR_DOMAIN_TPM_TYPE_LAST:
 default:
 break;
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0ea3e4c..d9945dd 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -864,7 +864,8 @@ VIR_ENUM_IMPL(virDomainTPMModel, VIR_DOMAIN_TPM_MODEL_LAST,
   "tpm-crb")
 
 VIR_ENUM_IMPL(virDomainTPMBackend, VIR_DOMAIN_TPM_TYPE_LAST,
-  "passthrough")
+  "passthrough",
+  "emulator")
 
 VIR_ENUM_IMPL(virDomainIOMMUModel, VIR_DOMAIN_IOMMU_MODEL_LAST,
   "intel")
@@ -2601,6 +2602,11 @@ void virDomainTPMDefFree(virDomainTPMDefPtr def)
 case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
 VIR_FREE(def->data.passthrough.source.data.file.path);
 break;
+case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+VIR_FREE(def->data.emulator.source.data.nix.path);
+VIR_FREE(def->data.emulator.storagepath);
+VIR_FREE(def->data.emulator.logfile);
+break;
 case VIR_DOMAIN_TPM_TYPE_LAST:
 break;
 }
@@ -12582,6 +12588,11 @@ virDomainSmartcardDefParseXML(virDomainXMLOptionPtr 
xmlopt,
  *   
  * 
  *
+ * or like this:
+ *
+ * 
+ *   
+ * 
  */
 static virDomainTPMDefPtr
 virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt,
@@ -12648,6 +12659,8 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt,
 def->data.passthrough.source.type = VIR_DOMAIN_CHR_TYPE_DEV;
 path = NULL;
 break;
+case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+break;
 case VIR_DOMAIN_TPM_TYPE_LAST:
 goto error;
 }
@@ -24815,22 +24828,23 @@ virDomainTPMDefFormat(virBufferPtr buf,
 virBufferAsprintf(buf, "\n",
   virDomainTPMModelTypeToString(def->model));
 virBufferAdjustIndent(buf, 2);
-virBufferAsprintf(buf, "\n",
+virBufferAsprintf(buf, "type));
-

[libvirt] [PATCH v3 13/14] tpm: Add support for choosing emulation of a TPM 2

[Stefan Berger]

This patch extends the TPM's device XML with TPM 2 support. This only works
for the emulator type backend and looks as follows:


  


The swtpm process now has --tpm2 as an additional parameter:

system_u:system_r:svirt_t:s0:c597,c632 tss 18477 11.8  0.0 28364  3868 ?
Rs   11:13  13:50 /usr/bin/swtpm socket --daemon --ctrl 
type=unixio,path=/var/run/libvirt/qemu/swtpm/testvm-swtpm.sock,mode=0660 
--tpmstate dir=/var/lib/libvirt/swtpm/testvm/tpm2,mode=0640 --log 
file=/var/log/swtpm/libvirt/qemu/testvm-swtpm.log --tpm2 --pid 
file=/var/run/libvirt/qemu/swtpm/testvm-swtpm.pid

The version of the TPM can be changed and the state of the TPM is preserved.

Signed-off-by: Stefan Berger 
---
 docs/formatdomain.html.in  | 17 +-
 docs/schemas/domaincommon.rng  | 12 
 src/conf/domain_conf.c | 21 ++-
 src/conf/domain_conf.h |  6 ++
 src/util/virtpm.c  | 79 --
 tests/qemuxml2argvdata/tpm-emulator-tpm2.args  | 27 +
 tests/qemuxml2argvdata/tpm-emulator-tpm2.xml   | 30 ++
 tests/qemuxml2argvtest.c   |  2 +
 tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml | 34 +++
 9 files changed, 221 insertions(+), 7 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/tpm-emulator-tpm2.args
 create mode 100644 tests/qemuxml2argvdata/tpm-emulator-tpm2.xml
 create mode 100644 tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 2a8912f..08df78a 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -7663,7 +7663,7 @@ qemu-kvm -net nic,model=? /dev/null
   ...
   devices
 tpm model='tpm-tis'
-  backend type='emulator'
+  backend type='emulator' tpmversion='2'
   /backend
 /tpm
   /devices
@@ -7713,6 +7713,21 @@ qemu-kvm -net nic,model=? /dev/null
   
 
   
+  tpmversion
+  
+
+  The tpmversion attribute indicates the version
+  of the TPM. By default a TPM 1.2 is created. This attribute
+  only works with the emulator backend. The following
+  versions are supported:
+
+
+  '1.2' : creates a TPM 1.2
+  '2' :  creates a TPM 2
+
+Note that once a certain version of a TPM has been created for
+a guest, the version must not be changed anymore.
+  
 
 
 NVRAM device
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index c65a9a3..a452a13 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4143,6 +4143,18 @@
   
 
   
+  
+
+  
+
+  
+1.2
+2
+  
+   
+  
+
+  
 
   
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a42574a..c98d26a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12609,7 +12609,7 @@ virDomainSmartcardDefParseXML(virDomainXMLOptionPtr 
xmlopt,
  * or like this:
  *
  * 
- *   
+ *   
  * 
  */
 static virDomainTPMDefPtr
@@ -12622,6 +12622,7 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt,
 char *path = NULL;
 char *model = NULL;
 char *backend = NULL;
+char *tpmversion = NULL;
 virDomainTPMDefPtr def;
 xmlNodePtr save = ctxt->node;
 xmlNodePtr *backends = NULL;
@@ -12668,6 +12669,20 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt,
 goto error;
 }
 
+tpmversion = virXMLPropString(backends[0], "tpmversion");
+if (!tpmversion || STREQ(tpmversion, "1.2")) {
+def->tpmversion = VIR_DOMAIN_TPM_VERSION_1_2;
+/* only TIS available for emulator */
+if (def->type == VIR_DOMAIN_TPM_TYPE_EMULATOR)
+def->model = VIR_DOMAIN_TPM_MODEL_TIS;
+} else if (STREQ(tpmversion, "2")) {
+def->tpmversion = VIR_DOMAIN_TPM_VERSION_2;
+} else {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("Unsupported TPM version '%s'"),
+   tpmversion);
+}
+
 switch (def->type) {
 case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
 path = virXPathString("string(./backend/device/@path)", ctxt);
@@ -12692,6 +12707,7 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt,
 VIR_FREE(model);
 VIR_FREE(backend);
 VIR_FREE(backends);
+VIR_FREE(tpmversion);
 ctxt->node = save;
 return def;
 
@@ -24849,6 +24865,9 @@ virDomainTPMDefFormat(virBufferPtr buf,
 virBufferAsprintf(buf, "type));
 
+if (def->tpmversion == VIR_DOMAIN_TPM_VERSION_2)
+virBufferAddLit(buf, " tpmversion='2'");
+
 switch (def->type) {
 case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
 virBufferAddLit(buf, ">\n");
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 

[libvirt] [PATCH v3 09/14] qemu: Implement a layer for external devices like tpm-emulator

[Stefan Berger]

Implement a layer for starting and stopping of external devices.
The tpm-emulator is the only user of this layer.

Signed-off-by: Stefan Berger 
---
 src/qemu/Makefile.inc.am  |   2 +
 src/qemu/qemu_extdevice.c | 300 ++
 src/qemu/qemu_extdevice.h |  43 +++
 src/qemu/qemu_process.c   |  13 ++
 4 files changed, 358 insertions(+)
 create mode 100644 src/qemu/qemu_extdevice.c
 create mode 100644 src/qemu/qemu_extdevice.h

diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am
index 63e7c87..d16e880 100644
--- a/src/qemu/Makefile.inc.am
+++ b/src/qemu/Makefile.inc.am
@@ -19,6 +19,8 @@ QEMU_DRIVER_SOURCES = \
qemu/qemu_domain_address.h \
qemu/qemu_cgroup.c \
qemu/qemu_cgroup.h \
+   qemu/qemu_extdevice.c \
+   qemu/qemu_extdevice.h \
qemu/qemu_hostdev.c \
qemu/qemu_hostdev.h \
qemu/qemu_hotplug.c \
diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c
new file mode 100644
index 000..f3f337d
--- /dev/null
+++ b/src/qemu/qemu_extdevice.c
@@ -0,0 +1,300 @@
+/*
+ * qemu_extdevice.c: QEMU external devices support
+ *
+ * Copyright (C) 2014, 2018 IBM Corporation
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * .
+ *
+ * Author: Stefan Berger 
+ */
+
+#include 
+
+#include "qemu_extdevice.h"
+#include "qemu_domain.h"
+
+#include "viralloc.h"
+#include "virlog.h"
+#include "virstring.h"
+#include "virtime.h"
+#include "virtpm.h"
+
+#define VIR_FROM_THIS VIR_FROM_QEMU
+
+VIR_LOG_INIT("qemu.qemu_extdevice")
+
+static int
+qemuExtDeviceLogCommand(qemuDomainLogContextPtr logCtxt,
+virCommandPtr cmd,
+const char *info)
+{
+int ret = -1;
+char *timestamp = NULL;
+char *logline = NULL;
+int logFD;
+
+logFD = qemuDomainLogContextGetWriteFD(logCtxt);
+
+if ((timestamp = virTimeStringNow()) == NULL)
+goto cleanup;
+
+if (virAsprintf(, "%s: Starting external device: %s\n",
+timestamp, info) < 0)
+goto cleanup;
+
+if (safewrite(logFD, logline, strlen(logline)) < 0)
+goto cleanup;
+
+virCommandWriteArgLog(cmd, logFD);
+
+ret = 0;
+
+ cleanup:
+VIR_FREE(timestamp);
+VIR_FREE(logline);
+
+return ret;
+}
+
+
+static int
+qemuExtTPMInitPaths(virQEMUDriverPtr driver,
+virDomainDefPtr def)
+{
+virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+int ret = 0;
+
+switch (def->tpm->type) {
+case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+ret = virTPMEmulatorInitPaths(def->tpm, cfg->swtpmStorageDir,
+  def->uuid);
+break;
+case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
+case VIR_DOMAIN_TPM_TYPE_LAST:
+break;
+}
+
+virObjectUnref(cfg);
+
+return ret;
+}
+
+
+static int
+qemuExtTPMPrepareHost(virQEMUDriverPtr driver,
+  virDomainDefPtr def)
+{
+virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+int ret = 0;
+char *shortName = NULL;
+
+switch (def->tpm->type) {
+case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+shortName = virDomainDefGetShortName(def);
+if (!shortName)
+goto cleanup;
+
+ret = virTPMEmulatorPrepareHost(def->tpm, cfg->swtpmLogDir,
+def->name, cfg->swtpm_user,
+cfg->swtpm_group,
+cfg->swtpmStateDir, cfg->user,
+shortName);
+break;
+case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
+case VIR_DOMAIN_TPM_TYPE_LAST:
+break;
+}
+
+cleanup:
+VIR_FREE(shortName);
+virObjectUnref(cfg);
+
+return ret;
+}
+
+
+/*
+ * qemuExtTPMStartEmulator:
+ *
+ * @driver: QEMU driver
+ * @def: domain definition
+ * @logCtxt: log context
+ *
+ * Start the external TPM Emulator:
+ * - have the command line built
+ * - start the external TPM Emulator and sync with it before QEMU start
+ */
+static int
+qemuExtTPMStartEmulator(virQEMUDriverPtr driver,
+virDomainDefPtr def,
+qemuDomainLogContextPtr logCtxt)
+{
+int ret = -1;
+virCommandPtr cmd = NULL;
+int exitstatus;
+char *errbuf = 

[libvirt] [PATCH v3 12/14] security: Label the external swtpm with SELinux labels

[Stefan Berger]

In this patch we label the swtpm process with SELinux labels. We give it the
same label as the QEMU process has. We label its state directory and files
as well.

The file and process labels now look as follows:

Directory: /var/lib/libvirt/swtpm

[root@localhost swtpm]# ls -lZ
total 4
rwx--. 2 tss  tss  system_u:object_r:svirt_image_t:s0:c254,c932 4096 Apr  5 
16:46 testvm

[root@localhost testvm]# ls -lZ
total 8
-rw-r--r--. 1 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 3648 Apr  5 
16:46 tpm-00.permall

The log in /var/log/swtpm/libvirt/qemu is labeled as follows:

-rw-r--r--. 1 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 2237 Apr  5 
16:46 vtpm.log

[root@localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep swtpm | 
grep ctrl | grep -v grep
system_u:system_r:svirt_t:s0:c254,c932 tss 25664 0.0  0.0 28172  3892 ?
Ss   16:57   0:00 /usr/bin/swtpm socket --daemon --ctrl 
type=unixio,path=/var/run/libvirt/qemu/swtpm/testvm-swtpm.sock,mode=0660 
--tpmstate dir=/var/lib/libvirt/swtpm/testvm/tpm1.2 --log 
file=/var/log/swtpm/libvirt/qemu/testvm-swtpm.log

[root@localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep qemu | 
grep tpm | grep -v grep
system_u:system_r:svirt_t:s0:c254,c932 qemu 25669 99.0  0.0 3096704 48500 ?
Sl   16:57   3:28 /bin/qemu-system-x86_64 [..]

Signed-off-by: Stefan Berger 
---
 src/libvirt_private.syms|  1 +
 src/qemu/qemu_extdevice.c   | 22 ++-
 src/security/security_driver.h  |  4 ++
 src/security/security_manager.c | 17 +
 src/security/security_manager.h |  3 ++
 src/security/security_selinux.c | 82 +
 src/security/security_stack.c   | 19 ++
 7 files changed, 147 insertions(+), 1 deletion(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index e533b95..79b8afa 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1334,6 +1334,7 @@ virSecurityManagerSetProcessLabel;
 virSecurityManagerSetSavedStateLabel;
 virSecurityManagerSetSocketLabel;
 virSecurityManagerSetTapFDLabel;
+virSecurityManagerSetTPMLabels;
 virSecurityManagerStackAddNested;
 virSecurityManagerTransactionAbort;
 virSecurityManagerTransactionCommit;
diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c
index f3f337d..eb7220d 100644
--- a/src/qemu/qemu_extdevice.c
+++ b/src/qemu/qemu_extdevice.c
@@ -166,12 +166,32 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver,
 
 virCommandSetErrorBuffer(cmd, );
 
-if (virCommandRun(cmd, ) < 0 || exitstatus != 0) {
+if (virSecurityManagerSetTPMLabels(driver->securityManager,
+   def) < 0)
+goto error;
+
+if (virSecurityManagerSetChildProcessLabel(driver->securityManager,
+   def, cmd) < 0)
+goto error;
+
+if (virSecurityManagerPreFork(driver->securityManager) < 0)
+goto error;
+
+/* make sure we run this with the appropriate user */
+virCommandSetUID(cmd, cfg->swtpm_user);
+virCommandSetGID(cmd, cfg->swtpm_group);
+
+ret = virCommandRun(cmd, );
+
+virSecurityManagerPostFork(driver->securityManager);
+
+if (ret < 0 || exitstatus != 0) {
 VIR_ERROR("Could not start 'swtpm'. exitstatus: %d\n"
   "stderr: %s\n", exitstatus, errbuf);
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Could not start 'swtpm'. exitstatus: %d, "
"error: %s"), exitstatus, errbuf);
+ret = -1;
 goto error;
 }
 
diff --git a/src/security/security_driver.h b/src/security/security_driver.h
index 95e7c4d..4aa415f 100644
--- a/src/security/security_driver.h
+++ b/src/security/security_driver.h
@@ -149,6 +149,8 @@ typedef int (*virSecurityDomainRestoreChardevLabel) 
(virSecurityManagerPtr mgr,
  virDomainDefPtr def,
  virDomainChrSourceDefPtr 
dev_source,
  bool chardevStdioLogd);
+typedef int (*virSecurityDomainSetTPMLabels) (virSecurityManagerPtr mgr,
+  virDomainDefPtr def);
 
 
 struct _virSecurityDriver {
@@ -213,6 +215,8 @@ struct _virSecurityDriver {
 
 virSecurityDomainSetChardevLabel domainSetSecurityChardevLabel;
 virSecurityDomainRestoreChardevLabel domainRestoreSecurityChardevLabel;
+
+virSecurityDomainSetTPMLabels domainSetSecurityTPMLabels;
 };
 
 virSecurityDriverPtr virSecurityDriverLookup(const char *name,
diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index 71f7f59..48777bb 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -1204,3 +1204,20 @@ 
virSecurityManagerRestoreChardevLabel(virSecurityManagerPtr mgr,
 virReportUnsupportedError();
 return -1;
 }
+
+
+int 

[libvirt] [PATCH v3 01/14] util: implement virFileReadOffsetQuiet()

[Stefan Berger]

Implement virFileReadOffsetQuiet() that reads a given maximum number
of bytes into a buffer that will be allocated. The reading starts
from a given offset.

Signed-off-by: Stefan Berger 
---
 src/libvirt_private.syms |  1 +
 src/util/virfile.c   | 14 +-
 src/util/virfile.h   |  3 +++
 3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 92b5e0f..f2a4921 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1807,6 +1807,7 @@ virFileReadHeaderFD;
 virFileReadHeaderQuiet;
 virFileReadLimFD;
 virFileReadLink;
+virFileReadOffsetQuiet;
 virFileReadValueBitmap;
 virFileReadValueInt;
 virFileReadValueScaledInt;
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 40f106d..526b9ad 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -1432,12 +1432,18 @@ virFileReadAll(const char *path, int maxlen, char **buf)
 }
 
 int
-virFileReadAllQuiet(const char *path, int maxlen, char **buf)
+virFileReadOffsetQuiet(const char *path, off_t offset,
+   int maxlen, char **buf)
 {
 int fd = open(path, O_RDONLY);
 if (fd < 0)
 return -errno;
 
+if (offset > 0 && lseek(fd, offset, SEEK_SET) < 0) {
+VIR_FORCE_CLOSE(fd);
+return -errno;
+}
+
 int len = virFileReadLimFD(fd, maxlen, buf);
 VIR_FORCE_CLOSE(fd);
 if (len < 0)
@@ -1446,6 +1452,12 @@ virFileReadAllQuiet(const char *path, int maxlen, char 
**buf)
 return len;
 }
 
+int
+virFileReadAllQuiet(const char *path, int maxlen, char **buf)
+{
+return virFileReadOffsetQuiet(path, 0, maxlen, buf);
+}
+
 /* Read @file into preallocated buffer @buf of size @len.
  * Return value is -errno in case of errors and size
  * of data read (no trailing zero) in case of success.
diff --git a/src/util/virfile.h b/src/util/virfile.h
index 341320b..13d3cf6 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -137,6 +137,9 @@ int virFileReadLimFD(int fd, int maxlen, char **buf)
 ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(3);
 int virFileReadAll(const char *path, int maxlen, char **buf)
 ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3);
+int virFileReadOffsetQuiet(const char *path, off_t offset,
+   int maxlen, char **buf)
+ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4);
 int virFileReadAllQuiet(const char *path, int maxlen, char **buf)
 ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3);
 int virFileReadBufQuiet(const char *file, char *buf, int len)
-- 
2.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 04/14] qemu: Extend QEMU capabilities with 'tpm-emulator'

[Stefan Berger]

Extend the QEMU capabilities with tpm-emulator support.

Signed-off-by: Stefan Berger 
---
 src/qemu/qemu_capabilities.c   | 5 +
 src/qemu/qemu_capabilities.h   | 1 +
 tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml   | 1 +
 tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 1 +
 tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml   | 1 +
 tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml   | 1 +
 tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml  | 1 +
 7 files changed, 11 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 7b2e863..3f5368d 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -475,6 +475,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   "disk-write-cache",
   "nbd-tls",
   "tpm-crb",
+  "tpm-emulator",
 );
 
 
@@ -2338,6 +2339,10 @@ static const struct tpmTypeToCaps 
virQEMUCapsTPMTypesToCaps[] = {
 .type = VIR_DOMAIN_TPM_TYPE_PASSTHROUGH,
 .caps = QEMU_CAPS_DEVICE_TPM_PASSTHROUGH,
 },
+{
+.type = VIR_DOMAIN_TPM_TYPE_EMULATOR,
+.caps = QEMU_CAPS_DEVICE_TPM_EMULATOR,
+},
 };
 
 const struct tpmTypeToCaps virQEMUCapsTPMModelsToCaps[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 8da18a8..945696a 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -459,6 +459,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 QEMU_CAPS_DISK_WRITE_CACHE, /* qemu block frontends support write-cache 
param */
 QEMU_CAPS_NBD_TLS, /* NBD server supports TLS transport */
 QEMU_CAPS_DEVICE_TPM_CRB, /* -device tpm-crb */
+QEMU_CAPS_DEVICE_TPM_EMULATOR, /* -tpmdev emulator */
 
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml 
b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
index 64bd554..fd981f4 100644
--- a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml
@@ -119,6 +119,7 @@
   
   
   
+  
   2011000
   0
   342058
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml 
b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml
index 197060a..6349d36 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml
@@ -158,6 +158,7 @@
   
   
   
+  
   2011090
   0
   342346
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml 
b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
index b0eb055..743a1aa 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
@@ -155,6 +155,7 @@
   
   
   
+  
   2011090
   0
   419215
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml 
b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
index 80f3ec6..bc98d6e 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
@@ -120,6 +120,7 @@
   
   
   
+  
   2011090
   0
   0
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml
index 7c346e5..edd7173 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml
@@ -197,6 +197,7 @@
   
   
   
+  
   2011090
   0
   390060
-- 
2.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] rpc: fixing compilation error due to deprecated ssh_get_publickey().

[Jiri Denemark]

On Tue, May 01, 2018 at 13:21:15 -0300, Julio Faracco wrote:
> After 0.7.5 release, libssh deprecated ssh_get_publickey() method to
> introduce ssh_get_server_publickey(). This commit check the current
> version of libssh and use the proper method during the compilation.
> See the error:
> 
> make[3]: Entering directory '/home/julio/Desktop/virt/libvirt/src'
>   CC   rpc/libvirt_net_rpc_la-virnetlibsshsession.lo
> rpc/virnetlibsshsession.c:217:9: error: 'ssh_get_publickey' is deprecated 
> [-Werror,-Wdeprecated-declarations]
> if (ssh_get_publickey(sess->session, ) != SSH_OK) {
> ^
> /usr/include/libssh/libssh.h:489:1: note: 'ssh_get_publickey' has been 
> explicitly marked deprecated here
> SSH_DEPRECATED LIBSSH_API int ssh_get_publickey(ssh_session session, ssh_key 
> *key);
> ^
> /usr/include/libssh/libssh.h:99:40: note: expanded from macro 'SSH_DEPRECATED'
>^
> 1 error generated.
> Makefile:8604: recipe for target 
> 'rpc/libvirt_net_rpc_la-virnetlibsshsession.lo' failed
> 
> Signed-off-by: Julio Faracco 
> ---
>  src/rpc/virnetlibsshsession.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/src/rpc/virnetlibsshsession.c b/src/rpc/virnetlibsshsession.c
> index 309e8a9340..96c5bc0882 100644
> --- a/src/rpc/virnetlibsshsession.c
> +++ b/src/rpc/virnetlibsshsession.c
> @@ -214,7 +214,11 @@ virLibsshServerKeyAsString(virNetLibsshSessionPtr sess)
>  size_t keyhashlen;
>  char *str;
>  
> +#if LIBSSH_VERSION_INT > 0x0705 /* 0.7.5 */
> +if (ssh_get_server_publickey(sess->session, ) != SSH_OK) {
> +#else
>  if (ssh_get_publickey(sess->session, ) != SSH_OK) {
> +#endif
>  virReportError(VIR_ERR_LIBSSH, "%s",
> _("failed to get the key of the current "
>   "session"));

How about making it easier to read and harder to mess up:

#if LIBSSH_VERSION_INT > 0x0705 /* 0.7.5 */
rc = ssh_get_server_publickey(sess->session, );
#else
rc = ssh_get_publickey(sess->session, );
#endif

if (rc != SSH_OK) {
...
}

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] ZFS backend does fail if used on non top level pools

[John Ferlan]

On 05/02/2018 03:36 AM, Christian Ehrhardt wrote:
> Hi,
> by discussing on bug [1] we found an issue in the ZFS storage backend.
> When one defines a zfs pool through XML (virsh pool-create --build) a
> top level ZFS pool will be created and works fine. The virsh
> pool-define-as counterpart to this is:
> 
> $ fallocate -l 100M /tmp/Mzfs
> $ sudo zpool create Mzfs /tmp/Mzfs
> $ virsh pool-define-as --name zfs --source-name Mzfs --type zfs
> $ virsh pool-start zfs
> $ virsh vol-create-as --pool zfs --name vol1 --capacity 10M
> $ virsh vol-list --pool zfs
>  vol1 /dev/zvol/Mzfs/vol1
> $ virsh pool-refresh zfs
>  Pool zfs refreshed
> $ virsh vol-list --pool zfs
>  vol1 /dev/zvol/Mzfs/vol1
> 
> Ok, all fine so far.
> But if one wants to use a zfs-FS as sub-pool things work a while but
> fail as soon as pool info is refreshed.
> 
> $ fallocate -l 100M /tmp/Xzfs
> $ sudo zpool create Xzfs /tmp/Xzfs

If one digs into the virStorageBackendZFSBuildPool they will see libvirt
pool create/build processing would "zpool create $name $path[0...n]"
where $name is the "source.name" (in your case Xzfs/images) and
$path[0...n] would be the various paths (in your case tmp/Xzfs).

So the source.name is probably wrong and should be def->name instead.

When a volume is created in virStorageBackendZFSCreateVol the
source.name and volume name are concatenated - so that seems right.

> $ sudo zfs create Xzfs/images

This one is the "unknown" for me.  What happens if you create
Xzfs/images/vol1 (or your command below) without first creating Xzfs/images?

What probably needs to happen, is the zfs backend needs to have a pool
create added which does the zpool command and then the existing pool
build command would check if def->name != def->source.name and then do
the zfs create @def->source.name...  Of course that also assumes that
the def->name is STRPREFIX os the def->source.name. e.g., in this
example the pool def->name is "Xzfs" and that's what starts the
source.name of "Xzfs/image".



> $ virsh pool-define-as --name Xzfs --source-name Xzfs/images --type zfs
> I ended with the pool not being started (expected after pool-define-as),
> but then
> $ virsh pool-start Xzfs
> $ virsh vol-create-as --pool Xzfs --name vol1 --capacity 10M
> $ virsh vol-list --pool Xzfs
>   vol1 /dev/zvol/Xzfs/images/vol1
> $ virsh pool-refresh zfs
>    Pool zfs refreshed
> $ virsh vol-list --pool Xzfs
>   # no output anymore
> 
> We happened to find this in the libvirt log:
>  error : virCommandWait:2601 : internal error: Child process
> (/sbin/zpool get -Hp health,size,free,allocated Xzfs/images) unexpected
> exit status 1: cannot open 'Xzfs/images': invalid character '/' in pool name
> 
> So it seems the data refresh would need to become aware of filesytems
> and strip this to the basename before doing the call.
> This comes from src/storage/storage_backend_zfs.c
> 
> cmd = virCommandNewArgList(ZPOOL,
>    "get", "-Hp",
>    "health,size,free,allocated",
>    def->source.name ,

I think instead of def->source.name here, def->name should be used.

I would think def->source.name would *only* be used in conjunction with
Storage Pool "Volume" API's while def->name would be used for anything
in Storage Pool as a whole API's.

>    NULL);
> 
> # fails
> zfs list -Hp -t volume -r -o name,volsize,refreservation Xzfs/images
> # would work
> zfs list -Hp -t volume -r -o name,volsize,refreservation Xzfs
> 
> But there might be more to it than what I see, either more changes
> needed to fully work with subpools or intentionally not working with them.
> For the latter we should still consider improving the error handling,
> like refusing right away with a reasonable message on "virsh
> pool-define-as".
> 
> I wondered if one with more experience in that area of libvirt would be
> willing to pick this up or has an opinion why it would not need to be
> changed that I might be missing?
> 

I don't have a zfs pool to play with, but if someone proposes patches
I'll look. I think there's a few hints above. Looks like Roman
Bogorodskiy was the original author and Richard Laager has also made
other improvements.

John

> [1]: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1767997
> 
> 
> --
> Christian Ehrhardt
> Software Engineer, Ubuntu Server
> Canonical Ltd
> 
> 
> --
> 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] Expose vfio device display/migration to libvirt and above, was Re: [PATCH 0/3] sample: vfio mdev display devices.

[Alex Williamson]

On Fri, 4 May 2018 10:16:09 +0100
Daniel P. Berrangé  wrote:

> On Thu, May 03, 2018 at 12:58:00PM -0600, Alex Williamson wrote:
> > Hi,
> > 
> > The previous discussion hasn't produced results, so let's start over.
> > Here's the situation:
> > 
> >  - We currently have kernel and QEMU support for the QEMU vfio-pci
> >display option.
> > 
> >  - The default for this option is 'auto', so the device will attempt to
> >generate a display if the underlying device supports it, currently
> >only GVTg and some future release of NVIDIA vGPU (plus Gerd's
> >sample mdpy and mbochs).
> > 
> >  - The display option is implemented via two different mechanism, a
> >vfio region (NVIDIA, mdpy) or a dma-buf (GVTg, mbochs).
> > 
> >  - Displays using dma-buf require OpenGL support, displays making
> >use of region support do not.
> > 
> >  - Enabling OpenGL support requires specific VM configurations, which
> >libvirt /may/ want to facilitate.
> > 
> >  - Probing display support for a given device is complicated by the
> >fact that GVTg and NVIDIA both impose requirements on the process
> >opening the device file descriptor through the vfio API:
> > 
> >- GVTg requires a KVM association or will fail to allow the device
> >  to be opened.
> > 
> >- NVIDIA requires that their vgpu-manager process can locate a UUID
> >  for the VM via the process commandline.
> > 
> >- These are both horrible impositions and prevent libvirt from
> >  simply probing the device itself.  
> 
> Agreed, these requirements are just horrific. Probing for features
> should not require this kind of level environmental setup. I can
> just about understand & accept how we ended up here, because this
> scenario is not one that was strongly considered when the first impls
> were being done. I don't think we should accept it as a long term
> requirement though.
> 
> > Erik Skultety, who initially raised the display question, has identified
> > one possible solution, which is to simply make the display configuration
> > the user's problem (apologies if I've misinterpreted Erik).  I believe
> > this would work something like:
> > 
> >  - libvirt identifies a version of QEMU that includes 'display' support
> >for vfio-pci devices and defaults to adding display=off for every
> >vfio-pci device [have we chosen the wrong default (auto) in QEMU?].
> > 
> >  - New XML support would allow a user to enable display support on the
> >vfio device.
> > 
> >  - Resolving any OpenGL dependencies of that change would be left to
> >the user.
> > 
> > A nice aspect of this is that policy decisions are left to the user and
> > clearly no interface changes are necessary, perhaps with the exception
> > of deciding whether we've made the wrong default choice for vfio-pci
> > devices in QEMU.  
> 
> Unless I'm mis-understanding this isn't really a solution to the
> problem, rather it is us simply giving up and telling someone else
> to try to fix the problem. The 'user' here is not a human - it is
> simply the next level up in the mgmt stack, eg OpenStack or oVirt.
> If we can't solve it acceptably in libvirt code, I don't have much
> hope that OpenStack can solve it in their code, since they have
> even stronger need to automate everything.

But to solve this at any level other than the user suggests there is
one "right" answer to automatically configuring the device.  Is there?
If a device supports a display, does the user necessarily want to
enable it?  If there's a difference between enabling a display for a
local user or a remote user, is there any reasonable expectation that
we can automatically make that determination?

> > On the other hand, if we do want to give libvirt a mechanism to probe
> > the display support for a device, we can make a simplified QEMU
> > instance be the mechanism through which we do that.  For example the
> > script[1] can be provided with either a PCI device or sysfs path to an
> > mdev device and run a minimal VM instance meeting the requirements of
> > both GVTg and NVIDIA to report the display support and GL requirements
> > for a device.  There are clearly some unrefined and atrocious bits of
> > this script, but it's only a proof of concept, the process management
> > can be improved and we can decide whether we want to provide qmp
> > mechanism to introspect the device rather than grep'ing error
> > messages.  The goal is simply to show that we could choose to embrace
> > QEMU and use it not as a VM, but simply a tool for poking at a device
> > given the restrictions the mdev vendor drivers have already imposed.  
> 
> Feels like a pretty heavy weight solution, that just encourages the
> drivers to continue down the undesirable path they're already on,
> possibly making the situation even worse over time.

I'm not getting the impression that the vendor drivers are considering
a change, or necessarily can change.  The NVIDIA UUID requirement

Re: [libvirt] [PATCH] rpc: fixing compilation error due to deprecated ssh_get_publickey().

[Daniel P . Berrangé]

On Fri, May 04, 2018 at 12:53:14PM -0400, John Ferlan wrote:
> 
> 
> On 05/01/2018 12:21 PM, Julio Faracco wrote:
> > After 0.7.5 release, libssh deprecated ssh_get_publickey() method to
> > introduce ssh_get_server_publickey(). This commit check the current
> > version of libssh and use the proper method during the compilation.
> > See the error:
> > 
> > make[3]: Entering directory '/home/julio/Desktop/virt/libvirt/src'
> >   CC   rpc/libvirt_net_rpc_la-virnetlibsshsession.lo
> > rpc/virnetlibsshsession.c:217:9: error: 'ssh_get_publickey' is deprecated 
> > [-Werror,-Wdeprecated-declarations]
> > if (ssh_get_publickey(sess->session, ) != SSH_OK) {
> > ^
> > /usr/include/libssh/libssh.h:489:1: note: 'ssh_get_publickey' has been 
> > explicitly marked deprecated here
> > SSH_DEPRECATED LIBSSH_API int ssh_get_publickey(ssh_session session, 
> > ssh_key *key);
> > ^
> > /usr/include/libssh/libssh.h:99:40: note: expanded from macro 
> > 'SSH_DEPRECATED'
> >^
> > 1 error generated.
> > Makefile:8604: recipe for target 
> > 'rpc/libvirt_net_rpc_la-virnetlibsshsession.lo' failed
> > 
> > Signed-off-by: Julio Faracco 
> > ---
> >  src/rpc/virnetlibsshsession.c | 4 
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/src/rpc/virnetlibsshsession.c b/src/rpc/virnetlibsshsession.c
> > index 309e8a9340..96c5bc0882 100644
> > --- a/src/rpc/virnetlibsshsession.c
> > +++ b/src/rpc/virnetlibsshsession.c
> > @@ -214,7 +214,11 @@ virLibsshServerKeyAsString(virNetLibsshSessionPtr sess)
> >  size_t keyhashlen;
> >  char *str;
> >  
> > +#if LIBSSH_VERSION_INT > 0x0705 /* 0.7.5 */
> > +if (ssh_get_server_publickey(sess->session, ) != SSH_OK) {
> > +#else
> >  if (ssh_get_publickey(sess->session, ) != SSH_OK) {
> > +#endif
> 
> Usually this involves changes to some m4/* file that would do the
> version check and existence of some API and set a WITH_xxx or HAVE_xxx
> type conditional which would then be used.

That all depends on whether we care about handling the possibility of
distros backporting the function to older versions of libssh. This
kind of backporting happens alot for some projects, like QEMU, but
not for others. IMHO this check against LIBSSH_VERSION_INT is fine.

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] rpc: fixing compilation error due to deprecated ssh_get_publickey().

[John Ferlan]

On 05/01/2018 12:21 PM, Julio Faracco wrote:
> After 0.7.5 release, libssh deprecated ssh_get_publickey() method to
> introduce ssh_get_server_publickey(). This commit check the current
> version of libssh and use the proper method during the compilation.
> See the error:
> 
> make[3]: Entering directory '/home/julio/Desktop/virt/libvirt/src'
>   CC   rpc/libvirt_net_rpc_la-virnetlibsshsession.lo
> rpc/virnetlibsshsession.c:217:9: error: 'ssh_get_publickey' is deprecated 
> [-Werror,-Wdeprecated-declarations]
> if (ssh_get_publickey(sess->session, ) != SSH_OK) {
> ^
> /usr/include/libssh/libssh.h:489:1: note: 'ssh_get_publickey' has been 
> explicitly marked deprecated here
> SSH_DEPRECATED LIBSSH_API int ssh_get_publickey(ssh_session session, ssh_key 
> *key);
> ^
> /usr/include/libssh/libssh.h:99:40: note: expanded from macro 'SSH_DEPRECATED'
>^
> 1 error generated.
> Makefile:8604: recipe for target 
> 'rpc/libvirt_net_rpc_la-virnetlibsshsession.lo' failed
> 
> Signed-off-by: Julio Faracco 
> ---
>  src/rpc/virnetlibsshsession.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/src/rpc/virnetlibsshsession.c b/src/rpc/virnetlibsshsession.c
> index 309e8a9340..96c5bc0882 100644
> --- a/src/rpc/virnetlibsshsession.c
> +++ b/src/rpc/virnetlibsshsession.c
> @@ -214,7 +214,11 @@ virLibsshServerKeyAsString(virNetLibsshSessionPtr sess)
>  size_t keyhashlen;
>  char *str;
>  
> +#if LIBSSH_VERSION_INT > 0x0705 /* 0.7.5 */
> +if (ssh_get_server_publickey(sess->session, ) != SSH_OK) {
> +#else
>  if (ssh_get_publickey(sess->session, ) != SSH_OK) {
> +#endif

Usually this involves changes to some m4/* file that would do the
version check and existence of some API and set a WITH_xxx or HAVE_xxx
type conditional which would then be used.

I typically try to avoid m4/* files and build stuff, so not my area of
expertise, but m4/virt-libssh.m4 is perhaps where you should start.
Perhaps usage of AC_CHECK_FUNCS

Look up HAVE_GNUTLS_CIPHER_ENCRYPT and how m4/virt-gnutls.m4 will check
for gnutls_cipher_encrypt for at least one example that comes to my mind.

John

>  virReportError(VIR_ERR_LIBSSH, "%s",
> _("failed to get the key of the current "
>   "session"));
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 5/5] Deprecate QEMU_CAPS_NESTING

[John Ferlan]

On 05/03/2018 06:35 AM, Ján Tomko wrote:
> Unused since commit .
> 
> Signed-off-by: Ján Tomko 
> ---
>  src/qemu/qemu_capabilities.h | 2 +-
>  src/qemu/qemu_command.c  | 3 ---
>  2 files changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index 0cf7f12230..07dd9ff4ec 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -105,7 +105,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
> syntax-check */
>  
>  /* 40 */
>  X_QEMU_CAPS_FSDEV, /* -fstype filesystem passthrough */
> -QEMU_CAPS_NESTING, /* -enable-nesting (SVM/VMX) */
> +X_QEMU_CAPS_NESTING, /* -enable-nesting (SVM/VMX) */
>  X_QEMU_CAPS_NAME_PROCESS, /* Is -name process= available */
>  X_QEMU_CAPS_DRIVE_READONLY, /* -drive readonly=on|off */
>  X_QEMU_CAPS_SMBIOS_TYPE, /* Is -smbios type= available */
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 099265da17..e8f5f97fa1 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -6870,9 +6870,6 @@ qemuBuildCpuCommandLine(virCommandPtr cmd,
>  if (cpu) {
>  virCommandAddArg(cmd, "-cpu");
>  virCommandAddArgFormat(cmd, "%s%s", cpu, cpu_flags ? cpu_flags : "");
> -
> -if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NESTING) && hasHwVirt)

Did you clip too much here?   hasHWVirt is defined and set, but not used
according to my compiler

John
> -virCommandAddArg(cmd, "-enable-nesting");
>  }
>  
>  ret = 0;
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 4/5] Depreacte QEMU_CAPS_TDF

[John Ferlan]

$subj:  Deprecate



On 05/03/2018 06:35 AM, Ján Tomko wrote:
> This capability is unused since we stopped parsing -help output.
> 
> Signed-off-by: Ján Tomko 
> ---
>  src/qemu/qemu_capabilities.h | 2 +-
>  src/qemu/qemu_command.c  | 5 +
>  2 files changed, 2 insertions(+), 5 deletions(-)
> 

[...]

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 0/5] Another batch of old capability cleanups

[John Ferlan]

On 05/03/2018 06:35 AM, Ján Tomko wrote:
> Ján Tomko (5):
>   qemu: remove qemuBuildObsoleteAccelArg
>   qemuBuildMachineCommandLine: use a switch for virDomainVirtType
>   Deprecate QEMU_CAPS_NO_KVM_PIT
>   Depreacte QEMU_CAPS_TDF
>   Deprecate QEMU_CAPS_NESTING
> 
>  src/qemu/qemu_capabilities.c   |   6 +-
>  src/qemu/qemu_capabilities.h   |   6 +-
>  src/qemu/qemu_command.c| 101 
> +++--
>  tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml   |   1 -
>  tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml   |   1 -
>  tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml   |   1 -
>  tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml   |   1 -
>  tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml  |   1 -
>  tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml  |   1 -
>  tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml   |   1 -
>  tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml   |   1 -
>  tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml   |   1 -
>  tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml   |   1 -
>  tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml   |   1 -
>  tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml   |   1 -
>  tests/qemuxml2argvdata/clock-hpet-off.args |   1 -
>  tests/qemuxml2argvdata/hugepages-numa.args |   1 -
>  tests/qemuxml2argvdata/no-kvm-pit-device.args  |  28 --
>  tests/qemuxml2argvdata/no-kvm-pit-device.xml   |  29 --
>  tests/qemuxml2argvdata/q35-virt-manager-basic.args |   1 -
>  tests/qemuxml2argvtest.c   |   3 +-
>  tests/qemuxml2xmltest.c|   1 -
>  22 files changed, 40 insertions(+), 149 deletions(-)
>  delete mode 100644 tests/qemuxml2argvdata/no-kvm-pit-device.args
>  delete mode 100644 tests/qemuxml2argvdata/no-kvm-pit-device.xml
> 

After fixing the 2 issues noted in patch 4 and 5,

(series)

Reviewed-by: John Ferlan 

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Expose vfio device display/migration to libvirt and above, was Re: [PATCH 0/3] sample: vfio mdev display devices.

[Alex Williamson]

On Fri, 4 May 2018 09:49:44 +0200
Erik Skultety  wrote:

> On Thu, May 03, 2018 at 12:58:00PM -0600, Alex Williamson wrote:
> > Hi,
> >
> > The previous discussion hasn't produced results, so let's start over.
> > Here's the situation:
> >
> >  - We currently have kernel and QEMU support for the QEMU vfio-pci
> >display option.
> >
> >  - The default for this option is 'auto', so the device will attempt to
> >generate a display if the underlying device supports it, currently
> >only GVTg and some future release of NVIDIA vGPU (plus Gerd's
> >sample mdpy and mbochs).
> >
> >  - The display option is implemented via two different mechanism, a
> >vfio region (NVIDIA, mdpy) or a dma-buf (GVTg, mbochs).
> >
> >  - Displays using dma-buf require OpenGL support, displays making
> >use of region support do not.
> >
> >  - Enabling OpenGL support requires specific VM configurations, which
> >libvirt /may/ want to facilitate.
> >
> >  - Probing display support for a given device is complicated by the
> >fact that GVTg and NVIDIA both impose requirements on the process
> >opening the device file descriptor through the vfio API:
> >
> >- GVTg requires a KVM association or will fail to allow the device
> >  to be opened.  
> 
> How exactly is this association checked?

The intel_vgpu_open() callback for the mdev device registers a vfio
group notifier for VFIO_GROUP_NOTIFY_SET_KVM events. The KVM pointer is
already registered via the addition of the vfio group to the vfio-kvm
pseudo device, so the registration synchronously triggers the notifier
callback and the result is tested slightly later in the open path in
kvmgt_guest_init().
 
> >
> >- NVIDIA requires that their vgpu-manager process can locate a
> > UUID for the VM via the process commandline.
> >
> >- These are both horrible impositions and prevent libvirt from
> >  simply probing the device itself.  
> 
> So I feel like we're trying to solve a problem coming from one layer
> on a bunch of different layers which inherently prevents us to
> produce a viable long term solution without dragging a significant
> amount of hacky nasty code and it is not the missing sysfs attributes
> I have in mind. Why does NVIDIA's vgpu-manager need to locate a UUID
> of a qemu VM? I assume that's to prevent multiple VM instances trying
> to use the same mdev device, in which case can't the vgpu-manager
> track references to how many "open" and "close" calls have been made

Hard to say, NVIDIA hasn't been terribly forthcoming about this
requirement, but probably not multiple users of the same mdev device
as that's already prevented through vfio in general.  Intel has
discussed that their requirement is to be able to track VM page table
updates so they can update their shadow tables, so effectively rather
than mediating interactions directly with the device, they're using a
KVM back channel to manage the DMA translation address space for the
device.

The flip side is that while these requirements are annoying and hard
for non-VM users to deal with, is there a next logical point in the
interaction with the vfio device where the vendor driver can reasonably
impose those requirements?  For instance, both vendors expose a
vfio-pci interface, so they could prevent the user driver from enabling
bus master in the PCI command register, but that's a fairly subtle
failure, typically drivers wouldn't even bother to read back after a
write to the bus master bit to see if it sticks and this sort of
enabling is done by the guest, not the hypervisor.  There's really no
error path for a write to the device.

> to the same device? This is just from a layman's perspective, but it
> would allow the following:
> - when libvirt starts, it initializes all its drivers (let's
> focus on QEMU)
> - as part of this initialization, libvirt probes QEMU for
> capabilities and caches them in order to use them when spawning VMs
> 
> Now, if we (theoretically) can settle on easing the restrictions Alex
> has mentioned, we in fact could introduce a QMP command to probe
> these devices and provide libvirt with useful information at that
> point in time. Of course, since the 3rd party vendor is "de-coupled"
> from qemu, libvirt would have no way to find out that the driver has
> changed in the meantime, thus still using the old information we
> gathered, ergo potentially causing the QEMU process to fail
> eventually. But then again, there's very often a strong
> recommendation to reboot your host after a driver update, especially
> in NVIDIA's case, which means this fact wouldn't matter. However,
> there's also a significant drawback to my proposal which probably
> renders it completely useless (but we can continue from there...) and
> that is the devices would either have to be present already (not an
> option) or QEMU would need to be enhanced in a way, that it would
> create a dummy device during QMP probing, open it, collect the

Re: [libvirt] [PATCH] lxc: convert to typesafe virConf accessors in lxc_native.c

[Cedric Bosdonnat]

On Fri, 2018-05-04 at 18:46 +0530, Prafull wrote:
> From: Prafullkumar Tale 
> 
> Signed-off-by: Prafullkumar Tale 
> ---
>  src/lxc/lxc_native.c | 141 
> +--
>  1 file changed, 70 insertions(+), 71 deletions(-)
> 
> diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c
> index 55ea774..35077e1 100644
> --- a/src/lxc/lxc_native.c
> +++ b/src/lxc/lxc_native.c
> @@ -199,19 +199,15 @@ lxcSetRootfs(virDomainDefPtr def,
>   virConfPtr properties)
>  {
>  int type = VIR_DOMAIN_FS_TYPE_MOUNT;
> -virConfValuePtr value;
> +char *value = NULL;
>  
> -if (!(value = virConfGetValue(properties, "lxc.rootfs")) ||
> -!value->str) {
> -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -   _("Missing lxc.rootfs configuration"));
> +if (virConfGetValueString(properties, "lxc.rootfs", ) <= 0)
>  return -1;
> -}
>  
> -if (STRPREFIX(value->str, "/dev/"))
> +if (STRPREFIX(value, "/dev/"))
>  type = VIR_DOMAIN_FS_TYPE_BLOCK;
>  
> -if (lxcAddFSDef(def, type, value->str, "/", false, 0) < 0)
> +if (lxcAddFSDef(def, type, value, "/", false, 0) < 0)
>  return -1;
>  
>  return 0;
> @@ -684,17 +680,17 @@ lxcConvertNetworkSettings(virDomainDefPtr def, 
> virConfPtr properties)
>  static int
>  lxcCreateConsoles(virDomainDefPtr def, virConfPtr properties)
>  {
> -virConfValuePtr value;
> +char *value = NULL;
>  int nbttys = 0;
>  virDomainChrDefPtr console;
>  size_t i;
>  
> -if (!(value = virConfGetValue(properties, "lxc.tty")) || !value->str)
> +if (virConfGetValueString(properties, "lxc.tty", ) <= 0)
>  return 0;
>  
> -if (virStrToLong_i(value->str, NULL, 10, ) < 0) {
> +if (virStrToLong_i(value, NULL, 10, ) < 0) {
>  virReportError(VIR_ERR_INTERNAL_ERROR, _("failed to parse int: 
> '%s'"),
> -   value->str);
> +   value);
>  return -1;
>  }
>  
> @@ -761,89 +757,91 @@ lxcIdmapWalkCallback(const char *name, virConfValuePtr 
> value, void *data)
>  static int
>  lxcSetMemTune(virDomainDefPtr def, virConfPtr properties)
>  {
> -virConfValuePtr value;
> +char *value = NULL;
>  unsigned long long size = 0;
>  
> -if ((value = virConfGetValue(properties,
> -"lxc.cgroup.memory.limit_in_bytes")) &&
> -value->str && STRNEQ(value->str, "-1")) {
> -if (lxcConvertSize(value->str, ) < 0)
> -return -1;
> +if (virConfGetValueString(properties,
> +  "lxc.cgroup.memory.limit_in_bytes",
> +  ) > 0) {
> +if (lxcConvertSize(value, ) < 0)
> +goto error;
>  size = size / 1024;
>  virDomainDefSetMemoryTotal(def, size);
>  def->mem.hard_limit = virMemoryLimitTruncate(size);
>  }
>  
> -if ((value = virConfGetValue(properties,
> -"lxc.cgroup.memory.soft_limit_in_bytes")) &&
> -value->str && STRNEQ(value->str, "-1")) {
> -if (lxcConvertSize(value->str, ) < 0)
> -return -1;
> -
> +if (virConfGetValueString(properties,
> +  "lxc.cgroup.memory.soft_limit_in_bytes",
> +  ) > 0) {
> +if (lxcConvertSize(value, ) < 0)
> +goto error;
>  def->mem.soft_limit = virMemoryLimitTruncate(size / 1024);
>  }
>  
> -if ((value = virConfGetValue(properties,
> -"lxc.cgroup.memory.memsw.limit_in_bytes")) &&
> -value->str && STRNEQ(value->str, "-1")) {
> -if (lxcConvertSize(value->str, ) < 0)
> -return -1;
> -
> +if (virConfGetValueString(properties,
> +  "lxc.cgroup.memory.memsw.limit_in_bytes",
> +  ) > 0) {
> +if (lxcConvertSize(value, ) < 0)
> +goto error;
>  def->mem.swap_hard_limit = virMemoryLimitTruncate(size / 1024);
>  }
>  return 0;
> +
> + error:
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   _("failed to parse integer: '%s'"), value);
> +return -1;
> +
>  }
>  
>  static int
>  lxcSetCpuTune(virDomainDefPtr def, virConfPtr properties)
>  {
> -virConfValuePtr value;
> +char *value = NULL;
>  
> -if ((value = virConfGetValue(properties, "lxc.cgroup.cpu.shares")) &&
> -value->str) {
> -if (virStrToLong_ull(value->str, NULL, 10, >cputune.shares) < 0)
> +if (virConfGetValueString(properties, "lxc.cgroup.cpu.shares",
> +  ) > 0) {
> +if (virStrToLong_ull(value, NULL, 10, >cputune.shares) < 0)
>  goto error;
>  def->cputune.sharesSpecified = true;
>  }
>  
> -if ((value = virConfGetValue(properties,
> - "lxc.cgroup.cpu.cfs_quota_us")) &&
> -

Re: [libvirt] [PATCH 0/8] vfio-ccw passthrough support

[John Ferlan]

On 04/26/2018 03:59 AM, Bjoern Walk wrote:
> Shalini Chellathurai Saroja  [2018-04-11, 05:49PM 
> +0200]:
>> Let us support the basic channel I/O passthrough infrastructure based on
>> vfio, which have been introduced in QEMU 2.10. The current focus is to
>> support dasd-eckd (cu_type/dev_type = 0x3990/0x3390) as the target
>> device.
>>
>> Shalini Chellathurai Saroja (8):
>>   qemu: introduce capability for virtual-css-bridge
>>   qemu: introduce vfio-ccw capability
>>   util: virhostdev: add virHostdevIsMdevDevice()
>>   qemu: vfio-ccw device address generation
>>   qemu: command line generation for vfio-ccw device
>>   tests: tests for vfio-ccw passthrough
>>   docs: documentation for vfio-ccw passthrough
>>   news: documentation of new feature
> 
> Any chance, we get at least a review before 4.3 hits? Would be
> appreciated.
> 
> 

So obviously this did not make the 4.3.0 and the series will need a
refresh due to the volume of change in qemu_capabilities.{c,h}.

I've reviewed a number of patches recently and made a similar comment in
all of them - when changing qemu_capabilities.{c,h} and updating the
various qemucapabilitiesdata/caps_*.xml files - do so in a separate
patch. That way if someone doesn't review the code right away, it's
actually fairly simple to recreate at least the capability for a
reviewer. Having it mixed in one patch with other qemu, conf, test, etc.
changes causes git am -3 to fail and thus makes review harder especially
when you don't get to reviews as soon as patches hit the list.

Someone may also want to consider creating a s390 specific version of
what Peter did for x86_64 for VIR_TEST_CAPS_LATEST in order to then
have/use the "latest" capabilities instead of adding bits to xml2argv
tests. I'm curious why the xml2xml test needed the bit adjustment - did
something fail?  Since there were no xml output data changes, that would
seem to indicate there isn't a need to modify the xml2xml test source.

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH REBASE 5/5] qemu: fix races in beingDestroyed usage

[John Ferlan]

On 05/03/2018 05:26 AM, Nikolay Shirokovskiy wrote:
> 
> 
> On 01.05.2018 01:03, John Ferlan wrote:
>>
>>
>> On 04/18/2018 10:44 AM, Nikolay Shirokovskiy wrote:
>>> Clearing beingDestroyed right after acquiring job condition is racy.
>>> It is not known when EOF will be delivired. Let's keep this flag
>>
>> delivered
>>
>>> set. This makes possible to make a clear distinction between monitor
>>> errors/eofs and domain being destroyed in qemuDomainObjWait.
>>>
>>> Also let's move setting destroyed flag out of qemuProcessBeginStopJob
>>> as the function is called from eof handler too.
>>>
>>> Signed-off-by: Nikolay Shirokovskiy 
>>> ---
>>>  src/qemu/qemu_domain.c  |  4 ++--
>>>  src/qemu/qemu_domain.h  |  2 +-
>>>  src/qemu/qemu_driver.c  |  8 +++-
>>>  src/qemu/qemu_process.c | 24 
>>>  4 files changed, 22 insertions(+), 16 deletions(-)
>>>
>>
>> This one didn't git am -3 apply as well, but I see you're changing
>> DomainObjWait so that makes sense as to why.
>>
>> I do recall looking at this code at one time, but I cannot recall my
>> exact conclusion given how qemuDomainObjBeginJobInternal allows the
>> QEMU_JOB_DESTROY to be run, but then waits for whatever is taking place
>> now to finish.
>>
>>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>>> index 1f40ff1..431901c 100644
>>> --- a/src/qemu/qemu_domain.c
>>> +++ b/src/qemu/qemu_domain.c
>>> @@ -11913,9 +11913,9 @@ qemuDomainObjWait(virDomainObjPtr vm, unsigned long 
>>> long until)
>>>  return -1;
>>>  }
>>>  
>>> -if (!virDomainObjIsActive(vm)) {
>>> +if (priv->destroyed) {
>>>  virReportError(VIR_ERR_OPERATION_FAILED, "%s",
>>> -   _("domain is not running"));
>>> +   _("domain is destroyed"));
>>>  return -1;
>>>  }
>>>  
>>> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
>>> index 494ed35..69112ea 100644
>>> --- a/src/qemu/qemu_domain.h
>>> +++ b/src/qemu/qemu_domain.h
>>> @@ -269,7 +269,7 @@ struct _qemuDomainObjPrivate {
>>>  bool agentError;
>>>  
>>>  bool gotShutdown;
>>> -bool beingDestroyed;
>>> +bool destroyed;
>>>  char *pidfile;
>>>  
>>>  virDomainPCIAddressSetPtr pciaddrs;
>>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>>> index 03969d8..4356c0d 100644
>>> --- a/src/qemu/qemu_driver.c
>>> +++ b/src/qemu/qemu_driver.c
>>> @@ -2227,7 +2227,13 @@ qemuDomainDestroyFlags(virDomainPtr dom,
>>>  state = virDomainObjGetState(vm, );
>>>  starting = (state == VIR_DOMAIN_PAUSED &&
>>>  reason == VIR_DOMAIN_PAUSED_STARTING_UP &&
>>> -!priv->beingDestroyed);
>>> +!priv->destroyed);
>>> +
>>> +/* We need to prevent monitor EOF callback from doing our work (and
>>> + * sending misleading events) while the vm is unlocked inside
>>> + * BeginJob/ProcessKill API
>>> + */
>>> +priv->destroyed = true;
>>
>> would this be the right place anyway?  especially since you don't clear
>> it in error paths after setting it.  Once the job starts and we either
>> goto cleanup or endjob on failure - how does a future call distinguish?
> 
> We send SIGTERM/SIGKILL right away after this flag is set so even if
> this API fails eventually keeping destroyed flag set looks good because we
> send signal to qemu and good chances are qemu will die because of that.
> 
> I guess it will be nicer then to move setting the flag to 
> qemuProcessBeginStopJob
> function to keep setting the flag and killing domain together.
> 
> Anyway we can clear the flag on failure too.
> 
>>
>> Not sure this works conceptually.  At least with the current model if
>> DestroyFlags finally gets the job, it checks the domain active state,
>> and goes to endjob after printing a message.  So if while waiting, EOF
>> occurred, there's no qemuProcessStop
> 
> Not true. After sending signal to qemu to terminate EOF will occur but
> handler will return right away because of beingDestroyed/destroyed flag
> check and then after destroyFlags gets the job it will call qemuProcessStop.
> 
> This is not the place I'm addressing with the patch. It is rather if 
> destroyFlags
> is called when we are migrating for example then the interrupted
> API call can report 'domain is not active'/'some monitor error' rather 
> then much nicer 'domain is destroyed' without this patch. See the
> scenario below.
> 
>>
>> Perhaps the only thing I recall wondering is whether we clear
>> beingDestroyed too soon... If we're successful, we're still destroying
>> but not until destroy is successful should the flag then be cleared
>> while still holding the job.
> 
> It does not matter if we clear the flag at the begin or the end of time
> we holding the job because during the job nobody else who needs the job
> too can progress.
> 
> I propose to prolong setting the flag after destroy job is finished (and
> thus rename flag too). Consider next 

Re: [libvirt] [PATCH REBASE 4/5] qemu: fix domain object wait to handle monitor errors

[John Ferlan]

On 05/03/2018 03:54 AM, Nikolay Shirokovskiy wrote:
> 
> 
> On 01.05.2018 01:03, John Ferlan wrote:
>>
>>
>> On 04/18/2018 10:44 AM, Nikolay Shirokovskiy wrote:
>>> Block job abort operation can not handle properly qemu crashes
>>> when waiting for abort/pivot completion. Deadlock scenario is next:
>>>
>>> - qemuDomainBlockJobAbort waits for pivot/abort completion
>>> - qemu crashes, then qemuProcessBeginStopJob broadcasts for VM condition and
>>>   then waits for job condition (taken by qemuDomainBlockJobAbort)
>>> - qemuDomainBlockJobAbort awakes but nothing really changed, VM is still
>>>   active (vm->def->id != -1) so thread starts waiting for completion again.
>>>   Now two threads are in deadlock.
>>>
>>> First let's add some condition besides domain active status so that waiting
>>> thread can check it when awakes. Second let's move signalling to the place
>>> where condition is set, namely monitor eof/error handlers. Having signalling
>>> in qemuProcessBeginStopJob is not useful.
>>>
>>> The patch copies monitor error to domain state because at time
>>> waiting thread awakes there can be no monitor and it is useful to
>>> report monitor error to user.
>>>
>>> The patch has a drawback that on destroying a domain when waiting for
>>> event from monitor we get not very convinient error message like
>>
>> convenient
>>
>>> 'EOF from monitor' from waiting API. On the other hand if qemu crashes
>>> this is more useful then 'domain is not running'. The first case
>>> will be addressed in another patch.
>>>
>>> The patch also fixes other places where we wait for event from qemu.
>>> Namely handling device removal and tray ejection. Other places which
>>> used virDomainObjWait (dump, migration, save) were good because of
>>> async jobs which allow concurrent destroy job.
>>>
>>> Signed-off-by: Nikolay Shirokovskiy 
>>> ---
>>>  src/conf/domain_conf.c| 43 ---
>>>  src/conf/domain_conf.h|  3 ---
>>>  src/libvirt_private.syms  |  2 --
>>>  src/qemu/qemu_domain.c| 45 
>>> +
>>>  src/qemu/qemu_domain.h|  5 -
>>>  src/qemu/qemu_driver.c|  6 +++---
>>>  src/qemu/qemu_hotplug.c   |  4 ++--
>>>  src/qemu/qemu_migration.c | 12 ++--
>>>  src/qemu/qemu_process.c   | 27 ++-
>>>  9 files changed, 82 insertions(+), 65 deletions(-)
>>>
>>
>> This no longer git am -3 applies and based on previous patch reviews, I
>> think perhaps this needs to be redone.
> 
> I'll resend as soon as we come to agreement on series. Now I'm not
> convinced to change much (except for using distinct flag to indicate error
> as mentioned in 2nd patch discussion, then I don't need 3d patch).
> 
> See comments below.
> 
>>
>> I don't believe moving/renaming virDomainObjWait is good/right, but I'm
>> sure there would be other opinions on that.  Yes, QEMU is currently the
>> only consumer... If it were to move, it should move to virdomainobjlist
>> since that's where innards of virDomainObjPtr are managed. The fact that
>> we look at ->parent.lock, well, ...
> 
> I would not move the function without reason. It gets new name 
> qemuDomainObjWait
> becase it use qemu specific data, namely monError.

Today only qemu uses this generic virDomainObjWait which is generic
without the need to have/use qemu specific things. Other domain
consumers could use it if they chose.

I'm not in favor of moving, renaming, repurposing for a very specific
issue for what is a generically used function. Maybe that's just me - so
you could try to get a different reviewer if you want.


> 
>>
>> Anyway, you're attempting to special case something and perhaps you just
>> need to create a qemuDomainObjWait that would call the timeout version
>> of the virDomainObjWait and be able to handle whether some other error
>> occurred.  Wouldn't the LastError before the current wait return NULL
>> (as in no error), then during the timeout period if LastError is
>> something couldn't that indicate the failure you're looking for.
> 
> I introduced qemuDomainObjWait not to put virDomainObjWait and 
> virDomainObjWaitUntil
> in the first place but to check monError. Then rather then keeping too 
> functions
> it's look more simple to use only one and branch on given timeout.
> 

I would think if the goal was to have specific code for a specific
purpose for specific functions, then introduce the qemuDomainObjWait,
but have it call virDomainObjWait[Until] based on the need you have.
Which in this case appears to fiddle with monError in some way.

Again - that's just my opinion

John

>>
>> I didn't spend a lot of time thinking about alternatives and how the
>> code should change, but when you mention the patch has a drawback - that
>> immediately raises concern.
> 
> But ... I addressed this issue in next patch as I wrote.
> 


[...]

--
libvir-list mailing list
libvir-list@redhat.com

Re: [libvirt] [dbus PATCH] Add detail argument to DomainEvent signal

[Pavel Hrdina]

On Fri, May 04, 2018 at 11:45:59AM +0200, Katerina Koukiou wrote:
> Adjust all DomainEvent tests to do detail type checking.
> 
> Signed-off-by: Katerina Koukiou 
> ---
> This commit is rebased on top of unmerged patches for removing enum<->string
> translation.
> 
>  data/org.libvirt.Connect.xml |  1 +
>  src/events.c |  4 ++--
>  tests/libvirttest.py | 55 
> 
>  tests/test_connect.py|  6 +++--
>  tests/test_domain.py | 15 
>  5 files changed, 72 insertions(+), 9 deletions(-)

Reviewed-by: Pavel Hrdina 

Similar thing should be done for Domain.State property.


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [dbus PATCH v2 9/9] Remove reason to string translation in virtDBusEventsDomainDiskChange

[Pavel Hrdina]

On Fri, May 04, 2018 at 10:38:35AM +0200, Katerina Koukiou wrote:
> Signed-off-by: Katerina Koukiou 
> ---
>  data/org.libvirt.Domain.xml |  2 +-
>  src/events.c| 17 ++---
>  2 files changed, 3 insertions(+), 16 deletions(-)

Reviewed-by: Pavel Hrdina 


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [dbus PATCH v2 8/9] Remove state to string translation in virtDBusDomainGetState

[Pavel Hrdina]

On Fri, May 04, 2018 at 10:38:34AM +0200, Katerina Koukiou wrote:
> Adjust tests to comply with the new type.
> 
> Signed-off-by: Katerina Koukiou 
> ---
>  data/org.libvirt.Domain.xml |  2 +-
>  src/domain.c| 31 +--
>  tests/libvirttest.py| 12 
>  tests/test_domain.py| 10 +-
>  4 files changed, 19 insertions(+), 36 deletions(-)

Reviewed-by: Pavel Hrdina 


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [dbus PATCH v2 7/9] Remove reason to string translation in virtDBusEventsDomainTrayChange

[Pavel Hrdina]

On Fri, May 04, 2018 at 10:38:33AM +0200, Katerina Koukiou wrote:
> Signed-off-by: Katerina Koukiou 
> ---
>  data/org.libvirt.Domain.xml |  2 +-
>  src/events.c| 15 +--
>  2 files changed, 2 insertions(+), 15 deletions(-)

Reviewed-by: Pavel Hrdina 


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [dbus PATCH v2 6/9] Remove virtDBusUtilEnum{From, From}String functions

[Pavel Hrdina]

On Fri, May 04, 2018 at 10:38:32AM +0200, Katerina Koukiou wrote:
> Signed-off-by: Katerina Koukiou 
> ---
>  src/util.c | 27 ---
>  src/util.h | 28 
>  2 files changed, 55 deletions(-)

Reviewed-by: Pavel Hrdina 


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [tck PATCH v2 0/3] Remove Test::AutoBuild leftovers

[Andrea Bolognani]

Changes from [v1]:

* rename the script instead of dropping it;

* perform some further clean ups.


[v1] https://www.redhat.com/archives/libvir-list/2018-May/msg00218.html

Andrea Bolognani (3):
  maint: Rename autobuild.sh to prepare-release.sh
  prepare-release: Drop references to Test::AutoBuild
  spec: Drop %{extra_release}

 perl-Sys-Virt-TCK.spec.PL  |  3 +--
 autobuild.sh => prepare-release.sh | 15 ++-
 2 files changed, 3 insertions(+), 15 deletions(-)
 rename autobuild.sh => prepare-release.sh (64%)

-- 
2.17.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [tck PATCH v2 1/3] maint: Rename autobuild.sh to prepare-release.sh

[Andrea Bolognani]

The script was originally used by the Test::AutoBuild
project to perform periodic automatic builds; however, that
effort has been abandoned a long time ago, and these days
libvirt-tck CI builds are happening on the Jenkins-based
CentOS CI environment under the libvirt umbrella[1], where
build recipes are maintained separately from the projects
themselves.

The script is still used to prepare releases, so it can't
be dropped from the repository: rename it so that its
purpose is more clearly communicated instead.

[1] https://ci.centos.org/view/libvirt/

Signed-off-by: Andrea Bolognani 
---
 autobuild.sh => prepare-release.sh | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename autobuild.sh => prepare-release.sh (100%)

diff --git a/autobuild.sh b/prepare-release.sh
similarity index 100%
rename from autobuild.sh
rename to prepare-release.sh
-- 
2.17.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [dbus PATCH v2 5/9] Change NetworkEvent argument from string to unsigned int

[Pavel Hrdina]

On Fri, May 04, 2018 at 10:38:31AM +0200, Katerina Koukiou wrote:
> Modify the relevant tests to comply with the new signal.
> 
> Note: argument matching in connect_to_signal method is
> only usable with string and thus had to be refactored.
> 
> Signed-off-by: Katerina Koukiou 
> ---
>  data/org.libvirt.Connect.xml |  2 +-
>  src/events.c | 14 +-
>  tests/libvirttest.py |  7 +++
>  tests/test_connect.py| 12 
>  tests/test_network.py| 18 --
>  5 files changed, 29 insertions(+), 24 deletions(-)

Reviewed-by: Pavel Hrdina 


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [tck PATCH v2 3/3] spec: Drop %{extra_release}

[Andrea Bolognani]

It was mainly meant to be used for automatic builds through
Test::AutoBuild, so it can be removed now.

Signed-off-by: Andrea Bolognani 
---
 perl-Sys-Virt-TCK.spec.PL | 3 +--
 prepare-release.sh| 4 +---
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/perl-Sys-Virt-TCK.spec.PL b/perl-Sys-Virt-TCK.spec.PL
index 9694227..aac6cf6 100644
--- a/perl-Sys-Virt-TCK.spec.PL
+++ b/perl-Sys-Virt-TCK.spec.PL
@@ -32,12 +32,11 @@ __DATA__
 %define perlversion %(perl -e 'use Config; print $Config{version}')
 
 %define appname Sys-Virt-TCK
-%define _extra_release %{?extra_release:%{extra_release}}
 
 Summary: Sys::Virt::TCK - libvirt Technology Compatibility Kit
 Name: perl-%{appname}
 Version: @VERSION@
-Release: 1%{_extra_release}
+Release: 1
 License: GPLv2
 Group: Development/Tools
 Source: http://libvirt.org/sources/tck/%{appname}-v%{version}.tar.gz
diff --git a/prepare-release.sh b/prepare-release.sh
index 7282123..75777a8 100755
--- a/prepare-release.sh
+++ b/prepare-release.sh
@@ -42,9 +42,7 @@ rm -f $NAME-*.tar.gz
 ./Build dist
 
 if [ -f /usr/bin/rpmbuild ]; then
-  NOW=`date +"%s"`
-  EXTRA_RELEASE=".$USER$NOW"
-  rpmbuild -ta --define "extra_release $EXTRA_RELEASE" --clean $NAME-*.tar.gz
+  rpmbuild -ta --clean $NAME-*.tar.gz
 fi
 
 exit 0
-- 
2.17.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [tck PATCH v2 2/3] prepare-release: Drop references to Test::AutoBuild

[Andrea Bolognani]

They are misleading, and no longer relevant anyway.

Signed-off-by: Andrea Bolognani 
---
 prepare-release.sh | 15 +++
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/prepare-release.sh b/prepare-release.sh
index 005993d..7282123 100755
--- a/prepare-release.sh
+++ b/prepare-release.sh
@@ -1,8 +1,5 @@
 #!/bin/sh
 #
-# This script is used to Test::AutoBuild (http://www.autobuild.org)
-# to perform automated builds of the Sys-Virt module
-#
 # Copyright (C) 2009 Red Hat, Inc.
 # Copyright (C) 2009 Daniel P. Berrange
 #
@@ -21,9 +18,7 @@ set -e
 
 rm -rf MANIFEST blib _build Build
 
-test -z "$AUTOBUILD_INSTALL_ROOT" && AUTOBUILD_INSTALL_ROOT=$HOME/builder
-
-perl Build.PL install_base=$AUTOBUILD_INSTALL_ROOT
+perl Build.PL install_base=$HOME/builder
 
 ./Build
 ./Build manifest
@@ -47,12 +42,8 @@ rm -f $NAME-*.tar.gz
 ./Build dist
 
 if [ -f /usr/bin/rpmbuild ]; then
-  if [ -n "$AUTOBUILD_COUNTER" ]; then
-EXTRA_RELEASE=".auto$AUTOBUILD_COUNTER"
-  else
-NOW=`date +"%s"`
-EXTRA_RELEASE=".$USER$NOW"
-  fi
+  NOW=`date +"%s"`
+  EXTRA_RELEASE=".$USER$NOW"
   rpmbuild -ta --define "extra_release $EXTRA_RELEASE" --clean $NAME-*.tar.gz
 fi
 
-- 
2.17.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [dbus PATCH v2 1/9] Abandon usage of all *TypeToString functions in domain.c

[Pavel Hrdina]

On Fri, May 04, 2018 at 10:38:27AM +0200, Katerina Koukiou wrote:
> Converting ENUMS to str can be user friendly though
> it can be problematic in matters of backward compatibility.
> 
> In particular when some ENUM in libvirt API will introduce a
> new constant, libvirt-dbus will fail with:
> 
> size of array ‘_GStaticAssertCompileTimeAssertion_5’ is negative
> 
> So using ints is preferable to avoid the previous issue.
> 
> Signed-off-by: Katerina Koukiou 
> ---
>  data/org.libvirt.Domain.xml |  14 ++--
>  src/domain.c| 172 
> 
>  tests/test_domain.py|   6 +-
>  3 files changed, 25 insertions(+), 167 deletions(-)

[...]

> diff --git a/src/domain.c b/src/domain.c
> index e305fa3..40cf2f7 100644
> --- a/src/domain.c
> +++ b/src/domain.c

[...]

> @@ -137,12 +68,8 @@ 
> virtDBusDomainMemoryStatsToGVariant(virDomainMemoryStatPtr stats,
>  
>  g_variant_builder_init(, G_VARIANT_TYPE("a{st}"));
>  
> -for (gint i = 0; i < nr_stats; i++) {
> -const gchar *memoryStat = 
> virtDBusDomainMemoryStatTypeToString(stats[i].tag);
> -if (!memoryStat)
> -continue;
> -g_variant_builder_add(, "{st}", memoryStat, stats[i].val);
> -}
> +for (gint i = 0; i < nr_stats; i++)
> +g_variant_builder_add(, "{ut}", stats[i].tag, stats[i].val);

MemoryStats method needs to be updated in the introspection file.

>  
>  return g_variant_builder_end();
>  }

[...]

> @@ -1355,7 +1252,6 @@ virtDBusDomainGetJobInfo(GVariant *inArgs G_GNUC_UNUSED,
>  virtDBusConnect *connect = userData;
>  g_autoptr(virDomain) domain = NULL;
>  g_autofree virDomainJobInfoPtr jobInfo = NULL;
> -const gchar *jobTypeStr;
>  
>  domain = virtDBusDomainGetVirDomain(connect, objectPath, error);
>  if (!domain)
> @@ -1365,13 +1261,7 @@ virtDBusDomainGetJobInfo(GVariant *inArgs 
> G_GNUC_UNUSED,
>  if (virDomainGetJobInfo(domain, jobInfo) < 0)
>  return virtDBusUtilSetLastVirtError(error);
>  
> -jobTypeStr = virtDBusDomainJobTypeToString(jobInfo->type);
> -if (!jobTypeStr) {
> -g_set_error(error, VIRT_DBUS_ERROR, VIRT_DBUS_ERROR_LIBVIRT,
> -"Can't format virDomainJobType '%d' to string.", 
> jobInfo->type);
> -return;
> -}
> -*outArgs = g_variant_new("((sttt))", jobTypeStr,
> +*outArgs = g_variant_new("((uttt))", jobInfo->type,

GetJobInfo method needs to be updated in the introspection file.

>   jobInfo->timeElapsed, jobInfo->timeRemaining,
>   jobInfo->dataTotal, jobInfo->dataProcessed,
>   jobInfo->dataRemaining, jobInfo->memTotal,

Reviewed-by: Pavel Hrdina 


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [dbus PATCH v2 3/9] Abandon usage of all *TypeToString functions in network.c

[Pavel Hrdina]

On Fri, May 04, 2018 at 10:38:29AM +0200, Katerina Koukiou wrote:
> Converting ENUMS to str can be user friendly though
> it can be problematic in matters of backward compatibility.
> 
> In particular when some ENUM in libvirt API will introduce a
> new constant, libvirt-dbus will fail with:
> 
> size of array ‘_GStaticAssertCompileTimeAssertion_5’ is negative
> 
> So using ints is preferable to avoid the previous issue.
> 
> Signed-off-by: Katerina Koukiou 
> ---
>  data/org.libvirt.Network.xml |  6 ++--
>  src/network.c| 66 
> +++-
>  tests/test_network.py|  2 +-
>  3 files changed, 8 insertions(+), 66 deletions(-)

Reviewed-by: Pavel Hrdina 


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [dbus PATCH v2 4/9] Change DomainEvent argument from string to unsigned int

[Pavel Hrdina]

On Fri, May 04, 2018 at 10:38:30AM +0200, Katerina Koukiou wrote:
> Modify the relevant tests to comply with the new signal.
> 
> Note: argument matching in connect_to_signal method is
> only usable with string and thus had to be refactored.
> 
> Signed-off-by: Katerina Koukiou 
> ---
>  data/org.libvirt.Connect.xml |  2 +-
>  src/events.c | 26 +-
>  tests/libvirttest.py | 13 +
>  tests/test_connect.py| 12 
>  tests/test_domain.py | 30 --
>  5 files changed, 43 insertions(+), 40 deletions(-)

Reviewed-by: Pavel Hrdina 


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [dbus PATCH v2 2/9] Abandon usage of all *TypeToString functions in connect.c

[Pavel Hrdina]

On Fri, May 04, 2018 at 10:38:28AM +0200, Katerina Koukiou wrote:
> Converting ENUMS to str can be user friendly though
> it can be problematic in matters of backward compatibility.
> 
> In particular when some ENUM in libvirt API will introduce a
> new constant, libvirt-dbus will fail with:
> 
> size of array ‘_GStaticAssertCompileTimeAssertion_5’ is negative
> 
> So using ints is preferable to avoid the previous issue.
> 
> Signed-off-by: Katerina Koukiou 
> ---
>  data/org.libvirt.Connect.xml |  2 +-
>  src/connect.c| 18 +-
>  2 files changed, 2 insertions(+), 18 deletions(-)

Reviewed-by: Pavel Hrdina 


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH REBASE 3/5] utils: export virCopyError

[John Ferlan]

On 05/03/2018 03:39 AM, Nikolay Shirokovskiy wrote:
> 
> 
> On 01.05.2018 01:03, John Ferlan wrote:
>>
>>
>> On 04/18/2018 10:44 AM, Nikolay Shirokovskiy wrote:
>>> Signed-off-by: Nikolay Shirokovskiy 
>>> ---
>>>  src/libvirt_private.syms |  1 +
>>>  src/util/virerror.c  | 12 +---
>>>  src/util/virerror.h  |  1 +
>>>  3 files changed, 11 insertions(+), 3 deletions(-)
>>>
>>
>> NACK, you should be using virErrorCopyNew
>>
>> John
> 
> I introduced monError in qemuDomainObjPrivatePtr in next patch and it is not 
> a pointer so
> I need this function. I did not make monError pointer for the same reason it 
> is not pointer
> in monitor object - I use monError both as flag and as error message.
> 

I saw what you did - the fact is virCopyError is coded as private to the
module. Just making it global because you have a use for it is I believe
incorrect.

Ironically in the next patch you have:

+virErrorPtr err = qemuMonitorLastError(mon);
+
+virCopyError(err, >monError);


The first call isn't guaranteed to set @err and all you're essentially
doing is wanting to copy from mon->lastError to priv->monError or
perhaps similar to virCopyLastError.

Maybe some new API in virerror.c should handle that.

John

>>
>>> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
>>> index b31f599..6bbbf77 100644
>>> --- a/src/libvirt_private.syms
>>> +++ b/src/libvirt_private.syms
>>> @@ -1702,6 +1702,7 @@ ebtablesRemoveForwardAllowIn;
>>>  
>>>  
>>>  # util/virerror.h
>>> +virCopyError;
>>>  virDispatchError;
>>>  virErrorCopyNew;
>>>  virErrorInitialize;
>>> diff --git a/src/util/virerror.c b/src/util/virerror.c
>>> index c000b00..2ff8a3e 100644
>>> --- a/src/util/virerror.c
>>> +++ b/src/util/virerror.c
>>> @@ -188,10 +188,16 @@ virErrorGenericFailure(virErrorPtr err)
>>>  }
>>>  
>>>  
>>> -/*
>>> - * Internal helper to perform a deep copy of an error
>>> +/**
>>> + * virCopyError:
>>> + * @from: error to copy from
>>> + * @to: error to copy to
>>> + *
>>> + * Copy error fields from @from to @to.
>>> + *
>>> + * Returns 0 on success, -1 on failure.
>>>   */
>>> -static int
>>> +int
>>>  virCopyError(virErrorPtr from,
>>>   virErrorPtr to)
>>>  {
>>> diff --git a/src/util/virerror.h b/src/util/virerror.h
>>> index 760bfa4..90ac619 100644
>>> --- a/src/util/virerror.h
>>> +++ b/src/util/virerror.h
>>> @@ -192,6 +192,7 @@ void virReportOOMErrorFull(int domcode,
>>>  
>>>  int virSetError(virErrorPtr newerr);
>>>  virErrorPtr virErrorCopyNew(virErrorPtr err);
>>> +int virCopyError(virErrorPtr from, virErrorPtr to);
>>>  void virDispatchError(virConnectPtr conn);
>>>  const char *virStrerror(int theerrno, char *errBuf, size_t errBufLen);
>>>  
>>>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [perl PATCH] spec: Drop %{extra_release}

[Daniel P . Berrangé]

On Fri, May 04, 2018 at 03:50:57PM +0200, Andrea Bolognani wrote:
> It was mainly meant to be used for automatic builds through
> Test::AutoBuild, so it can be removed now.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  perl-Sys-Virt.spec.PL | 2 +-
>  prepare-release.sh| 4 +---
>  2 files changed, 2 insertions(+), 4 deletions(-)

Reviewed-by: Daniel P. Berrangé 


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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [perl PATCH] spec: Drop %{extra_release}

[Andrea Bolognani]

It was mainly meant to be used for automatic builds through
Test::AutoBuild, so it can be removed now.

Signed-off-by: Andrea Bolognani 
---
 perl-Sys-Virt.spec.PL | 2 +-
 prepare-release.sh| 4 +---
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/perl-Sys-Virt.spec.PL b/perl-Sys-Virt.spec.PL
index dbb749d..ffa6c86 100644
--- a/perl-Sys-Virt.spec.PL
+++ b/perl-Sys-Virt.spec.PL
@@ -24,7 +24,7 @@ __DATA__
 
 Name:   perl-Sys-Virt
 Version:@VERSION@
-Release:1%{?dist}%{?extra_release}
+Release:1%{?dist}
 Summary:Represent and manage a libvirt hypervisor connection
 License:GPLv2+ or Artistic
 Group:  Development/Libraries
diff --git a/prepare-release.sh b/prepare-release.sh
index 9de67fe..fea03f4 100755
--- a/prepare-release.sh
+++ b/prepare-release.sh
@@ -52,7 +52,5 @@ rm -f $NAME-*.tar.gz
 make dist
 
 if [ -f /usr/bin/rpmbuild ]; then
-  NOW=`date +"%s"`
-  EXTRA_RELEASE=".$USER$NOW"
-  rpmbuild --nodeps -ta --define "extra_release $EXTRA_RELEASE" --clean 
$NAME-*.tar.gz
+  rpmbuild --nodeps -ta --clean $NAME-*.tar.gz
 fi
-- 
2.17.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH REBASE 2/5] qemu: monitor: set error flag even in OOM conditions

[John Ferlan]

On 05/03/2018 03:35 AM, Nikolay Shirokovskiy wrote:
> 
> 
> On 01.05.2018 01:03, John Ferlan wrote:
>>
>>
>> On 04/18/2018 10:44 AM, Nikolay Shirokovskiy wrote:
>>> lastError.code is used as flag in qemu monitor. Unfortunately
>>> due to temporary OOM conditions (very unlikely through as thread local
>>> error is allocated on first use) we can fail to set this flag
>>> in case of monitor eofs/errors. This can cause hangs.
>>>
>>> Let's make sure flag is always set in case eofs/errors.
>>>
>>> Signed-off-by: Nikolay Shirokovskiy 
>>> ---
>>>  src/qemu/qemu_monitor.c | 5 +
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
>>> index f642d9a..906a320 100644
>>> --- a/src/qemu/qemu_monitor.c
>>> +++ b/src/qemu/qemu_monitor.c
>>> @@ -757,6 +757,11 @@ qemuMonitorIO(int watch, int fd, int events, void 
>>> *opaque)
>>>  virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>>> _("Error while processing monitor IO"));
>>>  virCopyLastError(>lastError);
>>> +
>>> +/* set error code if due to OOM conditions we fail to set it 
>>> before */
>>> +if (mon->lastError.code == VIR_ERR_OK)
>>
>> So this can only happen if virCopyLastError fails, right?  And code == 0
> 
> Nope, virCopyLastError fail to set code field only in one case - if last
> error is NULL which can be only if in next lines even reporting fails:
> 
> virErrorPtr err = virGetLastError();  
>   
> if (!err) 
>   
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",  
>   
>_("Error while processing monitor IO")); 
> 

I read the code as if pthread_getspecific in virThreadLocalGet returns
NULL, then virLastErrorObject will try to VIR_ALLOC_QUIET and return
NULL if it fails. If that succeeds, then virThreadLocalSet is called to
pthread_setspecific and if it fails, then @err (from VIR_ALLOC_QUIET) is
VIR_FREE'd and a NULL is returned.

So NULL could be returned if VIR_ALLOC_QUIET or virThreadLocalSet fail.

>> because of memset(..., 0, ...).  Wouldn't you only be working around the
>> issue for one of the callers? and only setting *.code and not *.domain?
> 
> We use mon->lastError for 2 purposes:
> 
> - as flag, and it is critical that we set this flag appropriately. We
>   use only code field for this purpuse

Understood, but still ->domain would be 0; however, if you set it to
something (such as VIR_FROM_THREAD), then perhaps that too could be used
as a marker of sorts (e.g. code == value, thread == value, and message
== NULL means something fairly specific).

> 
> - as error message that client can retrieve in case if API call returns
>   error, it is no critical if we can not provide error message, moreover
>   we can fail to pass error message in RPC for example due to temporary OOM
>   conditions
> 
> I even think to remove flag usage from mon->lastError and introduce bool
> flag but this looks ugly like 2 fields to keep error. 
> 

In any case, perhaps you misinterpreted my remark. Only adjusting this
one caller fixes one condition, but if there are possibly more then
changing virCopyLastError instead of here would allow those others to
also have the adjustment...

>>
>> BTW: I feel we've been this way in rather recent history... See commit
>> 'e711a391' - I recall we had varying opinions on that one.
> 
> Yes, but this fix does not relate to usage mon->lastError as flag, rather
> then to second usage in the above distinction.
> 
>>
>> Anyway, assuming CopyLastError is the issue, then perhaps the right
>> place to fix this is there and I would think at least the following
>> would need to be set in the error path:
>>
>>to->code = VIR_ERR_NO_MEMORY;
>>to->domain = VIR_FROM_THREAD;
> 
> It can be misleading. For example if we forget to set last error
> and then try to copy it.
> 
>>
>> We cannot create a to->message...
>>
>> Of course, how can we be "sure" that the failure is because the
>> VIR_ALLOC_QUIET in virLastErrorObject caused the issue other than
>> changing it's return values or passing a "bool " that's only set
>> when the VIR_ALLOC_QUIET fails allowing the caller to decide what to do.
>> It's local, so change is limited. Thus virGetLastErrorMessage and
>> virCopyLastError could make use of the returned, while other callers
>> could just pass "".
>>
>> In the long run, it's a somewhat interesting corner case problem and we
>> may have some varying opinions over the "best way" to resolve.  Suffice
>> to say, libvirt probably isn't going to "survive" much longer, but
>> unless others read this response and have varying opinions and/or we
>> come to a resolution - can this be posted/consider on it's own merits?> 
>> Tks -
>>
>> John
> 
> May be it would be just more simple to not to use mon->lastError as 

Re: [libvirt] [perl PATCH v2 2/3] prepare-release: Drop references to Test::AutoBuild

[Daniel P . Berrangé]

On Fri, May 04, 2018 at 03:37:50PM +0200, Andrea Bolognani wrote:
> On Fri, 2018-05-04 at 14:20 +0100, Daniel P. Berrangé wrote:
> > On Fri, May 04, 2018 at 03:14:04PM +0200, Andrea Bolognani wrote:
> > > On Fri, 2018-05-04 at 13:38 +0100, Daniel P. Berrangé wrote:
> > > > We can actually chop the 'extra_release' bit out of the RPM spec
> > > > entirely, as it is only useful for automated CI builds.
> > > 
> > > Neat. I assume you meant that we can avoid defining it when calling
> > > rpmbuild in the script, not that we want to remove it from the spec
> > > altogether, right? The latter can still be useful.
> > 
> > I did actually mean removing it from the RPM spec entirely - Fedora
> > reviewers have complained about it existing multiple times, but at
> > least in past I could justify it for CI purposes.
> 
> Not sure I agree, but I don't feel strongly enough to oppose the
> change either :)
> 
> That IMHO puts it squarely into separate patch territory though,
> so if you're okay with it I'll push this one as originally posted
> and follow up with another that drops extra_release completely.

Sure, consider it 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] [perl PATCH v2 2/3] prepare-release: Drop references to Test::AutoBuild

[Andrea Bolognani]

On Fri, 2018-05-04 at 14:20 +0100, Daniel P. Berrangé wrote:
> On Fri, May 04, 2018 at 03:14:04PM +0200, Andrea Bolognani wrote:
> > On Fri, 2018-05-04 at 13:38 +0100, Daniel P. Berrangé wrote:
> > > We can actually chop the 'extra_release' bit out of the RPM spec
> > > entirely, as it is only useful for automated CI builds.
> > 
> > Neat. I assume you meant that we can avoid defining it when calling
> > rpmbuild in the script, not that we want to remove it from the spec
> > altogether, right? The latter can still be useful.
> 
> I did actually mean removing it from the RPM spec entirely - Fedora
> reviewers have complained about it existing multiple times, but at
> least in past I could justify it for CI purposes.

Not sure I agree, but I don't feel strongly enough to oppose the
change either :)

That IMHO puts it squarely into separate patch territory though,
so if you're okay with it I'll push this one as originally posted
and follow up with another that drops extra_release completely.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [perl PATCH v2 2/3] prepare-release: Drop references to Test::AutoBuild

[Daniel P . Berrangé]

On Fri, May 04, 2018 at 03:14:04PM +0200, Andrea Bolognani wrote:
> On Fri, 2018-05-04 at 13:38 +0100, Daniel P. Berrangé wrote:
> > On Fri, May 04, 2018 at 02:20:25PM +0200, Andrea Bolognani wrote:
> [...]
> > >  if [ -f /usr/bin/rpmbuild ]; then
> > > -  if [ -n "$AUTOBUILD_COUNTER" ]; then
> > > -EXTRA_RELEASE=".auto$AUTOBUILD_COUNTER"
> > > -  else
> > > -NOW=`date +"%s"`
> > > -EXTRA_RELEASE=".$USER$NOW"
> > > -  fi
> > > +  NOW=`date +"%s"`
> > > +  EXTRA_RELEASE=".$USER$NOW"
> > >rpmbuild --nodeps -ta --define "extra_release $EXTRA_RELEASE" --clean 
> > > $NAME-*.tar.gz
> > >  fi
> > 
> > We can actually chop the 'extra_release' bit out of the RPM spec
> > entirely, as it is only useful for automated CI builds.
> 
> Neat. I assume you meant that we can avoid defining it when calling
> rpmbuild in the script, not that we want to remove it from the spec
> altogether, right? The latter can still be useful.

I did actually mean removing it from the RPM spec entirely - Fedora
reviewers have complained about it existing multiple times, but at
least in past I could justify it for CI purposes.

> 
> Can I just squash in the following diff and push, or do you want me
> to respin?
> 
> 
> diff --git a/prepare-release.sh b/prepare-release.sh
> index 22f9155..25a5cda 100755
> --- a/prepare-release.sh
> +++ b/prepare-release.sh
> @@ -52,9 +52,7 @@ rm -f $NAME-*.tar.gz
>  make dist
> 
>  if [ -f /usr/bin/rpmbuild ]; then
> -  NOW=`date +"%s"`
> -  EXTRA_RELEASE=".$USER$NOW"
> -  rpmbuild --nodeps -ta --define "extra_release $EXTRA_RELEASE" --clean 
> $NAME-*.tar.gz
> +  rpmbuild --nodeps -ta --clean $NAME-*.tar.gz
>  fi
> 
>  # Skip debian pkg for now
> -- 
> Andrea Bolognani / Red Hat / Virtualization

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

[libvirt] [PATCH] lxc: convert to typesafe virConf accessors in lxc_native.c

[Prafull]

From: Prafullkumar Tale 

Signed-off-by: Prafullkumar Tale 
---
 src/lxc/lxc_native.c | 141 +--
 1 file changed, 70 insertions(+), 71 deletions(-)

diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c
index 55ea774..35077e1 100644
--- a/src/lxc/lxc_native.c
+++ b/src/lxc/lxc_native.c
@@ -199,19 +199,15 @@ lxcSetRootfs(virDomainDefPtr def,
  virConfPtr properties)
 {
 int type = VIR_DOMAIN_FS_TYPE_MOUNT;
-virConfValuePtr value;
+char *value = NULL;
 
-if (!(value = virConfGetValue(properties, "lxc.rootfs")) ||
-!value->str) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("Missing lxc.rootfs configuration"));
+if (virConfGetValueString(properties, "lxc.rootfs", ) <= 0)
 return -1;
-}
 
-if (STRPREFIX(value->str, "/dev/"))
+if (STRPREFIX(value, "/dev/"))
 type = VIR_DOMAIN_FS_TYPE_BLOCK;
 
-if (lxcAddFSDef(def, type, value->str, "/", false, 0) < 0)
+if (lxcAddFSDef(def, type, value, "/", false, 0) < 0)
 return -1;
 
 return 0;
@@ -684,17 +680,17 @@ lxcConvertNetworkSettings(virDomainDefPtr def, virConfPtr 
properties)
 static int
 lxcCreateConsoles(virDomainDefPtr def, virConfPtr properties)
 {
-virConfValuePtr value;
+char *value = NULL;
 int nbttys = 0;
 virDomainChrDefPtr console;
 size_t i;
 
-if (!(value = virConfGetValue(properties, "lxc.tty")) || !value->str)
+if (virConfGetValueString(properties, "lxc.tty", ) <= 0)
 return 0;
 
-if (virStrToLong_i(value->str, NULL, 10, ) < 0) {
+if (virStrToLong_i(value, NULL, 10, ) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR, _("failed to parse int: '%s'"),
-   value->str);
+   value);
 return -1;
 }
 
@@ -761,89 +757,91 @@ lxcIdmapWalkCallback(const char *name, virConfValuePtr 
value, void *data)
 static int
 lxcSetMemTune(virDomainDefPtr def, virConfPtr properties)
 {
-virConfValuePtr value;
+char *value = NULL;
 unsigned long long size = 0;
 
-if ((value = virConfGetValue(properties,
-"lxc.cgroup.memory.limit_in_bytes")) &&
-value->str && STRNEQ(value->str, "-1")) {
-if (lxcConvertSize(value->str, ) < 0)
-return -1;
+if (virConfGetValueString(properties,
+  "lxc.cgroup.memory.limit_in_bytes",
+  ) > 0) {
+if (lxcConvertSize(value, ) < 0)
+goto error;
 size = size / 1024;
 virDomainDefSetMemoryTotal(def, size);
 def->mem.hard_limit = virMemoryLimitTruncate(size);
 }
 
-if ((value = virConfGetValue(properties,
-"lxc.cgroup.memory.soft_limit_in_bytes")) &&
-value->str && STRNEQ(value->str, "-1")) {
-if (lxcConvertSize(value->str, ) < 0)
-return -1;
-
+if (virConfGetValueString(properties,
+  "lxc.cgroup.memory.soft_limit_in_bytes",
+  ) > 0) {
+if (lxcConvertSize(value, ) < 0)
+goto error;
 def->mem.soft_limit = virMemoryLimitTruncate(size / 1024);
 }
 
-if ((value = virConfGetValue(properties,
-"lxc.cgroup.memory.memsw.limit_in_bytes")) &&
-value->str && STRNEQ(value->str, "-1")) {
-if (lxcConvertSize(value->str, ) < 0)
-return -1;
-
+if (virConfGetValueString(properties,
+  "lxc.cgroup.memory.memsw.limit_in_bytes",
+  ) > 0) {
+if (lxcConvertSize(value, ) < 0)
+goto error;
 def->mem.swap_hard_limit = virMemoryLimitTruncate(size / 1024);
 }
 return 0;
+
+ error:
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("failed to parse integer: '%s'"), value);
+return -1;
+
 }
 
 static int
 lxcSetCpuTune(virDomainDefPtr def, virConfPtr properties)
 {
-virConfValuePtr value;
+char *value = NULL;
 
-if ((value = virConfGetValue(properties, "lxc.cgroup.cpu.shares")) &&
-value->str) {
-if (virStrToLong_ull(value->str, NULL, 10, >cputune.shares) < 0)
+if (virConfGetValueString(properties, "lxc.cgroup.cpu.shares",
+  ) > 0) {
+if (virStrToLong_ull(value, NULL, 10, >cputune.shares) < 0)
 goto error;
 def->cputune.sharesSpecified = true;
 }
 
-if ((value = virConfGetValue(properties,
- "lxc.cgroup.cpu.cfs_quota_us")) &&
-value->str && virStrToLong_ll(value->str, NULL, 10,
-  >cputune.quota) < 0)
-goto error;
+if (virConfGetValueString(properties, "lxc.cgroup.cpu.cfs_quota_us",
+  ) > 0) {
+if (virStrToLong_ll(value, NULL, 10, >cputune.quota) < 0)
+

Re: [libvirt] [perl PATCH v2 2/3] prepare-release: Drop references to Test::AutoBuild

[Andrea Bolognani]

On Fri, 2018-05-04 at 13:38 +0100, Daniel P. Berrangé wrote:
> On Fri, May 04, 2018 at 02:20:25PM +0200, Andrea Bolognani wrote:
[...]
> >  if [ -f /usr/bin/rpmbuild ]; then
> > -  if [ -n "$AUTOBUILD_COUNTER" ]; then
> > -EXTRA_RELEASE=".auto$AUTOBUILD_COUNTER"
> > -  else
> > -NOW=`date +"%s"`
> > -EXTRA_RELEASE=".$USER$NOW"
> > -  fi
> > +  NOW=`date +"%s"`
> > +  EXTRA_RELEASE=".$USER$NOW"
> >rpmbuild --nodeps -ta --define "extra_release $EXTRA_RELEASE" --clean 
> > $NAME-*.tar.gz
> >  fi
> 
> We can actually chop the 'extra_release' bit out of the RPM spec
> entirely, as it is only useful for automated CI builds.

Neat. I assume you meant that we can avoid defining it when calling
rpmbuild in the script, not that we want to remove it from the spec
altogether, right? The latter can still be useful.

Can I just squash in the following diff and push, or do you want me
to respin?


diff --git a/prepare-release.sh b/prepare-release.sh
index 22f9155..25a5cda 100755
--- a/prepare-release.sh
+++ b/prepare-release.sh
@@ -52,9 +52,7 @@ rm -f $NAME-*.tar.gz
 make dist

 if [ -f /usr/bin/rpmbuild ]; then
-  NOW=`date +"%s"`
-  EXTRA_RELEASE=".$USER$NOW"
-  rpmbuild --nodeps -ta --define "extra_release $EXTRA_RELEASE" --clean 
$NAME-*.tar.gz
+  rpmbuild --nodeps -ta --clean $NAME-*.tar.gz
 fi

 # Skip debian pkg for now
-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/2] all: replacing virGetLastError with virHas/GetLastErrorCode/Domain

[Erik Skultety]

On Fri, May 04, 2018 at 04:46:48AM +0100, ramyelkest wrote:
> Many places in the code call virGetLastError() just to check the
> raised error code, or domain. However virGetLastError() can return
> NULL, so the code has to check for that as well.
>
> So Instead we create functions virGetLastErrorCode and virGetLastErrorDomain
> (in addition to the existing virGetLastErrorMessage) that always return a
> valid error code/domain/message, to simplify callers.
>
> Also created virHasLastErrorCode for completion
>
> for:
> https://wiki.libvirt.org/page/BiteSizedTasks#Add_and_use_virGetLastErrorCode.28.29
>
> Notes:
> * There are a few instances of virGetLastErrorCode() left where we use 
> multiple
> fields from the error, which makes sense to keep it.
> * I didn't manage to use virGetLastErrorDomain() (due to the above) so I'm 
> inclined
> to remove it.

Same comments to the commit message as for patch 1.

...
>
> Signed-off-by: Ramy Elkest 
> ---
>  src/locking/lock_driver_lockd.c |  3 +--
>  src/lxc/lxc_controller.c|  4 +---
>  src/qemu/qemu_agent.c   |  3 +--
>  src/qemu/qemu_conf.c|  3 +--
>  src/qemu/qemu_domain.c  |  2 +-
>  src/qemu/qemu_driver.c  | 12 ++--
>  src/qemu/qemu_hotplug.c |  2 +-
>  src/qemu/qemu_migration.c   |  4 ++--
>  src/qemu/qemu_monitor.c |  5 ++---
>  src/qemu/qemu_monitor_json.c|  2 +-
>  src/qemu/qemu_process.c |  2 +-
>  src/remote/remote_driver.c  |  3 +--
>  src/rpc/virnetclient.c  |  2 +-
>  src/util/virfilecache.c |  3 +--
>  src/util/virxml.c   |  4 ++--
>  tests/commandtest.c |  2 +-
>  tests/testutils.c   |  6 ++
>  tests/virhostcputest.c  |  2 +-
>  tests/virstoragetest.c  |  8 
>  tools/virsh-domain-monitor.c|  7 +++
>  tools/virsh-domain.c|  4 +---
>  tools/virsh-util.c  |  3 +--
>  tools/vsh.c |  2 +-
>  23 files changed, 37 insertions(+), 51 deletions(-)

I found 2 more occurrences in src/util/virnetlibsshsession.c (l.502) and
src/util/virmodule.c (l.126) respectively, which can be optimized too.

...
>  if (!(loadData = cache->handlers.loadFile(file, name, cache->priv))) {
> -virErrorPtr err = virGetLastError();
>  VIR_WARN("Failed to load cached data from '%s' for '%s': %s",
> - file, name, err ? NULLSTR(err->message) : "unknown error");
> + file, name, virGetLastErrorMessage());

Good catch, although, this should be a separate patch IMO, let's say patch 1
and then the rest of the series. On a side note, I looked whether there aren't
other places which could use the virGetLastErrorMessage, sadly no, since the
interface isn't really flexible (among other problems with the whole error
subsystem) so that when there's no error we report "unknown error" which is
good as nothing, so providing a custom string in case there's no error or we
can't identify it would probably have been a smarter choice, oh well...

Other than that I'm fine with the changes (v2 shouldn't be a problem).

Erik

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [jenkins-ci PATCH] guests: Update vault

[Pavel Hrdina]

Renaming node in Jenkins changes the secret as well so we cannot reuse
the same secret as for the old node.

This updates the libvirt-fedora-28 secret to correct one.

Signed-off-by: Pavel Hrdina 
---

Pushed under trivial rule.

 guests/vars/vault.yml | 90 +--
 1 file changed, 45 insertions(+), 45 deletions(-)

diff --git a/guests/vars/vault.yml b/guests/vars/vault.yml
index 403bd9b..5bf89dc 100644
--- a/guests/vars/vault.yml
+++ b/guests/vars/vault.yml
@@ -1,46 +1,46 @@
 $ANSIBLE_VAULT;1.1;AES256
-3766396131623832643164623861363938613135376161343231383739663836313130363538
-3136326161376136376332636534643437626530316439650a636134636561333962376564626635
-31306539613534653133383161313834633133316361303539646234356335386639303332323063
-3937626336326131650a373737396639373063316131633634393961393634663961613532353134
-3262306537343833656164353866653665323536643730386364653162323939616131313634
-38653936663937613765663465353166656363356336636362353432636537633836623930653437
-3035323930343561353663686436336638636462626537623766343237376331643334633433
-37353531616466386234376639306261303561373035383338383366646537373231386362313562
-396536643838383365363138623635616365383530366466653932636238336234383865
-3962353561636165383862616430626634373237626638373131303439356164646361613838
-64333165393935636162613638626461313661623963626664366361366364643136313236306636
-61633665393630356133306338343563636332386539626366383133326662633638306330323437
-34613034326465346338633538363231386430363932656430346634663437333734363234346533
-35303962333032626435343030653338656133323932386264346562323133326164623961623466
-6264376563336362666439653038653335373531303930313239393166623539386239323337
-3939333731356365613164613060313230366462306636363535386663666131613662363566
-63643030633262336263316134636136616430636536363464386436616530336264396264393762
-3664653163623834376337643866616337616661323130636363353662316635643738683665
-37353962383238346132633230373666366638386162663263663134303936633034643737386362
-36393537373864323632376632376134303866393966336536376139393334613163356234623863
-36353837616535313266326538376663316366616561613833393037316264396532306430623338
-33636339623861313536323835326464636265386536646263643765306432626462363533373434
-32343961613563613734633736396139623463653230303337333862393836393533363637626662
-35376236613436323736643533613436646136373436363862383933323231663237343838313262
-34343337343031326139366636346632303738366439346433633738313464366438366235356462
-6537616539653262353335396332333635346661613431343237366166376462393732373532
-62616631643564653865386133663666313366313336323432316236336165386234323165366461
-35343238336235386364363732343862393039326234653539643861383832393465343536353739
-33633134333836393965343935316564653866363130343234633566643463343331306135386334
-39316166616463383964613432373437623964343164383638666531303064313063663330366566
-3364393039623262626639663436656463316837373062636131623463613061656138363534
-39383830626136636563663233376237626565643661656430336462626436386235643239646365
-34303938323237626362613765326138376130666466373033626338363161383432623465353131
-39373230336330666337393161306437313035373164366330333533636662313266336333626261
-37353638623362643138356434323864346265613835306665383564323139333539336231323534
-38323664396438663664343436333133656336663633396234303034633163643534346139663632
-34633364373066303363306366653566353338326566616138643761343166343536333239653966
-3962663938376265343964363166316130323330313063336532303531393830646532343261
-6536343437353865333730623238653465313266643838333862616337323063626432393939
-34663838363931353561396164623138366538376230333738373962356337366533643866343863
-3763336632333432393630396265346235383138626233626339633366336164623766653862
-32653262373433343136386363386231373938636661373363393331663365636333633164353563
-3864396637326135613166643231393431303836303430616237326235366630393564303935
-62383432633763623864323263616632396166363165336331363434316430626334616537376132
-6365336362373265363462343666356434323365633632626632666130356431
+66646139336431653964363035353762363664313738393832356132333835616363373032656335
+3439633266356532316564643762623935643736613839390a616631363336393962633464326264
+31373235323236303134356165386638663865353331353362386538386233363438623035343462
+3864656136376430360a64623537373463373934616463363234316262376534633865616632
+32643566633531323162366134333562363463336237646130393965646335316561393565663664
+36343164373564336233363963393031363937336137303433396265323261663632373534613365
+31613838363562366564646333646638303837626638323065326237623665646264633233633537
+3937343236396239646332353235343462363138376334623434346233356538366439393835
+30356437393232363432313334616462346164393437303330343562356461626464633935376239

Re: [libvirt] [PATCH] Qemu driver: Support network-backed pflash disks.

[John Ferlan]

On 05/02/2018 01:24 PM, Prerna wrote:
> 
> 
> On Sat, Apr 28, 2018 at 1:11 AM, John Ferlan  > wrote:
> 
> 
> 

[...]

>  
> 
> This will need a v2 anyway because the patch has too much going on and
> needs to be split up more.
> 
> 
> Will do. I should have properly mentioned this was an RFC rather than a
> ready-to-merge patch, and that this currently excluded test and
> documentation fixes.
>  

Even with an RFC - it's really more pleasant to break things up a bit.
Makes it easier to follow flow rather than one large patch where
multiple things are happening and you're left trying to figure that all
out.  But I also understand that sometimes when posting as RFC that
things languish longer than posting as v1, so it's a gamble as to which
to use...

> 
> 
> You need to break out the domain_conf, docs, etc. changes into one
> patch. Much of what you put into the cover that describes the XML
> should 
> 
> 
> Got it.
>  
> 
> go into this patch's commit message. There should be xml2xml test
> changes as well as adjustments to formatdomain.html.in
>  to describe the
> new options. The part of the cover that says automatically reformatting
> to use the storage source cannot happen. There's precedence for that 
> 
> when the  and  moved *into* the storage source we
> still have to accommodate for them outside. Internally, it can be placed
> as expected, but when formatting, we have to format as we read. After
> 
> 
> Ok. I had explicitly asked around on IRC if it was okay "breaking" the
> existing XML  semantics. Peter had mentioned it was okay to have the XML
> read as its old semantic. The formatter could later "translate" the old
> -style absolute filepaths into the "new-style" source-description that
> is introduced.
> I had kept that in mind while implementing this patch. If that is not to
> be done, I will need to redo parts of this patch.
>  

I see you pinged on IRC today - I had this marked as get back to because
it appears there's questions. Juggling lots of different series and your
response only came on Wed afternoon for me. It's now Friday morning.

In any case, I think that contradicts what was required of me to be done
when moving  and  into  from . See
commits 37537a7c and 8002d3c which handle formatting as the XML was read.

Now if someone *changes* the read XML to use , then
that's a different story. But if you find:

/usr/lib/xen/boot/hvmloader

then you should format that.

If you find


  


then you format that.

Hopefully the two examples provided give you an idea of the "accepted
mechanism" used in a previous change of XML format.

> 
> that, perhaps add the security changes in another, the cgroup in
> another, and finally the qemu adjustments in the last.  Not even sure if
> you need a capability as well.
> 
> 
> Why do you think we'd need a capability for this?  I'd be keen to
> understand why changes to XML spec  is not enough.
>  

Depends on the command line generated I guess - cannot recall right now
if that was clear while I was looking at the existing changes. In the
long run, you have to decide whether the arguments provided to QEMU have
existed since QEMU 1.5. If they have, then no capability, but if there's
some argument provided on the QEMU command line that was introduced in
(for example) 2.9 in order to allow usage of a networked path, then
you'd need a capability.

> 
> Finally this doesn't even compile for me.  You removed @path from
> _virDomainLoaderDef but neglected to adjust all consumers. I suggest
> using cscope and egrep'ing on "os.*loader->path" as well as ->nvram
> since you changed it's type.
> 
> 
> I know why. I had run and tested this to work, but my build
> configuration included the qemu driver and excluded every other driver.
> Given that this element extends to other drivers, I can understand the
> build issues. Again, should have mentioned this was an RFC.
>  
> 
> 
> I assume you've considered that network storage types need to deal with
> authentication and encryption passphrases, right?  What about using a
> srcpool storage source?
> 
> 
> Erm, no. This patch does not include support for
> encryption/authenticaton. I will need to add those.
>  

At least a storage source provides the capability to storage, parse,
format that data...

> 
> I'll briefly scan the rest.
> 

[...]

> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index 35666c1..c80f9d9 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -2883,8 +2883,8 @@ virDomainLoaderDefFree(virDomainLoaderDefPtr
> loader)
> >      if (!loader)
> >          return;
> > 
> > -    VIR_FREE(loader->path);
> > -    VIR_FREE(loader->nvram);
> > +    virStorageSourceFree(loader->loader_src);
> 
> loader_src is redundant to loader isn't 

Re: [libvirt] [perl PATCH v2 3/3] prepare-release: Drop references to Debian packages

[Daniel P . Berrangé]

On Fri, May 04, 2018 at 02:20:26PM +0200, Andrea Bolognani wrote:
> The Debian packaging was never included in the git repository
> to begin with.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  prepare-release.sh | 8 
>  1 file changed, 8 deletions(-)

Reviewed-by: Daniel P. Berrangé 


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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [perl PATCH v2 1/3] maint: Rename autobuild.sh to prepare-release.sh

[Daniel P . Berrangé]

On Fri, May 04, 2018 at 02:20:24PM +0200, Andrea Bolognani wrote:
> The script was originally used by the Test::AutoBuild
> project to perform periodic automatic builds; however, that
> effort has been abandoned a long time ago, and these days
> libvirt-perl CI builds are happening on the Jenkins-based
> CentOS CI environment under the libvirt umbrella[1], where
> build recipes are maintained separately from the projects
> themselves.
> 
> The script is still used to prepare releases, so it can't
> be dropped from the repository: rename it so that its
> purpose is more clearly communicated instead.
> 
> [1] https://ci.centos.org/view/libvirt/
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  autobuild.sh => prepare-release.sh | 0
>  1 file changed, 0 insertions(+), 0 deletions(-)
>  rename autobuild.sh => prepare-release.sh (100%)

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] [perl PATCH v2 2/3] prepare-release: Drop references to Test::AutoBuild

[Daniel P . Berrangé]

On Fri, May 04, 2018 at 02:20:25PM +0200, Andrea Bolognani wrote:
> They are no longer relevant and misleading.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  prepare-release.sh | 14 +++---
>  1 file changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/prepare-release.sh b/prepare-release.sh
> index 8a6d102..22f9155 100755
> --- a/prepare-release.sh
> +++ b/prepare-release.sh
> @@ -1,21 +1,17 @@
>  #!/bin/sh
> -#
> -# This script is used to Test::AutoBuild (http://www.autobuild.org)
> -# to perform automated builds of the Sys-Virt module
>  
>  NAME=Sys-Virt
>  
>  set -e
>  
>  test -n "$1" && RESULTS=$1 || RESULTS=results.log
> -: ${AUTOBUILD_INSTALL_ROOT=$HOME/builder}
>  
>  make -k realclean ||:
>  rm -rf MANIFEST blib pm_to_blib
>  
>  export TEST_MAINTAINER=1
>  
> -perl Makefile.PL  PREFIX=$AUTOBUILD_INSTALL_ROOT
> +perl Makefile.PL PREFIX=$HOME/builder
>  
>  rm -f MANIFEST
>  
> @@ -56,12 +52,8 @@ rm -f $NAME-*.tar.gz
>  make dist
>  
>  if [ -f /usr/bin/rpmbuild ]; then
> -  if [ -n "$AUTOBUILD_COUNTER" ]; then
> -EXTRA_RELEASE=".auto$AUTOBUILD_COUNTER"
> -  else
> -NOW=`date +"%s"`
> -EXTRA_RELEASE=".$USER$NOW"
> -  fi
> +  NOW=`date +"%s"`
> +  EXTRA_RELEASE=".$USER$NOW"
>rpmbuild --nodeps -ta --define "extra_release $EXTRA_RELEASE" --clean 
> $NAME-*.tar.gz
>  fi

We can actually chop the 'extra_release' bit out of the RPM spec
entirely, as it is only useful for automated CI builds.

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] [dbus PATCH v2 0/2] improve annotation of properties

[Katerina Koukiou]

On Fri, 2018-05-04 at 14:28 +0200, Pavel Hrdina wrote:
> Pavel Hrdina (2):
>   Change the default annotation for emitting changed properties to
> false
>   Annotate properties that will never change during the object
> lifecycle
> 
>  data/org.libvirt.Connect.xml | 3 +++
>  data/org.libvirt.Domain.xml  | 2 ++
>  data/org.libvirt.Network.xml | 3 +++
>  3 files changed, 8 insertions(+)

ACK both patches in the series.

Katerina

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [dbus PATCH v2 0/2] improve annotation of properties

[Pavel Hrdina]

Pavel Hrdina (2):
  Change the default annotation for emitting changed properties to false
  Annotate properties that will never change during the object lifecycle

 data/org.libvirt.Connect.xml | 3 +++
 data/org.libvirt.Domain.xml  | 2 ++
 data/org.libvirt.Network.xml | 3 +++
 3 files changed, 8 insertions(+)

-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [dbus PATCH v2 2/2] Annotate properties that will never change during the object lifecycle

[Pavel Hrdina]

These can be annotated as 'const' properties because they will never
change.

Reviewed-by: Katerina Koukiou 
Signed-off-by: Pavel Hrdina 
---
 data/org.libvirt.Connect.xml | 2 ++
 data/org.libvirt.Domain.xml  | 1 +
 data/org.libvirt.Network.xml | 2 ++
 3 files changed, 5 insertions(+)

diff --git a/data/org.libvirt.Connect.xml b/data/org.libvirt.Connect.xml
index 69bbc84..d7a9b5f 100644
--- a/data/org.libvirt.Connect.xml
+++ b/data/org.libvirt.Connect.xml
@@ -8,6 +8,7 @@
   https://libvirt.org/html/libvirt-libvirt-host.html#virConnectIsEncrypted
Note that monitoring of traffic on the D-Bus message bus is out 
of the scope of this property"/>
+  
 
 
   https://libvirt.org/html/libvirt-libvirt-host.html#virConnectIsSecure
Note that monitoring of traffic on the D-Bus message bus is out 
of the scope of this property"/>
+  
 
 
   
   https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetUUIDString"/>
+  
 
 
   
   https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkGetName"/>
+  
 
 
   
   https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkGetUUIDString"/>
+  
 
 
   https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [dbus PATCH v2 1/2] Change the default annotation for emitting changed properties to false

[Pavel Hrdina]

For some of these properties there is no libvirt event to detect the
change.  For some of the remaining properties we could somehow detect
the change but it would be a lot of code for nothing and it can be
added later if someone asks for that.

We could change the properties to methods but with the annotation we
can keep them as properties in order to allow to get them by single
D-Bus call.

Signed-off-by: Pavel Hrdina 
---

changes in v2:
- annotation was moved to  scope to make it the default
  one

 data/org.libvirt.Connect.xml | 1 +
 data/org.libvirt.Domain.xml  | 1 +
 data/org.libvirt.Network.xml | 1 +
 3 files changed, 3 insertions(+)

diff --git a/data/org.libvirt.Connect.xml b/data/org.libvirt.Connect.xml
index 3bda461..69bbc84 100644
--- a/data/org.libvirt.Connect.xml
+++ b/data/org.libvirt.Connect.xml
@@ -3,6 +3,7 @@
 
 
   
+
 
   https://libvirt.org/html/libvirt-libvirt-host.html#virConnectIsEncrypted
diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml
index eae6d97..089b896 100644
--- a/data/org.libvirt.Domain.xml
+++ b/data/org.libvirt.Domain.xml
@@ -3,6 +3,7 @@
 
 
   
+
 
   https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainIsActive"/>
diff --git a/data/org.libvirt.Network.xml b/data/org.libvirt.Network.xml
index 81bf081..5b6823e 100644
--- a/data/org.libvirt.Network.xml
+++ b/data/org.libvirt.Network.xml
@@ -3,6 +3,7 @@
 
 
   
+
 
   https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkIsActive"/>
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [perl PATCH v2 3/3] prepare-release: Drop references to Debian packages

[Andrea Bolognani]

The Debian packaging was never included in the git repository
to begin with.

Signed-off-by: Andrea Bolognani 
---
 prepare-release.sh | 8 
 1 file changed, 8 deletions(-)

diff --git a/prepare-release.sh b/prepare-release.sh
index 22f9155..9de67fe 100755
--- a/prepare-release.sh
+++ b/prepare-release.sh
@@ -56,11 +56,3 @@ if [ -f /usr/bin/rpmbuild ]; then
   EXTRA_RELEASE=".$USER$NOW"
   rpmbuild --nodeps -ta --define "extra_release $EXTRA_RELEASE" --clean 
$NAME-*.tar.gz
 fi
-
-# Skip debian pkg for now
-exit 0
-
-if [ -f /usr/bin/fakeroot ]; then
-  fakeroot debian/rules clean
-  fakeroot debian/rules DESTDIR=$HOME/packages/debian binary
-fi
-- 
2.17.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [perl PATCH v2 2/3] prepare-release: Drop references to Test::AutoBuild

[Andrea Bolognani]

They are no longer relevant and misleading.

Signed-off-by: Andrea Bolognani 
---
 prepare-release.sh | 14 +++---
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/prepare-release.sh b/prepare-release.sh
index 8a6d102..22f9155 100755
--- a/prepare-release.sh
+++ b/prepare-release.sh
@@ -1,21 +1,17 @@
 #!/bin/sh
-#
-# This script is used to Test::AutoBuild (http://www.autobuild.org)
-# to perform automated builds of the Sys-Virt module
 
 NAME=Sys-Virt
 
 set -e
 
 test -n "$1" && RESULTS=$1 || RESULTS=results.log
-: ${AUTOBUILD_INSTALL_ROOT=$HOME/builder}
 
 make -k realclean ||:
 rm -rf MANIFEST blib pm_to_blib
 
 export TEST_MAINTAINER=1
 
-perl Makefile.PL  PREFIX=$AUTOBUILD_INSTALL_ROOT
+perl Makefile.PL PREFIX=$HOME/builder
 
 rm -f MANIFEST
 
@@ -56,12 +52,8 @@ rm -f $NAME-*.tar.gz
 make dist
 
 if [ -f /usr/bin/rpmbuild ]; then
-  if [ -n "$AUTOBUILD_COUNTER" ]; then
-EXTRA_RELEASE=".auto$AUTOBUILD_COUNTER"
-  else
-NOW=`date +"%s"`
-EXTRA_RELEASE=".$USER$NOW"
-  fi
+  NOW=`date +"%s"`
+  EXTRA_RELEASE=".$USER$NOW"
   rpmbuild --nodeps -ta --define "extra_release $EXTRA_RELEASE" --clean 
$NAME-*.tar.gz
 fi
 
-- 
2.17.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [perl PATCH v2 1/3] maint: Rename autobuild.sh to prepare-release.sh

[Andrea Bolognani]

The script was originally used by the Test::AutoBuild
project to perform periodic automatic builds; however, that
effort has been abandoned a long time ago, and these days
libvirt-perl CI builds are happening on the Jenkins-based
CentOS CI environment under the libvirt umbrella[1], where
build recipes are maintained separately from the projects
themselves.

The script is still used to prepare releases, so it can't
be dropped from the repository: rename it so that its
purpose is more clearly communicated instead.

[1] https://ci.centos.org/view/libvirt/

Signed-off-by: Andrea Bolognani 
---
 autobuild.sh => prepare-release.sh | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename autobuild.sh => prepare-release.sh (100%)

diff --git a/autobuild.sh b/prepare-release.sh
similarity index 100%
rename from autobuild.sh
rename to prepare-release.sh
-- 
2.17.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [perl PATCH v2 0/3] maint: Rename autobuild.sh to prepare-release.sh

[Andrea Bolognani]

Changes from [v1]:

* rename the script instead of dropping it;

* perform some further clean ups.


[v1] https://www.redhat.com/archives/libvir-list/2018-May/msg00221.html

Andrea Bolognani (3):
  maint: Rename autobuild.sh to prepare-release.sh
  prepare-release: Drop references to Test::AutoBuild
  prepare-release: Drop references to Debian packages

 autobuild.sh => prepare-release.sh | 22 +++---
 1 file changed, 3 insertions(+), 19 deletions(-)
 rename autobuild.sh => prepare-release.sh (67%)

-- 
2.17.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/2] util: adding virHasLastError and virGetLastErrorCode/Domain

[Erik Skultety]

On Fri, May 04, 2018 at 04:46:47AM +0100, ramyelkest wrote:
> Many places in the code call virGetLastError() just to check the
> raised error code, or domain. However virGetLastError() can return
> NULL, so the code has to check for that as well.

s/as well/first.

>
> So Instead we create functions virGetLastErrorCode and virGetLastErrorDomain
> (in addition to the existing virGetLastErrorMessage) that always return a
> valid error code/domain/message, to simplify callers.

No paragraph, how about "This patch therefore introduces virGetLasErrorCode
function which always returns a valid error code, thus dropping the need to
perform any checks on the error object".

>
> Also created virHasLastErrorCode for completion
>
> My first commit, for:
> https://wiki.libvirt.org/page/BiteSizedTasks#Add_and_use_virGetLastErrorCode.28.29
>

You mentioned this in the cover letter already, no need to have it in the commit
messages too ;).

> Signed-off-by: Ramy Elkest 
> ---
>  include/libvirt/virterror.h |  3 +++
>  src/libvirt_public.syms |  7 +
>  src/util/virerror.c | 63 
> +
>  3 files changed, 73 insertions(+)
>
> diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
> index 3e7c7a0..8336258 100644
> --- a/include/libvirt/virterror.h
> +++ b/include/libvirt/virterror.h
> @@ -344,6 +344,9 @@ voidvirResetLastError   (void);
>  voidvirResetError   (virErrorPtr err);
>  voidvirFreeError(virErrorPtr err);
>
> +int virHasLastError (void);
> +int virGetLastErrorCode (void);
> +int virGetLastErrorDomain   (void);
>  const char *virGetLastErrorMessage  (void);
>
>  virErrorPtr virConnGetLastError (virConnectPtr conn);
> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
> index 95df3a0..3a641a3 100644
> --- a/src/libvirt_public.syms
> +++ b/src/libvirt_public.syms
> @@ -785,4 +785,11 @@ LIBVIRT_4.1.0 {
>  virStoragePoolLookupByTargetPath;
>  } LIBVIRT_3.9.0;
>
> +LIBVIRT_4.4.0 {
> +global:
> +virHasLastError;
> +virGetLastErrorCode;
> +virGetLastErrorDomain;
> +} LIBVIRT_4.1.0;
> +
>  #  define new API here using predicted next version number 
> diff --git a/src/util/virerror.c b/src/util/virerror.c
> index c000b00..818af2e 100644
> --- a/src/util/virerror.c
> +++ b/src/util/virerror.c
> @@ -272,6 +272,69 @@ virGetLastError(void)
>
>
>  /**
> + * virHasLastError:
> + *
> + * Check for a reported last error
> + *
> + * The error object is kept in thread local storage, so separate
> + * threads can safely access this concurrently.
> + *
> + * Returns 1 if there is an error and the error code is not VIR_ERR_OK,
> +   otherwise returns 0
> + */
> +int
> +virHasLastError(void)
> +{
> +virErrorPtr err = virLastErrorObject();
> +if (!err || err->code == VIR_ERR_OK)
> +return 0;
> +return 1;
> +}

As you noted in the cover letter, ^this one is essentially identical to
virGetLastErrorCode, minus the "1 vs err->code" and since VIR_ERR_OK == 0, you
can use virGetLastErrorCode the same way as you used virHasLastError as a
replacement in many occurrences of virGetLastError(), so I don't see the need
for this function.

> +
> +
> +/**
> + * virGetLastErrorCode:
> + *
> + * Get the most recent error code
> + *
> + * The error object is kept in thread local storage, so separate
> + * threads can safely access this concurrently.
> + *
> + * Returns the most recent error code in this thread,
> + * or VIR_ERR_OK if none is set
> + */
> +int
> +virGetLastErrorCode(void)
> +{
> +virErrorPtr err = virLastErrorObject();
> +if (!err)
> +return VIR_ERR_OK;
> +return err->code;
> +}
> +
> +
> +/**
> + * virGetLastErrorDomain:
> + *
> + * Get the most recent error domain
> + *
> + * The error object is kept in thread local storage, so separate
> + * threads can safely access this concurrently.
> + *
> + * Returns the most recent error code in this thread,
> + * or VIR_FROM_NONE if none is set
> + */
> +int
> +virGetLastErrorDomain(void)
> +{
> +virErrorPtr err = virLastErrorObject();
> +if (!err)
> +return VIR_FROM_NONE;
> +return err->domain;
> +}

I can see a need for ^this function, even though we don't have an immediate use
case for it internally, it's still a public API which someone else might
consume, otherwise they'd have to query the object itself.

Erik

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 0/7] Couple of tiny fixes for virsh and translation.

[Peter Krempa]

On Fri, May 04, 2018 at 17:28:47 +0800, Lin Ma wrote:
> Lin Ma (7):
>   virsh: Error out while domain not found for 'qemu-monitor-event'
> command
>   virsh: Error out while domain not found for 'event' command
>   po: fix typo: remove a redundant Chinese character
>   virsh: Simplify control flow for 'desc' command
>   virsh: Simplify control flow for 'qemu-agent-command' command
>   qemu: stats: Display the net type and net source in bulk stats
>   qemu: stats: Display net count,type and source even if domain is
> inactive

Patches 1, 2, 4, and 5 are ACKed and pushed.


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [dbus PATCH 3/4] Annotate properties for which we will not emit changed signal

[Pavel Hrdina]

On Thu, May 03, 2018 at 03:21:01PM +0200, Katerina Koukiou wrote:
> On Thu, 2018-05-03 at 14:46 +0200, Pavel Hrdina wrote:
> > For some of these properties there is no libvirt event to detect the
> > change and for properties where we could somehow detect the change
> > let's annotate them as well.
> > 
> > We could change the properties to methods but with the annotation we
> > can keep them as properties in order to allow to get them by single
> > D-Bus call.
> > 
> > Signed-off-by: Pavel Hrdina 
> > ---
> >  data/org.libvirt.Connect.xml | 3 +++
> >  data/org.libvirt.Domain.xml  | 9 +
> >  data/org.libvirt.Network.xml | 3 +++
> >  3 files changed, 15 insertions(+)
> > 
> 
> Maybe it would be nicer to set `false` as the default annotation for
> EmitsChangedSignal in the enclosing interface element and explicitly
> mark the rest. But I am ok with this as well.

Good point, I missed that in D-Bus specification that the annotation can
be for the whole interface.  I'll sent v2.

Thanks,
Pavel


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [dbus PATCH] Implement SetGuestVcpus method for Domain Interface

[Pavel Hrdina]

On Wed, May 02, 2018 at 04:51:42PM +0200, Katerina Koukiou wrote:
> Signed-off-by: Katerina Koukiou 
> ---
>  data/org.libvirt.Domain.xml |  7 +
>  src/domain.c| 62 
> +
>  2 files changed, 69 insertions(+)
> 
> diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml
> index db43b1c..eae6d97 100644
> --- a/data/org.libvirt.Domain.xml
> +++ b/data/org.libvirt.Domain.xml
> @@ -463,6 +463,13 @@
>
>
>  
> +
> +   +  value="See 
> https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainSetGuestVcpus"/>
> +  
> +  
> +  
> +
>  
>  value="See 
> https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainSetInterfaceParameters"/>
> diff --git a/src/domain.c b/src/domain.c
> index e305fa3..bbc4ead 100644
> --- a/src/domain.c
> +++ b/src/domain.c
> @@ -72,6 +72,39 @@ VIRT_DBUS_ENUM_IMPL(virtDBusDomainMetadata,
>  "title",
>  "element")
>  
> +static gchar *
> +virtDBusDomainConvertBoolArrayToGuestVcpumap(GVariantIter *iter)
> +{
> +g_autoptr(GVariantIter) tmpIter = NULL;

tmpIter is not used

> +gboolean set;
> +gint intervalCnt = 0;
> +guint intervalStart = 0;
> +gboolean setPrev = 0;
> +g_autofree GString *ret = NULL;
> +
> +ret = g_string_new("");
> +for (guint i = 0; ; i++) {
> +gboolean next = g_variant_iter_loop(iter, "b", );

I would rename this variable to stop and invert the logic, the return
value tells you whether there was some value to unpack or not.

> +
> +if (set && !setPrev)
> +intervalStart = i;
> +else if ((!set && setPrev) || next) {

Here you should remove the '|| next' part, otherwise the 'else if'
section is called for every unset CPU in the array of boolean mask.

> +if (intervalCnt > 0)
> +g_string_append_printf(ret, ",");

I would change it to 'boolean first' instead of intervalCnt to make it
clear what's the purpose of this variable.

> +if (intervalStart != i - 1)
> +g_string_append_printf(ret, "%d-%d", intervalStart, i - 1);
> +else
> +g_string_append_printf(ret, "%d", intervalStart);
> +intervalCnt++;
> +}
> +setPrev = set;
> +if (!next)
> +break;
> +}
> +
> +return ret->str;
> +}
> +
>  struct _virtDBusDomainFSInfoList {
>  virDomainFSInfoPtr *info;
>  gint count;
> @@ -2490,6 +2523,34 @@ virtDBusDomainSetBlockIOTune(GVariant *inArgs,
>  }
>  }
>  
> +static void
> +virtDBusDomainSetGuestVcpus(GVariant *inArgs,
> +GUnixFDList *inFDs G_GNUC_UNUSED,
> +const gchar *objectPath,
> +gpointer userData,
> +GVariant **outArgs G_GNUC_UNUSED,
> +GUnixFDList **outFDs G_GNUC_UNUSED,
> +GError **error)
> +{
> +virtDBusConnect *connect = userData;
> +g_autoptr(virDomain) domain = NULL;
> +g_autoptr(GVariantIter) iter = NULL;
> +gint state;
> +guint flags;
> +g_autofree gchar *ret = NULL;

s/ret/cpumap/

Reviewed-by: Pavel Hrdina 

with the following diff applied, to make the suggested changes clear:

diff --git a/src/domain.c b/src/domain.c
index bbc4ead..8210a04 100644
--- a/src/domain.c
+++ b/src/domain.c
@@ -75,30 +75,32 @@ VIRT_DBUS_ENUM_IMPL(virtDBusDomainMetadata,
 static gchar *
 virtDBusDomainConvertBoolArrayToGuestVcpumap(GVariantIter *iter)
 {
-g_autoptr(GVariantIter) tmpIter = NULL;
 gboolean set;
-gint intervalCnt = 0;
+gboolean first = TRUE;
 guint intervalStart = 0;
 gboolean setPrev = 0;
 g_autofree GString *ret = NULL;

 ret = g_string_new("");
 for (guint i = 0; ; i++) {
-gboolean next = g_variant_iter_loop(iter, "b", );
+gboolean stop = !g_variant_iter_loop(iter, "b", );

-if (set && !setPrev)
+if (set && !setPrev) {
 intervalStart = i;
-else if ((!set && setPrev) || next) {
-if (intervalCnt > 0)
+} else if (!set && setPrev) {
+if (!first)
 g_string_append_printf(ret, ",");
+else
+first = FALSE;
+
 if (intervalStart != i - 1)
 g_string_append_printf(ret, "%d-%d", intervalStart, i - 1);
 else
 g_string_append_printf(ret, "%d", intervalStart);
-intervalCnt++;
 }
 setPrev = set;
-if (!next)
+
+if (stop)
 break;
 }

@@ -2537,7 +2539,7 @@ virtDBusDomainSetGuestVcpus(GVariant *inArgs,
 g_autoptr(GVariantIter) iter = NULL;
 gint state;
 guint flags;
-g_autofree gchar *ret = NULL;
+g_autofree gchar *cpumap = NULL;

 g_variant_get(inArgs, "(abiu)", , 

Re: [libvirt] [PATCH 09/12] virsh: Enable multiple --event flags to 'event' command

[Peter Krempa]

On Fri, May 04, 2018 at 17:25:30 +0800, Lin Ma wrote:
> Signed-off-by: Lin Ma 
> ---
>  tools/virsh-domain.c | 76 
> +++-
>  1 file changed, 39 insertions(+), 37 deletions(-)
> 
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 89aefbf86a..b35c9adaaa 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -13347,10 +13347,6 @@ static const vshCmdInfo info_event[] = {
>  static const vshCmdOptDef opts_event[] = {
>  VIRSH_COMMON_OPT_DOMAIN_OT_STRING(N_("filter by domain name, id or 
> uuid"),
>0),
> -{.name = "event",
> - .type = VSH_OT_STRING,
> - .help = N_("which event type to wait for")
> -},
>  {.name = "all",
>   .type = VSH_OT_BOOL,
>   .help = N_("wait for all events instead of just one type")
> @@ -13371,6 +13367,10 @@ static const vshCmdOptDef opts_event[] = {
>   .type = VSH_OT_BOOL,
>   .help = N_("show timestamp for each printed event")
>  },
> +{.name = "event",
> + .type = VSH_OT_ARGV,
> + .help = N_("which event type to wait for")
> +},
>  {.name = NULL}
>  };
>  
> @@ -13382,12 +13382,14 @@ cmdEvent(vshControl *ctl, const vshCmd *cmd)
>  int timeout = 0;
>  virshDomEventData *data = NULL;
>  size_t i;
> -const char *eventName = NULL;
> +int *eventIdxList = NULL;
> +size_t nevents = 0;
>  int event = -1;
>  bool all = vshCommandOptBool(cmd, "all");
>  bool loop = vshCommandOptBool(cmd, "loop");
>  bool timestamp = vshCommandOptBool(cmd, "timestamp");
>  int count = 0;
> +const vshCmdOpt *opt = NULL;
>  virshControlPtr priv = ctl->privData;
>  
>  if (vshCommandOptBool(cmd, "list")) {
> @@ -13396,15 +13398,25 @@ cmdEvent(vshControl *ctl, const vshCmd *cmd)
>  return true;
>  }
>  
> -if (vshCommandOptStringReq(ctl, cmd, "event", ) < 0)
> -return false;
> -if (eventName) {
> -for (event = 0; event < VIR_DOMAIN_EVENT_ID_LAST; event++)
> -if (STREQ(eventName, vshEventCallbacks[event].name))
> -break;
> -if (event == VIR_DOMAIN_EVENT_ID_LAST) {
> -vshError(ctl, _("unknown event type %s"), eventName);
> -return false;
> +if (vshCommandOptBool(cmd, "event")) {
> +if (VIR_ALLOC_N(eventIdxList, 1) < 0)
> +goto cleanup;

This is not necessary, VIR_APPEND_ELEMENT does that

> +nevents = 1;
> +while ((opt = vshCommandOptArgv(ctl, cmd, opt))) {
> +if (opt->data) {
> +for (event = 0; event < VIR_DOMAIN_EVENT_ID_LAST; event++)
> +if (STREQ(opt->data, vshEventCallbacks[event].name))
> +break;
> +if (event == VIR_DOMAIN_EVENT_ID_LAST) {
> +vshError(ctl, _("unknown event type %s"), opt->data);
> +return false;

This leaks the eventIdxList array. Also it would be preferrable just to
use one array for the case when events are enumerated and when all are
used ... 

> +}
> +if (VIR_INSERT_ELEMENT(eventIdxList,

Use VIR_APPEND_ELEMENT instead.

> +   nevents - 1,
> +   nevents,
> +   event) < 0)
> +goto cleanup;
> +}
>  }
>  } else if (!all) {
>  vshError(ctl, "%s",
> @@ -13412,26 +13424,15 @@ cmdEvent(vshControl *ctl, const vshCmd *cmd)
>  return false;
>  }
>  
> -if (all) {
> -if (VIR_ALLOC_N(data, VIR_DOMAIN_EVENT_ID_LAST) < 0)
> -goto cleanup;
> -for (i = 0; i < VIR_DOMAIN_EVENT_ID_LAST; i++) {
> -data[i].ctl = ctl;
> -data[i].loop = loop;
> -data[i].count = 
> -data[i].timestamp = timestamp;
> -data[i].cb = [i];
> -data[i].id = -1;
> -}
> -} else {
> -if (VIR_ALLOC_N(data, 1) < 0)
> -goto cleanup;
> -data[0].ctl = ctl;
> -data[0].loop = vshCommandOptBool(cmd, "loop");
> -data[0].count = 
> -data[0].timestamp = timestamp;
> -data[0].cb = [event];
> -data[0].id = -1;
> +if (VIR_ALLOC_N(data, all ? VIR_DOMAIN_EVENT_ID_LAST : nevents - 1) < 0)
> +goto cleanup;
> +for (i = 0; i < (all ? VIR_DOMAIN_EVENT_ID_LAST : nevents - 1); i++) {
> +data[i].ctl = ctl;
> +data[i].loop = loop;
> +data[i].count = 
> +data[i].timestamp = timestamp;
> +data[i].cb = [all ? i : eventIdxList[i]];

No ternaries, please use the same array, just fill it differently.

> +data[i].id = -1;

You can refactor the above to use VIR_APPEND_ELEMENT too so that the
same array can be used.

>  }
>  if (vshCommandOptTimeoutToMs(ctl, cmd, ) < 0)
>  goto cleanup;


signature.asc

Re: [libvirt] [PATCH 02/12] virsh: Conditionally Ignore the first entry in list of completions

[Peter Krempa]

On Fri, May 04, 2018 at 17:25:23 +0800, Lin Ma wrote:
> The first entry in the returned array is the substitution for TEXT. It
> causes unnecessary output if other commands or options share the same
> prefix, e.g.
> 
> $ virsh des
> des  desc destroy
> 
> or
> 
> $ virsh domblklist --d
> --d--details  --domain
> 
> This patch fixes the above issue.
> 
> Signed-off-by: Lin Ma 
> ---
>  tools/vsh.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/vsh.c b/tools/vsh.c
> index 73ec007e56..38058c874a 100644
> --- a/tools/vsh.c
> +++ b/tools/vsh.c
> @@ -3458,6 +3458,7 @@ cmdComplete(vshControl *ctl, const vshCmd *cmd)
>  const vshCmdOpt *opt = NULL;
>  char **matches = NULL, **iter;
>  virBuffer buf = VIR_BUFFER_INITIALIZER;
> +int n;
>  
>  if (vshCommandOptStringQuiet(ctl, cmd, "string", ) <= 0)
>  goto cleanup;
> @@ -3493,8 +3494,11 @@ cmdComplete(vshControl *ctl, const vshCmd *cmd)
>  if (!(matches = vshReadlineCompletion(arg, 0, 0)))
>  goto cleanup;
>  
> -for (iter = matches; *iter; iter++)
> +for (n =0, iter = matches; *iter; iter++, n++) {

Fails 'make syntax-check':

Spacing around '=' or '==':
tools/vsh.c:3499: for (n =0, iter = matches; *iter; iter++, n++) {
maint.mk: incorrect formatting


> +if (n == 0 && matches[1])
> +continue;
>  printf("%s\n", *iter);
> +}
>  
>  ret = true;
>   cleanup:
> -- 
> 2.15.1
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 03/12] virsh: Create macros for VSH_OT_STRING "domain" option

[Peter Krempa]

On Fri, May 04, 2018 at 17:25:24 +0800, Lin Ma wrote:
> Signed-off-by: Lin Ma 
> ---
>  tools/virsh-domain-monitor.c | 3 +++
>  tools/virsh-domain.c | 3 +++
>  tools/virsh-snapshot.c   | 3 +++
>  tools/virsh.h| 8 
>  4 files changed, 17 insertions(+)
> 
> diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
> index 8e071779b4..8ad651626b 100644
> --- a/tools/virsh-domain-monitor.c
> +++ b/tools/virsh-domain-monitor.c
> @@ -43,6 +43,9 @@
>  #define VIRSH_COMMON_OPT_DOMAIN_FULL(cflags) \
>  VIRSH_COMMON_OPT_DOMAIN(N_("domain name, id or uuid"), cflags)
>  
> +#define VIRSH_COMMON_OPT_DOMAIN_OT_STRING_FULL(cflags) \
> +VIRSH_COMMON_OPT_DOMAIN_OT_STRING(N_("domain name, id or uuid"), cflags)
> +
>  VIR_ENUM_DECL(virshDomainIOError)
>  VIR_ENUM_IMPL(virshDomainIOError,
>VIR_DOMAIN_DISK_ERROR_LAST,
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 6601f94a12..8a63761fab 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -68,6 +68,9 @@
>  #define VIRSH_COMMON_OPT_DOMAIN_FULL(cflags) \
>  VIRSH_COMMON_OPT_DOMAIN(N_("domain name, id or uuid"), cflags)
>  
> +#define VIRSH_COMMON_OPT_DOMAIN_OT_STRING_FULL(cflags) \
> +VIRSH_COMMON_OPT_DOMAIN_OT_STRING(N_("domain name, id or uuid"), cflags)
> +
>  #define VIRSH_COMMON_OPT_DOMAIN_PERSISTENT \
>  {.name = "persistent", \
>   .type = VSH_OT_BOOL, \
> diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
> index e4908eea70..3d86ac84d1 100644
> --- a/tools/virsh-snapshot.c
> +++ b/tools/virsh-snapshot.c
> @@ -45,6 +45,9 @@
>  #define VIRSH_COMMON_OPT_DOMAIN_FULL(cflags) \
>  VIRSH_COMMON_OPT_DOMAIN(N_("domain name, id or uuid"), cflags)
>  
> +#define VIRSH_COMMON_OPT_DOMAIN_OT_STRING_FULL(cflags) \
> +VIRSH_COMMON_OPT_DOMAIN_OT_STRING(N_("domain name, id or uuid"), cflags)

Only one of the three definitions above is actually used ...

> +
>  /* Helper for snapshot-create and snapshot-create-as */
>  static bool
>  virshSnapshotCreate(vshControl *ctl, virDomainPtr dom, const char *buffer,
> diff --git a/tools/virsh.h b/tools/virsh.h
> index f2213ebb57..9dcf104cc4 100644
> --- a/tools/virsh.h
> +++ b/tools/virsh.h
> @@ -107,6 +107,14 @@
>   .help = _helpstr \
>  }

... since all of them share the docs string, why isn't it declared here?

>  
> +# define VIRSH_COMMON_OPT_DOMAIN_OT_STRING(_helpstr, cflags) \
> +{.name = "domain", \
> + .type = VSH_OT_STRING, \
> + .help = _helpstr, \
> + .completer = virshDomainNameCompleter, \
> + .completer_flags = cflags, \
> +}
> +
>  typedef struct _virshControl virshControl;
>  typedef virshControl *virshControlPtr;
>  
> -- 
> 2.15.1
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 06/12] virsh: Create macros for VSH_OT_ARGV "domain" option

[Peter Krempa]

On Fri, May 04, 2018 at 17:25:27 +0800, Lin Ma wrote:
> Signed-off-by: Lin Ma 
> ---
>  tools/virsh-domain-monitor.c | 3 +++
>  tools/virsh-domain.c | 3 +++
>  tools/virsh-snapshot.c   | 3 +++
>  tools/virsh.h| 9 +
>  4 files changed, 18 insertions(+)
> 
> diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
> index 8ad651626b..e4a21534cb 100644
> --- a/tools/virsh-domain-monitor.c
> +++ b/tools/virsh-domain-monitor.c
> @@ -46,6 +46,9 @@
>  #define VIRSH_COMMON_OPT_DOMAIN_OT_STRING_FULL(cflags) \
>  VIRSH_COMMON_OPT_DOMAIN_OT_STRING(N_("domain name, id or uuid"), cflags)
>  
> +#define VIRSH_COMMON_OPT_DOMAIN_OT_ARGV_FULL(cflags) \
> +VIRSH_COMMON_OPT_DOMAIN_OT_ARGV(N_("domain name, id or uuid"), cflags)

All of these defined here are unused (even in the other patches in this
series), so what's the point of adding them?

> +
>  VIR_ENUM_DECL(virshDomainIOError)
>  VIR_ENUM_IMPL(virshDomainIOError,
>VIR_DOMAIN_DISK_ERROR_LAST,
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 689f9d686b..89aefbf86a 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -71,6 +71,9 @@
>  #define VIRSH_COMMON_OPT_DOMAIN_OT_STRING_FULL(cflags) \
>  VIRSH_COMMON_OPT_DOMAIN_OT_STRING(N_("domain name, id or uuid"), cflags)
>  
> +#define VIRSH_COMMON_OPT_DOMAIN_OT_ARGV_FULL(cflags) \
> +VIRSH_COMMON_OPT_DOMAIN_OT_ARGV(N_("domain name, id or uuid"), cflags)
> +
>  #define VIRSH_COMMON_OPT_DOMAIN_PERSISTENT \
>  {.name = "persistent", \
>   .type = VSH_OT_BOOL, \
> diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
> index 3d86ac84d1..0c86b6c950 100644
> --- a/tools/virsh-snapshot.c
> +++ b/tools/virsh-snapshot.c
> @@ -48,6 +48,9 @@
>  #define VIRSH_COMMON_OPT_DOMAIN_OT_STRING_FULL(cflags) \
>  VIRSH_COMMON_OPT_DOMAIN_OT_STRING(N_("domain name, id or uuid"), cflags)
>  
> +#define VIRSH_COMMON_OPT_DOMAIN_OT_ARGV_FULL(cflags) \
> +VIRSH_COMMON_OPT_DOMAIN_OT_ARGV(N_("domain name, id or uuid"), cflags)
> +
>  /* Helper for snapshot-create and snapshot-create-as */

Also all of them have the same docs string so I really don't know why we
need to define them in every file separately ...

>  static bool
>  virshSnapshotCreate(vshControl *ctl, virDomainPtr dom, const char *buffer,
> diff --git a/tools/virsh.h b/tools/virsh.h
> index 9dcf104cc4..a33d108b2d 100644
> --- a/tools/virsh.h
> +++ b/tools/virsh.h
> @@ -115,6 +115,15 @@
>   .completer_flags = cflags, \
>  }
>  
> +# define VIRSH_COMMON_OPT_DOMAIN_OT_ARGV(_helpstr, cflags) \
> +{.name = "domain", \
> + .type = VSH_OT_ARGV, \
> + .flags = VSH_OFLAG_NONE, \
> + .help = _helpstr, \
> + .completer = virshDomainNameCompleter, \
> + .completer_flags = cflags, \
> +}
> +
>  typedef struct _virshControl virshControl;
>  typedef virshControl *virshControlPtr;
>  
> -- 
> 2.15.1
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 12/12] virsh: Add event name completion to 'event' command

[Peter Krempa]

On Fri, May 04, 2018 at 17:25:33 +0800, Lin Ma wrote:
> Signed-off-by: Lin Ma 
> ---
>  tools/virsh-completer.c | 36 
>  tools/virsh-completer.h |  3 +++
>  tools/virsh-domain.c|  1 +
>  3 files changed, 40 insertions(+)
> 
> diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
> index e3b8234b41..a188a8d7ab 100644
> --- a/tools/virsh-completer.c
> +++ b/tools/virsh-completer.c
> @@ -30,6 +30,7 @@
>  #include "viralloc.h"
>  #include "virstring.h"
>  #include "virxml.h"
> +extern const char *virDomainEventGetName(int event);

Please don't use externs. If you need the function export it.

>  
>  
>  char **
> @@ -522,3 +523,38 @@ virshSnapshotNameCompleter(vshControl *ctl,
>  virshDomainFree(dom);
>  return NULL;
>  }
> +
> +
> +char **
> +virshEventNameCompleter(vshControl *ctl,
> +const vshCmd *cmd ATTRIBUTE_UNUSED,
> +unsigned int flags)
> +{
> +virshControlPtr priv = ctl->privData;
> +size_t i = 0;
> +char **ret = NULL;
> +
> +virCheckFlags(0, NULL);
> +
> +if (!priv->conn || virConnectIsAlive(priv->conn) <= 0)
> +return NULL;
> +
> +if (VIR_ALLOC_N(ret, VIR_DOMAIN_EVENT_ID_LAST + 1) < 0)
> +goto error;
> +
> +for (i = 0; i < VIR_DOMAIN_EVENT_ID_LAST; i++) {
> +const char *name = virDomainEventGetName(i);
> +
> +if (name == NULL)
> +goto error;
> +
> +if (VIR_STRDUP(ret[i], name) < 0)
> +goto error;
> +}
> +
> +return ret;
> +
> +error:
> +VIR_FREE(ret);

This does not free the members in ret allocated by the strdup above.

> +return NULL;
> +}


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 11/12] domain_event: Introduce function virDomainEventGetName

[Peter Krempa]

On Fri, May 04, 2018 at 17:25:32 +0800, Lin Ma wrote:
> It will be used in next patch for event name completion.
> 
> Signed-off-by: Lin Ma 
> ---
>  src/conf/domain_event.c  | 62 
> 
>  src/conf/domain_event.h  |  3 +++
>  src/libvirt_private.syms |  1 +
>  3 files changed, 66 insertions(+)
> 
> diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
> index 97520706c9..f8bd457b34 100644
> --- a/src/conf/domain_event.c
> +++ b/src/conf/domain_event.c
> @@ -1968,6 +1968,68 @@ virDomainEventDispatchDefaultFunc(virConnectPtr conn,
>  }
>  
>  
> +const char *
> +virDomainEventGetName(int event)
> +{
> +virResetLastError();

This should not be necessary. This function shall not touch the error
since it does not report any. The caller needs to handle it properly.

> +
> +switch (event) {

Typecast event to the proper type so that compiler moans when new events
are added.

> +case VIR_DOMAIN_EVENT_ID_LIFECYCLE:
> +return VIR_DOMAIN_EVENT_LIFECYCLE;
> +case VIR_DOMAIN_EVENT_ID_REBOOT:
> +return VIR_DOMAIN_EVENT_REBOOT;

[...]


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [dbus PATCH v2] Implement SetGuestVcpus method for Domain Interface

[Katerina Koukiou]

Signed-off-by: Katerina Koukiou 
---
Fixed conditions since v1 was creating incorrect vcpumap.
If all vcpus will be passed False the API will not work, since '' is not
valid cpumap but this scenario should not be used.

 data/org.libvirt.Domain.xml |  7 +
 src/domain.c| 62 +
 2 files changed, 69 insertions(+)

diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml
index db43b1c..eae6d97 100644
--- a/data/org.libvirt.Domain.xml
+++ b/data/org.libvirt.Domain.xml
@@ -463,6 +463,13 @@
   
   
 
+
+  https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainSetGuestVcpus"/>
+  
+  
+  
+
 
   https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainSetInterfaceParameters"/>
diff --git a/src/domain.c b/src/domain.c
index e305fa3..28cbe76 100644
--- a/src/domain.c
+++ b/src/domain.c
@@ -72,6 +72,39 @@ VIRT_DBUS_ENUM_IMPL(virtDBusDomainMetadata,
 "title",
 "element")
 
+static gchar *
+virtDBusDomainConvertBoolArrayToGuestVcpumap(GVariantIter *iter)
+{
+g_autoptr(GVariantIter) tmpIter = NULL;
+gint intervalCnt = 0;
+guint intervalStart = 0;
+gboolean set;
+gboolean setPrev = 0;
+g_autofree GString *ret = NULL;
+
+ret = g_string_new("");
+for (guint i = 0; ; i++) {
+gboolean next = g_variant_iter_loop(iter, "b", );
+
+if (next && set && !setPrev)
+intervalStart = i;
+else if ((!next && setPrev) || (!set && setPrev)) {
+if (intervalCnt > 0)
+g_string_append_printf(ret, ",");
+if (intervalStart != i - 1)
+g_string_append_printf(ret, "%d-%d", intervalStart, i - 1);
+else
+g_string_append_printf(ret, "%d", intervalStart);
+intervalCnt++;
+}
+setPrev = set;
+if (!next)
+break;
+}
+
+return ret->str;
+}
+
 struct _virtDBusDomainFSInfoList {
 virDomainFSInfoPtr *info;
 gint count;
@@ -2490,6 +2523,34 @@ virtDBusDomainSetBlockIOTune(GVariant *inArgs,
 }
 }
 
+static void
+virtDBusDomainSetGuestVcpus(GVariant *inArgs,
+GUnixFDList *inFDs G_GNUC_UNUSED,
+const gchar *objectPath,
+gpointer userData,
+GVariant **outArgs G_GNUC_UNUSED,
+GUnixFDList **outFDs G_GNUC_UNUSED,
+GError **error)
+{
+virtDBusConnect *connect = userData;
+g_autoptr(virDomain) domain = NULL;
+g_autoptr(GVariantIter) iter = NULL;
+gint state;
+guint flags;
+g_autofree gchar *ret = NULL;
+
+g_variant_get(inArgs, "(abiu)", , , );
+
+domain = virtDBusDomainGetVirDomain(connect, objectPath, error);
+if (!domain)
+return;
+
+ret = virtDBusDomainConvertBoolArrayToGuestVcpumap(iter);
+
+if (virDomainSetGuestVcpus(domain, ret, state, flags) < 0)
+virtDBusUtilSetLastVirtError(error);
+}
+
 static void
 virtDBusDomainSetInterfaceParameters(GVariant *inArgs,
  GUnixFDList *inFDs G_GNUC_UNUSED,
@@ -2988,6 +3049,7 @@ static virtDBusGDBusMethodTable 
virtDBusDomainMethodTable[] = {
 { "SendProcessSignal", virtDBusDomainSendProcessSignal },
 { "SetBlkioParameters", virtDBusDomainSetBlkioParameters },
 { "SetBlockIOTune", virtDBusDomainSetBlockIOTune },
+{ "SetGuestVcpus", virtDBusDomainSetGuestVcpus },
 { "SetInterfaceParameters", virtDBusDomainSetInterfaceParameters },
 { "SetVcpus", virtDBusDomainSetVcpus },
 { "SetMemory", virtDBusDomainSetMemory },
-- 
2.15.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 10/12] virsh: Introduce some VIR_DOMAIN_EVENT_* macros representing domain event names

[Peter Krempa]

On Fri, May 04, 2018 at 17:25:31 +0800, Lin Ma wrote:
> Signed-off-by: Lin Ma 
> ---
>  include/libvirt/libvirt-domain.h | 201 
> +++
>  tools/virsh-domain.c |  50 +-
>  2 files changed, 226 insertions(+), 25 deletions(-)
> 
> diff --git a/include/libvirt/libvirt-domain.h 
> b/include/libvirt/libvirt-domain.h
> index 12fd34037e..f7dd510f7f 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -4424,6 +4424,207 @@ typedef enum {
>  } virDomainEventID;
>  
>  
> +/**
> + * VIR_DOMAIN_EVENT_LIFECYCLE:
> + *
> + * Macro for the event name "lifecycle"
> + */
> +
> +# define VIR_DOMAIN_EVENT_LIFECYCLE "lifecycle"

This would make all of these public API and I'm not persuaded this is a
good idea. Specifically since they are only used in virsh. The library
always refers to the events by number. Also some of the events have
multiple implementations which don't usually need to be referred to
publically depending on the use case of the event. Virsh implements all
of them just for the sake of completness.

NACK



signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 7/7] qemu: stats: Display net count, type and source even if domain is inactive

[Peter Krempa]

On Fri, May 04, 2018 at 17:28:54 +0800, Lin Ma wrote:
> Signed-off-by: Lin Ma 
> ---
>  src/qemu/qemu_driver.c | 15 ---
>  1 file changed, 8 insertions(+), 7 deletions(-)

The network is not enabled so there's no stats to report when the VM is
offline so I don't think this is justified.


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 3/7] po: fix typo: remove a redundant Chinese character

[Daniel P . Berrangé]

On Fri, May 04, 2018 at 12:07:39PM +0200, Peter Krempa wrote:
> On Fri, May 04, 2018 at 17:28:50 +0800, Lin Ma wrote:
> > Signed-off-by: Lin Ma 
> > ---
> 
> NACK
> 
> This needs to be fixed in Zanata where we pull the translations from
> since it would be overwritten by the next sync.

To save time, I will fix this mistake in Zanata, so it gets synced.


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 6/7] qemu: stats: Display the net type and net source in bulk stats

[Peter Krempa]

On Fri, May 04, 2018 at 17:28:53 +0800, Lin Ma wrote:
> Signed-off-by: Lin Ma 
> ---

Could you justify this? This is really configuration and will not change
during the lifetime of the interface, thus there's no pressing need to
report it in the stats which should report only data which is changing.


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 3/7] po: fix typo: remove a redundant Chinese character

[Peter Krempa]

On Fri, May 04, 2018 at 17:28:50 +0800, Lin Ma wrote:
> Signed-off-by: Lin Ma 
> ---

NACK

This needs to be fixed in Zanata where we pull the translations from
since it would be overwritten by the next sync.


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [dbus PATCH] Add detail argument to DomainEvent signal

[Katerina Koukiou]

Adjust all DomainEvent tests to do detail type checking.

Signed-off-by: Katerina Koukiou 
---
This commit is rebased on top of unmerged patches for removing enum<->string
translation.

 data/org.libvirt.Connect.xml |  1 +
 src/events.c |  4 ++--
 tests/libvirttest.py | 55 
 tests/test_connect.py|  6 +++--
 tests/test_domain.py | 15 
 5 files changed, 72 insertions(+), 9 deletions(-)

diff --git a/data/org.libvirt.Connect.xml b/data/org.libvirt.Connect.xml
index 8272da6..0f1456f 100644
--- a/data/org.libvirt.Connect.xml
+++ b/data/org.libvirt.Connect.xml
@@ -171,6 +171,7 @@
 value="See 
https://libvirt.org/html/libvirt-libvirt-domain.html#virConnectDomainEventCallback"/>
   
   
+  
 
 
   connectPath,
   VIRT_DBUS_CONNECT_INTERFACE,
   "DomainEvent",
-  g_variant_new("(ou)", path, event),
+  g_variant_new("(ouu)", path, event, detail),
   NULL);
 
 return 0;
diff --git a/tests/libvirttest.py b/tests/libvirttest.py
index 06ac0e4..eee67a0 100644
--- a/tests/libvirttest.py
+++ b/tests/libvirttest.py
@@ -100,6 +100,61 @@ class DomainEvent(IntEnum):
 CRASHED = 8
 
 
+class DomainEventDefinedDetailType(IntEnum):
+ADDED = 0
+UPDATED = 1
+RENAMED = 2
+FROM_SNAPSHOT = 3
+LAST = 4
+
+
+class DomainEventResumedDetailType(IntEnum):
+UNPAUSED = 0
+MIGRATED = 1
+FROM_SNAPSHOT = 2
+POSTCOPY = 3
+LAST = 4
+
+
+class DomainEventStartedDetailType(IntEnum):
+BOOTED = 0
+MIGRATED = 1
+RESTORED = 2
+FROM_SNAPSHOT = 3
+WAKEUP = 4
+LAST = 5
+
+
+class DomainEventStoppedDetailType(IntEnum):
+SHUTDOWN = 0
+DESTROYED = 1
+CRASHED = 2
+MIGRATED = 3
+SAVED = 4
+FAILED = 5
+FROM_SNAPSHOT = 6
+LAST = 7
+
+
+class DomainEventSuspendedDetailType(IntEnum):
+PAUSED = 0
+MIGRATED   = 1
+IOERROR = 2
+WATCHDOG   = 3
+RESTORED = 4
+FROM_SNAPSHOT = 5
+API_ERROR = 6
+POSTCOPY = 7
+POSTCOPY_FAILED = 8
+LAST = 9
+
+
+class DomainEventUndefinedDetailType(IntEnum):
+REMOVED = 0
+RENAMED = 1
+LAST = 2
+
+
 class DomainState(IntEnum):
 NOSTATE = 0
 RUNNING = 1
diff --git a/tests/test_connect.py b/tests/test_connect.py
index 7748822..a2bd17f 100755
--- a/tests/test_connect.py
+++ b/tests/test_connect.py
@@ -31,9 +31,10 @@ class TestConnect(libvirttest.BaseTestClass):
 '''
 
 def test_connect_domain_create_xml(self):
-def domain_started(path, event):
+def domain_started(path, event, detail):
 if event != libvirttest.DomainEvent.STARTED:
 return
+assert detail == libvirttest.DomainEventStartedDetailType.BOOTED
 assert isinstance(path, dbus.ObjectPath)
 self.loop.quit()
 
@@ -45,9 +46,10 @@ class TestConnect(libvirttest.BaseTestClass):
 self.main_loop()
 
 def test_comnect_domain_define_xml(self):
-def domain_defined(path, event):
+def domain_defined(path, event, detail):
 if event != libvirttest.DomainEvent.DEFINED:
 return
+assert detail == libvirttest.DomainEventDefinedDetailType.ADDED
 assert isinstance(path, dbus.ObjectPath)
 self.loop.quit()
 
diff --git a/tests/test_domain.py b/tests/test_domain.py
index c7e09cd..dfa19ed 100755
--- a/tests/test_domain.py
+++ b/tests/test_domain.py
@@ -47,9 +47,10 @@ class TestDomain(libvirttest.BaseTestClass):
 assert autostart_current == dbus.Boolean(autostart_expected)
 
 def test_domain_managed_save(self):
-def domain_stopped(path, event):
+def domain_stopped(path, event, detail):
 if event != libvirttest.DomainEvent.STOPPED:
 return
+assert detail == libvirttest.DomainEventStoppedDetailType.SAVED
 assert isinstance(path, dbus.ObjectPath)
 self.loop.quit()
 
@@ -74,9 +75,10 @@ class TestDomain(libvirttest.BaseTestClass):
 assert description_expected == 
domain.GetMetadata(metadata_description, "", 0)
 
 def test_resume(self):
-def domain_resumed(path, event):
+def domain_resumed(path, event, detail):
 if event != libvirttest.DomainEvent.RESUMED:
 return
+assert detail == libvirttest.DomainEventResumedDetailType.UNPAUSED
 assert isinstance(path, dbus.ObjectPath)
 self.loop.quit()
 
@@ -92,9 +94,10 @@ class TestDomain(libvirttest.BaseTestClass):
 self.main_loop()
 
 def test_shutdown(self):
-def domain_stopped(path, event):
+def domain_stopped(path, event, detail):
 if event != libvirttest.DomainEvent.STOPPED:
 return
+

[libvirt] [PATCH 5/7] virsh: Simplify control flow for 'qemu-agent-command' command

[Lin Ma]

Signed-off-by: Lin Ma 
---
 tools/virsh-domain.c | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 65170225a7..598d2fa4a4 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9796,19 +9796,17 @@ cmdQemuAgentCommand(vshControl *ctl, const vshCmd *cmd)
 unsigned int flags = 0;
 const vshCmdOpt *opt = NULL;
 virBuffer buf = VIR_BUFFER_INITIALIZER;
-bool pad = false;
 virJSONValuePtr pretty = NULL;
 
 dom = virshCommandOptDomain(ctl, cmd, NULL);
 if (dom == NULL)
 goto cleanup;
 
-while ((opt = vshCommandOptArgv(ctl, cmd, opt))) {
-if (pad)
-virBufferAddChar(, ' ');
-pad = true;
-virBufferAdd(, opt->data, -1);
-}
+while ((opt = vshCommandOptArgv(ctl, cmd, opt)))
+virBufferAsprintf(, "%s ", opt->data);
+
+virBufferTrim(, " ", -1);
+
 if (virBufferError()) {
 vshError(ctl, "%s", _("Failed to collect command"));
 goto cleanup;
-- 
2.15.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 0/7] Couple of tiny fixes for virsh and translation.

[Lin Ma]

Lin Ma (7):
  virsh: Error out while domain not found for 'qemu-monitor-event'
command
  virsh: Error out while domain not found for 'event' command
  po: fix typo: remove a redundant Chinese character
  virsh: Simplify control flow for 'desc' command
  virsh: Simplify control flow for 'qemu-agent-command' command
  qemu: stats: Display the net type and net source in bulk stats
  qemu: stats: Display net count,type and source even if domain is
inactive

 po/zh_CN.mini.po   |  2 +-
 src/libvirt-domain.c   |  2 ++
 src/qemu/qemu_driver.c | 11 ---
 tools/virsh-domain.c   | 33 -
 tools/virsh.pod|  2 ++
 5 files changed, 29 insertions(+), 21 deletions(-)

-- 
2.15.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 6/7] qemu: stats: Display the net type and net source in bulk stats

[Lin Ma]

Signed-off-by: Lin Ma 
---
 src/libvirt-domain.c   | 2 ++
 src/qemu/qemu_driver.c | 4 
 tools/virsh.pod| 2 ++
 3 files changed, 8 insertions(+)

diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 2d86e48979..e317ca00d0 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -11310,6 +11310,8 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
  * "net.count" - number of network interfaces on this domain
  *   as unsigned int.
  * "net..name" - name of the interface  as string.
+ * "net..type" - type of the interface  as string.
+ * "net..source" - source of the interface  as string.
  * "net..rx.bytes" - bytes received as unsigned long long.
  * "net..rx.pkts" - packets received as unsigned long long.
  * "net..rx.errs" - receive errors as unsigned long long.
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 83fc191085..7a9a2bcf97 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19823,6 +19823,10 @@ qemuDomainGetStatsInterface(virQEMUDriverPtr driver 
ATTRIBUTE_UNUSED,
 
 QEMU_ADD_NAME_PARAM(record, maxparams,
 "net", "name", i, net->ifname);
+QEMU_ADD_NAME_PARAM(record, maxparams,
+"net", "type", i, 
virDomainNetTypeToString(net->type));
+QEMU_ADD_NAME_PARAM(record, maxparams,
+"net", "source", i, net->data.bridge.brname);
 
 if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER) {
 if (virNetDevOpenvswitchInterfaceStats(net->ifname, ) < 0) {
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 929958a953..7ec57c0b4b 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -974,6 +974,8 @@ I<--interface> returns:
 
  "net.count" - number of network interfaces on this domain
  "net..name" - name of the interface 
+ "net..type" - type of the interface 
+ "net..source" - source of the interface 
  "net..rx.bytes" - number of bytes received
  "net..rx.pkts" - number of packets received
  "net..rx.errs" - number of receive errors
-- 
2.15.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 1/7] virsh: Error out while domain not found for 'qemu-monitor-event' command

[Lin Ma]

Signed-off-by: Lin Ma 
---
 tools/virsh-domain.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 2b775fc4cc..3b2a34f936 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9661,7 +9661,9 @@ cmdQemuMonitorEvent(vshControl *ctl, const vshCmd *cmd)
 return false;
 
 if (vshCommandOptBool(cmd, "domain"))
-dom = virshCommandOptDomain(ctl, cmd, NULL);
+if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
+goto cleanup;
+
 if (vshEventStart(ctl, timeout) < 0)
 goto cleanup;
 
-- 
2.15.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 4/7] virsh: Simplify control flow for 'desc' command

[Lin Ma]

Just like the commit 8941c800, It does the similar thing.

Signed-off-by: Lin Ma 
---
 tools/virsh-domain.c | 13 +
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 1f3ea0c939..65170225a7 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -8330,7 +8330,6 @@ cmdDesc(vshControl *ctl, const vshCmd *cmd)
 char *tmpstr;
 const vshCmdOpt *opt = NULL;
 virBuffer buf = VIR_BUFFER_INITIALIZER;
-bool pad = false;
 bool ret = false;
 unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
 
@@ -8348,18 +8347,16 @@ cmdDesc(vshControl *ctl, const vshCmd *cmd)
 if ((state = virshDomainState(ctl, dom, NULL)) < 0)
 goto cleanup;
 
-while ((opt = vshCommandOptArgv(ctl, cmd, opt))) {
-if (pad)
-virBufferAddChar(, ' ');
-pad = true;
-virBufferAdd(, opt->data, -1);
-}
-
 if (title)
 type = VIR_DOMAIN_METADATA_TITLE;
 else
 type = VIR_DOMAIN_METADATA_DESCRIPTION;
 
+while ((opt = vshCommandOptArgv(ctl, cmd, opt)))
+virBufferAsprintf(, "%s ", opt->data);
+
+virBufferTrim(, " ", -1);
+
 if (virBufferError()) {
 vshError(ctl, "%s", _("Failed to collect new description/title"));
 goto cleanup;
-- 
2.15.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 2/7] virsh: Error out while domain not found for 'event' command

[Lin Ma]

Signed-off-by: Lin Ma 
---
 tools/virsh-domain.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 3b2a34f936..1f3ea0c939 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -13445,7 +13445,9 @@ cmdEvent(vshControl *ctl, const vshCmd *cmd)
 goto cleanup;
 
 if (vshCommandOptBool(cmd, "domain"))
-dom = virshCommandOptDomain(ctl, cmd, NULL);
+if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
+goto cleanup;
+
 if (vshEventStart(ctl, timeout) < 0)
 goto cleanup;
 
-- 
2.15.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 3/7] po: fix typo: remove a redundant Chinese character

[Lin Ma]

Signed-off-by: Lin Ma 
---
 po/zh_CN.mini.po | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/po/zh_CN.mini.po b/po/zh_CN.mini.po
index c5574cf1df..0e47184c93 100644
--- a/po/zh_CN.mini.po
+++ b/po/zh_CN.mini.po
@@ -15340,7 +15340,7 @@ msgid "entry was missing 'type'"
 msgstr "条目缺少 'type' "
 
 msgid "enumerate devices on this host"
-msgstr "这台主机中中的枚举设备"
+msgstr "这台主机中的枚举设备"
 
 msgid "error"
 msgstr "错误"
-- 
2.15.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 7/7] qemu: stats: Display net count, type and source even if domain is inactive

[Lin Ma]

Signed-off-by: Lin Ma 
---
 src/qemu/qemu_driver.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7a9a2bcf97..33ae68129d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19804,9 +19804,6 @@ qemuDomainGetStatsInterface(virQEMUDriverPtr driver 
ATTRIBUTE_UNUSED,
 struct _virDomainInterfaceStats tmp;
 int ret = -1;
 
-if (!virDomainObjIsActive(dom))
-return 0;
-
 QEMU_ADD_COUNT_PARAM(record, maxparams, "net", dom->def->nnets);
 
 /* Check the path is one of the domain's network interfaces. */
@@ -19814,6 +19811,14 @@ qemuDomainGetStatsInterface(virQEMUDriverPtr driver 
ATTRIBUTE_UNUSED,
 virDomainNetDefPtr net = dom->def->nets[i];
 virDomainNetType actualType;
 
+QEMU_ADD_NAME_PARAM(record, maxparams,
+"net", "type", i, 
virDomainNetTypeToString(net->type));
+QEMU_ADD_NAME_PARAM(record, maxparams,
+"net", "source", i, net->data.bridge.brname);
+
+if (!virDomainObjIsActive(dom))
+return 0;
+
 if (!net->ifname)
 continue;
 
@@ -19823,10 +19828,6 @@ qemuDomainGetStatsInterface(virQEMUDriverPtr driver 
ATTRIBUTE_UNUSED,
 
 QEMU_ADD_NAME_PARAM(record, maxparams,
 "net", "name", i, net->ifname);
-QEMU_ADD_NAME_PARAM(record, maxparams,
-"net", "type", i, 
virDomainNetTypeToString(net->type));
-QEMU_ADD_NAME_PARAM(record, maxparams,
-"net", "source", i, net->data.bridge.brname);
 
 if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER) {
 if (virNetDevOpenvswitchInterfaceStats(net->ifname, ) < 0) {
-- 
2.15.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 12/12] virsh: Add event name completion to 'event' command

[Lin Ma]

Signed-off-by: Lin Ma 
---
 tools/virsh-completer.c | 36 
 tools/virsh-completer.h |  3 +++
 tools/virsh-domain.c|  1 +
 3 files changed, 40 insertions(+)

diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
index e3b8234b41..a188a8d7ab 100644
--- a/tools/virsh-completer.c
+++ b/tools/virsh-completer.c
@@ -30,6 +30,7 @@
 #include "viralloc.h"
 #include "virstring.h"
 #include "virxml.h"
+extern const char *virDomainEventGetName(int event);
 
 
 char **
@@ -522,3 +523,38 @@ virshSnapshotNameCompleter(vshControl *ctl,
 virshDomainFree(dom);
 return NULL;
 }
+
+
+char **
+virshEventNameCompleter(vshControl *ctl,
+const vshCmd *cmd ATTRIBUTE_UNUSED,
+unsigned int flags)
+{
+virshControlPtr priv = ctl->privData;
+size_t i = 0;
+char **ret = NULL;
+
+virCheckFlags(0, NULL);
+
+if (!priv->conn || virConnectIsAlive(priv->conn) <= 0)
+return NULL;
+
+if (VIR_ALLOC_N(ret, VIR_DOMAIN_EVENT_ID_LAST + 1) < 0)
+goto error;
+
+for (i = 0; i < VIR_DOMAIN_EVENT_ID_LAST; i++) {
+const char *name = virDomainEventGetName(i);
+
+if (name == NULL)
+goto error;
+
+if (VIR_STRDUP(ret[i], name) < 0)
+goto error;
+}
+
+return ret;
+
+error:
+VIR_FREE(ret);
+return NULL;
+}
diff --git a/tools/virsh-completer.h b/tools/virsh-completer.h
index fa443d3ad7..27d78dc7ac 100644
--- a/tools/virsh-completer.h
+++ b/tools/virsh-completer.h
@@ -69,5 +69,8 @@ char ** virshSecretUUIDCompleter(vshControl *ctl,
 char ** virshSnapshotNameCompleter(vshControl *ctl,
const vshCmd *cmd,
unsigned int flags);
+char ** virshEventNameCompleter(vshControl *ctl,
+const vshCmd *cmd,
+unsigned int flags);
 
 #endif
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index e21ba0117b..ce72d414b9 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -13369,6 +13369,7 @@ static const vshCmdOptDef opts_event[] = {
 },
 {.name = "event",
  .type = VSH_OT_ARGV,
+ .completer = virshEventNameCompleter,
  .help = N_("which event type to wait for")
 },
 {.name = NULL}
-- 
2.15.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 09/12] virsh: Enable multiple --event flags to 'event' command

[Lin Ma]

Signed-off-by: Lin Ma 
---
 tools/virsh-domain.c | 76 +++-
 1 file changed, 39 insertions(+), 37 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 89aefbf86a..b35c9adaaa 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -13347,10 +13347,6 @@ static const vshCmdInfo info_event[] = {
 static const vshCmdOptDef opts_event[] = {
 VIRSH_COMMON_OPT_DOMAIN_OT_STRING(N_("filter by domain name, id or uuid"),
   0),
-{.name = "event",
- .type = VSH_OT_STRING,
- .help = N_("which event type to wait for")
-},
 {.name = "all",
  .type = VSH_OT_BOOL,
  .help = N_("wait for all events instead of just one type")
@@ -13371,6 +13367,10 @@ static const vshCmdOptDef opts_event[] = {
  .type = VSH_OT_BOOL,
  .help = N_("show timestamp for each printed event")
 },
+{.name = "event",
+ .type = VSH_OT_ARGV,
+ .help = N_("which event type to wait for")
+},
 {.name = NULL}
 };
 
@@ -13382,12 +13382,14 @@ cmdEvent(vshControl *ctl, const vshCmd *cmd)
 int timeout = 0;
 virshDomEventData *data = NULL;
 size_t i;
-const char *eventName = NULL;
+int *eventIdxList = NULL;
+size_t nevents = 0;
 int event = -1;
 bool all = vshCommandOptBool(cmd, "all");
 bool loop = vshCommandOptBool(cmd, "loop");
 bool timestamp = vshCommandOptBool(cmd, "timestamp");
 int count = 0;
+const vshCmdOpt *opt = NULL;
 virshControlPtr priv = ctl->privData;
 
 if (vshCommandOptBool(cmd, "list")) {
@@ -13396,15 +13398,25 @@ cmdEvent(vshControl *ctl, const vshCmd *cmd)
 return true;
 }
 
-if (vshCommandOptStringReq(ctl, cmd, "event", ) < 0)
-return false;
-if (eventName) {
-for (event = 0; event < VIR_DOMAIN_EVENT_ID_LAST; event++)
-if (STREQ(eventName, vshEventCallbacks[event].name))
-break;
-if (event == VIR_DOMAIN_EVENT_ID_LAST) {
-vshError(ctl, _("unknown event type %s"), eventName);
-return false;
+if (vshCommandOptBool(cmd, "event")) {
+if (VIR_ALLOC_N(eventIdxList, 1) < 0)
+goto cleanup;
+nevents = 1;
+while ((opt = vshCommandOptArgv(ctl, cmd, opt))) {
+if (opt->data) {
+for (event = 0; event < VIR_DOMAIN_EVENT_ID_LAST; event++)
+if (STREQ(opt->data, vshEventCallbacks[event].name))
+break;
+if (event == VIR_DOMAIN_EVENT_ID_LAST) {
+vshError(ctl, _("unknown event type %s"), opt->data);
+return false;
+}
+if (VIR_INSERT_ELEMENT(eventIdxList,
+   nevents - 1,
+   nevents,
+   event) < 0)
+goto cleanup;
+}
 }
 } else if (!all) {
 vshError(ctl, "%s",
@@ -13412,26 +13424,15 @@ cmdEvent(vshControl *ctl, const vshCmd *cmd)
 return false;
 }
 
-if (all) {
-if (VIR_ALLOC_N(data, VIR_DOMAIN_EVENT_ID_LAST) < 0)
-goto cleanup;
-for (i = 0; i < VIR_DOMAIN_EVENT_ID_LAST; i++) {
-data[i].ctl = ctl;
-data[i].loop = loop;
-data[i].count = 
-data[i].timestamp = timestamp;
-data[i].cb = [i];
-data[i].id = -1;
-}
-} else {
-if (VIR_ALLOC_N(data, 1) < 0)
-goto cleanup;
-data[0].ctl = ctl;
-data[0].loop = vshCommandOptBool(cmd, "loop");
-data[0].count = 
-data[0].timestamp = timestamp;
-data[0].cb = [event];
-data[0].id = -1;
+if (VIR_ALLOC_N(data, all ? VIR_DOMAIN_EVENT_ID_LAST : nevents - 1) < 0)
+goto cleanup;
+for (i = 0; i < (all ? VIR_DOMAIN_EVENT_ID_LAST : nevents - 1); i++) {
+data[i].ctl = ctl;
+data[i].loop = loop;
+data[i].count = 
+data[i].timestamp = timestamp;
+data[i].cb = [all ? i : eventIdxList[i]];
+data[i].id = -1;
 }
 if (vshCommandOptTimeoutToMs(ctl, cmd, ) < 0)
 goto cleanup;
@@ -13441,9 +13442,9 @@ cmdEvent(vshControl *ctl, const vshCmd *cmd)
 if (vshEventStart(ctl, timeout) < 0)
 goto cleanup;
 
-for (i = 0; i < (all ? VIR_DOMAIN_EVENT_ID_LAST : 1); i++) {
+for (i = 0; i < (all ? VIR_DOMAIN_EVENT_ID_LAST : nevents - 1); i++) {
 if ((data[i].id = virConnectDomainEventRegisterAny(priv->conn, dom,
-   all ? i : event,
+   all ? i : 
eventIdxList[i],
data[i].cb->cb,
[i],

[libvirt] [PATCH 04/12] virsh: Apply macro for current VSH_OT_STRING "domain" options

[Lin Ma]

These VSH_OT_STRING "domain" options support domain name completion now.

Signed-off-by: Lin Ma 
---
 tools/virsh-domain.c | 18 +-
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 8a63761fab..689f9d686b 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9601,10 +9601,8 @@ static const vshCmdInfo info_qemu_monitor_event[] = {
 };
 
 static const vshCmdOptDef opts_qemu_monitor_event[] = {
-{.name = "domain",
- .type = VSH_OT_STRING,
- .help = N_("filter by domain name, id or uuid")
-},
+VIRSH_COMMON_OPT_DOMAIN_OT_STRING(N_("filter by domain name, id or uuid"),
+  0),
 {.name = "event",
  .type = VSH_OT_STRING,
  .help = N_("filter by event name")
@@ -10157,11 +10155,7 @@ static const vshCmdOptDef opts_domxmltonative[] = {
  .flags = VSH_OFLAG_REQ,
  .help = N_("target config data type format")
 },
-{.name = "domain",
- .type = VSH_OT_STRING,
- .flags = VSH_OFLAG_REQ_OPT,
- .help = N_("domain name, id or uuid")
-},
+VIRSH_COMMON_OPT_DOMAIN_OT_STRING_FULL(0),
 {.name = "xml",
  .type = VSH_OT_STRING,
  .help = N_("xml data file to export from")
@@ -13348,10 +13342,8 @@ static const vshCmdInfo info_event[] = {
 };
 
 static const vshCmdOptDef opts_event[] = {
-{.name = "domain",
- .type = VSH_OT_STRING,
- .help = N_("filter by domain name, id, or uuid")
-},
+VIRSH_COMMON_OPT_DOMAIN_OT_STRING(N_("filter by domain name, id or uuid"),
+  0),
 {.name = "event",
  .type = VSH_OT_STRING,
  .help = N_("which event type to wait for")
-- 
2.15.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 01/12] virsh: Add domain name completion to 'migrate-postcopy' command

[Lin Ma]

Signed-off-by: Lin Ma 
---
 tools/virsh-domain.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 2b775fc4cc..6601f94a12 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -11219,11 +11219,7 @@ static const vshCmdInfo info_migrate_postcopy[] = {
 };
 
 static const vshCmdOptDef opts_migrate_postcopy[] = {
-{.name = "domain",
- .type = VSH_OT_DATA,
- .flags = VSH_OFLAG_REQ,
- .help = N_("domain name, id or uuid")
-},
+VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE),
 {.name = NULL}
 };
 
-- 
2.15.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 11/12] domain_event: Introduce function virDomainEventGetName

[Lin Ma]

It will be used in next patch for event name completion.

Signed-off-by: Lin Ma 
---
 src/conf/domain_event.c  | 62 
 src/conf/domain_event.h  |  3 +++
 src/libvirt_private.syms |  1 +
 3 files changed, 66 insertions(+)

diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
index 97520706c9..f8bd457b34 100644
--- a/src/conf/domain_event.c
+++ b/src/conf/domain_event.c
@@ -1968,6 +1968,68 @@ virDomainEventDispatchDefaultFunc(virConnectPtr conn,
 }
 
 
+const char *
+virDomainEventGetName(int event)
+{
+virResetLastError();
+
+switch (event) {
+case VIR_DOMAIN_EVENT_ID_LIFECYCLE:
+return VIR_DOMAIN_EVENT_LIFECYCLE;
+case VIR_DOMAIN_EVENT_ID_REBOOT:
+return VIR_DOMAIN_EVENT_REBOOT;
+case VIR_DOMAIN_EVENT_ID_RTC_CHANGE:
+return VIR_DOMAIN_EVENT_RTC_CHANGE;
+case VIR_DOMAIN_EVENT_ID_WATCHDOG:
+return VIR_DOMAIN_EVENT_WATCHDOG;
+case VIR_DOMAIN_EVENT_ID_IO_ERROR:
+return VIR_DOMAIN_EVENT_IO_ERROR;
+case VIR_DOMAIN_EVENT_ID_GRAPHICS:
+return VIR_DOMAIN_EVENT_GRAPHICS;
+case VIR_DOMAIN_EVENT_ID_IO_ERROR_REASON:
+return VIR_DOMAIN_EVENT_IO_ERROR_REASON;
+case VIR_DOMAIN_EVENT_ID_CONTROL_ERROR:
+return VIR_DOMAIN_EVENT_CONTROL_ERROR;
+case VIR_DOMAIN_EVENT_ID_BLOCK_JOB:
+return VIR_DOMAIN_EVENT_BLOCK_JOB;
+case VIR_DOMAIN_EVENT_ID_DISK_CHANGE:
+return VIR_DOMAIN_EVENT_DISK_CHANGE;
+case VIR_DOMAIN_EVENT_ID_TRAY_CHANGE:
+return VIR_DOMAIN_EVENT_TRAY_CHANGE;
+case VIR_DOMAIN_EVENT_ID_PMWAKEUP:
+return VIR_DOMAIN_EVENT_PMWAKEUP;
+case VIR_DOMAIN_EVENT_ID_PMSUSPEND:
+return VIR_DOMAIN_EVENT_PMSUSPEND;
+case VIR_DOMAIN_EVENT_ID_BALLOON_CHANGE:
+return VIR_DOMAIN_EVENT_BALLOON_CHANGE;
+case VIR_DOMAIN_EVENT_ID_PMSUSPEND_DISK:
+return VIR_DOMAIN_EVENT_PMSUSPEND_DISK;
+case VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED:
+return VIR_DOMAIN_EVENT_DEVICE_REMOVED;
+case VIR_DOMAIN_EVENT_ID_BLOCK_JOB_2:
+return VIR_DOMAIN_EVENT_BLOCK_JOB_2;
+case VIR_DOMAIN_EVENT_ID_TUNABLE:
+return VIR_DOMAIN_EVENT_TUNABLE;
+case VIR_DOMAIN_EVENT_ID_AGENT_LIFECYCLE:
+return VIR_DOMAIN_EVENT_AGENT_LIFECYCLE;
+case VIR_DOMAIN_EVENT_ID_DEVICE_ADDED:
+return VIR_DOMAIN_EVENT_DEVICE_ADDED;
+case VIR_DOMAIN_EVENT_ID_MIGRATION_ITERATION:
+return VIR_DOMAIN_EVENT_MIGRATION_ITERATION;
+case VIR_DOMAIN_EVENT_ID_JOB_COMPLETED:
+return VIR_DOMAIN_EVENT_JOB_COMPLETED;
+case VIR_DOMAIN_EVENT_ID_DEVICE_REMOVAL_FAILED:
+return VIR_DOMAIN_EVENT_DEVICE_REMOVAL_FAILED;
+case VIR_DOMAIN_EVENT_ID_METADATA_CHANGE:
+return VIR_DOMAIN_EVENT_METADATA_CHANGE;
+case VIR_DOMAIN_EVENT_ID_BLOCK_THRESHOLD:
+return VIR_DOMAIN_EVENT_BLOCK_THRESHOLD;
+default:
+return NULL;
+}
+}
+
+
 virObjectEventPtr
 virDomainQemuMonitorEventNew(int id,
  const char *name,
diff --git a/src/conf/domain_event.h b/src/conf/domain_event.h
index 3992a29c58..9c3a1e3b88 100644
--- a/src/conf/domain_event.h
+++ b/src/conf/domain_event.h
@@ -327,4 +327,7 @@ virDomainQemuMonitorEventNew(int id,
  const char *details)
 ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
 
+const char *
+virDomainEventGetName(int event);
+
 #endif
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 92b5e0fa2b..0b6c414b66 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -641,6 +641,7 @@ virDomainEventWatchdogNewFromDom;
 virDomainEventWatchdogNewFromObj;
 virDomainQemuMonitorEventNew;
 virDomainQemuMonitorEventStateRegisterID;
+virDomainEventGetName;
 
 
 # conf/domain_nwfilter.h
-- 
2.15.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 10/12] virsh: Introduce some VIR_DOMAIN_EVENT_* macros representing domain event names

[Lin Ma]

Signed-off-by: Lin Ma 
---
 include/libvirt/libvirt-domain.h | 201 +++
 tools/virsh-domain.c |  50 +-
 2 files changed, 226 insertions(+), 25 deletions(-)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 12fd34037e..f7dd510f7f 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -4424,6 +4424,207 @@ typedef enum {
 } virDomainEventID;
 
 
+/**
+ * VIR_DOMAIN_EVENT_LIFECYCLE:
+ *
+ * Macro for the event name "lifecycle"
+ */
+
+# define VIR_DOMAIN_EVENT_LIFECYCLE "lifecycle"
+
+/**
+ * VIR_DOMAIN_EVENT_REBOOT:
+ *
+ * Macro for the event name "reboot"
+ */
+
+# define VIR_DOMAIN_EVENT_REBOOT "reboot"
+
+/**
+ * VIR_DOMAIN_EVENT_RTC_CHANGE:
+ *
+ * Macro for the event name "rtc-change"
+ */
+
+# define VIR_DOMAIN_EVENT_RTC_CHANGE "rtc-change"
+
+/**
+ * VIR_DOMAIN_EVENT_WATCHDOG:
+ *
+ * Macro for the event name "watchdog"
+ */
+
+# define VIR_DOMAIN_EVENT_WATCHDOG "watchdog"
+
+/**
+ * VIR_DOMAIN_EVENT_IO_ERROR:
+ *
+ * Macro for the event name "io-error"
+ */
+
+# define VIR_DOMAIN_EVENT_IO_ERROR "io-error"
+
+/**
+ * VIR_DOMAIN_EVENT_GRAPHICS:
+ *
+ * Macro for the event name "graphics"
+ */
+
+# define VIR_DOMAIN_EVENT_GRAPHICS "graphics"
+
+/**
+ * VIR_DOMAIN_EVENT_IO_ERROR_REASON:
+ *
+ * Macro for the event name "io-error-reason"
+ */
+
+# define VIR_DOMAIN_EVENT_IO_ERROR_REASON "io-error-reason"
+
+/**
+ * VIR_DOMAIN_EVENT_CONTROL_ERROR:
+ *
+ * Macro for the event name "control-error"
+ */
+
+# define VIR_DOMAIN_EVENT_CONTROL_ERROR "control-error"
+
+/**
+ * VIR_DOMAIN_EVENT_BLOCK_JOB:
+ *
+ * Macro for the event name "block-job"
+ */
+
+# define VIR_DOMAIN_EVENT_BLOCK_JOB "block-job"
+
+/**
+ * VIR_DOMAIN_EVENT_DISK_CHANGE:
+ *
+ * Macro for the event name "disk-change"
+ */
+
+# define VIR_DOMAIN_EVENT_DISK_CHANGE "disk-change"
+
+/**
+ * VIR_DOMAIN_EVENT_TRAY_CHANGE:
+ *
+ * Macro for the event name "tray-change"
+ */
+
+# define VIR_DOMAIN_EVENT_TRAY_CHANGE "tray-change"
+
+/**
+ * VIR_DOMAIN_EVENT_PMWAKEUP:
+ *
+ * Macro for the event name "pm-wakeup"
+ */
+
+# define VIR_DOMAIN_EVENT_PMWAKEUP "pm-wakeup"
+
+/**
+ * VIR_DOMAIN_EVENT_PMSUSPEND:
+ *
+ * Macro for the event name "pm-suspend"
+ */
+
+# define VIR_DOMAIN_EVENT_PMSUSPEND "pm-suspend"
+
+/**
+ * VIR_DOMAIN_EVENT_BALLOON_CHANGE:
+ *
+ * Macro for the event name "balloon-change"
+ */
+
+# define VIR_DOMAIN_EVENT_BALLOON_CHANGE "balloon-change"
+
+/**
+ * VIR_DOMAIN_EVENT_PMSUSPEND_DISK:
+ *
+ * Macro for the event name "pm-suspend-disk"
+ */
+
+# define VIR_DOMAIN_EVENT_PMSUSPEND_DISK "pm-suspend-disk"
+
+/**
+ * VIR_DOMAIN_EVENT_DEVICE_REMOVED:
+ *
+ * Macro for the event name "device-removed"
+ */
+
+# define VIR_DOMAIN_EVENT_DEVICE_REMOVED "device-removed"
+
+/**
+ * VIR_DOMAIN_EVENT_BLOCK_JOB_2:
+ *
+ * Macro for the event name "block-job-2"
+ */
+
+# define VIR_DOMAIN_EVENT_BLOCK_JOB_2 "block-job-2"
+
+/**
+ * VIR_DOMAIN_EVENT_TUNABLE:
+ *
+ * Macro for the event name "tunable"
+ */
+
+# define VIR_DOMAIN_EVENT_TUNABLE "tunable"
+
+/**
+ * VIR_DOMAIN_EVENT_AGENT_LIFECYCLE:
+ *
+ * Macro for the event name "agent-lifecycle"
+ */
+
+# define VIR_DOMAIN_EVENT_AGENT_LIFECYCLE "agent-lifecycle"
+
+/**
+ * VIR_DOMAIN_EVENT_DEVICE_ADDED:
+ *
+ * Macro for the event name "device-added"
+ */
+
+# define VIR_DOMAIN_EVENT_DEVICE_ADDED "device-added"
+
+/**
+ * VIR_DOMAIN_EVENT_MIGRATION_ITERATION:
+ *
+ * Macro for the event name "migration-iteration"
+ */
+
+# define VIR_DOMAIN_EVENT_MIGRATION_ITERATION "migration-iteration"
+
+/**
+ * VIR_DOMAIN_EVENT_JOB_COMPLETED:
+ *
+ * Macro for the event name "job-completed"
+ */
+
+# define VIR_DOMAIN_EVENT_JOB_COMPLETED "job-completed"
+
+/**
+ * VIR_DOMAIN_EVENT_DEVICE_REMOVAL_FAILED:
+ *
+ * Macro for the event name "device-removal-failed"
+ */
+
+# define VIR_DOMAIN_EVENT_DEVICE_REMOVAL_FAILED "device-removal-failed"
+
+/**
+ * VIR_DOMAIN_EVENT_METADATA_CHANGE:
+ *
+ * Macro for the event name "metadata-change"
+ */
+
+# define VIR_DOMAIN_EVENT_METADATA_CHANGE "metadata-change"
+
+/**
+ * VIR_DOMAIN_EVENT_BLOCK_THRESHOLD:
+ *
+ * Macro for the event name "block-threshold"
+ */
+
+# define VIR_DOMAIN_EVENT_BLOCK_THRESHOLD "block-threshold"
+
+
 /* Use VIR_DOMAIN_EVENT_CALLBACK() to cast the 'cb' parameter  */
 int virConnectDomainEventRegisterAny(virConnectPtr conn,
  virDomainPtr dom, /* Optional, to filter 
*/
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index b35c9adaaa..e21ba0117b 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -13283,53 +13283,53 @@ virshEventBlockThresholdPrint(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 
 
 static vshEventCallback vshEventCallbacks[] = {
-{ "lifecycle",
+{ VIR_DOMAIN_EVENT_LIFECYCLE,
   VIR_DOMAIN_EVENT_CALLBACK(virshEventLifecyclePrint), },
-{ "reboot", virshEventGenericPrint, },
-{ "rtc-change",
+{ VIR_DOMAIN_EVENT_REBOOT, 

[libvirt] [PATCH 08/12] vshReadlineOptionsGenerator: Add already provided VSH_OT_ARGV options to list

[Lin Ma]

It's helpful for users while they type certain kind of VSH_OT_ARGV options.
e.g.

$ virsh domstats --domain sles12sp3 --d

Signed-off-by: Lin Ma 
---
 tools/vsh.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/vsh.c b/tools/vsh.c
index 5a9916cbb0..9cfd4d92b8 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -2685,7 +2685,7 @@ vshReadlineOptionsGenerator(const char *text,
 }
 
 while (opt) {
-if (STREQ(opt->def->name, name)) {
+if (STREQ(opt->def->name, name) && opt->def->type != VSH_OT_ARGV) {
 exists = true;
 break;
 }
-- 
2.15.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 03/12] virsh: Create macros for VSH_OT_STRING "domain" option

[Lin Ma]

Signed-off-by: Lin Ma 
---
 tools/virsh-domain-monitor.c | 3 +++
 tools/virsh-domain.c | 3 +++
 tools/virsh-snapshot.c   | 3 +++
 tools/virsh.h| 8 
 4 files changed, 17 insertions(+)

diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 8e071779b4..8ad651626b 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -43,6 +43,9 @@
 #define VIRSH_COMMON_OPT_DOMAIN_FULL(cflags) \
 VIRSH_COMMON_OPT_DOMAIN(N_("domain name, id or uuid"), cflags)
 
+#define VIRSH_COMMON_OPT_DOMAIN_OT_STRING_FULL(cflags) \
+VIRSH_COMMON_OPT_DOMAIN_OT_STRING(N_("domain name, id or uuid"), cflags)
+
 VIR_ENUM_DECL(virshDomainIOError)
 VIR_ENUM_IMPL(virshDomainIOError,
   VIR_DOMAIN_DISK_ERROR_LAST,
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 6601f94a12..8a63761fab 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -68,6 +68,9 @@
 #define VIRSH_COMMON_OPT_DOMAIN_FULL(cflags) \
 VIRSH_COMMON_OPT_DOMAIN(N_("domain name, id or uuid"), cflags)
 
+#define VIRSH_COMMON_OPT_DOMAIN_OT_STRING_FULL(cflags) \
+VIRSH_COMMON_OPT_DOMAIN_OT_STRING(N_("domain name, id or uuid"), cflags)
+
 #define VIRSH_COMMON_OPT_DOMAIN_PERSISTENT \
 {.name = "persistent", \
  .type = VSH_OT_BOOL, \
diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index e4908eea70..3d86ac84d1 100644
--- a/tools/virsh-snapshot.c
+++ b/tools/virsh-snapshot.c
@@ -45,6 +45,9 @@
 #define VIRSH_COMMON_OPT_DOMAIN_FULL(cflags) \
 VIRSH_COMMON_OPT_DOMAIN(N_("domain name, id or uuid"), cflags)
 
+#define VIRSH_COMMON_OPT_DOMAIN_OT_STRING_FULL(cflags) \
+VIRSH_COMMON_OPT_DOMAIN_OT_STRING(N_("domain name, id or uuid"), cflags)
+
 /* Helper for snapshot-create and snapshot-create-as */
 static bool
 virshSnapshotCreate(vshControl *ctl, virDomainPtr dom, const char *buffer,
diff --git a/tools/virsh.h b/tools/virsh.h
index f2213ebb57..9dcf104cc4 100644
--- a/tools/virsh.h
+++ b/tools/virsh.h
@@ -107,6 +107,14 @@
  .help = _helpstr \
 }
 
+# define VIRSH_COMMON_OPT_DOMAIN_OT_STRING(_helpstr, cflags) \
+{.name = "domain", \
+ .type = VSH_OT_STRING, \
+ .help = _helpstr, \
+ .completer = virshDomainNameCompleter, \
+ .completer_flags = cflags, \
+}
+
 typedef struct _virshControl virshControl;
 typedef virshControl *virshControlPtr;
 
-- 
2.15.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list