mbox series

[v11,00/10] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration

Message ID 20231004075851.219173-1-het.gala@nutanix.com (mailing list archive)
Headers show
Series migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration | expand

Message

Het Gala Oct. 4, 2023, 7:58 a.m. UTC
This is v11 patchset of modified 'migrate' and 'migrate-incoming' QAPI design
for upstream review.

Update: Daniel has reviewed all patches and is okay with them. Markus has also
        given Acked-by tag for patches related to QAPI syntax change.
Fabiano, Juan and other migration maintainers, let me know if there are still
improvements to be made in this patchset series.

Link to previous upstream community patchset links:
v1: https://lists.gnu.org/archive/html/qemu-devel/2022-12/msg04339.html
v2: https://lists.gnu.org/archive/html/qemu-devel/2023-02/msg02106.html
v3: https://lists.gnu.org/archive/html/qemu-devel/2023-02/msg02473.html
v4: https://lists.gnu.org/archive/html/qemu-devel/2023-05/msg03064.html
v5: https://lists.gnu.org/archive/html/qemu-devel/2023-05/msg04845.html
v6: https://lists.gnu.org/archive/html/qemu-devel/2023-06/msg01251.html
v7: https://lists.gnu.org/archive/html/qemu-devel/2023-07/msg02027.html
v8: https://lists.gnu.org/archive/html/qemu-devel/2023-07/msg02770.html
v9: https://lists.gnu.org/archive/html/qemu-devel/2023-07/msg04216.html
v10: https://lists.gnu.org/archive/html/qemu-devel/2023-07/msg05022.html

v10 -> v11 changelog:
-------------------
- Resolved make check errors as its been almost two months since v10
  version of this patchset series went out. Till date migration workflow
  might have changed which caused make check errors.

Abstract:
---------

Current QAPI 'migrate' command design (for initiating a migration
stream) contains information regarding different migrate transport mechanism
(tcp / unix / exec), dest-host IP address, and binding port number in form of
a string. Thus the design does seem to have some design issues. Some of the
issues, stated below are:

1. Use of string URIs is a data encoding scheme within a data encoding scheme.
   QEMU code should directly be able to work with the results from QAPI,
   without resorting to do a second level of parsing (eg. socket_parse()).
2. For features / parameters related to migration, the migration tunables needs
   to be defined and updated upfront. For example, 'migrate-set-capability'
   and 'migrate-set-parameter' is required to enable multifd capability and
   multifd-number of channels respectively. Instead, 'Multifd-channels' can
   directly be represented as a single additional parameter to 'migrate'
   QAPI. 'migrate-set-capability' and 'migrate-set-parameter' commands could
   be used for runtime tunables that need setting after migration has already
   started.

The current patchset focuses on solving the first problem of multi-level
encoding of URIs. The patch defines 'migrate' command as a QAPI discriminated
union for the various transport backends (like socket, exec and rdma), and on
basis of transport backends, different migration parameters are defined.

(uri) string -->  (channel) Channel-type
                            Transport-type
                            Migration parameters based on transport type
------------------------------------------------------------------------------

Het Gala (10):
  migration: New QAPI type 'MigrateAddress'
  migration: convert migration 'uri' into 'MigrateAddress'
  migration: convert socket backend to accept MigrateAddress
  migration: convert rdma backend to accept MigrateAddress
  migration: convert exec backend to accept MigrateAddress.
  migration: New migrate and migrate-incoming argument 'channels'
  migration: modify migration_channels_and_uri_compatible() for new QAPI
    syntax
  migration: Implement MigrateChannelList to qmp migration flow.
  migration: Implement MigrateChannelList to hmp migration flow.
  migration: modify test_multifd_tcp_none() to use new QAPI syntax.

 migration/exec.c               |  72 +++++++++----
 migration/exec.h               |   8 +-
 migration/migration-hmp-cmds.c |  17 ++-
 migration/migration.c          | 190 ++++++++++++++++++++++++++-------
 migration/migration.h          |   3 +-
 migration/rdma.c               |  34 +++---
 migration/rdma.h               |   6 +-
 migration/socket.c             |  39 ++-----
 migration/socket.h             |   7 +-
 qapi/migration.json            | 150 +++++++++++++++++++++++++-
 softmmu/vl.c                   |   2 +-
 tests/qtest/migration-test.c   |   7 +-
 12 files changed, 409 insertions(+), 126 deletions(-)

Comments

Fabiano Rosas Oct. 4, 2023, 1:33 p.m. UTC | #1
Het Gala <het.gala@nutanix.com> writes:

> This is v11 patchset of modified 'migrate' and 'migrate-incoming' QAPI design
> for upstream review.
>
> Update: Daniel has reviewed all patches and is okay with them. Markus has also
>         given Acked-by tag for patches related to QAPI syntax change.
> Fabiano, Juan and other migration maintainers, let me know if there are still
> improvements to be made in this patchset series.
>
> Link to previous upstream community patchset links:
> v1: https://lists.gnu.org/archive/html/qemu-devel/2022-12/msg04339.html
> v2: https://lists.gnu.org/archive/html/qemu-devel/2023-02/msg02106.html
> v3: https://lists.gnu.org/archive/html/qemu-devel/2023-02/msg02473.html
> v4: https://lists.gnu.org/archive/html/qemu-devel/2023-05/msg03064.html
> v5: https://lists.gnu.org/archive/html/qemu-devel/2023-05/msg04845.html
> v6: https://lists.gnu.org/archive/html/qemu-devel/2023-06/msg01251.html
> v7: https://lists.gnu.org/archive/html/qemu-devel/2023-07/msg02027.html
> v8: https://lists.gnu.org/archive/html/qemu-devel/2023-07/msg02770.html
> v9: https://lists.gnu.org/archive/html/qemu-devel/2023-07/msg04216.html
> v10: https://lists.gnu.org/archive/html/qemu-devel/2023-07/msg05022.html
>
> v10 -> v11 changelog:
> -------------------
> - Resolved make check errors as its been almost two months since v10
>   version of this patchset series went out. Till date migration workflow
>   might have changed which caused make check errors.

Sorry, there must be a misunderstanding here. This series still has
problems. Just look at patch 6 that adds the "channel-type" parameter and
patch 10 that uses "channeltype" in the test (without hyphen). This
cannot work.

There's also several instances of g_autoptr being used incorrectly. I
could comment on every patch individually, but this series cannot have
passed make check.

Please resend this with the issues fixed and drop the Reviewed-bys from
the affected patches.

Summary of Failures:

  1/418 qemu:qtest+qtest-x86_64 / qtest-x86_64/migration-test         ERROR           0.44s   killed by signal 6 SIGABRT
  7/418 qemu:qtest+qtest-i386 / qtest-i386/migration-test             ERROR           0.47s   killed by signal 6 SIGABRT
121/418 qemu:qtest+qtest-x86_64 / qtest-x86_64/tpm-crb-swtpm-test     ERROR           0.55s   killed by signal 6 SIGABRT
128/418 qemu:qtest+qtest-x86_64 / qtest-x86_64/tpm-tis-swtpm-test     ERROR           0.72s   killed by signal 6 SIGABRT
131/418 qemu:qtest+qtest-i386 / qtest-i386/ahci-test                  ERROR          12.53s   killed by signal 6 SIGABRT
134/418 qemu:qtest+qtest-x86_64 / qtest-x86_64/ahci-test              ERROR          13.04s   killed by signal 6 SIGABRT
143/418 qemu:qtest+qtest-x86_64 / qtest-x86_64/virtio-net-failover    ERROR           2.95s   killed by signal 6 SIGABRT
147/418 qemu:qtest+qtest-i386 / qtest-i386/qos-test                   ERROR          16.12s   killed by signal 6 SIGABRT
148/418 qemu:qtest+qtest-x86_64 / qtest-x86_64/qos-test               ERROR          16.15s   killed by signal 6 SIGABRT
177/418 qemu:qtest+qtest-i386 / qtest-i386/tpm-crb-swtpm-test         ERROR           0.55s   killed by signal 6 SIGABRT
180/418 qemu:qtest+qtest-i386 / qtest-i386/tpm-tis-swtpm-test         ERROR           0.59s   killed by signal 6 SIGABRT
197/418 qemu:qtest+qtest-i386 / qtest-i386/virtio-net-failover        ERROR           2.38s   killed by signal 6 SIGABRT
305/418 qemu:block / io-qcow2-181                                     ERROR           0.52s   exit status 1
312/418 qemu:block / io-qcow2-060                                     ERROR           9.89s   exit status 1
316/418 qemu:block / io-qcow2-203                                     ERROR           0.84s   exit status 1
Het Gala Oct. 4, 2023, 1:45 p.m. UTC | #2
On 04/10/23 7:03 pm, Fabiano Rosas wrote:
> Het Gala <het.gala@nutanix.com> writes:
>
>> This is v11 patchset of modified 'migrate' and 'migrate-incoming' QAPI design
>> for upstream review.
>>
>> Update: Daniel has reviewed all patches and is okay with them. Markus has also
>>          given Acked-by tag for patches related to QAPI syntax change.
>> Fabiano, Juan and other migration maintainers, let me know if there are still
>> improvements to be made in this patchset series.
>>
>> Link to previous upstream community patchset links:
>> v1: https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2022-2D12_msg04339.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=jsRvKRy1JOiy05KX1CtLqWN1su5XNmKPKuJTSx5sZpU&e=
>> v2: https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D02_msg02106.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=mzt3n5PD1QclHfpZEh-VMoLkkwT8xqjPYN-1r7MOly0&e=
>> v3: https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D02_msg02473.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=fa9W71JU6-3xZrjLH7AmElgqwJGUkPeQv3P7n6EXxOM&e=
>> v4: https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D05_msg03064.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=Xr1y3EvBzEtWT9O1fVNapCb3WnD-aWR8UeXv6J6gZQM&e=
>> v5: https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D05_msg04845.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=OtK10W2Z0DobrktRfTCMYPxbcMaaZ6f6qoA65D4RG_A&e=
>> v6: https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D06_msg01251.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=XH-4qFQgdkAKmRsa9DuqaZgJMvGUi1p4-s05AsAEYRo&e=
>> v7: https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg02027.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=RwvfliI4wLm7S0TKl5RMku-gSSE-5fZPYH0MkzJdoPw&e=
>> v8: https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg02770.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=BZsKBJGVPDWXwGgb2-fAnS9pWzTYuLzI92TmuWBcB3k&e=
>> v9: https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg04216.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=YcWFU9I2u-R6QbVjweZ3lFvJlllm-i9o5_jtLBxC_oc&e=
>> v10: https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg05022.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=JQt63Ikbz21vmsLmSensQu8zknGuS9bls-IFpndor78&e=
>>
>> v10 -> v11 changelog:
>> -------------------
>> - Resolved make check errors as its been almost two months since v10
>>    version of this patchset series went out. Till date migration workflow
>>    might have changed which caused make check errors.
> Sorry, there must be a misunderstanding here. This series still has
> problems. Just look at patch 6 that adds the "channel-type" parameter and
> patch 10 that uses "channeltype" in the test (without hyphen). This
> cannot work.
Ack. I will change that.
> There's also several instances of g_autoptr being used incorrectly. I
> could comment on every patch individually, but this series cannot have
> passed make check.
Are we allowed to run the make checks ? I am not aware from where these 
failures are arising. It would be helpful if you could point out to me 
where g_autoptr is incorrectly used ?
> Please resend this with the issues fixed and drop the Reviewed-bys from
> the affected patches.
How to verify which are the affected patches here ?
> Summary of Failures:
>
>    1/418 qemu:qtest+qtest-x86_64 / qtest-x86_64/migration-test         ERROR           0.44s   killed by signal 6 SIGABRT
>    7/418 qemu:qtest+qtest-i386 / qtest-i386/migration-test             ERROR           0.47s   killed by signal 6 SIGABRT
> 121/418 qemu:qtest+qtest-x86_64 / qtest-x86_64/tpm-crb-swtpm-test     ERROR           0.55s   killed by signal 6 SIGABRT
> 128/418 qemu:qtest+qtest-x86_64 / qtest-x86_64/tpm-tis-swtpm-test     ERROR           0.72s   killed by signal 6 SIGABRT
> 131/418 qemu:qtest+qtest-i386 / qtest-i386/ahci-test                  ERROR          12.53s   killed by signal 6 SIGABRT
> 134/418 qemu:qtest+qtest-x86_64 / qtest-x86_64/ahci-test              ERROR          13.04s   killed by signal 6 SIGABRT
> 143/418 qemu:qtest+qtest-x86_64 / qtest-x86_64/virtio-net-failover    ERROR           2.95s   killed by signal 6 SIGABRT
> 147/418 qemu:qtest+qtest-i386 / qtest-i386/qos-test                   ERROR          16.12s   killed by signal 6 SIGABRT
> 148/418 qemu:qtest+qtest-x86_64 / qtest-x86_64/qos-test               ERROR          16.15s   killed by signal 6 SIGABRT
> 177/418 qemu:qtest+qtest-i386 / qtest-i386/tpm-crb-swtpm-test         ERROR           0.55s   killed by signal 6 SIGABRT
> 180/418 qemu:qtest+qtest-i386 / qtest-i386/tpm-tis-swtpm-test         ERROR           0.59s   killed by signal 6 SIGABRT
> 197/418 qemu:qtest+qtest-i386 / qtest-i386/virtio-net-failover        ERROR           2.38s   killed by signal 6 SIGABRT
> 305/418 qemu:block / io-qcow2-181                                     ERROR           0.52s   exit status 1
> 312/418 qemu:block / io-qcow2-060                                     ERROR           9.89s   exit status 1
> 316/418 qemu:block / io-qcow2-203                                     ERROR           0.84s   exit status 1
>
Regards,
Het Gala
Fabiano Rosas Oct. 4, 2023, 2:03 p.m. UTC | #3
Het Gala <het.gala@nutanix.com> writes:

> On 04/10/23 7:03 pm, Fabiano Rosas wrote:
>> Het Gala <het.gala@nutanix.com> writes:
>>
>>> This is v11 patchset of modified 'migrate' and 'migrate-incoming' QAPI design
>>> for upstream review.
>>>
>>> Update: Daniel has reviewed all patches and is okay with them. Markus has also
>>>          given Acked-by tag for patches related to QAPI syntax change.
>>> Fabiano, Juan and other migration maintainers, let me know if there are still
>>> improvements to be made in this patchset series.
>>>
>>> Link to previous upstream community patchset links:
>>> v1: https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2022-2D12_msg04339.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=jsRvKRy1JOiy05KX1CtLqWN1su5XNmKPKuJTSx5sZpU&e=
>>> v2: https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D02_msg02106.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=mzt3n5PD1QclHfpZEh-VMoLkkwT8xqjPYN-1r7MOly0&e=
>>> v3: https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D02_msg02473.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=fa9W71JU6-3xZrjLH7AmElgqwJGUkPeQv3P7n6EXxOM&e=
>>> v4: https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D05_msg03064.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=Xr1y3EvBzEtWT9O1fVNapCb3WnD-aWR8UeXv6J6gZQM&e=
>>> v5: https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D05_msg04845.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=OtK10W2Z0DobrktRfTCMYPxbcMaaZ6f6qoA65D4RG_A&e=
>>> v6: https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D06_msg01251.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=XH-4qFQgdkAKmRsa9DuqaZgJMvGUi1p4-s05AsAEYRo&e=
>>> v7: https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg02027.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=RwvfliI4wLm7S0TKl5RMku-gSSE-5fZPYH0MkzJdoPw&e=
>>> v8: https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg02770.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=BZsKBJGVPDWXwGgb2-fAnS9pWzTYuLzI92TmuWBcB3k&e=
>>> v9: https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg04216.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=YcWFU9I2u-R6QbVjweZ3lFvJlllm-i9o5_jtLBxC_oc&e=
>>> v10: https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg05022.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=JQt63Ikbz21vmsLmSensQu8zknGuS9bls-IFpndor78&e=
>>>
>>> v10 -> v11 changelog:
>>> -------------------
>>> - Resolved make check errors as its been almost two months since v10
>>>    version of this patchset series went out. Till date migration workflow
>>>    might have changed which caused make check errors.
>> Sorry, there must be a misunderstanding here. This series still has
>> problems. Just look at patch 6 that adds the "channel-type" parameter and
>> patch 10 that uses "channeltype" in the test (without hyphen). This
>> cannot work.
> Ack. I will change that.
>> There's also several instances of g_autoptr being used incorrectly. I
>> could comment on every patch individually, but this series cannot have
>> passed make check.
> Are we allowed to run the make checks ? I am not aware from where these 
> failures are arising. It would be helpful if you could point out to me 
> where g_autoptr is incorrectly used ?

I mean just the project's make check command:

cd build/
../configure
make -j$(nproc)
make -j$(nproc) check

>> Please resend this with the issues fixed and drop the Reviewed-bys from
>> the affected patches.
> How to verify which are the affected patches here ?

I'll comment in each patch individually.

We'll also have to add compatibility with the new file: URI that's
included in the latest migration pull request. I'll add comments on
where I think we'll need to add code to support that feature.
Fabiano Rosas Oct. 4, 2023, 3:32 p.m. UTC | #4
Fabiano Rosas <farosas@suse.de> writes:

> Het Gala <het.gala@nutanix.com> writes:
>
>> On 04/10/23 7:03 pm, Fabiano Rosas wrote:
>>> Het Gala <het.gala@nutanix.com> writes:
>>>
>>>> This is v11 patchset of modified 'migrate' and 'migrate-incoming' QAPI design
>>>> for upstream review.
>>>>
>>>> Update: Daniel has reviewed all patches and is okay with them. Markus has also
>>>>          given Acked-by tag for patches related to QAPI syntax change.
>>>> Fabiano, Juan and other migration maintainers, let me know if there are still
>>>> improvements to be made in this patchset series.
>>>>
>>>> Link to previous upstream community patchset links:
>>>> v1: https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2022-2D12_msg04339.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=jsRvKRy1JOiy05KX1CtLqWN1su5XNmKPKuJTSx5sZpU&e=
>>>> v2: https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D02_msg02106.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=mzt3n5PD1QclHfpZEh-VMoLkkwT8xqjPYN-1r7MOly0&e=
>>>> v3: https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D02_msg02473.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=fa9W71JU6-3xZrjLH7AmElgqwJGUkPeQv3P7n6EXxOM&e=
>>>> v4: https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D05_msg03064.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=Xr1y3EvBzEtWT9O1fVNapCb3WnD-aWR8UeXv6J6gZQM&e=
>>>> v5: https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D05_msg04845.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=OtK10W2Z0DobrktRfTCMYPxbcMaaZ6f6qoA65D4RG_A&e=
>>>> v6: https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D06_msg01251.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=XH-4qFQgdkAKmRsa9DuqaZgJMvGUi1p4-s05AsAEYRo&e=
>>>> v7: https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg02027.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=RwvfliI4wLm7S0TKl5RMku-gSSE-5fZPYH0MkzJdoPw&e=
>>>> v8: https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg02770.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=BZsKBJGVPDWXwGgb2-fAnS9pWzTYuLzI92TmuWBcB3k&e=
>>>> v9: https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg04216.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=YcWFU9I2u-R6QbVjweZ3lFvJlllm-i9o5_jtLBxC_oc&e=
>>>> v10: https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg05022.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=JQt63Ikbz21vmsLmSensQu8zknGuS9bls-IFpndor78&e=
>>>>
>>>> v10 -> v11 changelog:
>>>> -------------------
>>>> - Resolved make check errors as its been almost two months since v10
>>>>    version of this patchset series went out. Till date migration workflow
>>>>    might have changed which caused make check errors.
>>> Sorry, there must be a misunderstanding here. This series still has
>>> problems. Just look at patch 6 that adds the "channel-type" parameter and
>>> patch 10 that uses "channeltype" in the test (without hyphen). This
>>> cannot work.
>> Ack. I will change that.
>>> There's also several instances of g_autoptr being used incorrectly. I
>>> could comment on every patch individually, but this series cannot have
>>> passed make check.
>> Are we allowed to run the make checks ? I am not aware from where these 
>> failures are arising. It would be helpful if you could point out to me 
>> where g_autoptr is incorrectly used ?
>
> I mean just the project's make check command:
>
> cd build/
> ../configure
> make -j$(nproc)
> make -j$(nproc) check
>
>>> Please resend this with the issues fixed and drop the Reviewed-bys from
>>> the affected patches.
>> How to verify which are the affected patches here ?
>
> I'll comment in each patch individually.

Done.

We had some double-frees when using g_autoptr in structures that are
nested into another. The qapi code already descends and frees the
children.

There were also issues with allocating memory and later overwriting the
pointers.

This might still not put us in the most correct situation regarding
memory, but I think it will at least get make check passing. Feel free
to investigate the errors with make check and propose alternative
solutions. It has been a while since I looked at this series, I might
have missed something further.

> We'll also have to add compatibility with the new file: URI that's
> included in the latest migration pull request. I'll add comments on
> where I think we'll need to add code to support that feature.

I'll actually defer here until you post your series with the
fixes. It'll probably be easier if I just send individual additions to
your patches.
Het Gala Oct. 6, 2023, 4:17 p.m. UTC | #5
On 10/4/2023 7:33 PM, Fabiano Rosas wrote:
> Het Gala<het.gala@nutanix.com>  writes:
>
>> On 04/10/23 7:03 pm, Fabiano Rosas wrote:
>>> Het Gala<het.gala@nutanix.com>  writes:
>>>
>>>> This is v11 patchset of modified 'migrate' and 'migrate-incoming' QAPI design
>>>> for upstream review.
>>>>
>>>> Update: Daniel has reviewed all patches and is okay with them. Markus has also
>>>>           given Acked-by tag for patches related to QAPI syntax change.
>>>> Fabiano, Juan and other migration maintainers, let me know if there are still
>>>> improvements to be made in this patchset series.
>>>>
>>>> Link to previous upstream community patchset links:
>>>> v1:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2022-2D12_msg04339.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=jsRvKRy1JOiy05KX1CtLqWN1su5XNmKPKuJTSx5sZpU&e=
>>>> v2:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D02_msg02106.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=mzt3n5PD1QclHfpZEh-VMoLkkwT8xqjPYN-1r7MOly0&e=
>>>> v3:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D02_msg02473.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=fa9W71JU6-3xZrjLH7AmElgqwJGUkPeQv3P7n6EXxOM&e=
>>>> v4:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D05_msg03064.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=Xr1y3EvBzEtWT9O1fVNapCb3WnD-aWR8UeXv6J6gZQM&e=
>>>> v5:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D05_msg04845.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=OtK10W2Z0DobrktRfTCMYPxbcMaaZ6f6qoA65D4RG_A&e=
>>>> v6:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D06_msg01251.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=XH-4qFQgdkAKmRsa9DuqaZgJMvGUi1p4-s05AsAEYRo&e=
>>>> v7:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg02027.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=RwvfliI4wLm7S0TKl5RMku-gSSE-5fZPYH0MkzJdoPw&e=
>>>> v8:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg02770.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=BZsKBJGVPDWXwGgb2-fAnS9pWzTYuLzI92TmuWBcB3k&e=
>>>> v9:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg04216.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=YcWFU9I2u-R6QbVjweZ3lFvJlllm-i9o5_jtLBxC_oc&e=
>>>> v10:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg05022.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=JQt63Ikbz21vmsLmSensQu8zknGuS9bls-IFpndor78&e=
>>>>
>>>> v10 -> v11 changelog:
>>>> -------------------
>>>> - Resolved make check errors as its been almost two months since v10
>>>>     version of this patchset series went out. Till date migration workflow
>>>>     might have changed which caused make check errors.
>>> Sorry, there must be a misunderstanding here. This series still has
>>> problems. Just look at patch 6 that adds the "channel-type" parameter and
>>> patch 10 that uses "channeltype" in the test (without hyphen). This
>>> cannot work.
>> Ack. I will change that.
>>> There's also several instances of g_autoptr being used incorrectly. I
>>> could comment on every patch individually, but this series cannot have
>>> passed make check.
>> Are we allowed to run the make checks ? I am not aware from where these
>> failures are arising. It would be helpful if you could point out to me
>> where g_autoptr is incorrectly used ?
> I mean just the project's make check command:
>
> cd build/
> ../configure
> make -j$(nproc)
> make -j$(nproc) check
Okay, now I got it. I was not aware of make -j check to pass the qemu 
tests. Will make sure that it passes the make check.
>>> Please resend this with the issues fixed and drop the Reviewed-bys from
>>> the affected patches.
>> How to verify which are the affected patches here ?
> I'll comment in each patch individually.
Thankyou for commenting on the patches. Will try to make all the changes 
in the coming version.
> We'll also have to add compatibility with the new file: URI that's
> included in the latest migration pull request. I'll add comments on
> where I think we'll need to add code to support that feature.

Ack.

Regards,
Het Gala.
Het Gala Oct. 9, 2023, 1:25 p.m. UTC | #6
On 10/4/2023 9:02 PM, Fabiano Rosas wrote:
> Fabiano Rosas<farosas@suse.de>  writes:
>
>> Het Gala<het.gala@nutanix.com>  writes:
>>
>>> On 04/10/23 7:03 pm, Fabiano Rosas wrote:
>>>> Het Gala<het.gala@nutanix.com>  writes:
>>>>
>>>>> This is v11 patchset of modified 'migrate' and 'migrate-incoming' QAPI design
>>>>> for upstream review.
>>>>>
>>>>> Update: Daniel has reviewed all patches and is okay with them. Markus has also
>>>>>           given Acked-by tag for patches related to QAPI syntax change.
>>>>> Fabiano, Juan and other migration maintainers, let me know if there are still
>>>>> improvements to be made in this patchset series.
>>>>>
>>>>> Link to previous upstream community patchset links:
>>>>> v1:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2022-2D12_msg04339.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=jsRvKRy1JOiy05KX1CtLqWN1su5XNmKPKuJTSx5sZpU&e=
>>>>> v2:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D02_msg02106.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=mzt3n5PD1QclHfpZEh-VMoLkkwT8xqjPYN-1r7MOly0&e=
>>>>> v3:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D02_msg02473.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=fa9W71JU6-3xZrjLH7AmElgqwJGUkPeQv3P7n6EXxOM&e=
>>>>> v4:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D05_msg03064.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=Xr1y3EvBzEtWT9O1fVNapCb3WnD-aWR8UeXv6J6gZQM&e=
>>>>> v5:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D05_msg04845.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=OtK10W2Z0DobrktRfTCMYPxbcMaaZ6f6qoA65D4RG_A&e=
>>>>> v6:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D06_msg01251.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=XH-4qFQgdkAKmRsa9DuqaZgJMvGUi1p4-s05AsAEYRo&e=
>>>>> v7:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg02027.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=RwvfliI4wLm7S0TKl5RMku-gSSE-5fZPYH0MkzJdoPw&e=
>>>>> v8:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg02770.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=BZsKBJGVPDWXwGgb2-fAnS9pWzTYuLzI92TmuWBcB3k&e=
>>>>> v9:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg04216.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=YcWFU9I2u-R6QbVjweZ3lFvJlllm-i9o5_jtLBxC_oc&e=
>>>>> v10:https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2023-2D07_msg05022.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=xuVA--dLVo9lijpitqSt7EOEzBGpEvigXGCb9p_MIk0xmhQZ8bPasLgZ2aOlEBcz&s=JQt63Ikbz21vmsLmSensQu8zknGuS9bls-IFpndor78&e=
>>>>>
>>>>> v10 -> v11 changelog:
>>>>> -------------------
>>>>> - Resolved make check errors as its been almost two months since v10
>>>>>     version of this patchset series went out. Till date migration workflow
>>>>>     might have changed which caused make check errors.
>>>> Sorry, there must be a misunderstanding here. This series still has
>>>> problems. Just look at patch 6 that adds the "channel-type" parameter and
>>>> patch 10 that uses "channeltype" in the test (without hyphen). This
>>>> cannot work.
>>> Ack. I will change that.
>>>> There's also several instances of g_autoptr being used incorrectly. I
>>>> could comment on every patch individually, but this series cannot have
>>>> passed make check.
>>> Are we allowed to run the make checks ? I am not aware from where these
>>> failures are arising. It would be helpful if you could point out to me
>>> where g_autoptr is incorrectly used ?
>> I mean just the project's make check command:
>>
>> cd build/
>> ../configure
>> make -j$(nproc)
>> make -j$(nproc) check
Yes, I got it now. Thanks
>>>> Please resend this with the issues fixed and drop the Reviewed-bys from
>>>> the affected patches.
>>> How to verify which are the affected patches here ?
>> I'll comment in each patch individually.
> Done.
>
> We had some double-frees when using g_autoptr in structures that are
> nested into another. The qapi code already descends and frees the
> children.
>
> There were also issues with allocating memory and later overwriting the
> pointers.
>
> This might still not put us in the most correct situation regarding
> memory, but I think it will at least get make check passing. Feel free
> to investigate the errors with make check and propose alternative
> solutions. It has been a while since I looked at this series, I might
> have missed something further.
Yes, for now I have tried to address all the comments made in the 
individual patches and tried to fix issue of pointer overwriting 
wherever I could spot, also double frees in the hmp code workflow. By 
doing this, I have passed all the make checks, but if there are places 
which needs to be re-looked for the above issues you mentioned, please 
let me know.
>> We'll also have to add compatibility with the new file: URI that's
>> included in the latest migration pull request. I'll add comments on
>> where I think we'll need to add code to support that feature.
> I'll actually defer here until you post your series with the
> fixes. It'll probably be easier if I just send individual additions to
> your patches.

Ack, thanks for reviewing patches and giving valuable feedback. Sending 
new patchset series within sometime.

Regards,
Het Gala