[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v7 4/8] migration: Create socket-address paramet
From: |
Juan Quintela |
Subject: |
Re: [Qemu-devel] [PATCH v7 4/8] migration: Create socket-address parameter |
Date: |
Tue, 08 May 2018 10:02:32 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux) |
Daniel P. Berrangé <address@hidden> wrote:
> On Wed, Apr 04, 2018 at 01:27:27PM +0200, Juan Quintela wrote:
>> It will be used to store the uri parameters. We want this only for
>> tcp, so we don't set it for other uris. We need it to know what port
>> is migration running.
>>
>> Signed-off-by: Juan Quintela <address@hidden>
>> @@ -277,6 +279,21 @@ int migrate_send_rp_req_pages(MigrationIncomingState
>> *mis, const char *rbname,
>> return migrate_send_rp_message(mis, msg_type, msglen, bufc);
>> }
>>
>> +void migrate_set_address(SocketAddress *address)
>
> s/set/add/ in the method name since you're expecting this to be called
> multiple times, augmenting the value, not replacing it.
Makes sense, done.
>> @@ -564,6 +581,11 @@ MigrationParameters *qmp_query_migrate_parameters(Error
>> **errp)
>> params->x_multifd_page_count = s->parameters.x_multifd_page_count;
>> params->has_xbzrle_cache_size = true;
>> params->xbzrle_cache_size = s->parameters.xbzrle_cache_size;
>> + if (s->parameters.socket_address) {
>
> nitpick, should really check s->parameters.has_socket_address
Changed to state, no has_socket_address on mis-> anymore (if
socket_address != NULL, we know there are some)
>> @@ -152,6 +153,7 @@ static void
>> socket_start_incoming_migration(SocketAddress *saddr,
>> Error **errp)
>> {
>> QIONetListener *listener = qio_net_listener_new();
>> + int i;
>
> Nitpick size_t for array indexes
changed.
>>
>> qio_net_listener_set_name(listener, "migration-socket-listener");
>>
>> @@ -163,6 +165,15 @@ static void
>> socket_start_incoming_migration(SocketAddress *saddr,
>> qio_net_listener_set_client_func(listener,
>> socket_accept_incoming_migration,
>> NULL, NULL);
>> +
>> + for (i = 0; i < listener->nsioc; i++) {
>> + SocketAddress *address =
>> + qio_channel_socket_get_local_address(listener->sioc[i], errp);
>> + if (address < 0) {
>
> 'address' is a pointer, so comparing to ' < 0' is wrong - I'm surprised
> the compiler isn't issuing a warning about this.
Oops, I guess a copy & paste from other error check. No real clue why
compiler didn't detect it either.
>> +
>> +##
>> +# @DummyStruct:
>> +#
>> +# Both block-core and migration needs SocketAddressList
>> +# I am open to comments about how to share it
>
> What's the actual problem you're addressing with this ?
Until now, List of SocketAddress were onl used on block-core. QAPI
generator is able to cope with that. Now, we need to use them on both
migration.json and block-core.json. QAPI generator is smart enough to
detect that SocketAddressList templates have already been generated, but
it is not able to use them on both sides. if I put them in a common
place that are included from both .json, it is just generated correctly,
only once, and it compiles.
I haven't been able to find a way to convince qapi generator that I just
want it to generate the code here, without creating something that uses
it.
I am open to changes.
Thanks, Juan.
>
>> +#
>> +# @dummy-list: A dummy list
>> +#
>> +# Since: 2.13
>> +##
>> +{ 'struct': 'DummyStruct',
>> + 'data': { 'dummy-list': ['SocketAddress'] } }
>> --
>> 2.14.3
>>
>>
>
> Regards,
> Daniel