1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH 0/2] Don't report a VFIO error for mdev when IOMMU is disabled
  4. View patch

[libvirt] [PATCH 1/2] util:mdev: Improve the error msg on non-existent mdev prior to VM start

Erik Skultety posted 2 patches 7 years, 1 month ago
[libvirt] [PATCH 1/2] util:mdev: Improve the error msg on non-existent mdev prior to VM start
Posted by Erik Skultety 7 years, 1 month ago 
What one currently gets is:
failed to read '/sys/bus/mdev/devices/<UUID>/mdev_type/device_api': No
such file or directory

This indicates that something is missing within the device's sysfs tree
which likely might be not be the case here because the device simply
doesn't exist yet. So, when creating our internal mdev obj, let's check
whether the device exists first prior to trying to verify the
user-provided model within domain XML.

Signed-off-by: Erik Skultety <eskultet@redhat.com>
---
 src/util/virmdev.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/src/util/virmdev.c b/src/util/virmdev.c
index e4816cf20..2e3769aa6 100644
--- a/src/util/virmdev.c
+++ b/src/util/virmdev.c
@@ -150,13 +150,22 @@ virMediatedDeviceNew(const char *uuidstr, virMediatedDeviceModelType model)
 {
     virMediatedDevicePtr ret = NULL;
     virMediatedDevicePtr dev = NULL;
+    char *sysfspath = NULL;
+
+    if (!(sysfspath = virMediatedDeviceGetSysfsPath(uuidstr)))
+        goto cleanup;
+
+    if (!virFileExists(sysfspath)) {
+        virReportSystemError(errno, _("failed to read device '%s'"),
+                             sysfspath);
+        goto cleanup;
+    }
 
     if (VIR_ALLOC(dev) < 0)
-        return NULL;
-
-    if (!(dev->path = virMediatedDeviceGetSysfsPath(uuidstr)))
         goto cleanup;
 
+    VIR_STEAL_PTR(dev->path, sysfspath);
+
     /* Check whether the user-provided model corresponds with the actually
      * supported mediated device's API.
      */
@@ -167,6 +176,7 @@ virMediatedDeviceNew(const char *uuidstr, virMediatedDeviceModelType model)
     VIR_STEAL_PTR(ret, dev);
 
  cleanup:
+    VIR_FREE(sysfspath);
     virMediatedDeviceFree(dev);
     return ret;
 }
-- 
2.13.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] util:mdev: Improve the error msg on non-existent mdev prior to VM start
Posted by Peter Krempa 7 years, 1 month ago 
missing space in summary

On Fri, Mar 16, 2018 at 11:50:01 +0100, Erik Skultety wrote:
> What one currently gets is:
> failed to read '/sys/bus/mdev/devices/<UUID>/mdev_type/device_api': No
> such file or directory
> 
> This indicates that something is missing within the device's sysfs tree
> which likely might be not be the case here because the device simply
> doesn't exist yet. So, when creating our internal mdev obj, let's check
> whether the device exists first prior to trying to verify the
> user-provided model within domain XML.
> 
> Signed-off-by: Erik Skultety <eskultet@redhat.com>
> ---
>  src/util/virmdev.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/src/util/virmdev.c b/src/util/virmdev.c
> index e4816cf20..2e3769aa6 100644
> --- a/src/util/virmdev.c
> +++ b/src/util/virmdev.c
> @@ -150,13 +150,22 @@ virMediatedDeviceNew(const char *uuidstr, virMediatedDeviceModelType model)
>  {
>      virMediatedDevicePtr ret = NULL;
>      virMediatedDevicePtr dev = NULL;
> +    char *sysfspath = NULL;
> +
> +    if (!(sysfspath = virMediatedDeviceGetSysfsPath(uuidstr)))
> +        goto cleanup;
> +
> +    if (!virFileExists(sysfspath)) {
> +        virReportSystemError(errno, _("failed to read device '%s'"),
> +                             sysfspath);

You did not try to read the device at this point. It merely does not
exist.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] util:mdev: Improve the error msg on non-existent mdev prior to VM start
Posted by Erik Skultety 7 years, 1 month ago 
On Fri, Mar 16, 2018 at 12:08:46PM +0100, Peter Krempa wrote:
> missing space in summary
>
> On Fri, Mar 16, 2018 at 11:50:01 +0100, Erik Skultety wrote:
> > What one currently gets is:
> > failed to read '/sys/bus/mdev/devices/<UUID>/mdev_type/device_api': No
> > such file or directory
> >
> > This indicates that something is missing within the device's sysfs tree
> > which likely might be not be the case here because the device simply
> > doesn't exist yet. So, when creating our internal mdev obj, let's check
> > whether the device exists first prior to trying to verify the
> > user-provided model within domain XML.
> >
> > Signed-off-by: Erik Skultety <eskultet@redhat.com>
> > ---
> >  src/util/virmdev.c | 16 +++++++++++++---
> >  1 file changed, 13 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/util/virmdev.c b/src/util/virmdev.c
> > index e4816cf20..2e3769aa6 100644
> > --- a/src/util/virmdev.c
> > +++ b/src/util/virmdev.c
> > @@ -150,13 +150,22 @@ virMediatedDeviceNew(const char *uuidstr, virMediatedDeviceModelType model)
> >  {
> >      virMediatedDevicePtr ret = NULL;
> >      virMediatedDevicePtr dev = NULL;
> > +    char *sysfspath = NULL;
> > +
> > +    if (!(sysfspath = virMediatedDeviceGetSysfsPath(uuidstr)))
> > +        goto cleanup;
> > +
> > +    if (!virFileExists(sysfspath)) {
> > +        virReportSystemError(errno, _("failed to read device '%s'"),
> > +                             sysfspath);
>
> You did not try to read the device at this point. It merely does not
> exist.

Would you like "device '%s' not found" better?

Erik

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] util:mdev: Improve the error msg on non-existent mdev prior to VM start
Posted by Peter Krempa 7 years, 1 month ago 
On Fri, Mar 16, 2018 at 13:13:41 +0100, Erik Skultety wrote:
> On Fri, Mar 16, 2018 at 12:08:46PM +0100, Peter Krempa wrote:
> > missing space in summary
> >
> > On Fri, Mar 16, 2018 at 11:50:01 +0100, Erik Skultety wrote:
> > > What one currently gets is:
> > > failed to read '/sys/bus/mdev/devices/<UUID>/mdev_type/device_api': No
> > > such file or directory
> > >
> > > This indicates that something is missing within the device's sysfs tree
> > > which likely might be not be the case here because the device simply
> > > doesn't exist yet. So, when creating our internal mdev obj, let's check
> > > whether the device exists first prior to trying to verify the
> > > user-provided model within domain XML.
> > >
> > > Signed-off-by: Erik Skultety <eskultet@redhat.com>
> > > ---
> > >  src/util/virmdev.c | 16 +++++++++++++---
> > >  1 file changed, 13 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/src/util/virmdev.c b/src/util/virmdev.c
> > > index e4816cf20..2e3769aa6 100644
> > > --- a/src/util/virmdev.c
> > > +++ b/src/util/virmdev.c
> > > @@ -150,13 +150,22 @@ virMediatedDeviceNew(const char *uuidstr, virMediatedDeviceModelType model)
> > >  {
> > >      virMediatedDevicePtr ret = NULL;
> > >      virMediatedDevicePtr dev = NULL;
> > > +    char *sysfspath = NULL;
> > > +
> > > +    if (!(sysfspath = virMediatedDeviceGetSysfsPath(uuidstr)))
> > > +        goto cleanup;
> > > +
> > > +    if (!virFileExists(sysfspath)) {
> > > +        virReportSystemError(errno, _("failed to read device '%s'"),
> > > +                             sysfspath);
> >
> > You did not try to read the device at this point. It merely does not
> > exist.
> 
> Would you like "device '%s' not found" better?

How about:

"mediated device '%s' not found", uuidstr ?

I don't think that adding the full sysfs path is any helpful since it's
generated from the uuid anyways.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-8c64711

© 2016 - 2025 Red Hat, Inc.