Cole Robinson <crobinso@redhat.com> wrote on 11/17/2015 04:37:56 PM:

> From: Cole Robinson <crobinso@redhat.com>
> To: Stefan Berger/Watson/IBM@IBMUS, libvir-list@redhat.com,
> berrange@redhat.com
> Cc: Stefan Berger <stefanb@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@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