[libvirt] Question about verifying same uid:gid in src and dst for live migration

[Fei Li]

Hi,

When I do live migration using virsh command line based on NFS shared 
storage between two systems
having the same security mechanism and having the same kvm/qemu/libvirt 
version, I encounter the

following error:

debug : qemuMonitorJSONIOProcessLine:193 : Line [{"timestamp": {"seconds": 1524893525, 
"microseconds": 522686},
"event": "BLOCK_IO_ERROR", "data": {"device": "drive-virtio-disk0", "nospace": false, 
"node-name": "#block120",
"reason": "Permission denied", "operation": "write", "action": "report"}}]
...
error: internal error: qemu unexpectedly closed the monitor: 
qemu-system-x86_64: load of migration failed: Input/output error

...

According to the "Permission denied" && "write" information, I find the 
below 2 ways can fix this error:

- Change the mode of guest's .qcow2 file from 644 to 646
- Keep qemu's uid the same one between src host and dst host (They are 
not same before I change them)


My environment and test cases:

src:~ # id qemu
uid=473(qemu) gid=476(qemu) groups=488(kvm),476(qemu)
dst:~ # id qemu
uid=467(qemu) gid=470(qemu) groups=488(kvm),470(qemu)


In /etc/libvirt/qemu.conf, my confifuration is the following default:
# The user for QEMU processes run by the system instance. It can be
# specified as a user name or as a user id. The qemu driver will try to
# parse this value first as a name and then, if the name doesn't exist,
# as a user id.
#
# Since a sequence of digits is a valid user name, a leading plus sign
# can be used to ensure that a user id will not be interpreted as a user
# name.
#
# Some examples of valid values are:
#
#   user = "qemu"   # A user named "qemu"
#   user = "+0" # Super user (uid=0)
#   user = "100"# A user named "100" or a user with uid=100
#
#user = "root"

# The group for QEMU processes run by the system instance. It can be
# specified in a similar way to user.
#group = "root"

# Whether libvirt should dynamically change file ownership
# to match the configured user/group above. Defaults to 1.
# Set to 0 to disable file ownership changes.
#dynamic_ownership = 1


On the src, do live migration "virsh -d 0 migrate --live vm-name 
qemu+ssh://dst-ip/system":
- after a vm is defined, user:group=root:root
- after a vm is started, user:group=qemu:qemu
- after migration begins, user:group=467:470 (that is dst's uid:gid)
- after migration succeeds, user:group=467:470 (that is dst's uid:gid)
- after a vm is destroyed, user:group=root:root (back to the src's)
- after migration fails, user:group=467:470; the vm is still running in src but 
the file inside the guest
  becomes read-only even its mode is 644

Other notes:
- I tried libvirt v3.3.0 && v4.0.0 to do the same test, both can see 
such error.


After confirming that keeping qemu's uid identical between src host and 
dst host can fix such issue,
my question is whether a fix in libvirt should be pursued or just 
document the requirement for same

uid:gid across host systems in a migration cluster is ok?
BTW, if a fix is needed, maybe the pre-migration checks in libvirt could 
determine different uid and/or gid
and fail sooner with a better/explicit error like "Should keep the qemu 
uid in src and dst be the same for

migration, or elsemigration will fail"?

Does anyone have noticed this and could give some suggestions? Thanks a lot!

Have a nice day, thanks again
Fei


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

[libvirt] [PATCH 2/3] bhyve: add tests for wiring memory

[Fabian Freyer]

---
 tests/bhyveargv2xmldata/bhyveargv2xml-wired.args   |  7 +
 tests/bhyveargv2xmldata/bhyveargv2xml-wired.xml| 19 
 tests/bhyveargv2xmltest.c  |  1 +
 tests/bhyvexml2argvdata/bhyvexml2argv-wired.args   | 10 ++
 tests/bhyvexml2argvdata/bhyvexml2argv-wired.ldargs |  3 ++
 tests/bhyvexml2argvdata/bhyvexml2argv-wired.xml| 26 
 tests/bhyvexml2argvtest.c  |  1 +
 .../bhyvexml2xmloutdata/bhyvexml2xmlout-wired.xml  | 36 ++
 tests/bhyvexml2xmltest.c   |  1 +
 9 files changed, 104 insertions(+)
 create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-wired.args
 create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-wired.xml
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-wired.args
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-wired.ldargs
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-wired.xml
 create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-wired.xml

diff --git a/tests/bhyveargv2xmldata/bhyveargv2xml-wired.args 
b/tests/bhyveargv2xmldata/bhyveargv2xml-wired.args
new file mode 100644
index 0..5d0ad765b
--- /dev/null
+++ b/tests/bhyveargv2xmldata/bhyveargv2xml-wired.args
@@ -0,0 +1,7 @@
+/usr/sbin/bhyve \
+-c 1 \
+-m 214 \
+-H \
+-P \
+-S \
+-s 0:0,hostbridge bhyve
diff --git a/tests/bhyveargv2xmldata/bhyveargv2xml-wired.xml 
b/tests/bhyveargv2xmldata/bhyveargv2xml-wired.xml
new file mode 100644
index 0..0f4cea544
--- /dev/null
+++ b/tests/bhyveargv2xmldata/bhyveargv2xml-wired.xml
@@ -0,0 +1,19 @@
+
+  bhyve
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  219136
+  219136
+  
+
+  
+  1
+  
+hvm
+  
+  
+  destroy
+  destroy
+  destroy
+  
+  
+
diff --git a/tests/bhyveargv2xmltest.c b/tests/bhyveargv2xmltest.c
index e5d78530c..d55236484 100644
--- a/tests/bhyveargv2xmltest.c
+++ b/tests/bhyveargv2xmltest.c
@@ -163,6 +163,7 @@ mymain(void)
 driver.bhyvecaps = BHYVE_CAP_RTC_UTC;
 
 DO_TEST("base");
+DO_TEST("wired");
 DO_TEST("oneline");
 DO_TEST("name");
 DO_TEST("console");
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-wired.args 
b/tests/bhyvexml2argvdata/bhyvexml2argv-wired.args
new file mode 100644
index 0..13d4f4909
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-wired.args
@@ -0,0 +1,10 @@
+/usr/sbin/bhyve \
+-c 1 \
+-m 214 \
+-S \
+-u \
+-H \
+-P \
+-s 0:0,hostbridge \
+-s 2:0,ahci,hd:/tmp/freebsd.img \
+-s 3:0,virtio-net,faketapdev,mac=52:54:00:b9:94:02 bhyve
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-wired.ldargs 
b/tests/bhyvexml2argvdata/bhyvexml2argv-wired.ldargs
new file mode 100644
index 0..32538b558
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-wired.ldargs
@@ -0,0 +1,3 @@
+/usr/sbin/bhyveload \
+-m 214 \
+-d /tmp/freebsd.img bhyve
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-wired.xml 
b/tests/bhyvexml2argvdata/bhyvexml2argv-wired.xml
new file mode 100644
index 0..639e047dd
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-wired.xml
@@ -0,0 +1,26 @@
+
+  bhyve
+  df3be7e7-a104-11e3-aeb0-50e5492bd3dc
+  219136
+  
+
+  
+  1
+  
+hvm
+  
+  
+
+  
+  
+  
+  
+
+
+  
+  
+  
+  
+
+  
+
diff --git a/tests/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c
index 6f3b0c2eb..b08b1675f 100644
--- a/tests/bhyvexml2argvtest.c
+++ b/tests/bhyvexml2argvtest.c
@@ -179,6 +179,7 @@ mymain(void)
BHYVE_CAP_FBUF | BHYVE_CAP_XHCI;
 
 DO_TEST("base");
+DO_TEST("wired");
 DO_TEST("acpiapic");
 DO_TEST("disk-cdrom");
 DO_TEST("disk-virtio");
diff --git a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-wired.xml 
b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-wired.xml
new file mode 100644
index 0..ed564e277
--- /dev/null
+++ b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-wired.xml
@@ -0,0 +1,36 @@
+
+  bhyve
+  df3be7e7-a104-11e3-aeb0-50e5492bd3dc
+  219136
+  219136
+  
+
+  
+  1
+  
+hvm
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+
+  
+  
+  
+  
+
+
+
+  
+
+
+  
+  
+  
+  
+
+  
+
diff --git a/tests/bhyvexml2xmltest.c b/tests/bhyvexml2xmltest.c
index 4d9c1681d..6aaeab741 100644
--- a/tests/bhyvexml2xmltest.c
+++ b/tests/bhyvexml2xmltest.c
@@ -84,6 +84,7 @@ mymain(void)
 
 DO_TEST_DIFFERENT("acpiapic");
 DO_TEST_DIFFERENT("base");
+DO_TEST_DIFFERENT("wired");
 DO_TEST_DIFFERENT("bhyveload-bootorder");
 DO_TEST_DIFFERENT("bhyveload-bootorder1");
 DO_TEST_DIFFERENT("bhyveload-bootorder2");
-- 
2.15.1

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

[libvirt] [PATCH 1/3] bhyve: add support for wiring memory

[Fabian Freyer]

The  element will now pass the
wired (-S) flag to the bhyve command.
---
 src/bhyve/bhyve_command.c   | 3 +++
 src/bhyve/bhyve_parse_command.c | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
index 9413ae5c1..e3f7ded7d 100644
--- a/src/bhyve/bhyve_command.c
+++ b/src/bhyve/bhyve_command.c
@@ -474,6 +474,9 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn,
 virCommandAddArgFormat(cmd, "%llu",
VIR_DIV_UP(virDomainDefGetMemoryInitial(def), 
1024));
 
+if (def->mem.locked)
+virCommandAddArg(cmd, "-S"); /* Wire guest memory */
+
 /* Options */
 if (def->features[VIR_DOMAIN_FEATURE_ACPI] == VIR_TRISTATE_SWITCH_ON)
 virCommandAddArg(cmd, "-A"); /* Create an ACPI table */
diff --git a/src/bhyve/bhyve_parse_command.c b/src/bhyve/bhyve_parse_command.c
index fcaaed275..27916c528 100644
--- a/src/bhyve/bhyve_parse_command.c
+++ b/src/bhyve/bhyve_parse_command.c
@@ -721,6 +721,9 @@ bhyveParseBhyveCommandLine(virDomainDefPtr def,
 goto error;
 }
 break;
+   case 'S':
+   def->mem.locked = true;
+   break;
 }
 }
 
-- 
2.15.1

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

[libvirt] [PATCH 3/3] bhyve: document support for wiring guest memory

[Fabian Freyer]

---
 docs/drvbhyve.html.in | 14 ++
 docs/news.xml | 10 ++
 2 files changed, 24 insertions(+)

diff --git a/docs/drvbhyve.html.in b/docs/drvbhyve.html.in
index bde8298a5..5b5513d3d 100644
--- a/docs/drvbhyve.html.in
+++ b/docs/drvbhyve.html.in
@@ -430,6 +430,20 @@ supports Intel e1000 network adapter emulation. It's 
supported in libvirt
   model type='e1000'/
 /interface
 ...
+
+
+Wiring guest memory
+
+Since 4.4.0, it's possible to specify that guest 
memory should
+be wired and cannot be swapped out as follows:
+
+domain type="bhyve"
+...
+memoryBacking
+  locked/
+/memoryBacking
+...
+/domain
 
 
   
diff --git a/docs/news.xml b/docs/news.xml
index 80181415c..5b7dd4c4a 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -35,6 +35,16 @@
 
   
 
+  
+
+  bhyve: Support locking guest memory 
+
+
+  Bhyve's guest memory may be wired using the
+  
memoryBackinglocked//memoryBacking
+  element.
+
+  
 
 
 
-- 
2.15.1

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

[libvirt] [PATCH 0/3] bhyve: allow locking memory

[Fabian Freyer]

This patch series adds support for locking guest memory to the bhyve
driver using the following elements


  


When specified, the -S flag is passed to the bhyve binary.

Fabian Freyer (3):
  bhyve: add support for wiring memory
  bhyve: add tests for wiring memory
  bhyve: document support for wiring guest memory

 docs/drvbhyve.html.in  | 14 +
 docs/news.xml  | 10 ++
 src/bhyve/bhyve_command.c  |  3 ++
 src/bhyve/bhyve_parse_command.c|  3 ++
 tests/bhyveargv2xmldata/bhyveargv2xml-wired.args   |  7 +
 tests/bhyveargv2xmldata/bhyveargv2xml-wired.xml| 19 
 tests/bhyveargv2xmltest.c  |  1 +
 tests/bhyvexml2argvdata/bhyvexml2argv-wired.args   | 10 ++
 tests/bhyvexml2argvdata/bhyvexml2argv-wired.ldargs |  3 ++
 tests/bhyvexml2argvdata/bhyvexml2argv-wired.xml| 26 
 tests/bhyvexml2argvtest.c  |  1 +
 .../bhyvexml2xmloutdata/bhyvexml2xmlout-wired.xml  | 36 ++
 tests/bhyvexml2xmltest.c   |  1 +
 13 files changed, 134 insertions(+)
 create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-wired.args
 create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-wired.xml
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-wired.args
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-wired.ldargs
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-wired.xml
 create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-wired.xml

-- 
2.15.1

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

Re: [libvirt] [PATCHv2] Deprecate QEMU_CAPS_NESTING

[John Ferlan]

On 05/07/2018 04:38 AM, Ján Tomko wrote:
> Unused since commit .
> 
> Signed-off-by: Ján Tomko 
> ---
> v2: also delete the hasHwVirt variable as well as the whole 'svm'
> checking
> 
>  src/qemu/qemu_capabilities.h |  2 +-
>  src/qemu/qemu_command.c  | 24 
>  2 files changed, 1 insertion(+), 25 deletions(-)
> 

Reviewed-by: John Ferlan 

John

--
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/06/2018 03:15 PM, Julio Faracco wrote:
> Well,
> 
> Do we have a winner? :-)
> 
> --
> Julio Cesar Faracco
> 


sigh, top posting is not favored in technical groups.

In any case, I kind of like Eric's suggestion and I just figured you'd
end up coding it and posting it.

John

> 2018-05-04 18:23 GMT-03:00 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
> 

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

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

[John Ferlan]

On 05/04/2018 04:21 PM, Stefan Berger wrote:
> 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(+)
> 

I have to run for the day, I'll look again in the morning at this one...
 I did make one note below as I ran the series through Coverity.

John

> 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 

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

[John Ferlan]

On 05/04/2018 04:21 PM, Stefan Berger wrote:
> This patch extends the TPM's device XML with TPM 2 support. This only works
> for the emulator type backend and looks as follows:
> 
> 
>   

Perhaps this would be better as just version='2' since you're in a  block?

> 
> 
> 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.
> +  

I trust we check that somewhere ...

>  
>  
>  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);
> +

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

[John Ferlan]

On 05/04/2018 04:21 PM, Stefan Berger wrote:
> 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(-)
> 

I think this looks OK - not my specialty 0-) though.  I see
security_manager, selinux, etc. and my eyes start glazing over!

Anyway, I assume the reason there's no Restore processing is because
everything is deleted at shutdown, right?

> 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 

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

[John Ferlan]

On 05/04/2018 04:21 PM, Stefan Berger wrote:
> 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 \

syntax check will tell you
"path=/tmp/lib/domain--1-TPM-VM/monitor.sock," can fit on the previous line.

> +-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);

Probably should use DO_TEST_CAPS_LATEST here...

John

>  
>  
>  DO_TEST_PARSE_ERROR("pci-domain-invalid", NONE);
> 

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

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

[John Ferlan]

On 05/04/2018 04:21 PM, Stefan Berger wrote:
> 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)
>  }
>  }
>  

2 blank lines and
void
virDomainTPMDelete(virDomainDefPtr 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;

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

[John Ferlan]

On 05/04/2018 04:21 PM, Stefan Berger wrote:
> 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:
> + * - 

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

[John Ferlan]

On 05/04/2018 04:21 PM, Stefan Berger wrote:
> 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(+)
> 

I think you'd need to also alter libvirt.spec.in since you're adding new
directories... That's one of those make rpm type activities IIRC.

> 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) {

Is it intentionally the same as ->logDir?  Or did you want to have it's
own?  Doesn't matter to me - just asking.

> +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;
> +

This one has it's own...  although I wonder if it should be swtpm/run to
mimic cfg->stateDir

>  if (!(cfg->configBaseDir = virGetUserConfigDirectory()))
>  goto error;
>  
> @@ 

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

[John Ferlan]

On 05/04/2018 04:21 PM, Stefan Berger wrote:
> 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"

syntax-check would have told you unsafe cross-directory include - IOW
including conf/* files into util/* files is not allowed.

So I think you need to rethink where some of these functions will go. I
think they are mostly all used by the qemu_extdevice.c changes in patch
9, so perhaps they need to get folded into them.  There at least you can
grab the conf/domain_conf.h file.

> +#include "viralloc.h"

syntax-check would have told you not to include "viralloc.h" twice...

> +#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 "viruuid.h" to get virUUIDGenerate

> +#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;
> +

There's a love/hate relationship with static/globals...

>  /**
>   * virTPMCreateCancelPath:
>   * @devpath: Path to the TPM device
> @@ -74,3 +94,517 @@ virTPMCreateCancelPath(const char *devpath)
>   cleanup:
>  return path;
>  }

Two empty lines - pervasive comment here...

> +
> +/*
> + * 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"));

The message feels redundant.

> +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;
> +}
> +
> +/*
> + * 

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

[John Ferlan]

On 05/04/2018 04:21 PM, Stefan Berger wrote:
> 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;
>  }

virSecurityDACRestoreTPMFileLabel doesn't need to be changed? e.g.:


ret = virSecurityDACRestoreChardevLabel(mgr, def,
>data.emulator.source,
false);


> 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;
>  }
> 

Similarly for virSecuritySELinuxRestoreTPMFileLabelInt:

tpmdev = tpm->data.emulator.source.data.nix.path;
rc = virSecuritySELinuxRestoreFileLabel(mgr, tpmdev);

?

With the adjustments or at least an explanation in the commit message
why they cannot be Restored,

Reviewed-by: John Ferlan 

John

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

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

[John Ferlan]

On 05/04/2018 04:21 PM, Stefan Berger wrote:
> 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 

I take it we don't need ftw.h any more...

>  #if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R
>  # include 
>  #endif
> @@ -2949,6 +2950,54 @@ void virDirClose(DIR **dirp)
>  *dirp = NULL;
>  }
>  

Two empty lines

> +/*
> + * 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)

One arg per line.

> +{
> +struct dirent *ent;
> +int ret;

s/ret;/ret = -1;/

int direrr;

> +DIR *dir;
> +char *path;

path = NULL;

> +
> +if (virDirOpen(, name) < 0)
> +return -1;
> +
> +while ((ret = virDirRead(dir, , name)) > 0) {

s/ret/direrr

> +if (ent->d_type != DT_REG)
> +continue;
> +
> +if (virAsprintf(, "%s/%s", name, ent->d_name) < 0) {
> +ret = -1;
> +break;
> +}

Replace {...} w/

goto cleanup;

> +if (chown(path, uid, gid) < 0) {
> +ret = -1;

Unnecessary.

> +virReportSystemError(errno,
> + _("cannot chown '%s' to (%u, %u)"),
> + ent->d_name, (unsigned int) uid,
> + (unsigned int) gid);
   goto cleanup;

> +}
> +VIR_FREE(path);
> +if (ret < 0)
> +break;

Unnecessary

> +}
> +

if (direrr < 0)
goto cleanup;

ret = 0;

 cleanup:
VIR_FREE(path);

> +virDirClose();

return ret;

Skip the rest.

> +
> +if (ret < 0)
> +return -1;
> +
> +return 0;
> +}
> +

Two empty lines

With the adjustments,

Reviewed-by: John Ferlan 

John

>  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),
> 

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

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

[John Ferlan]

On 05/04/2018 04:21 PM, Stefan Berger wrote:
> 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(+)
> 

Reviewed-by: John Ferlan 

John

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

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

[John Ferlan]

On 05/04/2018 04:21 PM, Stefan Berger wrote:
> 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
> 

[...]

>  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, "virDomainTPMBackendTypeToString(def->type));
> -virBufferAdjustIndent(buf, 2);
>  
>  switch (def->type) {
>  case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
> -virBufferEscapeString(buf, "\n",
> +virBufferAddLit(buf, ">\n");
> +virBufferEscapeString(buf, "  \n",
>def->data.passthrough.source.data.file.path);

syntax-check would have told you to use virBufferAdjustIndent around
this and not use "   +virBufferAddLit(buf, "\n");
> +break;
> +case VIR_DOMAIN_TPM_TYPE_EMULATOR:
> +virBufferAddLit(buf, "/>\n");
>  break;
>  case VIR_DOMAIN_TPM_TYPE_LAST:
>  break;
>  }
>  
> -virBufferAdjustIndent(buf, -2);
> -virBufferAddLit(buf, "\n");
> -
>  virDomainDeviceInfoFormat(buf, >info, flags);
>  
>  virBufferAdjustIndent(buf, -2);

With the adjustment,

Reviewed-by: John Ferlan 

John

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

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

[John Ferlan]

On 05/04/2018 04:21 PM, Stefan Berger wrote:
> 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(+)
> 

? No src/libvirt_private.syms change to add the new function?

> 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;
>  }

Similar to 1/14 - 2 blank lines...

> +
> +/**
> + * virStringFilterLines:

I think this is more ExtractLines

as compared to virStringFilterChars which filters out certain chars from
a buffer and returns a buffer with those filtered out.

> + * @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.

Seems to be an incomplete thought, but I can adjust. So given Extract
vs. Filter:

 * Search for a given @needle in each line of the @input buffer and
 * create a return buffer that contains only those lines with the
 * @needle. Each output buffer line can be further restricted by
 * providing @maxlinelen in which case the output line would be
 * truncated @maxlinelen with "..." appended to the line to indicate
 * the truncation.
 *
 * Returns NULL on failure or a buffer with the extracted lines. It
 * is up to the caller to free the returned buffer.

> + */
> +char *
> +virStringFilterLines(char *input, const char *needle, size_t maxlinelen)

... and 1 line per argument.


> +{
> +char *sol = input;
> +char *eol;
> +char *buf = NULL;
> +size_t buflen = 1, llen;
> +const char *dots = "...";
> +

See below [1]

+if (!input || !needle) {
+virReportError(VIR_ERR_INALID_ARG,
+   _("neither input=%s nor needle=%s can be NULL"),
+   NULLSTR(input), NULLSTR(needle));
+return NULL;
+}
+

> +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)

Ran the patch through Coverity and it complained right here... I think
you meant "if (eol)", right?

> +*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);

[1]
Unfortunately all this does is ensure no one calls this with (NULL,
NULL,...)... In practice, if @input or @needle is assigned to NULL, the
compiler won't catch it.


With the adjustments,

Reviewed-by: John Ferlan 

But like patch 1, I do have some other concerns later in patch 7 when
this is used.

John

> +
>  #endif /* __VIR_STRING_H__ */
> 

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

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

[John Ferlan]

$SUBJ: "Implement"

On 05/04/2018 04:21 PM, Stefan Berger wrote:
> 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)
>  }
>  

Two blank lines between functions is the new normal...

>  int
> -virFileReadAllQuiet(const char *path, int maxlen, char **buf)
> +virFileReadOffsetQuiet(const char *path, off_t offset,
> +   int maxlen, char **buf)

...and each argument on it's own line...

>  {
>  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)

... here too (as well as the 2 empty lines on either side since we're
touching the code).

With the adjustments,

Reviewed-by: John Ferlan 

but I do have some other concerns later in patch 7 when this is used as
to whether it's necessary.

John

> +{
> +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)
> 

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

Re: [libvirt] [jenkins-ci PATCH] projects: Run libvirt's 'make check' on all machines

[Daniel P . Berrangé]

On Thu, May 03, 2018 at 06:25:19PM +0200, Andrea Bolognani wrote:
> Up until now, we had to skip it on FreeBSD due to some
> portability issues in the test suite, but as of libvirt
> commit 0b86e23d2569 they've all been fixed.

Unfortunately it appears that gnulib has a non-deterministic failure
in test-poll that hits on FreeBSD 10 and 11 :-(

> 
> Signed-off-by: Andrea Bolognani 
> ---
>  projects/libvirt.yaml | 7 ---
>  1 file changed, 7 deletions(-)
> 
> diff --git a/projects/libvirt.yaml b/projects/libvirt.yaml
> index 9b9b0b4..6f5464b 100644
> --- a/projects/libvirt.yaml
> +++ b/projects/libvirt.yaml
> @@ -18,13 +18,6 @@
>  - libvirt-fedora-rawhide
>- autotools-check-job:
>parent_jobs: 'libvirt-master-syntax-check'
> -  machines:
> -- libvirt-centos-7
> -- libvirt-debian-8
> -- libvirt-debian-9
> -- libvirt-fedora-27
> -- libvirt-fedora-28
> -- libvirt-fedora-rawhide
>local_env: |
>  export VIR_TEST_EXPENSIVE=1
>  export VIR_TEST_DEBUG=2
> -- 
> 2.14.3
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

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] xenconfig: Remove references to my name and email

[Jim Fehlig]

On 05/08/2018 03:22 AM, David Kiarie wrote:



On Tue, May 8, 2018 at 2:11 AM, Jim Fehlig > wrote:


On 05/07/2018 08:57 AM, David Kiarie wrote:



On Mon, May 7, 2018 at 5:55 PM, David Kiarie  >> wrote:



     On Mon, May 7, 2018 at 10:29 AM, Peter Krempa 
     >> wrote:

         On Sat, May 05, 2018 at 12:17:05 +0300, David Kiarie wrote:
         > On Sat, May 5, 2018 at 12:15 PM, David Kiarie

>>
         > wrote:
         >         > > Replace references to my name and email with a
pseudonym
         > >
         >         > Sorry, I just want my real name and email off these
files and I keep making
         > silly mistakes.

         How about just deleting them? We don't really support using
pseudonyms.


     Why is that ? With a person reason I don't want my name of these
files. I
     wrote the files and it took me a lot of work.

     I can still prove I wrote this code with the above email if anyone
wants me
     to as I still work as a developer.


I initally requested to have my name removed from the files but
apparently I hadn't signed off the patch so it was rejected.


You could have simply replied to the mail requesting a SOB and one of us
would have added it your patch and pushed it.

One month later, I figured it might be a bad idea to just give up all
the work I had done and opted to keep track of it.


I think that is a wise choice, in which case you should leave the authorship
as is :-).


I do have a good reason as to why I would like to remove my name from these 
files which I will not bother explaining. And I actually do want my name removed 
from these files.


Understood. We could have pushed the first version of your patch if it had a 
SOB. Presence of a SOB is enforced by a commit hook, so the patch couldn't be 
pushed without it. That wasn't enforced when you were working on the project. 
Apologies for not making that clearer at the beginning of this thread, saving 
everyone some time.


Removing my name on the other hand means I give up on something which I 
painstakingly worked on for almost one year.


As Daniel already mentioned, git contains a better history of your 
contributions. Interaction with the project is also recorded in the mail list 
archives.


Regards,
Jim

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

[libvirt] Question about using cpu mode "host-model" while providing a cpu model name

[Collin Walling]

Hi

I have noticed something that may be misconstrued regarding the libvirt domain 
xml format
for defining a cpu model. There seems to be a misalignment where the libvirt 
documentation
states something that is not supported, but libvirt itself gives no clear 
indication of
such. This is regarding the cpu mode "host-model" and providing a cpu model 
name between
the  tags.

>From the libvirt docs under header "CPU model and topology" paragraph "cpu" 
>subparagraph
"host-model", the following rule is defined (bolded or between asterisks):

"... The match attribute can't be used in this mode. *Specifying CPU model is 
not supported*
either, but model's fallback attribute may still be used. ..."

https://libvirt.org/formatdomain.html#elementsCPU

The above rule reads as "if mode is 'host-model' (and the architecture is not 
PowerPC) then
specifying a model name should not be allowed". However, this is not the 
observed behavior.
For example, I can define and start a guest with the following xml snippet 
without any issues:


cpu-name


Which seems to contradict what the documentation states.

This issue was reported by a colleague of mine who was confused by the cpu 
features that
were available to a guest when host-model and a model name are provided. 
Personally, I tend
to err on the side of providing host-model and a cpu-model-name being mutually 
exclusive.

I've attempted to find a solution to this problem myself by looking at 
virCPUDefParseXML,
but the fact that PowerPC exists as an exception and we do not know the 
architecture when
parsing a guest cpu xml makes minimal code changes challenging.

If we want to make changes to the code, then I imagine that the ideal solution 
would revolve
around only allowing cpu-name to be valid iff the cpu mode is 
set to "custom".
Otherwise some clarity on the documentation would suffice. Something like "A 
CPU model
specified in the domain xml will be ignored." Thoughts?

Thank you for your time. 

-- 
Respectfully,
- Collin Walling

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

[libvirt] [PATCH v2 06/12] vshReadlineParse: Ignore vshReadlineOptionsGenerator for VSH_OT_ARGV options

[Lin Ma]

Currently the VSH_OT_ARGV options don't support complete, But some of
VSH_OT_ARGV options are gonna support complete in upcoming patches.

Once applied the upcoming completion patches for VSH_OT_ARGV options, If
we don't ignore VSH_OT_ARGV here, The vshReadlineOptionsGenerator will
be called, Hence complete output will consist of the result by command
completer + the result by option completer, It's confusing.
e.g.
$ virsh domstats --domain 
--backing --interface  --list-paused  --perf  --vcpu
--balloon leap42.3 --list-persistent  --raw   win10
--block   --list-active--list-running sles12sp3
--cpu-total   --list-inactive  --list-shutoff sles15
--enforce --list-other --list-transient   --state

After this patch and the upcoming completion patches:
$ virsh domstats --domain 
leap42.3sles12sp3sles15win10

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

diff --git a/tools/vsh.c b/tools/vsh.c
index 57f7589b53..e45bb0d825 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -2824,7 +2824,9 @@ vshReadlineParse(const char *text, int state)
 if (!cmd) {
 list = vshReadlineCommandGenerator(text);
 } else {
-if (!opt || (opt->type != VSH_OT_DATA && opt->type != 
VSH_OT_STRING))
+if (!opt || (opt->type != VSH_OT_DATA &&
+ opt->type != VSH_OT_STRING &&
+ opt->type != VSH_OT_ARGV))
 list = vshReadlineOptionsGenerator(text, cmd, partial);
 
 if (opt && opt->completer) {
-- 
2.15.1

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

[libvirt] [PATCH v2 08/12] virsh: Apply macro for current VSH_OT_ARGV "domain" options

[Lin Ma]

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

diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 071619d0e3..fa93f3a312 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -1991,11 +1991,7 @@ static const vshCmdOptDef opts_domstats[] = {
  .type = VSH_OT_BOOL,
  .help = N_("add backing chain information to block stats"),
 },
-{.name = "domain",
- .type = VSH_OT_ARGV,
- .flags = VSH_OFLAG_NONE,
- .help = N_("list of domains to get stats for"),
-},
+VIRSH_COMMON_OPT_DOMAIN_OT_ARGV(N_("list of domains to get stats for"), 0),
 {.name = NULL}
 };
 
-- 
2.15.1

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

[libvirt] [PATCH v2 09/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 e45bb0d825..279d1b56e6 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 v2 12/12] virsh: Add event name completion to 'event' command

[Lin Ma]

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

diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
index e3b8234b41..ddf7b17caf 100644
--- a/tools/virsh-completer.c
+++ b/tools/virsh-completer.c
@@ -522,3 +522,40 @@ 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 = virshDomainEventGetName(i);
+
+if (name == NULL)
+goto error;
+
+if (VIR_STRDUP(ret[i], name) < 0)
+goto error;
+}
+
+return ret;
+
+ error:
+for (i = 0; i < VIR_DOMAIN_EVENT_ID_LAST; i++)
+VIR_FREE(ret[i]);
+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 36278ebc1c..0d58c0ff1a 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -13357,6 +13357,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 v2 10/12] virsh: Enable multiple --event flags to 'event' command

[Lin Ma]

Signed-off-by: Lin Ma 
---
 tools/virsh-domain.c | 107 +++
 1 file changed, 57 insertions(+), 50 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 30da953446..36278ebc1c 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -13335,10 +13335,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")
@@ -13359,6 +13355,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}
 };
 
@@ -13368,58 +13368,64 @@ cmdEvent(vshControl *ctl, const vshCmd *cmd)
 virDomainPtr dom = NULL;
 bool ret = false;
 int timeout = 0;
-virshDomEventData *data = NULL;
+virshDomEventData *dataList = NULL;
 size_t i;
-const char *eventName = NULL;
-int event = -1;
+int *eventIdxList = NULL;
+size_t nevents = 0;
+int eventid = -1;
 bool all = vshCommandOptBool(cmd, "all");
 bool loop = vshCommandOptBool(cmd, "loop");
 bool timestamp = vshCommandOptBool(cmd, "timestamp");
+bool event = vshCommandOptBool(cmd, "event");
 int count = 0;
+const vshCmdOpt *opt = NULL;
 virshControlPtr priv = ctl->privData;
 
 if (vshCommandOptBool(cmd, "list")) {
-for (event = 0; event < VIR_DOMAIN_EVENT_ID_LAST; event++)
-vshPrint(ctl, "%s\n", vshEventCallbacks[event].name);
+for (eventid = 0; eventid < VIR_DOMAIN_EVENT_ID_LAST; eventid++)
+vshPrint(ctl, "%s\n", vshEventCallbacks[eventid].name);
 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 (event) {
+while ((opt = vshCommandOptArgv(ctl, cmd, opt))) {
+if (opt->data) {
+for (eventid = 0; eventid < VIR_DOMAIN_EVENT_ID_LAST; 
eventid++)
+if (STREQ(opt->data, vshEventCallbacks[eventid].name))
+break;
+if (eventid == VIR_DOMAIN_EVENT_ID_LAST) {
+vshError(ctl, _("unknown event type %s"), opt->data);
+goto cleanup;
+}
+size_t n = nevents;
+virshDomEventData data;
+data.ctl = ctl;
+data.loop = loop;
+data.count = 
+data.timestamp = timestamp;
+data.cb = [eventid];
+data.id = -1;
+if (VIR_APPEND_ELEMENT(dataList, nevents, data) < 0)
+goto cleanup;
+if (VIR_APPEND_ELEMENT(eventIdxList, n, eventid) < 0)
+goto cleanup;
+}
 }
-} else if (!all) {
-vshError(ctl, "%s",
- _("one of --list, --all, or --event  is required"));
-return false;
-}
-
-if (all) {
-if (VIR_ALLOC_N(data, VIR_DOMAIN_EVENT_ID_LAST) < 0)
+} else if (all) {
+if (VIR_ALLOC_N(dataList, 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;
+dataList[i].ctl = ctl;
+dataList[i].loop = loop;
+dataList[i].count = 
+dataList[i].timestamp = timestamp;
+dataList[i].cb = [i];
+dataList[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;
+vshError(ctl, "%s",
+ _("one of --list, --all, or --event  is required"));
+return false;
 }
 if (vshCommandOptTimeoutToMs(ctl, cmd, ) < 0)
 goto cleanup;
@@ -13431,11 +13437,11 @@ cmdEvent(vshControl *ctl, const vshCmd *cmd)
 if (vshEventStart(ctl, timeout) < 0)
 goto cleanup;
 
-  

[libvirt] [PATCH v2 11/12] virsh: add helper for returning event name string

[Lin Ma]

Signed-off-by: Lin Ma 
---
 tools/virsh-util.c | 60 ++
 tools/virsh-util.h |  3 +++
 2 files changed, 63 insertions(+)

diff --git a/tools/virsh-util.c b/tools/virsh-util.c
index 44be3ad64b..686f9aef98 100644
--- a/tools/virsh-util.c
+++ b/tools/virsh-util.c
@@ -285,3 +285,63 @@ virshDomainGetXML(vshControl *ctl,
 
 return ret;
 }
+
+
+const char *
+virshDomainEventGetName(int event)
+{
+switch ((int)event) {
+case VIR_DOMAIN_EVENT_ID_LIFECYCLE:
+return "lifecycle";
+case VIR_DOMAIN_EVENT_ID_REBOOT:
+return "reboot";
+case VIR_DOMAIN_EVENT_ID_RTC_CHANGE:
+return "rtc-change";
+case VIR_DOMAIN_EVENT_ID_WATCHDOG:
+return "watchdog";
+case VIR_DOMAIN_EVENT_ID_IO_ERROR:
+return "io-error";
+case VIR_DOMAIN_EVENT_ID_GRAPHICS:
+return "graphics";
+case VIR_DOMAIN_EVENT_ID_IO_ERROR_REASON:
+return "io-error-reason";
+case VIR_DOMAIN_EVENT_ID_CONTROL_ERROR:
+return "control-error";
+case VIR_DOMAIN_EVENT_ID_BLOCK_JOB:
+return "block-job";
+case VIR_DOMAIN_EVENT_ID_DISK_CHANGE:
+return "disk-change";
+case VIR_DOMAIN_EVENT_ID_TRAY_CHANGE:
+return "tray-change";
+case VIR_DOMAIN_EVENT_ID_PMWAKEUP:
+return "pm-wakeup";
+case VIR_DOMAIN_EVENT_ID_PMSUSPEND:
+return "pm-suspend";
+case VIR_DOMAIN_EVENT_ID_BALLOON_CHANGE:
+return "balloon-change";
+case VIR_DOMAIN_EVENT_ID_PMSUSPEND_DISK:
+return "pm-suspend-disk";
+case VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED:
+return "device-removed";
+case VIR_DOMAIN_EVENT_ID_BLOCK_JOB_2:
+return "block-job-2";
+case VIR_DOMAIN_EVENT_ID_TUNABLE:
+return "tunable";
+case VIR_DOMAIN_EVENT_ID_AGENT_LIFECYCLE:
+return "agent-lifecycle";
+case VIR_DOMAIN_EVENT_ID_DEVICE_ADDED:
+return "device-added";
+case VIR_DOMAIN_EVENT_ID_MIGRATION_ITERATION:
+return "migration-iteration";
+case VIR_DOMAIN_EVENT_ID_JOB_COMPLETED:
+return "job-completed";
+case VIR_DOMAIN_EVENT_ID_DEVICE_REMOVAL_FAILED:
+return "device-removal-failed";
+case VIR_DOMAIN_EVENT_ID_METADATA_CHANGE:
+return "metadata-change";
+case VIR_DOMAIN_EVENT_ID_BLOCK_THRESHOLD:
+return "block-threshold";
+default:
+return NULL;
+}
+}
diff --git a/tools/virsh-util.h b/tools/virsh-util.h
index 9a0af3513d..02c4ace6bf 100644
--- a/tools/virsh-util.h
+++ b/tools/virsh-util.h
@@ -104,4 +104,7 @@ virshDomainGetXML(vshControl *ctl,
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4)
 ATTRIBUTE_NONNULL(5) ATTRIBUTE_RETURN_CHECK;
 
+const char *
+virshDomainEventGetName(int event);
+
 #endif /* VIRSH_UTIL_H */
-- 
2.15.1

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

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

[Lin Ma]

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..57f7589b53 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++) {
+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

[libvirt] [PATCH v2 05/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 ace5c02871..30da953446 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9592,10 +9592,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")
@@ -10148,11 +10146,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")
@@ -13339,10 +1,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 v2 01/12] virsh: Move the definition of macro VIRSH_COMMON_OPT_DOMAIN_FULL to virsh.h

[Lin Ma]

centralize the definition of macro VIRSH_COMMON_OPT_DOMAIN_FULL to virsh.h
to avoid unnecessary duplicated definition

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

diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 8e071779b4..071619d0e3 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -40,9 +40,6 @@
 #include "virxml.h"
 #include "virstring.h"
 
-#define VIRSH_COMMON_OPT_DOMAIN_FULL(cflags) \
-VIRSH_COMMON_OPT_DOMAIN(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 598d2fa4a4..aa11a81638 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -65,9 +65,6 @@
 # define SA_SIGINFO 0
 #endif
 
-#define VIRSH_COMMON_OPT_DOMAIN_FULL(cflags) \
-VIRSH_COMMON_OPT_DOMAIN(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..812fa91333 100644
--- a/tools/virsh-snapshot.c
+++ b/tools/virsh-snapshot.c
@@ -42,9 +42,6 @@
 #include "virxml.h"
 #include "conf/snapshot_conf.h"
 
-#define VIRSH_COMMON_OPT_DOMAIN_FULL(cflags) \
-VIRSH_COMMON_OPT_DOMAIN(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..9e717ef574 100644
--- a/tools/virsh.h
+++ b/tools/virsh.h
@@ -82,6 +82,9 @@
  .completer_flags = cflags, \
 }
 
+#define VIRSH_COMMON_OPT_DOMAIN_FULL(cflags) \
+VIRSH_COMMON_OPT_DOMAIN(N_("domain name, id or uuid"), cflags)
+
 # define VIRSH_COMMON_OPT_CONFIG(_helpstr) \
 {.name = "config", \
  .type = VSH_OT_BOOL, \
-- 
2.15.1

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

[libvirt] [PATCH v2 00/12] Some fixes and improvement for virsh completion in non interactive mode

[Lin Ma]

This patch series is about some fixes and improvement for virsh completion
in non interactive mode.

Some of them probably don't make sense, I sent them out for suggestions.

v2->v1:
* Add a new patch for centralize the definition of macro 
VIRSH_COMMON_OPT_DOMAIN_FULL to virsh.h

* rename the helper function from virDomainEventGetName virshDomainEventGetName
Follow Peter Krempa's suggestion:

* Drop the original patch#10 which about introduce some VIR_DOMAIN_EVENT_* 
macros

* code formatting fix

* centralize the definition of macro VIRSH_COMMON_OPT_DOMAIN_OT_STRING_FULL

* centralize the definition of macro VIRSH_COMMON_OPT_DOMAIN_OT_ARGV_FULL

* refactor some code in patch#10 for using same array, handle the case when
'--event' and '--all' are provided together through command line.

* Due to the helper function is only used for virsh, unnecessary to be publiced,
I move it from src/conf/domain_event.c to tools/virsh-util.c



Lin Ma (12):
  virsh: Move the definition of macro VIRSH_COMMON_OPT_DOMAIN_FULL to
virsh.h
  virsh: Add domain name completion to 'migrate-postcopy' command
  virsh: Conditionally Ignore the first entry in list of completions
  virsh: Create macros for VSH_OT_STRING "domain" option
  virsh: Apply macro for current VSH_OT_STRING "domain" options
  vshReadlineParse: Ignore vshReadlineOptionsGenerator for VSH_OT_ARGV
options
  virsh: Create macros for VSH_OT_ARGV "domain" option
  virsh: Apply macro for current VSH_OT_ARGV "domain" options
  vshReadlineOptionsGenerator: Add already provided VSH_OT_ARGV options
to list
  virsh: Enable multiple --event flags to 'event' command
  virsh: add helper for returning event name string
  virsh: Add event name completion to 'event' command

 tools/virsh-completer.c  |  37 
 tools/virsh-completer.h  |   3 +
 tools/virsh-domain-monitor.c |   9 +--
 tools/virsh-domain.c | 135 ---
 tools/virsh-snapshot.c   |   3 -
 tools/virsh-util.c   |  60 +++
 tools/virsh-util.h   |   3 +
 tools/virsh.h|  26 +
 tools/vsh.c  |  12 +++-
 9 files changed, 203 insertions(+), 85 deletions(-)

-- 
2.15.1

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

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

[Lin Ma]

Signed-off-by: Lin Ma 
---
 tools/virsh.h | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/tools/virsh.h b/tools/virsh.h
index 9e717ef574..b1b641bc41 100644
--- a/tools/virsh.h
+++ b/tools/virsh.h
@@ -110,6 +110,17 @@
  .help = _helpstr \
 }
 
+# define VIRSH_COMMON_OPT_DOMAIN_OT_STRING(_helpstr, cflags) \
+{.name = "domain", \
+ .type = VSH_OT_STRING, \
+ .help = _helpstr, \
+ .completer = virshDomainNameCompleter, \
+ .completer_flags = cflags, \
+}
+
+#define VIRSH_COMMON_OPT_DOMAIN_OT_STRING_FULL(cflags) \
+VIRSH_COMMON_OPT_DOMAIN_OT_STRING(N_("domain name, id or uuid"), 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

[libvirt] [PATCH v2 07/12] virsh: Create macros for VSH_OT_ARGV "domain" option

[Lin Ma]

Signed-off-by: Lin Ma 
---
 tools/virsh.h | 12 
 1 file changed, 12 insertions(+)

diff --git a/tools/virsh.h b/tools/virsh.h
index b1b641bc41..4353ff46d4 100644
--- a/tools/virsh.h
+++ b/tools/virsh.h
@@ -121,6 +121,18 @@
 #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(_helpstr, cflags) \
+{.name = "domain", \
+ .type = VSH_OT_ARGV, \
+ .flags = VSH_OFLAG_NONE, \
+ .help = _helpstr, \
+ .completer = virshDomainNameCompleter, \
+ .completer_flags = cflags, \
+}
+
+#define VIRSH_COMMON_OPT_DOMAIN_OT_ARGV_FULL(cflags) \
+VIRSH_COMMON_OPT_DOMAIN_OT_ARGV(N_("domain name, id or uuid"), 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

[libvirt] [PATCH v2 02/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 aa11a81638..ace5c02871 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -11213,11 +11213,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

Re: [libvirt] [PATCH] qemu: add sdl opengl support

[Maciej Wolny]

On 04/05/18 00:08, John Ferlan wrote:
> BTW: Since your patches add a capability - I would have expected a
> change to add a flag to one (or more) of the
> tests/qemucapabilitiesdata/caps_*.xml files, but none are modified.  So
> that means that the feature may not be introspectable, perhaps it's been
> part of qemu since 1.5 or it's something being added to the next qemu
> release. If it's a new feature, then when was it added? If it's been
> there since 1.5, then no capability flag is required.

I'm going to need some help understanding the QEMU capabilities test
(tests/qemucapabilitiestest.c). As I understand it, it run QEMU to get
the capabilities and then compares that to the XML file containing the
expected list of capabilities. What are the *.replies files used for?
And how is it do that on multiple architectures and QEMU versions
at the same time?

> 
> How that us determined for sdl is a mystery to me... I usually search
> through the qemu */*.json files. In this case I do find some remnants of
> 'gl' and 'sdl' in qapi/ui.json.  But how I read that output is that for
> 2.12 there's 'sdl' in DisplayOptions, but using 'DisplayNoOpts' which
> could mean no options for sdl are allowed, but I'm not sure. Maybe there
> is some change in flight - I don't follow qemu-devel that closely.
> 
> If I look back at commit id '937ebba00e' for the spice-gl addition
> (which yes, was one in one patch) - I can relate that to the similar
> spice gl fetch, but looking the recent top of qemu git tree, I don't see
> how this same mechanism can be used to determine whether the running
> qemu supports sdl using gl.

It looks like this option was introduced to QEMU in 0b71a5d5. v2.4.0 is
the earliest release which contains that commit.

> 
> So for code and other libvirt specific things...
> 
> BTW: The docs/formatdomain.html.in to add/describe the 'gl' option for
> 'sdl' needs to be provided.
> 
>> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
>> index 3569b9212..a2ef93c09 100644
>> --- a/docs/schemas/domaincommon.rng
>> +++ b/docs/schemas/domaincommon.rng
>> @@ -3031,6 +3031,14 @@
>>
>>  
>>
>> +  
>> +
>> +  
>> +
>> +  
>> +  
>> +
>> +  
> 
> I assume this is a copy of the spice gl - to reduce cut-paste-copy, you
> could create a "name" for it and share that name between spice and sdl.
> It's a common thing to do. Not required, but not difficult either.
> 
>>  
>>  
>>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index b0257068d..7d65ca9df 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -13448,6 +13448,7 @@ static int
>>  virDomainGraphicsDefParseXMLSDL(virDomainGraphicsDefPtr def,
>>  xmlNodePtr node)
>>  {
>> +xmlNodePtr cur;
>>  char *fullscreen = virXMLPropString(node, "fullscreen");
>>  int ret = -1;
>>  
>> @@ -13468,6 +13469,34 @@ 
>> virDomainGraphicsDefParseXMLSDL(virDomainGraphicsDefPtr def,
>>  def->data.sdl.xauth = virXMLPropString(node, "xauth");
>>  def->data.sdl.display = virXMLPropString(node, "display");
>>  
>> +cur = node->children;
>> +while (cur != NULL) {
>> +if (cur->type == XML_ELEMENT_NODE) {
>> +if (virXMLNodeNameEqual(cur, "gl")) {
>> +char *enable = virXMLPropString(cur, "enable");
>> +int enableVal;
>> +
>> +if (!   enable) {
>> +virReportError(VIR_ERR_XML_ERROR, "%s",
>> +   _("sdl gl element missing enable"));
>> +goto cleanup;
>> +}
>> +
>> +enableVal = virTristateBoolTypeFromString(enable);
>> +if (enableVal < 0) {
>> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> +   _("unknown enable value '%s'"), enable);
>> +VIR_FREE(enable);
>> +goto cleanup;
>> +}
>> +VIR_FREE(enable);
>> +
>> +def->data.sdl.gl = enableVal;
>> +}
>> +}
>> +cur = cur->next;
>> +}
>> +
> 
> I see this is just a copy of what Spice does, which probably needed some
> adjustment anyway... IIRC: Peter Krempa recently went through an
> exercise with the storage to change a number of these while loops using
> virXMLNodeNameEqual into more direct XML searches. Since this is only
> one element and attribute, I don't really think this loop is useful. I
> know it's possible to get the data via another means and some of the
> code already does that. The exact lines I'll leave to you to hash out.
> 
>>  ret = 0;
>>   cleanup:
>>  VIR_FREE(fullscreen);
>> @@ -25652,6 +25681,18 @@ virDomainGraphicsDefFormat(virBufferPtr buf,
>>  if (def->data.sdl.fullscreen)

Re: [libvirt] [PATCH 0/3] spec: drop deprecated el5 bits

[Daniel P . Berrangé]

On Tue, May 08, 2018 at 09:53:22AM -0400, Cole Robinson wrote:
> 2 patches from a Fedora contributor, 1 from me, dropping spec bits
> that aren't required on el6 and later builds
> 
> Cole Robinson (1):
>   spec: Remove Group: tags
> 
> Igor Gnatenko (2):
>   spec: Remove BuildRoot definition
>   spec: Remove %clean section
> 
>  libvirt.spec.in | 45 -
>  1 file changed, 45 deletions(-)

For all threee

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] [PATCH 2/3] spec: Remove %clean section

[Cole Robinson]

From: Igor Gnatenko 

None of currently supported distributions need that.
Last one was EL5 which is EOL for a while.

Signed-off-by: Igor Gnatenko 
Signed-off-by: Cole Robinson 
---
 libvirt.spec.in | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index b629e18e8d..81f99e5c0a 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1457,9 +1457,6 @@ mv 
$RPM_BUILD_ROOT%{_datadir}/systemtap/tapset/libvirt_qemu_probes.stp \
$RPM_BUILD_ROOT%{_datadir}/systemtap/tapset/libvirt_qemu_probes-64.stp
 %endif
 
-%clean
-rm -fr %{buildroot}
-
 %check
 cd tests
 # These tests don't current work in a mock build root
-- 
2.17.0

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

[libvirt] [PATCH 0/3] spec: drop deprecated el5 bits

[Cole Robinson]

2 patches from a Fedora contributor, 1 from me, dropping spec bits
that aren't required on el6 and later builds

Cole Robinson (1):
  spec: Remove Group: tags

Igor Gnatenko (2):
  spec: Remove BuildRoot definition
  spec: Remove %clean section

 libvirt.spec.in | 45 -
 1 file changed, 45 deletions(-)

-- 
2.17.0

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

[libvirt] [PATCH 3/3] spec: Remove Group: tags

[Cole Robinson]

It's only required on el5 which we don't support anymore. Everywhere
else it's not used for anything useful

https://fedoraproject.org/wiki/RPMGroups

Signed-off-by: Cole Robinson 
---
 libvirt.spec.in | 41 -
 1 file changed, 41 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 81f99e5c0a..9ea5e6b32a 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -254,7 +254,6 @@ Name: libvirt
 Version: @VERSION@
 Release: 1%{?dist}%{?extra_release}
 License: LGPLv2+
-Group: Development/Libraries
 URL: https://libvirt.org/
 
 %if %(echo %{version} | grep -q "\.0$"; echo $?) == 1
@@ -467,7 +466,6 @@ the libvirtd server exporting the virtualization support.
 
 %package docs
 Summary: API reference and website documentation
-Group: Development/Libraries
 
 %description docs
 Includes the API reference for the libvirt C library, and a complete
@@ -475,7 +473,6 @@ copy of the libvirt.org website documentation.
 
 %package daemon
 Summary: Server side daemon and supporting files for libvirt library
-Group: Development/Libraries
 
 # All runtime requirements for the libvirt package (runtime requrements
 # for subpackages are listed later in those subpackages)
@@ -522,7 +519,6 @@ for specific drivers.
 
 %package daemon-config-network
 Summary: Default configuration files for the libvirtd daemon
-Group: Development/Libraries
 
 Requires: libvirt-daemon = %{version}-%{release}
 Requires: libvirt-daemon-driver-network = %{version}-%{release}
@@ -532,7 +528,6 @@ Default configuration files for setting up NAT based 
networking
 
 %package daemon-config-nwfilter
 Summary: Network filter configuration files for the libvirtd daemon
-Group: Development/Libraries
 
 Requires: libvirt-daemon = %{version}-%{release}
 Requires: libvirt-daemon-driver-nwfilter = %{version}-%{release}
@@ -542,7 +537,6 @@ Network filter configuration files for cleaning guest 
traffic
 
 %package daemon-driver-network
 Summary: Network driver plugin for the libvirtd daemon
-Group: Development/Libraries
 Requires: libvirt-daemon = %{version}-%{release}
 Requires: dnsmasq >= 2.41
 Requires: radvd
@@ -559,7 +553,6 @@ bridge capabilities.
 
 %package daemon-driver-nwfilter
 Summary: Nwfilter driver plugin for the libvirtd daemon
-Group: Development/Libraries
 Requires: libvirt-daemon = %{version}-%{release}
 Requires: iptables
 %if 0%{?rhel} && 0%{?rhel} < 7
@@ -575,7 +568,6 @@ iptables and ip6tables capabilities
 
 %package daemon-driver-nodedev
 Summary: Nodedev driver plugin for the libvirtd daemon
-Group: Development/Libraries
 Requires: libvirt-daemon = %{version}-%{release}
 # needed for device enumeration
 %if 0%{?fedora} || 0%{?rhel} >= 7
@@ -592,7 +584,6 @@ capabilities.
 
 %package daemon-driver-interface
 Summary: Interface driver plugin for the libvirtd daemon
-Group: Development/Libraries
 Requires: libvirt-daemon = %{version}-%{release}
 %if (0%{?fedora} || 0%{?rhel} >= 7)
 Requires: netcf-libs >= 0.2.2
@@ -606,7 +597,6 @@ netcf library
 
 %package daemon-driver-secret
 Summary: Secret driver plugin for the libvirtd daemon
-Group: Development/Libraries
 Requires: libvirt-daemon = %{version}-%{release}
 
 %description daemon-driver-secret
@@ -615,7 +605,6 @@ an implementation of the secret key APIs.
 
 %package daemon-driver-storage-core
 Summary: Storage driver plugin including base backends for the libvirtd daemon
-Group: Development/Libraries
 Requires: libvirt-daemon = %{version}-%{release}
 Requires: nfs-utils
 # For mkfs
@@ -632,7 +621,6 @@ iSCSI, and multipath storage.
 
 %package daemon-driver-storage-logical
 Summary: Storage driver plugin for lvm volumes
-Group: Development/Libraries
 Requires: libvirt-daemon-driver-storage-core = %{version}-%{release}
 Requires: lvm2
 
@@ -643,7 +631,6 @@ volumes using lvm.
 
 %package daemon-driver-storage-disk
 Summary: Storage driver plugin for disk
-Group: Development/Libraries
 Requires: libvirt-daemon-driver-storage-core = %{version}-%{release}
 Requires: parted
 Requires: device-mapper
@@ -655,7 +642,6 @@ volumes using the host disks.
 
 %package daemon-driver-storage-scsi
 Summary: Storage driver plugin for local scsi devices
-Group: Development/Libraries
 Requires: libvirt-daemon-driver-storage-core = %{version}-%{release}
 
 %description daemon-driver-storage-scsi
@@ -665,7 +651,6 @@ host devices.
 
 %package daemon-driver-storage-iscsi
 Summary: Storage driver plugin for iscsi
-Group: Development/Libraries
 Requires: libvirt-daemon-driver-storage-core = %{version}-%{release}
 Requires: iscsi-initiator-utils
 
@@ -676,7 +661,6 @@ volumes using the host iscsi stack.
 
 %package daemon-driver-storage-mpath
 Summary: Storage driver plugin for multipath volumes
-Group: Development/Libraries
 Requires: libvirt-daemon-driver-storage-core = %{version}-%{release}
 Requires: device-mapper
 
@@ -688,7 +672,6 @@ multipath storage using device mapper.
 %if %{with_storage_gluster}
 %package daemon-driver-storage-gluster
 Summary: 

[libvirt] [PATCH 1/3] spec: Remove BuildRoot definition

[Cole Robinson]

From: Igor Gnatenko 

None of currently supported distributions need that.
It was needed last for EL5 which is EOL now

Signed-off-by: Igor Gnatenko 
Signed-off-by: Cole Robinson 
---
 libvirt.spec.in | 1 -
 1 file changed, 1 deletion(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index e261e259a4..b629e18e8d 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -255,7 +255,6 @@ Version: @VERSION@
 Release: 1%{?dist}%{?extra_release}
 License: LGPLv2+
 Group: Development/Libraries
-BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root
 URL: https://libvirt.org/
 
 %if %(echo %{version} | grep -q "\.0$"; echo $?) == 1
-- 
2.17.0

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

[libvirt] [jenkins-ci PATCH] Build libosinfo and osinfo-db-tools on mingw platform

[Daniel P . Berrangé]

Signed-off-by: Daniel P. Berrangé 
---
 guests/host_vars/libvirt-fedora-rawhide/main.yml |  2 ++
 guests/vars/mappings.yml |  6 ++
 guests/vars/projects/libosinfo+mingw.yml |  8 
 guests/vars/projects/osinfo-db-tools+mingw.yml   | 10 ++
 projects/libosinfo.yaml  | 12 
 projects/osinfo-db-tools.yaml| 12 
 6 files changed, 50 insertions(+)
 create mode 100644 guests/vars/projects/libosinfo+mingw.yml
 create mode 100644 guests/vars/projects/osinfo-db-tools+mingw.yml

diff --git a/guests/host_vars/libvirt-fedora-rawhide/main.yml 
b/guests/host_vars/libvirt-fedora-rawhide/main.yml
index 43555d0..82d46e8 100644
--- a/guests/host_vars/libvirt-fedora-rawhide/main.yml
+++ b/guests/host_vars/libvirt-fedora-rawhide/main.yml
@@ -4,6 +4,7 @@ PYTHONPATH: $VIRT_PREFIX/lib64/python3.6/site-packages
 
 projects:
   - libosinfo
+  - libosinfo+mingw
   - libvirt
   - libvirt+mingw
   - libvirt-cim
@@ -18,6 +19,7 @@ projects:
   - libvirt-tck
   - osinfo-db
   - osinfo-db-tools
+  - osinfo-db-tools+mingw
   - virt-manager
   - virt-viewer
   - virt-viewer+mingw
diff --git a/guests/vars/mappings.yml b/guests/vars/mappings.yml
index f1777fe..3e33bf1 100644
--- a/guests/vars/mappings.yml
+++ b/guests/vars/mappings.yml
@@ -368,6 +368,9 @@ mappings:
   mingw32-gtk-vnc2:
 FedoraRawhide: mingw32-gtk-vnc2
 
+  mingw32-libarchive:
+FedoraRawhide: mingw32-libarchive
+
   mingw32-libgovirt:
 FedoraRawhide: mingw32-libgovirt
 
@@ -437,6 +440,9 @@ mappings:
   mingw64-gtk-vnc2:
 FedoraRawhide: mingw64-gtk-vnc2
 
+  mingw64-libarchive:
+FedoraRawhide: mingw64-libarchive
+
   mingw64-libgovirt:
 FedoraRawhide: mingw64-libgovirt
 
diff --git a/guests/vars/projects/libosinfo+mingw.yml 
b/guests/vars/projects/libosinfo+mingw.yml
new file mode 100644
index 000..e3fc3cb
--- /dev/null
+++ b/guests/vars/projects/libosinfo+mingw.yml
@@ -0,0 +1,8 @@
+---
+packages:
+  - mingw32-glib2
+  - mingw32-libxml2
+  - mingw32-libxslt
+  - mingw64-glib2
+  - mingw64-libxml2
+  - mingw64-libxslt
diff --git a/guests/vars/projects/osinfo-db-tools+mingw.yml 
b/guests/vars/projects/osinfo-db-tools+mingw.yml
new file mode 100644
index 000..578e185
--- /dev/null
+++ b/guests/vars/projects/osinfo-db-tools+mingw.yml
@@ -0,0 +1,10 @@
+---
+packages:
+  - mingw32-glib2
+  - mingw32-libxml2
+  - mingw32-libxslt
+  - mingw32-libarchive
+  - mingw64-glib2
+  - mingw64-libxml2
+  - mingw64-libxslt
+  - mingw64-libarchive
diff --git a/projects/libosinfo.yaml b/projects/libosinfo.yaml
index 0d25447..8e3d105 100644
--- a/projects/libosinfo.yaml
+++ b/projects/libosinfo.yaml
@@ -13,3 +13,15 @@
   - autotools-rpm-job:
   parent_jobs: 'libosinfo-master-check'
   machines: '{rpm_machines}'
+  - autotools-build-job:
+  parent_jobs: 'osinfo-db-tools-master-build-mingw32'
+  variant: -mingw32
+  local_env: '{mingw32_local_env}'
+  autogen_args: '{mingw32_autogen_args}'
+  machines: '{mingw_machines}'
+  - autotools-build-job:
+  parent_jobs: 'osinfo-db-tools-master-build-mingw64'
+  variant: -mingw64
+  local_env: '{mingw64_local_env}'
+  autogen_args: '{mingw64_autogen_args}'
+  machines: '{mingw_machines}'
diff --git a/projects/osinfo-db-tools.yaml b/projects/osinfo-db-tools.yaml
index 6b451ef..cab85af 100644
--- a/projects/osinfo-db-tools.yaml
+++ b/projects/osinfo-db-tools.yaml
@@ -13,3 +13,15 @@
   - autotools-rpm-job:
   parent_jobs: 'osinfo-db-tools-master-check'
   machines: '{rpm_machines}'
+  - autotools-build-job:
+  parent_jobs:
+  variant: -mingw32
+  local_env: '{mingw32_local_env}'
+  autogen_args: '{mingw32_autogen_args}'
+  machines: '{mingw_machines}'
+  - autotools-build-job:
+  parent_jobs:
+  variant: -mingw64
+  local_env: '{mingw64_local_env}'
+  autogen_args: '{mingw64_autogen_args}'
+  machines: '{mingw_machines}'
-- 
2.17.0

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

[libvirt] [PATCH] Add a MAINTAINERS file to suggest reviewers for patches

[Daniel P . Berrangé]

Currently all patches are simply sent to the main libvirt development
mailing list. Sometimes individual developers are also CC'd but this is
typically the exception.

Libvirt does not follow a subsystem maintainer model, so there is no
notion of owners for the different areas of code, but there certainly
are people with high levels of expertize in specific areas.

This patch thus proposes pulling in QEMU's  get_maintainer.pl script and
creating MAINTAINERS file to list who the experts are for specific
areas. Combined with git-pubish, this will help ensure that patches are
brought to the attention of people with direct expertize.

For example:

$ git show b04629b62934caa8786e73c3db985672422fc662 | \
./build-aux/get_maintainer.pl
Jim Fehlig  (maintainer:libxl)
libvir-list@redhat.com (open list:All patches)

Or

$ git show e7cb9c4e230c3c77e35e72334c261b5b0a2211c6 | \
./build-aux/get_maintainer.pl
Jiri Denemar  (maintainer:CPU modelling)
libvir-list@redhat.com (open list:All patches)

Signed-off-by: Daniel P. Berrangé 
---
 .gitpublish |1 +
 MAINTAINERS |   67 ++
 build-aux/get_maintainer.pl | 2143 +++
 3 files changed, 2211 insertions(+)
 create mode 100644 MAINTAINERS
 create mode 100755 build-aux/get_maintainer.pl

diff --git a/.gitpublish b/.gitpublish
index 857f0d552c..6f2f9d11fa 100644
--- a/.gitpublish
+++ b/.gitpublish
@@ -1,3 +1,4 @@
 [gitpublishprofile "default"]
 base = master
 to = libvir-list@redhat.com
+cccmd = build-aux/get_maintainer.pl --noroles --norolestats --nogit 
--nogit-fallback 2>/dev/null
diff --git a/MAINTAINERS b/MAINTAINERS
new file mode 100644
index 00..5756fd8b00
--- /dev/null
+++ b/MAINTAINERS
@@ -0,0 +1,67 @@
+Libvirt Maintainers
+===
+
+The intention of this file is NOT to establish who owns what portions of the
+code base, but to provide a set of names that developers can consult when they
+have a question about a particular subset and also to provide a set of names
+to be CC'd when submitting a patch to obtain appropriate review.
+
+In general, if you have a question about inclusion of a patch, you should
+consult libvir-list@redhat.com and not any specific individual privately.
+
+Descriptions of section entries:
+
+   M: Mail patches to: FullName 
+   S: Status, one of the following:
+  Supported:   Someone is actually paid to look after this.
+  Maintained:  Someone actually looks after it.
+  Odd Fixes:   It has a maintainer but they don't have time to do
+   much other than throw the odd patch in. See below.
+  Orphan:  No current maintainer [but maybe you could take the
+   role as you write your new code].
+  Obsolete:Old code. Something tagged obsolete generally means
+   it has been replaced by a better system and you
+   should be using that.
+   F: Files and directories with wildcard patterns.
+  A trailing slash includes all files and subdirectory files.
+  F:   drivers/net/all files in and below drivers/net
+  F:   drivers/net/*   all files in drivers/net, but not below
+  F:   */net/* all files in "any top level directory"/net
+  One pattern per line.  Multiple F: lines acceptable.
+   X: Files and directories that are NOT maintained, same rules as F:
+  Files exclusions are tested before file matches.
+  Can be useful for excluding a specific subdirectory, for instance:
+  F:   net/
+  X:   net/ipv6/
+  matches all files in and below net excluding net/ipv6/
+   K: Keyword perl extended regex pattern to match content in a
+  patch or file.  For instance:
+  K: of_get_profile
+ matches patches or files that contain "of_get_profile"
+  K: \b(printk|pr_(info|err))\b
+ matches patches or files that contain one or more of the words
+ printk, pr_info or pr_err
+  One regex pattern per line.  Multiple K: lines acceptable.
+
+
+All patches
+L: libvir-list@redhat.com
+F: *
+F: */
+
+Common code
+---
+
+CPU modelling
+M: Jiri Denemar 
+S: Maintained
+F: src/cpu/
+
+Driver code
+---
+libxl
+M: Jim Fehlig 
+S: Maintained
+F: src/libxl/
+F: tests/*libxl*
+
diff --git a/build-aux/get_maintainer.pl b/build-aux/get_maintainer.pl
new file mode 100755
index 00..c94929d4f8
--- /dev/null
+++ b/build-aux/get_maintainer.pl
@@ -0,0 +1,2143 @@
+#!/usr/bin/env perl
+# (c) 2007, Joe Perches 
+#   created from checkpatch.pl
+#
+# Print selected MAINTAINERS information for
+# the files modified in a patch or for a file
+#
+# usage: perl scripts/get_maintainer.pl [OPTIONS] 
+#perl scripts/get_maintainer.pl [OPTIONS] -f 

Re: [libvirt] [PATCH] xenconfig: remove my name and email from files

[David Kiarie]

On Tue, May 8, 2018 at 3:45 PM, Daniel P. Berrangé 
wrote:

> On Tue, May 08, 2018 at 03:40:00PM +0300, David Kiarie wrote:
> > Remove my name and email from these files
> >
> > Signed-off-by: David Kiarie 
> > ---
> >  src/xenconfig/xen_xl.c | 1 -
> >  src/xenconfig/xen_xl.h | 1 -
> >  tests/xlconfigtest.c   | 1 -
> >  3 files changed, 3 deletions(-)
> >
> > diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
> > index 35d52f8a..f0d9177c 100644
> > --- a/src/xenconfig/xen_xl.c
> > +++ b/src/xenconfig/xen_xl.c
> > @@ -17,7 +17,6 @@
> >   * License along with this library.  If not, see
> >   * .
> >   *
> > - * Author: Kiarie Kahurani 
> >   * Author: Jim Fehlig 
> >   */
> >
> > diff --git a/src/xenconfig/xen_xl.h b/src/xenconfig/xen_xl.h
> > index 68f332ac..588d8d11 100644
> > --- a/src/xenconfig/xen_xl.h
> > +++ b/src/xenconfig/xen_xl.h
> > @@ -17,7 +17,6 @@
> >   * License along with this library.  If not, see
> >   * .
> >   *
> > - * Author: Kiarie Kahurani
> >   */
> >
> >  #ifndef __VIR_XEN_XL_H__
> > diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c
> > index 39f51e2d..36f7699f 100644
> > --- a/tests/xlconfigtest.c
> > +++ b/tests/xlconfigtest.c
> > @@ -19,7 +19,6 @@
> >   * .
> >   *
> >   * Author: Daniel P. Berrange 
> > - * Author: Kiarie Kahurani 
> >   *
> >   */
>
> Reviewed-by: Daniel P. Berrangé 
>
> Will push to git shortly.
>

Thanks!


>
> 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 10/12] storage_util: Move @type into _virStorageBackendQemuImgInfo

[John Ferlan]

We're about to split up the code a bit more, so we'll need this
to be in the local struct.

Signed-off-by: John Ferlan 
---
 src/storage/storage_util.c | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index bd8fb7ca92..c28f427a1a 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -794,6 +794,7 @@ storagePloopResize(virStorageVolDefPtr vol,
  */
 struct _virStorageBackendQemuImgInfo {
 int format;
+const char *type;
 const char *path;
 unsigned long long size_arg;
 unsigned long long allocation;
@@ -1125,9 +1126,9 @@ 
virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool,
  const char *secretPath)
 {
 virCommandPtr cmd = NULL;
-const char *type;
 struct _virStorageBackendQemuImgInfo info = {
 .format = vol->target.format,
+.type = NULL,
 .path = vol->target.path,
 .allocation = vol->target.allocation,
 .encryption = !!vol->target.encryption,
@@ -1149,7 +1150,7 @@ 
virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool,
 if (info.format == VIR_STORAGE_FILE_ISO)
 info.format = VIR_STORAGE_FILE_RAW;
 
-if (!(type = virStorageFileFormatTypeToString(info.format))) {
+if (!(info.type = virStorageFileFormatTypeToString(info.format))) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("unknown storage vol type %d"),
info.format);
@@ -1178,7 +1179,7 @@ 
virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool,
 return NULL;
 }
 if (vol->target.encryption->format == 
VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) {
-type = "luks";
+info.type = "luks";
 } else {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Only luks encryption is supported for raw 
files"));
@@ -1195,7 +1196,7 @@ 
virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool,
 return NULL;
 
 if (info.encryption &&
-storageBackendCreateQemuImgCheckEncryption(info.format, type, vol) < 0)
+storageBackendCreateQemuImgCheckEncryption(info.format, info.type, 
vol) < 0)
 return NULL;
 
 /* Size in KB */
@@ -1209,9 +1210,9 @@ 
virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool,
 
 if (info.inputPath)
 virCommandAddArgList(cmd, "convert", "-f", info.inputFormatStr,
- "-O", type, NULL);
+ "-O", info.type, NULL);
 else
-virCommandAddArgList(cmd, "create", "-f", type, NULL);
+virCommandAddArgList(cmd, "create", "-f", info.type, NULL);
 
 if (info.backingPath)
 virCommandAddArgList(cmd, "-b", info.backingPath, NULL);
-- 
2.14.3

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

[libvirt] [PATCH 11/12] storage_util: Introduce storageBackendCreateQemuImgSetInput

[John Ferlan]

Split up virStorageBackendCreateQemuImgCmdFromVol into two parts.
It's too long anyway and virStorageBackendCreateQemuImgCmdFromVol
should just handle the command line processing.

NB: Requires changing info.* into info->* references.

Signed-off-by: John Ferlan 
---
 src/storage/storage_util.c | 114 +
 1 file changed, 64 insertions(+), 50 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index c28f427a1a..f7da6743b0 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -1116,91 +1116,105 @@ storageBackendResizeQemuImgImageOpts(virCommandPtr cmd,
 }
 
 
-/* Create a qemu-img virCommand from the supplied arguments */
-virCommandPtr
-virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool,
- virStorageVolDefPtr vol,
- virStorageVolDefPtr inputvol,
- unsigned int flags,
- const char *create_tool,
- const char *secretPath)
+static int
+virStorageBackendCreateQemuImgSetInfo(virStoragePoolObjPtr pool,
+  virStorageVolDefPtr vol,
+  virStorageVolDefPtr inputvol,
+  struct _virStorageBackendQemuImgInfo 
*info)
 {
-virCommandPtr cmd = NULL;
-struct _virStorageBackendQemuImgInfo info = {
-.format = vol->target.format,
-.type = NULL,
-.path = vol->target.path,
-.allocation = vol->target.allocation,
-.encryption = !!vol->target.encryption,
-.preallocate = !!(flags & VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA),
-.compat = vol->target.compat,
-.features = vol->target.features,
-.nocow = vol->target.nocow,
-.secretPath = secretPath,
-.secretAlias = NULL,
-};
-virStorageEncryptionInfoDefPtr enc = NULL;
-
-virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, NULL);
-
 /* Treat output block devices as 'raw' format */
 if (vol->type == VIR_STORAGE_VOL_BLOCK)
-info.format = VIR_STORAGE_FILE_RAW;
+info->format = VIR_STORAGE_FILE_RAW;
 
-if (info.format == VIR_STORAGE_FILE_ISO)
-info.format = VIR_STORAGE_FILE_RAW;
+if (info->format == VIR_STORAGE_FILE_ISO)
+info->format = VIR_STORAGE_FILE_RAW;
 
-if (!(info.type = virStorageFileFormatTypeToString(info.format))) {
+if (!(info->type = virStorageFileFormatTypeToString(info->format))) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("unknown storage vol type %d"),
-   info.format);
-return NULL;
+   info->format);
+return -1;
 }
 
-if (info.preallocate && info.format != VIR_STORAGE_FILE_QCOW2) {
+if (info->preallocate && info->format != VIR_STORAGE_FILE_QCOW2) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("metadata preallocation only available with qcow2"));
-return NULL;
+return -1;
 }
-if (info.compat && info.format != VIR_STORAGE_FILE_QCOW2) {
+if (info->compat && info->format != VIR_STORAGE_FILE_QCOW2) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("compatibility option only available with qcow2"));
-return NULL;
+return -1;
 }
-if (info.features && info.format != VIR_STORAGE_FILE_QCOW2) {
+if (info->features && info->format != VIR_STORAGE_FILE_QCOW2) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("format features only available with qcow2"));
-return NULL;
+return -1;
 }
-if (info.format == VIR_STORAGE_FILE_RAW && vol->target.encryption) {
+if (info->format == VIR_STORAGE_FILE_RAW && vol->target.encryption) {
 if (inputvol) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("cannot use inputvol with encrypted raw volume"));
-return NULL;
+return -1;
 }
 if (vol->target.encryption->format == 
VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) {
-info.type = "luks";
+info->type = "luks";
 } else {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Only luks encryption is supported for raw 
files"));
-return NULL;
+return -1;
 }
 }
 
 if (inputvol &&
-storageBackendCreateQemuImgSetInput(inputvol, ) < 0)
-return NULL;
+storageBackendCreateQemuImgSetInput(inputvol, info) < 0)
+return -1;
 
 if (virStorageSourceHasBacking(>target) &&
-storageBackendCreateQemuImgSetBacking(pool, vol, inputvol, ) < 0)
-return NULL;
+storageBackendCreateQemuImgSetBacking(pool, 

[libvirt] [PATCH 04/12] storage_util: Rename virQEMUBuildLuksOpts

[John Ferlan]

Rename to storageBackendCreateQemuImgOpts - which is what it's doing.

Signed-off-by: John Ferlan 
---
 src/libvirt_private.syms   | 2 +-
 src/storage/storage_util.c | 2 +-
 src/util/virqemu.c | 8 
 src/util/virqemu.h | 6 +++---
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f8883dc50d..20352ffe99 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2593,8 +2593,8 @@ virQEMUBuildCommandLineJSON;
 virQEMUBuildCommandLineJSONArrayBitmap;
 virQEMUBuildCommandLineJSONArrayNumbered;
 virQEMUBuildDriveCommandlineFromJSON;
-virQEMUBuildLuksOpts;
 virQEMUBuildObjectCommandlineFromJSON;
+virQEMUBuildQemuImgKeySecretOpts;
 
 
 # util/virrandom.h
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 4dd73f2734..37a649d17b 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -823,7 +823,7 @@ 
storageBackendCreateQemuImgOpts(virStorageEncryptionInfoDefPtr enc,
 virBuffer buf = VIR_BUFFER_INITIALIZER;
 
 if (info.format == VIR_STORAGE_FILE_RAW && enc) {
-virQEMUBuildLuksOpts(, enc, info.secretAlias);
+virQEMUBuildQemuImgKeySecretOpts(, enc, info.secretAlias);
 } else {
 if (info.backingPath)
 virBufferAsprintf(, "backing_fmt=%s,",
diff --git a/src/util/virqemu.c b/src/util/virqemu.c
index d6652262fe..04cd71605e 100644
--- a/src/util/virqemu.c
+++ b/src/util/virqemu.c
@@ -303,7 +303,7 @@ virQEMUBuildBufferEscapeComma(virBufferPtr buf, const char 
*str)
 
 
 /**
- * virQEMUBuildLuksOpts:
+ * virQEMUBuildQemuImgKeySecretOpts:
  * @buf: buffer to build the string into
  * @enc: pointer to encryption info
  * @alias: alias to use
@@ -323,9 +323,9 @@ virQEMUBuildBufferEscapeComma(virBufferPtr buf, const char 
*str)
  *
  */
 void
-virQEMUBuildLuksOpts(virBufferPtr buf,
- virStorageEncryptionInfoDefPtr enc,
- const char *alias)
+virQEMUBuildQemuImgKeySecretOpts(virBufferPtr buf,
+ virStorageEncryptionInfoDefPtr enc,
+ const char *alias)
 {
 virBufferAsprintf(buf, "key-secret=%s,", alias);
 
diff --git a/src/util/virqemu.h b/src/util/virqemu.h
index 539d62ab14..2599481753 100644
--- a/src/util/virqemu.h
+++ b/src/util/virqemu.h
@@ -50,9 +50,9 @@ char *virQEMUBuildObjectCommandlineFromJSON(const char *type,
 char *virQEMUBuildDriveCommandlineFromJSON(virJSONValuePtr src);
 
 void virQEMUBuildBufferEscapeComma(virBufferPtr buf, const char *str);
-void virQEMUBuildLuksOpts(virBufferPtr buf,
-  virStorageEncryptionInfoDefPtr enc,
-  const char *alias)
+void virQEMUBuildQemuImgKeySecretOpts(virBufferPtr buf,
+  virStorageEncryptionInfoDefPtr enc,
+  const char *alias)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
 
 #endif /* __VIR_QEMU_H_ */
-- 
2.14.3

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

[libvirt] [PATCH 06/12] storage_util: Move secretPath generation

[John Ferlan]

Move generation of secretPath to storageBackendGenerateSecretData
and simplify a bit since we know vol->target.encryption is set plus
we have a local @enc.

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

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 64d4d1d7d2..d20a109306 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -1310,7 +1310,9 @@ 
storageBackendCreateQemuImgSecretPath(virStoragePoolObjPtr pool,
 
 
 static int
-storageBackendGenerateSecretData(virStorageVolDefPtr vol)
+storageBackendGenerateSecretData(virStoragePoolObjPtr pool,
+ virStorageVolDefPtr vol,
+ char **secretPath)
 {
 virStorageEncryptionPtr enc = vol->target.encryption;
 
@@ -1325,6 +1327,12 @@ storageBackendGenerateSecretData(virStorageVolDefPtr vol)
 return -1;
 }
 
+if (vol->target.format == VIR_STORAGE_FILE_RAW &&
+enc->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) {
+if (!(*secretPath = storageBackendCreateQemuImgSecretPath(pool, vol)))
+return -1;
+}
+
 return 0;
 }
 
@@ -1350,17 +1358,9 @@ storageBackendCreateQemuImg(virStoragePoolObjPtr pool,
 return -1;
 }
 
-if (storageBackendGenerateSecretData(vol) < 0)
+if (storageBackendGenerateSecretData(pool, vol, ) < 0)
 goto cleanup;
 
-if (vol->target.format == VIR_STORAGE_FILE_RAW &&
-vol->target.encryption &&
-vol->target.encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) {
-if (!(secretPath =
-  storageBackendCreateQemuImgSecretPath(pool, vol)))
-goto cleanup;
-}
-
 cmd = virStorageBackendCreateQemuImgCmdFromVol(pool, vol, inputvol,
flags, create_tool,
secretPath);
-- 
2.14.3

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

[libvirt] [PATCH 12/12] storage_util: Introduce storageBackendDoCreateQemuImg

[John Ferlan]

Extract out command line setup and run from storageBackendCreateQemuImg
as we'll need to run it twice soon.

Signed-off-by: John Ferlan 
---
 src/storage/storage_util.c | 37 +++--
 1 file changed, 27 insertions(+), 10 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index f7da6743b0..554fc757ed 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -1354,6 +1354,31 @@ storageBackendGenerateSecretData(virStoragePoolObjPtr 
pool,
 }
 
 
+static int
+storageBackendDoCreateQemuImg(virStoragePoolObjPtr pool,
+  virStorageVolDefPtr vol,
+  virStorageVolDefPtr inputvol,
+  unsigned int flags,
+  const char *create_tool,
+  const char *secretPath)
+{
+int ret;
+virCommandPtr cmd;
+
+cmd = virStorageBackendCreateQemuImgCmdFromVol(pool, vol, inputvol,
+   flags, create_tool,
+   secretPath);
+if (!cmd)
+return -1;
+
+ret = virStorageBackendCreateExecCommand(pool, vol, cmd);
+
+virCommandFree(cmd);
+
+return ret;
+}
+
+
 static int
 storageBackendCreateQemuImg(virStoragePoolObjPtr pool,
 virStorageVolDefPtr vol,
@@ -1362,7 +1387,6 @@ storageBackendCreateQemuImg(virStoragePoolObjPtr pool,
 {
 int ret = -1;
 char *create_tool;
-virCommandPtr cmd;
 char *secretPath = NULL;
 
 virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, -1);
@@ -1378,15 +1402,8 @@ storageBackendCreateQemuImg(virStoragePoolObjPtr pool,
 if (storageBackendGenerateSecretData(pool, vol, ) < 0)
 goto cleanup;
 
-cmd = virStorageBackendCreateQemuImgCmdFromVol(pool, vol, inputvol,
-   flags, create_tool,
-   secretPath);
-if (!cmd)
-goto cleanup;
-
-ret = virStorageBackendCreateExecCommand(pool, vol, cmd);
-
-virCommandFree(cmd);
+ret = storageBackendDoCreateQemuImg(pool, vol, inputvol, flags,
+create_tool, secretPath);
  cleanup:
 if (secretPath) {
 unlink(secretPath);
-- 
2.14.3

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

[libvirt] [PATCH 03/12] storage_util: Remove unnecessary check

[John Ferlan]

Commit id 'a48c71411' altered the logic a bit and didn't
remove an unnecessary check as info.encryption is true when
vol->target.encryption != NULL, so if we enter the if segment
with info.format == VIR_STORAGE_FILE_RAW && vol->target.encryption
!= NULL, then there's no way info.encryption could be false.

Signed-off-by: John Ferlan 
---
 src/storage/storage_util.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index cc1f6e7086..4dd73f2734 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -1175,11 +1175,6 @@ 
virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool,
_("cannot use inputvol with encrypted raw volume"));
 return NULL;
 }
-if (!info.encryption) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("missing encryption description"));
-return NULL;
-}
 if (vol->target.encryption->format == 
VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) {
 type = "luks";
 } else {
-- 
2.14.3

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

[libvirt] [PATCH 07/12] storage_util: Remove luks distinction from secret path and alias

[John Ferlan]

Remove the "luks" distinction as the code is about to become more
generic and be able to support qcow encryption as well.

Signed-off-by: John Ferlan 
---
 src/storage/storage_util.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index d20a109306..c72fd47024 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -1216,7 +1216,7 @@ 
virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool,
 
 if (info.format == VIR_STORAGE_FILE_RAW && vol->target.encryption &&
 vol->target.encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) {
-if (virAsprintf(, "%s_luks0", vol->name) < 0)
+if (virAsprintf(, "%s_encrypt0", vol->name) < 0)
 goto error;
 if (storageBackendCreateQemuImgSecretObject(cmd, info.secretPath,
 info.secretAlias) < 0)
@@ -1269,7 +1269,7 @@ 
storageBackendCreateQemuImgSecretPath(virStoragePoolObjPtr pool,
 
 if ((fd = mkostemp(secretPath, O_CLOEXEC)) < 0) {
 virReportSystemError(errno, "%s",
- _("failed to open luks secret file for write"));
+ _("failed to open secret file for write"));
 goto error;
 }
 
@@ -1280,7 +1280,7 @@ 
storageBackendCreateQemuImgSecretPath(virStoragePoolObjPtr pool,
 
 if (safewrite(fd, secret, secretlen) < 0) {
 virReportSystemError(errno, "%s",
- _("failed to write luks secret file"));
+ _("failed to write secret file"));
 goto error;
 }
 VIR_FORCE_CLOSE(fd);
@@ -1290,7 +1290,7 @@ 
storageBackendCreateQemuImgSecretPath(virStoragePoolObjPtr pool,
 if (chown(secretPath, vol->target.perms->uid,
   vol->target.perms->gid) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("failed to chown luks secret file"));
+   _("failed to chown secret file"));
 goto error;
 }
 }
@@ -2331,7 +2331,7 @@ storageBackendResizeQemuImg(virStoragePoolObjPtr pool,
   storageBackendCreateQemuImgSecretPath(pool, vol)))
 goto cleanup;
 
-if (virAsprintf(, "%s_luks0", vol->name) < 0)
+if (virAsprintf(, "%s_encrypt0", vol->name) < 0)
 goto cleanup;
 }
 
-- 
2.14.3

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

[libvirt] [PATCH 09/12] storage_util: Split preallocate set in storageBackendCreateQemuImgOpts

[John Ferlan]

The only way preallocate could be set is if the info->format was
not RAW (see storageBackendCreateQemuImgSetBacking), so let's just
extract it from the if/else surrounding the application of the
encryption options.

Signed-off-by: John Ferlan 
---
 src/storage/storage_util.c | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index cedec10403..bd8fb7ca92 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -831,12 +831,13 @@ 
storageBackendCreateQemuImgOpts(virStorageEncryptionInfoDefPtr enc,
 } else {
 if (info.encryption)
 virBufferAddLit(, "encryption=on,");
-if (info.preallocate) {
-if (info.size_arg > info.allocation)
-virBufferAddLit(, "preallocation=metadata,");
-else
-virBufferAddLit(, "preallocation=falloc,");
-}
+}
+
+if (info.preallocate) {
+if (info.size_arg > info.allocation)
+virBufferAddLit(, "preallocation=metadata,");
+else
+virBufferAddLit(, "preallocation=falloc,");
 }
 
 if (info.nocow)
-- 
2.14.3

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

[libvirt] [PATCH 08/12] storage_util: Split backing_fmt set in storageBackendCreateQemuImgOpts

[John Ferlan]

The only way backing_fmts could be set is if the info->format was
not RAW (see storageBackendCreateQemuImgSetBacking), so let's just
extract it from the if/else surrounding the application of the
encryption options.

Signed-off-by: John Ferlan 
---
 src/storage/storage_util.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index c72fd47024..cedec10403 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -822,12 +822,13 @@ 
storageBackendCreateQemuImgOpts(virStorageEncryptionInfoDefPtr enc,
 {
 virBuffer buf = VIR_BUFFER_INITIALIZER;
 
+if (info.backingPath)
+virBufferAsprintf(, "backing_fmt=%s,",
+  
virStorageFileFormatTypeToString(info.backingFormat));
+
 if (info.format == VIR_STORAGE_FILE_RAW && enc) {
 virQEMUBuildQemuImgKeySecretOpts(, enc, info.secretAlias);
 } else {
-if (info.backingPath)
-virBufferAsprintf(, "backing_fmt=%s,",
-  
virStorageFileFormatTypeToString(info.backingFormat));
 if (info.encryption)
 virBufferAddLit(, "encryption=on,");
 if (info.preallocate) {
-- 
2.14.3

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

[libvirt] [PATCH 01/12] storage_util: Some code cleanup

[John Ferlan]

Perform some code cleanup in areas that are about to be altered.

Signed-off-by: John Ferlan 
---
 src/storage/storage_util.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index bc048e3dff..4fddbf3f9e 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -874,7 +874,7 @@ 
storageBackendCreateQemuImgOpts(virStorageEncryptionInfoDefPtr enc,
 
 /* storageBackendCreateQemuImgCheckEncryption:
  * @format: format of file found
- * @conn: pointer to connection
+ * @type: TypeToString of format.type
  * @vol: pointer to volume def
  *
  * Ensure the proper setup for encryption.
@@ -1199,11 +1199,9 @@ 
virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool,
 return NULL;
 
 if (info.encryption &&
-storageBackendCreateQemuImgCheckEncryption(info.format, type,
-   vol) < 0)
+storageBackendCreateQemuImgCheckEncryption(info.format, type, vol) < 0)
 return NULL;
 
-
 /* Size in KB */
 info.size_arg = VIR_DIV_UP(vol->target.capacity, 1024);
 
-- 
2.14.3

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

[libvirt] [PATCH 02/12] storage_util: Cleanup usage of target.encryption

[John Ferlan]

Remove the != NULL checks, use !! for setting info.encryption.

Signed-off-by: John Ferlan 
---
 src/storage/storage_util.c | 18 --
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 4fddbf3f9e..cc1f6e7086 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -423,7 +423,7 @@ storageBackendCreateRaw(virStoragePoolObjPtr pool,
 reflink_copy = true;
 
 
-if (vol->target.encryption != NULL) {
+if (vol->target.encryption) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("storage pool does not support encrypted volumes"));
 goto cleanup;
@@ -707,7 +707,7 @@ storageBackendCreatePloop(virStoragePoolObjPtr pool 
ATTRIBUTE_UNUSED,
 return -1;
 }
 
-if (vol->target.encryption != NULL) {
+if (vol->target.encryption) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("encrypted ploop volumes are not supported with "
  "ploop init"));
@@ -1128,7 +1128,7 @@ 
virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool,
 .format = vol->target.format,
 .path = vol->target.path,
 .allocation = vol->target.allocation,
-.encryption = vol->target.encryption != NULL,
+.encryption = !!vol->target.encryption,
 .preallocate = !!(flags & VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA),
 .compat = vol->target.compat,
 .features = vol->target.features,
@@ -1169,8 +1169,7 @@ 
virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool,
_("format features only available with qcow2"));
 return NULL;
 }
-if (info.format == VIR_STORAGE_FILE_RAW &&
-vol->target.encryption != NULL) {
+if (info.format == VIR_STORAGE_FILE_RAW && vol->target.encryption) {
 if (inputvol) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("cannot use inputvol with encrypted raw volume"));
@@ -1220,8 +1219,7 @@ 
virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool,
 if (info.backingPath)
 virCommandAddArgList(cmd, "-b", info.backingPath, NULL);
 
-if (info.format == VIR_STORAGE_FILE_RAW &&
-vol->target.encryption != NULL &&
+if (info.format == VIR_STORAGE_FILE_RAW && vol->target.encryption &&
 vol->target.encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) {
 if (virAsprintf(, "%s_luks0", vol->name) < 0)
 goto error;
@@ -1415,10 +1413,10 @@ 
virStorageBackendGetBuildVolFromFunction(virStorageVolDefPtr vol,
  */
 if ((vol->type == VIR_STORAGE_VOL_FILE &&
  (vol->target.format != VIR_STORAGE_FILE_RAW ||
-  vol->target.encryption != NULL)) ||
+  vol->target.encryption)) ||
 (inputvol->type == VIR_STORAGE_VOL_FILE &&
  (inputvol->target.format != VIR_STORAGE_FILE_RAW ||
-  inputvol->target.encryption != NULL))) {
+  inputvol->target.encryption))) {
 return storageBackendCreateQemuImg;
 }
 
@@ -2105,7 +2103,7 @@ storageBackendVolBuildLocal(virStoragePoolObjPtr pool,
 virStorageBackendBuildVolFrom create_func;
 
 if (inputvol) {
-if (vol->target.encryption != NULL) {
+if (vol->target.encryption) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
"%s", _("storage pool does not support "
"building encrypted volumes from "
-- 
2.14.3

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

[libvirt] [PATCH 05/12] storage_util: Generate the qcow secret earlier

[John Ferlan]

Rather than having storageBackendCreateQemuImgCheckEncryption
perform the virStorageGenerateQcowEncryption, let's just do that
earlier during storageBackendCreateQemuImg so that the check
helper is just a check helper rather doing something different
based on whether the format is qcow[2] or raw based encryption.

Signed-off-by: John Ferlan 
---
 src/storage/storage_util.c | 31 +++
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 37a649d17b..64d4d1d7d2 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -901,10 +901,10 @@ storageBackendCreateQemuImgCheckEncryption(int format,
_("too many secrets for qcow encryption"));
 return -1;
 }
-if (enc->format == VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT ||
-enc->nsecrets == 0) {
-if (virStorageGenerateQcowEncryption(vol) < 0)
-return -1;
+if (enc->nsecrets == 0) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("no secret provided for qcow encryption"));
+return -1;
 }
 } else if (format == VIR_STORAGE_FILE_RAW) {
 if (enc->format != VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) {
@@ -1309,6 +1309,26 @@ 
storageBackendCreateQemuImgSecretPath(virStoragePoolObjPtr pool,
 }
 
 
+static int
+storageBackendGenerateSecretData(virStorageVolDefPtr vol)
+{
+virStorageEncryptionPtr enc = vol->target.encryption;
+
+if (!enc)
+return 0;
+
+if ((vol->target.format == VIR_STORAGE_FILE_QCOW ||
+ vol->target.format == VIR_STORAGE_FILE_QCOW2) &&
+(enc->format == VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT ||
+ enc->nsecrets == 0)) {
+if (virStorageGenerateQcowEncryption(vol) < 0)
+return -1;
+}
+
+return 0;
+}
+
+
 static int
 storageBackendCreateQemuImg(virStoragePoolObjPtr pool,
 virStorageVolDefPtr vol,
@@ -1330,6 +1350,9 @@ storageBackendCreateQemuImg(virStoragePoolObjPtr pool,
 return -1;
 }
 
+if (storageBackendGenerateSecretData(vol) < 0)
+goto cleanup;
+
 if (vol->target.format == VIR_STORAGE_FILE_RAW &&
 vol->target.encryption &&
 vol->target.encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) {
-- 
2.14.3

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

[libvirt] [PATCH 00/12] Various storage_util adjustments

[John Ferlan]

Essentially fallout and preparatory steps needed in order to alter 
the code to allow using qemu-img for creation of qcow[2] encrypted 
volume. The following series:

https://www.redhat.com/archives/libvir-list/2018-April/msg01578.html

is the impetus for these changes. What will follow once more testing
is done are the adjustments to be able to qemu-img create a qcow[2]
encrypted volume and changes to allow qemu-img convert to also work.
Currently create is broken for qcow[2] encryption and convert is
broken for both qcow[2] and luks encryption.

Patches 1&2 could be combined, but since some like separated adjustments
like that, I kept them separate.

John Ferlan (12):
  storage_util: Some code cleanup
  storage_util: Cleanup usage of target.encryption
  storage_util: Remove unnecessary check
  storage_util: Rename virQEMUBuildLuksOpts
  storage_util: Generate the qcow secret earlier
  storage_util: Move secretPath generation
  storage_util: Remove luks distinction from secret path and alias
  storage_util: Split backing_fmt set in storageBackendCreateQemuImgOpts
  storage_util: Split preallocate set in storageBackendCreateQemuImgOpts
  storage_util: Move @type into _virStorageBackendQemuImgInfo
  storage_util: Introduce storageBackendCreateQemuImgSetInput
  storage_util: Introduce storageBackendDoCreateQemuImg

 src/libvirt_private.syms   |   2 +-
 src/storage/storage_util.c | 252 +++--
 src/util/virqemu.c |   8 +-
 src/util/virqemu.h |   6 +-
 4 files changed, 158 insertions(+), 110 deletions(-)

-- 
2.14.3

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

Re: [libvirt] [PATCH] xenconfig: Remove references to my name and email

[Daniel P . Berrangé]

On Tue, May 08, 2018 at 03:42:27PM +0300, David Kiarie wrote:
> On Tue, May 8, 2018 at 2:21 PM, Daniel P. Berrangé 
> wrote:
> 
> > On Mon, May 07, 2018 at 05:55:51PM +0300, David Kiarie wrote:
> > > On Mon, May 7, 2018 at 10:29 AM, Peter Krempa 
> > wrote:
> > >
> > > > On Sat, May 05, 2018 at 12:17:05 +0300, David Kiarie wrote:
> > > > > On Sat, May 5, 2018 at 12:15 PM, David Kiarie <
> > davidkiar...@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > Replace references to my name and email with a pseudonym
> > > > > >
> > > > >
> > > > > Sorry, I just want my real name and email off these files and I keep
> > > > making
> > > > > silly mistakes.
> > > >
> > > > How about just deleting them? We don't really support using pseudonyms.
> > > >
> > >
> > > Why is that ? With a person reason I don't want my name of these files. I
> > > wrote the files and it took me a lot of work.
> > >
> > > I can still prove I wrote this code with the above email if anyone wants
> > me
> > > to as I still work as a developer.
> >
> > FWIW, the Author lines are largely irrelevant when looking at who authored
> > code, they are at best incomplete, and at worst outright misleading, so we
> > tend to recommend people not to pay attention to them. The GIT history
> > contains the true accurate authorship information and your name is still
> > recorded there, so the fact that you did the work would not be lost even
> > without the Author line present.
> >
> 
> Aiming to be an independent developer and already working on that (I am
> actually not formally employed at the moment), this would definitely be a
> huge part of my portifolio and I did actually do quite a bit of work on the
> code on those files.

BTW, I meant to include a link in the previous message - this shows how
to get a report of all your contributions:

  
https://libvirt.org/git/?p=libvirt.git;a=search;s=davidkiar...@gmail.com;st=author

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] xenconfig: remove my name and email from files

[Daniel P . Berrangé]

On Tue, May 08, 2018 at 03:40:00PM +0300, David Kiarie wrote:
> Remove my name and email from these files
> 
> Signed-off-by: David Kiarie 
> ---
>  src/xenconfig/xen_xl.c | 1 -
>  src/xenconfig/xen_xl.h | 1 -
>  tests/xlconfigtest.c   | 1 -
>  3 files changed, 3 deletions(-)
> 
> diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
> index 35d52f8a..f0d9177c 100644
> --- a/src/xenconfig/xen_xl.c
> +++ b/src/xenconfig/xen_xl.c
> @@ -17,7 +17,6 @@
>   * License along with this library.  If not, see
>   * .
>   *
> - * Author: Kiarie Kahurani 
>   * Author: Jim Fehlig 
>   */
>  
> diff --git a/src/xenconfig/xen_xl.h b/src/xenconfig/xen_xl.h
> index 68f332ac..588d8d11 100644
> --- a/src/xenconfig/xen_xl.h
> +++ b/src/xenconfig/xen_xl.h
> @@ -17,7 +17,6 @@
>   * License along with this library.  If not, see
>   * .
>   *
> - * Author: Kiarie Kahurani
>   */
>  
>  #ifndef __VIR_XEN_XL_H__
> diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c
> index 39f51e2d..36f7699f 100644
> --- a/tests/xlconfigtest.c
> +++ b/tests/xlconfigtest.c
> @@ -19,7 +19,6 @@
>   * .
>   *
>   * Author: Daniel P. Berrange 
> - * Author: Kiarie Kahurani 
>   *
>   */

Reviewed-by: Daniel P. Berrangé 

Will push to git shortly.

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] xenconfig: remove my name and email from files

[David Kiarie]

Remove my name and email from these files

Signed-off-by: David Kiarie 
---
 src/xenconfig/xen_xl.c | 1 -
 src/xenconfig/xen_xl.h | 1 -
 tests/xlconfigtest.c   | 1 -
 3 files changed, 3 deletions(-)

diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
index 35d52f8a..f0d9177c 100644
--- a/src/xenconfig/xen_xl.c
+++ b/src/xenconfig/xen_xl.c
@@ -17,7 +17,6 @@
  * License along with this library.  If not, see
  * .
  *
- * Author: Kiarie Kahurani 
  * Author: Jim Fehlig 
  */
 
diff --git a/src/xenconfig/xen_xl.h b/src/xenconfig/xen_xl.h
index 68f332ac..588d8d11 100644
--- a/src/xenconfig/xen_xl.h
+++ b/src/xenconfig/xen_xl.h
@@ -17,7 +17,6 @@
  * License along with this library.  If not, see
  * .
  *
- * Author: Kiarie Kahurani
  */
 
 #ifndef __VIR_XEN_XL_H__
diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c
index 39f51e2d..36f7699f 100644
--- a/tests/xlconfigtest.c
+++ b/tests/xlconfigtest.c
@@ -19,7 +19,6 @@
  * .
  *
  * Author: Daniel P. Berrange 
- * Author: Kiarie Kahurani 
  *
  */
 
-- 
2.17.0

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

Re: [libvirt] [PATCH] xenconfig: Remove references to my name and email

[David Kiarie]

On Tue, May 8, 2018 at 2:21 PM, Daniel P. Berrangé 
wrote:

> On Mon, May 07, 2018 at 05:55:51PM +0300, David Kiarie wrote:
> > On Mon, May 7, 2018 at 10:29 AM, Peter Krempa 
> wrote:
> >
> > > On Sat, May 05, 2018 at 12:17:05 +0300, David Kiarie wrote:
> > > > On Sat, May 5, 2018 at 12:15 PM, David Kiarie <
> davidkiar...@gmail.com>
> > > > wrote:
> > > >
> > > > > Replace references to my name and email with a pseudonym
> > > > >
> > > >
> > > > Sorry, I just want my real name and email off these files and I keep
> > > making
> > > > silly mistakes.
> > >
> > > How about just deleting them? We don't really support using pseudonyms.
> > >
> >
> > Why is that ? With a person reason I don't want my name of these files. I
> > wrote the files and it took me a lot of work.
> >
> > I can still prove I wrote this code with the above email if anyone wants
> me
> > to as I still work as a developer.
>
> FWIW, the Author lines are largely irrelevant when looking at who authored
> code, they are at best incomplete, and at worst outright misleading, so we
> tend to recommend people not to pay attention to them. The GIT history
> contains the true accurate authorship information and your name is still
> recorded there, so the fact that you did the work would not be lost even
> without the Author line present.
>

Aiming to be an independent developer and already working on that (I am
actually not formally employed at the moment), this would definitely be a
huge part of my portifolio and I did actually do quite a bit of work on the
code on those files.

I'm not sure what to do either so I just sent both patches.


> If anything I would suggst we could bulk remove them from all code since
> because of their incomplete & misleading nature, but that's a larger
> can of worms.
>
> 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] xenconfig: Remove references to my name and email

[David Kiarie]

Replace references to my name and email with a pseudonym

Signed-off-by: David Kiarie 
---
 src/xenconfig/xen_xl.c | 2 +-
 src/xenconfig/xen_xl.h | 2 +-
 tests/xlconfigtest.c   | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
index 35d52f8a..0343a73c 100644
--- a/src/xenconfig/xen_xl.c
+++ b/src/xenconfig/xen_xl.c
@@ -17,7 +17,7 @@
  * License along with this library.  If not, see
  * .
  *
- * Author: Kiarie Kahurani 
+ * Author: Oneko 
  * Author: Jim Fehlig 
  */
 
diff --git a/src/xenconfig/xen_xl.h b/src/xenconfig/xen_xl.h
index 68f332ac..4ce13f05 100644
--- a/src/xenconfig/xen_xl.h
+++ b/src/xenconfig/xen_xl.h
@@ -17,7 +17,7 @@
  * License along with this library.  If not, see
  * .
  *
- * Author: Kiarie Kahurani
+ * Author: Oneko 
  */
 
 #ifndef __VIR_XEN_XL_H__
diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c
index 39f51e2d..1bbc0b72 100644
--- a/tests/xlconfigtest.c
+++ b/tests/xlconfigtest.c
@@ -19,7 +19,7 @@
  * .
  *
  * Author: Daniel P. Berrange 
- * Author: Kiarie Kahurani 
+ * Author: Oneko 
  *
  */
 
-- 
2.17.0

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

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

[Boris Fiuczynski]

On 05/07/2018 01:51 PM, John Ferlan wrote:

[...]


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.

I am not sure if I understood you correctly. Are you referring to patch
1? If so the changes are caused by a new QEMU_CAPS_CCW capability
replacing the QEMU_CAPS_VIRTIO_CCW capability. More is explained in the
commit message of the patch. In short: With support of vfio-ccw it
became apparent that the existence of the ccw bus is not well sourced by
observing virtio-ccw and therefore we replaced it with the detection of
the virtual-css-bridge. Let me know if I understood you wrong.



Sorry it wasn't clear enough - changes were recently made to
tests/qemuxml2argvtest.c in order to run the tests using the latest
capabilities rather than needing to pass each capability through the
test. The macros also have a version specific macro which allows for
checking/output from "previous" QEMU releases.

However, the changes only modified macros for x86_64 - so my comment was
if someone felt so inclined to avoid needing/checking specific/certain
caps and only cared that the latest caps did something a certain way,
then adjusting those macros/tests to handle s390* specific things would
perhaps helps achieve that. Furthermore, if previous QEMU versions would
produce different results, then using the version specific checks/output
would provide that support.


Oh, I tried to connect your comment to the vfio-ccw passthrough patch 
series but it seems rather unrelated to me.
Anyway you are right that it could be useful to extend the macros 
DO_TEST_CAPS_LATEST and DO_TEST_CAPS_VER beyond x86_64 arch. I do see 
these macros more like additional test scenarios that are a bit more 
reality oriented. The currently used approach of explicitly setting the 
minimum required capabilities for a single test case is still making 
sense to me as well since it allows to (slightly) narrow down the tested 
code path to the actual to be tested code/feature.



--
Mit freundlichen Grüßen/Kind regards
   Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

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

Re: [libvirt] [PATCH] xenconfig: Remove references to my name and email

[Daniel P . Berrangé]

On Mon, May 07, 2018 at 05:55:51PM +0300, David Kiarie wrote:
> On Mon, May 7, 2018 at 10:29 AM, Peter Krempa  wrote:
> 
> > On Sat, May 05, 2018 at 12:17:05 +0300, David Kiarie wrote:
> > > On Sat, May 5, 2018 at 12:15 PM, David Kiarie 
> > > wrote:
> > >
> > > > Replace references to my name and email with a pseudonym
> > > >
> > >
> > > Sorry, I just want my real name and email off these files and I keep
> > making
> > > silly mistakes.
> >
> > How about just deleting them? We don't really support using pseudonyms.
> >
> 
> Why is that ? With a person reason I don't want my name of these files. I
> wrote the files and it took me a lot of work.
> 
> I can still prove I wrote this code with the above email if anyone wants me
> to as I still work as a developer.

FWIW, the Author lines are largely irrelevant when looking at who authored
code, they are at best incomplete, and at worst outright misleading, so we
tend to recommend people not to pay attention to them. The GIT history
contains the true accurate authorship information and your name is still
recorded there, so the fact that you did the work would not be lost even
without the Author line present. 

If anything I would suggst we could bulk remove them from all code since
because of their incomplete & misleading nature, but that's a larger
can of worms.

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] qemu: add sdl opengl support

[Daniel P . Berrangé]

On Tue, May 08, 2018 at 12:12:42PM +0100, Maciej Wolny wrote:
> On 02/05/18 11:54, Daniel P. Berrangé wrote:
> > On Wed, May 02, 2018 at 11:48:24AM +0100, Maciej Wolny wrote:
> >> On 02/05/18 08:13, Daniel P. Berrangé wrote:
> >>> On Tue, May 01, 2018 at 08:22:45PM +0100, Maciej Wolny wrote:
>  Add SDL graphics gl attribute, modify the domain XML schema, add a
>  test, modify the documentation to include the new option.
> 
>  Signed-off-by: Maciej Wolny 
>  ---
>    docs/schemas/domaincommon.rng  |  8 +
>    src/conf/domain_conf.c | 41 
>  ++
>    src/conf/domain_conf.h |  1 +
>    src/qemu/qemu_capabilities.c   |  2 ++
>    src/qemu/qemu_capabilities.h   |  1 +
>    src/qemu/qemu_command.c| 19 ++
>    .../qemuxml2argvdata/video-virtio-gpu-sdl-gl.args  | 28 +++
>    tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml | 38 
>  
>    tests/qemuxml2argvtest.c   |  5 +++
>    9 files changed, 143 insertions(+)
>    create mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.args
>    create mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml
> 
>  diff --git a/docs/schemas/domaincommon.rng 
>  b/docs/schemas/domaincommon.rng
>  index 3569b9212..a2ef93c09 100644
>  --- a/docs/schemas/domaincommon.rng
>  +++ b/docs/schemas/domaincommon.rng
>  @@ -3031,6 +3031,14 @@
>  
>    
>  
>  +  
>  +
>  +  
>  +
>  +  
>  +  
>  +
>  +  
>    
>    
>  
>  diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>  index b0257068d..7d65ca9df 100644
>  --- a/src/conf/domain_conf.c
>  +++ b/src/conf/domain_conf.c
>  @@ -13448,6 +13448,7 @@ static int
>    virDomainGraphicsDefParseXMLSDL(virDomainGraphicsDefPtr def,
>    xmlNodePtr node)
>    {
>  +xmlNodePtr cur;
>    char *fullscreen = virXMLPropString(node, "fullscreen");
>    int ret = -1;
>  @@ -13468,6 +13469,34 @@ 
>  virDomainGraphicsDefParseXMLSDL(virDomainGraphicsDefPtr def,
>    def->data.sdl.xauth = virXMLPropString(node, "xauth");
>    def->data.sdl.display = virXMLPropString(node, "display");
>  +cur = node->children;
>  +while (cur != NULL) {
>  +if (cur->type == XML_ELEMENT_NODE) {
>  +if (virXMLNodeNameEqual(cur, "gl")) {
>  +char *enable = virXMLPropString(cur, "enable");
>  +int enableVal;
>  +
>  +if (!enable) {
>  +virReportError(VIR_ERR_XML_ERROR, "%s",
>  +   _("sdl gl element missing enable"));
>  +goto cleanup;
>  +}
>  +
>  +enableVal = virTristateBoolTypeFromString(enable);
>  +if (enableVal < 0) {
>  +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>  +   _("unknown enable value '%s'"), 
>  enable);
>  +VIR_FREE(enable);
>  +goto cleanup;
>  +}
>  +VIR_FREE(enable);
>  +
>  +def->data.sdl.gl = enableVal;
>  +}
>  +}
>  +cur = cur->next;
>  +}
>  +
>    ret = 0;
> cleanup:
>    VIR_FREE(fullscreen);
>  @@ -25652,6 +25681,18 @@ virDomainGraphicsDefFormat(virBufferPtr buf,
>    if (def->data.sdl.fullscreen)
>    virBufferAddLit(buf, " fullscreen='yes'");
>  +if (!children && def->data.sdl.gl) {
>  +virBufferAddLit(buf, ">\n");
>  +virBufferAdjustIndent(buf, 2);
>  +children = true;
>  +}
>  +
>  +if (def->data.sdl.gl) {
>  +virBufferAsprintf(buf, "  +  
>  virTristateBoolTypeToString(def->data.sdl.gl));
>  +virBufferAddLit(buf, "/>\n");
>  +}
> >>>
> >>> SPICE  GL support allows a "rendernode" property to be set - I'm wondering
> >>> if that is relevant to SDL or not ?
> >>>
> >>>
> >>> Regards,
> >>> Daniel
> >>>
> >>
> >> I purposefully didn't look into this because we didn't need that option for
> >> our use cases - would you still merge this patch without implementing this
> >> option?
> > 
> > My thought was that if rendernode is also applicable for 

Re: [libvirt] [PATCH] qemu: add sdl opengl support

[Maciej Wolny]

On 02/05/18 11:54, Daniel P. Berrangé wrote:
> On Wed, May 02, 2018 at 11:48:24AM +0100, Maciej Wolny wrote:
>> On 02/05/18 08:13, Daniel P. Berrangé wrote:
>>> On Tue, May 01, 2018 at 08:22:45PM +0100, Maciej Wolny wrote:
 Add SDL graphics gl attribute, modify the domain XML schema, add a
 test, modify the documentation to include the new option.

 Signed-off-by: Maciej Wolny 
 ---
   docs/schemas/domaincommon.rng  |  8 +
   src/conf/domain_conf.c | 41 
 ++
   src/conf/domain_conf.h |  1 +
   src/qemu/qemu_capabilities.c   |  2 ++
   src/qemu/qemu_capabilities.h   |  1 +
   src/qemu/qemu_command.c| 19 ++
   .../qemuxml2argvdata/video-virtio-gpu-sdl-gl.args  | 28 +++
   tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml | 38 
 
   tests/qemuxml2argvtest.c   |  5 +++
   9 files changed, 143 insertions(+)
   create mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.args
   create mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml

 diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
 index 3569b9212..a2ef93c09 100644
 --- a/docs/schemas/domaincommon.rng
 +++ b/docs/schemas/domaincommon.rng
 @@ -3031,6 +3031,14 @@
 
   
 
 +  
 +
 +  
 +
 +  
 +  
 +
 +  
   
   
 
 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index b0257068d..7d65ca9df 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 @@ -13448,6 +13448,7 @@ static int
   virDomainGraphicsDefParseXMLSDL(virDomainGraphicsDefPtr def,
   xmlNodePtr node)
   {
 +xmlNodePtr cur;
   char *fullscreen = virXMLPropString(node, "fullscreen");
   int ret = -1;
 @@ -13468,6 +13469,34 @@ 
 virDomainGraphicsDefParseXMLSDL(virDomainGraphicsDefPtr def,
   def->data.sdl.xauth = virXMLPropString(node, "xauth");
   def->data.sdl.display = virXMLPropString(node, "display");
 +cur = node->children;
 +while (cur != NULL) {
 +if (cur->type == XML_ELEMENT_NODE) {
 +if (virXMLNodeNameEqual(cur, "gl")) {
 +char *enable = virXMLPropString(cur, "enable");
 +int enableVal;
 +
 +if (!enable) {
 +virReportError(VIR_ERR_XML_ERROR, "%s",
 +   _("sdl gl element missing enable"));
 +goto cleanup;
 +}
 +
 +enableVal = virTristateBoolTypeFromString(enable);
 +if (enableVal < 0) {
 +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
 +   _("unknown enable value '%s'"), 
 enable);
 +VIR_FREE(enable);
 +goto cleanup;
 +}
 +VIR_FREE(enable);
 +
 +def->data.sdl.gl = enableVal;
 +}
 +}
 +cur = cur->next;
 +}
 +
   ret = 0;
cleanup:
   VIR_FREE(fullscreen);
 @@ -25652,6 +25681,18 @@ virDomainGraphicsDefFormat(virBufferPtr buf,
   if (def->data.sdl.fullscreen)
   virBufferAddLit(buf, " fullscreen='yes'");
 +if (!children && def->data.sdl.gl) {
 +virBufferAddLit(buf, ">\n");
 +virBufferAdjustIndent(buf, 2);
 +children = true;
 +}
 +
 +if (def->data.sdl.gl) {
 +virBufferAsprintf(buf, ">>> +  
 virTristateBoolTypeToString(def->data.sdl.gl));
 +virBufferAddLit(buf, "/>\n");
 +}
>>>
>>> SPICE  GL support allows a "rendernode" property to be set - I'm wondering
>>> if that is relevant to SDL or not ?
>>>
>>>
>>> Regards,
>>> Daniel
>>>
>>
>> I purposefully didn't look into this because we didn't need that option for
>> our use cases - would you still merge this patch without implementing this
>> option?
> 
> My thought was that if rendernode is also applicable for SPICE, then
> instead of duplicating the struct fields and duplicating parsing, we
> should create a helper method for dealing with the  element that
> can be shared with SPICE & SDL.   If rendernode is not allowed with
> SPICE in QEMU, then your simpler approach is sufficient
> 
> 
> Regards,
> Daniel
> 

Re: [libvirt] unix_sock_dir for virtlogd

[Martin Kletzander]

On Fri, May 04, 2018 at 03:07:41AM +0300, Mathieu Tarral wrote:

Hi,

thanks for your reply Martin,

2018-05-04 1:44 GMT+03:00 Martin Kletzander :

On Thu, May 03, 2018 at 11:03:48PM +0300, Mathieu Tarral wrote:


Hi !

I'm trying to run libvirtd from git, and the daemon is starting now,
but when i try to start a VM (using virt-manager) it complains that
virtlogd socket is unreachable.

The problem is that it tries to reach it at the following location:

virNetSocketNewConnectUNIX:713 : Failed to connect socket to
'/home/tarrma/usr/var/run/libvirt/virtlogd-sock': Connection refused

however, i specified in my $HOME/usr/etc/libvirt/libvirtd.conf


How did you came up with this path?

unix_sock_dir = "/var/run/libvirt"

Then why virt-manager is not using the socket from
/var/run/libvirt/virtlogd.sock ??

Any ideas, where i misconfigured my installation ?



What did you specify as a prefix when building from git?  Did you do
`./autogen.sh --system`?  Or used some custom prefix?


I followed the instructions here:
https://libvirt.org/compiling.html

And as I didn't wanted to replace my Debian binaries, i opted for the
$HOME/usr prefix:

./autogen.sh --prefix=$HOME/usr
make
make install

Now i edited $HOME/usr/etc/libvirt/libvirtd.conf
to set unix_sock_dir = "/var/run/libvirt"



Did you also edit $HOME/usr/etc/libvirt/virtlogd.conf?


And running sudo ./usr/sbin/libvirtd creates the socket at the desired location.
And so does sudo ./usr/sbin/virtlogd.

But somehow, if i connect to qemu:///system with virt-manager, and attempt
to start a VM, it complains that it cannot find virtlogd socket:



The problem is that virLogManagerDaemonPath() doesn't consider any configuration
file, so it will always try to connect to LOCALSTATEDIR 
"/run/libvirt/virtlogd-sock"

This could be fixed, at least partially.  Would you mind creating a bug in our
bugzilla so that we don't lose track of the issue?

Feel free to use:

https://bugzilla.redhat.com/enter_bug.cgi?product=Virtualization%20Tools=libvirt

Or even better, get your first contribution by fixing that ;)

Have a nice day,
Martin

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

Re: [libvirt] [PATCH] xenconfig: Remove references to my name and email

[David Kiarie]

On Tue, May 8, 2018 at 12:22 PM, David Kiarie 
wrote:

>
>
> On Tue, May 8, 2018 at 2:11 AM, Jim Fehlig  wrote:
>
>> On 05/07/2018 08:57 AM, David Kiarie wrote:
>>
>>>
>>>
>>> On Mon, May 7, 2018 at 5:55 PM, David Kiarie >> > wrote:
>>>
>>>
>>>
>>> On Mon, May 7, 2018 at 10:29 AM, Peter Krempa >> > wrote:
>>>
>>> On Sat, May 05, 2018 at 12:17:05 +0300, David Kiarie wrote:
>>> > On Sat, May 5, 2018 at 12:15 PM, David Kiarie <
>>> davidkiar...@gmail.com >
>>> > wrote:
>>> > > > Replace references to my name and email with a
>>> pseudonym
>>> > >
>>> > > Sorry, I just want my real name and email off these
>>> files and I keep making
>>> > silly mistakes.
>>>
>>> How about just deleting them? We don't really support using
>>> pseudonyms.
>>>
>>>
>>> Why is that ? With a person reason I don't want my name of these
>>> files. I
>>> wrote the files and it took me a lot of work.
>>>
>>> I can still prove I wrote this code with the above email if anyone
>>> wants me
>>> to as I still work as a developer.
>>>
>>>
>>> I initally requested to have my name removed from the files but
>>> apparently I hadn't signed off the patch so it was rejected.
>>>
>>
>> You could have simply replied to the mail requesting a SOB and one of us
>> would have added it your patch and pushed it.
>>
>> One month later, I figured it might be a bad idea to just give up all the
>>> work I had done and opted to keep track of it.
>>>
>>
>> I think that is a wise choice, in which case you should leave the
>> authorship as is :-).
>>
>
> I do have a good reason as to why I would like to remove my name from
> these files which I will not bother explaining. And I actually do want my
> name removed from these files.
>
> Removing my name on the other hand means I give up on something which I
> painstakingly worked on for almost one year.
>
> I went for another choice but apparently someone suggested that they don't
> allow pseudonmyns on files and that's just synonymous with me asking you to
> remove my work from from the file.
>

It's just quite unbelieve-able that I am a black person, I trying to build
a career as a software developer and having a lot of problems with people
online - which is irrevant.

I wrote this code as part of GSoC - which is *not* a job and that means I
own the code I wrote and a bunch of professionals are busy making my life
even more difficult.

I need someone to either give me a reason as to why this patch will not be
merged or may be just clearly state that you will not merging this patch. I
don't need any suggestions.


>
>

>
>>
>> Regards,
>> Jim
>>
>
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] xenconfig: Remove references to my name and email

[David Kiarie]

On Tue, May 8, 2018 at 2:11 AM, Jim Fehlig  wrote:

> On 05/07/2018 08:57 AM, David Kiarie wrote:
>
>>
>>
>> On Mon, May 7, 2018 at 5:55 PM, David Kiarie > > wrote:
>>
>>
>>
>> On Mon, May 7, 2018 at 10:29 AM, Peter Krempa > > wrote:
>>
>> On Sat, May 05, 2018 at 12:17:05 +0300, David Kiarie wrote:
>> > On Sat, May 5, 2018 at 12:15 PM, David Kiarie <
>> davidkiar...@gmail.com >
>> > wrote:
>> > > > Replace references to my name and email with a
>> pseudonym
>> > >
>> > > Sorry, I just want my real name and email off these
>> files and I keep making
>> > silly mistakes.
>>
>> How about just deleting them? We don't really support using
>> pseudonyms.
>>
>>
>> Why is that ? With a person reason I don't want my name of these
>> files. I
>> wrote the files and it took me a lot of work.
>>
>> I can still prove I wrote this code with the above email if anyone
>> wants me
>> to as I still work as a developer.
>>
>>
>> I initally requested to have my name removed from the files but
>> apparently I hadn't signed off the patch so it was rejected.
>>
>
> You could have simply replied to the mail requesting a SOB and one of us
> would have added it your patch and pushed it.
>
> One month later, I figured it might be a bad idea to just give up all the
>> work I had done and opted to keep track of it.
>>
>
> I think that is a wise choice, in which case you should leave the
> authorship as is :-).
>

I do have a good reason as to why I would like to remove my name from these
files which I will not bother explaining. And I actually do want my name
removed from these files.

Removing my name on the other hand means I give up on something which I
painstakingly worked on for almost one year.

I went for another choice but apparently someone suggested that they don't
allow pseudonmyns on files and that's just synonymous with me asking you to
remove my work from from the file.


>
> Regards,
> Jim
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list