9:46 a.m.
Adapt the sysfs TPM command cancel path for the TPM driver that
does not use a miscdevice anymore since Linux 4.0. Support old
and new paths and check their availability.
Add a mockup for the test cases to avoid the testing for
availability of the cancel path.
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
src/util/virtpm.c | 22 ++++++++++++++++++++--
tests/qemuxml2argvmock.c | 15 +++++++++++++++
2 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/src/util/virtpm.c b/src/util/virtpm.c
index 88f8361..3872a31 100644
--- a/src/util/virtpm.c
+++ b/src/util/virtpm.c
@@ -23,9 +23,12 @@
#include <config.h>
#include <sys/stat.h>
+#include <fcntl.h>
#include "virstring.h"
#include "virerror.h"
+#include "viralloc.h"
+#include "virfile.h"
#include "virtpm.h"
#define VIR_FROM_THIS VIR_FROM_NONE
@@ -41,13 +44,28 @@ virTPMCreateCancelPath(const char *devpath)
{
char *path = NULL;
const char *dev;
+ const char *prefix[] = {"misc/", "tpm/"};
+ size_t i;
+ int fd;
if (devpath) {
dev = strrchr(devpath, '/');
if (dev) {
dev++;
- if (virAsprintf(&path, "/sys/class/misc/%s/device/cancel",
- dev) < 0)
+ for (i = 0; i < ARRAY_CARDINALITY(prefix); i++) {
+ if (virAsprintf(&path, "/sys/class/%s%s/device/cancel",
+ prefix[i], dev) < 0)
+ goto cleanup;
+
+ fd = open(path, O_WRONLY);
+ if (fd >= 0) {
+ VIR_FORCE_CLOSE(fd);
+ break;
+ }
+ VIR_FREE(path);
+ }
+ /* /dev/null does not allow to cancel cmds but it can be used */
+ if (!path && virAsprintf(&path, "/dev/null") < 0)
goto cleanup;
} else {
virReportError(VIR_ERR_INTERNAL_ERROR,
diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c
index d24725e..e58b8ce 100644
--- a/tests/qemuxml2argvmock.c
+++ b/tests/qemuxml2argvmock.c
@@ -24,9 +24,13 @@
#include "virnuma.h"
#include "virmock.h"
#include "virutil.h"
+#include "virstring.h"
+#include "virtpm.h"
#include <time.h>
#include <unistd.h>
+#define VIR_FROM_THIS VIR_FROM_NONE
+
long virGetSystemPageSize(void)
{
return 4096;
@@ -59,3 +63,14 @@ virNumaNodeIsAvailable(int node)
return node >= 0 && node <= virNumaGetMaxNode();
}
#endif /* WITH_NUMACTL && HAVE_NUMA_BITMASK_ISBITSET */
+
+char *
+virTPMCreateCancelPath(const char *devpath)
+{
+ char *path;
+ (void)devpath;
+
+ ignore_value(VIR_STRDUP(path, "/sys/class/misc/tpm0/device/cancel"));
+
+ return path;
+}
--
2.4.3
1:12 p.m.
First, thanks for following up on the fedora bugs so quickly!
On 11/17/2015 10:46 AM, Stefan Berger wrote:
Adapt the sysfs TPM command cancel path for the TPM driver that
does not use a miscdevice anymore since Linux 4.0. Support old
and new paths and check their availability.
Add a mockup for the test cases to avoid the testing for
availability of the cancel path.
I see you sent a qemu patch for a similar change. What's the benefit of
libvirt setting cancel_path if qemu can (and already attempts to) figure it out?
- Cole
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
src/util/virtpm.c | 22 ++++++++++++++++++++--
tests/qemuxml2argvmock.c | 15 +++++++++++++++
2 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/src/util/virtpm.c b/src/util/virtpm.c
index 88f8361..3872a31 100644
--- a/src/util/virtpm.c
+++ b/src/util/virtpm.c
@@ -23,9 +23,12 @@
#include <config.h>
#include <sys/stat.h>
+#include <fcntl.h>
#include "virstring.h"
#include "virerror.h"
+#include "viralloc.h"
+#include "virfile.h"
#include "virtpm.h"
#define VIR_FROM_THIS VIR_FROM_NONE
@@ -41,13 +44,28 @@ virTPMCreateCancelPath(const char *devpath)
{
char *path = NULL;
const char *dev;
+ const char *prefix[] = {"misc/", "tpm/"};
+ size_t i;
+ int fd;
if (devpath) {
dev = strrchr(devpath, '/');
if (dev) {
dev++;
- if (virAsprintf(&path, "/sys/class/misc/%s/device/cancel",
- dev) < 0)
+ for (i = 0; i < ARRAY_CARDINALITY(prefix); i++) {
+ if (virAsprintf(&path, "/sys/class/%s%s/device/cancel",
+ prefix[i], dev) < 0)
+ goto cleanup;
+
+ fd = open(path, O_WRONLY);
+ if (fd >= 0) {
+ VIR_FORCE_CLOSE(fd);
+ break;
+ }
+ VIR_FREE(path);
+ }
+ /* /dev/null does not allow to cancel cmds but it can be used */
+ if (!path && virAsprintf(&path, "/dev/null") < 0)
goto cleanup;
} else {
virReportError(VIR_ERR_INTERNAL_ERROR,
diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c
index d24725e..e58b8ce 100644
--- a/tests/qemuxml2argvmock.c
+++ b/tests/qemuxml2argvmock.c
@@ -24,9 +24,13 @@
#include "virnuma.h"
#include "virmock.h"
#include "virutil.h"
+#include "virstring.h"
+#include "virtpm.h"
#include <time.h>
#include <unistd.h>
+#define VIR_FROM_THIS VIR_FROM_NONE
+
long virGetSystemPageSize(void)
{
return 4096;
@@ -59,3 +63,14 @@ virNumaNodeIsAvailable(int node)
return node >= 0 && node <= virNumaGetMaxNode();
}
#endif /* WITH_NUMACTL && HAVE_NUMA_BITMASK_ISBITSET */
+
+char *
+virTPMCreateCancelPath(const char *devpath)
+{
+ char *path;
+ (void)devpath;
+
+ ignore_value(VIR_STRDUP(path, "/sys/class/misc/tpm0/device/cancel"));
+
+ return path;
+}
2:49 p.m.
On 11/17/2015 02:12 PM, Cole Robinson wrote:
First, thanks for following up on the fedora bugs so quickly!
On 11/17/2015 10:46 AM, Stefan Berger wrote:
> Adapt the sysfs TPM command cancel path for the TPM driver that
> does not use a miscdevice anymore since Linux 4.0. Support old
> and new paths and check their availability.
>
> Add a mockup for the test cases to avoid the testing for
> availability of the cancel path.
>
I see you sent a qemu patch for a similar change. What's the benefit of
libvirt setting cancel_path if qemu can (and already attempts to) figure it out?
The only benefit is that libvirt is controlling the parameter.
Stefan
2:55 p.m.
On 11/17/2015 03:49 PM, Stefan Berger wrote:
On 11/17/2015 02:12 PM, Cole Robinson wrote:
> First, thanks for following up on the fedora bugs so quickly!
>
> On 11/17/2015 10:46 AM, Stefan Berger wrote:
>> Adapt the sysfs TPM command cancel path for the TPM driver that
>> does not use a miscdevice anymore since Linux 4.0. Support old
>> and new paths and check their availability.
>>
>> Add a mockup for the test cases to avoid the testing for
>> availability of the cancel path.
>>
> I see you sent a qemu patch for a similar change. What's the benefit of
> libvirt setting cancel_path if qemu can (and already attempts to) figure it
> out?
The only benefit is that libvirt is controlling the parameter.
Then IMO we should let qemu handle it and just drop this path handling
entirely. If there's ever a need to differentiate from qemu's logic or let the
user override this value then we can add it back without much trouble
Thanks,
Cole
3:26 p.m.
Cole Robinson <crobinso(a)redhat.com> wrote on 11/17/2015 03:55:13 PM:
From: Cole Robinson <crobinso(a)redhat.com>
To: Stefan Berger <stefanb(a)linux.vnet.ibm.com>, Stefan Berger/
Watson/IBM@IBMUS, libvir-list(a)redhat.com, berrange(a)redhat.com
Date: 11/17/2015 03:55 PM
Subject: Re: [PATCH v2] tpm: adapt sysfs cancel path for new TPM driver
On 11/17/2015 03:49 PM, Stefan Berger wrote:
> On 11/17/2015 02:12 PM, Cole Robinson wrote:
>> First, thanks for following up on the fedora bugs so quickly!
>>
>> On 11/17/2015 10:46 AM, Stefan Berger wrote:
>>> Adapt the sysfs TPM command cancel path for the TPM driver that
>>> does not use a miscdevice anymore since Linux 4.0. Support old
>>> and new paths and check their availability.
>>>
>>> Add a mockup for the test cases to avoid the testing for
>>> availability of the cancel path.
>>>
>> I see you sent a qemu patch for a similar change. What's the benefit
of
>> libvirt setting cancel_path if qemu can (and already
attempts
to)figure it
>> out?
>
> The only benefit is that libvirt is controlling the parameter.
>
Then IMO we should let qemu handle it and just drop this path handling
entirely. If there's ever a need to differentiate from qemu's logic or
let
the
user override this value then we can add it back without much
trouble
Forgot about that: SELinux labeling of that file requires it afaik.
Stefan
3:37 p.m.
On 11/17/2015 10:46 AM, Stefan Berger wrote:
Adapt the sysfs TPM command cancel path for the TPM driver that
does not use a miscdevice anymore since Linux 4.0. Support old
and new paths and check their availability.
Add a mockup for the test cases to avoid the testing for
availability of the cancel path.
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
src/util/virtpm.c | 22 ++++++++++++++++++++--
tests/qemuxml2argvmock.c | 15 +++++++++++++++
2 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/src/util/virtpm.c b/src/util/virtpm.c
index 88f8361..3872a31 100644
--- a/src/util/virtpm.c
+++ b/src/util/virtpm.c
@@ -23,9 +23,12 @@
#include <config.h>
#include <sys/stat.h>
+#include <fcntl.h>
#include "virstring.h"
#include "virerror.h"
+#include "viralloc.h"
+#include "virfile.h"
#include "virtpm.h"
#define VIR_FROM_THIS VIR_FROM_NONE
@@ -41,13 +44,28 @@ virTPMCreateCancelPath(const char *devpath)
{
char *path = NULL;
const char *dev;
+ const char *prefix[] = {"misc/", "tpm/"};
+ size_t i;
+ int fd;
if (devpath) {
dev = strrchr(devpath, '/');
if (dev) {
dev++;
- if (virAsprintf(&path, "/sys/class/misc/%s/device/cancel",
- dev) < 0)
+ for (i = 0; i < ARRAY_CARDINALITY(prefix); i++) {
+ if (virAsprintf(&path, "/sys/class/%s%s/device/cancel",
+ prefix[i], dev) < 0)
+ goto cleanup;
+
+ fd = open(path, O_WRONLY);
+ if (fd >= 0) {
+ VIR_FORCE_CLOSE(fd);
+ break;
+ }
+ VIR_FREE(path);
+ }
You can use virFileExists(path) to simplify this
+ /* /dev/null does not allow to cancel cmds but it can be
used */
+ if (!path && virAsprintf(&path, "/dev/null") < 0)
goto cleanup;
} else {
Hmm, does this mean we might selinux relabel /dev/null? Maybe just fall back
to no cancel-path if we want this to be non-fatal
- Cole
virReportError(VIR_ERR_INTERNAL_ERROR,
diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c
index d24725e..e58b8ce 100644
--- a/tests/qemuxml2argvmock.c
+++ b/tests/qemuxml2argvmock.c
@@ -24,9 +24,13 @@
#include "virnuma.h"
#include "virmock.h"
#include "virutil.h"
+#include "virstring.h"
+#include "virtpm.h"
#include <time.h>
#include <unistd.h>
+#define VIR_FROM_THIS VIR_FROM_NONE
+
long virGetSystemPageSize(void)
{
return 4096;
@@ -59,3 +63,14 @@ virNumaNodeIsAvailable(int node)
return node >= 0 && node <= virNumaGetMaxNode();
}
#endif /* WITH_NUMACTL && HAVE_NUMA_BITMASK_ISBITSET */
+
+char *
+virTPMCreateCancelPath(const char *devpath)
+{
+ char *path;
+ (void)devpath;
+
+ ignore_value(VIR_STRDUP(path, "/sys/class/misc/tpm0/device/cancel"));
+
+ return path;
+}
3:59 p.m.
Cole Robinson <crobinso(a)redhat.com> wrote on 11/17/2015 04:37:56 PM:
From: Cole Robinson <crobinso(a)redhat.com>
To: Stefan Berger/Watson/IBM@IBMUS, libvir-list(a)redhat.com,
berrange(a)redhat.com
Cc: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
Date: 11/17/2015 04:38 PM
Subject: Re: [PATCH v2] tpm: adapt sysfs cancel path for new TPM driver
On 11/17/2015 10:46 AM, Stefan Berger wrote:
> Adapt the sysfs TPM command cancel path for the TPM driver that
> does not use a miscdevice anymore since Linux 4.0. Support old
> and new paths and check their availability.
>
> Add a mockup for the test cases to avoid the testing for
> availability of the cancel path.
>
> Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
> ---
> src/util/virtpm.c | 22 ++++++++++++++++++++--
> tests/qemuxml2argvmock.c | 15 +++++++++++++++
> 2 files changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/src/util/virtpm.c b/src/util/virtpm.c
> index 88f8361..3872a31 100644
> --- a/src/util/virtpm.c
> +++ b/src/util/virtpm.c
> @@ -23,9 +23,12 @@
> #include <config.h>
>
> #include <sys/stat.h>
> +#include <fcntl.h>
>
> #include "virstring.h"
> #include "virerror.h"
> +#include "viralloc.h"
> +#include "virfile.h"
> #include "virtpm.h"
>
> #define VIR_FROM_THIS VIR_FROM_NONE
> @@ -41,13 +44,28 @@ virTPMCreateCancelPath(const char *devpath)
> {
> char *path = NULL;
> const char *dev;
> + const char *prefix[] = {"misc/", "tpm/"};
> + size_t i;
> + int fd;
>
> if (devpath) {
> dev = strrchr(devpath, '/');
> if (dev) {
> dev++;
> - if (virAsprintf(&path,
"/sys/class/misc/%s/device/cancel",
> - dev) < 0)
> + for (i = 0; i < ARRAY_CARDINALITY(prefix); i++) {
> + if (virAsprintf(&path,
"/sys/class/%s%s/device/cancel",
> + prefix[i], dev) < 0)
> + goto cleanup;
> +
> + fd = open(path, O_WRONLY);
> + if (fd >= 0) {
> + VIR_FORCE_CLOSE(fd);
> + break;
> + }
> + VIR_FREE(path);
> + }
You can use virFileExists(path) to simplify this
Right.
> + /* /dev/null does not allow to cancel cmds but it canbe
used */
> + if (!path && virAsprintf(&path,
"/dev/null") < 0)
> goto cleanup;
> } else {
Hmm, does this mean we might selinux relabel /dev/null? Maybe just fall
back
to no cancel-path if we want this to be non-fatal
We have to pass something on the command line otherwise QEMU will try to
find a path and SELinux will not allow it to open the file. Basically
either one of these sysfs files has to be there.
I'll resort to displaying an error instead.
Regards,
Stefan
3285
days inactive
3285
days old
6 comments
3 participants
participants (3)
-
Cole Robinson -
Stefan Berger -
Stefan Berger