[libvirt] [PATCH v3 02/15] conf: hostdev: Introduce virDomainHostdevSubsysSCSIClear

2017-03-16 Thread Erik Skultety
Just a tiny wrapper over the SCSI def clearing logic to drop some if-else branches from a switch, mainly because extending the switch in the future would render the current code with branching less readable. Signed-off-by: Erik Skultety <eskul...@redhat.com> --- src/conf/domain_conf.

[libvirt] [PATCH v3 13/15] test: Add some test cases for our test suite regarding the mdevs

2017-03-16 Thread Erik Skultety
For now, these only cover the unmanaged, i.e. user pre-created devices. Signed-off-by: Erik Skultety <eskul...@redhat.com> --- ...ml2argv-hostdev-mdev-invalid-target-address.xml | 33 ++ ...muxml2argv-hostdev-mdev-src-address-invalid.xml | 35 +++ .../qemuxm

Re: [libvirt] [RFC PATCH v2 REBASE 00/18] Introduce vGPU mdev framework to libvirt

2017-03-20 Thread Erik Skultety
On Mon, Mar 20, 2017 at 11:03:40AM +0100, Erik Skultety wrote: > On Mon, Mar 20, 2017 at 03:27:19PM +0800, yonglihe wrote: > > > > tested v3, on the mdev-next branch: > > > > none-root: > > == > > 1. hacker the privilege operations > > sudo sh

Re: [libvirt] [RFC PATCH v2 REBASE 00/18] Introduce vGPU mdev framework to libvirt

2017-03-20 Thread Erik Skultety
On Mon, Mar 20, 2017 at 03:27:19PM +0800, yonglihe wrote: > > tested v3, on the mdev-next branch: > > none-root: > == > 1. hacker the privilege operations > sudo sh -c "ulimit -l 3074424832 && exec su $LOGNAME" > sudo chown ubuntu:ubuntu /dev/vfio/0 > > RFC: i don't know the correct way to

Re: [libvirt] [PATCH V3] util: Add more virsysfs functions for handling resctrl sysfs

2017-04-03 Thread Erik Skultety
On Fri, Mar 31, 2017 at 12:59:33PM +0200, Martin Kletzander wrote: > On Fri, Mar 31, 2017 at 12:28:26PM +0200, Erik Skultety wrote: > > > #define VIR_SYSFS_VALUE_MAXLEN 8192 > > > #define SYSFS_SYSTEM_PATH "/sys/devices/system" > > > +#de

Re: [libvirt] [RFC PATCH 00/11] Add mdev reporting capability to the nodedev driver

2017-04-05 Thread Erik Skultety
On Tue, Apr 04, 2017 at 04:23:18PM +0100, Daniel P. Berrange wrote: > On Wed, Mar 29, 2017 at 02:51:10PM +0200, Erik Skultety wrote: > > This series enables the node device driver to report information about the > > existing mediated devices on the host. There is no device cr

[libvirt] [PATCH] qemu: Add device id for mediated devices on qemu command line

2017-04-03 Thread Erik Skultety
Like all devices, add the 'id' option for mdevs as well. Patch also adjusts the test accordingly. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1438431 Signed-off-by: Erik Skultety <eskul...@redhat.com> --- src/qemu/qemu_command.c | 3 ++-

[libvirt] [PATCH] qemu: Fix mdev checking for VFIO support

2017-04-12 Thread Erik Skultety
://bugzilla.redhat.com/show_bug.cgi?id=1441291 Signed-off-by: Erik Skultety <eskul...@redhat.com> --- src/qemu/qemu_hostdev.c | 16 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c index 685bf5b59..9b5504832 100644 ---

Re: [libvirt] [PATCH] qemu: Fix mdev checking for VFIO support

2017-04-12 Thread Erik Skultety
On Wed, Apr 12, 2017 at 11:35:14AM +0100, Daniel P. Berrange wrote: > On Wed, Apr 12, 2017 at 12:26:35PM +0200, Erik Skultety wrote: > > Commit a4a39d90 added a check that checks for VFIO support with mediated > > devices. The problem is that the hostdev preparing functi

Re: [libvirt] [PATCH v2 07/12] tests: Add missing cache data for vircaps2xmltest

2017-04-07 Thread Erik Skultety
> diff --git a/tests/vircaps2xmldata/linux-caches/cpu/cpu0/cache/index0/id > b/tests/vircaps2xmldata/linux-caches/cpu/cpu0/cache/index0/id > new file mode 100644 > index ..573541ac9702 > --- /dev/null > +++ b/tests/vircaps2xmldata/linux-caches/cpu/cpu0/cache/index0/id > @@ -0,0 +1 @@

Re: [libvirt] [PATCH v2 04/12] tests: Add virfilemock -- the new super mock

2017-04-07 Thread Erik Skultety
> +#include > + > +#include "virmock.h" > +#include "virfilemock.h" ^^These are local, could you group them with the rest below the system ones? (This was pointed out to me during review once or twice). > + > +#include > +#include ^^These get pulled in by virmock.h, but it's always nice to be

Re: [libvirt] [PATCH v2 06/12] tests: Test vircaps2xmldata XMLs in virschematest

2017-04-07 Thread Erik Skultety
On Wed, Apr 05, 2017 at 04:36:29PM +0200, Martin Kletzander wrote: > Signed-off-by: Martin Kletzander > --- > tests/virschematest.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/tests/virschematest.c b/tests/virschematest.c > index

Re: [libvirt] [PATCH v2 05/12] util: Remove virsysfs and instead enhance virFileReadValue* functions

2017-04-07 Thread Erik Skultety
On Wed, Apr 05, 2017 at 04:36:28PM +0200, Martin Kletzander wrote: > It is no longer needed thanks to the great virfilemock.c. And this > way we don't have to add a new set of functions for each prefixed > path. > > While on that, add two functions that weren't there before, string and > scaled

Re: [libvirt] [PATCH v2 04/12] tests: Add virfilemock -- the new super mock

2017-04-07 Thread Erik Skultety
On Fri, Apr 07, 2017 at 10:08:35AM +0200, Erik Skultety wrote: > > +#include > > + > > +#include "virmock.h" > > +#include "virfilemock.h" > > ^^These are local, could you group them with the rest below the system ones? > (T

Re: [libvirt] [PATCH v2 10/12] Init host cache info in drivers

2017-04-07 Thread Erik Skultety
On Wed, Apr 05, 2017 at 04:36:33PM +0200, Martin Kletzander wrote: > Added only in drivers that were already calling > virCapabilitiesInitNUMA(). Instead of refactoring all the callers to > behave the same way in case of error, just follow what the callers are > doing for all the functions. >

Re: [libvirt] [PATCH v2 12/12] tests: Add resctrl test for vircaps2xmltest

2017-04-07 Thread Erik Skultety
On Wed, Apr 05, 2017 at 04:36:35PM +0200, Martin Kletzander wrote: > Add info from yet another machine, this time with resctrl data so that > we can extend tests easily in a test-driven way. > Again, if something was wrong with this, one couldn't know. ACK to both 11/12 and 12/12. Erik --

Re: [libvirt] [PATCH v2 09/12] Add host cache information into capabilities

2017-04-07 Thread Erik Skultety
On Wed, Apr 05, 2017 at 04:36:32PM +0200, Martin Kletzander wrote: > Signed-off-by: Martin Kletzander > --- > I'm adding only info about L3 caches, we can add more later, but for > now that's more than enough. I think it's worth putting this into the commit message, along

Re: [libvirt] [PATCH v2 08/12] Add RNG schema for host cache information in capabilities

2017-04-07 Thread Erik Skultety
On Wed, Apr 05, 2017 at 04:36:31PM +0200, Martin Kletzander wrote: > Signed-off-by: Martin Kletzander > --- > docs/schemas/capability.rng | 34 ++ > 1 file changed, 34 insertions(+) > This should be squashed into 9/12, since that is adding

Re: [libvirt] [PATCH v2 01/12] conf: Fix possible memleak in capabilities

2017-04-06 Thread Erik Skultety
On Wed, Apr 05, 2017 at 04:36:24PM +0200, Martin Kletzander wrote: > If formatting NUMA topology fails, the function returns immediatelly, > but the buffer structure allocated on the stack references lot of > heap-allocated memory and that would get lost in such case. > > Signed-off-by: Martin

Re: [libvirt] [PATCH v2 01/12] conf: Fix possible memleak in capabilities

2017-04-06 Thread Erik Skultety
On Thu, Apr 06, 2017 at 11:52:54AM +0200, Martin Kletzander wrote: > On Thu, Apr 06, 2017 at 11:29:21AM +0200, Erik Skultety wrote: > > On Thu, Apr 06, 2017 at 11:12:54AM +0200, Martin Kletzander wrote: > > > On Thu, Apr 06, 2017 at 09:50:49AM +0200, Erik Skultety wrote: >

Re: [libvirt] [PATCH v2 03/12] util: Add virStringTrimOptionalNewline

2017-04-06 Thread Erik Skultety
On Wed, Apr 05, 2017 at 04:36:26PM +0200, Martin Kletzander wrote: > And use it in virFileRead* > > Signed-off-by: Martin Kletzander > --- > src/util/virfile.c| 18 +++--- > src/util/virhostcpu.c | 4 ++-- > src/util/virstring.h | 8 >

Re: [libvirt] [PATCH v2 01/12] conf: Fix possible memleak in capabilities

2017-04-06 Thread Erik Skultety
On Thu, Apr 06, 2017 at 11:12:54AM +0200, Martin Kletzander wrote: > On Thu, Apr 06, 2017 at 09:50:49AM +0200, Erik Skultety wrote: > > On Wed, Apr 05, 2017 at 04:36:24PM +0200, Martin Kletzander wrote: > > > If formatting NUMA topology fails, the function re

Re: [libvirt] [RFC PATCH 06/11] nodedev: Introduce the mdev capability to the nodedev driver structure

2017-04-10 Thread Erik Skultety
> > +void virNodeDevCapMdevFree(virNodeDevCapMdevPtr mdev) > > +{ > > +if (!mdev) > > +return; > > + > > +VIR_FREE(mdev->type); > > +VIR_FREE(mdev->name); > > +VIR_FREE(mdev->description); > > +VIR_FREE(mdev->device_api); > > +VIR_FREE(mdev); > > +} > > + [...] >

Re: [libvirt] [PATCH v2 07/10] nodedev: Introduce the mdev capability to a PCI parent device

2017-04-20 Thread Erik Skultety
[...] > +static int > +udevPCIGetMdevCaps(struct udev_device *device, > + virNodeDevCapPCIDevPtr pcidata) > +{ > +int ret = -1; > +int direrr = -1; > +DIR *dir = NULL; > +struct dirent *entry; > +char *path = NULL; > +char *tmppath = NULL; > +

[libvirt] [PATCH v2 09/10] docs: Provide a nodedev driver stub documentation

2017-04-20 Thread Erik Skultety
There's lot more to document about the nodedev driver, besides PCI and SR-IOV (even this might need to be extended), but let's start small-ish and at least have a page for it linked from the drivers.html. Signed-off-by: Erik Skultety <eskul...@redhat.com> --- docs/drivers.html.in

[libvirt] [PATCH v2 02/10] conf: nodedev: Split virNodeDeviceDefFormat into more functions

2017-04-20 Thread Erik Skultety
Make the code look cleaner by moving the capability specific bits into separate functions. Signed-off-by: Erik Skultety <eskul...@redhat.com> --- src/conf/node_device_conf.c | 578 1 file changed, 322 insertions(+), 256 deletions(-) diff --git

[libvirt] [PATCH v2 06/10] nodedev: Introduce the mdev capability to the nodedev driver structure

2017-04-20 Thread Erik Skultety
' attributes), this structure serves in both of these cases with the difference being that the amount of data it holds depends on the specific scenario (child vs parent). Signed-off-by: Erik Skultety <eskul...@redhat.com> --- include/libvirt/libvirt-nodedev.h| 1 + src/conf/node_device_

[libvirt] [PATCH v2 10/10] docs: Document the mediated devices within the nodedev driver

2017-04-20 Thread Erik Skultety
Signed-off-by: Erik Skultety <eskul...@redhat.com> --- docs/drvnodedev.html.in | 164 +++- 1 file changed, 162 insertions(+), 2 deletions(-) diff --git a/docs/drvnodedev.html.in b/docs/drvnodedev.html.in index ed185c3df..6dece3806 100644 ---

[libvirt] [PATCH v2 07/10] nodedev: Introduce the mdev capability to a PCI parent device

2017-04-20 Thread Erik Skultety
: ... ... optional, raw, unstructured resource allocation data vfio-pci NUM ... ... ... Signed-off-by: Erik Skultety <eskul...@redhat.com> --- docs/schemas/nodedev.rng | 24 sr

[libvirt] [PATCH v2 00/10] Add mdev reporting capability to the nodedev driver

2017-04-20 Thread Erik Skultety
for information about the parent's nested capabilities. Erik Erik Skultety (10): nodedev: Make use of the compile-time missing enum in switch error conf: nodedev: Split virNodeDeviceDefFormat into more functions nodedev: udevProcessPCI: Drop syspath variable docs: Utilize our XSLT list ge

[libvirt] [PATCH v2 08/10] nodedev: Introduce mdev capability for child devices

2017-04-20 Thread Erik Skultety
child device XML: mdev_4b20d080_1b54_4048_85b3_a6a62d165c01 /sys/devices/.../4b20d080-1b54-4048-85b3-a6a62d165c01 pci__06_00_0 vfio_mdev Signed-off-by: Erik Skultety <eskul...@redhat.com> --- docs/schemas/nodedev.rng | 17 +++

[libvirt] [PATCH v2 05/10] nodedev: conf: Split PCI sub-capability parsing to a separate method

2017-04-20 Thread Erik Skultety
Since there's at least SRIOV and MDEV sub-capabilities to be parsed, let's make the code more readable by splitting it to several logical blocks. Signed-off-by: Erik Skultety <eskul...@redhat.com> --- src/conf/node_device_conf.c | 84 - 1 file c

[libvirt] [PATCH v2 04/10] docs: Utilize our XSLT list generating template more

2017-04-20 Thread Erik Skultety
Since we do have this template at hand, why not using it wherever possible (list of supported pool types and remote access section). Also, perform some stylistic micro adjustments. Signed-off-by: Erik Skultety <eskul...@redhat.com> --- docs/remote.html.in

[libvirt] [PATCH v2 03/10] nodedev: udevProcessPCI: Drop syspath variable

2017-04-20 Thread Erik Skultety
Since we have that information provided by @def which is not a private object, there is really no need for the variable. Signed-off-by: Erik Skultety <eskul...@redhat.com> --- src/node_device/node_device_udev.c | 10 +++--- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git

[libvirt] [PATCH v2 01/10] nodedev: Make use of the compile-time missing enum in switch error

2017-04-20 Thread Erik Skultety
there, udevGetDeviceType (which was called before) already made sure that any unrecognized device types had been handled properly. Signed-off-by: Erik Skultety <eskul...@redhat.com> --- src/node_device/node_device_udev.c | 45 +- 1 file changed, 15 insertions(+), 30 del

Re: [libvirt] [PATCH] qemu_capabilities: report SATA bus in domain capabilities

2017-03-06 Thread Erik Skultety
On Mon, Mar 06, 2017 at 05:24:16PM +0100, Pavel Hrdina wrote: > Signed-off-by: Pavel Hrdina > --- > src/qemu/qemu_capabilities.c | 4 > tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml | 1 + >

Re: [libvirt] [PATCH v2 6/6] nodedev: Reduce virNodeDevCapDataPtr usage

2017-03-03 Thread Erik Skultety
On Thu, Mar 02, 2017 at 11:14:27AM -0500, John Ferlan wrote: > Replace with more data specific pointer types. > udevGetDMIData can be altered this way as well. ACK with that change. Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 0/6] Split out node device object into its own module

2017-03-03 Thread Erik Skultety
On Thu, Mar 02, 2017 at 11:14:21AM -0500, John Ferlan wrote: > v1: http://www.redhat.com/archives/libvir-list/2017-March/msg00046.html > > Differences... > > patch 1 and 3 are the same and both were ACK'd > patch 2 removes the lower case function name changes (a/k/a "vir" removal) > patch 4 is

Re: [libvirt] [PATCH v2 02/12] util: Fix virDirRead() description

2017-04-06 Thread Erik Skultety
On Wed, Apr 05, 2017 at 04:36:25PM +0200, Martin Kletzander wrote: > Signed-off-by: Martin Kletzander > --- > src/util/virfile.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/util/virfile.c b/src/util/virfile.c > index

Re: [libvirt] Add mdev reporting capability to the nodedev driver

2017-04-18 Thread Erik Skultety
nge" <berra...@redhat.com> wrote: > > > > > > > On Wed, Apr 05, 2017 at 08:12:31AM +0200, Erik Skultety wrote: > > > > > On Tue, Apr 04, 2017 at 04:23:18PM +0100, Daniel P. Berrange wrote: > > > > > > On Wed, Mar 29, 2017 at 02:51:10PM +0200, Er

Re: [libvirt] [PATCH] libvirtd.conf: Drop max_requests

2017-08-16 Thread Erik Skultety
On Tue, Aug 15, 2017 at 11:16:08AM +0200, Michal Privoznik wrote: > Since its introduction in f61341173bdaa2e0 it was never > implemented nor there are plans to implement it. Drop it. ACK. Erik -- libvir-list mailing list libvir-list@redhat.com

Re: [libvirt] [PATCH] Don't autogenerate seclabels of type 'none'

2017-08-16 Thread Erik Skultety
On Mon, Aug 14, 2017 at 06:07:10PM -0600, Jim Fehlig wrote: > When security drivers are active and domain def contains > no elements, there is no need to autogenerate > seclabels when starting the domain, e.g. > > > > In fact, autogenerating the label can result in needless > save/restore and

Re: [libvirt] [PATCH 3/5] nodedev: Clear the udev_monitor reference once unref'd

2017-08-16 Thread Erik Skultety
On Tue, Aug 01, 2017 at 03:25:37PM -0400, John Ferlan wrote: > > > On 07/26/2017 02:44 AM, Erik Skultety wrote: > > Since we only have one udev_monitor reference throughout libvirt, we > > should either clear the pointer we copy around to prevent invalid memory > > acces

Re: [libvirt] [PATCH 4/5] nodedev: Introduce udevHandleOneDevice

2017-08-16 Thread Erik Skultety
On Tue, Aug 01, 2017 at 03:46:44PM -0400, John Ferlan wrote: > > > On 07/26/2017 02:44 AM, Erik Skultety wrote: > > Let this new method handle the device object we obtained from the > > monitor in order to enhance readability. > > > > Signed-off-by: E

Re: [libvirt] [PATCH V2 1/2] Fix building domain def in securityselinuxtest

2017-08-17 Thread Erik Skultety
On Wed, Aug 16, 2017 at 05:54:07PM -0600, Jim Fehlig wrote: > The virDomainDef created by testBuildDomainDef in securityselinuxtest > adds a seclabel but does not increment nseclabels. Also, it should > populate seclabel->model with 'selinux'. > > While at it, use the secdef itself to populate

Re: [libvirt] [PATCH V2 2/2] Don't autogenerate seclabels of type 'none'

2017-08-17 Thread Erik Skultety
On Wed, Aug 16, 2017 at 05:54:08PM -0600, Jim Fehlig wrote: > When security drivers are active but confinement is not enabled, > there is no need to autogenerate elements when starting > a domain def that contains no elements. In fact, > autogenerating the elements can result in needless

Re: [libvirt] [PATCH v4 14/14] nodedev: Remove driver locks around object list mgmt code

2017-07-13 Thread Erik Skultety
> /* Populate with known devices */ > This commentary should stay with the function it describes (below), so the context doesn't get lost. Erik > +nodeDeviceUnlock(); > if (udevEnumerateDevices(udev) != 0) > goto cleanup; > > -ret = 0; > +return 0; > > cleanup:

Re: [libvirt] [PATCH v4 14/14] nodedev: Remove driver locks around object list mgmt code

2017-07-11 Thread Erik Skultety
On Mon, Jul 03, 2017 at 05:25:30PM -0400, John Ferlan wrote: > Since virnodedeviceobj now has a self-lockable hash table, there's no > need to lock the table from the driver for processing. Thus remove the > locks from the driver for NodeDeviceObjList mgmt. > > Signed-off-by: John Ferlan

Re: [libvirt] [PATCH v4 12/14] nodedev: Convert virNodeDeviceObj to use virObjectLockable

2017-07-11 Thread Erik Skultety
On Mon, Jul 03, 2017 at 05:25:28PM -0400, John Ferlan wrote: > Now that we have a bit more control, let's convert our object into > a lockable object and let that magic handle the create and lock/unlock. > > This also involves creating a virNodeDeviceEndAPI in order to handle > the object cleaup

Re: [libvirt] [PATCH v4 12/14] nodedev: Convert virNodeDeviceObj to use virObjectLockable

2017-07-10 Thread Erik Skultety
On Mon, Jul 03, 2017 at 05:25:28PM -0400, John Ferlan wrote: > Now that we have a bit more control, let's convert our object into > a lockable object and let that magic handle the create and lock/unlock. > > This also involves creating a virNodeDeviceEndAPI in order to handle > the object cleaup

Re: [libvirt] [PATCH v4 14/14] nodedev: Remove driver locks around object list mgmt code

2017-07-14 Thread Erik Skultety
On Thu, Jul 13, 2017 at 07:23:58PM -0400, John Ferlan wrote: > > > On 07/11/2017 10:38 AM, Erik Skultety wrote: > > On Mon, Jul 03, 2017 at 05:25:30PM -0400, John Ferlan wrote: > >> Since virnodedeviceobj now has a self-lockable hash table, there's no > >> need

Re: [libvirt] [PATCH v4 12/14] nodedev: Convert virNodeDeviceObj to use virObjectLockable

2017-07-14 Thread Erik Skultety
> >> cleanup: > >> -if (obj) > >> -virNodeDeviceObjUnlock(obj); > >> +virNodeDeviceObjEndAPI(); > >> testDriverUnlock(driver); > >> virNodeDeviceDefFree(def); > >> virObjectUnref(dev); > >> @@ -5596,13 +5595,13 @@ testNodeDeviceDestroy(virNodeDevicePtr dev) > >>

Re: [libvirt] [PATCH 1/2] storage: Alter check for default managed setting

2017-07-18 Thread Erik Skultety
On Mon, Jul 03, 2017 at 02:52:05PM -0400, John Ferlan wrote: > Only alter the managed setting if it wasn't provided. If someone provided > 'no', then honor that rather than overwriting. IMO this deserves to be tracked by a BZ, since the current behaviour is in contrast to what the documentation

Re: [libvirt] [PATCH 2/2] conf: Fix vHBA checkParent logic for pool creation

2017-07-18 Thread Erik Skultety
On Mon, Jul 03, 2017 at 02:52:06PM -0400, John Ferlan wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1458708 > > When originally designed in commit id '42a021c1', providing the 'parent' > attribute to the > was checked to make sure that the "parent" of the wwnn/wwpn matched that > of the

Re: [libvirt] [PATCH v4 12/14] nodedev: Convert virNodeDeviceObj to use virObjectLockable

2017-07-17 Thread Erik Skultety
On Fri, Jul 14, 2017 at 10:12:12AM -0400, John Ferlan wrote: > > > On 07/14/2017 05:28 AM, Erik Skultety wrote: > >>>> cleanup: > >>>> -if (obj) > >>>> -virNodeDeviceObjUnlock(obj); > >>>&g

Re: [libvirt] [PATCH v4 14/14] nodedev: Remove driver locks around object list mgmt code

2017-07-17 Thread Erik Skultety
On Fri, Jul 14, 2017 at 11:37:36AM -0400, John Ferlan wrote: > > > On 07/14/2017 05:23 AM, Erik Skultety wrote: > > On Thu, Jul 13, 2017 at 07:23:58PM -0400, John Ferlan wrote: > >> > >> > >> On 07/11/2017 10:38 AM, Erik Skultety wrote: > >>>

Re: [libvirt] [PATCH v5 1/3] nodedev: Fix usage of virNodeDeviceObjListGetParentHost

2017-07-20 Thread Erik Skultety
> >> > >> @@ -594,6 +600,11 @@ nodeDeviceDestroy(virNodeDevicePtr device) > >> int ret = -1; > >> virNodeDeviceObjPtr obj = NULL; > >> virNodeDeviceDefPtr def; > >> +char *name = NULL; > >> +char *parent = NULL; > >> +char *parent_wwnn = NULL; > >> +char *parent_wwpn

Re: [libvirt] [PATCH v2 1/3] storage: Fix existing parent check for vHBA creation

2017-07-20 Thread Erik Skultety
On Wed, Jul 19, 2017 at 11:09:00AM -0400, John Ferlan wrote: > > > On 07/19/2017 10:21 AM, Erik Skultety wrote: > > On Wed, Jul 19, 2017 at 09:59:06AM -0400, John Ferlan wrote: > >> > >> > >> On 07/19/2017 07:38 AM, Erik Skultety wrote: > >>>

Re: [libvirt] [PATCH 0/2] Fix a couple of resource leaks

2017-07-20 Thread Erik Skultety
On Thu, Jul 20, 2017 at 07:00:38AM -0400, John Ferlan wrote: > Resource a couple Coverity found resource leaks > > John Ferlan (2): > daemon: Don't conditionally free @origErr in daemonStreamEvent > tests: Free @fakerootdir in error path > > daemon/stream.c | 2 +- >

Re: [libvirt] [PATCH] nodedev: fix an improper comment

2017-07-20 Thread Erik Skultety
On Thu, Jul 20, 2017 at 05:01:57PM +0800, Chen Hanxiao wrote: > From: Chen Hanxiao > > Actually we use virConnectNodeDeviceEventGenericCallback. > > Signed-off-by: Chen Hanxiao Slightly adjusted the commit subject and pushed. Thanks, Erik --

Re: [libvirt] [PATCH v5 2/3] nodedev: Convert virNodeDeviceObjListPtr to use hash tables

2017-07-20 Thread Erik Skultety
> @@ -39,13 +40,19 @@ struct _virNodeDeviceObj { > }; > > struct _virNodeDeviceObjList { > -size_t count; > -virNodeDeviceObjPtr *objs; > +virObjectLockable parent; > + > +/* name string -> virNodeDeviceObj mapping > + * for O(1), lockless lookup-by-uuid */ I think you

Re: [libvirt] [PATCH v2] storage: Disallow usage of the HBA for a fc_host backing

2017-07-25 Thread Erik Skultety
On Tue, Jul 25, 2017 at 07:36:48AM -0400, John Ferlan wrote: > > > On 07/25/2017 03:45 AM, Erik Skultety wrote: > >> +/** > >> + * @name: Name from a wwnn/wwpn lookup > >> + * > >> + * Validate that the @name fetched from the wwnn/wwpn is

Re: [libvirt] [PATCH v2] storage: Disallow usage of the HBA for a fc_host backing

2017-07-25 Thread Erik Skultety
> One would hope it doesn't fail... Even if it does, virSCSIHostGetNumber does report an error already. > > Your suggestions make checkName much cleaner: > > if (virSCSIHostGetNumber(name, _num) && > virVHBAIsVportCapable(NULL, host_num)) > return true; > if

Re: [libvirt] [PATCH v3] storage: Disallow usage of the HBA for a fc_host backing

2017-07-25 Thread Erik Skultety
> +/** > + * @name: Name from a wwnn/wwpn lookup > + * > + * Validate that the @name fetched from the wwnn/wwpn is a vHBA > + * and not an HBA as that should be a configuration error. It's only > + * possible to use an existing wwnn/wwpn of a vHBA because that's > + * what someone would have

Re: [libvirt] [PATCH v6 3/4] nodedev: Convert virNodeDeviceObjListPtr to use hash tables

2017-07-24 Thread Erik Skultety
> >> Replies don't include your patch; however, I will note if you jump to > >> patch 13: > > > > What do you mean? Don't you see squash.patch in the attachment? (yes, I > > attached a patch instead of inlining it, the reason for that being that the > > patch is not particularly small and inlining

Re: [libvirt] [PATCH v3 4/4] storage: Disallow usage of the HBA for a fc_host backing

2017-07-24 Thread Erik Skultety
On Thu, Jul 20, 2017 at 03:48:49PM -0400, John Ferlan wrote: > Disallow providing the wwnn/wwpn of the HBA in the adapter XML: > >wwpn='HBA_wwpn'/> > > This should be considered a configuration error since a vHBA > would not be created. In order to use the HBA as the backing the >

[libvirt] [PATCH v2 1/5] nodedev: Introduce udevCheckMonitorFD helper function

2017-07-28 Thread Erik Skultety
of poking udev for device events. Signed-off-by: Erik Skultety <eskul...@redhat.com> --- src/node_device/node_device_udev.c | 56 +++--- 1 file changed, 34 insertions(+), 22 deletions(-) diff --git a/src/node_device/node_device_udev.c b/src/node_

[libvirt] [PATCH v2 3/5] udev: Convert udevEventHandleThread to an actual thread routine

2017-07-28 Thread Erik Skultety
and signals the worker thread to handle them. Signed-off-by: Erik Skultety <eskul...@redhat.com> --- src/node_device/node_device_udev.c | 131 ++--- 1 file changed, 108 insertions(+), 23 deletions(-) diff --git a/src/node_device/node_device_udev.c b/src/node_

[libvirt] [PATCH v2 0/5] Workaround mdev uevent race affecting node device driver

2017-07-28 Thread Erik Skultety
) and only acquires the driver lock when doing sanity checks on the udev monitor. [1] https://bugzilla.redhat.com/show_bug.cgi?id=1463285 [2] https://www.redhat.com/archives/libvir-list/2017-July/msg01077.html [3] https://github.com/eskultety/libvirt/commits/mdev-nodedev-next Erik Skultety (5

[libvirt] [PATCH v2 4/5] util: Introduce virFileWaitForAccess

2017-07-28 Thread Erik Skultety
have to duplicate this ugly workaround even more. Signed-off-by: Erik Skultety <eskul...@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virfile.c | 29 + src/util/virfile.h | 2 ++ 3 files changed, 32 insertions(+) diff --git

[libvirt] [PATCH v2 2/5] udev: Split udevEventHandleCallback in two functions

2017-07-28 Thread Erik Skultety
directly. Signed-off-by: Erik Skultety <eskul...@redhat.com> --- src/node_device/node_device_udev.c | 53 ++ 1 file changed, 36 insertions(+), 17 deletions(-) diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 0b4

[libvirt] [PATCH v2 5/5] nodedev: Work around the uevent race by hooking up virFileWaitForAccess

2017-07-28 Thread Erik Skultety
from our device list forever. If those don't appear in the given time frame, we need to move on, since libvirt can't wait indefinitely. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1463285 Signed-off-by: Erik Skultety <eskul...@redhat.com> --- src/node_device/node_device_udev.

Re: [libvirt] [PATCH] nodedev: Fix double unlock of the driver on udevEnumerateDevices failure

2017-07-28 Thread Erik Skultety
On Fri, Jul 28, 2017 at 11:31:27AM +0200, Michal Privoznik wrote: > On 07/26/2017 10:45 AM, Erik Skultety wrote: > > Commit @4cb719b2dc moved the driver locks around since these have become > > unnecessary at spots where the code handles now self-lockable object > > list, bu

Re: [libvirt] [PATCH v2 0/5] Workaround mdev uevent race affecting node device driver

2017-07-28 Thread Erik Skultety
On Fri, Jul 28, 2017 at 09:43:57AM +0200, Erik Skultety wrote: > v1 here https://www.redhat.com/archives/libvir-list/2017-June/msg00826.html > > This series addresses [1]. It builds on top of [2], series which introduces > a few small fixes and improvements. Even though that one h

Re: [libvirt] [PATCH] nodedev: Fix double unlock of the driver on udevEnumerateDevices failure

2017-07-26 Thread Erik Skultety
On Wed, Jul 26, 2017 at 08:36:04AM -0400, John Ferlan wrote: > > > On 07/26/2017 04:45 AM, Erik Skultety wrote: > > Commit @4cb719b2dc moved the driver locks around since these have become > > unnecessary at spots where the code handles now self-lockable object > > li

[libvirt] [PATCH 1/5] nodedev: mdev: Report an error when virFileResolveLink fails

2017-07-26 Thread Erik Skultety
It might happen that virFileResolveLinkHelper fails on the lstat system call. virFileResolveLink expects the caller to report an error when it fails, however this wasn't the case for udevProcessMediatedDevice. Signed-off-by: Erik Skultety <eskul...@redhat.com> --- src/node_

[libvirt] [PATCH 4/5] nodedev: Introduce udevHandleOneDevice

2017-07-26 Thread Erik Skultety
Let this new method handle the device object we obtained from the monitor in order to enhance readability. Signed-off-by: Erik Skultety <eskul...@redhat.com> --- src/node_device/node_device_udev.c | 31 ++- 1 file changed, 18 insertions(+), 13 deletions(-) diff

[libvirt] [PATCH 2/5] nodedev: udev: Remove the udevEventHandleCallback on fatal error

2017-07-26 Thread Erik Skultety
if someone somewhere somehow hits this hypothetical case. Signed-off-by: Erik Skultety <eskul...@redhat.com> --- src/node_device/node_device_udev.c | 9 + 1 file changed, 9 insertions(+) diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 80c

[libvirt] [PATCH 3/5] nodedev: Clear the udev_monitor reference once unref'd

2017-07-26 Thread Erik Skultety
. Signed-off-by: Erik Skultety <eskul...@redhat.com> --- src/node_device/node_device_udev.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index ea10dc3ce..cd19e79c1 100644 --- a/src/node_device/node_device_udev.c

[libvirt] [PATCH 5/5] nodedev: Protect every access to udev_monitor by locking the driver

2017-07-26 Thread Erik Skultety
Since @udev_monitor isn't immutable within the driver, we need to protect every access to it by locking the driver, so that no spurious action changing the pointer while one thread is actively using it might occur. This patch just takes some precaution measures. Signed-off-by: Erik Skultety

[libvirt] [PATCH] nodedev: Fix double unlock of the driver on udevEnumerateDevices failure

2017-07-26 Thread Erik Skultety
-by: Erik Skultety <eskul...@redhat.com> --- src/node_device/node_device_udev.c | 17 ++--- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 4762f1969..4c35fd5c9 100644 --- a/src/node_

[libvirt] [PATCH 0/5] A few fixes and improvements to the nodedev driver

2017-07-26 Thread Erik Skultety
These are just the few I came across when working on another nodedev-related series. Erik Skultety (5): nodedev: mdev: Report an error when virFileResolveLink fails nodedev: udev: Remove the udevEventHandleCallback on fatal error nodedev: Clear the udev_monitor reference once unref'd

Re: [libvirt] [PATCH v2] storage: Disallow usage of the HBA for a fc_host backing

2017-07-25 Thread Erik Skultety
> +/** > + * @name: Name from a wwnn/wwpn lookup > + * > + * Validate that the @name fetched from the wwnn/wwpn is a vHBA and not > + * an HBA as that should be a configuration error. It's only possible to > + * use an existing wwnn/wwpn of a vHBA because that's what someone would > + * have

Re: [libvirt] [PATCH v2 1/3] storage: Fix existing parent check for vHBA creation

2017-07-19 Thread Erik Skultety
On Tue, Jul 18, 2017 at 11:10:38AM -0400, John Ferlan wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1472277 > > Commit id '106930aaa' altered the order of checking for an existing > vHBA (e.g something created via nodedev-create functionality outside > of the storage pool logic) which

Re: [libvirt] [PATCH v2 3/3] conf: Fix vHBA checkParent logic for pool creation

2017-07-19 Thread Erik Skultety
On Tue, Jul 18, 2017 at 11:10:40AM -0400, John Ferlan wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1458708 > > When originally designed in commit id '42a021c1', providing the > 'parent' attribute to the wwpn='vHBA_wwpn'/> was checked to make sure that the "parent" of > the wwnn/wwpn

Re: [libvirt] [PATCH v5 1/3] nodedev: Fix usage of virNodeDeviceObjListGetParentHost

2017-07-19 Thread Erik Skultety
On Mon, Jul 17, 2017 at 11:02:22AM -0400, John Ferlan wrote: > In order to avoid an eventual possible race, modify the call to not > use the @def, but rather take the various fields needed. The race could > occur because the Destroy path needs to Unlock (and Unref) the object. > This could lead to

Re: [libvirt] [PATCH v2 3/3] conf: Fix vHBA checkParent logic for pool creation

2017-07-19 Thread Erik Skultety
> diff --git a/src/storage/storage_backend_scsi.c > b/src/storage/storage_backend_scsi.c > index 365ab77..d225e4f 100644 > --- a/src/storage/storage_backend_scsi.c > +++ b/src/storage/storage_backend_scsi.c > @@ -220,6 +220,7 @@ checkParent(virConnectPtr conn, > const char *name, >

Re: [libvirt] [PATCH v2 1/3] storage: Fix existing parent check for vHBA creation

2017-07-19 Thread Erik Skultety
On Wed, Jul 19, 2017 at 09:59:06AM -0400, John Ferlan wrote: > > > On 07/19/2017 07:38 AM, Erik Skultety wrote: > > On Tue, Jul 18, 2017 at 11:10:38AM -0400, John Ferlan wrote: > >> https://bugzilla.redhat.com/show_bug.cgi?id=1472277 > >> > >> Commit id

Re: [libvirt] [PATCH v5 2/3] nodedev: Convert virNodeDeviceObjListPtr to use hash tables

2017-07-24 Thread Erik Skultety
On Thu, Jul 20, 2017 at 04:32:46PM -0400, John Ferlan wrote: > > > On 07/20/2017 10:48 AM, Erik Skultety wrote: > >> @@ -39,13 +40,19 @@ struct _virNodeDeviceObj { > >> }; > >> > >> struct _virNodeDeviceObjList { > >>

Re: [libvirt] [PATCH v6 3/4] nodedev: Convert virNodeDeviceObjListPtr to use hash tables

2017-07-24 Thread Erik Skultety
On Thu, Jul 20, 2017 at 10:08:14AM -0400, John Ferlan wrote: > Rather than use a forward linked list of elements, it'll be much more > efficient to use a hash table to reference the elements by unique name > and to perform hash searches. > > This patch does all the heavy lifting of converting the

Re: [libvirt] [PATCH v6 4/4] nodedev: Remove driver locks around object list mgmt code

2017-07-24 Thread Erik Skultety
On Thu, Jul 20, 2017 at 10:08:15AM -0400, John Ferlan wrote: > Since virnodedeviceobj now has a self-lockable hash table, there's no > need to lock the table from the driver for processing. Thus remove the > locks from the driver for NodeDeviceObjList mgmt. > > This includes the test driver as

Re: [libvirt] [PATCH v6 3/4] nodedev: Convert virNodeDeviceObjListPtr to use hash tables

2017-07-24 Thread Erik Skultety
> >> virNodeDeviceObjPtr > >> virNodeDeviceObjListFindBySysfsPath(virNodeDeviceObjListPtr devs, > >> const char *sysfs_path) > >> { > >> -size_t i; > >> +virNodeDeviceObjPtr obj; > >> > >> -for (i = 0; i < devs->count; i++) { > >> -

Re: [libvirt] [PATCH v3 3/4] storage: Check if provided parent is vHBA capable

2017-07-24 Thread Erik Skultety
On Thu, Jul 20, 2017 at 03:48:48PM -0400, John Ferlan wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1458708 > > If the parent provided for the storage pool adapter is not vHBA > capable, then issue a configuration error even though the provided > wwnn/wwpn were found. > > It is a

Re: [libvirt] [PATCH v3 01/12] nodedev: Alter virNodeDeviceObjRemove

2017-06-30 Thread Erik Skultety
On Thu, Jun 29, 2017 at 12:07:42PM -0400, John Ferlan wrote: > > > On 06/29/2017 10:28 AM, Erik Skultety wrote: > > On Thu, Jun 29, 2017 at 09:57:09AM -0400, John Ferlan wrote: > >> > >> > >> On 06/29/2017 08:12 AM, Erik Skultety wrote: > >>>

Re: [libvirt] [PATCH v3 08/12] nodedev: Dereference the obj/def in virNodeDeviceObjListFind* APIs

2017-06-30 Thread Erik Skultety
On Sat, Jun 03, 2017 at 09:11:58AM -0400, John Ferlan wrote: > Create local @obj and @def for the API's rather than referencing the > devs->objs[i][->def->]. It'll make future patches easier to read. > > Signed-off-by: John Ferlan > --- > src/conf/virnodedeviceobj.c | 60 >

Re: [libvirt] [PATCH v3 06/12] nodedev: Introduce virNodeDeviceObjListNew

2017-06-30 Thread Erik Skultety
On Sat, Jun 03, 2017 at 09:11:56AM -0400, John Ferlan wrote: > In preparation to make things private, make the ->devs be pointers to a > virNodeDeviceObjList and then manage everything inside virnodedeviceobj > > Signed-off-by: John Ferlan > --- > src/conf/virnodedeviceobj.c

Re: [libvirt] [PATCH v3 01/12] nodedev: Alter virNodeDeviceObjRemove

2017-06-30 Thread Erik Skultety
On Fri, Jun 30, 2017 at 07:14:13AM -0400, John Ferlan wrote: > > > On 06/30/2017 04:40 AM, Erik Skultety wrote: > > On Thu, Jun 29, 2017 at 12:07:42PM -0400, John Ferlan wrote: > >> > >> > >> On 06/29/2017 10:28 AM, Erik Skultety wrote: > >>>

Re: [libvirt] [PATCH v3 07/12] nodedev: Alter node device obj list function names

2017-06-30 Thread Erik Skultety
On Sat, Jun 03, 2017 at 09:11:57AM -0400, John Ferlan wrote: > Ensure that any function that walks the node device object list is prefixed > by virNodeDeviceObjList. > > Also, modify the @filter param name for virNodeDeviceObjListExport to > be @aclfilter. > > Signed-off-by: John Ferlan

Re: [libvirt] [PATCH v3 11/12] nodedev: Privatize _virNodeDeviceObj and _virNodeDeviceObjList

2017-06-30 Thread Erik Skultety
On Sat, Jun 03, 2017 at 09:12:01AM -0400, John Ferlan wrote: > Move the structures to withing virnodedeviceobj.c > > Move the typedefs from node_device_conf to virnodedeviceobj.h > > Signed-off-by: John Ferlan ACK Erik -- libvir-list mailing list libvir-list@redhat.com

Re: [libvirt] [PATCH v3 02/12] test: Adjust cleanup/error paths for nodedev test APIs

2017-06-29 Thread Erik Skultety
> > Just a minor nit, while I was checking where testDestroyVport gets called > > from > > - testStoragePoolDestroy, there's a spot that could be fixed the same way. > > Would you mind squashing this bit in as well? > > > > diff --git a/src/test/test_driver.c b/src/test/test_driver.c > > index

<    6   7   8   9   10   11   12   13   14   15   >