Re: [PATCH v6 02/12] monitor: Use getter/setter functions for cur_mon

[Markus Armbruster]

Markus Armbruster <armbru@redhat.com> writes:

> Kevin Wolf <kwolf@redhat.com> writes:
>
>> cur_mon really needs to be coroutine-local as soon as we move monitor
>> command handlers to coroutines and let them yield. As a first step, just
>> remove all direct accesses to cur_mon so that we can implement this in
>> the getter function later.
>>
>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
[...]
>> diff --git a/tests/test-util-sockets.c b/tests/test-util-sockets.c
>> index 2ca1e99f17..36fabb5e46 100644
>> --- a/tests/test-util-sockets.c
>> +++ b/tests/test-util-sockets.c
>> @@ -53,27 +53,27 @@ static void test_fd_is_socket_good(void)
>>  static int mon_fd = -1;
>>  static const char *mon_fdname;
>>  
>> -int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp)
>> -{
>> -    g_assert(cur_mon);
>> -    g_assert(mon == cur_mon);
>> -    if (mon_fd == -1 || !g_str_equal(mon_fdname, fdname)) {
>> -        error_setg(errp, "No fd named %s", fdname);
>> -        return -1;
>> -    }
>> -    return dup(mon_fd);
>> -}
>> -
>>  /* Syms in libqemustub.a are discarded at .o file granularity.
>>   * To replace monitor_get_fd() we must ensure everything in
>>   * stubs/monitor.c is defined, to make sure monitor.o is discarded
>>   * otherwise we get duplicate syms at link time.
>>   */
>>  __thread Monitor *cur_mon;
>
> Hmm.  Since monitor.o's @cur_mon now has internal linkage, the comment
> doesn't apply to @cur_mon anymore.  Easy to fix: move the variable
> before the comment.  Bonus: you don't have to move monitor_get_fd()
> then.
>
> Hmm^2, the comment is stale:
>
> * "libqemustub.a"
>
>   Gone since Commit ebedb37c8d "Makefile: Remove libqemustub.a".  Almost
>   three years.  git-grep finds three more occurences, all bogus.

Thomas posted a patch:

    Subject: [PATCH 07/11] Get rid of the libqemustub.a remainders
    Message-Id: <20200804170055.2851-8-thuth@redhat.com>

>
> * "stubs/monitor.c"
>
>   Commit 6ede81d576 "stubs: Update monitor stubs for
>   qemu-storage-daemon" moved stuff from stubs/monitor.c to
>   monitor-core.c.
>
> * "we must ensure everything in stubs/monitor.c is defined"
>
>   We don't.

These two remain.

> Mind to clean that up beforehand?

[...]