[libvirt] [PATCH] tpm: adapt sysfs cancel path for new TPM driver
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. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/util/virtpm.c | 21 +++++++++++++++++++-- .../qemuxml2argv-tpm-passthrough.args | 2 +- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/util/virtpm.c b/src/util/virtpm.c index 88f8361..40a758b 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,27 @@ virTPMCreateCancelPath(const char *devpath) { char *path = NULL; const char *dev; + const char *prefix[] = {"misc/", "tpm/"}; + int i, 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/qemuxml2argvdata/qemuxml2argv-tpm-passthrough.args b/tests/qemuxml2argvdata/qemuxml2argv-tpm-passthrough.args index f33120f..1e48603 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-tpm-passthrough.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-tpm-passthrough.args @@ -17,6 +17,6 @@ QEMU_AUDIO_DRV=none \ -boot c \ -usb \ -tpmdev passthrough,id=tpm-tpm0,path=/dev/tpm0,\ -cancel-path=/sys/class/misc/tpm0/device/cancel \ +cancel-path=/dev/null \ -device tpm-tis,tpmdev=tpm-tpm0,id=tpm0 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 -- 2.4.3
On 11/16/2015 10:12 PM, 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.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/util/virtpm.c | 21 +++++++++++++++++++-- .../qemuxml2argv-tpm-passthrough.args | 2 +- 2 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/src/util/virtpm.c b/src/util/virtpm.c index 88f8361..40a758b 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,27 @@ virTPMCreateCancelPath(const char *devpath) { char *path = NULL; const char *dev; + const char *prefix[] = {"misc/", "tpm/"}; + int i, 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);
To get that path right we need to test it's availability by opening it. Unfortunately there's problem with that now related to the test case below where, if the test is run as non-root, /dev/null will be returned, and if run as root possibly one of those constructed paths will be returned. What's the best way to fix this ? Regards, Stefan
+ 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/qemuxml2argvdata/qemuxml2argv-tpm-passthrough.args b/tests/qemuxml2argvdata/qemuxml2argv-tpm-passthrough.args index f33120f..1e48603 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-tpm-passthrough.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-tpm-passthrough.args @@ -17,6 +17,6 @@ QEMU_AUDIO_DRV=none \ -boot c \ -usb \ -tpmdev passthrough,id=tpm-tpm0,path=/dev/tpm0,\ -cancel-path=/sys/class/misc/tpm0/device/cancel \ +cancel-path=/dev/null \ -device tpm-tis,tpmdev=tpm-tpm0,id=tpm0 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
On Tue, Nov 17, 2015 at 09:18:29AM -0500, Stefan Berger wrote:
On 11/16/2015 10:12 PM, 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.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/util/virtpm.c | 21 +++++++++++++++++++-- .../qemuxml2argv-tpm-passthrough.args | 2 +- 2 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/src/util/virtpm.c b/src/util/virtpm.c index 88f8361..40a758b 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,27 @@ virTPMCreateCancelPath(const char *devpath) { char *path = NULL; const char *dev; + const char *prefix[] = {"misc/", "tpm/"}; + int i, 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);
To get that path right we need to test it's availability by opening it. Unfortunately there's problem with that now related to the test case below where, if the test is run as non-root, /dev/null will be returned, and if run as root possibly one of those constructed paths will be returned. What's the best way to fix this ?
The unit tests should never depend on host state, so we should not run this code at all. You sould mock override the virTPMCreateCancelPath method entirely I guess to return a fixed filename. 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 :|
On 11/17/2015 09:24 AM, Daniel P. Berrange wrote:
On Tue, Nov 17, 2015 at 09:18:29AM -0500, Stefan Berger wrote:
To get that path right we need to test it's availability by opening it. Unfortunately there's problem with that now related to the test case below where, if the test is run as non-root, /dev/null will be returned, and if run as root possibly one of those constructed paths will be returned. What's the best way to fix this ?
The unit tests should never depend on host state, so we should not run this code at all.
You sould mock override the virTPMCreateCancelPath method entirely I guess to return a fixed filename.
Great. Would you accept this code here in the mock? + +char * +virTPMCreateCancelPath(const char *devpath) +{ + const char *dev = "/sys/class/misc/tpm0/device/cancel"; + size_t len = strlen(dev); + char *path = malloc(len + 1); + + (void)devpath; + + if (path) + memcpy(path, dev, len + 1); + + return path; +} Stefan
Regards, Daniel
On Tue, Nov 17, 2015 at 10:24:51AM -0500, Stefan Berger wrote:
On 11/17/2015 09:24 AM, Daniel P. Berrange wrote:
On Tue, Nov 17, 2015 at 09:18:29AM -0500, Stefan Berger wrote:
To get that path right we need to test it's availability by opening it. Unfortunately there's problem with that now related to the test case below where, if the test is run as non-root, /dev/null will be returned, and if run as root possibly one of those constructed paths will be returned. What's the best way to fix this ?
The unit tests should never depend on host state, so we should not run this code at all.
You sould mock override the virTPMCreateCancelPath method entirely I guess to return a fixed filename.
Great. Would you accept this code here in the mock?
+ +char * +virTPMCreateCancelPath(const char *devpath) +{ + const char *dev = "/sys/class/misc/tpm0/device/cancel"; + size_t len = strlen(dev); + char *path = malloc(len + 1); + + (void)devpath; + + if (path) + memcpy(path, dev, len + 1); + + return path; +}
What's wrong with 'return strdup(dev)' rather than strlen+malloc+memcpy ? 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 :|
participants (3)
-
Daniel P. Berrange -
Stefan Berger -
Stefan Berger