Re: [libvirt PATCH v2 0/5] Add support for vDPA block devices

2023-09-13 Thread Erik Skultety
On Wed, Sep 13, 2023 at 12:57:09PM +0200, Erik Skultety wrote:
> On Wed, Sep 13, 2023 at 08:56:35AM +0100, Daniel P. Berrangé wrote:
> > On Tue, Sep 12, 2023 at 04:11:01PM -0500, Jonathon Jongsma wrote:
> > > On 9/12/23 7:00 AM, Peter Krempa wrote:
> > > > On Mon, Sep 11, 2023 at 16:53:42 -0500, Jonathon Jongsma wrote:
> > > > > see https://bugzilla.redhat.com/show_bug.cgi?id=1900770.
> > > > > 
> > > > > Changes in v2:
> > > > >   - Don't use virStorageSource->path for vdpa device path to avoid 
> > > > > clashing with
> > > > > existing path functionality
> > > > >   - Move vdpa device opening to the 
> > > > > qemuProcessPrepareHostStorageSource()
> > > > > function rather than the qemuDomainPrepareStorageSource() 
> > > > > function. This
> > > > > also required some additional support in the tests for setting up 
> > > > > the
> > > > > objects properly for testing.
> > > > >   - rebased to latest master branch
> > > > 
> > > > Reviewed-by: Peter Krempa 
> > > > 
> > > 
> > > I pushed this series, but for some reason only the ubuntu images are 
> > > failing
> > > CI with no useful output:
> > > https://gitlab.com/libvirt/libvirt/-/pipelines/1001459836
> > 
> > This is a behavioural regression from the recent CI refactoring of Eriks'.
> > 
> > We have lost the "--no-suite syntax-check --print-errorlogs" arguments
> > when running tests.
> 
> Sigh. I'm already running a pipeline with a fix that adds those test options
> back.
> 
> Erik

How about this one? 
https://listman.redhat.com/archives/libvir-list/2023-September/242071.html

Pipeline: https://gitlab.com/eskultety/libvirt/-/pipelines/1002436800

Erik



Re: [libvirt PATCH v2 0/5] Add support for vDPA block devices

2023-09-13 Thread Erik Skultety
On Wed, Sep 13, 2023 at 08:56:35AM +0100, Daniel P. Berrangé wrote:
> On Tue, Sep 12, 2023 at 04:11:01PM -0500, Jonathon Jongsma wrote:
> > On 9/12/23 7:00 AM, Peter Krempa wrote:
> > > On Mon, Sep 11, 2023 at 16:53:42 -0500, Jonathon Jongsma wrote:
> > > > see https://bugzilla.redhat.com/show_bug.cgi?id=1900770.
> > > > 
> > > > Changes in v2:
> > > >   - Don't use virStorageSource->path for vdpa device path to avoid 
> > > > clashing with
> > > > existing path functionality
> > > >   - Move vdpa device opening to the 
> > > > qemuProcessPrepareHostStorageSource()
> > > > function rather than the qemuDomainPrepareStorageSource() function. 
> > > > This
> > > > also required some additional support in the tests for setting up 
> > > > the
> > > > objects properly for testing.
> > > >   - rebased to latest master branch
> > > 
> > > Reviewed-by: Peter Krempa 
> > > 
> > 
> > I pushed this series, but for some reason only the ubuntu images are failing
> > CI with no useful output:
> > https://gitlab.com/libvirt/libvirt/-/pipelines/1001459836
> 
> This is a behavioural regression from the recent CI refactoring of Eriks'.
> 
> We have lost the "--no-suite syntax-check --print-errorlogs" arguments
> when running tests.

Sigh. I'm already running a pipeline with a fix that adds those test options
back.

Erik



Re: [libvirt PATCH v2 0/5] Add support for vDPA block devices

2023-09-13 Thread Daniel P . Berrangé
On Tue, Sep 12, 2023 at 04:11:01PM -0500, Jonathon Jongsma wrote:
> On 9/12/23 7:00 AM, Peter Krempa wrote:
> > On Mon, Sep 11, 2023 at 16:53:42 -0500, Jonathon Jongsma wrote:
> > > see https://bugzilla.redhat.com/show_bug.cgi?id=1900770.
> > > 
> > > Changes in v2:
> > >   - Don't use virStorageSource->path for vdpa device path to avoid 
> > > clashing with
> > > existing path functionality
> > >   - Move vdpa device opening to the qemuProcessPrepareHostStorageSource()
> > > function rather than the qemuDomainPrepareStorageSource() function. 
> > > This
> > > also required some additional support in the tests for setting up the
> > > objects properly for testing.
> > >   - rebased to latest master branch
> > 
> > Reviewed-by: Peter Krempa 
> > 
> 
> I pushed this series, but for some reason only the ubuntu images are failing
> CI with no useful output:
> https://gitlab.com/libvirt/libvirt/-/pipelines/1001459836

This is a behavioural regression from the recent CI refactoring of Eriks'.

We have lost the "--no-suite syntax-check --print-errorlogs" arguments
when running tests.

With 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 :|



Re: [libvirt PATCH v2 0/5] Add support for vDPA block devices

2023-09-13 Thread Peter Krempa
On Tue, Sep 12, 2023 at 16:11:01 -0500, Jonathon Jongsma wrote:
> On 9/12/23 7:00 AM, Peter Krempa wrote:
> > On Mon, Sep 11, 2023 at 16:53:42 -0500, Jonathon Jongsma wrote:
> > > see https://bugzilla.redhat.com/show_bug.cgi?id=1900770.
> > > 
> > > Changes in v2:
> > >   - Don't use virStorageSource->path for vdpa device path to avoid 
> > > clashing with
> > > existing path functionality
> > >   - Move vdpa device opening to the qemuProcessPrepareHostStorageSource()
> > > function rather than the qemuDomainPrepareStorageSource() function. 
> > > This
> > > also required some additional support in the tests for setting up the
> > > objects properly for testing.
> > >   - rebased to latest master branch
> > 
> > Reviewed-by: Peter Krempa 
> > 
> 
> I pushed this series, but for some reason only the ubuntu images are failing
> CI with no useful output:
> https://gitlab.com/libvirt/libvirt/-/pipelines/1001459836
> 
> I suspect it may be related to address sanitizer stuff, does anybody have
> tips on getting more information about this failure?

Usually valgrind does a good job of finding everyhting the sanitizer
complains about:

 $ valgrind --trace-children=yes --leak-check=full ./tests/qemuxml2argvtest

[...]

==352693== 18 bytes in 1 blocks are definitely lost in loss record 257 of 684
==352693==at 0x484182F: malloc (vg_replace_malloc.c:431)
==352693==by 0x51DA07F: xmlStrndup (in /usr/lib64/libxml2.so.2.10.4)
==352693==by 0x49D918E: virXMLPropStringRequired (virxml.c:321)
==352693==by 0x4A0D866: virDomainStorageSourceParse (domain_conf.c:7526)
==352693==by 0x4A0E9F0: virDomainDiskDefParseSourceXML (domain_conf.c:7926)
==352693==by 0x4A0ECCF: virDomainDiskDefParseXML (domain_conf.c:8000)
==352693==by 0x4A2DA93: virDomainDefParseXML (domain_conf.c:18767)
==352693==by 0x4A31678: virDomainDefParseNode (domain_conf.c:19557)
==352693==by 0x134000: testCompareXMLToArgv (qemuxml2argvtest.c:566)
==352693==by 0x1356B9: virTestRun (testutils.c:143)
==352693==by 0x135940: virTestRunLog (testutils.c:198)
==352693==by 0x11D422: mymain (qemuxml2argvtest.c:1164)


Looks like the new field is not freed. I'll post the patch soon



Re: [libvirt PATCH v2 0/5] Add support for vDPA block devices

2023-09-12 Thread Jonathon Jongsma

On 9/12/23 7:00 AM, Peter Krempa wrote:

On Mon, Sep 11, 2023 at 16:53:42 -0500, Jonathon Jongsma wrote:

see https://bugzilla.redhat.com/show_bug.cgi?id=1900770.

Changes in v2:
  - Don't use virStorageSource->path for vdpa device path to avoid clashing with
existing path functionality
  - Move vdpa device opening to the qemuProcessPrepareHostStorageSource()
function rather than the qemuDomainPrepareStorageSource() function. This
also required some additional support in the tests for setting up the
objects properly for testing.
  - rebased to latest master branch


Reviewed-by: Peter Krempa 



I pushed this series, but for some reason only the ubuntu images are 
failing CI with no useful output: 
https://gitlab.com/libvirt/libvirt/-/pipelines/1001459836


I suspect it may be related to address sanitizer stuff, does anybody 
have tips on getting more information about this failure?


Jonathon



Re: [libvirt PATCH v2 0/5] Add support for vDPA block devices

2023-09-12 Thread Peter Krempa
On Mon, Sep 11, 2023 at 16:53:42 -0500, Jonathon Jongsma wrote:
> see https://bugzilla.redhat.com/show_bug.cgi?id=1900770.
> 
> Changes in v2:
>  - Don't use virStorageSource->path for vdpa device path to avoid clashing 
> with
>existing path functionality
>  - Move vdpa device opening to the qemuProcessPrepareHostStorageSource()
>function rather than the qemuDomainPrepareStorageSource() function. This
>also required some additional support in the tests for setting up the
>objects properly for testing.
>  - rebased to latest master branch

Reviewed-by: Peter Krempa 



[libvirt PATCH v2 0/5] Add support for vDPA block devices

2023-09-11 Thread Jonathon Jongsma
see https://bugzilla.redhat.com/show_bug.cgi?id=1900770.

Changes in v2:
 - Don't use virStorageSource->path for vdpa device path to avoid clashing with
   existing path functionality
 - Move vdpa device opening to the qemuProcessPrepareHostStorageSource()
   function rather than the qemuDomainPrepareStorageSource() function. This
   also required some additional support in the tests for setting up the
   objects properly for testing.
 - rebased to latest master branch

Jonathon Jongsma (5):
  conf: add ability to configure a vdpa block disk device
  qemu: add virtio-blk-vhost-vdpa capability
  qemu: make vdpa connect function more generic
  qemu: consider vdpa block devices for memlock limits
  qemu: Implement support for vDPA block devices

 docs/formatdomain.rst | 19 +-
 src/ch/ch_monitor.c   |  1 +
 src/conf/domain_conf.c|  8 
 src/conf/schemas/domaincommon.rng | 13 +++
 src/conf/storage_source_conf.c|  7 +++-
 src/conf/storage_source_conf.h|  2 +
 src/libxl/xen_xl.c|  1 +
 src/qemu/qemu_block.c | 20 ++
 src/qemu/qemu_capabilities.c  |  2 +
 src/qemu/qemu_capabilities.h  |  1 +
 src/qemu/qemu_command.c   | 24 +++-
 src/qemu/qemu_command.h   |  1 +
 src/qemu/qemu_domain.c| 12 +-
 src/qemu/qemu_interface.c | 23 
 src/qemu/qemu_interface.h |  2 -
 src/qemu/qemu_migration.c |  2 +
 src/qemu/qemu_process.c   | 34 +
 src/qemu/qemu_snapshot.c  |  4 ++
 src/qemu/qemu_validate.c  | 33 +++--
 src/storage_file/storage_source.c |  1 +
 .../caps_8.1.0_x86_64.xml |  1 +
 tests/qemuhotplugmock.c   |  4 +-
 .../disk-vhostvdpa.x86_64-latest.args | 37 +++
 tests/qemuxml2argvdata/disk-vhostvdpa.xml | 21 +++
 tests/qemuxml2argvmock.c  |  2 +-
 tests/qemuxml2argvtest.c  | 34 +
 tests/testutilsqemu.c | 11 ++
 tests/testutilsqemu.h |  2 +
 28 files changed, 285 insertions(+), 37 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/disk-vhostvdpa.x86_64-latest.args
 create mode 100644 tests/qemuxml2argvdata/disk-vhostvdpa.xml

-- 
2.41.0