| Message ID | 20240308205951.111747-4-het.gala@nutanix.com |
|---|---|
| State | New |
| Headers | show |
| Series | qtest: migration: Add tests for introducing 'channels' argument in migrate QAPIs | expand |
Het Gala <het.gala@nutanix.com> writes: > migrate_get_socket_address implicitly converts SocketAddress into str. > Move migrate_get_socket_address inside migrate_get_connect_uri which > should return the uri string instead. > > Signed-off-by: Het Gala <het.gala@nutanix.com> > Suggested-by: Fabiano Rosas <farosas@suse.de> > --- > tests/qtest/migration-helpers.c | 26 ++++++++++++++++++-------- > 1 file changed, 18 insertions(+), 8 deletions(-) > > diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c > index 9af3c7d4d5..3c3fe9d8aa 100644 > --- a/tests/qtest/migration-helpers.c > +++ b/tests/qtest/migration-helpers.c > @@ -49,12 +49,12 @@ static char *SocketAddress_to_str(SocketAddress *addr) > } > } > > -static char * > +static SocketAddress * > migrate_get_socket_address(QTestState *who, const char *parameter) > { > QDict *rsp; > - char *result; > SocketAddressList *addrs; > + SocketAddress *addr; > Visitor *iv = NULL; > QObject *object; > > @@ -63,14 +63,24 @@ migrate_get_socket_address(QTestState *who, const char *parameter) > > iv = qobject_input_visitor_new(object); > visit_type_SocketAddressList(iv, NULL, &addrs, &error_abort); > + addr = addrs->value; > visit_free(iv); > > - /* we are only using a single address */ > - result = SocketAddress_to_str(addrs->value); > - > - qapi_free_SocketAddressList(addrs); > qobject_unref(rsp); > - return result; > + return addr; > +} > + > +static char * > +migrate_get_connect_uri(QTestState *who, const char *parameter) > +{ > + SocketAddress *addrs; > + char *connect_uri; > + > + addrs = migrate_get_socket_address(who, parameter); > + connect_uri = SocketAddress_to_str(addrs); > + > + qapi_free_SocketAddress(addrs); > + return connect_uri; > } > > bool migrate_watch_for_events(QTestState *who, const char *name, > @@ -130,7 +140,7 @@ void migrate_qmp(QTestState *who, QTestState *to, const char *uri, > > g_assert(!qdict_haskey(args, "uri")); > if (!uri) { > - connect_uri = migrate_get_socket_address(to, "socket-address"); > + connect_uri = migrate_get_connect_uri(to, "socket-address"); What's the point of the "socket-address" argument here? Seems a bit nonsensical to me to call: migrate_get_socket_address(..., "socket-address"). What about we just suppress this throughout the stack and directly call: object = qdict_get(rsp, "socket-address");
On 11/03/24 11:49 pm, Fabiano Rosas wrote: > Het Gala<het.gala@nutanix.com> writes: > >> >> bool migrate_watch_for_events(QTestState *who, const char *name, >> @@ -130,7 +140,7 @@ void migrate_qmp(QTestState *who, QTestState *to, const char *uri, >> >> g_assert(!qdict_haskey(args, "uri")); >> if (!uri) { >> - connect_uri = migrate_get_socket_address(to, "socket-address"); >> + connect_uri = migrate_get_connect_uri(to, "socket-address"); > What's the point of the "socket-address" argument here? Seems a bit > nonsensical to me to call: migrate_get_socket_address(..., "socket-address"). > > What about we just suppress this throughout the stack and directly call: > > object = qdict_get(rsp, "socket-address"); Fabiano, I didn't get clearly understand your point here. From what I understand, you want to call just 1. migrate_get_connect_uri(to) and migrate_get_connect_qdict(to) 2. delete migrate_get_socket_address(..., "socket-address") altogether 3. Just call qdict_get(rsp, "socket-address") which will return an object 4. Then convert this object into qdict and uri string respectively ? Hmm, If that's the case, converting to qdict shouldn't be a problem. But for uri string is there a simpler method or writing a parsing function would be needed ? Regards, Het Gala
Het Gala <het.gala@nutanix.com> writes: > On 11/03/24 11:49 pm, Fabiano Rosas wrote: >> Het Gala<het.gala@nutanix.com> writes: >> >>> >>> bool migrate_watch_for_events(QTestState *who, const char *name, >>> @@ -130,7 +140,7 @@ void migrate_qmp(QTestState *who, QTestState *to, const char *uri, >>> >>> g_assert(!qdict_haskey(args, "uri")); >>> if (!uri) { >>> - connect_uri = migrate_get_socket_address(to, "socket-address"); >>> + connect_uri = migrate_get_connect_uri(to, "socket-address"); >> What's the point of the "socket-address" argument here? Seems a bit >> nonsensical to me to call: migrate_get_socket_address(..., "socket-address"). >> >> What about we just suppress this throughout the stack and directly call: >> >> object = qdict_get(rsp, "socket-address"); > > Fabiano, I didn't get clearly understand your point here. From what I > understand, > you want to call just > 1. migrate_get_connect_uri(to) and migrate_get_connect_qdict(to) Yes. > 2. delete migrate_get_socket_address(..., "socket-address") altogether No, just the string argument, not the whole function: static char *migrate_get_socket_address(QTestState *who) <---- { QDict *rsp; char *result; SocketAddressList *addrs; Visitor *iv = NULL; QObject *object; rsp = migrate_query(who); object = qdict_get(rsp, "socket-address"); <----- ... } If the thing is called migrate_get_SOCKET_ADDRESS(), it's obvious that the "socket-address" is the parameter we want. We even call SocketAddress_to_str, so there's no point in having that argument there. We will never call the function with something else in 'parameter'.
On 12/03/24 2:21 am, Fabiano Rosas wrote: > Het Gala<het.gala@nutanix.com> writes: > >> On 11/03/24 11:49 pm, Fabiano Rosas wrote: >>> Het Gala<het.gala@nutanix.com> writes: >>> >>>> bool migrate_watch_for_events(QTestState *who, const char *name, >>>> @@ -130,7 +140,7 @@ void migrate_qmp(QTestState *who, QTestState *to, const char *uri, >>>> >>>> g_assert(!qdict_haskey(args, "uri")); >>>> if (!uri) { >>>> - connect_uri = migrate_get_socket_address(to, "socket-address"); >>>> + connect_uri = migrate_get_connect_uri(to, "socket-address"); >>> What's the point of the "socket-address" argument here? Seems a bit >>> nonsensical to me to call: migrate_get_socket_address(..., "socket-address"). >>> >>> What about we just suppress this throughout the stack and directly call: >>> >>> object = qdict_get(rsp, "socket-address"); >> Fabiano, I didn't get clearly understand your point here. From what I >> understand, >> you want to call just >> 1. migrate_get_connect_uri(to) and migrate_get_connect_qdict(to) > Yes. Ack >> 2. delete migrate_get_socket_address(..., "socket-address") altogether > No, just the string argument, not the whole function: > > static char *migrate_get_socket_address(QTestState *who) <---- > { > QDict *rsp; > char *result; > SocketAddressList *addrs; > Visitor *iv = NULL; > QObject *object; > > rsp = migrate_query(who); > object = qdict_get(rsp, "socket-address"); <----- > ... > } > > If the thing is called migrate_get_SOCKET_ADDRESS(), it's obvious that > the "socket-address" is the parameter we want. We even call > SocketAddress_to_str, so there's no point in having that argument > there. We will never call the function with something else in > 'parameter'. Ahh, okay. I got your point, and yes, it makes sense. Will just call migrate_get_socket_address(to) and let the qdict_get() call "socket-address" internally. Regards, Het Gala
diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c index 9af3c7d4d5..3c3fe9d8aa 100644 --- a/tests/qtest/migration-helpers.c +++ b/tests/qtest/migration-helpers.c @@ -49,12 +49,12 @@ static char *SocketAddress_to_str(SocketAddress *addr) } } -static char * +static SocketAddress * migrate_get_socket_address(QTestState *who, const char *parameter) { QDict *rsp; - char *result; SocketAddressList *addrs; + SocketAddress *addr; Visitor *iv = NULL; QObject *object; @@ -63,14 +63,24 @@ migrate_get_socket_address(QTestState *who, const char *parameter) iv = qobject_input_visitor_new(object); visit_type_SocketAddressList(iv, NULL, &addrs, &error_abort); + addr = addrs->value; visit_free(iv); - /* we are only using a single address */ - result = SocketAddress_to_str(addrs->value); - - qapi_free_SocketAddressList(addrs); qobject_unref(rsp); - return result; + return addr; +} + +static char * +migrate_get_connect_uri(QTestState *who, const char *parameter) +{ + SocketAddress *addrs; + char *connect_uri; + + addrs = migrate_get_socket_address(who, parameter); + connect_uri = SocketAddress_to_str(addrs); + + qapi_free_SocketAddress(addrs); + return connect_uri; } bool migrate_watch_for_events(QTestState *who, const char *name, @@ -130,7 +140,7 @@ void migrate_qmp(QTestState *who, QTestState *to, const char *uri, g_assert(!qdict_haskey(args, "uri")); if (!uri) { - connect_uri = migrate_get_socket_address(to, "socket-address"); + connect_uri = migrate_get_connect_uri(to, "socket-address"); } qdict_put_str(args, "uri", uri ? uri : connect_uri);
migrate_get_socket_address implicitly converts SocketAddress into str. Move migrate_get_socket_address inside migrate_get_connect_uri which should return the uri string instead. Signed-off-by: Het Gala <het.gala@nutanix.com> Suggested-by: Fabiano Rosas <farosas@suse.de> --- tests/qtest/migration-helpers.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-)