Re: [libvirt] [PATCH] libxl: provide impl for nodeGetSecurityModel
On 05/15/2015 12:52 PM, Jim Fehlig wrote:
> Currently, the libxl driver does not support any security drivers.
> When the qemu driver has no security driver configued,
> nodeGetSecurityModel succeeds but returns an empty virSecurityModel
> object. Do the same in the libxl driver instead of reporting
>
> this function is not supported by the connection driver:
> virNodeGetSecurityModel
>
> Signed-off-by: Jim Fehlig
> ---
>
> I was reminded of this today when looking through a libvirtd log.
> The system was running a test script that among other things
> called 'virsh dominfo'. Each time dominfo was called, the log
> was spammed with "this function is not supported by the connection
> driver: virNodeGetSecurityModel".
>
> src/libxl/libxl_driver.c | 18 ++
> 1 file changed, 18 insertions(+)
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 60c139e..d6b20ae 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -5027,6 +5027,23 @@ libxlDomainMigrateConfirm3Params(virDomainPtr domain,
> return libxlDomainMigrationConfirm(driver, vm, flags, cancelled);
> }
>
> +static int libxlNodeGetSecurityModel(virConnectPtr conn,
> + virSecurityModelPtr secmodel)
> +{
> +memset(secmodel, 0, sizeof(*secmodel));
I wonder if src/libvirt-host.c should take care of this for all callers.
But it doesn't need to happen in this patch.
ACK
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] netdev: assure that SRIOV PF is online before modifying VF params
On 05/15/2015 11:13 AM, Laine Stump wrote: > On 05/15/2015 05:35 AM, Michal Privoznik wrote: >> On 14.05.2015 21:38, Laine Stump wrote: >>> The kernel won't complain if you set the mac address and vlan tag for >>> an SRIOV VF via its PF, and it will even let you assign the PF to a >>> guest using PCI device assignment or macvtap passthrough. But if the >>> PF isn't online, the device won't be usable in the guest. This patch >>> makes sure that it is turned on. >>> >>> Since multiple guests/VFs could use the same PF, there is no point in >>> ever setting the PF *off*line. >>> >>> This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=893738 >>> >>> Originally filed against RHEL6, but present in every version of >>> libvirt until today. >>> --- >>> src/util/virnetdev.c | 11 +++ >>> 1 file changed, 11 insertions(+) >>> >>> diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c >>> index e14b401..7022dfa 100644 >>> --- a/src/util/virnetdev.c >>> +++ b/src/util/virnetdev.c >>> @@ -2258,6 +2258,17 @@ virNetDevReplaceVfConfig(const char *pflinkdev, int >>> vf, >>> char macstr[VIR_MAC_STRING_BUFLEN]; >>> char *fileData = NULL; >>> int ifindex = -1; >>> +bool pfIsOnline; >>> + >>> +/* Assure that PF is online prior to twiddling with the VF. It >>> + * *should* be, but if the PF isn't online the changes made to the >>> + * VF via the PF won't take effect, yet there will be no error >>> + * reported. >>> + */ >>> +if (virNetDevGetOnline(pflinkdev, &pfIsOnline) < 0) >>> +return ret; >>> +if (!pfIsOnline && virNetDevSetOnline(pflinkdev, true) < 0) >>> +return ret; >>> >>> if (virNetDevGetVfConfig(pflinkdev, vf, &oldmac, &oldvlanid) < 0) >>> goto cleanup; >>> >> ACK. Should we set the device back to its previous state if something >> goes wrong later in the function? > I don't think so. The PF needs to be on in order for any VF to work > properly, and we can't assume that nobody else has started trying to use > it since we set it online. (this is similar to what we do with the > ip_forward setting in the network driver). > > I guess an alternative would be to *never* set the PF online in libvirt, > but instead to log an error an fail; still better than silently failing > to pass traffic. Any opinions on which is the better approach? Dan Berrange just pointed out on IRC that setting the PF online would cause it to do IPv6 autoconfig on the host (that's the default when there is no config for the interface), which might not be what the admin expected or wanted, so it's a better idea to fail in the case that the PF is offline, and tell them they need to turn it on in the host network config. I just redid the patch to fail/log an error: https://www.redhat.com/archives/libvir-list/2015-May/msg00541.html Thanks for the quick review though (and the question that made me re-think) :-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2] netdev: fail when setting up an SRIOV VF if PF is offline
If an SRIOV PF is offline, the kernel won't complain if you set the
mac address and vlan tag for a VF via this PF, and it will even let
you assign the VF to a guest using PCI device assignment or macvtap
passthrough. But in this case (the PF isn't online), the device won't
be usable in the guest.
Silently setting the PF online would solve the connectivity problem,
but as pointed out by Dan Berrange, when an interface is set online
with no associated config, the kernel will by default turn on IPv6
autoconf, which could create unexpected security problems for the
host. For this reason, this patch instead logs an error and fails the
operation.
This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=893738
Originally filed against RHEL6, but present in every version of
libvirt until today.
---
src/util/virnetdev.c | 22 ++
1 file changed, 22 insertions(+)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index e14b401..d0580a0 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -2258,6 +2258,28 @@ virNetDevReplaceVfConfig(const char *pflinkdev, int vf,
char macstr[VIR_MAC_STRING_BUFLEN];
char *fileData = NULL;
int ifindex = -1;
+bool pfIsOnline;
+
+/* Assure that PF is online prior to twiddling with the VF. It
+ * *should* be, but if the PF isn't online the changes made to the
+ * VF via the PF won't take effect, yet there will be no error
+ * reported. In the case that it isn't online, fail and report the
+ * error, since setting an unconfigured interface online
+ * automatically turns on IPv6 autoconfig, which may not be what
+ * the admin expects, so we want them to explicitly enable the PF
+ * in the host system network config.
+ */
+if (virNetDevGetOnline(pflinkdev, &pfIsOnline) < 0)
+ goto cleanup;
+if (!pfIsOnline) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ "Unable to configure VF %d of PF '%s' "
+ "because the PF is not online. Please "
+ "change host network config to put the "
+ "PF online.",
+ vf, pflinkdev);
+goto cleanup;
+}
if (virNetDevGetVfConfig(pflinkdev, vf, &oldmac, &oldvlanid) < 0)
goto cleanup;
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] libxl: provide impl for nodeGetSecurityModel
Currently, the libxl driver does not support any security drivers.
When the qemu driver has no security driver configued,
nodeGetSecurityModel succeeds but returns an empty virSecurityModel
object. Do the same in the libxl driver instead of reporting
this function is not supported by the connection driver:
virNodeGetSecurityModel
Signed-off-by: Jim Fehlig
---
I was reminded of this today when looking through a libvirtd log.
The system was running a test script that among other things
called 'virsh dominfo'. Each time dominfo was called, the log
was spammed with "this function is not supported by the connection
driver: virNodeGetSecurityModel".
src/libxl/libxl_driver.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 60c139e..d6b20ae 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -5027,6 +5027,23 @@ libxlDomainMigrateConfirm3Params(virDomainPtr domain,
return libxlDomainMigrationConfirm(driver, vm, flags, cancelled);
}
+static int libxlNodeGetSecurityModel(virConnectPtr conn,
+ virSecurityModelPtr secmodel)
+{
+memset(secmodel, 0, sizeof(*secmodel));
+
+if (virNodeGetSecurityModelEnsureACL(conn) < 0)
+return -1;
+
+/*
+ * Currently the libxl driver does not support security model.
+ * Similar to the qemu driver, treat this as success and simply
+ * return no data in secmodel. Avoids spamming the libvirt log
+ * with "this function is not supported by the connection driver:
+ * virNodeGetSecurityModel"
+ */
+return 0;
+}
static virHypervisorDriver libxlHypervisorDriver = {
.name = LIBXL_DRIVER_NAME,
@@ -5122,6 +5139,7 @@ static virHypervisorDriver libxlHypervisorDriver = {
.domainMigratePerform3Params = libxlDomainMigratePerform3Params, /* 1.2.6
*/
.domainMigrateFinish3Params = libxlDomainMigrateFinish3Params, /* 1.2.6 */
.domainMigrateConfirm3Params = libxlDomainMigrateConfirm3Params, /* 1.2.6
*/
+.nodeGetSecurityModel = libxlNodeGetSecurityModel, /* 1.2.16 */
};
static virConnectDriver libxlConnectDriver = {
--
2.3.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/4] libvirt: qemu: enable/disable protected key management ops
On 05/15/2015 12:23 PM, Ján Tomko wrote:
On Fri, May 15, 2015 at 04:43:29PM +0200, Michal Privoznik wrote:
From: Tony Krowiak
Introduces two new -machine option parameters to the QEMU command to
enable/disable the CPACF protected key management operations for a guest:
aes-key-wrap='on|off'
dea-key-wrap='on|off'
The QEMU code maps the corresponding domain configuration elements to the
QEMU -machine option parameters to create the QEMU command:
--> aes-key-wrap=on
--> aes-key-wrap=off
--> dea-key-wrap=on
--> dea-key-wrap=off
Signed-off-by: Tony Krowiak
Signed-off-by: Daniel Hansel
Signed-off-by: Boris Fiuczynski
Reviewed-by: Boris Fiuczynski
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_capabilities.c | 4 +++
src/qemu/qemu_capabilities.h | 2 ++
src/qemu/qemu_command.c | 73
3 files changed, 79 insertions(+)
So the difference to v1 is that they are no longer turned on by default
if QEMU supports it. (I hope I did not miss anything else; it would
have been helpful if you listed the important changes)
I agree that this should not be done on XML parsing as was done in v1.
Would it make sense to treat the missing option (STATE_ABSENT) as
'turn it on if qemu supports it'?
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 2939f8d..98fc5f8 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -38,6 +38,7 @@
#include "virnetdevbridge.h"
#include "virstring.h"
#include "virtime.h"
+#include "virutil.h"
Why is this include needed?
#include "viruuid.h"
#include "c-ctype.h"
#include "domain_nwfilter.h"
@@ -7286,6 +7287,39 @@ qemuBuildObsoleteAccelArg(virCommandPtr cmd,
return 0;
}
+static bool
+qemuAppendKeyWrapMachineParm(virBuffer *buf, virQEMUCapsPtr qemuCaps,
+ int flag, const char *pname, int pstate)
+{
+if (pstate != VIR_TRISTATE_SWITCH_ABSENT) {
+if (!virQEMUCapsGet(qemuCaps, flag)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("%s is not available with this QEMU binary"),
pname);
+return false;
+}
Can't we allow state='off' with QEMU that does not support it?
You have an ACK from me with the include removed. Please wait for
feedback from the author before pushing.
These changes will break the test cases, so they will need to be updated
to reflect
these changes before pushing.
Jan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/4] libvirt: qemu: enable/disable protected key management ops
On 05/15/2015 12:23 PM, Ján Tomko wrote:
On Fri, May 15, 2015 at 04:43:29PM +0200, Michal Privoznik wrote:
From: Tony Krowiak
Introduces two new -machine option parameters to the QEMU command to
enable/disable the CPACF protected key management operations for a guest:
aes-key-wrap='on|off'
dea-key-wrap='on|off'
The QEMU code maps the corresponding domain configuration elements to the
QEMU -machine option parameters to create the QEMU command:
--> aes-key-wrap=on
--> aes-key-wrap=off
--> dea-key-wrap=on
--> dea-key-wrap=off
Signed-off-by: Tony Krowiak
Signed-off-by: Daniel Hansel
Signed-off-by: Boris Fiuczynski
Reviewed-by: Boris Fiuczynski
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_capabilities.c | 4 +++
src/qemu/qemu_capabilities.h | 2 ++
src/qemu/qemu_command.c | 73
3 files changed, 79 insertions(+)
So the difference to v1 is that they are no longer turned on by default
if QEMU supports it. (I hope I did not miss anything else; it would
have been helpful if you listed the important changes)
I agree that this should not be done on XML parsing as was done in v1.
Would it make sense to treat the missing option (STATE_ABSENT) as
'turn it on if qemu supports it'?
Some background:
My original design was similar to Michal's in that if key wrapping was not
configured for the guest in the domain XML, then the machine options
would not
be inserted into the QEMU command line. Our internal reviewers commented
that
there should be default values for libvirt that match the QEMU defaults, so
I did exactly as you suggested here, inserting default values into the QEMU
command line on STATE_ABSENT. Our internal reviewers then pointed out
that the
dumpxml command would return a configuration that did not match that of
the running
guest, so I added the XML post parsing piece to set default values into
virDomainDef if
QEMU supports the key wrapping machine options. In any case, I'm not
married to
any of these ideas, so you have my ACK pending Jan's suggestions.
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 2939f8d..98fc5f8 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -38,6 +38,7 @@
#include "virnetdevbridge.h"
#include "virstring.h"
#include "virtime.h"
+#include "virutil.h"
Why is this include needed?
I believe that this is no longer needed and is a remnant of an earlier
iteration that I failed to
remove. My bad.
#include "viruuid.h"
#include "c-ctype.h"
#include "domain_nwfilter.h"
@@ -7286,6 +7287,39 @@ qemuBuildObsoleteAccelArg(virCommandPtr cmd,
return 0;
}
+static bool
+qemuAppendKeyWrapMachineParm(virBuffer *buf, virQEMUCapsPtr qemuCaps,
+ int flag, const char *pname, int pstate)
+{
+if (pstate != VIR_TRISTATE_SWITCH_ABSENT) {
+if (!virQEMUCapsGet(qemuCaps, flag)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("%s is not available with this QEMU binary"),
pname);
+return false;
+}
Can't we allow state='off' with QEMU that does not support it?
We can certainly bypass the appending of the machine option if
state='off', but if I am
not mistaken, sending a machine option to QEMU that it does not support
will cause
QEMU to throw an error. I think it is wisest to inform the user of a
configuration
error here.
You have an ACK from me with the include removed. Please wait for
feedback from the author before pushing.
Jan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] libvirt: conf: parse XML for protected key management ops
On 05/15/2015 10:39 AM, Michal Privoznik wrote: On 27.04.2015 23:57, akrow...@linux.vnet.ibm.com wrote: From: Tony Krowiak Parse the domain configuration XML elements that enable/disable access to the protected key management operations for a guest: ... ... Signed-off-by: Tony Krowiak Signed-off-by: Viktor Mihajlovski Signed-off-by: Daniel Hansel Reviewed-by: Boris Fiuczynski --- src/conf/domain_conf.c | 189 ++ src/conf/domain_conf.h | 20 + src/libvirt_private.syms |2 + 3 files changed, 211 insertions(+), 0 deletions(-) Er. I'm just too lazy to point out all the bits. I'm gonna rework and post v2. Does that mean you will be reworking the parsing code as well? Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] libvirt: qemu: enable/disable protected key management ops
On 05/15/2015 10:39 AM, Michal Privoznik wrote:
On 27.04.2015 23:57, akrow...@linux.vnet.ibm.com wrote:
From: Tony Krowiak
Introduces two new -machine option parameters to the QEMU command to
enable/disable the CPACF protected key management operations for a guest:
aes-key-wrap='on|off'
dea-key-wrap='on|off'
The QEMU code maps the corresponding domain configuration elements to the
QEMU -machine option parameters to create the QEMU command:
--> aes-key-wrap=on
--> aes-key-wrap=off
--> dea-key-wrap=on
--> dea-key-wrap=off
Signed-off-by: Tony Krowiak
Signed-off-by: Daniel Hansel
Signed-off-by: Boris Fiuczynski
Reviewed-by: Boris Fiuczynski
---
src/qemu/qemu_capabilities.c |5 +++
src/qemu/qemu_capabilities.h |2 +
src/qemu/qemu_command.c | 72 ++
src/qemu/qemu_domain.c | 39 ++-
4 files changed, 117 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index a458611..d1b9f6f 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -279,6 +279,9 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
"qxl.vgamem_mb",
"qxl-vga.vgamem_mb",
"pc-dimm",
+
+ "aes-key-wrap", /* 185 */
+ "dea-key-wrap",
);
@@ -2518,6 +2521,8 @@ static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = {
{ "msg", "timestamp", QEMU_CAPS_MSG_TIMESTAMP },
{ "numa", NULL, QEMU_CAPS_NUMA },
{ "drive", "throttling.bps-total-max", QEMU_CAPS_DRIVE_IOTUNE_MAX},
+{ "machine", "aes-key-wrap", QEMU_CAPS_AES_KEY_WRAP },
+{ "machine", "dea-key-wrap", QEMU_CAPS_DEA_KEY_WRAP },
};
static int
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index c7b1ac7..31e0494 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -224,6 +224,8 @@ typedef enum {
QEMU_CAPS_QXL_VGAMEM = 182, /* -device qxl.vgamem_mb */
QEMU_CAPS_QXL_VGA_VGAMEM = 183, /* -device qxl-vga.vgamem_mb */
QEMU_CAPS_DEVICE_PC_DIMM = 184, /* pc-dimm device */
+QEMU_CAPS_AES_KEY_WRAP = 185, /* -machine aes_key_wrap */
+QEMU_CAPS_DEA_KEY_WRAP = 186, /* -machine dea_key_wrap */
QEMU_CAPS_LAST, /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 247954f..8ff1d88 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -38,6 +38,7 @@
#include "virnetdevbridge.h"
#include "virstring.h"
#include "virtime.h"
+#include "virutil.h"
#include "viruuid.h"
#include "c-ctype.h"
#include "domain_nwfilter.h"
@@ -7295,6 +7296,39 @@ qemuBuildObsoleteAccelArg(virCommandPtr cmd,
return 0;
}
+static bool
+qemuAppendKeyWrapMachineParm(virBuffer *buf, virQEMUCapsPtr qemuCaps,
+ int flag, const char *pname, int pstate)
+{
+if (pstate != VIR_TRISTATE_SWITCH_ABSENT) {
+if (!virQEMUCapsGet(qemuCaps, flag)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("%s is not available with this QEMU binary"),
pname);
+return false;
+}
+
+virBufferAsprintf(buf, ",%s=%s", pname,
+ virTristateSwitchTypeToString(pstate));
+}
+
+return true;
+}
+
+static bool
+qemuAppendKeyWrapMachineParms(virBuffer *buf, virQEMUCapsPtr qemuCaps,
+ const virDomainDef *def)
+{
+if (!qemuAppendKeyWrapMachineParm(buf, qemuCaps, QEMU_CAPS_AES_KEY_WRAP,
+ "aes-key-wrap", def->keywrap.aes))
+return false;
+
+if (!qemuAppendKeyWrapMachineParm(buf, qemuCaps, QEMU_CAPS_DEA_KEY_WRAP,
+ "dea-key-wrap", def->keywrap.dea))
+return false;
+
+return true;
+}
+
static int
qemuBuildMachineArgStr(virCommandPtr cmd,
const virDomainDef *def,
@@ -7329,6 +7363,14 @@ qemuBuildMachineArgStr(virCommandPtr cmd,
}
obsoleteAccel = true;
+
+if ((def->keywrap.aes != VIR_TRISTATE_SWITCH_ABSENT) ||
+(def->keywrap.dea != VIR_TRISTATE_SWITCH_ABSENT)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("key wrap support is not available "
+ "with this QEMU binary"));
+return -1;
+}
} else {
virBuffer buf = VIR_BUFFER_INITIALIZER;
@@ -7373,6 +7415,11 @@ qemuBuildMachineArgStr(virCommandPtr cmd,
}
}
+if (!qemuAppendKeyWrapMachineParms(&buf, qemuCaps, def)) {
+virBufferFreeAndReset(&buf);
+return -1;
+}
+
virCommandAddArgBuffer(cmd, &buf);
}
@@ -12772,6 +12819,9 @@ qemuParseCommand
Re: [libvirt] [PATCH v2 4/4] libvirt: tests: test protected key mgmt ops support
On Fri, May 15, 2015 at 04:43:30PM +0200, Michal Privoznik wrote: > From: Tony Krowiak > > Test the support for enabling/disabling CPACF protected key management > operations for a guest. > > Signed-off-by: Tony Krowiak > Signed-off-by: Viktor Mihajlovski > Reviewed-by: Boris Fiuczynski > Signed-off-by: Michal Privoznik > --- ACK, this should be squashed with the patch adding the qemu command line formatting. Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/4] libvirt: qemu: enable/disable protected key management ops
On Fri, May 15, 2015 at 04:43:29PM +0200, Michal Privoznik wrote:
> From: Tony Krowiak
>
> Introduces two new -machine option parameters to the QEMU command to
> enable/disable the CPACF protected key management operations for a guest:
>
> aes-key-wrap='on|off'
> dea-key-wrap='on|off'
>
> The QEMU code maps the corresponding domain configuration elements to the
> QEMU -machine option parameters to create the QEMU command:
>
>--> aes-key-wrap=on
> --> aes-key-wrap=off
>--> dea-key-wrap=on
> --> dea-key-wrap=off
>
> Signed-off-by: Tony Krowiak
> Signed-off-by: Daniel Hansel
> Signed-off-by: Boris Fiuczynski
> Reviewed-by: Boris Fiuczynski
> Signed-off-by: Michal Privoznik
> ---
> src/qemu/qemu_capabilities.c | 4 +++
> src/qemu/qemu_capabilities.h | 2 ++
> src/qemu/qemu_command.c | 73
>
> 3 files changed, 79 insertions(+)
>
So the difference to v1 is that they are no longer turned on by default
if QEMU supports it. (I hope I did not miss anything else; it would
have been helpful if you listed the important changes)
I agree that this should not be done on XML parsing as was done in v1.
Would it make sense to treat the missing option (STATE_ABSENT) as
'turn it on if qemu supports it'?
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 2939f8d..98fc5f8 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -38,6 +38,7 @@
> #include "virnetdevbridge.h"
> #include "virstring.h"
> #include "virtime.h"
> +#include "virutil.h"
Why is this include needed?
> #include "viruuid.h"
> #include "c-ctype.h"
> #include "domain_nwfilter.h"
> @@ -7286,6 +7287,39 @@ qemuBuildObsoleteAccelArg(virCommandPtr cmd,
> return 0;
> }
>
> +static bool
> +qemuAppendKeyWrapMachineParm(virBuffer *buf, virQEMUCapsPtr qemuCaps,
> + int flag, const char *pname, int pstate)
> +{
> +if (pstate != VIR_TRISTATE_SWITCH_ABSENT) {
> +if (!virQEMUCapsGet(qemuCaps, flag)) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("%s is not available with this QEMU binary"),
> pname);
> +return false;
> +}
Can't we allow state='off' with QEMU that does not support it?
You have an ACK from me with the include removed. Please wait for
feedback from the author before pushing.
Jan
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] virsh: Improve error message on integer value parsing failure.
Replace more than 30 ad-hoc error messages with a single, generic one
that contains the name of the option being processed and some hints
to help the user understand what could have gone wrong.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1207043
---
tests/vcpupin| 4 +-
tools/virsh-domain-monitor.c | 10 +++--
tools/virsh-domain.c | 102 ---
tools/virsh-host.c | 44 ++-
tools/virsh-interface.c | 4 +-
tools/virsh-network.c| 4 +-
tools/virsh-volume.c | 16 +--
7 files changed, 135 insertions(+), 49 deletions(-)
diff --git a/tests/vcpupin b/tests/vcpupin
index cd09145..ab0d38f 100755
--- a/tests/vcpupin
+++ b/tests/vcpupin
@@ -34,7 +34,7 @@ fail=0
$abs_top_builddir/tools/virsh --connect test:///default vcpupin test a 0,1 >
out 2>&1
test $? = 1 || fail=1
cat <<\EOF > exp || fail=1
-error: vcpupin: Invalid vCPU number.
+error: Numeric value for option is malformed or out of range
EOF
compare exp out || fail=1
@@ -52,7 +52,7 @@ compare exp out || fail=1
$abs_top_builddir/tools/virsh --connect test:///default vcpupin test -100 0,1
> out 2>&1
test $? = 1 || fail=1
cat <<\EOF > exp || fail=1
-error: vcpupin: Invalid vCPU number.
+error: Numeric value for option is malformed or out of range
EOF
compare exp out || fail=1
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 91c57e2..51276d3 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -341,8 +341,9 @@ cmdDomMemStat(vshControl *ctl, const vshCmd *cmd)
* This is not really an unsigned long, but it
*/
if ((rv = vshCommandOptInt(cmd, "period", &period)) < 0) {
-vshError(ctl, "%s",
- _("Unable to parse integer parameter."));
+vshError(ctl,
+ _("Numeric value for <%s> option is malformed or out of
range"),
+ "period");
goto cleanup;
}
if (rv > 0) {
@@ -1439,8 +1440,9 @@ cmdDomTime(vshControl *ctl, const vshCmd *cmd)
if (rv < 0) {
/* invalid integer format */
-vshError(ctl, "%s",
- _("Unable to parse integer parameter to --time."));
+vshError(ctl,
+ _("Numeric value for <%s> option is malformed or out of
range"),
+ "time");
goto cleanup;
} else if (rv > 0) {
/* valid integer to set */
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 20f8c75..10d01b6 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1552,7 +1552,9 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd)
return false;
if ((rv = vshCommandOptInt(cmd, "weight", &weight)) < 0) {
-vshError(ctl, "%s", _("Unable to parse integer parameter"));
+vshError(ctl,
+ _("Numeric value for <%s> option is malformed or out of
range"),
+ "weight");
goto cleanup;
} else if (rv > 0) {
if (weight <= 0) {
@@ -1691,7 +1693,9 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd,
goto cleanup;
if (vshCommandOptULWrap(cmd, "bandwidth", &bandwidth) < 0) {
-vshError(ctl, "%s", _("bandwidth must be a number"));
+vshError(ctl,
+ _("Numeric value for <%s> option is malformed or out of
range"),
+ "bandwidth");
goto cleanup;
}
@@ -2213,15 +2217,21 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd)
* than trying to guess which value will work well across both
* APIs with their different sizes and scales. */
if (vshCommandOptULWrap(cmd, "bandwidth", &bandwidth) < 0) {
-vshError(ctl, "%s", _("bandwidth must be a number"));
+vshError(ctl,
+ _("Numeric value for <%s> option is malformed or out of
range"),
+ "bandwidth");
goto cleanup;
}
if (vshCommandOptUInt(cmd, "granularity", &granularity) < 0) {
-vshError(ctl, "%s", _("granularity must be a number"));
+vshError(ctl,
+ _("Numeric value for <%s> option is malformed or out of
range"),
+ "granularity");
goto cleanup;
}
if (vshCommandOptULongLong(cmd, "buf-size", &buf_size) < 0) {
-vshError(ctl, "%s", _("buf-size must be a number"));
+vshError(ctl,
+ _("Numeric value for <%s> option is malformed or out of
range"),
+ "buf-size");
goto cleanup;
}
@@ -2791,7 +2801,9 @@ cmdBlockResize(vshControl *ctl, const vshCmd *cmd)
return false;
if (vshCommandOptScaledInt(cmd, "size", &size, 1024, ULLONG_MAX) < 0) {
-vshError(ctl, "%s", _("Unable to parse integer"));
+vshError(ctl,
+ _("Numeric value for <%s> option is malformed or out of
range"),
+ "size");
return false;
}
@@ -3395,7 +3407,9 @@ cmdDomPMSuspend(vshCo
[libvirt] [PATCH 0/3] virsh: Fix and enhance handling of numeric options
The first commit improves error messages by making them much more consistent and a little bit more helpful to the user. The other two commits contain smaller fixes and enhancement. Andrea Bolognani (3): virsh: Improve error message on integer value parsing failure. virsh: Fix dommemstat --period option type. virsh: Improve handling of send-process-signal --pid. tests/vcpupin| 4 +- tools/virsh-domain-monitor.c | 12 +++-- tools/virsh-domain.c | 114 ++- tools/virsh-host.c | 44 - tools/virsh-interface.c | 4 +- tools/virsh-network.c| 4 +- tools/virsh-volume.c | 16 -- 7 files changed, 141 insertions(+), 57 deletions(-) -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] virsh: Improve handling of send-process-signal --pid.
Use vshCommandOptLongLong() instead of retrieving the value as a
string and converting it to a number manually.
---
tools/virsh-domain.c | 12 +---
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 10d01b6..36f3e6c 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -8262,7 +8262,6 @@ cmdSendProcessSignal(vshControl *ctl, const vshCmd *cmd)
{
virDomainPtr dom;
bool ret = false;
-const char *pidstr;
const char *signame;
long long pid_value;
int signum;
@@ -8270,17 +8269,16 @@ cmdSendProcessSignal(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
-if (vshCommandOptStringReq(ctl, cmd, "pid", &pidstr) < 0)
+if (vshCommandOptLongLong(cmd, "pid", &pid_value) < 0) {
+vshError(ctl,
+ _("Numeric value for <%s> option is malformed or out of
range"),
+ "pid");
goto cleanup;
+}
if (vshCommandOptStringReq(ctl, cmd, "signame", &signame) < 0)
goto cleanup;
-if (virStrToLong_ll(pidstr, NULL, 10, &pid_value) < 0) {
-vshError(ctl, _("malformed PID value: %s"), pidstr);
-goto cleanup;
-}
-
if ((signum = getSignalNumber(ctl, signame)) < 0) {
vshError(ctl, _("malformed signal name: %s"), signame);
goto cleanup;
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] virsh: Fix dommemstat --period option type.
The option didn't have VSH_OT_INT type even thought it's expected
to be numeric, as shown by the fact that vshCommandOptInt() is later
used to retrieve its value.
---
tools/virsh-domain-monitor.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 51276d3..a42c150 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -287,7 +287,7 @@ static const vshCmdOptDef opts_dommemstat[] = {
.help = N_("domain name, id or uuid")
},
{.name = "period",
- .type = VSH_OT_STRING,
+ .type = VSH_OT_INT,
.flags = VSH_OFLAG_REQ_OPT,
.help = N_("period in seconds to set collection")
},
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/4] libvirt: conf: parse XML for protected key management ops
On Fri, May 15, 2015 at 04:43:28PM +0200, Michal Privoznik wrote: > From: Tony Krowiak > > Parse the domain configuration XML elements that enable/disable access to > the protected key management operations for a guest: > > > ... > > > > ... > > > Signed-off-by: Tony Krowiak > Signed-off-by: Viktor Mihajlovski > Signed-off-by: Daniel Hansel > Reviewed-by: Boris Fiuczynski > Signed-off-by: Michal Privoznik > --- > src/conf/domain_conf.c | 156 > +++ > src/conf/domain_conf.h | 17 ++ > src/libvirt_private.syms | 2 + > 3 files changed, 175 insertions(+) > ACK after squashing it together with the previous patch. Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/4] libvirt: docs: XML to enable/disable protected key mgmt ops
On Fri, May 15, 2015 at 04:43:27PM +0200, Michal Privoznik wrote: > From: Tony Krowiak > > Two new domain configuration XML elements have been added to enable/disable They haven't been added yet :) This should be squashed in with the patch implementing XML parsing and formatting of the attributes. > the protected key management operations for a guest: > > > ... > > > > ... > > > Signed-off-by: Tony Krowiak > Signed-off-by: Viktor Mihajlovski > Reviewed-by: Boris Fiuczynski > Signed-off-by: Michal Privoznik > --- > docs/formatdomain.html.in | 37 + > docs/schemas/domaincommon.rng | 24 > 2 files changed, 61 insertions(+) > > diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in > index e0b6ba7..db3c81c 100644 > --- a/docs/formatdomain.html.in > +++ b/docs/formatdomain.html.in > @@ -6227,6 +6227,43 @@ qemu-kvm -net nic,model=? /dev/null >being on a file system that lacks security labeling. > > > +Key Wrap Is the shape attribute needed here? We don't use it for other 'a name's. > + > + The content of the optional keywrap element specifies > +whether the guest will be allowed to perform the S390 cryptographic > key > +management operations. A clear key can be protected by encrypting it > +under a unique wrapping key that is generated for each guest VM > running > +on the host. Two variations of wrapping keys are generated: one > version > +for encrypting protected keys using the DEA/TDEA algorithm, and > another > +version for keys encrypted using the AES algorithm. If a > +keywrap element is not included, the guest will be > granted > +access to both AES and DEA/TDEA key wrapping by default. > + > + Same question about this attribute. > +> + ... > + > + > +At least one cipher element must be nested within the > +keywrap element. > +cipher > +The name attribute identifies the algorithm > +for encrypting a protected key. The values supported for this > attribute > +are aes for encryption under the AES wrapping key, or > +dea for encryption under the DEA/TDEA wrapping key. The > +state attribute indicates whether the cryptographic key > +management operations should be turned on for the specified > encryption > +algorithm. The value can be set to on or > off. > +A default state of on will be assumed if a > +cipher element is not included for the AES or DEA/TDEA > +encryption algorithm. > + > + > +Note: DEA/TDEA is synonymous with DES/TDES. > Example configs > > > diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng > index c151e92..1e67776 100644 > --- a/docs/schemas/domaincommon.rng > +++ b/docs/schemas/domaincommon.rng > @@ -67,6 +67,9 @@ > > > > + > + > + > > > > @@ -382,6 +385,27 @@ > > > > + > + > + > + > + > + > + aes > + dea > + > + > + > + > +on > +off > + can be used here > + > + > + > + > + > + >> + > + The / needs to be before the tag name. > + ... > +
Re: [libvirt] [PATCH] netdev: assure that SRIOV PF is online before modifying VF params
On 15.05.2015 17:13, Laine Stump wrote: > On 05/15/2015 05:35 AM, Michal Privoznik wrote: >> On 14.05.2015 21:38, Laine Stump wrote: >>> The kernel won't complain if you set the mac address and vlan tag for >>> an SRIOV VF via its PF, and it will even let you assign the PF to a >>> guest using PCI device assignment or macvtap passthrough. But if the >>> PF isn't online, the device won't be usable in the guest. This patch >>> makes sure that it is turned on. >>> >>> Since multiple guests/VFs could use the same PF, there is no point in >>> ever setting the PF *off*line. >>> >>> This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=893738 >>> >>> Originally filed against RHEL6, but present in every version of >>> libvirt until today. >>> --- >>> src/util/virnetdev.c | 11 +++ >>> 1 file changed, 11 insertions(+) >>> >>> diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c >>> index e14b401..7022dfa 100644 >>> --- a/src/util/virnetdev.c >>> +++ b/src/util/virnetdev.c >>> @@ -2258,6 +2258,17 @@ virNetDevReplaceVfConfig(const char *pflinkdev, int >>> vf, >>> char macstr[VIR_MAC_STRING_BUFLEN]; >>> char *fileData = NULL; >>> int ifindex = -1; >>> +bool pfIsOnline; >>> + >>> +/* Assure that PF is online prior to twiddling with the VF. It >>> + * *should* be, but if the PF isn't online the changes made to the >>> + * VF via the PF won't take effect, yet there will be no error >>> + * reported. >>> + */ >>> +if (virNetDevGetOnline(pflinkdev, &pfIsOnline) < 0) >>> +return ret; >>> +if (!pfIsOnline && virNetDevSetOnline(pflinkdev, true) < 0) >>> +return ret; >>> >>> if (virNetDevGetVfConfig(pflinkdev, vf, &oldmac, &oldvlanid) < 0) >>> goto cleanup; >>> >> ACK. Should we set the device back to its previous state if something >> goes wrong later in the function? > > I don't think so. The PF needs to be on in order for any VF to work > properly, and we can't assume that nobody else has started trying to use > it since we set it online. (this is similar to what we do with the > ip_forward setting in the network driver). > > I guess an alternative would be to *never* set the PF online in libvirt, > but instead to log an error an fail; still better than silently failing > to pass traffic. Any opinions on which is the better approach? I think the better is if libvirt sets the PF online. So my ACK to the patch still holds. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 0/9] qemu: fix device alias usage, ide controllers,
On Thu, May 14, 2015 at 03:36:20PM -0400, Laine Stump wrote: > V1 was here: > > http://www.redhat.com/archives/libvir-list/2015-May/msg00124.html > > This started out with the intent to generate an error/failure on > request for an IDE controller on a machinetype that doesn't support > IDE (currently anything except 440fx-based machinetypes), or a 2nd IDE > controller on a machinetype that only supports one (i.e. 440fx). This > led to a few other related fixes, and some "fixes related to the > related fixes". > > I've pushed some of the ACKed patches, and dropped the two patches > that changed qemuBuildDeviceAddress and qemuAssignDeviceControllerAlias > to use switches instead of "if .. else if .." (03/13 and 04/13) and > dropped the two SCSI patches (12/13 and 13/13), but left a couple of > ACKed patches in for clarity. > > Detailed differences from V1 in each patch. > > Laine Stump (9): > conf: utility to return alias of a controller based on type/index > qemu: fix exceptions in qemuAssignDeviceControllerAlias > qemu: use controller alias when constructing device/controller args > qemu: use alias to refer to non-multibus PCI controller > qemu: use controller's alias in commandline for scsi-generic > qemu: use USB controller's alias instead of qemuUSBId() > qemu: remove test for allowing ide controller in s390, rename usb > tests > qemu: clean up qemuBuildCommandline loop that builds controller args > qemu: log error when domain has an unsupported IDE controller > ... > 21 files changed, 226 insertions(+), 163 deletions(-) ACK series. Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 8/9] qemu: clean up qemuBuildCommandline loop that builds controller args
On Thu, May 14, 2015 at 03:36:28PM -0400, Laine Stump wrote:
> Reorganize the loop that builds controller args to remove unnecessary
> duplicated code and superfluous else clauses. No functional change
> (this was split out from the following patch to make review easier).
This note can be dropped from git history.
> ---
>
> New in V2 - this was a part of patch 11/13 in V1. jtomko requested
> that I separate out the cleanup from the bugfix, so this is the
> cleanup of existing code, and the bugfix will be in the next patch.
Thanks, it's much easier to follow now.
>
> src/qemu/qemu_command.c | 81
> -
> 1 file changed, 39 insertions(+), 42 deletions(-)
>
>
> -/* Only recent QEMU implements a SATA (AHCI) controller */
> -if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_SATA) {
> -if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_ICH9_AHCI)) {
> -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> - _("SATA is not supported with this "
> - "QEMU binary"));
> -goto error;
> -} else if (cont->idx == 0 &&
> qemuDomainMachineIsQ35(def)) {
> -/* first SATA controller on Q35 machines is implicit
> */
> +/* first SATA controller on Q35 machines is implicit */
> +if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_SATA &&
> +cont->idx == 0 && qemuDomainMachineIsQ35(def))
> continue;
Unless intended, this is indented one level too many.
> -} else {
> -char *devstr;
> -
> -virCommandAddArg(cmd, "-device");
> -if (!(devstr = qemuBuildControllerDevStr(def, cont,
> - qemuCaps,
> NULL)))
> -goto error;
>
> -virCommandAddArg(cmd, devstr);
> -VIR_FREE(devstr);
> -}
Jan
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] netdev: assure that SRIOV PF is online before modifying VF params
On 05/15/2015 05:35 AM, Michal Privoznik wrote: > On 14.05.2015 21:38, Laine Stump wrote: >> The kernel won't complain if you set the mac address and vlan tag for >> an SRIOV VF via its PF, and it will even let you assign the PF to a >> guest using PCI device assignment or macvtap passthrough. But if the >> PF isn't online, the device won't be usable in the guest. This patch >> makes sure that it is turned on. >> >> Since multiple guests/VFs could use the same PF, there is no point in >> ever setting the PF *off*line. >> >> This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=893738 >> >> Originally filed against RHEL6, but present in every version of >> libvirt until today. >> --- >> src/util/virnetdev.c | 11 +++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c >> index e14b401..7022dfa 100644 >> --- a/src/util/virnetdev.c >> +++ b/src/util/virnetdev.c >> @@ -2258,6 +2258,17 @@ virNetDevReplaceVfConfig(const char *pflinkdev, int >> vf, >> char macstr[VIR_MAC_STRING_BUFLEN]; >> char *fileData = NULL; >> int ifindex = -1; >> +bool pfIsOnline; >> + >> +/* Assure that PF is online prior to twiddling with the VF. It >> + * *should* be, but if the PF isn't online the changes made to the >> + * VF via the PF won't take effect, yet there will be no error >> + * reported. >> + */ >> +if (virNetDevGetOnline(pflinkdev, &pfIsOnline) < 0) >> +return ret; >> +if (!pfIsOnline && virNetDevSetOnline(pflinkdev, true) < 0) >> +return ret; >> >> if (virNetDevGetVfConfig(pflinkdev, vf, &oldmac, &oldvlanid) < 0) >> goto cleanup; >> > ACK. Should we set the device back to its previous state if something > goes wrong later in the function? I don't think so. The PF needs to be on in order for any VF to work properly, and we can't assume that nobody else has started trying to use it since we set it online. (this is similar to what we do with the ip_forward setting in the network driver). I guess an alternative would be to *never* set the PF online in libvirt, but instead to log an error an fail; still better than silently failing to pass traffic. Any opinions on which is the better approach? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/6] nodedev: change if-else if in update_caps to switch
Makes it nicer as update bits are added for different cap types.
---
src/node_device/node_device_driver.c | 27 +++
1 file changed, 23 insertions(+), 4 deletions(-)
diff --git a/src/node_device/node_device_driver.c
b/src/node_device/node_device_driver.c
index a14191c..a6b32fe 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -50,12 +50,31 @@ static int update_caps(virNodeDeviceObjPtr dev)
virNodeDevCapsDefPtr cap = dev->def->caps;
while (cap) {
-if (cap->data.type == VIR_NODE_DEV_CAP_SCSI_HOST)
+switch (cap->data.type) {
+case VIR_NODE_DEV_CAP_SCSI_HOST:
detect_scsi_host_caps(&dev->def->caps->data);
-if (cap->data.type == VIR_NODE_DEV_CAP_NET &&
-virNetDevGetLinkInfo(cap->data.net.ifname, &cap->data.net.lnk) < 0)
-return -1;
+break;
+case VIR_NODE_DEV_CAP_NET:
+if (virNetDevGetLinkInfo(cap->data.net.ifname, &cap->data.net.lnk)
< 0)
+return -1;
+break;
+/* all types that (supposedly) don't require any updates
+ * relative to what's in the cache.
+ */
+case VIR_NODE_DEV_CAP_SYSTEM:
+case VIR_NODE_DEV_CAP_PCI_DEV:
+case VIR_NODE_DEV_CAP_USB_DEV:
+case VIR_NODE_DEV_CAP_USB_INTERFACE:
+case VIR_NODE_DEV_CAP_SCSI_TARGET:
+case VIR_NODE_DEV_CAP_SCSI:
+case VIR_NODE_DEV_CAP_STORAGE:
+case VIR_NODE_DEV_CAP_FC_HOST:
+case VIR_NODE_DEV_CAP_VPORTS:
+case VIR_NODE_DEV_CAP_SCSI_GENERIC:
+case VIR_NODE_DEV_CAP_LAST:
+break;
+}
cap = cap->next;
}
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/6] node device: prepare node_device_linux_sysfs.c to add more functions
This file contains only a single function, detect_scsi_host_caps(),
which is declared in node_device_driver.h and called from both the hal
and udev backends. Other things common to the hal and udev drivers
can be placed in that file though. As a prelude to adding further
functions, this patch renames the existing function to something
closer in line with other internal libvirt function names
(nodeDeviceSysfsGetSCSIHostCaps()), and puts the declarations into a
separate .h file.
---
src/Makefile.am | 5 +++--
src/node_device/node_device_driver.c | 7 ---
src/node_device/node_device_driver.h | 2 --
src/node_device/node_device_hal.c | 7 ---
src/node_device/node_device_linux_sysfs.c | 7 ---
src/node_device/node_device_linux_sysfs.h | 30 ++
src/node_device/node_device_udev.c| 9 +
7 files changed, 50 insertions(+), 17 deletions(-)
create mode 100644 src/node_device/node_device_linux_sysfs.h
diff --git a/src/Makefile.am b/src/Makefile.am
index d28a8ed..579421d 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1,6 +1,6 @@
## Process this file with automake to produce Makefile.in
-## Copyright (C) 2005-2014 Red Hat, Inc.
+## Copyright (C) 2005-2015 Red Hat, Inc.
##
## This library is free software; you can redistribute it and/or
## modify it under the terms of the GNU Lesser General Public
@@ -966,7 +966,8 @@ ACCESS_DRIVER_POLKIT_POLICY = \
NODE_DEVICE_DRIVER_SOURCES = \
node_device/node_device_driver.c\
node_device/node_device_driver.h\
- node_device/node_device_linux_sysfs.c
+ node_device/node_device_linux_sysfs.c \
+ node_device/node_device_linux_sysfs.h
NODE_DEVICE_DRIVER_HAL_SOURCES = \
node_device/node_device_hal.c \
diff --git a/src/node_device/node_device_driver.c
b/src/node_device/node_device_driver.c
index a6b32fe..c9db00a 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -35,8 +35,9 @@
#include "virfile.h"
#include "virstring.h"
#include "node_device_conf.h"
-#include "node_device_hal.h"
#include "node_device_driver.h"
+#include "node_device_hal.h"
+#include "node_device_linux_sysfs.h"
#include "virutil.h"
#include "viraccessapicheck.h"
#include "virnetdev.h"
@@ -52,7 +53,7 @@ static int update_caps(virNodeDeviceObjPtr dev)
while (cap) {
switch (cap->data.type) {
case VIR_NODE_DEV_CAP_SCSI_HOST:
-detect_scsi_host_caps(&dev->def->caps->data);
+nodeDeviceSysfsGetSCSIHostCaps(&dev->def->caps->data);
break;
case VIR_NODE_DEV_CAP_NET:
if (virNetDevGetLinkInfo(cap->data.net.ifname, &cap->data.net.lnk)
< 0)
@@ -282,7 +283,7 @@ nodeDeviceLookupSCSIHostByWWN(virConnectPtr conn,
while (cap) {
if (cap->data.type == VIR_NODE_DEV_CAP_SCSI_HOST) {
-detect_scsi_host_caps(&cap->data);
+nodeDeviceSysfsGetSCSIHostCaps(&cap->data);
if (cap->data.scsi_host.flags &
VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) {
if (STREQ(cap->data.scsi_host.wwnn, wwnn) &&
diff --git a/src/node_device/node_device_driver.h
b/src/node_device/node_device_driver.h
index 81bca98..0f4ea57 100644
--- a/src/node_device/node_device_driver.h
+++ b/src/node_device/node_device_driver.h
@@ -44,8 +44,6 @@ extern virNodeDeviceDriverStatePtr driver;
int nodedevRegister(void);
-int detect_scsi_host_caps(virNodeDevCapDataPtr d);
-
int nodeNumOfDevices(virConnectPtr conn, const char *cap, unsigned int flags);
int nodeListDevices(virConnectPtr conn, const char *cap, char **const names,
int maxnames, unsigned int flags);
diff --git a/src/node_device/node_device_hal.c
b/src/node_device/node_device_hal.c
index 38e09c4..2379e88 100644
--- a/src/node_device/node_device_hal.c
+++ b/src/node_device/node_device_hal.c
@@ -1,7 +1,7 @@
/*
* node_device_hal.c: node device enumeration - HAL-based implementation
*
- * Copyright (C) 2011-2014 Red Hat, Inc.
+ * Copyright (C) 2011-2015 Red Hat, Inc.
* Copyright (C) 2008 Virtual Iron Software, Inc.
* Copyright (C) 2008 David F. Lively
*
@@ -29,7 +29,9 @@
#include
#include "node_device_conf.h"
+#include "node_device_driver.h"
#include "node_device_hal.h"
+#include "node_device_linux_sysfs.h"
#include "virerror.h"
#include "driver.h"
#include "datatypes.h"
@@ -37,7 +39,6 @@
#include "viruuid.h"
#include "virpci.h"
#include "virlog.h"
-#include "node_device_driver.h"
#include "virdbus.h"
#include "virstring.h"
@@ -248,7 +249,7 @@ gather_scsi_host_cap(LibHalContext *ctx, const char *udi,
(void)get_int_prop(ctx, udi, "scsi_host.host", (int *)&d->scsi_host.host);
-retval = detect_scsi_host_caps
[libvirt] [PATCH 6/6] node_device: replace duplicated code in hal and udev backends
Both the hal and udev drivers call virPCI*() functions to the the
SRIOV VF/PF info about PCI devices, and the UDEV backend calls
virPCI*() to get IOMMU group info. Since there is now a single
function call in node_device_linux_sysfs.c to do all of this, replace
all that code in the two backends with calls to
nodeDeviceSysfsGetPCIRelatedDevCaps().
Note that this results in the HAL driver (probably) unnecessarily
calling virPCIDevieAddressGetIOMMUGroupNum(), but in the case that the
host doesn't support IOMMU groups, that function turns into a NOP (it
returns -2, which causes the caller to skip the call to
virPCIDeviceAddressGetIOMMUGroupAddresses()). So in the worst case it
is a few extra cycles spent, and in the best case a mythical platform
that supported IOMMU groups but used HAL rather than UDEV would gain
proper reporting of IOMMU group info.
---
src/node_device/node_device_hal.c | 12 +---
src/node_device/node_device_udev.c | 31 ++-
2 files changed, 3 insertions(+), 40 deletions(-)
diff --git a/src/node_device/node_device_hal.c
b/src/node_device/node_device_hal.c
index 2379e88..2d3bc17 100644
--- a/src/node_device/node_device_hal.c
+++ b/src/node_device/node_device_hal.c
@@ -153,20 +153,10 @@ gather_pci_cap(LibHalContext *ctx, const char *udi,
ignore_value(virStrToLong_ui(p+1, &p, 16, &d->pci_dev.function));
}
-if (!virPCIGetPhysicalFunction(sysfs_path,
- &d->pci_dev.physical_function))
-d->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION;
-
-int ret = virPCIGetVirtualFunctions(sysfs_path,
-&d->pci_dev.virtual_functions,
-&d->pci_dev.num_virtual_functions);
-if (ret < 0) {
+if (nodeDeviceSysfsGetPCIRelatedDevCaps(sysfs_path, d) < 0) {
VIR_FREE(sysfs_path);
return -1;
}
-
-if (d->pci_dev.num_virtual_functions > 0)
-d->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
VIR_FREE(sysfs_path);
}
diff --git a/src/node_device/node_device_udev.c
b/src/node_device/node_device_udev.c
index 29da036..100b44d 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -413,11 +413,10 @@ static int udevProcessPCI(struct udev_device *device,
{
const char *syspath = NULL;
virNodeDevCapDataPtr data = &def->caps->data;
-virPCIDeviceAddress addr;
virPCIEDeviceInfoPtr pci_express = NULL;
virPCIDevicePtr pciDev = NULL;
udevPrivate *priv = driver->privateData;
-int tmpGroup, ret = -1;
+int ret = -1;
char *p;
int rc;
@@ -496,34 +495,8 @@ static int udevProcessPCI(struct udev_device *device,
data->pci_dev.numa_node = -1;
}
-if (!virPCIGetPhysicalFunction(syspath, &data->pci_dev.physical_function))
-data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION;
-
-rc = virPCIGetVirtualFunctions(syspath,
- &data->pci_dev.virtual_functions,
- &data->pci_dev.num_virtual_functions);
-/* Out of memory */
-if (rc < 0)
-goto out;
-else if (!rc && (data->pci_dev.num_virtual_functions > 0))
-data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
-
-/* iommu group */
-addr.domain = data->pci_dev.domain;
-addr.bus = data->pci_dev.bus;
-addr.slot = data->pci_dev.slot;
-addr.function = data->pci_dev.function;
-tmpGroup = virPCIDeviceAddressGetIOMMUGroupNum(&addr);
-if (tmpGroup == -1) {
-/* error was already reported */
+if (nodeDeviceSysfsGetPCIRelatedDevCaps(syspath, data) < 0)
goto out;
-/* -2 return means there is no iommu_group data */
-} else if (tmpGroup >= 0) {
-if (virPCIDeviceAddressGetIOMMUGroupAddresses(&addr,
&data->pci_dev.iommuGroupDevices,
-
&data->pci_dev.nIommuGroupDevices) < 0)
-goto out;
-data->pci_dev.iommuGroupNumber = tmpGroup;
-}
if (!(pciDev = virPCIDeviceNew(data->pci_dev.domain,
data->pci_dev.bus,
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/6] conf: make virNodeDevCapData an official type
For some reason a union (_virNodeDevCapData) that had only been
declared inside the toplevel struct virNodeDevCapsDef was being used
as an argument to functions all over the place. Since it was only a
union, the "type" attribute wasn't necessarily sent with it. While
this works, it just seems wrong.
This patch creates a toplevel typedef for virNodeDevCapData and
virNodeDevCapDataPtr, making it a struct that has the type attribute
as a member, along with an anonymous union of everything that used to
be in union _virNodeDevCapData. This way we only have to change the
following:
s/union _virNodeDevCapData */virNodeDevCapDataPtr /
and
s/caps->type/caps->data.type/
This will make me feel less guilty when adding functions that need a
pointer to one of these.
---
src/conf/node_device_conf.c | 50 +++
src/conf/node_device_conf.h | 16 ++
src/libxl/libxl_driver.c | 4 +--
src/node_device/node_device_driver.c | 14 -
src/node_device/node_device_driver.h | 2 +-
src/node_device/node_device_hal.c | 20 ++---
src/node_device/node_device_linux_sysfs.c | 6 ++--
src/node_device/node_device_udev.c| 30 +--
src/qemu/qemu_driver.c| 2 +-
src/test/test_driver.c| 6 ++--
src/xen/xen_driver.c | 4 +--
11 files changed, 79 insertions(+), 75 deletions(-)
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index feae3d4..e6f3f27 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -1,7 +1,7 @@
/*
* node_device_conf.c: config handling for node devices
*
- * Copyright (C) 2009-2014 Red Hat, Inc.
+ * Copyright (C) 2009-2015 Red Hat, Inc.
* Copyright (C) 2008 Virtual Iron Software, Inc.
* Copyright (C) 2008 David F. Lively
*
@@ -76,9 +76,9 @@ int virNodeDeviceHasCap(const virNodeDeviceObj *dev, const
char *cap)
{
virNodeDevCapsDefPtr caps = dev->def->caps;
while (caps) {
-if (STREQ(cap, virNodeDevCapTypeToString(caps->type)))
+if (STREQ(cap, virNodeDevCapTypeToString(caps->data.type)))
return 1;
-else if (caps->type == VIR_NODE_DEV_CAP_SCSI_HOST)
+else if (caps->data.type == VIR_NODE_DEV_CAP_SCSI_HOST)
if ((STREQ(cap, "fc_host") &&
(caps->data.scsi_host.flags &
VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST)) ||
@@ -285,12 +285,12 @@ char *virNodeDeviceDefFormat(const virNodeDeviceDef *def)
for (caps = def->caps; caps; caps = caps->next) {
char uuidstr[VIR_UUID_STRING_BUFLEN];
-union _virNodeDevCapData *data = &caps->data;
+virNodeDevCapDataPtr data = &caps->data;
virBufferAsprintf(&buf, "\n",
- virNodeDevCapTypeToString(caps->type));
+ virNodeDevCapTypeToString(caps->data.type));
virBufferAdjustIndent(&buf, 2);
-switch (caps->type) {
+switch (caps->data.type) {
case VIR_NODE_DEV_CAP_SYSTEM:
if (data->system.product_name)
virBufferEscapeString(&buf, "%s\n",
@@ -661,7 +661,7 @@ static int
virNodeDevCapStorageParseXML(xmlXPathContextPtr ctxt,
virNodeDeviceDefPtr def,
xmlNodePtr node,
- union _virNodeDevCapData *data)
+ virNodeDevCapDataPtr data)
{
xmlNodePtr orignode, *nodes = NULL;
size_t i;
@@ -755,7 +755,7 @@ static int
virNodeDevCapSCSIParseXML(xmlXPathContextPtr ctxt,
virNodeDeviceDefPtr def,
xmlNodePtr node,
- union _virNodeDevCapData *data)
+ virNodeDevCapDataPtr data)
{
xmlNodePtr orignode;
int ret = -1;
@@ -800,7 +800,7 @@ static int
virNodeDevCapSCSITargetParseXML(xmlXPathContextPtr ctxt,
virNodeDeviceDefPtr def,
xmlNodePtr node,
-union _virNodeDevCapData *data)
+virNodeDevCapDataPtr data)
{
xmlNodePtr orignode;
int ret = -1;
@@ -828,7 +828,7 @@ static int
virNodeDevCapSCSIHostParseXML(xmlXPathContextPtr ctxt,
virNodeDeviceDefPtr def,
xmlNodePtr node,
- union _virNodeDevCapData *data,
+ virNodeDevCapDataPtr data,
int create,
const char *virt_type)
{
@@ -932,7 +932,7 @@ static int
virNodeDevCapNetParseXML(xmlXPathContextPtr ctxt,
virNodeDeviceDefPtr def,
xmlNodePtr node,
- union _virNodeDevCapData *data)
+ virNodeDevCapDataPtr data)
{
xmlNodePtr origno
[libvirt] [PATCH 0/6] node_device: update sriov/iommu info before dumpxml of a device
Patch 5/6 resolves this bug: https://bugzilla.redhat.com/show_bug.cgi?id=981546 (filed against RHEL7, but existing in every version of libvirt that supports reporting of SRIOV virtual function info via the NodeDevice APIs - 0.7.5, believe it or not). The rest of the series is there to make the bugfix less of a hack (and make it easier to fix future similar bugs). Laine Stump (6): conf: make virNodeDevCapData an official type nodedev: change if-else if in update_caps to switch node device: prepare node_device_linux_sysfs.c to add more functions node_device: new functions to get sriov/iommu info from sysfs node_device: update sriov/iommu info before dumpxml of a device node_device: replace duplicated code in hal and udev backends src/Makefile.am | 5 +- src/conf/node_device_conf.c | 50 src/conf/node_device_conf.h | 16 +++-- src/libxl/libxl_driver.c | 4 +- src/node_device/node_device_driver.c | 50 src/node_device/node_device_driver.h | 2 - src/node_device/node_device_hal.c | 39 +--- src/node_device/node_device_linux_sysfs.c | 99 +-- src/node_device/node_device_linux_sysfs.h | 32 ++ src/node_device/node_device_udev.c| 70 +++--- src/qemu/qemu_driver.c| 2 +- src/test/test_driver.c| 6 +- src/xen/xen_driver.c | 4 +- 13 files changed, 247 insertions(+), 132 deletions(-) create mode 100644 src/node_device/node_device_linux_sysfs.h -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/6] node_device: update sriov/iommu info before dumpxml of a device
Because reloading a PF driver with a different number of VFs doesn't result in any sort of event sent from udev to the libvirt node_device driver, libvirt's cache of that info can be out of date when a request arrives for the info about a device. To fix this, we refresh that data at the time of the dumpxml request, similar to what is already done for netdev link info and SCSI host capabilities. Since the same is true for iommu group information (for example, some other device in the same iommu group could have been detached from the host), we also create a function to update the iommu group info from sysfs, and a common function that does both. (a later patch will call this common function from the udev and hal backends). This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=981546 --- src/node_device/node_device_driver.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index c9db00a..34ba1fa 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -59,12 +59,16 @@ static int update_caps(virNodeDeviceObjPtr dev) if (virNetDevGetLinkInfo(cap->data.net.ifname, &cap->data.net.lnk) < 0) return -1; break; +case VIR_NODE_DEV_CAP_PCI_DEV: + if (nodeDeviceSysfsGetPCIRelatedDevCaps(dev->def->sysfs_path, + &dev->def->caps->data) < 0) + return -1; + break; /* all types that (supposedly) don't require any updates * relative to what's in the cache. */ case VIR_NODE_DEV_CAP_SYSTEM: -case VIR_NODE_DEV_CAP_PCI_DEV: case VIR_NODE_DEV_CAP_USB_DEV: case VIR_NODE_DEV_CAP_USB_INTERFACE: case VIR_NODE_DEV_CAP_SCSI_TARGET: -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/6] node_device: new functions to get sriov/iommu info from sysfs
The udev and hal drivers both already call the same functions as these
new functions added to node_device_linux_sysfs.c, but 1) we need to
call them from node_device_driver.c, and 2) it would be nice to
eliminate the duplicated code from the hal and udev backends.
---
src/node_device/node_device_linux_sysfs.c | 90 +++
src/node_device/node_device_linux_sysfs.h | 2 +
2 files changed, 92 insertions(+)
diff --git a/src/node_device/node_device_linux_sysfs.c
b/src/node_device/node_device_linux_sysfs.c
index fc82b32..6d5a406 100644
--- a/src/node_device/node_device_linux_sysfs.c
+++ b/src/node_device/node_device_linux_sysfs.c
@@ -137,6 +137,96 @@ nodeDeviceSysfsGetSCSIHostCaps(virNodeDevCapDataPtr d)
return ret;
}
+
+static int
+nodeDeviceSysfsGetPCISRIOVCaps(const char *sysfsPath,
+ virNodeDevCapDataPtr data)
+{
+size_t i;
+int ret;
+
+/* this could be a refresh, so clear out the old data */
+for (i = 0; i < data->pci_dev.num_virtual_functions; i++)
+ VIR_FREE(data->pci_dev.virtual_functions[i]);
+VIR_FREE(data->pci_dev.virtual_functions);
+data->pci_dev.num_virtual_functions = 0;
+data->pci_dev.flags &= ~VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
+data->pci_dev.flags &= ~VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION;
+
+if (!virPCIGetPhysicalFunction(sysfsPath,
&data->pci_dev.physical_function))
+data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION;
+
+ret = virPCIGetVirtualFunctions(sysfsPath,
&data->pci_dev.virtual_functions,
+&data->pci_dev.num_virtual_functions);
+if (ret < 0)
+return ret;
+
+if (data->pci_dev.num_virtual_functions > 0)
+data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
+
+return ret;
+}
+
+
+static int
+nodeDeviceSysfsGetPCIIOMMUGroupCaps(virNodeDevCapDataPtr data)
+{
+size_t i;
+int tmpGroup, ret = -1;
+virPCIDeviceAddress addr;
+
+/* this could be a refresh, so clear out the old data */
+for (i = 0; i < data->pci_dev.nIommuGroupDevices; i++)
+ VIR_FREE(data->pci_dev.iommuGroupDevices[i]);
+VIR_FREE(data->pci_dev.iommuGroupDevices);
+data->pci_dev.nIommuGroupDevices = 0;
+data->pci_dev.iommuGroupNumber = 0;
+
+addr.domain = data->pci_dev.domain;
+addr.bus = data->pci_dev.bus;
+addr.slot = data->pci_dev.slot;
+addr.function = data->pci_dev.function;
+tmpGroup = virPCIDeviceAddressGetIOMMUGroupNum(&addr);
+if (tmpGroup == -1) {
+/* error was already reported */
+goto cleanup;
+}
+if (tmpGroup == -2) {
+/* -2 return means there is no iommu_group data */
+ret = 0;
+goto cleanup;
+}
+if (tmpGroup >= 0) {
+if (virPCIDeviceAddressGetIOMMUGroupAddresses(&addr,
&data->pci_dev.iommuGroupDevices,
+
&data->pci_dev.nIommuGroupDevices) < 0)
+goto cleanup;
+data->pci_dev.iommuGroupNumber = tmpGroup;
+}
+
+ret = 0;
+ cleanup:
+return ret;
+}
+
+
+/* nodeDeviceSysfsGetPCIRelatedCaps() get info that is stored in sysfs
+ * about devices related to this device, i.e. things that can change
+ * without this device itself changing. These must be refreshed
+ * anytime full XML of the device is requested, because they can
+ * change with no corresponding notification from the kernel/udev.
+ */
+int
+nodeDeviceSysfsGetPCIRelatedDevCaps(const char *sysfsPath,
+virNodeDevCapDataPtr data)
+{
+if (nodeDeviceSysfsGetPCISRIOVCaps(sysfsPath, data) < 0)
+return -1;
+if (nodeDeviceSysfsGetPCIIOMMUGroupCaps(data) < 0)
+return -1;
+return 0;
+}
+
+
#else
int
diff --git a/src/node_device/node_device_linux_sysfs.h
b/src/node_device/node_device_linux_sysfs.h
index 307a8aa..e4afdd7 100644
--- a/src/node_device/node_device_linux_sysfs.h
+++ b/src/node_device/node_device_linux_sysfs.h
@@ -26,5 +26,7 @@
# include "node_device_conf.h"
int nodeDeviceSysfsGetSCSIHostCaps(virNodeDevCapDataPtr d);
+int nodeDeviceSysfsGetPCIRelatedDevCaps(const char *sysfsPath,
+virNodeDevCapDataPtr data);
#endif /* __VIR_NODE_DEVICE_LINUX_SYSFS_H__ */
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 0/4] Enable support for s390 crypto key mgmt operations
I've taken Tony's patches from here: https://www.redhat.com/archives/libvir-list/2015-April/msg01395.html polished them a bit, and resend. Tony Krowiak (4): libvirt: docs: XML to enable/disable protected key mgmt ops libvirt: conf: parse XML for protected key management ops libvirt: qemu: enable/disable protected key management ops libvirt: tests: test protected key mgmt ops support docs/formatdomain.html.in | 37 + docs/schemas/domaincommon.rng | 24 src/conf/domain_conf.c | 156 + src/conf/domain_conf.h | 17 +++ src/libvirt_private.syms | 2 + src/qemu/qemu_capabilities.c | 4 + src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_command.c| 73 ++ tests/qemuargv2xmltest.c | 6 + .../qemuxml2argv-machine-aeskeywrap-off-argv.args | 6 + .../qemuxml2argv-machine-aeskeywrap-off-argv.xml | 27 .../qemuxml2argv-machine-aeskeywrap-off-cap.args | 7 + .../qemuxml2argv-machine-aeskeywrap-off-cap.xml| 28 .../qemuxml2argv-machine-aeskeywrap-off-caps.args | 7 + .../qemuxml2argv-machine-aeskeywrap-off-caps.xml | 28 .../qemuxml2argv-machine-aeskeywrap-on-argv.args | 6 + .../qemuxml2argv-machine-aeskeywrap-on-argv.xml| 27 .../qemuxml2argv-machine-aeskeywrap-on-cap.args| 7 + .../qemuxml2argv-machine-aeskeywrap-on-cap.xml | 28 .../qemuxml2argv-machine-aeskeywrap-on-caps.args | 7 + .../qemuxml2argv-machine-aeskeywrap-on-caps.xml| 27 .../qemuxml2argv-machine-deakeywrap-off-argv.args | 6 + .../qemuxml2argv-machine-deakeywrap-off-argv.xml | 27 .../qemuxml2argv-machine-deakeywrap-off-cap.args | 7 + .../qemuxml2argv-machine-deakeywrap-off-cap.xml| 28 .../qemuxml2argv-machine-deakeywrap-off-caps.args | 7 + .../qemuxml2argv-machine-deakeywrap-off-caps.xml | 28 .../qemuxml2argv-machine-deakeywrap-on-argv.args | 6 + .../qemuxml2argv-machine-deakeywrap-on-argv.xml| 27 .../qemuxml2argv-machine-deakeywrap-on-cap.args| 7 + .../qemuxml2argv-machine-deakeywrap-on-cap.xml | 28 .../qemuxml2argv-machine-deakeywrap-on-caps.args | 7 + .../qemuxml2argv-machine-deakeywrap-on-caps.xml| 28 .../qemuxml2argv-machine-keywrap-none-argv.args| 6 + .../qemuxml2argv-machine-keywrap-none-argv.xml | 24 .../qemuxml2argv-machine-keywrap-none-caps.args| 7 + .../qemuxml2argv-machine-keywrap-none-caps.xml | 25 .../qemuxml2argv-machine-keywrap-none.args | 7 + .../qemuxml2argv-machine-keywrap-none.xml | 25 tests/qemuxml2argvtest.c | 81 +++ 40 files changed, 907 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-aeskeywrap-off-argv.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-aeskeywrap-off-argv.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-aeskeywrap-off-cap.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-aeskeywrap-off-cap.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-aeskeywrap-off-caps.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-aeskeywrap-off-caps.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-aeskeywrap-on-argv.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-aeskeywrap-on-argv.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-aeskeywrap-on-cap.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-aeskeywrap-on-cap.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-aeskeywrap-on-caps.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-aeskeywrap-on-caps.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-deakeywrap-off-argv.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-deakeywrap-off-argv.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-deakeywrap-off-cap.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-deakeywrap-off-cap.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-deakeywrap-off-caps.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-deakeywrap-off-caps.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-deakeywrap-on-argv.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-deakeywrap-on-argv.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-deakeywrap-on-cap.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-deakeywrap-on-cap.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-deakeywrap-on-caps.args create mode 100644 tests/qemu
[libvirt] [PATCH v2 4/4] libvirt: tests: test protected key mgmt ops support
From: Tony Krowiak Test the support for enabling/disabling CPACF protected key management operations for a guest. Signed-off-by: Tony Krowiak Signed-off-by: Viktor Mihajlovski Reviewed-by: Boris Fiuczynski Signed-off-by: Michal Privoznik --- tests/qemuargv2xmltest.c | 6 ++ .../qemuxml2argv-machine-aeskeywrap-off-argv.args | 6 ++ .../qemuxml2argv-machine-aeskeywrap-off-argv.xml | 27 .../qemuxml2argv-machine-aeskeywrap-off-cap.args | 7 ++ .../qemuxml2argv-machine-aeskeywrap-off-cap.xml| 28 .../qemuxml2argv-machine-aeskeywrap-off-caps.args | 7 ++ .../qemuxml2argv-machine-aeskeywrap-off-caps.xml | 28 .../qemuxml2argv-machine-aeskeywrap-on-argv.args | 6 ++ .../qemuxml2argv-machine-aeskeywrap-on-argv.xml| 27 .../qemuxml2argv-machine-aeskeywrap-on-cap.args| 7 ++ .../qemuxml2argv-machine-aeskeywrap-on-cap.xml | 28 .../qemuxml2argv-machine-aeskeywrap-on-caps.args | 7 ++ .../qemuxml2argv-machine-aeskeywrap-on-caps.xml| 27 .../qemuxml2argv-machine-deakeywrap-off-argv.args | 6 ++ .../qemuxml2argv-machine-deakeywrap-off-argv.xml | 27 .../qemuxml2argv-machine-deakeywrap-off-cap.args | 7 ++ .../qemuxml2argv-machine-deakeywrap-off-cap.xml| 28 .../qemuxml2argv-machine-deakeywrap-off-caps.args | 7 ++ .../qemuxml2argv-machine-deakeywrap-off-caps.xml | 28 .../qemuxml2argv-machine-deakeywrap-on-argv.args | 6 ++ .../qemuxml2argv-machine-deakeywrap-on-argv.xml| 27 .../qemuxml2argv-machine-deakeywrap-on-cap.args| 7 ++ .../qemuxml2argv-machine-deakeywrap-on-cap.xml | 28 .../qemuxml2argv-machine-deakeywrap-on-caps.args | 7 ++ .../qemuxml2argv-machine-deakeywrap-on-caps.xml| 28 .../qemuxml2argv-machine-keywrap-none-argv.args| 6 ++ .../qemuxml2argv-machine-keywrap-none-argv.xml | 24 +++ .../qemuxml2argv-machine-keywrap-none-caps.args| 7 ++ .../qemuxml2argv-machine-keywrap-none-caps.xml | 25 +++ .../qemuxml2argv-machine-keywrap-none.args | 7 ++ .../qemuxml2argv-machine-keywrap-none.xml | 25 +++ tests/qemuxml2argvtest.c | 81 ++ 32 files changed, 592 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-aeskeywrap-off-argv.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-aeskeywrap-off-argv.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-aeskeywrap-off-cap.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-aeskeywrap-off-cap.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-aeskeywrap-off-caps.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-aeskeywrap-off-caps.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-aeskeywrap-on-argv.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-aeskeywrap-on-argv.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-aeskeywrap-on-cap.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-aeskeywrap-on-cap.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-aeskeywrap-on-caps.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-aeskeywrap-on-caps.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-deakeywrap-off-argv.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-deakeywrap-off-argv.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-deakeywrap-off-cap.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-deakeywrap-off-cap.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-deakeywrap-off-caps.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-deakeywrap-off-caps.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-deakeywrap-on-argv.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-deakeywrap-on-argv.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-deakeywrap-on-cap.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-deakeywrap-on-cap.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-deakeywrap-on-caps.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-deakeywrap-on-caps.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-keywrap-none-argv.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-keywrap-none-argv.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-keywrap-none-caps.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-keywrap-none-caps.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-keywrap-none.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-keywrap-none.xml diff --git a/tests/qemuargv2xmltest.c b/tes
[libvirt] [PATCH v2 2/4] libvirt: conf: parse XML for protected key management ops
From: Tony Krowiak
Parse the domain configuration XML elements that enable/disable access to
the protected key management operations for a guest:
...
...
Signed-off-by: Tony Krowiak
Signed-off-by: Viktor Mihajlovski
Signed-off-by: Daniel Hansel
Reviewed-by: Boris Fiuczynski
Signed-off-by: Michal Privoznik
---
src/conf/domain_conf.c | 156 +++
src/conf/domain_conf.h | 17 ++
src/libvirt_private.syms | 2 +
3 files changed, 175 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f3b706e..ee8b474 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -477,6 +477,11 @@ VIR_ENUM_IMPL(virDomainSoundModel,
VIR_DOMAIN_SOUND_MODEL_LAST,
"ich9",
"usb")
+VIR_ENUM_IMPL(virDomainKeyWrapCipherName,
+ VIR_DOMAIN_KEY_WRAP_CIPHER_NAME_LAST,
+ "aes",
+ "dea")
+
VIR_ENUM_IMPL(virDomainMemballoonModel, VIR_DOMAIN_MEMBALLOON_MODEL_LAST,
"virtio",
"xen",
@@ -834,6 +839,131 @@ virDomainXMLOptionClassDispose(void *obj)
(xmlopt->config.privFree)(xmlopt->config.priv);
}
+/**
+ * virDomainKeyWrapCipherDefParseXML:
+ *
+ * @def Domain definition
+ * @node An XML cipher node
+ * @ctxt The XML context
+ *
+ * Parse the attributes from the cipher node and store the state
+ * attribute in @def.
+ *
+ * A cipher node has the form of
+ *
+ *
+ *
+ * Returns: 0 if the parse succeeded
+ * -1 otherwise
+ */
+static int
+virDomainKeyWrapCipherDefParseXML(virDomainKeyWrapDefPtr keywrap,
+ xmlNodePtr node,
+ xmlXPathContextPtr ctxt)
+{
+
+char *name = NULL;
+char *state = NULL;
+int state_type;
+int name_type;
+int ret = -1;
+xmlNodePtr oldnode = ctxt->node;
+
+ctxt->node = node;
+if (!(name = virXPathString("string(./@name)", ctxt))) {
+virReportError(VIR_ERR_CONF_SYNTAX, "%s",
+ _("missing name for cipher"));
+goto cleanup;
+}
+
+if ((name_type = virDomainKeyWrapCipherNameTypeFromString(name)) < 0) {
+virReportError(VIR_ERR_CONF_SYNTAX,
+ _("%s is not a supported cipher name"), name);
+goto cleanup;
+}
+
+if (!(state = virXPathString("string(./@state)", ctxt))) {
+virReportError(VIR_ERR_CONF_SYNTAX,
+ _("missing state for cipher named %s"), name);
+goto cleanup;
+}
+
+if ((state_type = virTristateSwitchTypeFromString(state)) < 0) {
+virReportError(VIR_ERR_CONF_SYNTAX,
+ _("%s is not a supported cipher state"), state);
+goto cleanup;
+}
+
+switch ((virDomainKeyWrapCipherName) name_type) {
+case VIR_DOMAIN_KEY_WRAP_CIPHER_NAME_AES:
+if (keywrap->aes != VIR_TRISTATE_SWITCH_ABSENT) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("A domain definition can have no more than "
+ "one cipher node with name %s"),
+ virDomainKeyWrapCipherNameTypeToString(name_type));
+
+goto cleanup;
+}
+keywrap->aes = state_type;
+break;
+
+case VIR_DOMAIN_KEY_WRAP_CIPHER_NAME_DEA:
+if (keywrap->dea != VIR_TRISTATE_SWITCH_ABSENT) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("A domain definition can have no more than "
+ "one cipher node with name %s"),
+ virDomainKeyWrapCipherNameTypeToString(name_type));
+
+goto cleanup;
+}
+keywrap->dea = state_type;
+break;
+
+case VIR_DOMAIN_KEY_WRAP_CIPHER_NAME_LAST:
+break;
+}
+
+ret = 0;
+
+ cleanup:
+VIR_FREE(name);
+VIR_FREE(state);
+ctxt->node = oldnode;
+return ret;
+}
+
+static int
+virDomainKeyWrapDefParseXML(virDomainDefPtr def, xmlXPathContextPtr ctxt)
+{
+size_t i;
+int ret = -1;
+xmlNodePtr *nodes = NULL;
+int n;
+
+if (!(n = virXPathNodeSet("./keywrap/cipher", ctxt, &nodes)))
+return 0;
+
+if (VIR_ALLOC(def->keywrap) < 0)
+goto cleanup;
+
+for (i = 0; i < n; i++) {
+if (virDomainKeyWrapCipherDefParseXML(def->keywrap, nodes[i], ctxt) <
0)
+goto cleanup;
+}
+
+if (!def->keywrap->aes &&
+!def->keywrap->dea)
+VIR_FREE(def->keywrap);
+
+ret = 0;
+
+ cleanup:
+if (ret < 0)
+VIR_FREE(def->keywrap);
+VIR_FREE(nodes);
+return ret;
+}
+
/**
* virDomainXMLOptionNew:
@@ -2361,6 +2491,8 @@ void virDomainDefFree(virDomainDefPtr def)
virDomainShmemDefFree(def->shmems[i]);
VIR_FREE(def->shmems);
+VIR_FREE(def->keywrap);
+
if (def->namespaceData && def->ns.free)
(def->ns.free)(def->namespaceData);
@@ -15535,6 +15667,9 @@ vir
[libvirt] [PATCH v2 3/4] libvirt: qemu: enable/disable protected key management ops
From: Tony Krowiak
Introduces two new -machine option parameters to the QEMU command to
enable/disable the CPACF protected key management operations for a guest:
aes-key-wrap='on|off'
dea-key-wrap='on|off'
The QEMU code maps the corresponding domain configuration elements to the
QEMU -machine option parameters to create the QEMU command:
--> aes-key-wrap=on
--> aes-key-wrap=off
--> dea-key-wrap=on
--> dea-key-wrap=off
Signed-off-by: Tony Krowiak
Signed-off-by: Daniel Hansel
Signed-off-by: Boris Fiuczynski
Reviewed-by: Boris Fiuczynski
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_capabilities.c | 4 +++
src/qemu/qemu_capabilities.h | 2 ++
src/qemu/qemu_command.c | 73
3 files changed, 79 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 25c15bf..2757636 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -281,6 +281,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
"pc-dimm",
"machine-vmport-opt", /* 185 */
+ "aes-key-wrap",
+ "dea-key-wrap",
);
@@ -2523,6 +2525,8 @@ static struct virQEMUCapsCommandLineProps
virQEMUCapsCommandLine[] = {
{ "msg", "timestamp", QEMU_CAPS_MSG_TIMESTAMP },
{ "numa", NULL, QEMU_CAPS_NUMA },
{ "drive", "throttling.bps-total-max", QEMU_CAPS_DRIVE_IOTUNE_MAX},
+{ "machine", "aes-key-wrap", QEMU_CAPS_AES_KEY_WRAP },
+{ "machine", "dea-key-wrap", QEMU_CAPS_DEA_KEY_WRAP },
};
static int
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 81557b7..4da9637 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -225,6 +225,8 @@ typedef enum {
QEMU_CAPS_QXL_VGA_VGAMEM = 183, /* -device qxl-vga.vgamem_mb */
QEMU_CAPS_DEVICE_PC_DIMM = 184, /* pc-dimm device */
QEMU_CAPS_MACHINE_VMPORT_OPT = 185, /* -machine xxx,vmport=on/off/auto */
+QEMU_CAPS_AES_KEY_WRAP = 186, /* -machine aes_key_wrap */
+QEMU_CAPS_DEA_KEY_WRAP = 187, /* -machine dea_key_wrap */
QEMU_CAPS_LAST, /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 2939f8d..98fc5f8 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -38,6 +38,7 @@
#include "virnetdevbridge.h"
#include "virstring.h"
#include "virtime.h"
+#include "virutil.h"
#include "viruuid.h"
#include "c-ctype.h"
#include "domain_nwfilter.h"
@@ -7286,6 +7287,39 @@ qemuBuildObsoleteAccelArg(virCommandPtr cmd,
return 0;
}
+static bool
+qemuAppendKeyWrapMachineParm(virBuffer *buf, virQEMUCapsPtr qemuCaps,
+ int flag, const char *pname, int pstate)
+{
+if (pstate != VIR_TRISTATE_SWITCH_ABSENT) {
+if (!virQEMUCapsGet(qemuCaps, flag)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("%s is not available with this QEMU binary"),
pname);
+return false;
+}
+
+virBufferAsprintf(buf, ",%s=%s", pname,
+ virTristateSwitchTypeToString(pstate));
+}
+
+return true;
+}
+
+static bool
+qemuAppendKeyWrapMachineParms(virBuffer *buf, virQEMUCapsPtr qemuCaps,
+ const virDomainKeyWrapDef *keywrap)
+{
+if (!qemuAppendKeyWrapMachineParm(buf, qemuCaps, QEMU_CAPS_AES_KEY_WRAP,
+ "aes-key-wrap", keywrap->aes))
+return false;
+
+if (!qemuAppendKeyWrapMachineParm(buf, qemuCaps, QEMU_CAPS_DEA_KEY_WRAP,
+ "dea-key-wrap", keywrap->dea))
+return false;
+
+return true;
+}
+
static int
qemuBuildMachineArgStr(virCommandPtr cmd,
const virDomainDef *def,
@@ -7320,6 +7354,13 @@ qemuBuildMachineArgStr(virCommandPtr cmd,
}
obsoleteAccel = true;
+
+if (def->keywrap) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("key wrap support is not available "
+ "with this QEMU binary"));
+return -1;
+}
} else {
virBuffer buf = VIR_BUFFER_INITIALIZER;
virTristateSwitch vmport = def->features[VIR_DOMAIN_FEATURE_VMPORT];
@@ -7378,6 +7419,12 @@ qemuBuildMachineArgStr(virCommandPtr cmd,
}
}
+if (def->keywrap &&
+!qemuAppendKeyWrapMachineParms(&buf, qemuCaps, def->keywrap)) {
+virBufferFreeAndReset(&buf);
+return -1;
+}
+
virCommandAddArgBuffer(cmd, &buf);
}
@@ -12806,6 +12853,32 @@ qemuParseCommandLine(virCapsPtr qemuCaps,
} else if (STRPREFIX(param, "accel=kvm")) {
def->virtType = VIR_DOMAIN_VIRT_KVM;
def->features[VIR_DOMAIN_FEATURE_PAE] =
VIR_TRISTATE_SW
[libvirt] [PATCH v2 1/4] libvirt: docs: XML to enable/disable protected key mgmt ops
From: Tony Krowiak Two new domain configuration XML elements have been added to enable/disable the protected key management operations for a guest: ... ... Signed-off-by: Tony Krowiak Signed-off-by: Viktor Mihajlovski Reviewed-by: Boris Fiuczynski Signed-off-by: Michal Privoznik --- docs/formatdomain.html.in | 37 + docs/schemas/domaincommon.rng | 24 2 files changed, 61 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index e0b6ba7..db3c81c 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -6227,6 +6227,43 @@ qemu-kvm -net nic,model=? /dev/null being on a file system that lacks security labeling. +Key Wrap + + The content of the optional keywrap element specifies +whether the guest will be allowed to perform the S390 cryptographic key +management operations. A clear key can be protected by encrypting it +under a unique wrapping key that is generated for each guest VM running +on the host. Two variations of wrapping keys are generated: one version +for encrypting protected keys using the DEA/TDEA algorithm, and another +version for keys encrypted using the AES algorithm. If a +keywrap element is not included, the guest will be granted +access to both AES and DEA/TDEA key wrapping by default. + + ++ ... + + +At least one cipher element must be nested within the +keywrap element. +cipher +The name attribute identifies the algorithm +for encrypting a protected key. The values supported for this attribute +are aes for encryption under the AES wrapping key, or +dea for encryption under the DEA/TDEA wrapping key. The +state attribute indicates whether the cryptographic key +management operations should be turned on for the specified encryption +algorithm. The value can be set to on or off. +A default state of on will be assumed if a +cipher element is not included for the AES or DEA/TDEA +encryption algorithm. + + +Note: DEA/TDEA is synonymous with DES/TDES. Example configs diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index c151e92..1e67776 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -67,6 +67,9 @@ + + + @@ -382,6 +385,27 @@ + + + + + + + aes + dea + + + + +on +off + + + + + + ++ + + ... +
Re: [libvirt] [PATCH 2/4] libvirt: conf: parse XML for protected key management ops
On 27.04.2015 23:57, akrow...@linux.vnet.ibm.com wrote: > From: Tony Krowiak > > Parse the domain configuration XML elements that enable/disable access to > the protected key management operations for a guest: > > > ... > > > > ... > > > Signed-off-by: Tony Krowiak > Signed-off-by: Viktor Mihajlovski > Signed-off-by: Daniel Hansel > Reviewed-by: Boris Fiuczynski > --- > src/conf/domain_conf.c | 189 > ++ > src/conf/domain_conf.h | 20 + > src/libvirt_private.syms |2 + > 3 files changed, 211 insertions(+), 0 deletions(-) Er. I'm just too lazy to point out all the bits. I'm gonna rework and post v2. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] libvirt: qemu: enable/disable protected key management ops
On 27.04.2015 23:57, akrow...@linux.vnet.ibm.com wrote:
> From: Tony Krowiak
>
> Introduces two new -machine option parameters to the QEMU command to
> enable/disable the CPACF protected key management operations for a guest:
>
> aes-key-wrap='on|off'
> dea-key-wrap='on|off'
>
> The QEMU code maps the corresponding domain configuration elements to the
> QEMU -machine option parameters to create the QEMU command:
>
>--> aes-key-wrap=on
> --> aes-key-wrap=off
>--> dea-key-wrap=on
> --> dea-key-wrap=off
>
> Signed-off-by: Tony Krowiak
> Signed-off-by: Daniel Hansel
> Signed-off-by: Boris Fiuczynski
> Reviewed-by: Boris Fiuczynski
> ---
> src/qemu/qemu_capabilities.c |5 +++
> src/qemu/qemu_capabilities.h |2 +
> src/qemu/qemu_command.c | 72
> ++
> src/qemu/qemu_domain.c | 39 ++-
> 4 files changed, 117 insertions(+), 1 deletions(-)
>
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index a458611..d1b9f6f 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -279,6 +279,9 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
>"qxl.vgamem_mb",
>"qxl-vga.vgamem_mb",
>"pc-dimm",
> +
> + "aes-key-wrap", /* 185 */
> + "dea-key-wrap",
> );
>
>
> @@ -2518,6 +2521,8 @@ static struct virQEMUCapsCommandLineProps
> virQEMUCapsCommandLine[] = {
> { "msg", "timestamp", QEMU_CAPS_MSG_TIMESTAMP },
> { "numa", NULL, QEMU_CAPS_NUMA },
> { "drive", "throttling.bps-total-max", QEMU_CAPS_DRIVE_IOTUNE_MAX},
> +{ "machine", "aes-key-wrap", QEMU_CAPS_AES_KEY_WRAP },
> +{ "machine", "dea-key-wrap", QEMU_CAPS_DEA_KEY_WRAP },
> };
>
> static int
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index c7b1ac7..31e0494 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -224,6 +224,8 @@ typedef enum {
> QEMU_CAPS_QXL_VGAMEM = 182, /* -device qxl.vgamem_mb */
> QEMU_CAPS_QXL_VGA_VGAMEM = 183, /* -device qxl-vga.vgamem_mb */
> QEMU_CAPS_DEVICE_PC_DIMM = 184, /* pc-dimm device */
> +QEMU_CAPS_AES_KEY_WRAP = 185, /* -machine aes_key_wrap */
> +QEMU_CAPS_DEA_KEY_WRAP = 186, /* -machine dea_key_wrap */
>
> QEMU_CAPS_LAST, /* this must always be the last item */
> } virQEMUCapsFlags;
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 247954f..8ff1d88 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -38,6 +38,7 @@
> #include "virnetdevbridge.h"
> #include "virstring.h"
> #include "virtime.h"
> +#include "virutil.h"
> #include "viruuid.h"
> #include "c-ctype.h"
> #include "domain_nwfilter.h"
> @@ -7295,6 +7296,39 @@ qemuBuildObsoleteAccelArg(virCommandPtr cmd,
> return 0;
> }
>
> +static bool
> +qemuAppendKeyWrapMachineParm(virBuffer *buf, virQEMUCapsPtr qemuCaps,
> + int flag, const char *pname, int pstate)
> +{
> +if (pstate != VIR_TRISTATE_SWITCH_ABSENT) {
> +if (!virQEMUCapsGet(qemuCaps, flag)) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("%s is not available with this QEMU binary"),
> pname);
> +return false;
> +}
> +
> +virBufferAsprintf(buf, ",%s=%s", pname,
> + virTristateSwitchTypeToString(pstate));
> +}
> +
> +return true;
> +}
> +
> +static bool
> +qemuAppendKeyWrapMachineParms(virBuffer *buf, virQEMUCapsPtr qemuCaps,
> + const virDomainDef *def)
> +{
> +if (!qemuAppendKeyWrapMachineParm(buf, qemuCaps, QEMU_CAPS_AES_KEY_WRAP,
> + "aes-key-wrap", def->keywrap.aes))
> +return false;
> +
> +if (!qemuAppendKeyWrapMachineParm(buf, qemuCaps, QEMU_CAPS_DEA_KEY_WRAP,
> + "dea-key-wrap", def->keywrap.dea))
> +return false;
> +
> +return true;
> +}
> +
> static int
> qemuBuildMachineArgStr(virCommandPtr cmd,
> const virDomainDef *def,
> @@ -7329,6 +7363,14 @@ qemuBuildMachineArgStr(virCommandPtr cmd,
> }
>
> obsoleteAccel = true;
> +
> +if ((def->keywrap.aes != VIR_TRISTATE_SWITCH_ABSENT) ||
> +(def->keywrap.dea != VIR_TRISTATE_SWITCH_ABSENT)) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("key wrap support is not available "
> + "with this QEMU binary"));
> +return -1;
> +}
> } else {
> virBuffer buf = VIR_BUFFER_INITIALIZER;
>
> @@ -7373,6 +7415,11 @@ qemuBuildMachineArgStr(virCommandPtr cmd,
> }
> }
>
> +if (!qemuAppendKeyWrapMachineParms(&buf, qemuCaps, def)) {
> +
Re: [libvirt] [PATCH] util: make it more robust to calculate timeout value
On Fri, May 15, 2015 at 01:09:09PM +0200, Michal Privoznik wrote:
> On 15.05.2015 08:26, zhang bo wrote:
> > When we change system clock to years ago, a certain CPU may use up 100%
> > cputime.
> > The reason is that in function virEventPollCalculateTimeout(), we assign
> > the
> > unsigned long long result to an INT variable,
> > *timeout = then - now; // timeout is INT, and then/now are long long
> > if (*timeout < 0)
> > *timeout = 0;
> > there's a chance that variable @then minus variable @now may be a very
> > large number
> > that overflows INT value expression, then *timeout will be negative and be
> > assigned to 0.
> > Next the 'poll' in function virEventPollRunOnce() will get into an
> > 'endless' while loop there.
> > thus, the cpu that virEventPollRunOnce() thread runs on will go up to 100%.
> >
> > Although as we discussed before in
> > https://www.redhat.com/archives/libvir-list/2015-May/msg00400.html
> > it should be prohibited to set-time while other applications are running,
> > but it does
> > seems to have no harm to make the codes more robust.
> >
> > Signed-off-by: Wang Yufei
> > Signed-off-by: Zhang Bo
> > ---
> > src/util/vireventpoll.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/util/vireventpoll.c b/src/util/vireventpoll.c
> > index ffda206..5f5a149 100644
> > --- a/src/util/vireventpoll.c
> > +++ b/src/util/vireventpoll.c
> > @@ -357,9 +357,10 @@ static int virEventPollCalculateTimeout(int *timeout)
> > return -1;
> >
> > EVENT_DEBUG("Schedule timeout then=%llu now=%llu", then, now);
> > -*timeout = then - now;
> > -if (*timeout < 0)
> > +if (then < now)
> > *timeout = 0;
> > +else
> > +*timeout = (then - now) & 0x7FFF;
>
> You're trying to make this an unsigned value. What's wrong with plain
> typecast?
>
> > } else {
> > *timeout = -1;
> > }
> >
>
> I must say this is ugly. If the system clock is changed, all the
> timeouts should fire, shouldn't they? Otherwise important events can be
> missed.
As I said in previous thread, I think that this is really just papering
over one specific issue, and you are still going to have a multitude of
problems with every app on the system when you change the system clock
in this kind of way. I'm just not convinced we should be trying to hack
around it in this one case, as it is just giving us a false illusion
that things are going to continue to work, when in reality they'll just
break somewhere else instead. eg the pthread_cond_wait() timeouts.
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: make it more robust to calculate timeout value
On 15.05.2015 08:26, zhang bo wrote:
> When we change system clock to years ago, a certain CPU may use up 100%
> cputime.
> The reason is that in function virEventPollCalculateTimeout(), we assign the
> unsigned long long result to an INT variable,
> *timeout = then - now; // timeout is INT, and then/now are long long
> if (*timeout < 0)
> *timeout = 0;
> there's a chance that variable @then minus variable @now may be a very large
> number
> that overflows INT value expression, then *timeout will be negative and be
> assigned to 0.
> Next the 'poll' in function virEventPollRunOnce() will get into an 'endless'
> while loop there.
> thus, the cpu that virEventPollRunOnce() thread runs on will go up to 100%.
>
> Although as we discussed before in
> https://www.redhat.com/archives/libvir-list/2015-May/msg00400.html
> it should be prohibited to set-time while other applications are running, but
> it does
> seems to have no harm to make the codes more robust.
>
> Signed-off-by: Wang Yufei
> Signed-off-by: Zhang Bo
> ---
> src/util/vireventpoll.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/src/util/vireventpoll.c b/src/util/vireventpoll.c
> index ffda206..5f5a149 100644
> --- a/src/util/vireventpoll.c
> +++ b/src/util/vireventpoll.c
> @@ -357,9 +357,10 @@ static int virEventPollCalculateTimeout(int *timeout)
> return -1;
>
> EVENT_DEBUG("Schedule timeout then=%llu now=%llu", then, now);
> -*timeout = then - now;
> -if (*timeout < 0)
> +if (then < now)
> *timeout = 0;
> +else
> +*timeout = (then - now) & 0x7FFF;
You're trying to make this an unsigned value. What's wrong with plain
typecast?
> } else {
> *timeout = -1;
> }
>
I must say this is ugly. If the system clock is changed, all the
timeouts should fire, shouldn't they? Otherwise important events can be
missed.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] qemu: Deal with panic device on pSeries.
The guest firmware provides the same functionality as the pvpanic
device, which is not available in QEMU on pSeries: make sure the
XML reflects this fact by automatically adding a element
when not already present.
On the other hand, unlike the pvpanic device, the guest firmware
can't be configured, so report an error if an address has been
provided in the XML.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1182388
---
src/qemu/qemu_command.c| 25 -
src/qemu/qemu_domain.c | 14 ++
.../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml | 1 +
.../qemuxml2argv-pseries-nvram.xml | 1 +
.../qemuxml2argv-pseries-panic-address.xml | 32 ++
.../qemuxml2argv-pseries-panic-missing.args| 7 +
.../qemuxml2argv-pseries-panic-missing.xml | 29
.../qemuxml2argv-pseries-panic-no-address.args | 7 +
.../qemuxml2argv-pseries-panic-no-address.xml | 30
tests/qemuxml2argvtest.c | 6
.../qemuxml2xmlout-pseries-panic-missing.xml | 30
tests/qemuxml2xmltest.c| 2 ++
12 files changed, 177 insertions(+), 7 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-address.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-missing.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-missing.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-no-address.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-no-address.xml
create mode 100644
tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 5d0a167..138a8b6 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -10737,13 +10737,28 @@ qemuBuildCommandLine(virConnectPtr conn,
}
if (def->panic) {
-if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PANIC)) {
+if (ARCH_IS_PPC64(def->os.arch) && STRPREFIX(def->os.machine,
"pseries")) {
+/* For pSeries guests, the firmware provides the same
+ * functionality of the pvpanic device. The address
+ * cannot be configured by the user */
+if (def->panic->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("setting the panic device address is not "
+ "supported for pSeries guests"));
+goto error;
+}
+} else {
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PANIC)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("your QEMU is too old to support pvpanic"));
+goto error;
+}
+
if (def->panic->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA) {
virCommandAddArg(cmd, "-device");
virCommandAddArgFormat(cmd, "pvpanic,ioport=%d",
def->panic->info.addr.isa.iobase);
-} else if (def->panic->info.type ==
- VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
+} else if (def->panic->info.type ==
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
virCommandAddArgList(cmd, "-device", "pvpanic", NULL);
} else {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -10751,10 +10766,6 @@ qemuBuildCommandLine(virConnectPtr conn,
"with ISA address type"));
goto error;
}
-} else {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("your QEMU is too old to support pvpanic"));
-goto error;
}
}
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index fa8229f..557b0b6 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -911,6 +911,7 @@ qemuDomainDefPostParse(virDomainDefPtr def,
bool addDefaultMemballoon = true;
bool addDefaultUSBKBD = false;
bool addDefaultUSBMouse = false;
+bool addPanicDevice = false;
if (def->os.bootloader || def->os.bootloaderArgs) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -963,6 +964,11 @@ qemuDomainDefPostParse(virDomainDefPtr def,
addPCIRoot = true;
addDefaultUSBKBD = true;
addDefaultUSBMouse = true;
+/* For pSeries guests, the firmware provides the same
+ * functionality of the pvpanic device, so automatically
+ * add the definition if not already present */
+if (STRPREFIX(def->os.machine, "pseries"))
+addPanicDevice = true;
break;
case VIR_ARCH_ALPHA:
@@ -1045,6 +1051
Re: [libvirt] [PATCH] netdev: assure that SRIOV PF is online before modifying VF params
On 14.05.2015 21:38, Laine Stump wrote: > The kernel won't complain if you set the mac address and vlan tag for > an SRIOV VF via its PF, and it will even let you assign the PF to a > guest using PCI device assignment or macvtap passthrough. But if the > PF isn't online, the device won't be usable in the guest. This patch > makes sure that it is turned on. > > Since multiple guests/VFs could use the same PF, there is no point in > ever setting the PF *off*line. > > This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=893738 > > Originally filed against RHEL6, but present in every version of > libvirt until today. > --- > src/util/virnetdev.c | 11 +++ > 1 file changed, 11 insertions(+) > > diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c > index e14b401..7022dfa 100644 > --- a/src/util/virnetdev.c > +++ b/src/util/virnetdev.c > @@ -2258,6 +2258,17 @@ virNetDevReplaceVfConfig(const char *pflinkdev, int vf, > char macstr[VIR_MAC_STRING_BUFLEN]; > char *fileData = NULL; > int ifindex = -1; > +bool pfIsOnline; > + > +/* Assure that PF is online prior to twiddling with the VF. It > + * *should* be, but if the PF isn't online the changes made to the > + * VF via the PF won't take effect, yet there will be no error > + * reported. > + */ > +if (virNetDevGetOnline(pflinkdev, &pfIsOnline) < 0) > +return ret; > +if (!pfIsOnline && virNetDevSetOnline(pflinkdev, true) < 0) > +return ret; > > if (virNetDevGetVfConfig(pflinkdev, vf, &oldmac, &oldvlanid) < 0) > goto cleanup; > ACK. Should we set the device back to its previous state if something goes wrong later in the function? Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: Ignore panic device on pSeries.
On Sun, 2015-05-10 at 17:50 -0400, Cole Robinson wrote: > is about a specific device though, which qemu ppc doesn't > support. Even if the firmware provides the logical feature (getting PANICKED > notifications from qemu), it doesn't support the device or any explicit qemu > CLI config, so IMO it's correct to reject . Apps/users will just have > to take that into account, along with all the other XML differences for x86 vs > ppc64. > > An alternative could be the unconditionally add to ppc64 XML since the > logical feature is always available... but you'd probably have to invent a new > scheme or something > > Instead I think it's just a documentation patch. I really like the alternative approach you suggested: it's the same feature after all, so the XML should be the same regardless of how it's actually implemented for the specific machine type. I've reworked my patch accordingly, I'm going to send a v2 in a few minutes. Please let me know what you think of it. > Another thing from that bug: The error message 'your QEMU is too old to > support pvpanic' isn't accurate for non-x86, so it's probably better to change > it to 'this QEMU binary does not support pvpanic' or similar, maybe look at > existing error messages to find a better example. I agree. I feel it belongs to a different commit, though. > FWIW, since this is hypervisor specific, this type of stuff shouldn't be in > the (intended to be) generic domain_conf.c. Check out > qemu_domain.c:qemuDomainDefPostParse instead My mistake. Thanks both to you and Daniel for pointing that out. Cheers. -- Andrea Bolognani -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
