Re: [libvirt] [RFC PATCH 6/6] qemu: Format pseries.cap-hpt-mps on the command line

[Peter Krempa]

On Wed, May 23, 2018 at 19:09:59 +0200, Andrea Bolognani wrote:
> On Wed, 2018-05-23 at 18:40 +0200, Peter Krempa wrote:
> > On Wed, May 23, 2018 at 18:18:02 +0200, Andrea Bolognani wrote:
> > > +/* QEMU expects the argument to be a number of left shifts:
> > > + * for example, if you wanted to limit the guest to 4 KiB 
> > > pages,
> > > + * since 4096 == 1 << 12, you would need to add 
> > > cap-hpt-mps=12
> > > + * to the command line.
> > 
> > So basically you need to pass the exponent of a power of 2 that yields
> > this number. The number of left shifts may be slightly confusing ...
> 
> I guess it depends on the reader; the two definitions are
> equivalent anyway, so no harm in having both in the comment :)

Using both are fine, but without mentioning that it is in fact a power of
two-only thing I was thinking that the interface is off by one or
something:

4096 == 1 << 12 == 2 << 11

> In general, I'd say it's not the most user-friendly interface on
> QEMU's side, but I believe it's dictated by hardware / emulator
> details, given how it ends up being used: see
> 
>   http://lists.nongnu.org/archive/html/qemu-devel/2018-04/msg02822.html
> 
> To be fair, it would perhaps make sense to perform the conversion
> directly inside QEMU, in order to make it more convenient not only
> for libvirt but for for people driving it directly as well.

If strictly only powers of two make sense for this knob then this gives
you input validation for free. On the other hand, specifying a big
number can overflow internally if it is ever used in the non-exponent
form. I think the format does not matter much, since libvirt's job is to
shield users from such weirdness.


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

Re: [libvirt] [RFC PATCH 5/6] conf: Parse and format HPT maxpagesize

[Peter Krempa]

On Wed, May 23, 2018 at 18:52:57 +0200, Andrea Bolognani wrote:
> On Wed, 2018-05-23 at 18:36 +0200, Peter Krempa wrote:
> > On Wed, May 23, 2018 at 18:18:01 +0200, Andrea Bolognani wrote:
> > > +def->hpt_maxpagesize = VIR_ROUND_UP(def->hpt_maxpagesize, 
> > > 1024);
> > 
> > The code in the patch using it with qemu actually expects so this is a
> > power of 2, so you'll need a different rounding function. 
> 
> Good point!
> 
> Looks like VIR_ROUND_UP_POWER_OF_TWO() will do the trick.

Yes. We usually do the rounding in the code which prepares the
definition for the startup of the VM though, so that other hypervisor
drivers don't need to share the limits of qemu.

Given that this is a ppc64 specific option it probably does not matter
much.


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

Re: [libvirt] [PATCH] news: Add TLS non-shared storage migration

[Peter Krempa]

On Wed, May 23, 2018 at 18:40:00 +0200, Andrea Bolognani wrote:
> On Wed, 2018-05-23 at 16:59 +0200, Peter Krempa wrote:
> > Signed-off-by: Peter Krempa 
> > ---
> >  docs/news.xml | 10 ++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/docs/news.xml b/docs/news.xml
> > index 8f2c7d5dff..329b1c7129 100644
> > --- a/docs/news.xml
> > +++ b/docs/news.xml
> > @@ -54,6 +54,16 @@
> >a QEMU virtual machine.
> >  
> >
> > +  
> > +
> > +  Add support for migration of QEMU VMs with non-shared
> storage over TLS
> 
> This would be more consistent with the rest of the entries, and
> easier to quickly scan over, written as
> 
>   qemu: Add support for migration of VMs with non-shared storage
>   over TLS
> 
> With that changed,

I must say I don't like the lazy format that is usually just copypasted
series heading. In some cases it is not informative enough for
non-developers. Using this format which looks awfully close to the
convention on the mailing list encourages this laziness.

I'll change it for the sake of consistency though.

> 
>   Reviewed-by: Andrea Bolognani 
> 
> -- 
> Andrea Bolognani / Red Hat / Virtualization
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list


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

Re: [libvirt] [RFC PATCH 2/6] conf: Tweak HPT parsing and formatting

[Peter Krempa]

On Wed, May 23, 2018 at 18:50:04 +0200, Andrea Bolognani wrote:
> On Wed, 2018-05-23 at 18:42 +0200, Peter Krempa wrote:
> > On Wed, May 23, 2018 at 18:17:58 +0200, Andrea Bolognani wrote:
> > > +if (hasResizing) {
> > > +if (virAsprintf(, " resizing='%s'",
> > > +virDomainHPTResizingTypeToStri
> > > ng(def->hpt_resizing)) < 0) {
> > > +goto error;
> > > +}
> > > +} else {
> > > +if (VIR_STRDUP(resizing, "") < 0)
> > > +goto error;
> > > +}
> > >  
> > > -virBufferAsprintf(buf, "\n",
> > > -  virDomainHPTResizingTypeToString
> > > (def->hpt_resizing));
> > > +virBufferAsprintf(buf, "\n",
> > 
> > This formulation looks fishy.
> 
> I don't love it either, but I've tried a bunch of alternative
> approaches and this seemed like the most sane to me.
> 
> If you have suggestions on how to improve it, considering that the
> end result is what you see after patch 5/6, please do share! :)

virXMLFormatElement automatically closes the tag if the provided
'attrBuf' is empty. Currently it will not work for this particular case
but I think it is worth to add a version which will format the element
even if both buffers are empty.


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

[libvirt] [PATCH V2] tests: xmconfigtest: add tests for cmdline formating

[Jim Fehlig]

Commit 656151bf fixed formatting of the  element. Perhaps it
would have been noticed and fixed earlier if we had a test. With this
change, all possible cases of formating  from xmconfig are
covered

1. no 'extra=' or 'root=' in xm.cfg
2. 'extra=' but no 'root=' in xm.cfg
3. 'root=' but no 'extra=' in xm.cfg
4. both 'root=' and 'extra=' in xm.cfg

Case 1 is covered by all existing paravirt tests since they have no
'extra=' or 'root='. Case 2 is covered by adding 'extra=' to a few
of the existing paravirt tests. Cases 3 and 4 are covered by new
tests that only test conversion of xm.cfg to xml.

Signed-off-by: Jim Fehlig 
---
 tests/xmconfigdata/test-paravirt-extra-root.cfg | 13 ++
 tests/xmconfigdata/test-paravirt-extra-root.xml | 34 +
 tests/xmconfigdata/test-paravirt-maxvcpus.cfg   |  1 +
 tests/xmconfigdata/test-paravirt-maxvcpus.xml   |  1 +
 tests/xmconfigdata/test-paravirt-root.cfg   | 12 +
 tests/xmconfigdata/test-paravirt-root.xml   | 34 +
 tests/xmconfigdata/test-paravirt-vcpu.cfg   |  1 +
 tests/xmconfigdata/test-paravirt-vcpu.xml   |  1 +
 tests/xmconfigtest.c|  2 ++
 9 files changed, 99 insertions(+)
 create mode 100644 tests/xmconfigdata/test-paravirt-extra-root.cfg
 create mode 100644 tests/xmconfigdata/test-paravirt-extra-root.xml
 create mode 100644 tests/xmconfigdata/test-paravirt-root.cfg
 create mode 100644 tests/xmconfigdata/test-paravirt-root.xml

diff --git a/tests/xmconfigdata/test-paravirt-extra-root.cfg 
b/tests/xmconfigdata/test-paravirt-extra-root.cfg
new file mode 100644
index 00..2569f2f22c
--- /dev/null
+++ b/tests/xmconfigdata/test-paravirt-extra-root.cfg
@@ -0,0 +1,13 @@
+name = "XenGuest1"
+uuid = "c7a5fdb0-cdaf-9455-926a-d65c16db1809"
+memory = 512
+vcpus = 2
+localtime = 0
+on_poweroff = "destroy"
+on_reboot = "restart"
+on_crash = "restart"
+vif = [ "mac=00:16:3e:66:94:9c,bridge=br0,script=vif-bridge" ]
+bootloader = "/usr/bin/pygrub"
+root = "/dev/xvda2"
+extra = "console=hvc0"
+disk = [ "phy:/dev/HostVG/XenGuest1,xvda,w" ]
diff --git a/tests/xmconfigdata/test-paravirt-extra-root.xml 
b/tests/xmconfigdata/test-paravirt-extra-root.xml
new file mode 100644
index 00..325b07d8e1
--- /dev/null
+++ b/tests/xmconfigdata/test-paravirt-extra-root.xml
@@ -0,0 +1,34 @@
+
+  XenGuest1
+  c7a5fdb0-cdaf-9455-926a-d65c16db1809
+  524288
+  524288
+  2
+  /usr/bin/pygrub
+  
+linux
+root=/dev/xvda2 console=hvc0
+  
+  
+  destroy
+  restart
+  restart
+  
+
+  
+  
+  
+
+
+  
+  
+  
+
+
+  
+
+
+
+
+  
+
diff --git a/tests/xmconfigdata/test-paravirt-maxvcpus.cfg 
b/tests/xmconfigdata/test-paravirt-maxvcpus.cfg
index 8d1ac4d786..f06db61171 100644
--- a/tests/xmconfigdata/test-paravirt-maxvcpus.cfg
+++ b/tests/xmconfigdata/test-paravirt-maxvcpus.cfg
@@ -10,4 +10,5 @@ on_reboot = "restart"
 on_crash = "restart"
 vif = [ "mac=00:16:3e:66:94:9c,bridge=br0,script=vif-bridge" ]
 bootloader = "/usr/bin/pygrub"
+extra = "console=hvc0"
 disk = [ "phy:/dev/HostVG/XenGuest1,xvda,w" ]
diff --git a/tests/xmconfigdata/test-paravirt-maxvcpus.xml 
b/tests/xmconfigdata/test-paravirt-maxvcpus.xml
index ce66503dc5..3012821ad4 100644
--- a/tests/xmconfigdata/test-paravirt-maxvcpus.xml
+++ b/tests/xmconfigdata/test-paravirt-maxvcpus.xml
@@ -7,6 +7,7 @@
   /usr/bin/pygrub
   
 linux
+console=hvc0
   
   
   destroy
diff --git a/tests/xmconfigdata/test-paravirt-root.cfg 
b/tests/xmconfigdata/test-paravirt-root.cfg
new file mode 100644
index 00..8cdfbd9384
--- /dev/null
+++ b/tests/xmconfigdata/test-paravirt-root.cfg
@@ -0,0 +1,12 @@
+name = "XenGuest1"
+uuid = "c7a5fdb0-cdaf-9455-926a-d65c16db1809"
+memory = 512
+vcpus = 2
+localtime = 0
+on_poweroff = "destroy"
+on_reboot = "restart"
+on_crash = "restart"
+vif = [ "mac=00:16:3e:66:94:9c,bridge=br0,script=vif-bridge" ]
+bootloader = "/usr/bin/pygrub"
+root = "/dev/xvda2"
+disk = [ "phy:/dev/HostVG/XenGuest1,xvda,w" ]
diff --git a/tests/xmconfigdata/test-paravirt-root.xml 
b/tests/xmconfigdata/test-paravirt-root.xml
new file mode 100644
index 00..12d1e3bdbd
--- /dev/null
+++ b/tests/xmconfigdata/test-paravirt-root.xml
@@ -0,0 +1,34 @@
+
+  XenGuest1
+  c7a5fdb0-cdaf-9455-926a-d65c16db1809
+  524288
+  524288
+  2
+  /usr/bin/pygrub
+  
+linux
+root=/dev/xvda2
+  
+  
+  destroy
+  restart
+  restart
+  
+
+  
+  
+  
+
+
+  
+  
+  
+
+
+  
+
+
+
+
+  
+
diff --git a/tests/xmconfigdata/test-paravirt-vcpu.cfg 
b/tests/xmconfigdata/test-paravirt-vcpu.cfg
index 8d1ac4d786..f06db61171 100644
--- a/tests/xmconfigdata/test-paravirt-vcpu.cfg
+++ b/tests/xmconfigdata/test-paravirt-vcpu.cfg
@@ -10,4 +10,5 @@ on_reboot = "restart"
 on_crash = "restart"
 vif = [ "mac=00:16:3e:66:94:9c,bridge=br0,script=vif-bridge" ]
 bootloader = "/usr/bin/pygrub"
+extra = 

[libvirt] [PATCH v6 4/9] qemu/cgroup: add /dev/sev in shared devices list

[Brijesh Singh]

QEMU uses /dev/sev device while creating the SEV guest, lets add /dev/sev
in the list of devices allowed to be accessed by the QEMU.

Signed-off-by: Brijesh Singh <>
---
 docs/drvqemu.html.in   | 1 +
 src/qemu/qemu.conf | 2 +-
 src/qemu/qemu_cgroup.c | 2 +-
 src/qemu/test_libvirtd_qemu.aug.in | 1 +
 4 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/docs/drvqemu.html.in b/docs/drvqemu.html.in
index cbd159da3e10..7c33a1830c7b 100644
--- a/docs/drvqemu.html.in
+++ b/docs/drvqemu.html.in
@@ -397,6 +397,7 @@ chmod o+x /path/to/directory
 /dev/random, /dev/urandom,
 /dev/ptmx, /dev/kvm, /dev/kqemu,
 /dev/rtc, /dev/hpet, /dev/net/tun
+/dev/sev
 
 
 
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index 31738ff19cfc..c6aeeb610310 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -451,7 +451,7 @@
 #"/dev/null", "/dev/full", "/dev/zero",
 #"/dev/random", "/dev/urandom",
 #"/dev/ptmx", "/dev/kvm", "/dev/kqemu",
-#"/dev/rtc","/dev/hpet"
+#"/dev/rtc","/dev/hpet", "/dev/sev"
 #]
 #
 # RDMA migration requires the following extra files to be added to the list:
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 546a4c8e6330..21dcb3cefe37 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -47,7 +47,7 @@ const char *const defaultDeviceACL[] = {
 "/dev/null", "/dev/full", "/dev/zero",
 "/dev/random", "/dev/urandom",
 "/dev/ptmx", "/dev/kvm", "/dev/kqemu",
-"/dev/rtc", "/dev/hpet",
+"/dev/rtc", "/dev/hpet", "/dev/sev",
 NULL,
 };
 #define DEVICE_PTY_MAJOR 136
diff --git a/src/qemu/test_libvirtd_qemu.aug.in 
b/src/qemu/test_libvirtd_qemu.aug.in
index 95885e9f06ae..847d77ae0622 100644
--- a/src/qemu/test_libvirtd_qemu.aug.in
+++ b/src/qemu/test_libvirtd_qemu.aug.in
@@ -60,6 +60,7 @@ module Test_libvirtd_qemu =
 { "8" = "/dev/kqemu" }
 { "9" = "/dev/rtc" }
 { "10" = "/dev/hpet" }
+{ "11" = "/dev/sev" }
 }
 { "save_image_format" = "raw" }
 { "dump_image_format" = "raw" }
-- 
2.14.3

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

[libvirt] [PATCH v6 7/9] remote: implement the remote protocol for launch security

[Brijesh Singh]

Add remote support for launch security info.

Signed-off-by: Brijesh Singh 
---
 src/remote/remote_daemon_dispatch.c | 47 +
 src/remote/remote_driver.c  | 42 -
 src/remote/remote_protocol.x| 20 +++-
 src/remote_protocol-structs | 11 +
 4 files changed, 118 insertions(+), 2 deletions(-)

diff --git a/src/remote/remote_daemon_dispatch.c 
b/src/remote/remote_daemon_dispatch.c
index a8a5932d7134..30b22fbce092 100644
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
@@ -3109,6 +3109,53 @@ remoteDispatchNodeGetMemoryStats(virNetServerPtr server 
ATTRIBUTE_UNUSED,
 return rv;
 }
 
+static int
+remoteDispatchDomainGetLaunchSecurityInfo(virNetServerPtr server 
ATTRIBUTE_UNUSED,
+  virNetServerClientPtr client 
ATTRIBUTE_UNUSED,
+  virNetMessagePtr msg 
ATTRIBUTE_UNUSED,
+  virNetMessageErrorPtr rerr,
+  
remote_domain_get_launch_security_info_args *args,
+  
remote_domain_get_launch_security_info_ret *ret)
+{
+virDomainPtr dom = NULL;
+virTypedParameterPtr params = NULL;
+int nparams = 0;
+int rv = -1;
+struct daemonClientPrivate *priv =
+virNetServerClientGetPrivateData(client);
+
+if (!priv->conn) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
+goto cleanup;
+}
+
+if (!(dom = get_nonnull_domain(priv->conn, args->dom)))
+goto cleanup;
+
+if (virDomainGetLaunchSecurityInfo(dom, , , args->flags) < 
0)
+goto cleanup;
+
+if (nparams > REMOTE_DOMAIN_LAUNCH_SECURITY_INFO_PARAMS_MAX) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large"));
+goto cleanup;
+}
+
+if (virTypedParamsSerialize(params, nparams,
+(virTypedParameterRemotePtr *) 
>params.params_val,
+>params.params_len,
+args->flags) < 0)
+goto cleanup;
+
+rv = 0;
+
+ cleanup:
+if (rv < 0)
+virNetMessageSaveError(rerr);
+virTypedParamsFree(params, nparams);
+virObjectUnref(dom);
+return rv;
+}
+
 static int
 remoteDispatchDomainGetPerfEvents(virNetServerPtr server ATTRIBUTE_UNUSED,
   virNetServerClientPtr client 
ATTRIBUTE_UNUSED,
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 95437b43657e..fc649276799e 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -1965,6 +1965,45 @@ remoteDomainGetNumaParameters(virDomainPtr domain,
 return rv;
 }
 
+static int
+remoteDomainGetLaunchSecurityInfo(virDomainPtr domain,
+  virTypedParameterPtr *params,
+  int *nparams,
+  unsigned int flags)
+{
+int rv = -1;
+remote_domain_get_launch_security_info_args args;
+remote_domain_get_launch_security_info_ret ret;
+struct private_data *priv = domain->conn->privateData;
+
+remoteDriverLock(priv);
+
+make_nonnull_domain(, domain);
+args.flags = flags;
+
+memset(, 0, sizeof(ret));
+if (call(domain->conn, priv, 0, 
REMOTE_PROC_DOMAIN_GET_LAUNCH_SECURITY_INFO,
+ (xdrproc_t) xdr_remote_domain_get_launch_security_info_args, 
(char *) ,
+ (xdrproc_t) xdr_remote_domain_get_launch_security_info_ret, (char 
*) ) == -1)
+goto done;
+
+if (virTypedParamsDeserialize((virTypedParameterRemotePtr) 
ret.params.params_val,
+  ret.params.params_len,
+  
REMOTE_DOMAIN_LAUNCH_SECURITY_INFO_PARAMS_MAX,
+  params,
+  nparams) < 0)
+goto cleanup;
+
+rv = 0;
+
+ cleanup:
+xdr_free((xdrproc_t) xdr_remote_domain_get_launch_security_info_ret,
+ (char *) );
+ done:
+remoteDriverUnlock(priv);
+return rv;
+}
+
 static int
 remoteDomainGetPerfEvents(virDomainPtr domain,
   virTypedParameterPtr *params,
@@ -8448,7 +8487,8 @@ static virHypervisorDriver hypervisor_driver = {
 .domainSetGuestVcpus = remoteDomainSetGuestVcpus, /* 2.0.0 */
 .domainSetVcpu = remoteDomainSetVcpu, /* 3.1.0 */
 .domainSetBlockThreshold = remoteDomainSetBlockThreshold, /* 3.2.0 */
-.domainSetLifecycleAction = remoteDomainSetLifecycleAction /* 3.9.0 */
+.domainSetLifecycleAction = remoteDomainSetLifecycleAction, /* 3.9.0 */
+.domainGetLaunchSecurityInfo = remoteDomainGetLaunchSecurityInfo /* 4.4.0 
*/
 };
 
 static virNetworkDriver network_driver = {
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 

[libvirt] [PATCH v6 9/9] virsh: implement new command for launch security

[Brijesh Singh]

Add new 'launch-security' command, the command can be used to get or set
the launch security information when booting encrypted VMs.

Signed-off-by: Brijesh Singh 
---
 tools/virsh-domain.c | 81 
 tools/virsh.pod  |  5 
 2 files changed, 86 insertions(+)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index cfbbf5a7bc39..27bb702c8bb7 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -13870,6 +13870,81 @@ cmdDomFSInfo(vshControl *ctl, const vshCmd *cmd)
 return ret >= 0;
 }
 
+/*
+ * "launch-security" command
+ */
+static const vshCmdInfo info_launch_security[] = {
+{.name = "help",
+.data = N_("Get or set launch-security information")
+},
+{.name = "desc",
+.data = N_("Get or set the current launch-security information for "
+   "a guest domain.\n"
+   "To get the launch-security information use following"
+   "command: \n\n"
+   "virsh # launch-security ")
+},
+{.name = NULL}
+};
+
+static const vshCmdOptDef opts_launch_security[] = {
+VIRSH_COMMON_OPT_DOMAIN_FULL(0),
+VIRSH_COMMON_OPT_DOMAIN_CONFIG,
+VIRSH_COMMON_OPT_DOMAIN_LIVE,
+VIRSH_COMMON_OPT_DOMAIN_CURRENT,
+{.name = NULL}
+};
+
+static void
+virshPrintLaunchSecurityInfo(vshControl *ctl, virTypedParameterPtr params,
+ int nparams)
+{
+size_t i;
+
+for (i = 0; i < nparams; i++) {
+if (params[i].type == VIR_TYPED_PARAM_STRING)
+vshPrintExtra(ctl, "%-15s: %s\n", params[i].field, 
params[i].value.s);
+}
+}
+
+static bool
+cmdLaunchSecurity(vshControl *ctl, const vshCmd *cmd)
+{
+virDomainPtr dom;
+int nparams = 0;
+virTypedParameterPtr params = NULL;
+bool ret = false;
+unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
+bool current = vshCommandOptBool(cmd, "current");
+bool config = vshCommandOptBool(cmd, "config");
+bool live = vshCommandOptBool(cmd, "live");
+
+VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+
+if (config)
+flags |= VIR_DOMAIN_AFFECT_CONFIG;
+if (live)
+flags |= VIR_DOMAIN_AFFECT_LIVE;
+
+if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
+return false;
+
+if (virDomainGetLaunchSecurityInfo(dom, , , flags) != 0) {
+vshError(ctl, "%s", _("Unable to get launch security info"));
+goto cleanup;
+}
+
+virshPrintLaunchSecurityInfo(ctl, params, nparams);
+
+ret = true;
+ cleanup:
+virTypedParamsFree(params, nparams);
+virshDomainFree(dom);
+return ret;
+}
+
+
 const vshCmdDef domManagementCmds[] = {
 {.name = "attach-device",
  .handler = cmdAttachDevice,
@@ -14485,5 +14560,11 @@ const vshCmdDef domManagementCmds[] = {
  .info = info_domblkthreshold,
  .flags = 0
 },
+{.name = "launch-security-info",
+ .handler = cmdLaunchSecurity,
+ .opts = opts_launch_security,
+ .info = info_launch_security,
+ .flags = 0
+},
 {.name = NULL}
 };
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 929958a9533c..31bb26bda2ac 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2899,6 +2899,11 @@ See B for information on I.
 Output the IP address and port number for the VNC display. If the information
 is not available the processes will provide an exit code of 1.
 
+=item B I
+
+Get the measurement of the memory contents encrypted through the launch
+sequence when I is provided.
+
 =back
 
 =head1 DEVICE COMMANDS
-- 
2.14.3

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

[libvirt] [PATCH v6 2/9] qemu: introduce SEV feature in hypervisor capabilities

[Brijesh Singh]

Extend hypervisor capabilities to include sev feature. When available,
hypervisor supports launching an encrypted VM on AMD platform. The
sev feature tag provides additional details like Platform Diffie-Hellman
(PDH) key and certificate chain which can be used by the guest owner to
establish a cryptographic session with the SEV firmware to negotiate
keys used for attestation or to provide secret during launch.

Signed-off-by: Brijesh Singh 
---
 docs/formatdomaincaps.html.in  | 40 
 docs/schemas/domaincaps.rng| 20 
 src/conf/domain_capabilities.c | 20 
 src/conf/domain_capabilities.h |  1 +
 src/qemu/qemu_capabilities.c   |  2 ++
 5 files changed, 83 insertions(+)

diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in
index b68ae4b4f1f3..f37b059ba6b1 100644
--- a/docs/formatdomaincaps.html.in
+++ b/docs/formatdomaincaps.html.in
@@ -434,6 +434,12 @@
   /enum
 /gic
 vmcoreinfo supported='yes'/
+sev
+  pdhUWxKSlNrVlRTRk5KVGtkSVFVMUU=/pdh
+  
cert-chainVVd4S1NsTnJWbFJUUms1S1ZHdGtTVkZWTVVVPQ==/cert-chain
+  cbitpos47/cbitpos
+  reduced-phys-bits1/reduced-phys-bits
+/sev
   /features
 /domainCapabilities
 
@@ -462,5 +468,39 @@
 
 Reports whether the vmcoreinfo feature can be enabled
 
+SEV capabilities
+
+AMD Secure Encrypted Virtualization (SEV) capabilities are exposed under
+the sev element.
+SEV is an extension to the AMD-V architecture which supports running
+virtual machines (VMs) under the control of a hypervisor. When supported,
+guest owner can create a VM whose memory contents will be transparently
+encrypted with a key unique to that VM.
+
+
+For more details on SEV feature see:
+  https://support.amd.com/TechDocs/55766_SEV-KM%20API_Specification.pdf;>
+SEV API spec and http://amd-dev.wpengine.netdna-cdn.com/wordpress/media/2013/12/AMD_Memory_Encryption_Whitepaper_v7-Public.pdf;>
+SEV White Paper
+
+
+
+  pdh
+  A base64 encoded platform Diffie-Hellman public key which can be
+  exported to remote entities that desire to establish a secure transport
+  context with the SEV platform in order to transmit data securely.
+  cert-chain
+   A base64 encoded platform certificate chain that includes the 
platform
+  endorsement key (PEK), owners certificate authority (OCD), and chip
+  endorsement key (CEK).
+  cbitpos
+  When memory encryption is enabled, one of the physical address bits
+  (aka the C-bit) is utilized to mark if a memory page is protected. The
+  C-bit position is Hypervisor dependent.
+  reduced-phys-bits
+  When memory encryption is enabled, we lose certain bits in physical
+  address space. The number of bits we lose is hypervisor dependent.
+
+
   
 
diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng
index 5913d711a3fe..26265645b82a 100644
--- a/docs/schemas/domaincaps.rng
+++ b/docs/schemas/domaincaps.rng
@@ -184,6 +184,9 @@
   
 
 
+
+
+
   
 
   
@@ -201,6 +204,23 @@
 
   
 
+  
+
+  
+
+  
+  
+
+  
+  
+
+  
+  
+
+  
+
+  
+
   
 
   
diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index 6e2ab0a28796..3b767c45cbb3 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -542,6 +542,25 @@ virDomainCapsFeatureGICFormat(virBufferPtr buf,
 FORMAT_EPILOGUE(gic);
 }
 
+static void
+virDomainCapsFeatureSEVFormat(virBufferPtr buf,
+  virSEVCapabilityPtr const sev)
+{
+if (!sev)
+return;
+
+virBufferAddLit(buf, "\n");
+virBufferAdjustIndent(buf, 2);
+virBufferAsprintf(buf, "%d\n", sev->cbitpos);
+virBufferAsprintf(buf, "%d\n",
+  sev->reduced_phys_bits);
+virBufferEscapeString(buf, "%s\n", sev->pdh);
+virBufferEscapeString(buf, "%s\n",
+  sev->cert_chain);
+virBufferAdjustIndent(buf, -2);
+virBufferAddLit(buf, "\n");
+}
+
 
 char *
 virDomainCapsFormat(virDomainCapsPtr const caps)
@@ -585,6 +604,7 @@ virDomainCapsFormat(virDomainCapsPtr const caps)
 virDomainCapsFeatureGICFormat(, >gic);
 virBufferAsprintf(, "\n",
   caps->vmcoreinfo ? "yes" : "no");
+virDomainCapsFeatureSEVFormat(, caps->sev);
 
 virBufferAdjustIndent(, -2);
 virBufferAddLit(, "\n");
diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
index c1093234ceb8..e33bef525ef4 100644
--- a/src/conf/domain_capabilities.h
+++ b/src/conf/domain_capabilities.h
@@ -172,6 +172,7 @@ struct _virDomainCaps {
 
 virDomainCapsFeatureGIC gic;
 bool vmcoreinfo;
+virSEVCapabilityPtr sev;
 /* add new domain features here */
 };
 
diff --git 

[libvirt] [PATCH v6 8/9] qemu: Add support to launch security info

[Brijesh Singh]

This patch implements the internal driver API for launch event into
qemu driver. When SEV is enabled, execute 'query-sev-launch-measurement'
to get the measurement of memory encrypted through launch sequence.

Signed-off-by: Brijesh Singh 
---
 src/qemu/qemu_driver.c   | 68 
 src/qemu/qemu_monitor.c  |  8 ++
 src/qemu/qemu_monitor.h  |  3 ++
 src/qemu/qemu_monitor_json.c | 42 +++
 src/qemu/qemu_monitor_json.h |  2 ++
 5 files changed, 123 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3a328e5d4679..6569dea32fce 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -21194,6 +21194,73 @@ qemuDomainSetLifecycleAction(virDomainPtr dom,
 }
 
 
+static int
+qemuDomainGetSevMeasurement(virQEMUDriverPtr driver,
+virDomainObjPtr vm,
+virTypedParameterPtr *params,
+int *nparams,
+unsigned int flags)
+{
+int ret = -1;
+char *tmp;
+int maxpar = 0;
+
+virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1);
+
+if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
+return -1;
+
+if (qemuDomainObjEnterMonitorAsync(driver, vm, QEMU_ASYNC_JOB_NONE) < 0)
+goto endjob;
+
+tmp = qemuMonitorGetSevMeasurement(QEMU_DOMAIN_PRIVATE(vm)->mon);
+if (tmp == NULL)
+goto endjob;
+
+if (qemuDomainObjExitMonitor(driver, vm) < 0)
+goto endjob;
+
+if (virTypedParamsAddString(params, nparams, ,
+VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT,
+tmp) < 0)
+goto endjob;
+
+ret = 0;
+
+ endjob:
+qemuDomainObjEndJob(driver, vm);
+return ret;
+}
+
+
+static int
+qemuDomainGetLaunchSecurityInfo(virDomainPtr domain,
+virTypedParameterPtr *params,
+int *nparams,
+unsigned int flags)
+{
+virQEMUDriverPtr driver = domain->conn->privateData;
+virDomainObjPtr vm;
+int ret = -1;
+
+if (!(vm = qemuDomObjFromDomain(domain)))
+goto cleanup;
+
+if (virDomainGetLaunchSecurityInfoEnsureACL(domain->conn, vm->def) < 0)
+goto cleanup;
+
+if (vm->def->sev) {
+if (qemuDomainGetSevMeasurement(driver, vm, params, nparams, flags) < 
0)
+goto cleanup;
+}
+
+ret = 0;
+
+ cleanup:
+virDomainObjEndAPI();
+return ret;
+}
+
 static virHypervisorDriver qemuHypervisorDriver = {
 .name = QEMU_DRIVER_NAME,
 .connectURIProbe = qemuConnectURIProbe,
@@ -21414,6 +21481,7 @@ static virHypervisorDriver qemuHypervisorDriver = {
 .domainSetVcpu = qemuDomainSetVcpu, /* 3.1.0 */
 .domainSetBlockThreshold = qemuDomainSetBlockThreshold, /* 3.2.0 */
 .domainSetLifecycleAction = qemuDomainSetLifecycleAction, /* 3.9.0 */
+.domainGetLaunchSecurityInfo = qemuDomainGetLaunchSecurityInfo, /* 4.2.0 */
 };
 
 
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 3b034930408c..977cbe5a41f8 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4226,3 +4226,11 @@ qemuMonitorBlockdevDel(qemuMonitorPtr mon,
 
 return qemuMonitorJSONBlockdevDel(mon, nodename);
 }
+
+char *
+qemuMonitorGetSevMeasurement(qemuMonitorPtr mon)
+{
+QEMU_CHECK_MONITOR_NULL(mon);
+
+return qemuMonitorJSONGetSevMeasurement(mon);
+}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index b1b7ef09c929..8a64ae5f3d96 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1137,4 +1137,7 @@ int qemuMonitorBlockdevAdd(qemuMonitorPtr mon,
 int qemuMonitorBlockdevDel(qemuMonitorPtr mon,
const char *nodename);
 
+char *
+qemuMonitorGetSevMeasurement(qemuMonitorPtr mon);
+
 #endif /* QEMU_MONITOR_H */
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 24d3a2ff412f..041f595ca1e4 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -8024,3 +8024,45 @@ qemuMonitorJSONBlockdevDel(qemuMonitorPtr mon,
 virJSONValueFree(reply);
 return ret;
 }
+
+/**
+ * The function is used to retrieve the measurement of SEV guest.
+ * The measurement is signature of the memory contents that was encrypted
+ * through the SEV launch flow.
+ *
+ * A example jason output:
+ *
+ * { "execute" : "query-sev-launch-measure" }
+ * { "return" : { "data" : "4l8LXeNlSPUDlXPJG5966/8%YZ" } }
+ */
+char *
+qemuMonitorJSONGetSevMeasurement(qemuMonitorPtr mon)
+{
+const char *tmp;
+char *measurement = NULL;
+virJSONValuePtr cmd;
+virJSONValuePtr reply = NULL;
+virJSONValuePtr data;
+
+if (!(cmd = qemuMonitorJSONMakeCommand("query-sev-launch-measure", NULL)))
+ return NULL;
+
+if (qemuMonitorJSONCommand(mon, cmd, ) < 0)
+goto cleanup;
+
+if 

[libvirt] [PATCH v6 3/9] conf: introduce launch-security element in domain

[Brijesh Singh]

The launch-security element can be used to define the security
model to use when launching a domain. Currently we support 'sev'.

When 'sev' is used, the VM will be launched with AMD SEV feature enabled.
SEV feature supports running encrypted VM under the control of KVM.
Encrypted VMs have their pages (code and data) secured such that only the
guest itself has access to the unencrypted version. Each encrypted VM is
associated with a unique encryption key; if its data is accessed to a
different entity using a different key the encrypted guests data will be
incorrectly decrypted, leading to unintelligible data.

Signed-off-by: Brijesh Singh 
---
 docs/formatdomain.html.in  | 115 ++
 docs/schemas/domaincommon.rng  |  39 ++
 src/conf/domain_conf.c | 133 +
 src/conf/domain_conf.h |  27 +
 tests/genericxml2xmlindata/launch-security-sev.xml |  24 
 tests/genericxml2xmltest.c |   2 +
 6 files changed, 340 insertions(+)
 create mode 100644 tests/genericxml2xmlindata/launch-security-sev.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 665d0f25293e..cab08ea52003 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -8310,6 +8310,121 @@ qemu-kvm -net nic,model=? /dev/null
 
 Note: DEA/TDEA is synonymous with DES/TDES.
 
+Secure Encrypted Virtualization (SEV)
+
+
+   The contents of the launch-security type='sev' 
element
+   is used to provide the guest owners input used for creating an encrypted
+   VM using the AMD SEV feature.
+
+   SEV is an extension to the AMD-V architecture which supports running
+   encrypted virtual machine (VMs) under the control of KVM. Encrypted
+   VMs have their pages (code and data) secured such that only the guest
+   itself has access to the unencrypted version. Each encrypted VM is
+   associated with a unique encryption key; if its data is accessed to a
+   different entity using a different key the encrypted guests data will
+   be incorrectly decrypted, leading to unintelligible data.
+
+   For more information see various input parameters and its format see 
the SEV API spec
+   https://support.amd.com/TechDocs/55766_SEV-KM%20API_Specification.pdf;> 
https://support.amd.com/TechDocs/55766_SEV-KM%20API_Specification.pdf 
+   Since 4.4.0
+   
+
+domain
+  ...
+  launch-security type='sev'
+policy 0x0001 /policy
+cbitpos 47 /cbitpos
+reduced-phys-bits 1 /reduced-phys-bits
+session AAACCCDD=FFFCCCDSDS /session
+dh-cert RBBBSDDD=FDDCCCDDDG /dh
+  /sev
+  ...
+/domain
+
+
+
+  cbitpos
+  The required cbitpos element provides the C-bit (aka 
encryption bit)
+  location in guest page table entry. The value of cbitpos is
+  hypervisor dependent and can be obtained through the sev 
element
+  from the domain capabilities.
+  
+  reduced-phys-bits
+  The required reduced-phys-bits element provides the 
physical
+  address bit reducation. Similar to cbitpos the value of 

+  reduced-phys-bit is hypervisor dependent and can be obtained
+  through the sev element from the domain capabilities.
+  
+  policy
+  The required policy element provides the guest policy
+  which must be maintained by the SEV firmware. This policy is enforced by
+  the firmware and restricts what configuration and operational commands
+  can be performed on this guest by the hypervisor. The guest policy
+  provided during guest launch is bound to the guest and cannot be changed
+  throughout the lifetime of the guest. The policy is also transmitted
+  during snapshot and migration flows and enforced on the destination 
platform.
+
+  The guest policy is a 4 unsigned byte with the fields shown in Table:
+
+  
+
+   Bit(s) 
+   Description 
+
+
+   0 
+   Debugging of the guest is disallowed when set 
+
+
+   1 
+   Sharing keys with other guests is disallowed when set 
+
+
+   2 
+   SEV-ES is required when set
+
+
+   3 
+   Sending the guest to another platform is disallowed when 
set
+
+
+   4 
+   The guest must not be transmitted to another platform that is
+   not in the domain when set. 
+
+
+   5 
+   The guest must not be transmitted to another platform that is
+   not SEV capable when set. 
+
+
+   15:6 
+   reserved 
+
+
+   16:32 
+   The guest must not be transmitted to another platform with a
+   lower firmware version. 
+
+  
+
+  
+  dh-cert
+  The 

[libvirt] [PATCH v6 1/9] qemu: provide support to query the SEV capability

[Brijesh Singh]

QEMU version >= 2.12 provides support for launching an encrypted VMs on
AMD x86 platform using Secure Encrypted Virtualization (SEV) feature.
This patch adds support to query the SEV capability from the qemu.

Signed-off-by: Brijesh Singh 
---
 src/conf/domain_capabilities.h | 13 
 src/qemu/qemu_capabilities.c   | 47 ++
 src/qemu/qemu_capabilities.h   |  4 ++
 src/qemu/qemu_capspriv.h   |  4 ++
 src/qemu/qemu_monitor.c|  9 +++
 src/qemu/qemu_monitor.h|  3 +
 src/qemu/qemu_monitor_json.c   | 74 ++
 src/qemu/qemu_monitor_json.h   |  3 +
 .../caps_2.12.0.x86_64.replies | 10 +++
 tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml  |  3 +-
 10 files changed, 169 insertions(+), 1 deletion(-)

diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
index 9b852e8649bf..c1093234ceb8 100644
--- a/src/conf/domain_capabilities.h
+++ b/src/conf/domain_capabilities.h
@@ -137,6 +137,19 @@ struct _virDomainCapsCPU {
 virDomainCapsCPUModelsPtr custom;
 };
 
+/*
+ * SEV capabilities
+ */
+typedef struct _virSEVCapability virSEVCapability;
+typedef virSEVCapability *virSEVCapabilityPtr;
+struct _virSEVCapability {
+char *pdh;
+char *cert_chain;
+unsigned int cbitpos;
+unsigned int reduced_phys_bits;
+};
+
+
 struct _virDomainCaps {
 virObjectLockable parent;
 
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 8a63db5f4f33..49b74f7e12c1 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -489,6 +489,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   "screendump_device",
   "hda-output",
   "blockdev-del",
+  "sev-guest",
 );
 
 
@@ -555,6 +556,8 @@ struct _virQEMUCaps {
 size_t ngicCapabilities;
 virGICCapability *gicCapabilities;
 
+virSEVCapability *sevCapabilities;
+
 virQEMUCapsHostCPUData kvmCPU;
 virQEMUCapsHostCPUData tcgCPU;
 };
@@ -1121,6 +1124,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
 { "virtual-css-bridge", QEMU_CAPS_CCW },
 { "vfio-ccw", QEMU_CAPS_DEVICE_VFIO_CCW },
 { "hda-output", QEMU_CAPS_HDA_OUTPUT },
+{ "sev-guest", QEMU_CAPS_SEV_GUEST },
 };
 
 static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsVirtioBalloon[] = {
@@ -2050,6 +2054,28 @@ virQEMUCapsSetGICCapabilities(virQEMUCapsPtr qemuCaps,
 }
 
 
+void
+virQEMUSevCapabilitiesFree(virSEVCapability *cap)
+{
+if (!cap)
+return;
+
+VIR_FREE(cap->pdh);
+VIR_FREE(cap->cert_chain);
+VIR_FREE(cap);
+}
+
+
+void
+virQEMUCapsSetSEVCapabilities(virQEMUCapsPtr qemuCaps,
+  virSEVCapability *capabilities)
+{
+virQEMUSevCapabilitiesFree(qemuCaps->sevCapabilities);
+
+qemuCaps->sevCapabilities = capabilities;
+}
+
+
 static int
 virQEMUCapsProbeQMPCommands(virQEMUCapsPtr qemuCaps,
 qemuMonitorPtr mon)
@@ -2580,6 +2606,21 @@ virQEMUCapsProbeQMPGICCapabilities(virQEMUCapsPtr 
qemuCaps,
 }
 
 
+static int
+virQEMUCapsProbeQMPSEVCapabilities(virQEMUCapsPtr qemuCaps,
+   qemuMonitorPtr mon)
+{
+virSEVCapability *caps = NULL;
+
+if (qemuMonitorGetSEVCapabilities(mon, ) < 0)
+return -1;
+
+virQEMUCapsSetSEVCapabilities(qemuCaps, caps);
+
+return 0;
+}
+
+
 bool
 virQEMUCapsCPUFilterFeatures(const char *name,
  void *opaque)
@@ -3965,6 +4006,12 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
 virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE_VFIO_CCW);
 }
 
+/* Probe for SEV capabilities */
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV_GUEST)) {
+if (virQEMUCapsProbeQMPSEVCapabilities(qemuCaps, mon) < 0)
+virQEMUCapsClear(qemuCaps, QEMU_CAPS_SEV_GUEST);
+}
+
 ret = 0;
  cleanup:
 return ret;
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 3e120e64c0b4..8b7eef4359b7 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -473,6 +473,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 QEMU_CAPS_SCREENDUMP_DEVICE, /* screendump command accepts device & head */
 QEMU_CAPS_HDA_OUTPUT, /* -device hda-output */
 QEMU_CAPS_BLOCKDEV_DEL, /* blockdev-del is supported */
+QEMU_CAPS_SEV_GUEST, /* -object sev-guest,... */
 
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
@@ -599,4 +600,7 @@ bool virQEMUCapsGuestIsNative(virArch host,
 bool virQEMUCapsCPUFilterFeatures(const char *name,
   void *opaque);
 
+void
+virQEMUSevCapabilitiesFree(virSEVCapability *capabilities);
+
 #endif /* __QEMU_CAPABILITIES_H__*/
diff --git 

[libvirt] [PATCH v6 0/9] x86: Secure Encrypted Virtualization (AMD)

[Brijesh Singh]

This patch series provides support for launching an encrypted guest using
AMD's new Secure Encrypted  Virtualization (SEV) feature.

SEV is an extension to the AMD-V architecture which supports running
multiple VMs under the control of a hypervisor. When enabled, SEV feature
allows the memory contents of a virtual machine (VM) to be transparently
encrypted with a key unique to the guest VM.

At very high level the flow looks this:

1. mgmt tool calls virConnectGetDomainCapabilities. This returns an XML document
that includes the following


...
  
 
 
 
 


If  is provided then we indicate that hypervisor is capable of launching
SEV guest. 

2. (optional) mgmt tool can provide the PDH and Cert-chain to guest owner in 
case
if guest owner wish to establish a secure connection with SEV firmware to
negotiate a key used for validating the measurement.

3. mgmt tool requests to start a guest calling virCreateXML(), passing 
VIR_DOMAIN_START_PAUSED.
The xml would include


 /* the value is same as what is obtained via 
virConnectGetDomainCapabilities()
 /* the value is same as what is 
obtained via virConnectGetDomainCapabilities()
   ..  /* guest owners diffie-hellman key */ (optional)
   .. /* guest owners session blob */ (optional)
   .. /* guest policy */ (optional)


4. Libvirt generate the QEMU cli arg to enable the SEV feature, a typical
args looks like this:

# $QEMU ..
-machine memory-encryption=sev0 \
-object sev-guest,id=sev0,dh-cert-file=

5. Libvirt generates lifecycle VIR_DOMAIN_EVENT_SUSPENDED_PAUSED event

6. mgmt tool gets the VIR_DOMAIN_EVENT_SUSPENDED_PAUSED and calls 
virDomainGetLaunchSecretInfo()
to retrieve the measurement of encrypted memory.

7. (optional) mgmt tool can provide the measurement value to guest owner, which 
can
validate the measurement and gives GO/NO-GO answer. If mgmt tool gets GO then
it resumes the guest otherwise it calls destroy() to kill the guest.

8. mgmt tool resumes the guest

TODO:
* SEV guest require to use DMA apis for the virtio devices. In order to use the 
DMA
apis the virtio devices must have this tag



It is a bit unclear to me where these changes need to go. Do we need to
modify the libvirt to automatically add these when SEV is enabled or
we ask mgmt tool to make sure that it creates XML with right tag to enable
the DMA APIs for virtio devices. I am looking for some suggestions.

Using these patches we have succesfully booted and tested a guest both with and
without SEV enabled.

SEV Firmware API spec is available at:
https://support.amd.com/TechDocs/55766_SEV-KM%20API_Specification.pdf

Change since v5:
* drop the seperate test patch and merge the code with other patches.
* rename the xml from sev -> launch-security-sev
* make policy field mandatory
* address multiple feedback from previous reviews.

Changes since v4:
* add /dev/sev in shared device list

Changes since v3:
* rename QEMU_CAPS_SEV -> QEMU_CAPS_SEV_GUEST
* update caps_2.12.0.x86_64.replies to include query-sev-capabilities data

Changes since v2:
* make cbitpos, policy and reduced-phys-bits as unsigned int
* update virDomainGetLaunchSecurityInfo to accept virTypedParameterPtr *params
instead of virTypedParameterPtr params.

Changes since v1:
* rename  ->  for domain
* add more information about policy and other fields in domaincaps.html
* split the domain_conf support in two patches
* add virDomainGetLaunchInfo() to retrieve the SEV measurement
* extend virsh command to show the domain's launch security information
* add test cases to validate newly added  element
* fix issues reported with 'make check' and 'make syntax-check'

The complete git tree is available at:
https://github.com/codomania/libvirt/tree/v6

Brijesh Singh (9):
  qemu: provide support to query the SEV capability
  qemu: introduce SEV feature in hypervisor capabilities
  conf: introduce launch-security element in domain
  qemu/cgroup: add /dev/sev in shared devices list
  qemu: add support to launch SEV guest
  libvirt: add new public API to get launch security info
  remote: implement the remote protocol for launch security
  qemu: Add support to launch security info
  virsh: implement new command for launch security

 docs/drvqemu.html.in   |   1 +
 docs/formatdomain.html.in  | 115 ++
 docs/formatdomaincaps.html.in  |  40 +++
 docs/schemas/domaincaps.rng|  20 
 docs/schemas/domaincommon.rng  |  39 ++
 include/libvirt/libvirt-domain.h   |  17 +++
 src/conf/domain_capabilities.c |  20 
 src/conf/domain_capabilities.h |  14 +++
 src/conf/domain_conf.c | 133 +
 src/conf/domain_conf.h |  27 +
 src/driver-hypervisor.h|   7 ++
 src/libvirt-domain.c   |  48 

[libvirt] [PATCH v6 6/9] libvirt: add new public API to get launch security info

[Brijesh Singh]

The API can be used outside the libvirt to get the launch security
information. When SEV is enabled, the API can be used to get the
measurement of the launch process.

Signed-off-by: Brijesh Singh 
---
 include/libvirt/libvirt-domain.h | 17 ++
 src/driver-hypervisor.h  |  7 ++
 src/libvirt-domain.c | 48 
 src/libvirt_public.syms  |  5 +
 4 files changed, 77 insertions(+)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index d7cbd187969d..f252d18da72f 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -4764,4 +4764,21 @@ int virDomainSetLifecycleAction(virDomainPtr domain,
 unsigned int action,
 unsigned int flags);
 
+/**
+ * Launch Security API
+ */
+
+/**
+ * VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT:
+ *
+ * Macro represents the launch measurement of the SEV guest,
+ * as VIR_TYPED_PARAM_STRING.
+ */
+#define VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT "sev-measurement"
+
+int virDomainGetLaunchSecurityInfo(virDomainPtr domain,
+   virTypedParameterPtr *params,
+   int *nparams,
+   unsigned int flags);
+
 #endif /* __VIR_LIBVIRT_DOMAIN_H__ */
diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
index e71a72a44132..bdf13536d337 100644
--- a/src/driver-hypervisor.h
+++ b/src/driver-hypervisor.h
@@ -1286,6 +1286,12 @@ typedef int
   unsigned int action,
   unsigned int flags);
 
+typedef int
+(*virDrvDomainGetLaunchSecurityInfo)(virDomainPtr domain,
+ virTypedParameterPtr *params,
+ int *nparams,
+ unsigned int flags);
+
 
 typedef struct _virHypervisorDriver virHypervisorDriver;
 typedef virHypervisorDriver *virHypervisorDriverPtr;
@@ -1532,6 +1538,7 @@ struct _virHypervisorDriver {
 virDrvDomainSetVcpu domainSetVcpu;
 virDrvDomainSetBlockThreshold domainSetBlockThreshold;
 virDrvDomainSetLifecycleAction domainSetLifecycleAction;
+virDrvDomainGetLaunchSecurityInfo domainGetLaunchSecurityInfo;
 };
 
 
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 2d86e48979d3..dd5a8712484d 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -12101,3 +12101,51 @@ int virDomainSetLifecycleAction(virDomainPtr domain,
 virDispatchError(domain->conn);
 return -1;
 }
+
+/**
+ * virDomainGetLaunchSecurityInfo:
+ * @domain: a domain object
+ * @params: where to store security info
+ * @nparams: number of items in @params
+ * @flags: currently used, set to 0.
+ *
+ * Get the launch security info. In case of the SEV guest, this will
+ * return the launch measurement.
+ *
+ * Returns -1 in case of failure, 0 in case of success.
+ */
+int virDomainGetLaunchSecurityInfo(virDomainPtr domain,
+   virTypedParameterPtr *params,
+   int *nparams,
+   unsigned int flags)
+{
+virConnectPtr conn = domain->conn;
+
+VIR_DOMAIN_DEBUG(domain, "params=%p, nparams=%p flags=0x%x",
+ params, nparams, flags);
+
+virResetLastError();
+
+virCheckDomainReturn(domain, -1);
+virCheckNonNullArgGoto(params, error);
+virCheckNonNullArgGoto(nparams, error);
+virCheckReadOnlyGoto(conn->flags, error);
+
+if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
+ VIR_DRV_FEATURE_TYPED_PARAM_STRING))
+flags |= VIR_TYPED_PARAM_STRING_OKAY;
+
+if (conn->driver->domainGetLaunchSecurityInfo) {
+int ret;
+ret = conn->driver->domainGetLaunchSecurityInfo(domain, params,
+nparams, flags);
+if (ret < 0)
+goto error;
+return ret;
+}
+virReportUnsupportedError();
+
+ error:
+virDispatchError(domain->conn);
+return -1;
+}
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index 95df3a0dbc7b..5ccae5da8883 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -785,4 +785,9 @@ LIBVIRT_4.1.0 {
 virStoragePoolLookupByTargetPath;
 } LIBVIRT_3.9.0;
 
+LIBVIRT_4.4.0 {
+global:
+virDomainGetLaunchSecurityInfo;
+} LIBVIRT_4.1.0;
+
 #  define new API here using predicted next version number 
-- 
2.14.3

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

[libvirt] [PATCH v6 5/9] qemu: add support to launch SEV guest

[Brijesh Singh]

QEMU >= 2.12 provides 'sev-guest' object which is used to launch encrypted

VMs on AMD platform using SEV feature. The various inputs required to
launch SEV guest is provided through the  tag. A typical
SEV guest launch command line looks like this:

# $QEMU ...\
  -object sev-guest,id=sev0,cbitpos=47,reduced-phys-bits=5 ...\
  -machine memory-encryption=sev0 \

Signed-off-by: Brijesh Singh 
---
 src/qemu/qemu_command.c | 41 
 src/qemu/qemu_process.c | 62 +
 tests/qemuxml2argvdata/launch-security-sev.args | 29 
 tests/qemuxml2argvdata/launch-security-sev.xml  | 37 +++
 tests/qemuxml2argvtest.c|  4 ++
 5 files changed, 173 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/launch-security-sev.args
 create mode 100644 tests/qemuxml2argvdata/launch-security-sev.xml

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index cb397c75586a..63941e10ad83 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7203,6 +7203,9 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
 virQEMUCapsGet(qemuCaps, QEMU_CAPS_LOADPARM))
 qemuAppendLoadparmMachineParm(, def);
 
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV_GUEST) && def->sev)
+virBufferAddLit(, ",memory-encryption=sev0");
+
 virCommandAddArgBuffer(cmd, );
 
 ret = 0;
@@ -9566,6 +9569,41 @@ qemuBuildTPMCommandLine(virCommandPtr cmd,
 return 0;
 }
 
+static int
+qemuBuildSevCommandLine(virDomainObjPtr vm, virCommandPtr cmd,
+virDomainSevDefPtr sev)
+{
+virBuffer obj = VIR_BUFFER_INITIALIZER;
+qemuDomainObjPrivatePtr priv = vm->privateData;
+char *path = NULL;
+
+if (!sev)
+return 0;
+
+VIR_DEBUG("policy=0x%x cbitpos=%d reduced_phys_bits=%d",
+  sev->policy, sev->cbitpos, sev->reduced_phys_bits);
+
+virBufferAsprintf(, "sev-guest,id=sev0,cbitpos=%d", sev->cbitpos);
+virBufferAsprintf(, ",reduced-phys-bits=%d", sev->reduced_phys_bits);
+virBufferAsprintf(, ",policy=0x%x", sev->policy);
+
+if (sev->dh_cert) {
+if (virAsprintf(, "%s/dh_cert.base64", priv->libDir) < 0)
+return -1;
+virBufferAsprintf(, ",dh-cert-file=%s", path);
+VIR_FREE(path);
+}
+
+if (sev->session) {
+if (virAsprintf(, "%s/session.base64", priv->libDir) < 0)
+return -1;
+virBufferAsprintf(, ",session-file=%s", path);
+VIR_FREE(path);
+}
+
+virCommandAddArgList(cmd, "-object", virBufferContentAndReset(), NULL);
+return 0;
+}
 
 static int
 qemuBuildVMCoreInfoCommandLine(virCommandPtr cmd,
@@ -10097,6 +10135,9 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
 if (qemuBuildVMCoreInfoCommandLine(cmd, def, qemuCaps) < 0)
 goto error;
 
+if (qemuBuildSevCommandLine(vm, cmd, def->sev) < 0)
+goto error;
+
 if (snapshot)
 virCommandAddArgList(cmd, "-loadvm", snapshot->def->name, NULL);
 
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index ac2049b95df5..3cf818aee034 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5919,6 +5919,65 @@ qemuProcessPrepareDomain(virQEMUDriverPtr driver,
 }
 
 
+static int
+qemuBuildSevCreateFile(const char *configDir,
+   const char *name,
+   const char *data)
+{
+char *configFile;
+
+if (!(configFile = virFileBuildPath(configDir, name, ".base64")))
+return -1;
+
+if (virFileRewriteStr(configFile, S_IRUSR | S_IWUSR, data) < 0) {
+virReportSystemError(errno, _("failed to write data to config '%s'"),
+ configFile);
+goto error;
+}
+
+VIR_FREE(configFile);
+return 0;
+
+ error:
+VIR_FREE(configFile);
+return -1;
+}
+
+
+static int
+qemuProcessPrepareSevGuestInput(virDomainObjPtr vm)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+virDomainDefPtr def = vm->def;
+virQEMUCapsPtr qemuCaps = priv->qemuCaps;
+virDomainSevDefPtr sev = def->sev;
+
+if (!sev)
+return 0;
+
+VIR_DEBUG("Prepare SEV guest");
+
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV_GUEST)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+_("Domain %s asked for 'sev' launch but this "
+  "QEMU does not support SEV feature"), vm->def->name);
+return -1;
+}
+
+if (sev->dh_cert) {
+if (qemuBuildSevCreateFile(priv->libDir, "dh_cert", sev->dh_cert) < 0)
+return -1;
+}
+
+if (sev->session) {
+if (qemuBuildSevCreateFile(priv->libDir, "session", sev->session) < 0)
+return -1;
+}
+
+return 0;
+}
+
+
 static int
 qemuProcessPrepareHostStorage(virQEMUDriverPtr driver,
   virDomainObjPtr vm,
@@ -6043,6 +6102,9 @@ qemuProcessPrepareHost(virQEMUDriverPtr driver,
   

[libvirt] Likely build race, "/usr/bin/ld: cannot find -lvirt"

[Ian Jackson]

tl;dr:

I think there is a bug in libvirt's build system which, with
low probability, causes a build failure containing this message:
  /usr/bin/ld: cannot find -lvirt

Complete build logs of two attempts:

  
http://logs.test-lab.xenproject.org/osstest/logs/123046/build-i386-libvirt/6.ts-libvirt-build.log

  
http://logs.test-lab.xenproject.org/osstest/logs/123096/build-i386-libvirt/6.ts-libvirt-build.log

Snippet from 123046 containing the error is enclosed below.


Longer explanation:

I have two new machines for the Xen Project CI, which I am trying to
commission.  As part of commissioning I run a complete test run (a
"flight" in osstest terminology) on just those new hosts.  The i386
libvirt build failed:

  
http://logs.test-lab.xenproject.org/osstest/logs/123046/build-i386-libvirt/6.ts-libvirt-build.log

Everything else that would be expected to work was fine.  The test
programme was identical to flight 122815, except that that ran on
other hosts in the test farm (and, there, it passed).  The error is
the kind of error one sees with missing dependencies in parallel
builds, etc.

I wanted to have some 32-bit libvirt tests actually run, so I reran a
new flight containing the relevant parts.  That failed too in a very
similar way:

  
http://logs.test-lab.xenproject.org/osstest/logs/123096/build-i386-libvirt/6.ts-libvirt-build.log

The two machines are Dell R230s (and therefore hardly unusual).  The
main novelty of these machines is that the firmware is UEFI booting in
UEFI mode.  I doubt that has anything to do with it.  The host,
including compiler, is Debian jessie i386.

As you can see from the log, we were trying to build libvirt
  764a7483f189e6de841163647c14296e693dbb2e
What may be less obvious is that we were trying to build it against
xen.git#0306a1311d02ea52b4a9a9bc339f8bab9354c5e3.
  
http://logs.test-lab.xenproject.org/osstest/logs/123064/build-i386-libvirt/info.html
  http://logs.test-lab.xenproject.org/osstest/logs/123046/build-i386/info.html

Does this seem like a likely explanation ?  Have other people
experienced occasional problems with make -j ?  If someone wants to
suggest a patch that might fix it I can test it.

In the meantime I have set off a number of new attempts, to try to
guess the failure probability, and also one attempt on other hosts to
check that nothing unexpected was broken.

Ian.


/usr/bin/ld: cannot find -lvirt
/usr/bin/ld: cannot find -lvirt
 /bin/mkdir -p 
'/home/osstest/build.123046.build-i386-libvirt/dist/usr/local/lib/libvirt/storage-backend'
 /bin/bash ../libtool   --mode=install /usr/bin/install -c   
libvirt_storage_backend_fs.la libvirt_storage_backend_logical.la 
libvirt_storage_backend_scsi.la libvirt_storage_backend_mpath.la 
'/home/osstest/build.123046.build-i386-libvirt/dist/usr/local/lib/libvirt/storage-backend'
libtool: install: warning: relinking `libvirt_storage_backend_fs.la'
libtool: install: (cd 
/home/osstest/build.123046.build-i386-libvirt/libvirt/src; /bin/bash 
/home/osstest/build.123046.build-i386-libvirt/libvirt/libtool  --silent --tag 
CC --mode=relink gcc -std=gnu99 -I./conf -I/usr/include/libxml2 -fno-common -W 
-Waddress -Waggressive-loop-optimizations -Wall -Wattributes 
-Wbad-function-cast -Wbuiltin-macro-redefined -Wcast-align -Wchar-subscripts 
-Wclobbered -Wcomment -Wcomments -Wcoverage-mismatch -Wcpp -Wdate-time 
-Wdeprecated-declarations -Wdiv-by-zero -Wdouble-promotion -Wempty-body 
-Wendif-labels -Wextra -Wformat-contains-nul -Wformat-extra-args 
-Wformat-security -Wformat-y2k -Wformat-zero-length -Wfree-nonheap-object 
-Wignored-qualifiers -Wimplicit -Wimplicit-function-declaration -Wimplicit-int 
-Winit-self -Winline -Wint-to-pointer-cast -Winvalid-memory-model -Winvalid-pch 
-Wjump-misses-init -Wlogical-op -Wmain -Wmaybe-uninitialized 
-Wmemset-transposed-args -Wmissing-braces -Wmissing-declarations 
-Wmissing-field-initializers -Wmissing-includ
 e-dirs -Wmissing-parameter-type -Wmissing-prototypes -Wmultichar -Wnarrowing 
-Wnested-externs -Wnonnull -Wold-style-declaration -Wold-style-definition 
-Wopenmp-simd -Woverflow -Woverride-init -Wpacked-bitfield-compat -Wparentheses 
-Wpointer-arith -Wpointer-sign -Wpointer-to-int-cast -Wpragmas -Wpsabi 
-Wreturn-local-addr -Wreturn-type -Wsequence-point -Wshadow 
-Wsizeof-pointer-memaccess -Wstrict-aliasing -Wstrict-prototypes 
-Wsuggest-attribute=const -Wsuggest-attribute=format 
-Wsuggest-attribute=noreturn -Wsuggest-attribute=pure -Wswitch -Wsync-nand 
-Wtrampolines -Wtrigraphs -Wtype-limits -Wuninitialized -Wunknown-pragmas 
-Wunused -Wunused-but-set-parameter -Wunused-but-set-variable -Wunused-function 
-Wunused-label -Wunused-local-typedefs -Wunused-parameter -Wunused-result 
-Wunused-value -Wunused-variable -Wvarargs -Wvariadic-macros 
-Wvector-operation-performance -Wvolatile-register-var -Wwrite-strings 
-Wnormalized=nfc -Wno-sign-compare -Wjump-misses-init -Wswitch-enum 
-Wno-format-no
 nliteral -fstack-protector-strong -fexceptions -fasyn
chronous-unwind-tables 

Re: [libvirt] AppArmor support for TPM emulator; was:Re: [PATCH 00/12] Add support for TPM emulator

[Stefan Berger]

On 05/23/2018 02:03 PM, John Ferlan wrote:


On 05/23/2018 09:20 AM, Stefan Berger wrote:

On 05/23/2018 08:07 AM, John Ferlan wrote:

On 05/22/2018 04:44 PM, Stefan Berger wrote:

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

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

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

    Stefan

v5->v6:
    - Addressed John Ferlan's comments
    - rebased on latest tip
    - Added patch 12.

v4->v5:
    - Addressed John Ferlan's, Boris Fiuczysnki's and Marc Hartmayer's
comments
    - rebased on latest tip

v3->v4:
    - Addressed John Ferlan's comments
    - Fixed bugs I found while testing
    - rebased on latest tip

Stefan Berger (12):
    conf: Add support for external swtpm TPM emulator to domain XML
    qemu: Extend QEMU capabilities with 'tpm-emulator'
    util: Implement virFileChownFiles()
    security: Add DAC and SELinux security for tpm-emulator
    qemu: Extend qemu_conf with tpm-emulator support
    qemu: Extend QEMU with external TPM support
    qemu: Add support for external swtpm TPM emulator
    tests: Add test cases for external swtpm TPM emulator
    security: Label the external swtpm with SELinux labels
    conf: Add support for choosing emulation of a TPM 2
    qemu: Add swtpm to emulator cgroup
    news: Update news with new TPM emulator feature

   docs/formatdomain.html.in  |  43 +
   docs/news.xml  |   9 +
   docs/schemas/domaincommon.rng  |  17 +
   libvirt.spec.in    |   2 +
   src/conf/domain_audit.c    |   2 +
   src/conf/domain_conf.c |  53 +-
   src/conf/domain_conf.h |  12 +
   src/libvirt_private.syms   |   3 +
   src/qemu/Makefile.inc.am   |  10 +
   src/qemu/libvirtd_qemu.aug |   5 +
   src/qemu/qemu.conf |   8 +
   src/qemu/qemu_capabilities.c   |   5 +
   src/qemu/qemu_capabilities.h   |   1 +
   src/qemu/qemu_cgroup.c |  36 +
   src/qemu/qemu_cgroup.h |   2 +
   src/qemu/qemu_command.c    |  34 +-
   src/qemu/qemu_conf.c   |  43 +
   src/qemu/qemu_conf.h   |   6 +
   src/qemu/qemu_domain.c |   3 +
   src/qemu/qemu_extdevice.c  | 180 
   src/qemu/qemu_extdevice.h  |  59 ++
   src/qemu/qemu_process.c    |  16 +
   src/qemu/qemu_security.c   |  69 ++
   src/qemu/qemu_security.h   |  11 +
   src/qemu/qemu_tpm.c    | 946
+
   src/qemu/qemu_tpm.h    |  56 ++
   src/qemu/test_libvirtd_qemu.aug.in |   2 +
   src/security/security_dac.c    |   7 +
   src/security/security_driver.h |   7 +
   src/security/security_manager.c    |  36 +
   src/security/security_manager.h    |   6 +
   src/security/security_selinux.c    | 172 
   src/security/security_stack.c  |  40 +
   src/util/virfile.c |  55 ++
   src/util/virfile.h |   3 +
   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 +
   .../tpm-emulator-tpm2.x86_64-latest.args   |  33 +
   tests/qemuxml2argvdata/tpm-emulator-tpm2.xml   |  30 +
   .../tpm-emulator.x86_64-latest.args    |  33 +
   tests/qemuxml2argvdata/tpm-emulator.xml    |  30 +
   tests/qemuxml2argvtest.c   |  16 +-
   tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml |  34 +
   tests/qemuxml2xmloutdata/tpm-emulator.xml  |  34 +
   tests/qemuxml2xmltest.c    |   1 +
   48 files 

Re: [libvirt] [PATCH 04/22] virsh: Enhance documentation of cpu-models command

[Jiri Denemark]

On Wed, May 23, 2018 at 16:53:37 +0200, Kashyap Chamarthy wrote:
> On Wed, May 23, 2018 at 02:43:23PM +0200, Jiri Denemark wrote:
> > On Wed, May 23, 2018 at 13:08:51 +0200, Kashyap Chamarthy wrote:
> > > On Wed, May 16, 2018 at 10:39:23AM +0200, Jiri Denemark wrote:
> > > > Signed-off-by: Jiri Denemark 
> > > > ---
> > > >  tools/virsh.pod | 8 +++-
> > > >  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> [...]
> 
> > > > +Moreover, for some architectures libvirt does not know any CPU models 
> > > > and
> > > > +the usable CPU models are only limited by the hypervisor. 
> > > 
> > > Wonder if it is worth adding a small example for the above.
> > 
> > An example for what exactly?
> 
> I meant an example of a usable CPU model that libvirt doesn't know for
> the said architecture(s).  Maybe it's not worth it; not quite sure.

The usable models are all models accepted by hypervisor for that
architecture. So, e.g., libvirt won't list any CPU model for aarch64
when you call "virsh cpu-models aarch64", it will just print "all CPU
models are accepted" and "virsh domcapabilities --arch aarch64" will
show a very long list of CPU models libvirt would actually accept.
Similarly for s390x.

Jirka

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

Re: [libvirt] [PATCH 10/12] conf: Add support for choosing emulation of a TPM 2

[Stefan Berger]

On 05/23/2018 11:55 AM, Ján Tomko wrote:

On Tue, May 22, 2018 at 04:44:51PM -0400, 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:

   
 
   

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 
Reviewed-by: John Ferlan 
---
docs/formatdomain.html.in  | 15 -
docs/schemas/domaincommon.rng  | 12 
src/conf/domain_conf.c | 27 -
src/conf/domain_conf.h |  6 ++
src/qemu/qemu_tpm.c    | 64 
+-

.../tpm-emulator-tpm2.x86_64-latest.args   | 33 +++
tests/qemuxml2argvdata/tpm-emulator-tpm2.xml   | 30 ++
tests/qemuxml2argvtest.c   |  1 +
tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml | 34 
9 files changed, 217 insertions(+), 5 deletions(-)
create mode 100644 
tests/qemuxml2argvdata/tpm-emulator-tpm2.x86_64-latest.args

create mode 100644 tests/qemuxml2argvdata/tpm-emulator-tpm2.xml
create mode 100644 tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml




@@ -24941,6 +24963,9 @@ virDomainTPMDefFormat(virBufferPtr buf,
    virBufferAsprintf(buf, "type));

+    if (def->version == VIR_DOMAIN_TPM_VERSION_2)
+    virBufferAddLit(buf, " version='2'");
+


Any reason for not formatting version 1.2?
We should format implicit defaults in the XML if possible.


Basically I did it because the previous default 1.2 didn't have it. So I 
though I'd keep it as is for 1.2 and only write out 2.





    switch (def->type) {
    case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
    virBufferAddLit(buf, ">\n");
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 92466278ab..e2409899bc 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1291,12 +1291,18 @@ typedef enum {
    VIR_DOMAIN_TPM_TYPE_LAST
} virDomainTPMBackendType;

+typedef enum {
+    VIR_DOMAIN_TPM_VERSION_1_2,
+    VIR_DOMAIN_TPM_VERSION_2,
+} virDomainTPMVersion;


With a corresponding VIR_ENUM_IMPL and VIR_ENUM_DECL,
you can use the *{To,From}String functions for parsing/formatting
the version.


+
# define VIR_DOMAIN_TPM_DEFAULT_DEVICE "/dev/tpm0"

struct _virDomainTPMDef {
    virDomainTPMBackendType type;
    virDomainDeviceInfo info;
    virDomainTPMModel model;
+    virDomainTPMVersion version;
    union {
    struct {
    virDomainChrSourceDef source;
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index 11b91aa915..508685c455 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -54,6 +54,41 @@ static char *swtpm_path;
static char *swtpm_setup;
static char *swtpm_ioctl;

+bool swtpm_supports_tpm2;
+
+/*
+ * qemuTPMCheckForTPM2Support
+ *
+ * Check whether swtpm_setup supports TPM 2
+ */
+static void
+qemuTPMCheckForTPM2Support(void)
+{
+    virCommandPtr cmd;
+    char *help = NULL;
+
+    if (!swtpm_setup)
+    return;
+
+    cmd = virCommandNew(swtpm_setup);
+    if (!cmd)
+    return;
+
+    virCommandAddArg(cmd, "--help");
+    virCommandSetOutputBuffer(cmd, );
+
+    if (virCommandRun(cmd, NULL) < 0)
+    goto cleanup;
+
+    if (strstr(help, "--tpm2"))
+    swtpm_supports_tpm2 = true;


This bool is never read.


Maybe while doing some of the recent changes the reading of the variable 
got lost.




Given that version 2 has to be requested in the XML and we don't try to
use it automatically, I'd suggest just dropping this function. We don't
need to parse another tool's --help output to make up for the removal
of parsing --help of qemu and qemu-img.


swtpm doesn't have all the bells and whistles of QEMU that we would have 
a JSON interface to query the features from. So if a bad command line 
parameter is passed to swtpm, it will dump the help screen. Here's the 
output I get from trying to run a VM with an attached TPM 2 but there's 
no TPM 2 support compiled into swtpm (basically because it was created 
from the master branch not from the preview branch):


# virsh start testvm-tpm2
Error: Failed to start domain testvm-tpm2
error: internal error: Could not start 'swtpm'. exitstatus: 1, error: 
socket: unrecognized option '--tpm2'

Usage: /usr/bin/swtpm socket [options]

The following options are supported:

-p|--port  : use the given port
-f|--fd  : use the given socket file descriptor
[...]


I 

Re: [libvirt] AppArmor support for TPM emulator; was:Re: [PATCH 00/12] Add support for TPM emulator

[John Ferlan]

On 05/23/2018 09:20 AM, Stefan Berger wrote:
> On 05/23/2018 08:07 AM, John Ferlan wrote:
>>
>> On 05/22/2018 04:44 PM, Stefan Berger wrote:
>>> This series of patches adds support for the TPM emulator backend that
>>> is available in QEMU and based on swtpm + libtpms. It allows to attach a
>>> TPM 1.2 or 2 to a QEMU VM. sVirt labels are used for labeling the swtpm
>>> process, its Unix socket, and log file with the same label that the
>>> QEMU process gets. Besides that swtpm is added to the emulator cgroup to
>>> restrict its CPU usage.
>>>
>>> The device XML can be changed from a TPM 1.2 to a TPM 2 and back to a
>>> TPM 1.2. The device state is not removed during those changes but only
>>> when the domain is undefined.
>>>
>>> The swtpm needs persistent storage to store its state. For that I am
>>> using the uuid of the VM as part of the path since the name of the VM
>>> can be changed. Logfiles, PID files, and socket names are based on the
>>> name of the VM, though.
>>>
>>>    Stefan
>>>
>>> v5->v6:
>>>    - Addressed John Ferlan's comments
>>>    - rebased on latest tip
>>>    - Added patch 12.
>>>
>>> v4->v5:
>>>    - Addressed John Ferlan's, Boris Fiuczysnki's and Marc Hartmayer's
>>> comments
>>>    - rebased on latest tip
>>>
>>> v3->v4:
>>>    - Addressed John Ferlan's comments
>>>    - Fixed bugs I found while testing
>>>    - rebased on latest tip
>>>
>>> Stefan Berger (12):
>>>    conf: Add support for external swtpm TPM emulator to domain XML
>>>    qemu: Extend QEMU capabilities with 'tpm-emulator'
>>>    util: Implement virFileChownFiles()
>>>    security: Add DAC and SELinux security for tpm-emulator
>>>    qemu: Extend qemu_conf with tpm-emulator support
>>>    qemu: Extend QEMU with external TPM support
>>>    qemu: Add support for external swtpm TPM emulator
>>>    tests: Add test cases for external swtpm TPM emulator
>>>    security: Label the external swtpm with SELinux labels
>>>    conf: Add support for choosing emulation of a TPM 2
>>>    qemu: Add swtpm to emulator cgroup
>>>    news: Update news with new TPM emulator feature
>>>
>>>   docs/formatdomain.html.in  |  43 +
>>>   docs/news.xml  |   9 +
>>>   docs/schemas/domaincommon.rng  |  17 +
>>>   libvirt.spec.in    |   2 +
>>>   src/conf/domain_audit.c    |   2 +
>>>   src/conf/domain_conf.c |  53 +-
>>>   src/conf/domain_conf.h |  12 +
>>>   src/libvirt_private.syms   |   3 +
>>>   src/qemu/Makefile.inc.am   |  10 +
>>>   src/qemu/libvirtd_qemu.aug |   5 +
>>>   src/qemu/qemu.conf |   8 +
>>>   src/qemu/qemu_capabilities.c   |   5 +
>>>   src/qemu/qemu_capabilities.h   |   1 +
>>>   src/qemu/qemu_cgroup.c |  36 +
>>>   src/qemu/qemu_cgroup.h |   2 +
>>>   src/qemu/qemu_command.c    |  34 +-
>>>   src/qemu/qemu_conf.c   |  43 +
>>>   src/qemu/qemu_conf.h   |   6 +
>>>   src/qemu/qemu_domain.c |   3 +
>>>   src/qemu/qemu_extdevice.c  | 180 
>>>   src/qemu/qemu_extdevice.h  |  59 ++
>>>   src/qemu/qemu_process.c    |  16 +
>>>   src/qemu/qemu_security.c   |  69 ++
>>>   src/qemu/qemu_security.h   |  11 +
>>>   src/qemu/qemu_tpm.c    | 946
>>> +
>>>   src/qemu/qemu_tpm.h    |  56 ++
>>>   src/qemu/test_libvirtd_qemu.aug.in |   2 +
>>>   src/security/security_dac.c    |   7 +
>>>   src/security/security_driver.h |   7 +
>>>   src/security/security_manager.c    |  36 +
>>>   src/security/security_manager.h    |   6 +
>>>   src/security/security_selinux.c    | 172 
>>>   src/security/security_stack.c  |  40 +
>>>   src/util/virfile.c |  55 ++
>>>   src/util/virfile.h |   3 +
>>>   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 +
>>>   .../tpm-emulator-tpm2.x86_64-latest.args   |  33 +
>>>   tests/qemuxml2argvdata/tpm-emulator-tpm2.xml   |  30 +
>>>   .../tpm-emulator.x86_64-latest.args    |  33 +
>>>   tests/qemuxml2argvdata/tpm-emulator.xml    |  30 +
>>>   

Re: [libvirt] [PATCH 06/12] qemu: Extend QEMU with external TPM support

[Stefan Berger]

On 05/23/2018 11:41 AM, Ján Tomko wrote:

On Tue, May 22, 2018 at 04:44:47PM -0400, Stefan Berger wrote:
Implement 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.

Further, the external TPM needs storage on the host that we need to set
up before it can be run. We can clean up the host once the domain is
undefined.

This patch also implements a small layer for external device support 
that

calls into the TPM device layer if a domain has an attached TPM. This is
the layer we will wire up later on.



Later on meaning in other patch series? Adding it for just one device
seems excessive, but that might be my personal opinion.


Signed-off-by: Stefan Berger 
Reviewed-by: John Ferlan 
---
src/qemu/Makefile.inc.am  |   4 +
src/qemu/qemu_domain.c    |   2 +
src/qemu/qemu_extdevice.c | 154 ++
src/qemu/qemu_extdevice.h |  53 
src/qemu/qemu_process.c   |  12 +
src/qemu/qemu_tpm.c   | 751 
++

src/qemu/qemu_tpm.h   |  50 +++
7 files changed, 1026 insertions(+)
create mode 100644 src/qemu/qemu_extdevice.c
create mode 100644 src/qemu/qemu_extdevice.h
create mode 100644 src/qemu/qemu_tpm.c
create mode 100644 src/qemu/qemu_tpm.h




+/*
+ * virtTPMGetTPMStorageDir:
+ *
+ * @storagepath: directory for swtpm's persistent state
+ *
+ * Derive the 'TPMStorageDir' from the storagepath by searching
+ * for the last '/'.
+ */
+static char *
+qemuTPMGetTPMStorageDir(const char *storagepath)
+{
+    const char *tail = strrchr(storagepath, '/');
+    char *path = NULL;
+
+    if (!tail) {
+    virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Could not get tail of storagedir %s"),
+   storagepath);
+    return NULL;
+    }
+    ignore_value(VIR_STRNDUP(path, storagepath, tail - storagepath));
+
+    return path;
+}


In other places we already use mdir_name from gnulib for this
functionality.



I didn't know about this API. I will change it to that.





+/*
+ * qemuTPMCreateEmulatorStorage
+ *
+ * @storagepath: directory for swtpm's persistent state
+ * @created: a pointer to a bool that will be set to true if the
+ *   storage was created because it did not exist yet


Can't the caller call virFileExists and act accordingly?



We could, except that we have to call this function since it is 
adjusting access rights to files for the swtpm_user and swtpm_group.






+ * @swtpm_user: The uid that needs to be able to access the directory
+ * @swtpm_group: The gid that needs to be able to access the directory
+ *
+ * Unless the storage path for the swtpm for the given VM
+ * already exists, create it and make it accessible for the given 
userid.

+ * Adapt ownership of the directory and all swtpm's state files there.
+ */


[...]


+static int
+qemuTPMEmulatorPrepareHost(virDomainTPMDefPtr tpm,
+   const char *logDir,
+   const char *vmname,
+   uid_t swtpm_user,
+   gid_t swtpm_group,
+   const char *swtpmStateDir,
+   uid_t qemu_user,
+   const char *shortName)
+{
+    int ret = -1;
+
+    if (qemuTPMEmulatorInit() < 0)
+    return -1;
+
+    /* create log dir ... allow 'tss' user to cd into it */
+    if (virFileMakePathWithMode(logDir, 0711) < 0)
+    return -1;
+
+    /* ... and adjust ownership */
+    if (virDirCreate(logDir, 0730, swtpm_user, swtpm_group,
+ VIR_DIR_CREATE_ALLOW_EXIST) < 0)
+    goto cleanup;
+
+    /* create logfile name ... */
+    if (!tpm->data.emulator.logfile &&
+    virAsprintf(>data.emulator.logfile, "%s/%s-swtpm.log",
+    logDir, vmname) < 0)


This should also use shortName.



The shortName has the ID of the domain in the name. So for short-lived 
logs I would say yes. Though this should be a log like the one for the 
VM that gets appended to every time the VM restarts. I'd rather not 
change this.






+    goto cleanup;
+
+    /* ... and make sure it can be accessed by swtpm_user */
+    if (virFileExists(tpm->data.emulator.logfile) &&
+    chown(tpm->data.emulator.logfile, swtpm_user, swtpm_group) < 
0) {

+    virReportSystemError(errno,
+ _("Could not chown on swtpm logfile %s"),
+ tpm->data.emulator.logfile);
+    goto cleanup;
+    }
+
+    /*
+  create our swtpm state dir ...
+  - QEMU user needs to be able to access the socket there
+  - swtpm group needs to be able to create files there
+  - in privileged mode 0570 would be enough, for non-privileged 
mode

+    we need 0770
+    */
+    if (virDirCreate(swtpmStateDir, 0770, 

Re: [libvirt] [RFC PATCH 6/6] qemu: Format pseries.cap-hpt-mps on the command line

[Andrea Bolognani]

On Wed, 2018-05-23 at 18:40 +0200, Peter Krempa wrote:
> On Wed, May 23, 2018 at 18:18:02 +0200, Andrea Bolognani wrote:
> > +/* QEMU expects the argument to be a number of left shifts:
> > + * for example, if you wanted to limit the guest to 4 KiB 
> > pages,
> > + * since 4096 == 1 << 12, you would need to add cap-hpt-mps=12
> > + * to the command line.
> 
> So basically you need to pass the exponent of a power of 2 that yields
> this number. The number of left shifts may be slightly confusing ...

I guess it depends on the reader; the two definitions are
equivalent anyway, so no harm in having both in the comment :)

In general, I'd say it's not the most user-friendly interface on
QEMU's side, but I believe it's dictated by hardware / emulator
details, given how it ends up being used: see

  http://lists.nongnu.org/archive/html/qemu-devel/2018-04/msg02822.html

To be fair, it would perhaps make sense to perform the conversion
directly inside QEMU, in order to make it more convenient not only
for libvirt but for for people driving it directly as well.

CC'ing David so that he can weigh in on the idea.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [RFC PATCH 5/6] conf: Parse and format HPT maxpagesize

[Andrea Bolognani]

On Wed, 2018-05-23 at 18:36 +0200, Peter Krempa wrote:
> On Wed, May 23, 2018 at 18:18:01 +0200, Andrea Bolognani wrote:
> > +def->hpt_maxpagesize = VIR_ROUND_UP(def->hpt_maxpagesize, 
> > 1024);
> 
> The code in the patch using it with qemu actually expects so this is a
> power of 2, so you'll need a different rounding function. 

Good point!

Looks like VIR_ROUND_UP_POWER_OF_TWO() will do the trick.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [RFC PATCH 2/6] conf: Tweak HPT parsing and formatting

[Peter Krempa]

On Wed, May 23, 2018 at 18:17:58 +0200, Andrea Bolognani wrote:
> This doesn't seem very useful at the moment, but it will make
> sense once we introduce another HPT-related setting.
> 
> The output XML is decoupled from the input XML in preparation
> of future changes as well; while doing so, we can shave a few
> lines off the latter.
> 
> This commit is best viewed with 'git show -w'.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  src/conf/domain_conf.c| 25 ---
>  src/qemu/qemu_command.c   | 32 ++-
>  tests/qemuxml2argvdata/pseries-features.xml   | 14 ++--
>  tests/qemuxml2xmloutdata/pseries-features.xml | 29 -
>  4 files changed, 68 insertions(+), 32 deletions(-)
>  mode change 12 => 100644 tests/qemuxml2xmloutdata/pseries-features.xml
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 355e497002..20b845f02a 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -27207,14 +27207,31 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>
> virDomainIOAPICTypeToString(def->features[i]));
>  break;
>  
> -case VIR_DOMAIN_FEATURE_HPT:
> +case VIR_DOMAIN_FEATURE_HPT: {
> +bool hasResizing = def->hpt_resizing != 
> VIR_DOMAIN_HPT_RESIZING_NONE;
> +char *resizing = NULL;
> +
>  if (def->features[i] != VIR_TRISTATE_SWITCH_ON ||
> -def->hpt_resizing == VIR_DOMAIN_HPT_RESIZING_NONE)
> +!hasResizing) {
>  break;
> +}
> +
> +if (hasResizing) {
> +if (virAsprintf(, " resizing='%s'",
> +
> virDomainHPTResizingTypeToString(def->hpt_resizing)) < 0) {
> +goto error;
> +}
> +} else {
> +if (VIR_STRDUP(resizing, "") < 0)
> +goto error;
> +}
>  
> -virBufferAsprintf(buf, "\n",
> -  
> virDomainHPTResizingTypeToString(def->hpt_resizing));
> +virBufferAsprintf(buf, "\n",

This formulation looks fishy.

> +  resizing);
> +
> +VIR_FREE(resizing);
>  break;
> +}
>  
>  /* coverity[dead_error_begin] */
>  case VIR_DOMAIN_FEATURE_LAST:


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

Re: [libvirt] [RFC PATCH 2/6] conf: Tweak HPT parsing and formatting

[Andrea Bolognani]

On Wed, 2018-05-23 at 18:42 +0200, Peter Krempa wrote:
> On Wed, May 23, 2018 at 18:17:58 +0200, Andrea Bolognani wrote:
> > +if (hasResizing) {
> > +if (virAsprintf(, " resizing='%s'",
> > +virDomainHPTResizingTypeToStri
> > ng(def->hpt_resizing)) < 0) {
> > +goto error;
> > +}
> > +} else {
> > +if (VIR_STRDUP(resizing, "") < 0)
> > +goto error;
> > +}
> >  
> > -virBufferAsprintf(buf, "\n",
> > -  virDomainHPTResizingTypeToString
> > (def->hpt_resizing));
> > +virBufferAsprintf(buf, "\n",
> 
> This formulation looks fishy.

I don't love it either, but I've tried a bunch of alternative
approaches and this seemed like the most sane to me.

If you have suggestions on how to improve it, considering that the
end result is what you see after patch 5/6, please do share! :)

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [RFC PATCH 6/6] qemu: Format pseries.cap-hpt-mps on the command line

[Peter Krempa]

On Wed, May 23, 2018 at 18:18:02 +0200, Andrea Bolognani wrote:
> This makes the feature fully functional.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1571078
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  src/qemu/qemu_command.c  | 26 
>  tests/qemuxml2argvdata/pseries-features.args |  3 ++-
>  2 files changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index b446a08613..983839e81c 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -7193,6 +7193,32 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
>  
>  virBufferAsprintf(, ",resize-hpt=%s", str);
>  }
> +
> +if (def->hpt_maxpagesize > 0) {
> +unsigned long long tmp = def->hpt_maxpagesize;
> +unsigned int shifts = 0;
> +
> +if (!virQEMUCapsGet(qemuCaps, 
> QEMU_CAPS_MACHINE_PSERIES_CAP_HPT_MPS)) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +   _("Limiting the page size for HPT guest is "
> + "not supported by this QEMU binary"));
> +goto cleanup;
> +}
> +
> +/* QEMU expects the argument to be a number of left shifts:
> + * for example, if you wanted to limit the guest to 4 KiB pages,
> + * since 4096 == 1 << 12, you would need to add cap-hpt-mps=12
> + * to the command line.

So basically you need to pass the exponent of a power of 2 that yields
this number. The number of left shifts may be slightly confusing ...

> + *
> + * Convert from our internal representation, which is bytes,
> + * to the one QEMU expects */
> +while (tmp > 1) {
> +tmp = tmp >> 1;
> +shifts++;
> +}
> +
> +virBufferAsprintf(, ",cap-hpt-mps=%u", shifts);
> +}
>  }
>  
>  if (cpu && cpu->model &&
> diff --git a/tests/qemuxml2argvdata/pseries-features.args 
> b/tests/qemuxml2argvdata/pseries-features.args
> index f5c1959cca..4e581a69a1 100644
> --- a/tests/qemuxml2argvdata/pseries-features.args
> +++ b/tests/qemuxml2argvdata/pseries-features.args
> @@ -7,7 +7,8 @@ QEMU_AUDIO_DRV=none \
>  /usr/bin/qemu-system-ppc64 \
>  -name guest \
>  -S \
> --machine pseries,accel=tcg,usb=off,dump-guest-core=off,resize-hpt=required \
> +-machine pseries,accel=tcg,usb=off,dump-guest-core=off,resize-hpt=required,\
> +cap-hpt-mps=30 \
>  -m 512 \
>  -smp 1,sockets=1,cores=1,threads=1 \
>  -uuid 1ccfd97d-5eb4-478a-bbe6-88d254c16db7 \
> -- 
> 2.17.0
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list


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

Re: [libvirt] [PATCH] news: Add TLS non-shared storage migration

[Andrea Bolognani]

On Wed, 2018-05-23 at 16:59 +0200, Peter Krempa wrote:
> Signed-off-by: Peter Krempa 
> ---
>  docs/news.xml | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/docs/news.xml b/docs/news.xml
> index 8f2c7d5dff..329b1c7129 100644
> --- a/docs/news.xml
> +++ b/docs/news.xml
> @@ -54,6 +54,16 @@
>a QEMU virtual machine.
>  
>
> +  
> +
> +  Add support for migration of QEMU VMs with non-shared
storage over TLS

This would be more consistent with the rest of the entries, and
easier to quickly scan over, written as

  qemu: Add support for migration of VMs with non-shared storage
  over TLS

With that changed,

  Reviewed-by: Andrea Bolognani 

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [RFC PATCH 5/6] conf: Parse and format HPT maxpagesize

[Peter Krempa]

On Wed, May 23, 2018 at 18:18:01 +0200, Andrea Bolognani wrote:
> This commit is best viewed with 'git show -w'.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  docs/schemas/domaincommon.rng | 21 ++---
>  src/conf/domain_conf.c| 44 ---
>  src/conf/domain_conf.h|  1 +
>  tests/qemuxml2argvdata/pseries-features.xml   |  4 +-
>  tests/qemuxml2argvtest.c  |  1 +
>  tests/qemuxml2xmloutdata/pseries-features.xml |  4 +-
>  tests/qemuxml2xmltest.c   |  1 +
>  7 files changed, 60 insertions(+), 16 deletions(-)
> 

[...]

> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 20b845f02a..e84cfb0d05 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -19493,8 +19493,24 @@ virDomainDefParseXML(xmlDocPtr xml,
>  VIR_FREE(tmp);
>  }
>  
> -if (def->hpt_resizing != VIR_DOMAIN_HPT_RESIZING_NONE)
> +if (virDomainParseScaledValue("./features/hpt/maxpagesize",
> +  NULL,
> +  ctxt,
> +  >hpt_maxpagesize,
> +  1024,
> +  ULLONG_MAX,
> +  false) < 0) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +   "%s",
> +   _("Unable to parse HPT maxpagesize setting"));
> +goto error;
> +}
> +def->hpt_maxpagesize = VIR_ROUND_UP(def->hpt_maxpagesize, 1024);

The code in the patch using it with qemu actually expects so this is a
power of 2, so you'll need a different rounding function. 


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

Re: [libvirt] [GSoC] Design ideas for implementing cleanup attribute

[Peter Krempa]

On Wed, May 23, 2018 at 18:05:17 +0200, Pavel Hrdina wrote:

[...]

> I liked the way how GLib is solving the issue so we can simply use the
> same approach since it looks reasonable.
> 
> There would be three different macros that would be used to annotate
> variable with attribute cleanup:
> 
> VIR_AUTOFREE char *str = NULL;

For consistency I'd prefer if the argument is in parentheses similarly
to the ones below.

> 
> - this would call virFree on that variable
> 
> VIR_AUTOPTR(virDomain) domain = NULL;
> 
> - this would call registered free function on that variable
> - to register the free function you would use:
> 
> VIR_DEFINE_AUTOPTR_FUNC(virDomain, virDomainFree);

Did you mean virDomainPtr?

How is the type matched? Does it have to be a typedef'd type to use
this?



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

Re: [libvirt] [PATCH 08/12] tests: Add test cases for external swtpm TPM emulator

[Stefan Berger]

On 05/23/2018 11:43 AM, Ján Tomko wrote:

On Tue, May 22, 2018 at 04:44:49PM -0400, Stefan Berger wrote:
This patch adds extensions to existing test cases and specific test 
cases

for the tpm-emulator.

Signed-off-by: Stefan Berger 
Reviewed-by: John Ferlan 
---
.../tpm-emulator.x86_64-latest.args    | 33 
++

tests/qemuxml2argvtest.c   | 15 +-
2 files changed, 47 insertions(+), 1 deletion(-)
create mode 100644 
tests/qemuxml2argvdata/tpm-emulator.x86_64-latest.args





diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 78454acb1a..587f15242e 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;
+   }


This is indented by three spaces instead of four.


Fixed.


Jano



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

[libvirt] [RFC PATCH 6/6] qemu: Format pseries.cap-hpt-mps on the command line

[Andrea Bolognani]

This makes the feature fully functional.

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

Signed-off-by: Andrea Bolognani 
---
 src/qemu/qemu_command.c  | 26 
 tests/qemuxml2argvdata/pseries-features.args |  3 ++-
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index b446a08613..983839e81c 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7193,6 +7193,32 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
 
 virBufferAsprintf(, ",resize-hpt=%s", str);
 }
+
+if (def->hpt_maxpagesize > 0) {
+unsigned long long tmp = def->hpt_maxpagesize;
+unsigned int shifts = 0;
+
+if (!virQEMUCapsGet(qemuCaps, 
QEMU_CAPS_MACHINE_PSERIES_CAP_HPT_MPS)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("Limiting the page size for HPT guest is "
+ "not supported by this QEMU binary"));
+goto cleanup;
+}
+
+/* QEMU expects the argument to be a number of left shifts:
+ * for example, if you wanted to limit the guest to 4 KiB pages,
+ * since 4096 == 1 << 12, you would need to add cap-hpt-mps=12
+ * to the command line.
+ *
+ * Convert from our internal representation, which is bytes,
+ * to the one QEMU expects */
+while (tmp > 1) {
+tmp = tmp >> 1;
+shifts++;
+}
+
+virBufferAsprintf(, ",cap-hpt-mps=%u", shifts);
+}
 }
 
 if (cpu && cpu->model &&
diff --git a/tests/qemuxml2argvdata/pseries-features.args 
b/tests/qemuxml2argvdata/pseries-features.args
index f5c1959cca..4e581a69a1 100644
--- a/tests/qemuxml2argvdata/pseries-features.args
+++ b/tests/qemuxml2argvdata/pseries-features.args
@@ -7,7 +7,8 @@ QEMU_AUDIO_DRV=none \
 /usr/bin/qemu-system-ppc64 \
 -name guest \
 -S \
--machine pseries,accel=tcg,usb=off,dump-guest-core=off,resize-hpt=required \
+-machine pseries,accel=tcg,usb=off,dump-guest-core=off,resize-hpt=required,\
+cap-hpt-mps=30 \
 -m 512 \
 -smp 1,sockets=1,cores=1,threads=1 \
 -uuid 1ccfd97d-5eb4-478a-bbe6-88d254c16db7 \
-- 
2.17.0

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

[libvirt] [RFC PATCH 4/6] tests: Pretend we have pseries.cap-hpt-mps in 2.12

[Andrea Bolognani]

That's not the case, of course, but the relevant QEMU code
has not been merged upstream yet and this is a cheap way to
show the capability is actually detected correctly.

Do not merge.

Signed-off-by: Andrea Bolognani 
---
 tests/qemucapabilitiesdata/caps_2.12.0.ppc64.replies | 5 +
 tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml | 3 ++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.replies 
b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.replies
index e71d69519d..55f70fb0a0 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.replies
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.replies
@@ -5532,6 +5532,11 @@
   "name": "cap-vsx",
   "description": "Allow Vector Scalar Extensions (VSX)",
   "type": "bool"
+},
+{
+  "name": "cap-hpt-mps",
+  "description": "Maximum page shift for Hash Page Table guests (12, 16, 
24, 34)",
+  "type": "int"
 }
   ],
   "id": "libvirt-38"
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml 
b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
index eb89c6cd2d..155ed2c246 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
@@ -163,9 +163,10 @@
   
   
   
+  
   2011090
   0
-  423940
+  424089
   v2.12.0-rc0
   ppc64
   
-- 
2.17.0

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

[libvirt] [RFC PATCH 3/6] qemu: Introduce QEMU_CAPS_MACHINE_PSERIES_CAP_HPT_MPS

[Andrea Bolognani]

Signed-off-by: Andrea Bolognani 
---
 src/qemu/qemu_capabilities.c  |   8 +
 src/qemu/qemu_capabilities.h  |   1 +
 .../caps_2.12.0.aarch64.replies   |  24 ++-
 .../caps_2.12.0.aarch64.xml   |   2 +-
 .../caps_2.12.0.ppc64.replies | 175 +-
 .../caps_2.12.0.ppc64.xml |   2 +-
 .../caps_2.12.0.s390x.replies |  26 ++-
 .../caps_2.12.0.s390x.xml |   2 +-
 .../caps_2.12.0.x86_64.replies|  30 +--
 .../caps_2.12.0.x86_64.xml|   2 +-
 10 files changed, 233 insertions(+), 39 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 8a63db5f4f..ab034b7693 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -489,6 +489,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   "screendump_device",
   "hda-output",
   "blockdev-del",
+  "machine.pseries.cap-hpt-mps",
 );
 
 
@@ -1401,10 +1402,17 @@ static struct virQEMUCapsStringFlags 
virQEMUCapsObjectPropsMemoryBackendFile[] =
 { "discard-data", QEMU_CAPS_OBJECT_MEMORY_FILE_DISCARD },
 };
 
+static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsSPAPRMachine[] = {
+{ "cap-hpt-mps", QEMU_CAPS_MACHINE_PSERIES_CAP_HPT_MPS },
+};
+
 static virQEMUCapsObjectTypeProps virQEMUCapsObjectProps[] = {
 { "memory-backend-file", virQEMUCapsObjectPropsMemoryBackendFile,
   ARRAY_CARDINALITY(virQEMUCapsObjectPropsMemoryBackendFile),
   QEMU_CAPS_OBJECT_MEMORY_FILE },
+{ "spapr-machine", virQEMUCapsObjectPropsSPAPRMachine,
+  ARRAY_CARDINALITY(virQEMUCapsObjectPropsSPAPRMachine),
+  -1 },
 };
 
 static void
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 3e120e64c0..fecf966f05 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -473,6 +473,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 QEMU_CAPS_SCREENDUMP_DEVICE, /* screendump command accepts device & head */
 QEMU_CAPS_HDA_OUTPUT, /* -device hda-output */
 QEMU_CAPS_BLOCKDEV_DEL, /* blockdev-del is supported */
+QEMU_CAPS_MACHINE_PSERIES_CAP_HPT_MPS, /* -machine pseries,cap-hpt-mps= */
 
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.replies 
b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.replies
index 3ca0ea13fa..5bcbc3e9b7 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.replies
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.replies
@@ -5329,6 +5329,14 @@
   "id": "libvirt-36"
 }
 
+{
+"id": "libvirt-37",
+"error": {
+"class": "DeviceNotFound",
+"desc": "Class 'spapr-machine' not found"
+}
+}
+
 {
   "return": [
 {
@@ -5623,7 +5631,7 @@
   "cpu-max": 1
 }
   ],
-  "id": "libvirt-37"
+  "id": "libvirt-38"
 }
 
 {
@@ -5799,20 +5807,20 @@
   "static": false
 }
   ],
-  "id": "libvirt-38"
+  "id": "libvirt-39"
 }
 
 {
   "return": [
   ],
-  "id": "libvirt-39"
+  "id": "libvirt-40"
 }
 
 {
   "return": [
 "emulator"
   ],
-  "id": "libvirt-40"
+  "id": "libvirt-41"
 }
 
 {
@@ -6973,7 +6981,7 @@
   "option": "drive"
 }
   ],
-  "id": "libvirt-41"
+  "id": "libvirt-42"
 }
 
 {
@@ -7035,7 +7043,7 @@
   "capability": "dirty-bitmaps"
 }
   ],
-  "id": "libvirt-42"
+  "id": "libvirt-43"
 }
 
 {
@@ -18403,7 +18411,7 @@
   "meta-type": "object"
 }
   ],
-  "id": "libvirt-43"
+  "id": "libvirt-44"
 }
 
 {
@@ -18419,7 +18427,7 @@
   "kernel": false
 }
   ],
-  "id": "libvirt-44"
+  "id": "libvirt-45"
 }
 
 {
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml 
b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml
index 0dbd354887..6efd4b4147 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml
@@ -168,7 +168,7 @@
   
   2011090
   0
-  343099
+  343234
   v2.12.0-rc0
   aarch64
   
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.replies 
b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.replies
index 1e93cd6dca..e71d69519d 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.replies
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.replies
@@ -5376,6 +5376,167 @@
   "id": "libvirt-37"
 }
 
+{
+  "return": [
+{
+  "name": "graphics",
+  "description": "Set on/off to enable/disable graphics emulation",
+  "type": "bool"
+},
+{
+  "name": "phandle-start",
+  "description": "The first phandle ID we may generate dynamically",
+  "type": "int"
+},
+{
+  "name": "dump-guest-core",
+  "description": "Include guest memory in  a core dump",
+  "type": "bool"
+},
+{
+  "name": "kernel-irqchip",
+  "description": "Configure KVM in-kernel irqchip",
+ 

[libvirt] [RFC PATCH 1/6] conf: Reintroduce virDomainDef::hpt_resizing

[Andrea Bolognani]

We're going to introduce a second HPT-related setting soon,
at which point using a single location to store everything is
no longer going to cut it.

This mostly, but not completely, reverts 3dd1eb3b2650.

Signed-off-by: Andrea Bolognani 
---
 src/conf/domain_conf.c  | 21 ++---
 src/conf/domain_conf.h  |  1 +
 src/qemu/qemu_command.c |  7 ---
 src/qemu/qemu_domain.c  |  2 +-
 4 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2253098090..355e497002 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -19489,9 +19489,12 @@ virDomainDefParseXML(xmlDocPtr xml,
tmp);
 goto error;
 }
-def->features[val] = value;
+def->hpt_resizing = (virDomainHPTResizing) value;
 VIR_FREE(tmp);
 }
+
+if (def->hpt_resizing != VIR_DOMAIN_HPT_RESIZING_NONE)
+def->features[val] = VIR_TRISTATE_SWITCH_ON;
 break;
 
 /* coverity[dead_error_begin] */
@@ -21621,13 +21624,16 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr 
src,
 break;
 
 case VIR_DOMAIN_FEATURE_HPT:
-if (src->features[i] != dst->features[i]) {
+if (src->features[i] != dst->features[i] ||
+src->hpt_resizing != dst->hpt_resizing) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("State of feature '%s' differs: "
- "source: '%s=%s', destination: '%s=%s'"),
+ "source: '%s,%s=%s', destination: 
'%s,%s=%s'"),
featureName,
-   "resizing", 
virDomainHPTResizingTypeToString(src->features[i]),
-   "resizing", 
virDomainHPTResizingTypeToString(dst->features[i]));
+   virTristateSwitchTypeToString(src->features[i]),
+   "resizing", 
virDomainHPTResizingTypeToString(src->hpt_resizing),
+   virTristateSwitchTypeToString(dst->features[i]),
+   "resizing", 
virDomainHPTResizingTypeToString(dst->hpt_resizing));
 return false;
 }
 break;
@@ -27202,11 +27208,12 @@ virDomainDefFormatInternal(virDomainDefPtr def,
 break;
 
 case VIR_DOMAIN_FEATURE_HPT:
-if (def->features[i] == VIR_DOMAIN_HPT_RESIZING_NONE)
+if (def->features[i] != VIR_TRISTATE_SWITCH_ON ||
+def->hpt_resizing == VIR_DOMAIN_HPT_RESIZING_NONE)
 break;
 
 virBufferAsprintf(buf, "\n",
-  
virDomainHPTResizingTypeToString(def->features[i]));
+  
virDomainHPTResizingTypeToString(def->hpt_resizing));
 break;
 
 /* coverity[dead_error_begin] */
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 37356df42d..cb0945b6c0 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2376,6 +2376,7 @@ struct _virDomainDef {
 int kvm_features[VIR_DOMAIN_KVM_LAST];
 unsigned int hyperv_spinlocks;
 virGICVersion gic_version;
+virDomainHPTResizing hpt_resizing;
 char *hyperv_vendor_id;
 int apic_eoi;
 
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index cb397c7558..ddbde54a8c 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7172,17 +7172,18 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
 }
 }
 
-if (def->features[VIR_DOMAIN_FEATURE_HPT] != VIR_DOMAIN_HPT_RESIZING_NONE) 
{
+if (def->features[VIR_DOMAIN_FEATURE_HPT] == VIR_TRISTATE_SWITCH_ON) {
 const char *str;
 
-if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT)) {
+if (def->hpt_resizing != VIR_DOMAIN_HPT_RESIZING_NONE &&
+!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("HTP resizing is not supported by this "
  "QEMU binary"));
 goto cleanup;
 }
 
-str = 
virDomainHPTResizingTypeToString(def->features[VIR_DOMAIN_FEATURE_HPT]);
+str = virDomainHPTResizingTypeToString(def->hpt_resizing);
 if (!str) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("Invalid setting for HPT resizing"));
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ee676a2789..4bf57b74b6 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3623,7 +3623,7 @@ qemuDomainDefValidateFeatures(const virDomainDef *def)
 break;
 
 case VIR_DOMAIN_FEATURE_HPT:
-if 

[libvirt] [RFC PATCH 5/6] conf: Parse and format HPT maxpagesize

[Andrea Bolognani]

This commit is best viewed with 'git show -w'.

Signed-off-by: Andrea Bolognani 
---
 docs/schemas/domaincommon.rng | 21 ++---
 src/conf/domain_conf.c| 44 ---
 src/conf/domain_conf.h|  1 +
 tests/qemuxml2argvdata/pseries-features.xml   |  4 +-
 tests/qemuxml2argvtest.c  |  1 +
 tests/qemuxml2xmloutdata/pseries-features.xml |  4 +-
 tests/qemuxml2xmltest.c   |  1 +
 7 files changed, 60 insertions(+), 16 deletions(-)

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index f16e157397..7b631f7d93 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -5018,13 +5018,20 @@
 
   
 
-  
-
-  enabled
-  disabled
-  required
-
-  
+  
+
+  
+enabled
+disabled
+required
+  
+
+  
+  
+
+  
+
+  
 
   
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 20b845f02a..e84cfb0d05 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -19493,8 +19493,24 @@ virDomainDefParseXML(xmlDocPtr xml,
 VIR_FREE(tmp);
 }
 
-if (def->hpt_resizing != VIR_DOMAIN_HPT_RESIZING_NONE)
+if (virDomainParseScaledValue("./features/hpt/maxpagesize",
+  NULL,
+  ctxt,
+  >hpt_maxpagesize,
+  1024,
+  ULLONG_MAX,
+  false) < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   "%s",
+   _("Unable to parse HPT maxpagesize setting"));
+goto error;
+}
+def->hpt_maxpagesize = VIR_ROUND_UP(def->hpt_maxpagesize, 1024);
+
+if (def->hpt_resizing != VIR_DOMAIN_HPT_RESIZING_NONE ||
+def->hpt_maxpagesize > 0) {
 def->features[val] = VIR_TRISTATE_SWITCH_ON;
+}
 break;
 
 /* coverity[dead_error_begin] */
@@ -21625,15 +21641,18 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr 
src,
 
 case VIR_DOMAIN_FEATURE_HPT:
 if (src->features[i] != dst->features[i] ||
-src->hpt_resizing != dst->hpt_resizing) {
+src->hpt_resizing != dst->hpt_resizing ||
+src->hpt_maxpagesize != dst->hpt_maxpagesize) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("State of feature '%s' differs: "
- "source: '%s,%s=%s', destination: 
'%s,%s=%s'"),
+ "source: '%s,%s=%s,%s=%llu', destination: 
'%s,%s=%s,%s=%llu'"),
featureName,
virTristateSwitchTypeToString(src->features[i]),
"resizing", 
virDomainHPTResizingTypeToString(src->hpt_resizing),
+   "maxpagesize", src->hpt_maxpagesize,
virTristateSwitchTypeToString(dst->features[i]),
-   "resizing", 
virDomainHPTResizingTypeToString(dst->hpt_resizing));
+   "resizing", 
virDomainHPTResizingTypeToString(dst->hpt_resizing),
+   "maxpagesize", dst->hpt_maxpagesize);
 return false;
 }
 break;
@@ -27209,10 +27228,11 @@ virDomainDefFormatInternal(virDomainDefPtr def,
 
 case VIR_DOMAIN_FEATURE_HPT: {
 bool hasResizing = def->hpt_resizing != 
VIR_DOMAIN_HPT_RESIZING_NONE;
+bool hasMaxPageSize = def->hpt_maxpagesize > 0;
 char *resizing = NULL;
 
 if (def->features[i] != VIR_TRISTATE_SWITCH_ON ||
-!hasResizing) {
+(!hasResizing && !hasMaxPageSize)) {
 break;
 }
 
@@ -27226,8 +27246,18 @@ virDomainDefFormatInternal(virDomainDefPtr def,
 goto error;
 }
 
-virBufferAsprintf(buf, "\n",
-  resizing);
+if (hasMaxPageSize) {
+virBufferAsprintf(buf, "\n",
+  resizing);
+virBufferAdjustIndent(buf, 2);
+virBufferAsprintf(buf, "%llu\n",
+  def->hpt_maxpagesize / 1024);
+virBufferAdjustIndent(buf, -2);
+virBufferAddLit(buf, "\n");
+} else {
+virBufferAsprintf(buf, "\n",
+

[libvirt] [RFC PATCH 2/6] conf: Tweak HPT parsing and formatting

[Andrea Bolognani]

This doesn't seem very useful at the moment, but it will make
sense once we introduce another HPT-related setting.

The output XML is decoupled from the input XML in preparation
of future changes as well; while doing so, we can shave a few
lines off the latter.

This commit is best viewed with 'git show -w'.

Signed-off-by: Andrea Bolognani 
---
 src/conf/domain_conf.c| 25 ---
 src/qemu/qemu_command.c   | 32 ++-
 tests/qemuxml2argvdata/pseries-features.xml   | 14 ++--
 tests/qemuxml2xmloutdata/pseries-features.xml | 29 -
 4 files changed, 68 insertions(+), 32 deletions(-)
 mode change 12 => 100644 tests/qemuxml2xmloutdata/pseries-features.xml

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 355e497002..20b845f02a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -27207,14 +27207,31 @@ virDomainDefFormatInternal(virDomainDefPtr def,
   
virDomainIOAPICTypeToString(def->features[i]));
 break;
 
-case VIR_DOMAIN_FEATURE_HPT:
+case VIR_DOMAIN_FEATURE_HPT: {
+bool hasResizing = def->hpt_resizing != 
VIR_DOMAIN_HPT_RESIZING_NONE;
+char *resizing = NULL;
+
 if (def->features[i] != VIR_TRISTATE_SWITCH_ON ||
-def->hpt_resizing == VIR_DOMAIN_HPT_RESIZING_NONE)
+!hasResizing) {
 break;
+}
+
+if (hasResizing) {
+if (virAsprintf(, " resizing='%s'",
+
virDomainHPTResizingTypeToString(def->hpt_resizing)) < 0) {
+goto error;
+}
+} else {
+if (VIR_STRDUP(resizing, "") < 0)
+goto error;
+}
 
-virBufferAsprintf(buf, "\n",
-  
virDomainHPTResizingTypeToString(def->hpt_resizing));
+virBufferAsprintf(buf, "\n",
+  resizing);
+
+VIR_FREE(resizing);
 break;
+}
 
 /* coverity[dead_error_begin] */
 case VIR_DOMAIN_FEATURE_LAST:
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index ddbde54a8c..b446a08613 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7173,24 +7173,26 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
 }
 
 if (def->features[VIR_DOMAIN_FEATURE_HPT] == VIR_TRISTATE_SWITCH_ON) {
-const char *str;
 
-if (def->hpt_resizing != VIR_DOMAIN_HPT_RESIZING_NONE &&
-!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("HTP resizing is not supported by this "
- "QEMU binary"));
-goto cleanup;
-}
+if (def->hpt_resizing != VIR_DOMAIN_HPT_RESIZING_NONE) {
+const char *str;
 
-str = virDomainHPTResizingTypeToString(def->hpt_resizing);
-if (!str) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("Invalid setting for HPT resizing"));
-goto cleanup;
-}
+if (!virQEMUCapsGet(qemuCaps, 
QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("HTP resizing is not supported by this "
+ "QEMU binary"));
+goto cleanup;
+}
+
+str = virDomainHPTResizingTypeToString(def->hpt_resizing);
+if (!str) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("Invalid setting for HPT resizing"));
+goto cleanup;
+}
 
-virBufferAsprintf(, ",resize-hpt=%s", str);
+virBufferAsprintf(, ",resize-hpt=%s", str);
+}
 }
 
 if (cpu && cpu->model &&
diff --git a/tests/qemuxml2argvdata/pseries-features.xml 
b/tests/qemuxml2argvdata/pseries-features.xml
index 5dd0dbd0be..5ef1a744c8 100644
--- a/tests/qemuxml2argvdata/pseries-features.xml
+++ b/tests/qemuxml2argvdata/pseries-features.xml
@@ -2,27 +2,17 @@
   guest
   1ccfd97d-5eb4-478a-bbe6-88d254c16db7
   524288
-  524288
   1
   
 hvm
-
   
   
 
   
-  
-  destroy
-  restart
-  destroy
   
 /usr/bin/qemu-system-ppc64
-
-
-  
-  
-
+
+
 
-
   
 
diff --git a/tests/qemuxml2xmloutdata/pseries-features.xml 
b/tests/qemuxml2xmloutdata/pseries-features.xml
deleted file mode 12
index 1b01dbace6..00
--- a/tests/qemuxml2xmloutdata/pseries-features.xml
+++ /dev/null
@@ -1 +0,0 @@
-../qemuxml2argvdata/pseries-features.xml
\ No newline at end of file
diff 

[libvirt] [RFC PATCH 0/6] qemu: Support pagesize tuning for pSeries guests

[Andrea Bolognani]

The QEMU part, which is RFC as well, can be found at

  http://lists.nongnu.org/archive/html/qemu-devel/2018-04/msg02818.html

Applies cleanly on top of c49013f26c6b40b741f4d5fc61269898f7fd25b8.

Andrea Bolognani (6):
  conf: Reintroduce virDomainDef::hpt_resizing
  conf: Tweak HPT parsing and formatting
  qemu: Introduce QEMU_CAPS_MACHINE_PSERIES_CAP_HPT_MPS
  tests: Pretend we have pseries.cap-hpt-mps in 2.12
  conf: Parse and format HPT maxpagesize
  qemu: Format pseries.cap-hpt-mps on the command line

 docs/schemas/domaincommon.rng |  21 +-
 src/conf/domain_conf.c|  72 ++-
 src/conf/domain_conf.h|   2 +
 src/qemu/qemu_capabilities.c  |   8 +
 src/qemu/qemu_capabilities.h  |   1 +
 src/qemu/qemu_command.c   |  57 --
 src/qemu/qemu_domain.c|   2 +-
 .../caps_2.12.0.aarch64.replies   |  24 ++-
 .../caps_2.12.0.aarch64.xml   |   2 +-
 .../caps_2.12.0.ppc64.replies | 180 +-
 .../caps_2.12.0.ppc64.xml |   3 +-
 .../caps_2.12.0.s390x.replies |  26 ++-
 .../caps_2.12.0.s390x.xml |   2 +-
 .../caps_2.12.0.x86_64.replies|  30 +--
 .../caps_2.12.0.x86_64.xml|   2 +-
 tests/qemuxml2argvdata/pseries-features.args  |   3 +-
 tests/qemuxml2argvdata/pseries-features.xml   |  18 +-
 tests/qemuxml2argvtest.c  |   1 +
 tests/qemuxml2xmloutdata/pseries-features.xml |  31 ++-
 tests/qemuxml2xmltest.c   |   1 +
 20 files changed, 401 insertions(+), 85 deletions(-)
 mode change 12 => 100644 tests/qemuxml2xmloutdata/pseries-features.xml

-- 
2.17.0

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

Re: [libvirt] [GSoC] Design ideas for implementing cleanup attribute

[Pavel Hrdina]

On Sun, Mar 25, 2018 at 01:55:07AM +0530, Sukrit Bhatnagar wrote:
> Hi,
> 
> I am interested in implementing the GCC cleanup attribute for automatic
> resource freeing as part of GSoC'18. I have shared a proposal for the same.
> 
> This mail is to discuss the code design for implementing it.
> 
> 
> Here are some of my ideas:
> 
> This attribute requires a cleanup function that is called automatically
> when the corresponding variable goes out of scope. There are some functions
> whose logic can be reused:
> 
> - Functions such as virCommandFree, virConfFreeList and virCgroupFree can
> be directly used as cleanup functions. They have parameter and return type
> valid for a cleanup function.
> 
> - Functions such as virFileClose and virFileFclose need some additional
> consideration as they return a value. I think we can set some global
> variable in a separate source file (just like errno variable from errno.h).
> Then the value to be returned can be accessed globally.
> 
> - Functions such as virDomainEventGraphicsDispose need an entirely new
> design. They are used as callbacks in object classes and passed as an
> argument in virClassNew. This would require making changes to
> virObjectUnref's code too. *This is the part I am not sure how to implement
> cleanup logic for.*
> 
> 
> Also, since the __attribute__((__cleanup__(anyfunc))) looks ugly, a macro
> like autoclean (ideas for macro name welcome!) can be used instead. As
> Martin pointed out in my proposal, for some types, this can be done right
> after typedef declarations, so that the type itself contains this attribute.
> 
> Basically, at most places where VIR_FREE is used to release memory
> explicitly, the corresponding variable can use the attribute. The existing
> virFree function also can be reused as it takes void pointer as an argument
> and returns nothing.
> One of the exceptions to this will be those variables which are struct
> members. The cleanup of member has to be done when the enclosing struct
> variable is cleaned.
> 
> I can create new files vircleanup.{c,h} for defining cleanup functions for
> types which do not have an existing cleanup/free function. This can be done
> separately for each driver supported.
> For example, cleanups pertaining to lxc driver will be in
> src/lxc/lxc_cleanup.c.

Hi,

I would like to apologize that it took me that long to reply.

We've already discussed this a little bit off-list so I would like to
summarize my idea about the design of this effort.

I liked the way how GLib is solving the issue so we can simply use the
same approach since it looks reasonable.

There would be three different macros that would be used to annotate
variable with attribute cleanup:

VIR_AUTOFREE char *str = NULL;

- this would call virFree on that variable

VIR_AUTOPTR(virDomain) domain = NULL;

- this would call registered free function on that variable
- to register the free function you would use:

VIR_DEFINE_AUTOPTR_FUNC(virDomain, virDomainFree);

VIR_AUTOCLEAR(virDomain) domain = { 0 };

- this would call registered clear function to free the content of
  that structure
- to register that clear function you would use:

VIR_DEFINE_AUTOCLEAR_FUNC(virDomain, virDomainClear);

In GLib there are three "define" macros and all of them creates a
wrapper function to make sure that the Free/Clear function is called
only if necessary.  This is a safe-guard because it's a public API.
We don't have to have this safe-guard in place since almost all of our
Free functions already check whether the pointer is null or not.

For reference how it works in GLib you can look here [1].

Pavel

[1] 


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

Re: [libvirt] [PATCH 10/12] conf: Add support for choosing emulation of a TPM 2

[Ján Tomko]

On Tue, May 22, 2018 at 04:44:51PM -0400, 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:

   
 
   

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 
Reviewed-by: John Ferlan 
---
docs/formatdomain.html.in  | 15 -
docs/schemas/domaincommon.rng  | 12 
src/conf/domain_conf.c | 27 -
src/conf/domain_conf.h |  6 ++
src/qemu/qemu_tpm.c| 64 +-
.../tpm-emulator-tpm2.x86_64-latest.args   | 33 +++
tests/qemuxml2argvdata/tpm-emulator-tpm2.xml   | 30 ++
tests/qemuxml2argvtest.c   |  1 +
tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml | 34 
9 files changed, 217 insertions(+), 5 deletions(-)
create mode 100644 tests/qemuxml2argvdata/tpm-emulator-tpm2.x86_64-latest.args
create mode 100644 tests/qemuxml2argvdata/tpm-emulator-tpm2.xml
create mode 100644 tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml




@@ -24941,6 +24963,9 @@ virDomainTPMDefFormat(virBufferPtr buf,
virBufferAsprintf(buf, "type));

+if (def->version == VIR_DOMAIN_TPM_VERSION_2)
+virBufferAddLit(buf, " version='2'");
+


Any reason for not formatting version 1.2?
We should format implicit defaults in the XML if possible.


switch (def->type) {
case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
virBufferAddLit(buf, ">\n");
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 92466278ab..e2409899bc 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1291,12 +1291,18 @@ typedef enum {
VIR_DOMAIN_TPM_TYPE_LAST
} virDomainTPMBackendType;

+typedef enum {
+VIR_DOMAIN_TPM_VERSION_1_2,
+VIR_DOMAIN_TPM_VERSION_2,
+} virDomainTPMVersion;


With a corresponding VIR_ENUM_IMPL and VIR_ENUM_DECL,
you can use the *{To,From}String functions for parsing/formatting
the version.


+
# define VIR_DOMAIN_TPM_DEFAULT_DEVICE "/dev/tpm0"

struct _virDomainTPMDef {
virDomainTPMBackendType type;
virDomainDeviceInfo info;
virDomainTPMModel model;
+virDomainTPMVersion version;
union {
struct {
virDomainChrSourceDef source;
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index 11b91aa915..508685c455 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -54,6 +54,41 @@ static char *swtpm_path;
static char *swtpm_setup;
static char *swtpm_ioctl;

+bool swtpm_supports_tpm2;
+
+/*
+ * qemuTPMCheckForTPM2Support
+ *
+ * Check whether swtpm_setup supports TPM 2
+ */
+static void
+qemuTPMCheckForTPM2Support(void)
+{
+virCommandPtr cmd;
+char *help = NULL;
+
+if (!swtpm_setup)
+return;
+
+cmd = virCommandNew(swtpm_setup);
+if (!cmd)
+return;
+
+virCommandAddArg(cmd, "--help");
+virCommandSetOutputBuffer(cmd, );
+
+if (virCommandRun(cmd, NULL) < 0)
+goto cleanup;
+
+if (strstr(help, "--tpm2"))
+swtpm_supports_tpm2 = true;


This bool is never read.

Given that version 2 has to be requested in the XML and we don't try to
use it automatically, I'd suggest just dropping this function. We don't
need to parse another tool's --help output to make up for the removal
of parsing --help of qemu and qemu-img.

Jano


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

Re: [libvirt] [Qemu-devel] [PATCH v7 1/3] qmp: adding 'wakeup-suspend-support' in query-target

[Markus Armbruster]

Eduardo Habkost  writes:

> On Wed, May 23, 2018 at 11:17:55AM +0200, Markus Armbruster wrote:
>> Eduardo Habkost  writes:
>> > On Mon, May 21, 2018 at 04:46:36PM -0300, Daniel Henrique Barboza wrote:
> [...]
>> >> Since no objection was made back then, this logic was put into 
>> >> query-target
>> >> starting
>> >> in v2. Still, I don't have any favorites though: query-target looks ok,
>> >> query-machine
>> >> looks ok and a new API looks ok too. It's all about what makes (more) 
>> >> sense
>> >> in the
>> >> management level, I think.
>> >
>> > I understand the original objection from Eric: having to add a
>> > new command for every runtime flag we want to expose to the user
>> > looks wrong to me.
>> 
>> Agreed.
>> 
>> > However, extending query-machines and query-target looks wrong
>> > too, however.  query-target looks wrong because this not a
>> > property of the target.  query-machines is wrong because this is
>> > not a static property of the machine-type, but of the running
>> > machine instance.
>> 
>> Of the two, query-machines looks less wrong.
>> 
>> Arguably, -no-acpi should not exist.  It's an ad hoc flag that sneakily
>> splits a few machine types into two variants, with and without ACPI.
>> It's silently ignored for other machine types, even APCI-capable ones.
>> 
>> If the machine type variants with and without ACPI were separate types,
>> wakeup-suspend-support would be a static property of the machine type.
>> 
>> However, "separate types" probably doesn't scale: I'm afraid we'd end up
>> with an undesirable number of machine types.  Avoiding that is exactly
>> why we have machine types with configurable options.  I suspect that's
>> how ACPI should be configured (if at all).
>> 
>> So, should we make -no-acpi sugar for a machine type parameter?  And
>> then deprecate -no-acpi for good measure?
>
> I think we should.

Would you like to take care of it?

>> > Can we have a new query command that could be an obvious
>> > container for simple machine capabilities that are not static?  A
>> > name like "query-machine" would be generic enough for that, I
>> > guess.
>> 
>> Having command names differ only in a single letter is awkward, but
>> let's focus on things other than naming now, and use
>> query-current-machine like a working title.
>> 
>> query-machines is wrong because wakeup-suspend-support isn't static for
>> some machine types.
>> 
>> query-current-machine is also kind of wrong because
>> wakeup-suspend-support *is* static for most machine types.
>
> The most appropriate solution depends a lot on how/when
> management software needs to query this.

Right.

> If they only need to query it at runtime for a running VM,
> there's no reason for us to go of our way and add complexity just
> to make it look like static data in query-machines.
>
> On the other hand, if they really need to query it before
> configuring/starting a VM, it won't be useful at all to make it
> available only at runtime.
>
> Daniel, when/how exactly software would need to query the new
> flag?
>
>
>> Worse, a machine type property that is static for all machine types now
>> could conceivably become dynamic when we add a machine type
>> configuration knob.
>> 
>
> This isn't the first time a machine capability that seems static
> actually depends on other configuration arguments.  We will
> probably need to address this eventually.

Then the best time to address it is now, provided we can :)

>> Would a way to tie the property to the configuration knob help?
>> Something like wakeup-suspend-support taking values true (supported),
>> false (not supported), and "acpi" (supported if machine type
>> configuration knob "acpi" is switched on).
>> 
>
> I would prefer a more generic mechanism.  Maybe make
> 'query-machines' accept a 'machine-options' argument?

This can support arbitrary configuration dependencies, unlike my
proposal.  However, I fear combinatorial explosion would make querying
anything but "default configuration" and "current configuration"
impractical, and "default configuration" would be basically useless, as
you can't predict how arguments will affect the value query-machines.

Whether this is an issue depends on how management software wants to use
query-machines.

Whether the ability to support arbitrary configuration dependencies is a
useful feature or an invitation to stupid stunts is another open
question :)

Here's a synthesis of the two proposals: have query-machines spell out
which of its results are determinate, and which configuration bits need
to be supplied to resolve the indeterminate ones.  For machine type
"pc-q35-*", wakeup-suspend-support would always yield true, but for
"pc-i440fx-*" it would return true when passed an acpi: true argument,
false when passed an acpi: false argument, and an encoding of
"indeterminate, you need to pass an acpi argument to learn more" when
passed no acpi argument.

I'm not saying this synthesis makes 

Re: [libvirt] [PATCH 08/12] tests: Add test cases for external swtpm TPM emulator

[Ján Tomko]

On Tue, May 22, 2018 at 04:44:49PM -0400, Stefan Berger wrote:

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

Signed-off-by: Stefan Berger 
Reviewed-by: John Ferlan 
---
.../tpm-emulator.x86_64-latest.args| 33 ++
tests/qemuxml2argvtest.c   | 15 +-
2 files changed, 47 insertions(+), 1 deletion(-)
create mode 100644 tests/qemuxml2argvdata/tpm-emulator.x86_64-latest.args




diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 78454acb1a..587f15242e 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;
+   }


This is indented by three spaces instead of four.

Jano


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

Re: [libvirt] [PATCH 06/12] qemu: Extend QEMU with external TPM support

[Ján Tomko]

On Tue, May 22, 2018 at 04:44:47PM -0400, Stefan Berger wrote:

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

Further, the external TPM needs storage on the host that we need to set
up before it can be run. We can clean up the host once the domain is
undefined.

This patch also implements a small layer for external device support that
calls into the TPM device layer if a domain has an attached TPM. This is
the layer we will wire up later on.



Later on meaning in other patch series? Adding it for just one device
seems excessive, but that might be my personal opinion.


Signed-off-by: Stefan Berger 
Reviewed-by: John Ferlan 
---
src/qemu/Makefile.inc.am  |   4 +
src/qemu/qemu_domain.c|   2 +
src/qemu/qemu_extdevice.c | 154 ++
src/qemu/qemu_extdevice.h |  53 
src/qemu/qemu_process.c   |  12 +
src/qemu/qemu_tpm.c   | 751 ++
src/qemu/qemu_tpm.h   |  50 +++
7 files changed, 1026 insertions(+)
create mode 100644 src/qemu/qemu_extdevice.c
create mode 100644 src/qemu/qemu_extdevice.h
create mode 100644 src/qemu/qemu_tpm.c
create mode 100644 src/qemu/qemu_tpm.h




+/*
+ * virtTPMGetTPMStorageDir:
+ *
+ * @storagepath: directory for swtpm's persistent state
+ *
+ * Derive the 'TPMStorageDir' from the storagepath by searching
+ * for the last '/'.
+ */
+static char *
+qemuTPMGetTPMStorageDir(const char *storagepath)
+{
+const char *tail = strrchr(storagepath, '/');
+char *path = NULL;
+
+if (!tail) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Could not get tail of storagedir %s"),
+   storagepath);
+return NULL;
+}
+ignore_value(VIR_STRNDUP(path, storagepath, tail - storagepath));
+
+return path;
+}


In other places we already use mdir_name from gnulib for this
functionality.


+/*
+ * qemuTPMCreateEmulatorStorage
+ *
+ * @storagepath: directory for swtpm's persistent state
+ * @created: a pointer to a bool that will be set to true if the
+ *   storage was created because it did not exist yet


Can't the caller call virFileExists and act accordingly?


+ * @swtpm_user: The uid that needs to be able to access the directory
+ * @swtpm_group: The gid that needs to be able to access the directory
+ *
+ * Unless the storage path for the swtpm for the given VM
+ * already exists, create it and make it accessible for the given userid.
+ * Adapt ownership of the directory and all swtpm's state files there.
+ */


[...]


+static int
+qemuTPMEmulatorPrepareHost(virDomainTPMDefPtr tpm,
+   const char *logDir,
+   const char *vmname,
+   uid_t swtpm_user,
+   gid_t swtpm_group,
+   const char *swtpmStateDir,
+   uid_t qemu_user,
+   const char *shortName)
+{
+int ret = -1;
+
+if (qemuTPMEmulatorInit() < 0)
+return -1;
+
+/* create log dir ... allow 'tss' user to cd into it */
+if (virFileMakePathWithMode(logDir, 0711) < 0)
+return -1;
+
+/* ... and adjust ownership */
+if (virDirCreate(logDir, 0730, swtpm_user, swtpm_group,
+ VIR_DIR_CREATE_ALLOW_EXIST) < 0)
+goto cleanup;
+
+/* create logfile name ... */
+if (!tpm->data.emulator.logfile &&
+virAsprintf(>data.emulator.logfile, "%s/%s-swtpm.log",
+logDir, vmname) < 0)


This should also use shortName.


+goto cleanup;
+
+/* ... and make sure it can be accessed by swtpm_user */
+if (virFileExists(tpm->data.emulator.logfile) &&
+chown(tpm->data.emulator.logfile, swtpm_user, swtpm_group) < 0) {
+virReportSystemError(errno,
+ _("Could not chown on swtpm logfile %s"),
+ tpm->data.emulator.logfile);
+goto cleanup;
+}
+
+/*
+  create our swtpm state dir ...
+  - QEMU user needs to be able to access the socket there
+  - swtpm group needs to be able to create files there
+  - in privileged mode 0570 would be enough, for non-privileged mode
+we need 0770
+*/
+if (virDirCreate(swtpmStateDir, 0770, qemu_user, swtpm_group,
+ VIR_DIR_CREATE_ALLOW_EXIST) < 0)
+goto cleanup;
+
+/* create the socket filename */
+if (!tpm->data.emulator.source.data.nix.path &&
+!(tpm->data.emulator.source.data.nix.path =
+  qemuTPMCreateEmulatorSocket(swtpmStateDir, shortName)))
+goto cleanup;
+tpm->data.emulator.source.type = VIR_DOMAIN_CHR_TYPE_UNIX;
+
+ret = 0;
+
+ cleanup:
+
+return ret;
+}
+


[...]


+static int

[libvirt] [PATCH] news: Add TLS non-shared storage migration

[Peter Krempa]

Signed-off-by: Peter Krempa 
---
 docs/news.xml | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/docs/news.xml b/docs/news.xml
index 8f2c7d5dff..329b1c7129 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -54,6 +54,16 @@
   a QEMU virtual machine.
 
   
+  
+
+  Add support for migration of QEMU VMs with non-shared storage over 
TLS
+
+
+  It's now possible to use the VIR_MIGRATE_TLS flag together with
+  VIR_MIGRATE_NON_SHARED_DISK. The connection is then secured using the
+  TLS environment which is setup for the migration connection.
+
+  
 
 
   
-- 
2.16.2

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

Re: [libvirt] [PATCH 04/22] virsh: Enhance documentation of cpu-models command

[Kashyap Chamarthy]

On Wed, May 23, 2018 at 02:43:23PM +0200, Jiri Denemark wrote:
> On Wed, May 23, 2018 at 13:08:51 +0200, Kashyap Chamarthy wrote:
> > On Wed, May 16, 2018 at 10:39:23AM +0200, Jiri Denemark wrote:
> > > Signed-off-by: Jiri Denemark 
> > > ---
> > >  tools/virsh.pod | 8 +++-
> > >  1 file changed, 7 insertions(+), 1 deletion(-)

[...]

> > > +Moreover, for some architectures libvirt does not know any CPU models and
> > > +the usable CPU models are only limited by the hypervisor. 
> > 
> > Wonder if it is worth adding a small example for the above.
> 
> An example for what exactly?

I meant an example of a usable CPU model that libvirt doesn't know for
the said architecture(s).  Maybe it's not worth it; not quite sure.

> > > This command will
> > > +print that all CPU models are accepted for these architectures.
> > 
> > s/CPU models are accepted/CPU models that are accepted/
> 
> It's certainly possible my wording was not perfect, but this change
> would make it wrong.

Err, sorry, I misread that sentence; please disregard the above.

[...]

-- 
/kashyap

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

Re: [libvirt] [Qemu-devel] [PATCH v7 1/3] qmp: adding 'wakeup-suspend-support' in query-target

[Daniel Henrique Barboza]

On 05/21/2018 03:14 PM, Eduardo Habkost wrote:

Issue#2: the flag isn't a property of the target.  Due to -no-acpi, it's
not even a property of the machine type.  If it was, query-machines
would be the natural owner of the flag.

Perhaps query-machines is still the proper owner.  The value of
wakeup-suspend-support would have to depend on -no-acpi for the machine
types that honor it.  Not ideal; I'd prefer MachineInfo to be static.
Tolerable?  I guess that's also a libvirt question.

It depends when libvirt is going to query it.  Is it OK to only
query it after the VM is already up and running?  If it is, then
we can simply expose it as a read-only property of the machine
object.

Or, if we don't want to rely on qom-get as a stable API, we can
add a new query command (query-machine? query-power-management?)


In the first version this logic was included in a new query command called
"query-wakeup-from-suspend-support":

https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg00889.html

In that review it was suggested that this logic could be a flag in 
either query-target

or query-machines API. Before sending the v2 I sent the following comment:

"After investigating, I think that it's simpler to hook the wakeup 
support info into
TargetInfo than MachineInfo, given that the detection I'm using for this 
new property
is based on the current runtime state. Hooking into MachineInfo would 
require to
change the MachineClass to add a new property, then setting it up for 
the machines
that have the wakeup support (only x86 so far). Definitely doable, but 
if we don't
have any favorites between MachineInfo and TargetInfo I'd rather pick 
the simpler

route.

So, if no one objects, I'll rework this series by putting the logic 
inside query-target

instead of a new API."

Since no objection was made back then, this logic was put into 
query-target starting
in v2. Still, I don't have any favorites though: query-target looks ok, 
query-machine
looks ok and a new API looks ok too. It's all about what makes (more) 
sense in the

management level, I think.


danielhb



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

[libvirt] Issue

[Mehdi AROUI]

HEllo,

I try to install libvirt-python in my windows 10, with version 3.6 of
python, but I have the message
* pkg-config binary is required to compile libvirt-python*

how to fix that.

thanks in advance
regards.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [PATCH v7 1/3] qmp: adding 'wakeup-suspend-support' in query-target

[Daniel Henrique Barboza]

Hi,

On 05/23/2018 09:27 AM, Eduardo Habkost wrote:

On Wed, May 23, 2018 at 11:17:55AM +0200, Markus Armbruster wrote:

Eduardo Habkost  writes:

On Mon, May 21, 2018 at 04:46:36PM -0300, Daniel Henrique Barboza wrote:

[...]

Since no objection was made back then, this logic was put into query-target
starting
in v2. Still, I don't have any favorites though: query-target looks ok,
query-machine
looks ok and a new API looks ok too. It's all about what makes (more) sense
in the
management level, I think.

I understand the original objection from Eric: having to add a
new command for every runtime flag we want to expose to the user
looks wrong to me.

Agreed.


However, extending query-machines and query-target looks wrong
too, however.  query-target looks wrong because this not a
property of the target.  query-machines is wrong because this is
not a static property of the machine-type, but of the running
machine instance.

Of the two, query-machines looks less wrong.

Arguably, -no-acpi should not exist.  It's an ad hoc flag that sneakily
splits a few machine types into two variants, with and without ACPI.
It's silently ignored for other machine types, even APCI-capable ones.

If the machine type variants with and without ACPI were separate types,
wakeup-suspend-support would be a static property of the machine type.

However, "separate types" probably doesn't scale: I'm afraid we'd end up
with an undesirable number of machine types.  Avoiding that is exactly
why we have machine types with configurable options.  I suspect that's
how ACPI should be configured (if at all).

So, should we make -no-acpi sugar for a machine type parameter?  And
then deprecate -no-acpi for good measure?

I think we should.



Can we have a new query command that could be an obvious
container for simple machine capabilities that are not static?  A
name like "query-machine" would be generic enough for that, I
guess.

Having command names differ only in a single letter is awkward, but
let's focus on things other than naming now, and use
query-current-machine like a working title.

query-machines is wrong because wakeup-suspend-support isn't static for
some machine types.

query-current-machine is also kind of wrong because
wakeup-suspend-support *is* static for most machine types.


The most appropriate solution depends a lot on how/when
management software needs to query this.

If they only need to query it at runtime for a running VM,
there's no reason for us to go of our way and add complexity just
to make it look like static data in query-machines.

On the other hand, if they really need to query it before
configuring/starting a VM, it won't be useful at all to make it
available only at runtime.

Daniel, when/how exactly software would need to query the new
flag?


The original idea of this series was to provide a way to inform management
when not to execute a pm-suspend* command. This is a command from the
guest agent, thus it's only available when the guest is already running. 
As far

as I know there is no way to suspend the VM  without using the guest agent.

Thus, unless management wants to store this state to avoid querying it 
multiple
times during the VM lifetime (I remember from Libvirt code that it 
stores some
sort of capabilities of the domain in an internal state, although I 
can't recall if
this info would be eligible for that), there is no need to query this 
until the VM

is booted.








Worse, a machine type property that is static for all machine types now
could conceivably become dynamic when we add a machine type
configuration knob.


This isn't the first time a machine capability that seems static
actually depends on other configuration arguments.  We will
probably need to address this eventually.



Would a way to tie the property to the configuration knob help?
Something like wakeup-suspend-support taking values true (supported),
false (not supported), and "acpi" (supported if machine type
configuration knob "acpi" is switched on).


I would prefer a more generic mechanism.  Maybe make
'query-machines' accept a 'machine-options' argument?



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

[libvirt] [PATCH v2 5/5] qemu: Remove code for setting up disk passphrases

[Peter Krempa]

Now that the old qcow2 encryption is removed we can safely delete all
this code since it's not needed any more.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_monitor.c  |  13 --
 src/qemu/qemu_monitor.h  |   4 --
 src/qemu/qemu_monitor_json.c |  28 
 src/qemu/qemu_monitor_json.h |   4 --
 src/qemu/qemu_process.c  | 103 ---
 tests/qemumonitorjsontest.c  |   2 -
 6 files changed, 154 deletions(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 43f1d2f816..88a9226e7f 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3044,19 +3044,6 @@ qemuMonitorAddDrive(qemuMonitorPtr mon,
 }


-int
-qemuMonitorSetDrivePassphrase(qemuMonitorPtr mon,
-  const char *alias,
-  const char *passphrase)
-{
-VIR_DEBUG("alias=%s passphrase=%p(value hidden)", alias, passphrase);
-
-QEMU_CHECK_MONITOR(mon);
-
-return qemuMonitorJSONSetDrivePassphrase(mon, alias, passphrase);
-}
-
-
 int
 qemuMonitorCreateSnapshot(qemuMonitorPtr mon, const char *name)
 {
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index c28db1a52b..6200908f25 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -811,10 +811,6 @@ int qemuMonitorAddDrive(qemuMonitorPtr mon,
 int qemuMonitorDriveDel(qemuMonitorPtr mon,
 const char *drivestr);

-int qemuMonitorSetDrivePassphrase(qemuMonitorPtr mon,
-  const char *alias,
-  const char *passphrase);
-
 int qemuMonitorCreateSnapshot(qemuMonitorPtr mon, const char *name);
 int qemuMonitorLoadSnapshot(qemuMonitorPtr mon, const char *name);
 int qemuMonitorDeleteSnapshot(qemuMonitorPtr mon, const char *name);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 9f5c358795..80e710902c 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4062,34 +4062,6 @@ int qemuMonitorJSONDelObject(qemuMonitorPtr mon,
 }


-int qemuMonitorJSONSetDrivePassphrase(qemuMonitorPtr mon,
-  const char *alias,
-  const char *passphrase)
-{
-int ret = -1;
-virJSONValuePtr cmd;
-virJSONValuePtr reply = NULL;
-
-cmd = qemuMonitorJSONMakeCommand("block_passwd",
- "s:device", alias,
- "s:password", passphrase,
- NULL);
-if (!cmd)
-return -1;
-
-if (qemuMonitorJSONCommand(mon, cmd, ) < 0)
-goto cleanup;
-
-if (qemuMonitorJSONCheckError(cmd, reply) < 0)
-goto cleanup;
-
-ret = 0;
- cleanup:
-virJSONValueFree(cmd);
-virJSONValueFree(reply);
-return ret;
-}
-
 int
 qemuMonitorJSONDiskSnapshot(qemuMonitorPtr mon, virJSONValuePtr actions,
 const char *device, const char *file,
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index f4ac8319ac..1c83760dc6 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -237,10 +237,6 @@ int qemuMonitorJSONAddObject(qemuMonitorPtr mon,
 int qemuMonitorJSONDelObject(qemuMonitorPtr mon,
  const char *objalias);

-int qemuMonitorJSONSetDrivePassphrase(qemuMonitorPtr mon,
-  const char *alias,
-  const char *passphrase);
-
 int qemuMonitorJSONDiskSnapshot(qemuMonitorPtr mon,
 virJSONValuePtr actions,
 const char *device,
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index ac2049b95d..d76e3e28a0 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -370,74 +370,6 @@ qemuProcessFindDomainDiskByAlias(virDomainObjPtr vm,
 return NULL;
 }

-static int
-qemuProcessGetVolumeQcowPassphrase(virDomainDiskDefPtr disk,
-   char **secretRet,
-   size_t *secretLen)
-{
-virConnectPtr conn = NULL;
-char *passphrase;
-unsigned char *data;
-size_t size;
-int ret = -1;
-virStorageEncryptionPtr enc;
-
-if (!disk->src->encryption) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("disk %s does not have any encryption information"),
-   disk->src->path);
-return -1;
-}
-enc = disk->src->encryption;
-
-if (!(conn = virGetConnectSecret()))
-goto cleanup;
-
-if (enc->format != VIR_STORAGE_ENCRYPTION_FORMAT_QCOW ||
-enc->nsecrets != 1 ||
-enc->secrets[0]->type !=
-VIR_STORAGE_ENCRYPTION_SECRET_TYPE_PASSPHRASE) {
-virReportError(VIR_ERR_XML_ERROR,
-   _("invalid  for volume %s"),
-   virDomainDiskGetSource(disk));
-goto 

[libvirt] [PATCH v2 3/5] tests: qemublock: Switch to qcow2+luks in test files

[Peter Krempa]

The next patch will forbid the old qcow2 encryption completely. Remove
it from the tests.

Signed-off-by: Peter Krempa 
---
 .../qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.json | 2 +-
 .../qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.xml  | 2 +-
 .../xml2json/network-qcow2-backing-chain-encryption_auth.json   | 2 +-
 .../xml2json/network-qcow2-backing-chain-encryption_auth.xml| 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git 
a/tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.json 
b/tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.json
index 3469c06654..376fce9f9e 100644
--- a/tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.json
+++ b/tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.json
@@ -21,7 +21,7 @@
   "read-only": true,
   "driver": "qcow2",
   "encrypt": {
-"format": "aes",
+"format": "luks",
 "key-secret": "node-b-f-encalias"
   },
   "file": "node-b-s",
diff --git 
a/tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.xml 
b/tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.xml
index a1292284bf..75a3a8f029 100644
--- a/tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.xml
+++ b/tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.xml
@@ -20,7 +20,7 @@
   
 
   
-  
+  
 
   
 
diff --git 
a/tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-encryption_auth.json
 
b/tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-encryption_auth.json
index 6e5abbfbdd..fdb6f2ab1a 100644
--- 
a/tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-encryption_auth.json
+++ 
b/tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-encryption_auth.json
@@ -33,7 +33,7 @@
   "read-only": true,
   "driver": "qcow2",
   "encrypt": {
-"format": "aes",
+"format": "luks",
 "key-secret": "node-b-f-encalias"
   },
   "file": "node-b-s",
diff --git 
a/tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-encryption_auth.xml
 
b/tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-encryption_auth.xml
index bc2925b4cf..a62c0321ec 100644
--- 
a/tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-encryption_auth.xml
+++ 
b/tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-encryption_auth.xml
@@ -26,7 +26,7 @@
   
 
   
-  
+  
 
   
   
-- 
2.16.2

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

[libvirt] [PATCH v2 1/5] tests: qemuxml2argv: Drop disk encryption from 'interface-server' test

[Peter Krempa]

The disk encryption part is no way relevant to the rest of the test so
drop it.

Signed-off-by: Peter Krempa 
---
 tests/qemuxml2argvdata/interface-server.xml   | 3 ---
 tests/qemuxml2xmloutdata/interface-server.xml | 3 ---
 2 files changed, 6 deletions(-)

diff --git a/tests/qemuxml2argvdata/interface-server.xml 
b/tests/qemuxml2argvdata/interface-server.xml
index a92aff4218..7bf119197a 100644
--- a/tests/qemuxml2argvdata/interface-server.xml
+++ b/tests/qemuxml2argvdata/interface-server.xml
@@ -53,9 +53,6 @@
   
   
   
-  
-
-  
   
 
 
diff --git a/tests/qemuxml2xmloutdata/interface-server.xml 
b/tests/qemuxml2xmloutdata/interface-server.xml
index 049b1472a8..75b12bf96f 100644
--- a/tests/qemuxml2xmloutdata/interface-server.xml
+++ b/tests/qemuxml2xmloutdata/interface-server.xml
@@ -53,9 +53,6 @@
   
   
   
-  
-
-  
   
 
 
-- 
2.16.2

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

[libvirt] [PATCH v2 0/5] qemu: Forbid old qcow/qcow2 encryption

[Peter Krempa]

The old qcow/qcow2 encryption format is so broken that qemu decided to
drop it completely. This series forbids the use of such images even with
qemus prior to this and removes all the cruft necessary to support it.

v2:
 - fixed check to include the qcow format too
 - reworded the error message slightly
 - split second patch into two with proper justification for the
   user-alias test since checking LUKS there actually makes sense

Peter Krempa (5):
  tests: qemuxml2argv: Drop disk encryption from 'interface-server' test
  tests: qemuxml2argv: Verify that disk secret alias is correct with
user-aliases
  tests: qemublock: Switch to qcow2+luks in test files
  qemu: domain: Forbid storage with old QCOW2 encryption
  qemu: Remove code for setting up disk passphrases

 src/qemu/qemu_domain.c |  10 ++
 src/qemu/qemu_monitor.c|  13 ---
 src/qemu/qemu_monitor.h|   4 -
 src/qemu/qemu_monitor_json.c   |  28 --
 src/qemu/qemu_monitor_json.h   |   4 -
 src/qemu/qemu_process.c| 103 -
 .../file-qcow2-backing-chain-encryption.json   |   2 +-
 .../file-qcow2-backing-chain-encryption.xml|   2 +-
 ...etwork-qcow2-backing-chain-encryption_auth.json |   2 +-
 ...network-qcow2-backing-chain-encryption_auth.xml |   2 +-
 tests/qemumonitorjsontest.c|   2 -
 tests/qemuxml2argvdata/encrypted-disk-usage.args   |   8 +-
 tests/qemuxml2argvdata/encrypted-disk-usage.xml|   2 +-
 tests/qemuxml2argvdata/encrypted-disk.args |   8 +-
 tests/qemuxml2argvdata/encrypted-disk.xml  |   2 +-
 tests/qemuxml2argvdata/interface-server.xml|   3 -
 tests/qemuxml2argvdata/user-aliases.args   |   8 +-
 tests/qemuxml2argvdata/user-aliases.xml|   2 +-
 tests/qemuxml2argvtest.c   |   7 +-
 tests/qemuxml2xmloutdata/encrypted-disk.xml|   2 +-
 tests/qemuxml2xmloutdata/interface-server.xml  |   3 -
 tests/qemuxml2xmltest.c|   6 +-
 22 files changed, 46 insertions(+), 177 deletions(-)

-- 
2.16.2

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

[libvirt] [PATCH v2 4/5] qemu: domain: Forbid storage with old QCOW2 encryption

[Peter Krempa]

The encryption was buggy and qemu actually dropped it upstream. Forbid
it for all versions since it would cause other problems too.

Problems with the old encryption include weak crypto, corruption of
images with blockjobs and a lot of usability problems.

This requires changing of the encryption type for the encrypted disk
tests.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_domain.c   | 10 ++
 tests/qemuxml2argvdata/encrypted-disk-usage.args |  8 +++-
 tests/qemuxml2argvdata/encrypted-disk-usage.xml  |  2 +-
 tests/qemuxml2argvdata/encrypted-disk.args   |  8 +++-
 tests/qemuxml2argvdata/encrypted-disk.xml|  2 +-
 tests/qemuxml2argvtest.c |  4 ++--
 tests/qemuxml2xmloutdata/encrypted-disk.xml  |  2 +-
 tests/qemuxml2xmltest.c  |  4 ++--
 8 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ee676a2789..23dd4dab0e 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4349,6 +4349,16 @@ qemuDomainValidateStorageSource(virStorageSourcePtr src,
 return -1;
 }

+if ((src->format == VIR_STORAGE_FILE_QCOW ||
+ src->format == VIR_STORAGE_FILE_QCOW2) &&
+src->encryption &&
+(src->encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT ||
+ src->encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_QCOW)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("old qcow/qcow2 encryption is not supported"));
+return -1;
+}
+
 if (src->format == VIR_STORAGE_FILE_QCOW2 &&
 src->encryption &&
 src->encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS &&
diff --git a/tests/qemuxml2argvdata/encrypted-disk-usage.args 
b/tests/qemuxml2argvdata/encrypted-disk-usage.args
index 8c7ce3d653..32307cea71 100644
--- a/tests/qemuxml2argvdata/encrypted-disk-usage.args
+++ b/tests/qemuxml2argvdata/encrypted-disk-usage.args
@@ -7,6 +7,8 @@ QEMU_AUDIO_DRV=none \
 /usr/bin/qemu-system-i686 \
 -name encryptdisk \
 -S \
+-object secret,id=masterKey0,format=raw,\
+file=/tmp/lib/domain--1-encryptdisk/master-key.aes \
 -machine pc,accel=tcg,usb=off,dump-guest-core=off \
 -m 1024 \
 -smp 1,sockets=1,cores=1,threads=1 \
@@ -22,7 +24,11 @@ 
path=/tmp/lib/domain--1-encryptdisk/monitor.sock,server,nowait \
 -no-acpi \
 -boot c \
 -usb \
--drive file=/storage/guest_disks/encryptdisk,format=qcow2,if=none,\
+-object secret,id=virtio-disk0-luks-secret0,\
+data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\
+keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \
+-drive file=/storage/guest_disks/encryptdisk,encrypt.format=luks,\
+encrypt.key-secret=virtio-disk0-luks-secret0,format=qcow2,if=none,\
 id=drive-virtio-disk0 \
 -device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,\
 id=virtio-disk0 \
diff --git a/tests/qemuxml2argvdata/encrypted-disk-usage.xml 
b/tests/qemuxml2argvdata/encrypted-disk-usage.xml
index ad8f17e3df..205283b59d 100644
--- a/tests/qemuxml2argvdata/encrypted-disk-usage.xml
+++ b/tests/qemuxml2argvdata/encrypted-disk-usage.xml
@@ -18,7 +18,7 @@
   
   
   
-  
+  
 
   
   
diff --git a/tests/qemuxml2argvdata/encrypted-disk.args 
b/tests/qemuxml2argvdata/encrypted-disk.args
index 8c7ce3d653..32307cea71 100644
--- a/tests/qemuxml2argvdata/encrypted-disk.args
+++ b/tests/qemuxml2argvdata/encrypted-disk.args
@@ -7,6 +7,8 @@ QEMU_AUDIO_DRV=none \
 /usr/bin/qemu-system-i686 \
 -name encryptdisk \
 -S \
+-object secret,id=masterKey0,format=raw,\
+file=/tmp/lib/domain--1-encryptdisk/master-key.aes \
 -machine pc,accel=tcg,usb=off,dump-guest-core=off \
 -m 1024 \
 -smp 1,sockets=1,cores=1,threads=1 \
@@ -22,7 +24,11 @@ 
path=/tmp/lib/domain--1-encryptdisk/monitor.sock,server,nowait \
 -no-acpi \
 -boot c \
 -usb \
--drive file=/storage/guest_disks/encryptdisk,format=qcow2,if=none,\
+-object secret,id=virtio-disk0-luks-secret0,\
+data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\
+keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \
+-drive file=/storage/guest_disks/encryptdisk,encrypt.format=luks,\
+encrypt.key-secret=virtio-disk0-luks-secret0,format=qcow2,if=none,\
 id=drive-virtio-disk0 \
 -device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,\
 id=virtio-disk0 \
diff --git a/tests/qemuxml2argvdata/encrypted-disk.xml 
b/tests/qemuxml2argvdata/encrypted-disk.xml
index 391461b200..275724bdaf 100644
--- a/tests/qemuxml2argvdata/encrypted-disk.xml
+++ b/tests/qemuxml2argvdata/encrypted-disk.xml
@@ -18,7 +18,7 @@
   
   
   
-  
+  
 
   
   
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 38530cdb5c..f9ac79f4a4 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1635,8 +1635,8 @@ mymain(void)
 

[libvirt] [PATCH v2 2/5] tests: qemuxml2argv: Verify that disk secret alias is correct with user-aliases

[Peter Krempa]

Change the disk encryption type to qcow2+luks so that the appropriate
secret objects are generated. This tests that the proper alias is used
for the passphrase secret object.

Signed-off-by: Peter Krempa 
---
 tests/qemuxml2argvdata/user-aliases.args | 8 +++-
 tests/qemuxml2argvdata/user-aliases.xml  | 2 +-
 tests/qemuxml2argvtest.c | 3 ++-
 tests/qemuxml2xmltest.c  | 2 +-
 4 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/tests/qemuxml2argvdata/user-aliases.args 
b/tests/qemuxml2argvdata/user-aliases.args
index 5ef52fc556..293dc919d5 100644
--- a/tests/qemuxml2argvdata/user-aliases.args
+++ b/tests/qemuxml2argvdata/user-aliases.args
@@ -7,6 +7,8 @@ QEMU_AUDIO_DRV=none \
 /usr/bin/qemu-system-x86_64 \
 -name gentoo \
 -S \
+-object secret,id=masterKey0,format=raw,\
+file=/tmp/lib/domain--1-gentoo/master-key.aes \
 -machine pc-i440fx-1.4,accel=kvm,usb=off,dump-guest-core=off \
 -m 4096 \
 -smp 4,sockets=4,cores=1,threads=1 \
@@ -43,7 +45,11 @@ id=drive-ua-myDisk1,cache=none \
 -drive file=/var/lib/libvirt/images/gentoo.qcow2,format=qcow2,if=none,\
 id=drive-ua-myDisk2 \
 -device virtio-blk-pci,bus=pci.0,addr=0x5,drive=drive-ua-myDisk2,id=ua-myDisk2 
\
--drive file=/var/lib/libvirt/images/OtherDemo.img,format=qcow2,if=none,\
+-object secret,id=ua-myEncryptedDisk1-luks-secret0,\
+data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\
+keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \
+-drive file=/var/lib/libvirt/images/OtherDemo.img,encrypt.format=luks,\
+encrypt.key-secret=ua-myEncryptedDisk1-luks-secret0,format=qcow2,if=none,\
 id=drive-ua-myEncryptedDisk1 \
 -device virtio-blk-pci,bus=pci.0,addr=0x7,drive=drive-ua-myEncryptedDisk1,\
 id=ua-myEncryptedDisk1 \
diff --git a/tests/qemuxml2argvdata/user-aliases.xml 
b/tests/qemuxml2argvdata/user-aliases.xml
index 9ce123b477..98b4845e52 100644
--- a/tests/qemuxml2argvdata/user-aliases.xml
+++ b/tests/qemuxml2argvdata/user-aliases.xml
@@ -55,7 +55,7 @@
   
   
   
-  
+  
 
   
   
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 1d023129ac..38530cdb5c 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2823,7 +2823,8 @@ mymain(void)
 QEMU_CAPS_PIIX_DISABLE_S4, QEMU_CAPS_VNC,
 QEMU_CAPS_DEVICE_ISA_SERIAL,
 QEMU_CAPS_HDA_DUPLEX,
-QEMU_CAPS_CCID_EMULATED);
+QEMU_CAPS_CCID_EMULATED,
+QEMU_CAPS_QCOW2_LUKS, QEMU_CAPS_OBJECT_SECRET);
 DO_TEST("user-aliases2", QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_ICH9_AHCI);
 DO_TEST("user-aliases-usb", QEMU_CAPS_KVM,
 QEMU_CAPS_PIIX_DISABLE_S3, QEMU_CAPS_PIIX_DISABLE_S4,
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index e31d8212fe..b4f9161056 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1171,7 +1171,7 @@ mymain(void)
 DO_TEST("pseries-cpu-exact",
 QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE);

-DO_TEST("user-aliases", NONE);
+DO_TEST("user-aliases", QEMU_CAPS_QCOW2_LUKS);
 DO_TEST("input-virtio-ccw",
 QEMU_CAPS_CCW,
 QEMU_CAPS_VIRTIO_KEYBOARD,
-- 
2.16.2

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

[libvirt] [PATCH] Fix indentation of virshAllocpagesPagesizeCompleter arguments.

[Roland Schulz]

---
 tools/virsh-completer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
index cbd5326d0..1df4d55af 100644
--- a/tools/virsh-completer.c
+++ b/tools/virsh-completer.c
@@ -568,8 +568,8 @@ virshSnapshotNameCompleter(vshControl *ctl,
 
 char **
 virshAllocpagesPagesizeCompleter(vshControl *ctl,
-const vshCmd *cmd ATTRIBUTE_UNUSED,
-unsigned int flags)
+ const vshCmd *cmd ATTRIBUTE_UNUSED,
+ unsigned int flags)
 {
 unsigned long long byteval = 0;
 xmlXPathContextPtr ctxt = NULL;
-- 
2.17.0

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

Re: [libvirt] [PATCH v2] Edit test capabilities to contain different cell pagesizes.

[Michal Privoznik]

On 05/23/2018 02:40 PM, Roland Schulz wrote:
> Signed-off-by: Roland Schulz 
> ---
>  src/test/test_driver.c | 18 +++---
>  1 file changed, 11 insertions(+), 7 deletions(-)

In the $SUBJ I'd add the prefix "test driver:" to tell explicitly what
part of code this commit touches.

> 
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index 467587b19..3fe0c2831 100644
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -322,30 +322,34 @@ testBuildCapabilities(virConnectPtr conn)
>  if (virCapabilitiesAddHostFeature(caps, "nonpae") < 0)
>  goto error;
>  
> -if (VIR_ALLOC_N(caps->host.pagesSize, 2) < 0)
> +if (VIR_ALLOC_N(caps->host.pagesSize, 4) < 0)
>  goto error;
>  
>  caps->host.pagesSize[caps->host.nPagesSize++] = 4;
> +caps->host.pagesSize[caps->host.nPagesSize++] = 8;
>  caps->host.pagesSize[caps->host.nPagesSize++] = 2048;
> +caps->host.pagesSize[caps->host.nPagesSize++] = 1024 * 1024;
>  
>  for (i = 0; i < privconn->numCells; i++) {
>  virCapsHostNUMACellCPUPtr cpu_cells;
>  virCapsHostNUMACellPageInfoPtr pages;
> -size_t nPages;
> +size_t nPages = caps->host.nPagesSize - 1;
>  
>  if (VIR_ALLOC_N(cpu_cells, privconn->cells[i].numCpus) < 0 ||
> -VIR_ALLOC_N(pages, caps->host.nPagesSize) < 0) {
> +VIR_ALLOC_N(pages, caps->host.nPagesSize - 1) < 0) {

or s/caps->host.nPagesSize - 1/nPages/ since they will always have the
same value.

>  VIR_FREE(cpu_cells);
>  goto error;
>  }
> -
> -nPages = caps->host.nPagesSize;
> +if (i == 1)
> +pages[0].size = caps->host.pagesSize[0];
> +else
> +pages[0].size = caps->host.pagesSize[1];

I'd move this after the memcpy so that pages[] initialization is kept
together with the for() loop below the memcpy(). IOW, memcpy should be
moved before this if() statement.

>  
>  memcpy(cpu_cells, privconn->cells[i].cpus,
> sizeof(*cpu_cells) * privconn->cells[i].numCpus);
>  
> -for (j = 0; j < nPages; j++)
> -pages[j].size = caps->host.pagesSize[j];
> +for (j = 1; j < nPages; j++)
> +pages[j].size = caps->host.pagesSize[j+1];

Too much spaces ;-) Also, "j + 1" instead of "j+1".

Fixing all the issues, ACKing and pushing.

Michal

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

Re: [libvirt] [PATCH] Fix memory leak issues in virshAllocpagesPagesizeCompleter.

[Michal Privoznik]

On 05/23/2018 02:40 PM, Roland Schulz wrote:
> Signed-off-by: Roland Schulz 
> ---
>  tools/virsh-completer.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
> index 1435d1d4c..c72f3bcad 100644
> --- a/tools/virsh-completer.c
> +++ b/tools/virsh-completer.c
> @@ -576,6 +576,7 @@ virshAllocpagesPagesizeCompleter(vshControl *ctl,
>  virshControlPtr priv = ctl->privData;
>  unsigned int npages = 0;
>  xmlNodePtr *pages = NULL;
> +xmlDocPtr doc = NULL;
>  double size = 0;
>  size_t i = 0;
>  const char *suffix = NULL;
> @@ -595,7 +596,7 @@ virshAllocpagesPagesizeCompleter(vshControl *ctl,
>  if (!(cap_xml = virConnectGetCapabilities(priv->conn)))
>  goto error;
>  
> -if (!(virXMLParseStringCtxt(cap_xml, _("capabilities"), )))
> +if ((doc = virXMLParseStringCtxt(cap_xml, _("capabilities"), )) == 
> NULL)
>  goto error;

You can keep the !ptr comparison just like @cap_xml is tested a few
lines above.

>  
>  if (cellno && vshCommandOptStringQuiet(ctl, cmd, "cellno", ) > 
> 0) {
> @@ -631,12 +632,11 @@ virshAllocpagesPagesizeCompleter(vshControl *ctl,
>  
>   cleanup:
>  xmlXPathFreeContext(ctxt);
> -for (i = 0; i < npages; i++)
> -VIR_FREE(pages[i]);
>  VIR_FREE(pages);
> -VIR_FREE(cap_xml);
> +xmlFreeDoc(doc);
>  VIR_FREE(path);
>  VIR_FREE(pagesize);
> +VIR_FREE(cap_xml);
>  VIR_FREE(unit);
>  
>  return ret;
> 

ACKed and pushed. Thank you for addressing all the points I raised.

Michal

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

[libvirt] AppArmor support for TPM emulator; was:Re: [PATCH 00/12] Add support for TPM emulator

[Stefan Berger]

On 05/23/2018 08:07 AM, John Ferlan wrote:


On 05/22/2018 04:44 PM, Stefan Berger wrote:

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

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

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

   Stefan

v5->v6:
   - Addressed John Ferlan's comments
   - rebased on latest tip
   - Added patch 12.

v4->v5:
   - Addressed John Ferlan's, Boris Fiuczysnki's and Marc Hartmayer's comments
   - rebased on latest tip

v3->v4:
   - Addressed John Ferlan's comments
   - Fixed bugs I found while testing
   - rebased on latest tip

Stefan Berger (12):
   conf: Add support for external swtpm TPM emulator to domain XML
   qemu: Extend QEMU capabilities with 'tpm-emulator'
   util: Implement virFileChownFiles()
   security: Add DAC and SELinux security for tpm-emulator
   qemu: Extend qemu_conf with tpm-emulator support
   qemu: Extend QEMU with external TPM support
   qemu: Add support for external swtpm TPM emulator
   tests: Add test cases for external swtpm TPM emulator
   security: Label the external swtpm with SELinux labels
   conf: Add support for choosing emulation of a TPM 2
   qemu: Add swtpm to emulator cgroup
   news: Update news with new TPM emulator feature

  docs/formatdomain.html.in  |  43 +
  docs/news.xml  |   9 +
  docs/schemas/domaincommon.rng  |  17 +
  libvirt.spec.in|   2 +
  src/conf/domain_audit.c|   2 +
  src/conf/domain_conf.c |  53 +-
  src/conf/domain_conf.h |  12 +
  src/libvirt_private.syms   |   3 +
  src/qemu/Makefile.inc.am   |  10 +
  src/qemu/libvirtd_qemu.aug |   5 +
  src/qemu/qemu.conf |   8 +
  src/qemu/qemu_capabilities.c   |   5 +
  src/qemu/qemu_capabilities.h   |   1 +
  src/qemu/qemu_cgroup.c |  36 +
  src/qemu/qemu_cgroup.h |   2 +
  src/qemu/qemu_command.c|  34 +-
  src/qemu/qemu_conf.c   |  43 +
  src/qemu/qemu_conf.h   |   6 +
  src/qemu/qemu_domain.c |   3 +
  src/qemu/qemu_extdevice.c  | 180 
  src/qemu/qemu_extdevice.h  |  59 ++
  src/qemu/qemu_process.c|  16 +
  src/qemu/qemu_security.c   |  69 ++
  src/qemu/qemu_security.h   |  11 +
  src/qemu/qemu_tpm.c| 946 +
  src/qemu/qemu_tpm.h|  56 ++
  src/qemu/test_libvirtd_qemu.aug.in |   2 +
  src/security/security_dac.c|   7 +
  src/security/security_driver.h |   7 +
  src/security/security_manager.c|  36 +
  src/security/security_manager.h|   6 +
  src/security/security_selinux.c| 172 
  src/security/security_stack.c  |  40 +
  src/util/virfile.c |  55 ++
  src/util/virfile.h |   3 +
  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 +
  .../tpm-emulator-tpm2.x86_64-latest.args   |  33 +
  tests/qemuxml2argvdata/tpm-emulator-tpm2.xml   |  30 +
  .../tpm-emulator.x86_64-latest.args|  33 +
  tests/qemuxml2argvdata/tpm-emulator.xml|  30 +
  tests/qemuxml2argvtest.c   |  16 +-
  tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml |  34 +
  tests/qemuxml2xmloutdata/tpm-emulator.xml  |  34 +
  tests/qemuxml2xmltest.c|   1 +
  48 files changed, 2165 insertions(+), 10 deletions(-)
  create mode 100644 src/qemu/qemu_extdevice.c
  create mode 100644 src/qemu/qemu_extdevice.h
  create mode 100644 

Re: [libvirt] [PATCH 4/8] qemu: monitor: Drop QEMU_CHECK_MONITOR_JSON... macros

[Ján Tomko]

On Wed, May 23, 2018 at 03:04:47PM +0200, Peter Krempa wrote:

On Tue, May 22, 2018 at 15:24:06 +0200, Ján Tomko wrote:

On Tue, May 22, 2018 at 02:35:44PM +0200, Peter Krempa wrote:
> Monitor is now JSON only. Drop the old cruft.
>

In theory, you should still be able to get a domain with a text monitor
via qemu-attach.

In practice, attaching to such domains seems pointless - if we don't
support starting them, touching their mointor might just break them.


I can leave the part of the macro that checks whether the monitor is
JSON still in. That way it will at least report sane errors.



Sounds good to me.

Even with that change:
Reviewed-by: Ján Tomko 

Jano


Maybe we should drop qemu-attach as well?


I hope so, but it should be probably done separately.






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

Re: [libvirt] [PATCH 4/8] qemu: monitor: Drop QEMU_CHECK_MONITOR_JSON... macros

[Peter Krempa]

On Tue, May 22, 2018 at 15:24:06 +0200, Ján Tomko wrote:
> On Tue, May 22, 2018 at 02:35:44PM +0200, Peter Krempa wrote:
> > Monitor is now JSON only. Drop the old cruft.
> > 
> 
> In theory, you should still be able to get a domain with a text monitor
> via qemu-attach.
> 
> In practice, attaching to such domains seems pointless - if we don't
> support starting them, touching their mointor might just break them.

I can leave the part of the macro that checks whether the monitor is
JSON still in. That way it will at least report sane errors.

> Maybe we should drop qemu-attach as well?

I hope so, but it should be probably done separately.



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

Re: [libvirt] [PATCH] qemu: monitor: Remove diskSecretLookup monitor event

[Ján Tomko]

On Tue, May 22, 2018 at 03:57:57PM +0200, Peter Krempa wrote:

After the text monitor was deleted this event can't be triggered.
Remove it and all the unnecessary code.

Signed-off-by: Peter Krempa 
---

This applies on top of the text monitor removal series. I forgot to
include it while splitting branches.

src/qemu/qemu_monitor.c | 16 
src/qemu/qemu_monitor.h | 12 
src/qemu/qemu_process.c | 28 
3 files changed, 56 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 08/22] Introduce virConnectCompareHypervisorCPU public API

[Jiri Denemark]

On Tue, May 22, 2018 at 18:27:51 -0400, Collin Walling wrote:
> On 05/16/2018 04:39 AM, Jiri Denemark wrote:
> > This new API compares the given CPU description with the CPU the
> > specified hypervisor is able to provide on the host. It is a more useful
> > version of virConnectCompareCPU, which compares the CPU definition with
> > the host CPU without considering any specific hypervisor and its
> > abilities.
> > 
> > Signed-off-by: Jiri Denemark 
> > ---
> >  include/libvirt/libvirt-host.h |  7 
> >  src/driver-hypervisor.h| 10 +
> >  src/libvirt-host.c | 72 +-
> >  src/libvirt_public.syms|  5 +++
> >  4 files changed, 93 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/libvirt/libvirt-host.h b/include/libvirt/libvirt-host.h
> > index 07b5d15943..e2054baebc 100644
> > --- a/include/libvirt/libvirt-host.h
> > +++ b/include/libvirt/libvirt-host.h
> > @@ -640,6 +640,13 @@ typedef enum {
> >  int virConnectCompareCPU(virConnectPtr conn,
> >   const char *xmlDesc,
> >   unsigned int flags);
> > +int virConnectCompareHypervisorCPU(virConnectPtr conn,
> > +   const char *emulator,
> > +   const char *arch,
> > +   const char *machine,
> > +   const char *virttype,
> > +   const char *xmlCPU,
> > +   unsigned int flags);
> >  
> >  int virConnectGetCPUModelNames(virConnectPtr conn,
> > const char *arch,
> > diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
> > index e71a72a441..d64de2d54c 100644
> > --- a/src/driver-hypervisor.h
> > +++ b/src/driver-hypervisor.h
> > @@ -673,6 +673,15 @@ typedef int
> > const char *cpu,
> > unsigned int flags);
> >  
> > +typedef int
> > +(*virDrvConnectCompareHypervisorCPU)(virConnectPtr conn,
> > + const char *emulator,
> > + const char *arch,
> > + const char *machine,
> > + const char *virttype,
> > + const char *xmlCPU,
> > + unsigned int flags);
> > +
> >  typedef char *
> >  (*virDrvConnectBaselineCPU)(virConnectPtr conn,
> >  const char **xmlCPUs,
> > @@ -1532,6 +1541,7 @@ struct _virHypervisorDriver {
> >  virDrvDomainSetVcpu domainSetVcpu;
> >  virDrvDomainSetBlockThreshold domainSetBlockThreshold;
> >  virDrvDomainSetLifecycleAction domainSetLifecycleAction;
> > +virDrvConnectCompareHypervisorCPU connectCompareHypervisorCPU;
> >  };
> >  
> >  
> > diff --git a/src/libvirt-host.c b/src/libvirt-host.c
> > index ed689b9ec2..17cf183499 100644
> > --- a/src/libvirt-host.c
> > +++ b/src/libvirt-host.c
> > @@ -954,7 +954,11 @@ virConnectIsSecure(virConnectPtr conn)
> >   * @xmlDesc: XML describing the CPU to compare with host CPU
> >   * @flags: bitwise-OR of virConnectCompareCPUFlags
> >   *
> > - * Compares the given CPU description with the host CPU
> > + * Compares the given CPU description with the host CPU.
> > + *
> > + * See vitConnectCompareHypervisorCPU() if you want to consider hypervisor
> > + * abilities and compare the CPU to the CPU which a hypervisor is able to
> > + * provide on the host.
> >   *
> >   * Returns comparison result according to enum virCPUCompareResult. If
> >   * VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE is used and @xmlDesc CPU is
> > @@ -992,6 +996,72 @@ virConnectCompareCPU(virConnectPtr conn,
> >  }
> >  
> >  
> > +/**
> > + * virConnectCompareHypervisorCPU:
> > + * @conn: pointer to the hypervisor connection
> > + * @emulator: path to the emulator binary
> > + * @arch: domain architecture
> > + * @machine: machine type
> > + * @virttype: virtualization type
> > + * @xmlCPU: XML describing the CPU to be compared
> > + * @flags: bitwise-OR of virConnectCompareCPUFlags
> > + *
> > + * Compares the given CPU description with the CPU the specified 
> > hypervisor is
> > + * able to provide on the host. Any of @emulator, @arch, @machine, and
> > + * @virttype parameters may be NULL; libvirt will choose sensible defaults
> > + * tailored to the host and its current configuration.
> > + *
> > + * This is different from virConnectCompareCPU() which compares the CPU
> > + * definition with the host CPU without considering any specific 
> > hypervisor and
> > + * its abilities.
> > + *
> > + * Returns comparison result according to enum virCPUCompareResult. If
> > + * VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE is used and @xmlCPU is
> > + * incompatible with the CPU the specified hypervisor is able to provide 
> > on the
> > + * host, this function will return 

Re: [libvirt] [PATCH 07/22] qemu_capabilities: Introduce virQEMUCapsCacheLookupDefault

[Jiri Denemark]

On Tue, May 22, 2018 at 18:24:57 -0400, Collin Walling wrote:
> On 05/16/2018 04:39 AM, Jiri Denemark wrote:
> > virConnectGetDomainCapabilities needs to lookup QEMU capabilities
> > matching a specified binary, architecture, virt type, and machine type
> > while using default values when any of the parameters are not provided
> > by the user. Let's extract the lookup code into
> > virQEMUCapsCacheLookupDefault to make it reusable.
> > 
> > Signed-off-by: Jiri Denemark 
> > ---
> >  src/qemu/qemu_capabilities.c | 118 +++
> >  src/qemu/qemu_capabilities.h |   8 +++
> >  src/qemu/qemu_driver.c   |  86 -
> >  3 files changed, 137 insertions(+), 75 deletions(-)
> > 
> 
> [...]
> 
> >  
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> > index 9037818e2a..6c086b9ef8 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
> > @@ -19249,10 +19249,9 @@ qemuConnectGetDomainCapabilities(virConnectPtr 
> > conn,
> >  char *ret = NULL;
> >  virQEMUDriverPtr driver = conn->privateData;
> >  virQEMUCapsPtr qemuCaps = NULL;
> > -int virttype = VIR_DOMAIN_VIRT_NONE;
> > -virDomainVirtType capsType;
> > +virArch arch;
> > +virDomainVirtType virttype;
> >  virDomainCapsPtr domCaps = NULL;
> > -int arch = virArchFromHost(); /* virArch */
> >  virQEMUDriverConfigPtr cfg = NULL;
> >  virCapsPtr caps = NULL;
> >  
> > @@ -19266,80 +19265,17 @@ qemuConnectGetDomainCapabilities(virConnectPtr 
> > conn,
> >  if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
> >  goto cleanup;
> >  
> > -if (virttype_str &&
> > -(virttype = virDomainVirtTypeFromString(virttype_str)) < 0) {
> > -virReportError(VIR_ERR_INVALID_ARG,
> > -   _("unknown virttype: %s"),
> > -   virttype_str);
> > +qemuCaps = virQEMUCapsCacheLookupDefault(driver->qemuCapsCache,
> > + emulatorbin,
> > + arch_str,
> > + virttype_str,
> > + machine,
> > + , , );
> > +if (!qemuCaps)
> >  goto cleanup;
> > -}
> >  
> > -if (arch_str && (arch = virArchFromString(arch_str)) == VIR_ARCH_NONE) 
> > {
> > -virReportError(VIR_ERR_INVALID_ARG,
> > -   _("unknown architecture: %s"),
> > -   arch_str);
> > -goto cleanup;
> > -}
> > -
> > -if (emulatorbin) {
> > -virArch arch_from_caps;
> > -
> > -if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache,
> > -emulatorbin)))
> > -goto cleanup;
> > -
> > -arch_from_caps = virQEMUCapsGetArch(qemuCaps);
> > -
> > -if (arch_from_caps != arch &&
> > -!((ARCH_IS_X86(arch) && ARCH_IS_X86(arch_from_caps)) ||
> > -  (ARCH_IS_PPC(arch) && ARCH_IS_PPC(arch_from_caps)) ||
> > -  (ARCH_IS_ARM(arch) && ARCH_IS_ARM(arch_from_caps)) ||
> > -  (ARCH_IS_S390(arch) && ARCH_IS_S390(arch_from_caps {
> > -virReportError(VIR_ERR_INVALID_ARG,
> > -   _("architecture from emulator '%s' doesn't "
> > - "match given architecture '%s'"),
> > -   virArchToString(arch_from_caps),
> > -   virArchToString(arch));
> > -goto cleanup;
> > -}
> 
> Are all these checks necessary? Can't you get away with just checking 
> arch_from_caps != arch?

Yes, because i686 != x86_64, while ARCH_IS_X86(i686) &&
ARCH_IS_X86(x86_64) is true.

Jirka

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

Re: [libvirt] [PATCH 04/22] virsh: Enhance documentation of cpu-models command

[Jiri Denemark]

On Wed, May 23, 2018 at 13:08:51 +0200, Kashyap Chamarthy wrote:
> On Wed, May 16, 2018 at 10:39:23AM +0200, Jiri Denemark wrote:
> > Signed-off-by: Jiri Denemark 
> > ---
> >  tools/virsh.pod | 8 +++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/virsh.pod b/tools/virsh.pod
> > index 5f72e11dec..5fc8201893 100644
> > --- a/tools/virsh.pod
> > +++ b/tools/virsh.pod
> > @@ -599,7 +599,13 @@ incompatibility will be printed out.
> >  
> >  =item B I
> >  
> > -Print the list of CPU models known for the specified architecture.
> > +Print the list of CPU models known by libvirt for the specified 
> > architecture.
> > +Whether a specific hypervisor is able to create a domain which uses any of
> > +the printed CPU models is a separate question which can be answered by
> > +looking the domain capabilities XML returned by B command.
> 
> s/looking the/looking at the/

Oops.

> 
> > +Moreover, for some architectures libvirt does not know any CPU models and
> > +the usable CPU models are only limited by the hypervisor. 
> 
> Wonder if it is worth adding a small example for the above.

An example for what exactly?

> > This command will
> > +print that all CPU models are accepted for these architectures.
> 
> s/CPU models are accepted/CPU models that are accepted/

It's certainly possible my wording was not perfect, but this change
would make it wrong.

Jirka

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

[libvirt] [PATCH] Fix memory leak issues in virshAllocpagesPagesizeCompleter.

[Roland Schulz]

Signed-off-by: Roland Schulz 
---
 tools/virsh-completer.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
index 1435d1d4c..c72f3bcad 100644
--- a/tools/virsh-completer.c
+++ b/tools/virsh-completer.c
@@ -576,6 +576,7 @@ virshAllocpagesPagesizeCompleter(vshControl *ctl,
 virshControlPtr priv = ctl->privData;
 unsigned int npages = 0;
 xmlNodePtr *pages = NULL;
+xmlDocPtr doc = NULL;
 double size = 0;
 size_t i = 0;
 const char *suffix = NULL;
@@ -595,7 +596,7 @@ virshAllocpagesPagesizeCompleter(vshControl *ctl,
 if (!(cap_xml = virConnectGetCapabilities(priv->conn)))
 goto error;
 
-if (!(virXMLParseStringCtxt(cap_xml, _("capabilities"), )))
+if ((doc = virXMLParseStringCtxt(cap_xml, _("capabilities"), )) == 
NULL)
 goto error;
 
 if (cellno && vshCommandOptStringQuiet(ctl, cmd, "cellno", ) > 0) {
@@ -631,12 +632,11 @@ virshAllocpagesPagesizeCompleter(vshControl *ctl,
 
  cleanup:
 xmlXPathFreeContext(ctxt);
-for (i = 0; i < npages; i++)
-VIR_FREE(pages[i]);
 VIR_FREE(pages);
-VIR_FREE(cap_xml);
+xmlFreeDoc(doc);
 VIR_FREE(path);
 VIR_FREE(pagesize);
+VIR_FREE(cap_xml);
 VIR_FREE(unit);
 
 return ret;
-- 
2.17.0

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

[libvirt] [PATCH v2] Edit test capabilities to contain different cell pagesizes.

[Roland Schulz]

Signed-off-by: Roland Schulz 
---
 src/test/test_driver.c | 18 +++---
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 467587b19..3fe0c2831 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -322,30 +322,34 @@ testBuildCapabilities(virConnectPtr conn)
 if (virCapabilitiesAddHostFeature(caps, "nonpae") < 0)
 goto error;
 
-if (VIR_ALLOC_N(caps->host.pagesSize, 2) < 0)
+if (VIR_ALLOC_N(caps->host.pagesSize, 4) < 0)
 goto error;
 
 caps->host.pagesSize[caps->host.nPagesSize++] = 4;
+caps->host.pagesSize[caps->host.nPagesSize++] = 8;
 caps->host.pagesSize[caps->host.nPagesSize++] = 2048;
+caps->host.pagesSize[caps->host.nPagesSize++] = 1024 * 1024;
 
 for (i = 0; i < privconn->numCells; i++) {
 virCapsHostNUMACellCPUPtr cpu_cells;
 virCapsHostNUMACellPageInfoPtr pages;
-size_t nPages;
+size_t nPages = caps->host.nPagesSize - 1;
 
 if (VIR_ALLOC_N(cpu_cells, privconn->cells[i].numCpus) < 0 ||
-VIR_ALLOC_N(pages, caps->host.nPagesSize) < 0) {
+VIR_ALLOC_N(pages, caps->host.nPagesSize - 1) < 0) {
 VIR_FREE(cpu_cells);
 goto error;
 }
-
-nPages = caps->host.nPagesSize;
+if (i == 1)
+pages[0].size = caps->host.pagesSize[0];
+else
+pages[0].size = caps->host.pagesSize[1];
 
 memcpy(cpu_cells, privconn->cells[i].cpus,
sizeof(*cpu_cells) * privconn->cells[i].numCpus);
 
-for (j = 0; j < nPages; j++)
-pages[j].size = caps->host.pagesSize[j];
+for (j = 1; j < nPages; j++)
+pages[j].size = caps->host.pagesSize[j+1];
 
 pages[0].avail = privconn->cells[i].mem / pages[0].size;
 
-- 
2.17.0

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

Re: [libvirt] [PATCH 02/22] virsh: Extract common code from cmdCPU{Compare, Baseline}

[Jiri Denemark]

On Tue, May 22, 2018 at 17:33:14 -0400, Collin Walling wrote:
> On 05/16/2018 04:39 AM, Jiri Denemark wrote:
> > Both cpu-compare and cpu-baseline commands accept more that just CPU
> > definition XML(s). For users' convenience they are able to extract the
> > CPU definition(s) even from domain XML or capabilities XML. The main
> > differences between the two commands is in the number of CPU definitions
> > they expect: cpu-compare wants only one CPU definition while
> > cpu-baseline expects one or more CPUs.
> > 
> > The extracted code forms a new vshExtractCPUDefXML function.
> > 
> > Signed-off-by: Jiri Denemark 
> > ---
> >  tools/virsh-host.c | 160 +
> >  1 file changed, 75 insertions(+), 85 deletions(-)
> > 
> > diff --git a/tools/virsh-host.c b/tools/virsh-host.c
> > index 6d6e3cfc85..51497db385 100644
> > --- a/tools/virsh-host.c
> > +++ b/tools/virsh-host.c
> > @@ -1106,6 +1106,72 @@ cmdURI(vshControl *ctl, const vshCmd *cmd 
> > ATTRIBUTE_UNUSED)
> >  return true;
> >  }
> >  
> > +
> > +/* Extracts the CPU definition XML strings from a file which may contain 
> > either
> > + *  - just the CPU definitions,
> > + *  - domain XMLs, or
> > + *  - capabilities XMLs.
> > + *
> > + * Returns NULL terminated string list.
> > + */
> > +static char **
> > +vshExtractCPUDefXMLs(vshControl *ctl,
> > + const char *xmlFile)
> > +{
> > +char **cpus = NULL;
> > +char *buffer = NULL;
> > +char *xmlStr = NULL;
> > +xmlDocPtr xml = NULL;
> > +xmlXPathContextPtr ctxt = NULL;
> > +xmlNodePtr *nodes = NULL;
> > +size_t i;
> > +int n;
> > +
> > +if (virFileReadAll(xmlFile, VSH_MAX_XML_FILE, ) < 0)
> > +goto error;
> > +
> > +if (virAsprintf(, "%s", buffer) < 0)
> > +goto error;
> > +
> 
> Why wrap the xml in the  tags?

Because you can only have one root element in an XML document. Thus to
be able to parse a file with several root elements, we need to
encapsulate the content into a container.

> 
> > +if (!(xml = virXMLParseStringCtxt(xmlStr, xmlFile, )))
> > +goto error;
> > +
> > +n = virXPathNodeSet("/container/cpu|"
> > +"/container/domain/cpu|"
> > +"/container/capabilities/host/cpu",
> > +ctxt, );
> > +if (n < 0)
> > +goto error;
> > +
> > +if (n == 0) {
> > +vshError(ctl, _("File '%s' does not contain any  element or "
> > +"valid domain or capabilities XML"), xmlFile);
> > +goto error;
> > +}
> > +
> > +cpus = vshCalloc(ctl, n + 1, sizeof(const char *));
> > +
> > +for (i = 0; i < n; i++) {
> > +if (!(cpus[i] = virXMLNodeToString(xml, nodes[i]))) {
> > +vshSaveLibvirtError();
> > +goto error;
> > +}
> > +}
> > +
> > + cleanup:
> > +VIR_FREE(buffer);
> > +VIR_FREE(xmlStr);
> > +xmlFreeDoc(xml);
> > +xmlXPathFreeContext(ctxt);
> > +VIR_FREE(nodes);
> > +return cpus;
> > +
> > + error:
> > +virStringListFree(cpus);
> > +goto cleanup;
> > +}
> > +
> > +
> >  /*
> >   * "cpu-compare" command
> >   */
> > @@ -1133,13 +1199,9 @@ cmdCPUCompare(vshControl *ctl, const vshCmd *cmd)
> >  {
> >  const char *from = NULL;
> >  bool ret = false;
> > -char *buffer;
> >  int result;
> > -char *snippet = NULL;
> > +char **cpus = NULL;
> >  unsigned int flags = 0;
> > -xmlDocPtr xml = NULL;
> > -xmlXPathContextPtr ctxt = NULL;
> > -xmlNodePtr node;
> >  virshControlPtr priv = ctl->privData;
> >  
> >  if (vshCommandOptBool(cmd, "error"))
> > @@ -1148,27 +1210,10 @@ cmdCPUCompare(vshControl *ctl, const vshCmd *cmd)
> >  if (vshCommandOptStringReq(ctl, cmd, "file", ) < 0)
> >  return false;
> >  
> > -if (virFileReadAll(from, VSH_MAX_XML_FILE, ) < 0)
> > +if (!(cpus = vshExtractCPUDefXMLs(ctl, from)))
> >  return false;
> >  
> > -/* try to extract the CPU element from as it would appear in a domain 
> > XML*/
> > -if (!(xml = virXMLParseStringCtxt(buffer, from, )))
> > -goto cleanup;
> > -
> > -if ((node = virXPathNode("/cpu|"
> > - "/domain/cpu|"
> > -  "/capabilities/host/cpu", ctxt))) {
> > -if (!(snippet = virXMLNodeToString(xml, node))) {
> > -vshSaveLibvirtError();
> > -goto cleanup;
> > -}
> > -} else {
> > -vshError(ctl, _("File '%s' does not contain a  element or is 
> > not "
> > -"a valid domain or capabilities XML"), from);
> > -goto cleanup;
> > -}
> > -
> > -result = virConnectCompareCPU(priv->conn, snippet, flags);
> > +result = virConnectCompareCPU(priv->conn, cpus[0], flags);
> 
> I wonder if it's worth commenting here or adding to the cpu compare docs that 
> comparison only
> compares the host CPU 

Re: [libvirt] [Qemu-devel] [PATCH v7 1/3] qmp: adding 'wakeup-suspend-support' in query-target

[Eduardo Habkost]

On Wed, May 23, 2018 at 11:17:55AM +0200, Markus Armbruster wrote:
> Eduardo Habkost  writes:
> > On Mon, May 21, 2018 at 04:46:36PM -0300, Daniel Henrique Barboza wrote:
[...]
> >> Since no objection was made back then, this logic was put into query-target
> >> starting
> >> in v2. Still, I don't have any favorites though: query-target looks ok,
> >> query-machine
> >> looks ok and a new API looks ok too. It's all about what makes (more) sense
> >> in the
> >> management level, I think.
> >
> > I understand the original objection from Eric: having to add a
> > new command for every runtime flag we want to expose to the user
> > looks wrong to me.
> 
> Agreed.
> 
> > However, extending query-machines and query-target looks wrong
> > too, however.  query-target looks wrong because this not a
> > property of the target.  query-machines is wrong because this is
> > not a static property of the machine-type, but of the running
> > machine instance.
> 
> Of the two, query-machines looks less wrong.
> 
> Arguably, -no-acpi should not exist.  It's an ad hoc flag that sneakily
> splits a few machine types into two variants, with and without ACPI.
> It's silently ignored for other machine types, even APCI-capable ones.
> 
> If the machine type variants with and without ACPI were separate types,
> wakeup-suspend-support would be a static property of the machine type.
> 
> However, "separate types" probably doesn't scale: I'm afraid we'd end up
> with an undesirable number of machine types.  Avoiding that is exactly
> why we have machine types with configurable options.  I suspect that's
> how ACPI should be configured (if at all).
> 
> So, should we make -no-acpi sugar for a machine type parameter?  And
> then deprecate -no-acpi for good measure?

I think we should.


> 
> > Can we have a new query command that could be an obvious
> > container for simple machine capabilities that are not static?  A
> > name like "query-machine" would be generic enough for that, I
> > guess.
> 
> Having command names differ only in a single letter is awkward, but
> let's focus on things other than naming now, and use
> query-current-machine like a working title.
> 
> query-machines is wrong because wakeup-suspend-support isn't static for
> some machine types.
> 
> query-current-machine is also kind of wrong because
> wakeup-suspend-support *is* static for most machine types.
> 

The most appropriate solution depends a lot on how/when
management software needs to query this.

If they only need to query it at runtime for a running VM,
there's no reason for us to go of our way and add complexity just
to make it look like static data in query-machines.

On the other hand, if they really need to query it before
configuring/starting a VM, it won't be useful at all to make it
available only at runtime.

Daniel, when/how exactly software would need to query the new
flag?


> Worse, a machine type property that is static for all machine types now
> could conceivably become dynamic when we add a machine type
> configuration knob.
> 

This isn't the first time a machine capability that seems static
actually depends on other configuration arguments.  We will
probably need to address this eventually.


> Would a way to tie the property to the configuration knob help?
> Something like wakeup-suspend-support taking values true (supported),
> false (not supported), and "acpi" (supported if machine type
> configuration knob "acpi" is switched on).
> 

I would prefer a more generic mechanism.  Maybe make
'query-machines' accept a 'machine-options' argument?

-- 
Eduardo

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

Re: [libvirt] [PATCH 00/12] Add support for TPM emulator

[John Ferlan]

On 05/22/2018 04:44 PM, Stefan Berger wrote:
> This series of patches adds support for the TPM emulator backend that
> is available in QEMU and based on swtpm + libtpms. It allows to attach a
> TPM 1.2 or 2 to a QEMU VM. sVirt labels are used for labeling the swtpm
> process, its Unix socket, and log file with the same label that the
> QEMU process gets. Besides that swtpm is added to the emulator cgroup to
> restrict its CPU usage.
> 
> The device XML can be changed from a TPM 1.2 to a TPM 2 and back to a
> TPM 1.2. The device state is not removed during those changes but only
> when the domain is undefined.
> 
> The swtpm needs persistent storage to store its state. For that I am
> using the uuid of the VM as part of the path since the name of the VM
> can be changed. Logfiles, PID files, and socket names are based on the
> name of the VM, though.
> 
>   Stefan
> 
> v5->v6:
>   - Addressed John Ferlan's comments
>   - rebased on latest tip
>   - Added patch 12.
> 
> v4->v5:
>   - Addressed John Ferlan's, Boris Fiuczysnki's and Marc Hartmayer's comments
>   - rebased on latest tip
> 
> v3->v4:
>   - Addressed John Ferlan's comments
>   - Fixed bugs I found while testing
>   - rebased on latest tip
> 
> Stefan Berger (12):
>   conf: Add support for external swtpm TPM emulator to domain XML
>   qemu: Extend QEMU capabilities with 'tpm-emulator'
>   util: Implement virFileChownFiles()
>   security: Add DAC and SELinux security for tpm-emulator
>   qemu: Extend qemu_conf with tpm-emulator support
>   qemu: Extend QEMU with external TPM support
>   qemu: Add support for external swtpm TPM emulator
>   tests: Add test cases for external swtpm TPM emulator
>   security: Label the external swtpm with SELinux labels
>   conf: Add support for choosing emulation of a TPM 2
>   qemu: Add swtpm to emulator cgroup
>   news: Update news with new TPM emulator feature
> 
>  docs/formatdomain.html.in  |  43 +
>  docs/news.xml  |   9 +
>  docs/schemas/domaincommon.rng  |  17 +
>  libvirt.spec.in|   2 +
>  src/conf/domain_audit.c|   2 +
>  src/conf/domain_conf.c |  53 +-
>  src/conf/domain_conf.h |  12 +
>  src/libvirt_private.syms   |   3 +
>  src/qemu/Makefile.inc.am   |  10 +
>  src/qemu/libvirtd_qemu.aug |   5 +
>  src/qemu/qemu.conf |   8 +
>  src/qemu/qemu_capabilities.c   |   5 +
>  src/qemu/qemu_capabilities.h   |   1 +
>  src/qemu/qemu_cgroup.c |  36 +
>  src/qemu/qemu_cgroup.h |   2 +
>  src/qemu/qemu_command.c|  34 +-
>  src/qemu/qemu_conf.c   |  43 +
>  src/qemu/qemu_conf.h   |   6 +
>  src/qemu/qemu_domain.c |   3 +
>  src/qemu/qemu_extdevice.c  | 180 
>  src/qemu/qemu_extdevice.h  |  59 ++
>  src/qemu/qemu_process.c|  16 +
>  src/qemu/qemu_security.c   |  69 ++
>  src/qemu/qemu_security.h   |  11 +
>  src/qemu/qemu_tpm.c| 946 
> +
>  src/qemu/qemu_tpm.h|  56 ++
>  src/qemu/test_libvirtd_qemu.aug.in |   2 +
>  src/security/security_dac.c|   7 +
>  src/security/security_driver.h |   7 +
>  src/security/security_manager.c|  36 +
>  src/security/security_manager.h|   6 +
>  src/security/security_selinux.c| 172 
>  src/security/security_stack.c  |  40 +
>  src/util/virfile.c |  55 ++
>  src/util/virfile.h |   3 +
>  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 +
>  .../tpm-emulator-tpm2.x86_64-latest.args   |  33 +
>  tests/qemuxml2argvdata/tpm-emulator-tpm2.xml   |  30 +
>  .../tpm-emulator.x86_64-latest.args|  33 +
>  tests/qemuxml2argvdata/tpm-emulator.xml|  30 +
>  tests/qemuxml2argvtest.c   |  16 +-
>  tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml |  34 +
>  tests/qemuxml2xmloutdata/tpm-emulator.xml  |  34 +
>  tests/qemuxml2xmltest.c|   1 +
>  48 files changed, 2165 insertions(+), 10 deletions(-)
>  create mode 100644 

Re: [libvirt] [PATCH 05/22] Improve documentation of virConnectGetCPUModelNames

[Kashyap Chamarthy]

On Wed, May 16, 2018 at 10:39:24AM +0200, Jiri Denemark wrote:
> Signed-off-by: Jiri Denemark 
> ---
>  src/libvirt-host.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libvirt-host.c b/src/libvirt-host.c
> index 76087badd8..ed689b9ec2 100644
> --- a/src/libvirt-host.c
> +++ b/src/libvirt-host.c
> @@ -1003,7 +1003,13 @@ virConnectCompareCPU(virConnectPtr conn,
>   *  NULL if only the list length is needed.
>   * @flags: extra flags; not used yet, so callers should always pass 0.
>   *
> - * Get the list of supported CPU models for a specific architecture.
> + * Get the list of CPU models supported by libvirt for a specific 
> architecture.
> + *
> + * The returned list limits CPU models usable with libvirt (empty list means,
> + * there's no limit imposed by libvirt) and it does not reflect capabalities 
> of
> + * any hypervisor. 

s/any hypervisor/any particular hypervisor/

> See the XML returned by virConnectGetDomainCapabilities()
> + * for a list of CPU models supported by libvirt for domains created on a
> + * specific hypervisor.
>   *
>   * Returns -1 on error, number of elements in @models on success (0 means
>   * libvirt accepts any CPU model).

Reviewed-by: Kashyap Chamarthy 

-- 
/kashyap

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

Re: [libvirt] [PATCH 03/22] virsh: Enhance documentation of cpu-compare command

[Kashyap Chamarthy]

On Wed, May 16, 2018 at 10:39:22AM +0200, Jiri Denemark wrote:
> Signed-off-by: Jiri Denemark 
> ---
>  tools/virsh.pod | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index 929958a953..5f72e11dec 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -589,7 +589,9 @@ Compare CPU definition from XML  with host CPU. The 
> XML  may
>  contain either host or guest CPU definition. The host CPU definition is the
>   element and its contents as printed by B command. The
>  guest CPU definition is the  element and its contents from domain XML
> -definition. For more information on guest CPU definition see:
> +definition. In addition to the  element itself, this command accepts
> +full domain or capabilities XML containing the  element. For more
> +information on guest CPU definition see:
>  L. If I<--error> is
>  specified, the command will return an error when the given CPU is
>  incompatible with host CPU and a message providing more details about the

Reviewed-by: Kashyap Chamarthy 

-- 
/kashyap

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

Re: [libvirt] [PATCH 04/22] virsh: Enhance documentation of cpu-models command

[Kashyap Chamarthy]

On Wed, May 16, 2018 at 10:39:23AM +0200, Jiri Denemark wrote:
> Signed-off-by: Jiri Denemark 
> ---
>  tools/virsh.pod | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index 5f72e11dec..5fc8201893 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -599,7 +599,13 @@ incompatibility will be printed out.
>  
>  =item B I
>  
> -Print the list of CPU models known for the specified architecture.
> +Print the list of CPU models known by libvirt for the specified architecture.
> +Whether a specific hypervisor is able to create a domain which uses any of
> +the printed CPU models is a separate question which can be answered by
> +looking the domain capabilities XML returned by B command.

s/looking the/looking at the/

> +Moreover, for some architectures libvirt does not know any CPU models and
> +the usable CPU models are only limited by the hypervisor. 

Wonder if it is worth adding a small example for the above.

> This command will
> +print that all CPU models are accepted for these architectures.

s/CPU models are accepted/CPU models that are accepted/

>  
>  =item B [I<--shell>] [I<--xml>] [I...]
>  

With the nits addresed:

Reviewed-by: Kashyap Chamarthy 

-- 
/kashyap

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

Re: [libvirt] [PATCH 22/22] news: Mention new CPU related APIs

[Kashyap Chamarthy]

On Wed, May 16, 2018 at 10:39:41AM +0200, Jiri Denemark wrote:
> Signed-off-by: Jiri Denemark 
> ---
>  docs/news.xml | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/docs/news.xml b/docs/news.xml
> index 7d40e85b9a..bd7885e91a 100644
> --- a/docs/news.xml
> +++ b/docs/news.xml
> @@ -54,6 +54,15 @@
>a QEMU virtual machine.
>  
>
> +  
> +
> +  Introduce new virConnectBaselineHypervisorCPU and 
> virConnectBaselineHypervisorCPU APIs

Duplicate:

s/virConnectBaselineHypervisorCPU/virConnectCompareHypervisorCPU/

> +
> +
> +  Unlike the old virConnectBaselineCPU and virConnectBaselineCPU 
> APIs,

Here too:

s/virConnectBaselineCPU/virConnectCompareCPU/

> +  both new APIs consider capabilities of a specific hypervisor.

If it in scope of the 'news.xml' file, a one-sentence description of 
each of the two APIs would be useful.  It's just a nice to have, so feel
free to disregard this.

FWIW, I will aim to try out these two new APIs this week / next week.

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

-- 
/kashyap

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

Re: [libvirt] [Qemu-devel] [PATCH v7 1/3] qmp: adding 'wakeup-suspend-support' in query-target

[Markus Armbruster]

Eduardo Habkost  writes:

> On Mon, May 21, 2018 at 04:46:36PM -0300, Daniel Henrique Barboza wrote:
>> 
>> 
>> On 05/21/2018 03:14 PM, Eduardo Habkost wrote:
>> > > Issue#2: the flag isn't a property of the target.  Due to -no-acpi, it's
>> > > not even a property of the machine type.  If it was, query-machines
>> > > would be the natural owner of the flag.
>> > > 
>> > > Perhaps query-machines is still the proper owner.  The value of
>> > > wakeup-suspend-support would have to depend on -no-acpi for the machine
>> > > types that honor it.  Not ideal; I'd prefer MachineInfo to be static.
>> > > Tolerable?  I guess that's also a libvirt question.
>> > It depends when libvirt is going to query it.  Is it OK to only
>> > query it after the VM is already up and running?  If it is, then
>> > we can simply expose it as a read-only property of the machine
>> > object.
>> > 
>> > Or, if we don't want to rely on qom-get as a stable API, we can
>> > add a new query command (query-machine? query-power-management?)
>> > 
>> In the first version this logic was included in a new query command called
>> "query-wakeup-from-suspend-support":
>> 
>> https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg00889.html
>> 
>> In that review it was suggested that this logic could be a flag in either
>> query-target
>> or query-machines API. Before sending the v2 I sent the following comment:
>> 
>> "After investigating, I think that it's simpler to hook the wakeup support
>> info into
>> TargetInfo than MachineInfo, given that the detection I'm using for this new
>> property
>> is based on the current runtime state. Hooking into MachineInfo would
>> require to
>> change the MachineClass to add a new property, then setting it up for the
>> machines
>> that have the wakeup support (only x86 so far). Definitely doable, but if we
>> don't
>> have any favorites between MachineInfo and TargetInfo I'd rather pick the
>> simpler
>> route.
>> 
>> So, if no one objects, I'll rework this series by putting the logic inside
>> query-target
>> instead of a new API."
>
> Apologies for not noticing this series months ago.  :(

Seconded.  Daniel, this (minor) mess is absolutely not your fault.

>> Since no objection was made back then, this logic was put into query-target
>> starting
>> in v2. Still, I don't have any favorites though: query-target looks ok,
>> query-machine
>> looks ok and a new API looks ok too. It's all about what makes (more) sense
>> in the
>> management level, I think.
>
> I understand the original objection from Eric: having to add a
> new command for every runtime flag we want to expose to the user
> looks wrong to me.

Agreed.

> However, extending query-machines and query-target looks wrong
> too, however.  query-target looks wrong because this not a
> property of the target.  query-machines is wrong because this is
> not a static property of the machine-type, but of the running
> machine instance.

Of the two, query-machines looks less wrong.

Arguably, -no-acpi should not exist.  It's an ad hoc flag that sneakily
splits a few machine types into two variants, with and without ACPI.
It's silently ignored for other machine types, even APCI-capable ones.

If the machine type variants with and without ACPI were separate types,
wakeup-suspend-support would be a static property of the machine type.

However, "separate types" probably doesn't scale: I'm afraid we'd end up
with an undesirable number of machine types.  Avoiding that is exactly
why we have machine types with configurable options.  I suspect that's
how ACPI should be configured (if at all).

So, should we make -no-acpi sugar for a machine type parameter?  And
then deprecate -no-acpi for good measure?

> Can we have a new query command that could be an obvious
> container for simple machine capabilities that are not static?  A
> name like "query-machine" would be generic enough for that, I
> guess.

Having command names differ only in a single letter is awkward, but
let's focus on things other than naming now, and use
query-current-machine like a working title.

query-machines is wrong because wakeup-suspend-support isn't static for
some machine types.

query-current-machine is also kind of wrong because
wakeup-suspend-support *is* static for most machine types.

Worse, a machine type property that is static for all machine types now
could conceivably become dynamic when we add a machine type
configuration knob.

Would a way to tie the property to the configuration knob help?
Something like wakeup-suspend-support taking values true (supported),
false (not supported), and "acpi" (supported if machine type
configuration knob "acpi" is switched on).

> Markus, Eric, what do you think?

Haven't made up my mind, yet :)

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

Re: [libvirt] [PATCH 11/15] qemu: domain: Add private data for NBD migration storage source definition

[Peter Krempa]

On Tue, May 22, 2018 at 20:26:56 -0400, John Ferlan wrote:
> 
> 
> On 05/18/2018 07:29 AM, Peter Krempa wrote:
> > Allow saving various aspects necessary to do NBD migration via blockdev
> > by storing a 'virStorageSource' in the disk private data meant to store
> > the NBD target of migration. Along with this add code to parse and
> > format it into the status XML.
> > 
> > Signed-off-by: Peter Krempa 
> > ---
> >  src/qemu/qemu_domain.c | 162 
> > ++---
> >  src/qemu/qemu_domain.h |   1 +
> >  2 files changed, 156 insertions(+), 7 deletions(-)
> > 
> > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> > index 94a9c5d1bc..632c025bef 100644
> > --- a/src/qemu/qemu_domain.c
> > +++ b/src/qemu/qemu_domain.c
> 
> [...]
> 
> 
> > 
> > @@ -2092,6 +2155,7 @@ qemuDomainObjPrivateXMLFormatJob(virBufferPtr buf,
> >  virBuffer attrBuf = VIR_BUFFER_INITIALIZER;
> >  virBuffer childBuf = VIR_BUFFER_INITIALIZER;
> >  qemuDomainJob job = priv->job.active;
> > +int ret = -1;
> > 
> >  if (!qemuDomainTrackJob(job))
> >  job = QEMU_JOB_NONE;
> > @@ -2115,13 +2179,23 @@ qemuDomainObjPrivateXMLFormatJob(virBufferPtr buf,
> >  if (priv->job.asyncJob != QEMU_ASYNC_JOB_NONE)
> >  virBufferAsprintf(, " flags='0x%lx'", priv->job.apiFlags);
> > 
> > -if (priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_OUT)
> > -qemuDomainObjPrivateXMLFormatNBDMigration(, vm);
> > +if (priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_OUT &&
> > +qemuDomainObjPrivateXMLFormatNBDMigration(, vm) < 0)
> > +goto cleanup;
> > 
> >  if (priv->job.migParams)
> >  qemuMigrationParamsFormat(, priv->job.migParams);
> > 
> > -return virXMLFormatElement(buf, "job", , );
> > +if (virXMLFormatElement(buf, "job", , ) < 0)
> > +goto cleanup;
> > +
> > +ret = 0;
> > +
> > + cleanup:
> > +virBufferFreeAndReset();
> > +virBufferFreeAndReset();
> 
> So I assume the lack of FreeAndReset was existing prior to this patch
> since d8be0f4bc?

Yes. I assumed that virXMLFormatElement clears them on error, but it
apparently does not, so adding them here actually fixes a possible bug.

This chagne would be necessary even with the above assumption being
valid as this patch adds a possible error code path.


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

Re: [libvirt] [PATCH v4 0/4] qemu: Add support for -device hda-output (disable line-in)

[Erik Skultety]

On Tue, May 22, 2018 at 01:12:31PM +0200, Filip Alac wrote:
> Add support for hda 'output' codec for ich6 and ich9 sound devices,
> which allow us to disable line-in for guest.
> 'hda-output' codec is available since 0.14 version of qemu.
>
> Fixes:
>  https://bugzilla.redhat.com/show_bug.cgi?id=1126641
>
> v1:
>  https://www.redhat.com/archives/libvir-list/2018-May/msg01311.html
>
> v2:
>  https://www.redhat.com/archives/libvir-list/2018-May/msg01450.html
>  - Split patch into series of patches
>  - Add description of feature in docs/news.xml
>  - Improve the documentation in docs/formatdomain.html.in
>  - Fixes coding standard errors/inconsistencies
>
> v3:
>  https://www.redhat.com/archives/libvir-list/2018-May/msg01506.html
>  - Moved all tests to third patch
>  - Made possible to compile every patch by alone except fourth which
>  depends on the first and on the second patch.
>
> v4:
>  - Moved some qemu_command and qemu_capabilities hunks from the first
>  patch into second patch.

Reviewed-by: Erik Skultety 

I tweaked the commit messages a tiny bit as well as the news feature
description and pushed the series.

Thanks,
Erik

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

[libvirt] [ [PATCH v3 8/8] virsh: Add event name completion to 'nodedev-event' command

[Lin Ma]

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

diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
index 1fab758e12..162b7a0e1c 100644
--- a/tools/virsh-completer.c
+++ b/tools/virsh-completer.c
@@ -26,6 +26,7 @@
 #include "virsh-domain.h"
 #include "virsh.h"
 #include "virsh-pool.h"
+#include "virsh-nodedev.h"
 #include "virsh-util.h"
 #include "virsh-secret.h"
 #include "internal.h"
@@ -737,3 +738,33 @@ virshPoolEventNameCompleter(vshControl *ctl,
 virStringListFree(ret);
 return NULL;
 }
+
+
+char **
+virshNodedevEventNameCompleter(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_NODE_DEVICE_EVENT_ID_LAST) < 0)
+goto error;
+
+for (i = 0; i < VIR_NODE_DEVICE_EVENT_ID_LAST; i++) {
+if (VIR_STRDUP(ret[i], vshNodedevEventCallbacks[i].name) < 0)
+goto error;
+}
+
+return ret;
+
+ error:
+virStringListFree(ret);
+return NULL;
+}
diff --git a/tools/virsh-completer.h b/tools/virsh-completer.h
index 76fa31cb47..1190130ec0 100644
--- a/tools/virsh-completer.h
+++ b/tools/virsh-completer.h
@@ -89,4 +89,8 @@ char ** virshPoolEventNameCompleter(vshControl *ctl,
 const vshCmd *cmd,
 unsigned int flags);
 
+char ** virshNodedevEventNameCompleter(vshControl *ctl,
+   const vshCmd *cmd,
+   unsigned int flags);
+
 #endif
diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c
index 66cf46c22f..f6bdeadd62 100644
--- a/tools/virsh-nodedev.c
+++ b/tools/virsh-nodedev.c
@@ -870,6 +870,7 @@ static const vshCmdOptDef opts_node_device_event[] = {
 },
 {.name = "event",
  .type = VSH_OT_STRING,
+ .completer = virshNodedevEventNameCompleter,
  .help = N_("which event type to wait for")
 },
 {.name = "loop",
-- 
2.16.2

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

[libvirt] [ [PATCH v3 5/8] virsh-pool: Rename vshEventCallback to vshPoolEventCallback

[Lin Ma]

The next patch will use it in virsh-completer.c for returning the name
list of pool events.

Signed-off-by: Lin Ma 
---
 tools/virsh-pool.c | 18 ++
 tools/virsh-pool.h |  8 
 2 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
index 36bf8d9faa..f0544d2e2d 100644
--- a/tools/virsh-pool.c
+++ b/tools/virsh-pool.c
@@ -1999,18 +1999,12 @@ virshPoolEventToString(int event)
 return str ? _(str) : _("unknown");
 }
 
-struct vshEventCallback {
-const char *name;
-virConnectStoragePoolEventGenericCallback cb;
-};
-typedef struct vshEventCallback vshEventCallback;
-
 struct virshPoolEventData {
 vshControl *ctl;
 bool loop;
 bool timestamp;
 int count;
-vshEventCallback *cb;
+vshPoolEventCallback *cb;
 };
 typedef struct virshPoolEventData virshPoolEventData;
 
@@ -2079,12 +2073,12 @@ vshEventGenericPrint(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 vshEventDone(data->ctl);
 }
 
-static vshEventCallback vshEventCallbacks[] = {
+vshPoolEventCallback vshPoolEventCallbacks[] = {
 { "lifecycle",
   VIR_STORAGE_POOL_EVENT_CALLBACK(vshEventLifecyclePrint), },
 { "refresh", vshEventGenericPrint, }
 };
-verify(VIR_STORAGE_POOL_EVENT_ID_LAST == ARRAY_CARDINALITY(vshEventCallbacks));
+verify(VIR_STORAGE_POOL_EVENT_ID_LAST == 
ARRAY_CARDINALITY(vshPoolEventCallbacks));
 
 
 static const vshCmdInfo info_pool_event[] = {
@@ -2141,7 +2135,7 @@ cmdPoolEvent(vshControl *ctl, const vshCmd *cmd)
 size_t i;
 
 for (i = 0; i < VIR_STORAGE_POOL_EVENT_ID_LAST; i++)
-vshPrint(ctl, "%s\n", vshEventCallbacks[i].name);
+vshPrint(ctl, "%s\n", vshPoolEventCallbacks[i].name);
 return true;
 }
 
@@ -2153,7 +2147,7 @@ cmdPoolEvent(vshControl *ctl, const vshCmd *cmd)
 }
 
 for (event = 0; event < VIR_STORAGE_POOL_EVENT_ID_LAST; event++)
-if (STREQ(eventName, vshEventCallbacks[event].name))
+if (STREQ(eventName, vshPoolEventCallbacks[event].name))
 break;
 if (event == VIR_STORAGE_POOL_EVENT_ID_LAST) {
 vshError(ctl, _("unknown event type %s"), eventName);
@@ -2164,7 +2158,7 @@ cmdPoolEvent(vshControl *ctl, const vshCmd *cmd)
 data.loop = vshCommandOptBool(cmd, "loop");
 data.timestamp = vshCommandOptBool(cmd, "timestamp");
 data.count = 0;
-data.cb = [event];
+data.cb = [event];
 if (vshCommandOptTimeoutToMs(ctl, cmd, ) < 0)
 return false;
 
diff --git a/tools/virsh-pool.h b/tools/virsh-pool.h
index 5f2671ae63..1148d7cebb 100644
--- a/tools/virsh-pool.h
+++ b/tools/virsh-pool.h
@@ -37,6 +37,14 @@ virshCommandOptPoolBy(vshControl *ctl, const vshCmd *cmd, 
const char *optname,
 virshCommandOptPoolBy(_ctl, _cmd, _optname, _name, \
   VIRSH_BYUUID | VIRSH_BYNAME)
 
+struct vshPoolEventCallback {
+const char *name;
+virConnectStoragePoolEventGenericCallback cb;
+};
+typedef struct vshPoolEventCallback vshPoolEventCallback;
+
+extern vshPoolEventCallback vshPoolEventCallbacks[];
+
 extern const vshCmdDef storagePoolCmds[];
 
 #endif /* VIRSH_POOL_H */
-- 
2.16.2

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

[libvirt] [ [PATCH v3 7/8] virsh-nodedev: Rename vshEventCallback to vshNodedevEventCallback

[Lin Ma]

The next patch will use it in virsh-completer.c for returning the name
list of nodedev events.

Signed-off-by: Lin Ma 
---
 tools/virsh-nodedev.c | 18 ++
 tools/virsh-nodedev.h |  8 
 2 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c
index d25fe0e09b..66cf46c22f 100644
--- a/tools/virsh-nodedev.c
+++ b/tools/virsh-nodedev.c
@@ -773,18 +773,12 @@ virshNodeDeviceEventToString(int event)
 return str ? _(str) : _("unknown");
 }
 
-struct vshEventCallback {
-const char *name;
-virConnectNodeDeviceEventGenericCallback cb;
-};
-typedef struct vshEventCallback vshEventCallback;
-
 struct virshNodeDeviceEventData {
 vshControl *ctl;
 bool loop;
 bool timestamp;
 int count;
-vshEventCallback *cb;
+vshNodedevEventCallback *cb;
 };
 typedef struct virshNodeDeviceEventData virshNodeDeviceEventData;
 
@@ -850,12 +844,12 @@ vshEventGenericPrint(virConnectPtr conn ATTRIBUTE_UNUSED,
 vshEventDone(data->ctl);
 }
 
-static vshEventCallback vshEventCallbacks[] = {
+vshNodedevEventCallback vshNodedevEventCallbacks[] = {
 { "lifecycle",
   VIR_NODE_DEVICE_EVENT_CALLBACK(vshEventLifecyclePrint), },
 { "update", vshEventGenericPrint, }
 };
-verify(VIR_NODE_DEVICE_EVENT_ID_LAST == ARRAY_CARDINALITY(vshEventCallbacks));
+verify(VIR_NODE_DEVICE_EVENT_ID_LAST == 
ARRAY_CARDINALITY(vshNodedevEventCallbacks));
 
 
 static const vshCmdInfo info_node_device_event[] = {
@@ -914,7 +908,7 @@ cmdNodeDeviceEvent(vshControl *ctl, const vshCmd *cmd)
 size_t i;
 
 for (i = 0; i < VIR_NODE_DEVICE_EVENT_ID_LAST; i++)
-vshPrint(ctl, "%s\n", vshEventCallbacks[i].name);
+vshPrint(ctl, "%s\n", vshNodedevEventCallbacks[i].name);
 return true;
 }
 
@@ -926,7 +920,7 @@ cmdNodeDeviceEvent(vshControl *ctl, const vshCmd *cmd)
 }
 
 for (event = 0; event < VIR_NODE_DEVICE_EVENT_ID_LAST; event++)
-if (STREQ(eventName, vshEventCallbacks[event].name))
+if (STREQ(eventName, vshNodedevEventCallbacks[event].name))
 break;
 if (event == VIR_NODE_DEVICE_EVENT_ID_LAST) {
 vshError(ctl, _("unknown event type %s"), eventName);
@@ -937,7 +931,7 @@ cmdNodeDeviceEvent(vshControl *ctl, const vshCmd *cmd)
 data.loop = vshCommandOptBool(cmd, "loop");
 data.timestamp = vshCommandOptBool(cmd, "timestamp");
 data.count = 0;
-data.cb = [event];
+data.cb = [event];
 if (vshCommandOptTimeoutToMs(ctl, cmd, ) < 0)
 return false;
 if (vshCommandOptStringReq(ctl, cmd, "device", _value) < 0)
diff --git a/tools/virsh-nodedev.h b/tools/virsh-nodedev.h
index c64f7df83c..931beb982d 100644
--- a/tools/virsh-nodedev.h
+++ b/tools/virsh-nodedev.h
@@ -28,6 +28,14 @@
 
 # include "virsh.h"
 
+struct vshNodedevEventCallback {
+const char *name;
+virConnectNodeDeviceEventGenericCallback cb;
+};
+typedef struct vshNodedevEventCallback vshNodedevEventCallback;
+
+extern vshNodedevEventCallback vshNodedevEventCallbacks[];
+
 extern const vshCmdDef nodedevCmds[];
 
 #endif /* VIRSH_NODEDEV_H */
-- 
2.16.2

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

[libvirt] [ [PATCH v3 6/8] virsh: Add event name completion to 'pool-event' command

[Lin Ma]

Signed-off-by: Lin Ma 
---
 tools/virsh-completer.c | 30 ++
 tools/virsh-completer.h |  4 
 tools/virsh-pool.c  |  1 +
 3 files changed, 35 insertions(+)

diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
index c0c3c5571a..1fab758e12 100644
--- a/tools/virsh-completer.c
+++ b/tools/virsh-completer.c
@@ -707,3 +707,33 @@ virshEventNameCompleter(vshControl *ctl,
 virStringListFree(ret);
 return NULL;
 }
+
+
+char **
+virshPoolEventNameCompleter(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_STORAGE_POOL_EVENT_ID_LAST) < 0)
+goto error;
+
+for (i = 0; i < VIR_STORAGE_POOL_EVENT_ID_LAST; i++) {
+if (VIR_STRDUP(ret[i], vshPoolEventCallbacks[i].name) < 0)
+goto error;
+}
+
+return ret;
+
+ error:
+virStringListFree(ret);
+return NULL;
+}
diff --git a/tools/virsh-completer.h b/tools/virsh-completer.h
index 9f8dc3dfa8..76fa31cb47 100644
--- a/tools/virsh-completer.h
+++ b/tools/virsh-completer.h
@@ -85,4 +85,8 @@ char ** virshSecretEventNameCompleter(vshControl *ctl,
   const vshCmd *cmd,
   unsigned int flags);
 
+char ** virshPoolEventNameCompleter(vshControl *ctl,
+const vshCmd *cmd,
+unsigned int flags);
+
 #endif
diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
index f0544d2e2d..5f59e35894 100644
--- a/tools/virsh-pool.c
+++ b/tools/virsh-pool.c
@@ -2098,6 +2098,7 @@ static const vshCmdOptDef opts_pool_event[] = {
 },
 {.name = "event",
  .type = VSH_OT_STRING,
+ .completer = virshPoolEventNameCompleter,
  .help = N_("which event type to wait for")
 },
 {.name = "loop",
-- 
2.16.2

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

[libvirt] [ [PATCH v3 4/8] virsh: Add event name completion to 'event' command

[Lin Ma]

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

diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
index b402fd22c3..c0c3c5571a 100644
--- a/tools/virsh-completer.c
+++ b/tools/virsh-completer.c
@@ -23,6 +23,7 @@
 #include 
 
 #include "virsh-completer.h"
+#include "virsh-domain.h"
 #include "virsh.h"
 #include "virsh-pool.h"
 #include "virsh-util.h"
@@ -676,3 +677,33 @@ virshSecretEventNameCompleter(vshControl *ctl 
ATTRIBUTE_UNUSED,
 virStringListFree(ret);
 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++) {
+if (VIR_STRDUP(ret[i], vshEventCallbacks[i].name) < 0)
+goto error;
+}
+
+return ret;
+
+ error:
+virStringListFree(ret);
+return NULL;
+}
diff --git a/tools/virsh-completer.h b/tools/virsh-completer.h
index c662267882..9f8dc3dfa8 100644
--- a/tools/virsh-completer.h
+++ b/tools/virsh-completer.h
@@ -73,6 +73,9 @@ char ** virshSecretUUIDCompleter(vshControl *ctl,
 char ** virshSnapshotNameCompleter(vshControl *ctl,
const vshCmd *cmd,
unsigned int flags);
+char ** virshEventNameCompleter(vshControl *ctl,
+const vshCmd *cmd,
+unsigned int flags);
 
 char ** virshAllocpagesPagesizeCompleter(vshControl *ctl,
 const vshCmd *cmd,
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 347c744bc9..4af7aa2155 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -13339,6 +13339,7 @@ static const vshCmdOptDef opts_event[] = {
   0),
 {.name = "event",
  .type = VSH_OT_STRING,
+ .completer = virshEventNameCompleter,
  .help = N_("which event type to wait for")
 },
 {.name = "all",
-- 
2.16.2

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

[libvirt] [ [PATCH v3 3/8] virsh: Move vshEventCallback structure definition to virsh-domain.h

[Lin Ma]

The next patch will use it in virsh-completer.c for returning the strings
of domain event name.

Signed-off-by: Lin Ma 
---
 tools/virsh-domain.c | 8 +---
 tools/virsh-domain.h | 8 
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index cfbbf5a7bc..347c744bc9 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -12832,12 +12832,6 @@ virshDomainEventTrayChangeToString(int reason)
 return str ? _(str) : _("unknown");
 }
 
-struct vshEventCallback {
-const char *name;
-virConnectDomainEventGenericCallback cb;
-};
-typedef struct vshEventCallback vshEventCallback;
-
 struct virshDomEventData {
 vshControl *ctl;
 bool loop;
@@ -13278,7 +13272,7 @@ virshEventBlockThresholdPrint(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 }
 
 
-static vshEventCallback vshEventCallbacks[] = {
+vshEventCallback vshEventCallbacks[] = {
 { "lifecycle",
   VIR_DOMAIN_EVENT_CALLBACK(virshEventLifecyclePrint), },
 { "reboot", virshEventGenericPrint, },
diff --git a/tools/virsh-domain.h b/tools/virsh-domain.h
index 3f9d12a5ff..e8e5611244 100644
--- a/tools/virsh-domain.h
+++ b/tools/virsh-domain.h
@@ -28,6 +28,14 @@
 
 # include "virsh.h"
 
+struct vshEventCallback {
+const char *name;
+virConnectDomainEventGenericCallback cb;
+};
+typedef struct vshEventCallback vshEventCallback;
+
+extern vshEventCallback vshEventCallbacks[];
+
 extern const vshCmdDef domManagementCmds[];
 
 #endif /* VIRSH_DOMAIN_H */
-- 
2.16.2

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

[libvirt] [ [PATCH v3 1/8] virsh-secret: Rename vshEventCallback to vshSecretEventCallback

[Lin Ma]

The next patch will use it in virsh-completer.c for returning the name
list of secret events.

The patch code originally authored by Michal Privoznik, Please refer to
https://www.redhat.com/archives/libvir-list/2018-May/msg01022.html

I splitted it to 2 patches with tiny change.

Signed-off-by: Lin Ma 
---
 tools/virsh-secret.c | 17 ++---
 tools/virsh-secret.h |  8 
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/tools/virsh-secret.c b/tools/virsh-secret.c
index 9e4ec61a88..07bc54d1bf 100644
--- a/tools/virsh-secret.c
+++ b/tools/virsh-secret.c
@@ -571,18 +571,12 @@ virshSecretEventToString(int event)
 return str ? _(str) : _("unknown");
 }
 
-struct vshEventCallback {
-const char *name;
-virConnectSecretEventGenericCallback cb;
-};
-typedef struct vshEventCallback vshEventCallback;
-
 struct virshSecretEventData {
 vshControl *ctl;
 bool loop;
 bool timestamp;
 int count;
-vshEventCallback *cb;
+vshSecretEventCallback *cb;
 };
 typedef struct virshSecretEventData virshSecretEventData;
 
@@ -652,11 +646,12 @@ vshEventGenericPrint(virConnectPtr conn ATTRIBUTE_UNUSED,
 vshEventDone(data->ctl);
 }
 
-static vshEventCallback vshEventCallbacks[] = {
+vshSecretEventCallback vshSecretEventCallbacks[] = {
 { "lifecycle",
   VIR_SECRET_EVENT_CALLBACK(vshEventLifecyclePrint), },
 { "value-changed", vshEventGenericPrint, },
 };
+verify(VIR_SECRET_EVENT_ID_LAST == ARRAY_CARDINALITY(vshSecretEventCallbacks));
 
 static const vshCmdInfo info_secret_event[] = {
 {.name = "help",
@@ -713,7 +708,7 @@ cmdSecretEvent(vshControl *ctl, const vshCmd *cmd)
 size_t i;
 
 for (i = 0; i < VIR_SECRET_EVENT_ID_LAST; i++)
-vshPrint(ctl, "%s\n", vshEventCallbacks[i].name);
+vshPrint(ctl, "%s\n", vshSecretEventCallbacks[i].name);
 return true;
 }
 
@@ -724,7 +719,7 @@ cmdSecretEvent(vshControl *ctl, const vshCmd *cmd)
 return false;
 }
 for (event = 0; event < VIR_SECRET_EVENT_ID_LAST; event++)
-if (STREQ(eventName, vshEventCallbacks[event].name))
+if (STREQ(eventName, vshSecretEventCallbacks[event].name))
 break;
 if (event == VIR_SECRET_EVENT_ID_LAST) {
 vshError(ctl, _("unknown event type %s"), eventName);
@@ -735,7 +730,7 @@ cmdSecretEvent(vshControl *ctl, const vshCmd *cmd)
 data.loop = vshCommandOptBool(cmd, "loop");
 data.timestamp = vshCommandOptBool(cmd, "timestamp");
 data.count = 0;
-data.cb = [event];
+data.cb = [event];
 if (vshCommandOptTimeoutToMs(ctl, cmd, ) < 0)
 return false;
 
diff --git a/tools/virsh-secret.h b/tools/virsh-secret.h
index dda22b021e..c70a2b5c75 100644
--- a/tools/virsh-secret.h
+++ b/tools/virsh-secret.h
@@ -28,6 +28,14 @@
 
 # include "virsh.h"
 
+struct vshSecretEventCallback {
+const char *name;
+virConnectSecretEventGenericCallback cb;
+};
+typedef struct vshSecretEventCallback vshSecretEventCallback;
+
+extern vshSecretEventCallback vshSecretEventCallbacks[];
+
 extern const vshCmdDef secretCmds[];
 
 #endif /* VIRSH_SECRET_H */
-- 
2.16.2

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

[libvirt] [ [PATCH v3 2/8] virsh: Add event name completion to 'secret-event' command

[Lin Ma]

The patch code originally authored by Michal Privoznik, Please refer to
https://www.redhat.com/archives/libvir-list/2018-May/msg01022.html

Signed-off-by: Lin Ma 
---
 tools/virsh-completer.c | 27 +++
 tools/virsh-completer.h |  4 
 tools/virsh-secret.c|  1 +
 3 files changed, 32 insertions(+)

diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
index 1435d1d4c6..b402fd22c3 100644
--- a/tools/virsh-completer.c
+++ b/tools/virsh-completer.c
@@ -26,6 +26,7 @@
 #include "virsh.h"
 #include "virsh-pool.h"
 #include "virsh-util.h"
+#include "virsh-secret.h"
 #include "internal.h"
 #include "virutil.h"
 #include "viralloc.h"
@@ -649,3 +650,29 @@ virshAllocpagesPagesizeCompleter(vshControl *ctl,
 VIR_FREE(ret);
 goto cleanup;
 }
+
+
+char **
+virshSecretEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
+  const vshCmd *cmd ATTRIBUTE_UNUSED,
+  unsigned int flags)
+{
+size_t i;
+char **ret = NULL;
+
+virCheckFlags(0, NULL);
+
+if (VIR_ALLOC_N(ret, VIR_SECRET_EVENT_ID_LAST) < 0)
+goto error;
+
+for (i = 0; i < VIR_SECRET_EVENT_ID_LAST; i++) {
+if (VIR_STRDUP(ret[i], vshSecretEventCallbacks[i].name) < 0)
+goto error;
+}
+
+return ret;
+
+ error:
+virStringListFree(ret);
+return NULL;
+}
diff --git a/tools/virsh-completer.h b/tools/virsh-completer.h
index c7b181879e..c662267882 100644
--- a/tools/virsh-completer.h
+++ b/tools/virsh-completer.h
@@ -78,4 +78,8 @@ char ** virshAllocpagesPagesizeCompleter(vshControl *ctl,
 const vshCmd *cmd,
 unsigned int flags);
 
+char ** virshSecretEventNameCompleter(vshControl *ctl,
+  const vshCmd *cmd,
+  unsigned int flags);
+
 #endif
diff --git a/tools/virsh-secret.c b/tools/virsh-secret.c
index 07bc54d1bf..ba0210e765 100644
--- a/tools/virsh-secret.c
+++ b/tools/virsh-secret.c
@@ -671,6 +671,7 @@ static const vshCmdOptDef opts_secret_event[] = {
 },
 {.name = "event",
  .type = VSH_OT_STRING,
+ .completer = virshSecretEventNameCompleter,
  .help = N_("which event type to wait for")
 },
 {.name = "loop",
-- 
2.16.2

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

[libvirt] [ [PATCH v3 0/8] virsh completion for event, secret-event, pool-event and nodedev-event

[Lin Ma]

v2 -> v3:
Most of patches in original v2 patchset were pushed, only 2 patches left,
So create a new patchset for including the 2 patches and 6 new completion
patches which about secret-event, pool-event and nodedev-event.

(Borrowed code & idea from Michal Privoznik for these patches design)


Lin Ma (8):
  virsh-secret: Rename vshEventCallback to vshSecretEventCallback
  virsh: Add event name completion to 'secret-event' command
  virsh: Move vshEventCallback structure definition to virsh-domain.h
  virsh: Add event name completion to 'event' command
  virsh-pool: Rename vshEventCallback to vshPoolEventCallback
  virsh: Add event name completion to 'pool-event' command
  virsh-nodedev: Rename vshEventCallback to vshNodedevEventCallback
  virsh: Add event name completion to 'nodedev-event' command

 tools/virsh-completer.c | 119 
 tools/virsh-completer.h |  15 ++
 tools/virsh-domain.c|   9 +---
 tools/virsh-domain.h|   8 
 tools/virsh-nodedev.c   |  19 +++-
 tools/virsh-nodedev.h   |   8 
 tools/virsh-pool.c  |  19 +++-
 tools/virsh-pool.h  |   8 
 tools/virsh-secret.c|  18 +++-
 tools/virsh-secret.h|   8 
 10 files changed, 189 insertions(+), 42 deletions(-)

-- 
2.16.2

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

[libvirt] [ [PATCH v3 3/8] virsh: Move vshEventCallback structure definition to virsh-domain.h

[Lin Ma]

The next patch will use it in virsh-completer.c for returning the strings
of domain event name.

Signed-off-by: Lin Ma 
---
 tools/virsh-domain.c | 8 +---
 tools/virsh-domain.h | 8 
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index cfbbf5a7bc..347c744bc9 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -12832,12 +12832,6 @@ virshDomainEventTrayChangeToString(int reason)
 return str ? _(str) : _("unknown");
 }
 
-struct vshEventCallback {
-const char *name;
-virConnectDomainEventGenericCallback cb;
-};
-typedef struct vshEventCallback vshEventCallback;
-
 struct virshDomEventData {
 vshControl *ctl;
 bool loop;
@@ -13278,7 +13272,7 @@ virshEventBlockThresholdPrint(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 }
 
 
-static vshEventCallback vshEventCallbacks[] = {
+vshEventCallback vshEventCallbacks[] = {
 { "lifecycle",
   VIR_DOMAIN_EVENT_CALLBACK(virshEventLifecyclePrint), },
 { "reboot", virshEventGenericPrint, },
diff --git a/tools/virsh-domain.h b/tools/virsh-domain.h
index 3f9d12a5ff..e8e5611244 100644
--- a/tools/virsh-domain.h
+++ b/tools/virsh-domain.h
@@ -28,6 +28,14 @@
 
 # include "virsh.h"
 
+struct vshEventCallback {
+const char *name;
+virConnectDomainEventGenericCallback cb;
+};
+typedef struct vshEventCallback vshEventCallback;
+
+extern vshEventCallback vshEventCallbacks[];
+
 extern const vshCmdDef domManagementCmds[];
 
 #endif /* VIRSH_DOMAIN_H */
-- 
2.16.2

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

[libvirt] [ [PATCH v3 2/8] virsh: Add event name completion to 'secret-event' command

[Lin Ma]

The patch code originally authored by Michal Privoznik, Please refer to
https://www.redhat.com/archives/libvir-list/2018-May/msg01022.html

Signed-off-by: Lin Ma 
---
 tools/virsh-completer.c | 27 +++
 tools/virsh-completer.h |  4 
 tools/virsh-secret.c|  1 +
 3 files changed, 32 insertions(+)

diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
index 1435d1d4c6..b402fd22c3 100644
--- a/tools/virsh-completer.c
+++ b/tools/virsh-completer.c
@@ -26,6 +26,7 @@
 #include "virsh.h"
 #include "virsh-pool.h"
 #include "virsh-util.h"
+#include "virsh-secret.h"
 #include "internal.h"
 #include "virutil.h"
 #include "viralloc.h"
@@ -649,3 +650,29 @@ virshAllocpagesPagesizeCompleter(vshControl *ctl,
 VIR_FREE(ret);
 goto cleanup;
 }
+
+
+char **
+virshSecretEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
+  const vshCmd *cmd ATTRIBUTE_UNUSED,
+  unsigned int flags)
+{
+size_t i;
+char **ret = NULL;
+
+virCheckFlags(0, NULL);
+
+if (VIR_ALLOC_N(ret, VIR_SECRET_EVENT_ID_LAST) < 0)
+goto error;
+
+for (i = 0; i < VIR_SECRET_EVENT_ID_LAST; i++) {
+if (VIR_STRDUP(ret[i], vshSecretEventCallbacks[i].name) < 0)
+goto error;
+}
+
+return ret;
+
+ error:
+virStringListFree(ret);
+return NULL;
+}
diff --git a/tools/virsh-completer.h b/tools/virsh-completer.h
index c7b181879e..c662267882 100644
--- a/tools/virsh-completer.h
+++ b/tools/virsh-completer.h
@@ -78,4 +78,8 @@ char ** virshAllocpagesPagesizeCompleter(vshControl *ctl,
 const vshCmd *cmd,
 unsigned int flags);
 
+char ** virshSecretEventNameCompleter(vshControl *ctl,
+  const vshCmd *cmd,
+  unsigned int flags);
+
 #endif
diff --git a/tools/virsh-secret.c b/tools/virsh-secret.c
index 07bc54d1bf..ba0210e765 100644
--- a/tools/virsh-secret.c
+++ b/tools/virsh-secret.c
@@ -671,6 +671,7 @@ static const vshCmdOptDef opts_secret_event[] = {
 },
 {.name = "event",
  .type = VSH_OT_STRING,
+ .completer = virshSecretEventNameCompleter,
  .help = N_("which event type to wait for")
 },
 {.name = "loop",
-- 
2.16.2

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

[libvirt] [ [PATCH v3 1/8] virsh-secret: Rename vshEventCallback to vshSecretEventCallback

[Lin Ma]

The next patch will use it in virsh-completer.c for returning the name
list of secret events.

The patch code originally authored by Michal Privoznik, Please refer to
https://www.redhat.com/archives/libvir-list/2018-May/msg01022.html

I splitted it to 2 patches with tiny change.

Signed-off-by: Lin Ma 
---
 tools/virsh-secret.c | 17 ++---
 tools/virsh-secret.h |  8 
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/tools/virsh-secret.c b/tools/virsh-secret.c
index 9e4ec61a88..07bc54d1bf 100644
--- a/tools/virsh-secret.c
+++ b/tools/virsh-secret.c
@@ -571,18 +571,12 @@ virshSecretEventToString(int event)
 return str ? _(str) : _("unknown");
 }
 
-struct vshEventCallback {
-const char *name;
-virConnectSecretEventGenericCallback cb;
-};
-typedef struct vshEventCallback vshEventCallback;
-
 struct virshSecretEventData {
 vshControl *ctl;
 bool loop;
 bool timestamp;
 int count;
-vshEventCallback *cb;
+vshSecretEventCallback *cb;
 };
 typedef struct virshSecretEventData virshSecretEventData;
 
@@ -652,11 +646,12 @@ vshEventGenericPrint(virConnectPtr conn ATTRIBUTE_UNUSED,
 vshEventDone(data->ctl);
 }
 
-static vshEventCallback vshEventCallbacks[] = {
+vshSecretEventCallback vshSecretEventCallbacks[] = {
 { "lifecycle",
   VIR_SECRET_EVENT_CALLBACK(vshEventLifecyclePrint), },
 { "value-changed", vshEventGenericPrint, },
 };
+verify(VIR_SECRET_EVENT_ID_LAST == ARRAY_CARDINALITY(vshSecretEventCallbacks));
 
 static const vshCmdInfo info_secret_event[] = {
 {.name = "help",
@@ -713,7 +708,7 @@ cmdSecretEvent(vshControl *ctl, const vshCmd *cmd)
 size_t i;
 
 for (i = 0; i < VIR_SECRET_EVENT_ID_LAST; i++)
-vshPrint(ctl, "%s\n", vshEventCallbacks[i].name);
+vshPrint(ctl, "%s\n", vshSecretEventCallbacks[i].name);
 return true;
 }
 
@@ -724,7 +719,7 @@ cmdSecretEvent(vshControl *ctl, const vshCmd *cmd)
 return false;
 }
 for (event = 0; event < VIR_SECRET_EVENT_ID_LAST; event++)
-if (STREQ(eventName, vshEventCallbacks[event].name))
+if (STREQ(eventName, vshSecretEventCallbacks[event].name))
 break;
 if (event == VIR_SECRET_EVENT_ID_LAST) {
 vshError(ctl, _("unknown event type %s"), eventName);
@@ -735,7 +730,7 @@ cmdSecretEvent(vshControl *ctl, const vshCmd *cmd)
 data.loop = vshCommandOptBool(cmd, "loop");
 data.timestamp = vshCommandOptBool(cmd, "timestamp");
 data.count = 0;
-data.cb = [event];
+data.cb = [event];
 if (vshCommandOptTimeoutToMs(ctl, cmd, ) < 0)
 return false;
 
diff --git a/tools/virsh-secret.h b/tools/virsh-secret.h
index dda22b021e..c70a2b5c75 100644
--- a/tools/virsh-secret.h
+++ b/tools/virsh-secret.h
@@ -28,6 +28,14 @@
 
 # include "virsh.h"
 
+struct vshSecretEventCallback {
+const char *name;
+virConnectSecretEventGenericCallback cb;
+};
+typedef struct vshSecretEventCallback vshSecretEventCallback;
+
+extern vshSecretEventCallback vshSecretEventCallbacks[];
+
 extern const vshCmdDef secretCmds[];
 
 #endif /* VIRSH_SECRET_H */
-- 
2.16.2

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

[libvirt] [ [PATCH v3 0/8] virsh completion for event, secret-event, pool-event and nodedev-event

[Lin Ma]

v2 -> v3:
Most of patches in original v2 patchset were pushed, only 2 patches left,
So create a new patchset for including the 2 patches and 6 new completion
patches which about secret-event, pool-event and nodedev-event.

(Borrowed code & idea from Michal Privoznik for these patches design)


Lin Ma (8):
  virsh-secret: Rename vshEventCallback to vshSecretEventCallback
  virsh: Add event name completion to 'secret-event' command
  virsh: Move vshEventCallback structure definition to virsh-domain.h
  virsh: Add event name completion to 'event' command
  virsh-pool: Rename vshEventCallback to vshPoolEventCallback
  virsh: Add event name completion to 'pool-event' command
  virsh-nodedev: Rename vshEventCallback to vshNodedevEventCallback
  virsh: Add event name completion to 'nodedev-event' command

 tools/virsh-completer.c | 119 
 tools/virsh-completer.h |  15 ++
 tools/virsh-domain.c|   9 +---
 tools/virsh-domain.h|   8 
 tools/virsh-nodedev.c   |  19 +++-
 tools/virsh-nodedev.h   |   8 
 tools/virsh-pool.c  |  19 +++-
 tools/virsh-pool.h  |   8 
 tools/virsh-secret.c|  18 +++-
 tools/virsh-secret.h|   8 
 10 files changed, 189 insertions(+), 42 deletions(-)

-- 
2.16.2

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

Re: [libvirt] [ v3 0/4] Introduce network-backed loader & NVRAM.

[Prerna]

On Mon, May 21, 2018 at 4:40 PM, Prerna Saxena 
wrote:

> Libvirt domain XML allows only local filepaths to specify a loader element
> or its matching NVRAM. Given that VMs may themselves move across hypervisor
> hosts, it should be possible to allocate loaders/NVRAM disks on network
> storage
> for uninterrupted access.
>
> This series extends the loader & NVRAM disk elements to be described as
> virStorageSource* elements, as discussed in :
> https://www.redhat.com/archives/libvir-list/2018-March/msg01721.html
>
> Sample XML with new annotation:
>
> 
>   
> 
> 
>   
> 
>   
> 
>
> References:
> --
> v0/ Proposal: https://www.redhat.com/archives/libvir-list/2018-
> March/msg01721.html.v1
> v1: https://www.redhat.com/archives/libvir-list/2018-April/msg02024.html
> v2: https://www.redhat.com/archives/libvir-list/2018-May/msg00948.html
>
> Changelog:
> -
> Changes since v2:
> - Consolidated patches with related data structures to avoid build
> breakage.
> - Passes make check & make syntax-check.
>
> Prerna Saxena (4):
>   Schema: Introduce XML schema for network-backed loader and nvram
> elements.
>   Loader: Add a more elaborate definition.
>   Test: Add a test snippet to evaluate command line generation for
> loader/nvram specified via virStorageSource
>   Documentation: Add a blurb for the newly added XML snippets for loader
> and nvram.
>
>  docs/formatdomain.html.in  |  36 +++-
>  docs/schemas/domaincommon.rng  | 108 +--
>  src/bhyve/bhyve_command.c  |   6 +-
>  src/conf/domain_conf.c | 250
> +++--
>  src/conf/domain_conf.h |  11 +-
>  src/qemu/qemu_cgroup.c |  13 +-
>  src/qemu/qemu_command.c|  21 ++-
>  src/qemu/qemu_domain.c |  31 ++-
>  src/qemu/qemu_driver.c |   7 +-
>  src/qemu/qemu_parse_command.c  |  30 ++-
>  src/qemu/qemu_process.c|  54 --
>  src/security/security_dac.c|   6 +-
>  src/security/security_selinux.c|   6 +-
>  src/security/virt-aa-helper.c  |  14 +-
>  src/vbox/vbox_common.c |  11 +-
>  src/xenapi/xenapi_driver.c |   4 +-
>  src/xenconfig/xen_sxpr.c   |  19 +-
>  src/xenconfig/xen_xm.c |   9 +-
>  tests/qemuxml2argvdata/bios-nvram-network.args |  31 +++
>  tests/qemuxml2argvdata/bios-nvram-network.xml  |  42 +
>  tests/qemuxml2argvtest.c   |   1 +
>  21 files changed, 606 insertions(+), 104 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/bios-nvram-network.args
>  create mode 100644 tests/qemuxml2argvdata/bios-nvram-network.xml
>
> --
>

Just FYI, I will be on vacation starting tomorrow until June 4. I will
address all review comments as soon as I'm back.

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