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

Jonathon Jongsma posted 5 patches 7 months, 1 week ago
Failed in applying to current master (apply log)
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
[libvirt PATCH v2 0/5] Add support for vDPA block devices
Posted by Jonathon Jongsma 7 months, 1 week ago
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
Re: [libvirt PATCH v2 0/5] Add support for vDPA block devices
Posted by Peter Krempa 7 months, 1 week ago
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 <pkrempa@redhat.com>
Re: [libvirt PATCH v2 0/5] Add support for vDPA block devices
Posted by Jonathon Jongsma 7 months, 1 week ago
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 <pkrempa@redhat.com>
> 

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
Posted by Daniel P. Berrangé 7 months, 1 week ago
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 <pkrempa@redhat.com>
> > 
> 
> 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
Posted by Erik Skultety 7 months, 1 week ago
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 <pkrempa@redhat.com>
> > > 
> > 
> > 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
Posted by Erik Skultety 7 months, 1 week ago
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 <pkrempa@redhat.com>
> > > > 
> > > 
> > > 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
Posted by Peter Krempa 7 months, 1 week ago
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 <pkrempa@redhat.com>
> > 
> 
> 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