[PATCH v5 00/12] nodedev state and update

Boris Fiuczynski posted 12 patches 2 months ago
Failed in applying to current master (apply log)
NEWS.rst                                      |  12 +
docs/drvnodedev.rst                           |   4 +-
docs/manpages/virsh.rst                       |  36 +-
include/libvirt/libvirt-nodedev.h             |  31 ++
libvirt.spec.in                               |   1 +
src/access/viraccessperm.c                    |   1 +
src/access/viraccessperm.h                    |   6 +
src/conf/node_device_conf.c                   |  76 +--
src/conf/node_device_conf.h                   |  14 +-
src/conf/virnodedeviceobj.c                   |  50 ++
src/conf/virnodedeviceobj.h                   |   3 +
src/driver-nodedev.h                          |   6 +
src/libvirt-nodedev.c                         |  51 +-
src/libvirt_private.syms                      |   1 +
src/libvirt_public.syms                       |   5 +
src/node_device/node_device_driver.c          | 474 ++++++++++++++----
src/node_device/node_device_driver.h          |  17 +-
src/node_device/node_device_udev.c            |   5 +-
src/remote/remote_driver.c                    |   1 +
src/remote/remote_protocol.x                  |  17 +-
src/remote_protocol-structs                   |   6 +
src/test/test_driver.c                        |   6 +-
src/util/virmdev.h                            |   6 +
...60c_c60c_c60c_c60c_c60cc60cc60c_update.xml |  16 +
tests/nodedevmdevctldata/mdevctl-modify.argv  |  25 +
tests/nodedevmdevctldata/mdevctl-modify.json  |   4 +
tests/nodedevmdevctltest.c                    |  94 +++-
...v_c60cc60c_c60c_c60c_c60c_c60cc60cc60c.xml |  14 +
...d_b7f0_4fea_b468_f1da537d301b_inactive.xml |   1 +
...v_c60cc60c_c60c_c60c_c60c_c60cc60cc60c.xml |  10 +
...c_c60c_c60c_c60c_c60cc60cc60c_inactive.xml |   9 +
...9_36ea_4111_8f0a_8c9a70e21366_inactive.xml |   1 +
...9_495e_4243_ad9f_beb3f14c23d9_inactive.xml |   1 +
...4_f554_4dc1_809d_b2a01e8e48ad_inactive.xml |   8 +
...6_1ca8_49ac_b176_871d16c13076_inactive.xml |   1 +
tests/nodedevxml2xmltest.c                    |  59 ++-
tools/virsh-nodedev.c                         | 140 +++++-
37 files changed, 1057 insertions(+), 155 deletions(-)
create mode 100644 tests/nodedevmdevctldata/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c_update.xml
create mode 100644 tests/nodedevmdevctldata/mdevctl-modify.argv
create mode 100644 tests/nodedevmdevctldata/mdevctl-modify.json
create mode 100644 tests/nodedevschemadata/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c.xml
create mode 120000 tests/nodedevxml2xmlout/mdev_3627463d_b7f0_4fea_b468_f1da537d301b_inactive.xml
create mode 100644 tests/nodedevxml2xmlout/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c.xml
create mode 100644 tests/nodedevxml2xmlout/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c_inactive.xml
create mode 120000 tests/nodedevxml2xmlout/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366_inactive.xml
create mode 120000 tests/nodedevxml2xmlout/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9_inactive.xml
create mode 100644 tests/nodedevxml2xmlout/mdev_ee0b88c4_f554_4dc1_809d_b2a01e8e48ad_inactive.xml
create mode 120000 tests/nodedevxml2xmlout/mdev_fedc4916_1ca8_49ac_b176_871d16c13076_inactive.xml
[PATCH v5 00/12] nodedev state and update
Posted by Boris Fiuczynski 2 months ago
The series adds a dual state to the mdev node devices as these objects
can be active and defined at the same time. These two states can
become different. To be able to also introspect the persistent and
transient nodedevs filtering is added. To be able to also dump the XML
of an inactive state while the node device is active a new option is
added.

The last four patches add the capability to update a mdev node device.
This can be done on the persistent configuration, on the active
configuration or on both. To support this v1.3.0 of mdevctl is required.
nodeDeviceDefineXML() does now support modifying a persistent configuration.

Changes since v4:
* reworked findPersistentMdevNodeDevice()
* changed the way the split into the modify path and the cleanup/unlocking is
  done in nodeDeviceDefineXML()

Changes since v3:
* replaced in all patches occurrences of persisted with persistent

Changes since v2:
* made error messages in virNodeDeviceObjUpdateModificationImpact() device
  type agnostic
* renamed virNodeDeviceUpdateXML* into virNodeDeviceUpdate*
* renamed nodeDeviceDefCompareMdevs() into nodeDeviceDefValidateUpdate()
* renamed multiple local variable names
* removed method for config cloning by commenting cross config compare in
  nodeDeviceDefValidateUpdate()
* changed nodeDeviceDefineXML() to modify an existing persistent configuration

Changes since v1:
* replaced spec file requirement for v1.3.0 of mdevctl by a dynamic
  support check and an unsupported message if not available
* renamed persisted and persist into persistent
* removed persistent precheck in virsh
* addressed all other review comments made on v1
* added NEWS

Boris Fiuczynski (12):
  virmdev: prepare type and attributes for dual state
  node_device: refactor mdev attributes handling
  node_device: remove unnecessary checks in virNodeDeviceDefFormat
  nodedev: add an active config to mdev
  tools: add option inactive to nodedev-dumpxml
  nodedev: add persistent and transient filter on list
  tools: add switches persistent and transient to nodedev-list
  virsh: doc fix on nodedev-list
  api: add virNodeDeviceUpdate()
  nodedev: Implement virNodeDeviceUpdate
  virsh: add nodedev-update
  nodedev: allow modify on define of a persistent node device

 NEWS.rst                                      |  12 +
 docs/drvnodedev.rst                           |   4 +-
 docs/manpages/virsh.rst                       |  36 +-
 include/libvirt/libvirt-nodedev.h             |  31 ++
 libvirt.spec.in                               |   1 +
 src/access/viraccessperm.c                    |   1 +
 src/access/viraccessperm.h                    |   6 +
 src/conf/node_device_conf.c                   |  76 +--
 src/conf/node_device_conf.h                   |  14 +-
 src/conf/virnodedeviceobj.c                   |  50 ++
 src/conf/virnodedeviceobj.h                   |   3 +
 src/driver-nodedev.h                          |   6 +
 src/libvirt-nodedev.c                         |  51 +-
 src/libvirt_private.syms                      |   1 +
 src/libvirt_public.syms                       |   5 +
 src/node_device/node_device_driver.c          | 474 ++++++++++++++----
 src/node_device/node_device_driver.h          |  17 +-
 src/node_device/node_device_udev.c            |   5 +-
 src/remote/remote_driver.c                    |   1 +
 src/remote/remote_protocol.x                  |  17 +-
 src/remote_protocol-structs                   |   6 +
 src/test/test_driver.c                        |   6 +-
 src/util/virmdev.h                            |   6 +
 ...60c_c60c_c60c_c60c_c60cc60cc60c_update.xml |  16 +
 tests/nodedevmdevctldata/mdevctl-modify.argv  |  25 +
 tests/nodedevmdevctldata/mdevctl-modify.json  |   4 +
 tests/nodedevmdevctltest.c                    |  94 +++-
 ...v_c60cc60c_c60c_c60c_c60c_c60cc60cc60c.xml |  14 +
 ...d_b7f0_4fea_b468_f1da537d301b_inactive.xml |   1 +
 ...v_c60cc60c_c60c_c60c_c60c_c60cc60cc60c.xml |  10 +
 ...c_c60c_c60c_c60c_c60cc60cc60c_inactive.xml |   9 +
 ...9_36ea_4111_8f0a_8c9a70e21366_inactive.xml |   1 +
 ...9_495e_4243_ad9f_beb3f14c23d9_inactive.xml |   1 +
 ...4_f554_4dc1_809d_b2a01e8e48ad_inactive.xml |   8 +
 ...6_1ca8_49ac_b176_871d16c13076_inactive.xml |   1 +
 tests/nodedevxml2xmltest.c                    |  59 ++-
 tools/virsh-nodedev.c                         | 140 +++++-
 37 files changed, 1057 insertions(+), 155 deletions(-)
 create mode 100644 tests/nodedevmdevctldata/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c_update.xml
 create mode 100644 tests/nodedevmdevctldata/mdevctl-modify.argv
 create mode 100644 tests/nodedevmdevctldata/mdevctl-modify.json
 create mode 100644 tests/nodedevschemadata/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c.xml
 create mode 120000 tests/nodedevxml2xmlout/mdev_3627463d_b7f0_4fea_b468_f1da537d301b_inactive.xml
 create mode 100644 tests/nodedevxml2xmlout/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c.xml
 create mode 100644 tests/nodedevxml2xmlout/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c_inactive.xml
 create mode 120000 tests/nodedevxml2xmlout/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366_inactive.xml
 create mode 120000 tests/nodedevxml2xmlout/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9_inactive.xml
 create mode 100644 tests/nodedevxml2xmlout/mdev_ee0b88c4_f554_4dc1_809d_b2a01e8e48ad_inactive.xml
 create mode 120000 tests/nodedevxml2xmlout/mdev_fedc4916_1ca8_49ac_b176_871d16c13076_inactive.xml

-- 
2.42.0
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH v5 00/12] nodedev state and update
Posted by Jonathon Jongsma 2 months ago
It looks OK to me, but I'd personally like to see a second ACK on at 
least the API-related changes.

Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>



On 2/22/24 7:01 AM, Boris Fiuczynski wrote:
> The series adds a dual state to the mdev node devices as these objects
> can be active and defined at the same time. These two states can
> become different. To be able to also introspect the persistent and
> transient nodedevs filtering is added. To be able to also dump the XML
> of an inactive state while the node device is active a new option is
> added.
> 
> The last four patches add the capability to update a mdev node device.
> This can be done on the persistent configuration, on the active
> configuration or on both. To support this v1.3.0 of mdevctl is required.
> nodeDeviceDefineXML() does now support modifying a persistent configuration.
> 
> Changes since v4:
> * reworked findPersistentMdevNodeDevice()
> * changed the way the split into the modify path and the cleanup/unlocking is
>    done in nodeDeviceDefineXML()
> 
> Changes since v3:
> * replaced in all patches occurrences of persisted with persistent
> 
> Changes since v2:
> * made error messages in virNodeDeviceObjUpdateModificationImpact() device
>    type agnostic
> * renamed virNodeDeviceUpdateXML* into virNodeDeviceUpdate*
> * renamed nodeDeviceDefCompareMdevs() into nodeDeviceDefValidateUpdate()
> * renamed multiple local variable names
> * removed method for config cloning by commenting cross config compare in
>    nodeDeviceDefValidateUpdate()
> * changed nodeDeviceDefineXML() to modify an existing persistent configuration
> 
> Changes since v1:
> * replaced spec file requirement for v1.3.0 of mdevctl by a dynamic
>    support check and an unsupported message if not available
> * renamed persisted and persist into persistent
> * removed persistent precheck in virsh
> * addressed all other review comments made on v1
> * added NEWS
> 
> Boris Fiuczynski (12):
>    virmdev: prepare type and attributes for dual state
>    node_device: refactor mdev attributes handling
>    node_device: remove unnecessary checks in virNodeDeviceDefFormat
>    nodedev: add an active config to mdev
>    tools: add option inactive to nodedev-dumpxml
>    nodedev: add persistent and transient filter on list
>    tools: add switches persistent and transient to nodedev-list
>    virsh: doc fix on nodedev-list
>    api: add virNodeDeviceUpdate()
>    nodedev: Implement virNodeDeviceUpdate
>    virsh: add nodedev-update
>    nodedev: allow modify on define of a persistent node device
> 
>   NEWS.rst                                      |  12 +
>   docs/drvnodedev.rst                           |   4 +-
>   docs/manpages/virsh.rst                       |  36 +-
>   include/libvirt/libvirt-nodedev.h             |  31 ++
>   libvirt.spec.in                               |   1 +
>   src/access/viraccessperm.c                    |   1 +
>   src/access/viraccessperm.h                    |   6 +
>   src/conf/node_device_conf.c                   |  76 +--
>   src/conf/node_device_conf.h                   |  14 +-
>   src/conf/virnodedeviceobj.c                   |  50 ++
>   src/conf/virnodedeviceobj.h                   |   3 +
>   src/driver-nodedev.h                          |   6 +
>   src/libvirt-nodedev.c                         |  51 +-
>   src/libvirt_private.syms                      |   1 +
>   src/libvirt_public.syms                       |   5 +
>   src/node_device/node_device_driver.c          | 474 ++++++++++++++----
>   src/node_device/node_device_driver.h          |  17 +-
>   src/node_device/node_device_udev.c            |   5 +-
>   src/remote/remote_driver.c                    |   1 +
>   src/remote/remote_protocol.x                  |  17 +-
>   src/remote_protocol-structs                   |   6 +
>   src/test/test_driver.c                        |   6 +-
>   src/util/virmdev.h                            |   6 +
>   ...60c_c60c_c60c_c60c_c60cc60cc60c_update.xml |  16 +
>   tests/nodedevmdevctldata/mdevctl-modify.argv  |  25 +
>   tests/nodedevmdevctldata/mdevctl-modify.json  |   4 +
>   tests/nodedevmdevctltest.c                    |  94 +++-
>   ...v_c60cc60c_c60c_c60c_c60c_c60cc60cc60c.xml |  14 +
>   ...d_b7f0_4fea_b468_f1da537d301b_inactive.xml |   1 +
>   ...v_c60cc60c_c60c_c60c_c60c_c60cc60cc60c.xml |  10 +
>   ...c_c60c_c60c_c60c_c60cc60cc60c_inactive.xml |   9 +
>   ...9_36ea_4111_8f0a_8c9a70e21366_inactive.xml |   1 +
>   ...9_495e_4243_ad9f_beb3f14c23d9_inactive.xml |   1 +
>   ...4_f554_4dc1_809d_b2a01e8e48ad_inactive.xml |   8 +
>   ...6_1ca8_49ac_b176_871d16c13076_inactive.xml |   1 +
>   tests/nodedevxml2xmltest.c                    |  59 ++-
>   tools/virsh-nodedev.c                         | 140 +++++-
>   37 files changed, 1057 insertions(+), 155 deletions(-)
>   create mode 100644 tests/nodedevmdevctldata/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c_update.xml
>   create mode 100644 tests/nodedevmdevctldata/mdevctl-modify.argv
>   create mode 100644 tests/nodedevmdevctldata/mdevctl-modify.json
>   create mode 100644 tests/nodedevschemadata/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c.xml
>   create mode 120000 tests/nodedevxml2xmlout/mdev_3627463d_b7f0_4fea_b468_f1da537d301b_inactive.xml
>   create mode 100644 tests/nodedevxml2xmlout/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c.xml
>   create mode 100644 tests/nodedevxml2xmlout/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c_inactive.xml
>   create mode 120000 tests/nodedevxml2xmlout/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366_inactive.xml
>   create mode 120000 tests/nodedevxml2xmlout/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9_inactive.xml
>   create mode 100644 tests/nodedevxml2xmlout/mdev_ee0b88c4_f554_4dc1_809d_b2a01e8e48ad_inactive.xml
>   create mode 120000 tests/nodedevxml2xmlout/mdev_fedc4916_1ca8_49ac_b176_871d16c13076_inactive.xml
> 
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH v5 00/12] nodedev state and update
Posted by Boris Fiuczynski 2 months ago
On 2/22/24 20:04, Jonathon Jongsma wrote:
> It looks OK to me, but I'd personally like to see a second ACK on at 
> least the API-related changes.
> 
> Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
> 

@Jonathon,
thanks for your review.

@all,
I would really appreciate it if someone could take a look at the series 
before the freeze. Thanks.

> 
> 
> On 2/22/24 7:01 AM, Boris Fiuczynski wrote:
>> The series adds a dual state to the mdev node devices as these objects
>> can be active and defined at the same time. These two states can
>> become different. To be able to also introspect the persistent and
>> transient nodedevs filtering is added. To be able to also dump the XML
>> of an inactive state while the node device is active a new option is
>> added.
>>
>> The last four patches add the capability to update a mdev node device.
>> This can be done on the persistent configuration, on the active
>> configuration or on both. To support this v1.3.0 of mdevctl is required.
>> nodeDeviceDefineXML() does now support modifying a persistent 
>> configuration.
>>
>> Changes since v4:
>> * reworked findPersistentMdevNodeDevice()
>> * changed the way the split into the modify path and the 
>> cleanup/unlocking is
>>    done in nodeDeviceDefineXML()
>>
>> Changes since v3:
>> * replaced in all patches occurrences of persisted with persistent
>>
>> Changes since v2:
>> * made error messages in virNodeDeviceObjUpdateModificationImpact() 
>> device
>>    type agnostic
>> * renamed virNodeDeviceUpdateXML* into virNodeDeviceUpdate*
>> * renamed nodeDeviceDefCompareMdevs() into nodeDeviceDefValidateUpdate()
>> * renamed multiple local variable names
>> * removed method for config cloning by commenting cross config compare in
>>    nodeDeviceDefValidateUpdate()
>> * changed nodeDeviceDefineXML() to modify an existing persistent 
>> configuration
>>
>> Changes since v1:
>> * replaced spec file requirement for v1.3.0 of mdevctl by a dynamic
>>    support check and an unsupported message if not available
>> * renamed persisted and persist into persistent
>> * removed persistent precheck in virsh
>> * addressed all other review comments made on v1
>> * added NEWS
>>
>> Boris Fiuczynski (12):
>>    virmdev: prepare type and attributes for dual state
>>    node_device: refactor mdev attributes handling
>>    node_device: remove unnecessary checks in virNodeDeviceDefFormat
>>    nodedev: add an active config to mdev
>>    tools: add option inactive to nodedev-dumpxml
>>    nodedev: add persistent and transient filter on list
>>    tools: add switches persistent and transient to nodedev-list
>>    virsh: doc fix on nodedev-list
>>    api: add virNodeDeviceUpdate()
>>    nodedev: Implement virNodeDeviceUpdate
>>    virsh: add nodedev-update
>>    nodedev: allow modify on define of a persistent node device
>>
>>   NEWS.rst                                      |  12 +
>>   docs/drvnodedev.rst                           |   4 +-
>>   docs/manpages/virsh.rst                       |  36 +-
>>   include/libvirt/libvirt-nodedev.h             |  31 ++
>>   libvirt.spec.in                               |   1 +
>>   src/access/viraccessperm.c                    |   1 +
>>   src/access/viraccessperm.h                    |   6 +
>>   src/conf/node_device_conf.c                   |  76 +--
>>   src/conf/node_device_conf.h                   |  14 +-
>>   src/conf/virnodedeviceobj.c                   |  50 ++
>>   src/conf/virnodedeviceobj.h                   |   3 +
>>   src/driver-nodedev.h                          |   6 +
>>   src/libvirt-nodedev.c                         |  51 +-
>>   src/libvirt_private.syms                      |   1 +
>>   src/libvirt_public.syms                       |   5 +
>>   src/node_device/node_device_driver.c          | 474 ++++++++++++++----
>>   src/node_device/node_device_driver.h          |  17 +-
>>   src/node_device/node_device_udev.c            |   5 +-
>>   src/remote/remote_driver.c                    |   1 +
>>   src/remote/remote_protocol.x                  |  17 +-
>>   src/remote_protocol-structs                   |   6 +
>>   src/test/test_driver.c                        |   6 +-
>>   src/util/virmdev.h                            |   6 +
>>   ...60c_c60c_c60c_c60c_c60cc60cc60c_update.xml |  16 +
>>   tests/nodedevmdevctldata/mdevctl-modify.argv  |  25 +
>>   tests/nodedevmdevctldata/mdevctl-modify.json  |   4 +
>>   tests/nodedevmdevctltest.c                    |  94 +++-
>>   ...v_c60cc60c_c60c_c60c_c60c_c60cc60cc60c.xml |  14 +
>>   ...d_b7f0_4fea_b468_f1da537d301b_inactive.xml |   1 +
>>   ...v_c60cc60c_c60c_c60c_c60c_c60cc60cc60c.xml |  10 +
>>   ...c_c60c_c60c_c60c_c60cc60cc60c_inactive.xml |   9 +
>>   ...9_36ea_4111_8f0a_8c9a70e21366_inactive.xml |   1 +
>>   ...9_495e_4243_ad9f_beb3f14c23d9_inactive.xml |   1 +
>>   ...4_f554_4dc1_809d_b2a01e8e48ad_inactive.xml |   8 +
>>   ...6_1ca8_49ac_b176_871d16c13076_inactive.xml |   1 +
>>   tests/nodedevxml2xmltest.c                    |  59 ++-
>>   tools/virsh-nodedev.c                         | 140 +++++-
>>   37 files changed, 1057 insertions(+), 155 deletions(-)
>>   create mode 100644 
>> tests/nodedevmdevctldata/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c_update.xml
>>   create mode 100644 tests/nodedevmdevctldata/mdevctl-modify.argv
>>   create mode 100644 tests/nodedevmdevctldata/mdevctl-modify.json
>>   create mode 100644 
>> tests/nodedevschemadata/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c.xml
>>   create mode 120000 
>> tests/nodedevxml2xmlout/mdev_3627463d_b7f0_4fea_b468_f1da537d301b_inactive.xml
>>   create mode 100644 
>> tests/nodedevxml2xmlout/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c.xml
>>   create mode 100644 
>> tests/nodedevxml2xmlout/mdev_c60cc60c_c60c_c60c_c60c_c60cc60cc60c_inactive.xml
>>   create mode 120000 
>> tests/nodedevxml2xmlout/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366_inactive.xml
>>   create mode 120000 
>> tests/nodedevxml2xmlout/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9_inactive.xml
>>   create mode 100644 
>> tests/nodedevxml2xmlout/mdev_ee0b88c4_f554_4dc1_809d_b2a01e8e48ad_inactive.xml
>>   create mode 120000 
>> tests/nodedevxml2xmlout/mdev_fedc4916_1ca8_49ac_b176_871d16c13076_inactive.xml
>>
> _______________________________________________
> Devel mailing list -- devel@lists.libvirt.org
> To unsubscribe send an email to devel-leave@lists.libvirt.org

-- 
Mit freundlichen Grüßen/Kind regards
    Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Wolfgang Wendt
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH v5 00/12] nodedev state and update
Posted by Michal Prívozník 2 months ago
On 2/23/24 18:38, Boris Fiuczynski wrote:
> On 2/22/24 20:04, Jonathon Jongsma wrote:
>> It looks OK to me, but I'd personally like to see a second ACK on at
>> least the API-related changes.
>>
>> Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
>>
> 
> @Jonathon,
> thanks for your review.
> 
> @all,
> I would really appreciate it if someone could take a look at the series
> before the freeze. Thanks.

Indeed, I've marked this for review but then got sidetracked, sorry.

Anyway, the code looks good. I just cleaned it up a bit (white spaces,
comments, curly braces) - nothing major. Appended Jonathon's Reviewed-by
where it was missing and merged (since Jirka wants to freeze right about
now).

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH v5 00/12] nodedev state and update
Posted by Andrea Bolognani 2 months ago
On Mon, Feb 26, 2024 at 11:21:47AM +0100, Michal Prívozník wrote:
> On 2/23/24 18:38, Boris Fiuczynski wrote:
> > On 2/22/24 20:04, Jonathon Jongsma wrote:
> >> It looks OK to me, but I'd personally like to see a second ACK on at
> >> least the API-related changes.
> >>
> >> Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
> >
> > @Jonathon,
> > thanks for your review.
> >
> > @all,
> > I would really appreciate it if someone could take a look at the series
> > before the freeze. Thanks.
>
> Indeed, I've marked this for review but then got sidetracked, sorry.
>
> Anyway, the code looks good. I just cleaned it up a bit (white spaces,
> comments, curly braces) - nothing major. Appended Jonathon's Reviewed-by
> where it was missing and merged (since Jirka wants to freeze right about
> now).
>
> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

This seems to have introduced a memory leak. See the failures
reported by the Ubuntu jobs in

https://gitlab.com/libvirt/libvirt/-/pipelines/1190698985

The previous pipeline was successful.

-- 
Andrea Bolognani / Red Hat / Virtualization
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH v5 00/12] nodedev state and update
Posted by Michal Prívozník 2 months ago
On 2/26/24 12:36, Andrea Bolognani wrote:
> On Mon, Feb 26, 2024 at 11:21:47AM +0100, Michal Prívozník wrote:
>> On 2/23/24 18:38, Boris Fiuczynski wrote:
>>> On 2/22/24 20:04, Jonathon Jongsma wrote:
>>>> It looks OK to me, but I'd personally like to see a second ACK on at
>>>> least the API-related changes.
>>>>
>>>> Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
>>>
>>> @Jonathon,
>>> thanks for your review.
>>>
>>> @all,
>>> I would really appreciate it if someone could take a look at the series
>>> before the freeze. Thanks.
>>
>> Indeed, I've marked this for review but then got sidetracked, sorry.
>>
>> Anyway, the code looks good. I just cleaned it up a bit (white spaces,
>> comments, curly braces) - nothing major. Appended Jonathon's Reviewed-by
>> where it was missing and merged (since Jirka wants to freeze right about
>> now).
>>
>> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
> 
> This seems to have introduced a memory leak. See the failures
> reported by the Ubuntu jobs in
> 
> https://gitlab.com/libvirt/libvirt/-/pipelines/1190698985
> 
> The previous pipeline was successful.
> 

Yep, I've noticed that right after push, but was leaving for lunch.
Anyway, here is the fix:

https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/XQA6K7MGGHWT4PQ23IOX4CVM3O5SRUM6/

Michal
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org