1. Patchew
  2. Libvirt
  3. View series

[libvirt] [PATCH] util: Fix broken MinGW builds caused by commit 9bc01ad8

Erik Skultety posted 1 patch 4 years, 9 months ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/bf89139ebbdec514813f5b6aa967a39ac0d4b972.1563517769.git.eskultet@redhat.com
src/util/virpci.c | 8 ++++++++
1 file changed, 8 insertions(+)
[libvirt] [PATCH] util: Fix broken MinGW builds caused by commit 9bc01ad8
Posted by Erik Skultety 4 years, 9 months ago 
virPCIGetSysfsFile is conditionally compiled only on Linux platforms.

Signed-off-by: Erik Skultety <eskultet@redhat.com>
---

Pushed under the build breaker rule.

 src/util/virpci.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/util/virpci.c b/src/util/virpci.c
index 75e8daadd5..ae3a0fa886 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -3099,6 +3099,14 @@ virPCIGetVirtualFunctionIndex(const char *pf_sysfs_device_link ATTRIBUTE_UNUSED,

 }

+int
+virPCIGetSysfsFile(char *virPCIDeviceName ATTRIBUTE_UNUSED,
+                   char **pci_sysfs_device_link ATTRIBUTE_UNUSED)
+{
+    virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported));
+    return -1;
+}
+
 int
 virPCIDeviceAddressGetSysfsFile(virPCIDeviceAddressPtr dev ATTRIBUTE_UNUSED,
                                 char **pci_sysfs_device_link ATTRIBUTE_UNUSED)
--
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: Fix broken MinGW builds caused by commit 9bc01ad8
Posted by Peter Krempa 4 years, 9 months ago 
On Fri, Jul 19, 2019 at 08:30:34 +0200, Erik Skultety wrote:
> virPCIGetSysfsFile is conditionally compiled only on Linux platforms.
> 
> Signed-off-by: Erik Skultety <eskultet@redhat.com>
> ---
> 
> Pushed under the build breaker rule.
> 
>  src/util/virpci.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/src/util/virpci.c b/src/util/virpci.c
> index 75e8daadd5..ae3a0fa886 100644
> --- a/src/util/virpci.c
> +++ b/src/util/virpci.c
> @@ -3099,6 +3099,14 @@ virPCIGetVirtualFunctionIndex(const char *pf_sysfs_device_link ATTRIBUTE_UNUSED,
> 
>  }
> 
> +int
> +virPCIGetSysfsFile(char *virPCIDeviceName ATTRIBUTE_UNUSED,
> +                   char **pci_sysfs_device_link ATTRIBUTE_UNUSED)
> +{
> +    virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported));

I think you forgot quotes around the "unsupported" string.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: Fix broken MinGW builds caused by commit 9bc01ad8
Posted by Peter Krempa 4 years, 9 months ago 
On Fri, Jul 19, 2019 at 08:39:35 +0200, Peter Krempa wrote:
> On Fri, Jul 19, 2019 at 08:30:34 +0200, Erik Skultety wrote:
> > virPCIGetSysfsFile is conditionally compiled only on Linux platforms.
> > 
> > Signed-off-by: Erik Skultety <eskultet@redhat.com>
> > ---
> > 
> > Pushed under the build breaker rule.
> > 
> >  src/util/virpci.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/src/util/virpci.c b/src/util/virpci.c
> > index 75e8daadd5..ae3a0fa886 100644
> > --- a/src/util/virpci.c
> > +++ b/src/util/virpci.c
> > @@ -3099,6 +3099,14 @@ virPCIGetVirtualFunctionIndex(const char *pf_sysfs_device_link ATTRIBUTE_UNUSED,
> > 
> >  }
> > 
> > +int
> > +virPCIGetSysfsFile(char *virPCIDeviceName ATTRIBUTE_UNUSED,
> > +                   char **pci_sysfs_device_link ATTRIBUTE_UNUSED)
> > +{
> > +    virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported));
> 
> I think you forgot quotes around the "unsupported" string.

Never mind. I looked at the code and 'unsupported' is a global static
string in that module. I didn't expect that
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: Fix broken MinGW builds caused by commit 9bc01ad8
Posted by Erik Skultety 4 years, 9 months ago 
On Fri, Jul 19, 2019 at 08:39:35AM +0200, Peter Krempa wrote:
> On Fri, Jul 19, 2019 at 08:30:34 +0200, Erik Skultety wrote:
> > virPCIGetSysfsFile is conditionally compiled only on Linux platforms.
> >
> > Signed-off-by: Erik Skultety <eskultet@redhat.com>
> > ---
> >
> > Pushed under the build breaker rule.
> >
> >  src/util/virpci.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/src/util/virpci.c b/src/util/virpci.c
> > index 75e8daadd5..ae3a0fa886 100644
> > --- a/src/util/virpci.c
> > +++ b/src/util/virpci.c
> > @@ -3099,6 +3099,14 @@ virPCIGetVirtualFunctionIndex(const char *pf_sysfs_device_link ATTRIBUTE_UNUSED,
> >
> >  }
> >
> > +int
> > +virPCIGetSysfsFile(char *virPCIDeviceName ATTRIBUTE_UNUSED,
> > +                   char **pci_sysfs_device_link ATTRIBUTE_UNUSED)
> > +{
> > +    virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported));
>
> I think you forgot quotes around the "unsupported" string.

unsupported is a const char * variable.

Erik

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: Fix broken MinGW builds caused by commit 9bc01ad8
Posted by Daniel P. Berrangé 4 years, 9 months ago 
On Fri, Jul 19, 2019 at 08:44:59AM +0200, Erik Skultety wrote:
> On Fri, Jul 19, 2019 at 08:39:35AM +0200, Peter Krempa wrote:
> > On Fri, Jul 19, 2019 at 08:30:34 +0200, Erik Skultety wrote:
> > > virPCIGetSysfsFile is conditionally compiled only on Linux platforms.
> > >
> > > Signed-off-by: Erik Skultety <eskultet@redhat.com>
> > > ---
> > >
> > > Pushed under the build breaker rule.
> > >
> > >  src/util/virpci.c | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > >
> > > diff --git a/src/util/virpci.c b/src/util/virpci.c
> > > index 75e8daadd5..ae3a0fa886 100644
> > > --- a/src/util/virpci.c
> > > +++ b/src/util/virpci.c
> > > @@ -3099,6 +3099,14 @@ virPCIGetVirtualFunctionIndex(const char *pf_sysfs_device_link ATTRIBUTE_UNUSED,
> > >
> > >  }
> > >
> > > +int
> > > +virPCIGetSysfsFile(char *virPCIDeviceName ATTRIBUTE_UNUSED,
> > > +                   char **pci_sysfs_device_link ATTRIBUTE_UNUSED)
> > > +{
> > > +    virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported));
> >
> > I think you forgot quotes around the "unsupported" string.
> 
> unsupported is a const char * variable.

Ewww, that all has to be purged as this is the wrong error code. This
example should be:

   virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s@",
                  _("sysfs does not exist on this platform"))


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: Fix broken MinGW builds caused by commit 9bc01ad8
Posted by Erik Skultety 4 years, 9 months ago 
On Fri, Jul 19, 2019 at 09:34:03AM +0100, Daniel P. Berrangé wrote:
> On Fri, Jul 19, 2019 at 08:44:59AM +0200, Erik Skultety wrote:
> > On Fri, Jul 19, 2019 at 08:39:35AM +0200, Peter Krempa wrote:
> > > On Fri, Jul 19, 2019 at 08:30:34 +0200, Erik Skultety wrote:
> > > > virPCIGetSysfsFile is conditionally compiled only on Linux platforms.
> > > >
> > > > Signed-off-by: Erik Skultety <eskultet@redhat.com>
> > > > ---
> > > >
> > > > Pushed under the build breaker rule.
> > > >
> > > >  src/util/virpci.c | 8 ++++++++
> > > >  1 file changed, 8 insertions(+)
> > > >
> > > > diff --git a/src/util/virpci.c b/src/util/virpci.c
> > > > index 75e8daadd5..ae3a0fa886 100644
> > > > --- a/src/util/virpci.c
> > > > +++ b/src/util/virpci.c
> > > > @@ -3099,6 +3099,14 @@ virPCIGetVirtualFunctionIndex(const char *pf_sysfs_device_link ATTRIBUTE_UNUSED,
> > > >
> > > >  }
> > > >
> > > > +int
> > > > +virPCIGetSysfsFile(char *virPCIDeviceName ATTRIBUTE_UNUSED,
> > > > +                   char **pci_sysfs_device_link ATTRIBUTE_UNUSED)
> > > > +{
> > > > +    virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported));
> > >
> > > I think you forgot quotes around the "unsupported" string.
> >
> > unsupported is a const char * variable.
>
> Ewww, that all has to be purged as this is the wrong error code. This
> example should be:
>
>    virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s@",
>                   _("sysfs does not exist on this platform"))

Wouldn't changing the default error code be enough? All the functions in that
conditional block are related to sysfs operations and when the generic error
says "Not supported on non-linux platforms", it still holds to be true.

Erik

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: Fix broken MinGW builds caused by commit 9bc01ad8
Posted by Daniel P. Berrangé 4 years, 9 months ago 
On Fri, Jul 19, 2019 at 11:01:59AM +0200, Erik Skultety wrote:
> On Fri, Jul 19, 2019 at 09:34:03AM +0100, Daniel P. Berrangé wrote:
> > On Fri, Jul 19, 2019 at 08:44:59AM +0200, Erik Skultety wrote:
> > > On Fri, Jul 19, 2019 at 08:39:35AM +0200, Peter Krempa wrote:
> > > > On Fri, Jul 19, 2019 at 08:30:34 +0200, Erik Skultety wrote:
> > > > > virPCIGetSysfsFile is conditionally compiled only on Linux platforms.
> > > > >
> > > > > Signed-off-by: Erik Skultety <eskultet@redhat.com>
> > > > > ---
> > > > >
> > > > > Pushed under the build breaker rule.
> > > > >
> > > > >  src/util/virpci.c | 8 ++++++++
> > > > >  1 file changed, 8 insertions(+)
> > > > >
> > > > > diff --git a/src/util/virpci.c b/src/util/virpci.c
> > > > > index 75e8daadd5..ae3a0fa886 100644
> > > > > --- a/src/util/virpci.c
> > > > > +++ b/src/util/virpci.c
> > > > > @@ -3099,6 +3099,14 @@ virPCIGetVirtualFunctionIndex(const char *pf_sysfs_device_link ATTRIBUTE_UNUSED,
> > > > >
> > > > >  }
> > > > >
> > > > > +int
> > > > > +virPCIGetSysfsFile(char *virPCIDeviceName ATTRIBUTE_UNUSED,
> > > > > +                   char **pci_sysfs_device_link ATTRIBUTE_UNUSED)
> > > > > +{
> > > > > +    virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported));
> > > >
> > > > I think you forgot quotes around the "unsupported" string.
> > >
> > > unsupported is a const char * variable.
> >
> > Ewww, that all has to be purged as this is the wrong error code. This
> > example should be:
> >
> >    virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s@",
> >                   _("sysfs does not exist on this platform"))
> 
> Wouldn't changing the default error code be enough? All the functions in that
> conditional block are related to sysfs operations and when the generic error
> says "Not supported on non-linux platforms", it still holds to be true.

Personally I prefer to see more specific error messages, as when the user
reports an error typically all we get is the message, not the function,
so its easier to distinguish where it came from

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: Fix broken MinGW builds caused by commit 9bc01ad8
Posted by Erik Skultety 4 years, 9 months ago 
On Fri, Jul 19, 2019 at 10:11:36AM +0100, Daniel P. Berrangé wrote:
> On Fri, Jul 19, 2019 at 11:01:59AM +0200, Erik Skultety wrote:
> > On Fri, Jul 19, 2019 at 09:34:03AM +0100, Daniel P. Berrangé wrote:
> > > On Fri, Jul 19, 2019 at 08:44:59AM +0200, Erik Skultety wrote:
> > > > On Fri, Jul 19, 2019 at 08:39:35AM +0200, Peter Krempa wrote:
> > > > > On Fri, Jul 19, 2019 at 08:30:34 +0200, Erik Skultety wrote:
> > > > > > virPCIGetSysfsFile is conditionally compiled only on Linux platforms.
> > > > > >
> > > > > > Signed-off-by: Erik Skultety <eskultet@redhat.com>
> > > > > > ---
> > > > > >
> > > > > > Pushed under the build breaker rule.
> > > > > >
> > > > > >  src/util/virpci.c | 8 ++++++++
> > > > > >  1 file changed, 8 insertions(+)
> > > > > >
> > > > > > diff --git a/src/util/virpci.c b/src/util/virpci.c
> > > > > > index 75e8daadd5..ae3a0fa886 100644
> > > > > > --- a/src/util/virpci.c
> > > > > > +++ b/src/util/virpci.c
> > > > > > @@ -3099,6 +3099,14 @@ virPCIGetVirtualFunctionIndex(const char *pf_sysfs_device_link ATTRIBUTE_UNUSED,
> > > > > >
> > > > > >  }
> > > > > >
> > > > > > +int
> > > > > > +virPCIGetSysfsFile(char *virPCIDeviceName ATTRIBUTE_UNUSED,
> > > > > > +                   char **pci_sysfs_device_link ATTRIBUTE_UNUSED)
> > > > > > +{
> > > > > > +    virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported));
> > > > >
> > > > > I think you forgot quotes around the "unsupported" string.
> > > >
> > > > unsupported is a const char * variable.
> > >
> > > Ewww, that all has to be purged as this is the wrong error code. This
> > > example should be:
> > >
> > >    virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s@",
> > >                   _("sysfs does not exist on this platform"))
> >
> > Wouldn't changing the default error code be enough? All the functions in that
> > conditional block are related to sysfs operations and when the generic error
> > says "Not supported on non-linux platforms", it still holds to be true.
>
> Personally I prefer to see more specific error messages, as when the user
> reports an error typically all we get is the message, not the function,
> so its easier to distinguish where it came from

In general, that's true, but in this case, all the function would have to
report that sysfs doesn't exist, so from our POV, it didn't really help in
identifying the function, you'd need the debug logs anyways.

Erik

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.