Re: [libvirt] [PATCH 12/15] util: pci: Introduce virPCIGetMdevTypes helper

2018-01-29 Thread Erik Skultety 
On Mon, Jan 29, 2018 at 02:02:19PM +0100, Michal Privoznik wrote
> On 01/29/2018 01:24 PM, Erik Skultety wrote:
> > On Fri, Jan 26, 2018 at 12:39:00PM +0100, Michal Privoznik wrote:
> >> On 01/25/2018 10:23 AM, Erik Skultety wrote:
> >>> This is a replacement for the existing udevPCIGetMdevTypesCap which is
> >>> static to the udev backend. This simple helper constructs the sysfs path
> >>> from the device's base path for each mdev type and queries the
> >>> corresponding attributes of that type.
> >>>
> >>> Signed-off-by: Erik Skultety 
> >>> ---
> >>>  src/libvirt_private.syms |  1 +
> >>>  src/util/virpci.c| 58 
> >>> 
> >>>  src/util/virpci.h|  4 
> >>>  3 files changed, 63 insertions(+)
> >>>
> >>> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> >>> index 75eaf1d4c..8d4c8dd3f 100644
> >>> --- a/src/libvirt_private.syms
> >>> +++ b/src/libvirt_private.syms
> >>> @@ -2456,6 +2456,7 @@ virPCIDeviceWaitForCleanup;
> >>>  virPCIEDeviceInfoFree;
> >>>  virPCIGetDeviceAddressFromSysfsLink;
> >>>  virPCIGetHeaderType;
> >>> +virPCIGetMdevTypes;
> >>>  virPCIGetNetName;
> >>>  virPCIGetPhysicalFunction;
> >>>  virPCIGetVirtualFunctionIndex;
> >>> diff --git a/src/util/virpci.c b/src/util/virpci.c
> >>> index fe57bef32..12d7ef0e4 100644
> >>> --- a/src/util/virpci.c
> >>> +++ b/src/util/virpci.c
> >>> @@ -3027,6 +3027,64 @@ virPCIGetVirtualFunctionInfo(const char 
> >>> *vf_sysfs_device_path,
> >>>  return ret;
> >>>  }
> >>>
> >>> +
> >>> +int
> >>> +virPCIGetMdevTypes(const char *sysfspath,
> >>> +   virMediatedDeviceTypePtr **types)
> >>
> >> Since this function returns size_t on success, I guess the retval should
> >> be type of ssize_t at least. We are not guaranteed that size_t will fit
> >
> > ssize_t wouldn't really help, since assigning size_t might overflow, so the
> > only safe bet is long long, but I mean, do you really expect there to be 
> > more
> > than INT_MAX mdev types for a device? That would be a lot of types to 
> > support.
>
> In kernel, they do a lot of size_t -> ssize_t conversion in cases
> where's it's next to impossible to overflow. And I believe this is one
> of them.

Fair enough, consider it changed.

Erik

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

Re: [libvirt] [PATCH 12/15] util: pci: Introduce virPCIGetMdevTypes helper

2018-01-29 Thread Michal Privoznik 
On 01/29/2018 01:24 PM, Erik Skultety wrote:
> On Fri, Jan 26, 2018 at 12:39:00PM +0100, Michal Privoznik wrote:
>> On 01/25/2018 10:23 AM, Erik Skultety wrote:
>>> This is a replacement for the existing udevPCIGetMdevTypesCap which is
>>> static to the udev backend. This simple helper constructs the sysfs path
>>> from the device's base path for each mdev type and queries the
>>> corresponding attributes of that type.
>>>
>>> Signed-off-by: Erik Skultety 
>>> ---
>>>  src/libvirt_private.syms |  1 +
>>>  src/util/virpci.c| 58 
>>> 
>>>  src/util/virpci.h|  4 
>>>  3 files changed, 63 insertions(+)
>>>
>>> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
>>> index 75eaf1d4c..8d4c8dd3f 100644
>>> --- a/src/libvirt_private.syms
>>> +++ b/src/libvirt_private.syms
>>> @@ -2456,6 +2456,7 @@ virPCIDeviceWaitForCleanup;
>>>  virPCIEDeviceInfoFree;
>>>  virPCIGetDeviceAddressFromSysfsLink;
>>>  virPCIGetHeaderType;
>>> +virPCIGetMdevTypes;
>>>  virPCIGetNetName;
>>>  virPCIGetPhysicalFunction;
>>>  virPCIGetVirtualFunctionIndex;
>>> diff --git a/src/util/virpci.c b/src/util/virpci.c
>>> index fe57bef32..12d7ef0e4 100644
>>> --- a/src/util/virpci.c
>>> +++ b/src/util/virpci.c
>>> @@ -3027,6 +3027,64 @@ virPCIGetVirtualFunctionInfo(const char 
>>> *vf_sysfs_device_path,
>>>  return ret;
>>>  }
>>>
>>> +
>>> +int
>>> +virPCIGetMdevTypes(const char *sysfspath,
>>> +   virMediatedDeviceTypePtr **types)
>>
>> Since this function returns size_t on success, I guess the retval should
>> be type of ssize_t at least. We are not guaranteed that size_t will fit
> 
> ssize_t wouldn't really help, since assigning size_t might overflow, so the
> only safe bet is long long, but I mean, do you really expect there to be more
> than INT_MAX mdev types for a device? That would be a lot of types to support.

In kernel, they do a lot of size_t -> ssize_t conversion in cases
where's it's next to impossible to overflow. And I believe this is one
of them.

Michal

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

Re: [libvirt] [PATCH 12/15] util: pci: Introduce virPCIGetMdevTypes helper

2018-01-29 Thread Erik Skultety 
On Fri, Jan 26, 2018 at 12:39:00PM +0100, Michal Privoznik wrote:
> On 01/25/2018 10:23 AM, Erik Skultety wrote:
> > This is a replacement for the existing udevPCIGetMdevTypesCap which is
> > static to the udev backend. This simple helper constructs the sysfs path
> > from the device's base path for each mdev type and queries the
> > corresponding attributes of that type.
> >
> > Signed-off-by: Erik Skultety 
> > ---
> >  src/libvirt_private.syms |  1 +
> >  src/util/virpci.c| 58 
> > 
> >  src/util/virpci.h|  4 
> >  3 files changed, 63 insertions(+)
> >
> > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> > index 75eaf1d4c..8d4c8dd3f 100644
> > --- a/src/libvirt_private.syms
> > +++ b/src/libvirt_private.syms
> > @@ -2456,6 +2456,7 @@ virPCIDeviceWaitForCleanup;
> >  virPCIEDeviceInfoFree;
> >  virPCIGetDeviceAddressFromSysfsLink;
> >  virPCIGetHeaderType;
> > +virPCIGetMdevTypes;
> >  virPCIGetNetName;
> >  virPCIGetPhysicalFunction;
> >  virPCIGetVirtualFunctionIndex;
> > diff --git a/src/util/virpci.c b/src/util/virpci.c
> > index fe57bef32..12d7ef0e4 100644
> > --- a/src/util/virpci.c
> > +++ b/src/util/virpci.c
> > @@ -3027,6 +3027,64 @@ virPCIGetVirtualFunctionInfo(const char 
> > *vf_sysfs_device_path,
> >  return ret;
> >  }
> >
> > +
> > +int
> > +virPCIGetMdevTypes(const char *sysfspath,
> > +   virMediatedDeviceTypePtr **types)
>
> Since this function returns size_t on success, I guess the retval should
> be type of ssize_t at least. We are not guaranteed that size_t will fit

ssize_t wouldn't really help, since assigning size_t might overflow, so the
only safe bet is long long, but I mean, do you really expect there to be more
than INT_MAX mdev types for a device? That would be a lot of types to support.

Erik

> into int (although in this case it will - currently you will have only
> limited number of MDEVs if any).
>
> ACK
>
> Michal

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

Re: [libvirt] [PATCH 12/15] util: pci: Introduce virPCIGetMdevTypes helper

2018-01-26 Thread Michal Privoznik 
On 01/25/2018 10:23 AM, Erik Skultety wrote:
> This is a replacement for the existing udevPCIGetMdevTypesCap which is
> static to the udev backend. This simple helper constructs the sysfs path
> from the device's base path for each mdev type and queries the
> corresponding attributes of that type.
> 
> Signed-off-by: Erik Skultety 
> ---
>  src/libvirt_private.syms |  1 +
>  src/util/virpci.c| 58 
> 
>  src/util/virpci.h|  4 
>  3 files changed, 63 insertions(+)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 75eaf1d4c..8d4c8dd3f 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2456,6 +2456,7 @@ virPCIDeviceWaitForCleanup;
>  virPCIEDeviceInfoFree;
>  virPCIGetDeviceAddressFromSysfsLink;
>  virPCIGetHeaderType;
> +virPCIGetMdevTypes;
>  virPCIGetNetName;
>  virPCIGetPhysicalFunction;
>  virPCIGetVirtualFunctionIndex;
> diff --git a/src/util/virpci.c b/src/util/virpci.c
> index fe57bef32..12d7ef0e4 100644
> --- a/src/util/virpci.c
> +++ b/src/util/virpci.c
> @@ -3027,6 +3027,64 @@ virPCIGetVirtualFunctionInfo(const char 
> *vf_sysfs_device_path,
>  return ret;
>  }
>  
> +
> +int
> +virPCIGetMdevTypes(const char *sysfspath,
> +   virMediatedDeviceTypePtr **types)

Since this function returns size_t on success, I guess the retval should
be type of ssize_t at least. We are not guaranteed that size_t will fit
into int (although in this case it will - currently you will have only
limited number of MDEVs if any).

ACK

Michal

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

[libvirt] [PATCH 12/15] util: pci: Introduce virPCIGetMdevTypes helper

2018-01-25 Thread Erik Skultety 
This is a replacement for the existing udevPCIGetMdevTypesCap which is
static to the udev backend. This simple helper constructs the sysfs path
from the device's base path for each mdev type and queries the
corresponding attributes of that type.

Signed-off-by: Erik Skultety 
---
 src/libvirt_private.syms |  1 +
 src/util/virpci.c| 58 
 src/util/virpci.h|  4 
 3 files changed, 63 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 75eaf1d4c..8d4c8dd3f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2456,6 +2456,7 @@ virPCIDeviceWaitForCleanup;
 virPCIEDeviceInfoFree;
 virPCIGetDeviceAddressFromSysfsLink;
 virPCIGetHeaderType;
+virPCIGetMdevTypes;
 virPCIGetNetName;
 virPCIGetPhysicalFunction;
 virPCIGetVirtualFunctionIndex;
diff --git a/src/util/virpci.c b/src/util/virpci.c
index fe57bef32..12d7ef0e4 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -3027,6 +3027,64 @@ virPCIGetVirtualFunctionInfo(const char 
*vf_sysfs_device_path,
 return ret;
 }
 
+
+int
+virPCIGetMdevTypes(const char *sysfspath,
+   virMediatedDeviceTypePtr **types)
+{
+int ret = -1;
+int dirret = -1;
+DIR *dir = NULL;
+struct dirent *entry;
+char *types_path = NULL;
+char *tmppath = NULL;
+virMediatedDeviceTypePtr mdev_type = NULL;
+virMediatedDeviceTypePtr *mdev_types = NULL;
+size_t ntypes = 0;
+size_t i;
+
+if (virAsprintf(_path, "%s/mdev_supported_types", sysfspath) < 0)
+return -1;
+
+if ((dirret = virDirOpenIfExists(, types_path)) < 0)
+goto cleanup;
+
+if (dirret == 0) {
+ret = 0;
+goto cleanup;
+}
+
+while ((dirret = virDirRead(dir, , types_path)) > 0) {
+/* append the type id to the path and read the attributes from there */
+if (virAsprintf(, "%s/%s", types_path, entry->d_name) < 0)
+goto cleanup;
+
+if (virMediatedDeviceTypeReadAttrs(tmppath, _type) < 0)
+goto cleanup;
+
+if (VIR_APPEND_ELEMENT(mdev_types, ntypes, mdev_type) < 0)
+goto cleanup;
+
+VIR_FREE(tmppath);
+}
+
+if (dirret < 0)
+goto cleanup;
+
+VIR_STEAL_PTR(*types, mdev_types);
+ret = ntypes;
+ntypes = 0;
+ cleanup:
+virMediatedDeviceTypeFree(mdev_type);
+for (i = 0; i < ntypes; i++)
+virMediatedDeviceTypeFree(mdev_types[i]);
+VIR_FREE(mdev_types);
+VIR_FREE(types_path);
+VIR_FREE(tmppath);
+VIR_DIR_CLOSE(dir);
+return ret;
+}
+
 #else
 static const char *unsupported = N_("not supported on non-linux platforms");
 
diff --git a/src/util/virpci.h b/src/util/virpci.h
index f1fbe39e6..a0bc0a474 100644
--- a/src/util/virpci.h
+++ b/src/util/virpci.h
@@ -25,6 +25,7 @@
 # define __VIR_PCI_H__
 
 # include "internal.h"
+# include "virmdev.h"
 # include "virobject.h"
 # include "virutil.h"
 
@@ -249,4 +250,7 @@ int virPCIGetHeaderType(virPCIDevicePtr dev, int *hdrType);
 
 void virPCIEDeviceInfoFree(virPCIEDeviceInfoPtr dev);
 
+int virPCIGetMdevTypes(const char *sysfspath,
+   virMediatedDeviceType ***types);
+
 #endif /* __VIR_PCI_H__ */
-- 
2.13.6

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