Re: [PATCH] hw/9pfs: Drop unused print_sg helper

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 12/11/25 13:30, Christian Schoenebeck wrote:
> On Wednesday, November 12, 2025 8:03:28 AM CET Philippe Mathieu-Daudé wrote:
>> Hi Osama,
>>
>> On 10/11/25 16:54, Osama Abdelkader wrote:
>>> Remove the print_sg() debug helper and its always-disabled call sites
>>> in v9fs_read() and v9fs_write(). The function was only reachable via
>>> if (0) blocks, so it has been dead code for a long time.
>>>
>>> Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
>>> ---
> 
> TBH low care level for this on my side. It's just passive debug code and the
> suggested change does not change anything on the resulting binary.
> 
> It's not that this debug code is not used at all, but very seldom. Last time I
> personally used it was like 3 years ago.
> 
>>>
>>>    hw/9pfs/9p.c | 20 --------------------
>>>    1 file changed, 20 deletions(-)
>>>
>>> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
>>> index bc4a016ee3..a8de894f4c 100644
>>> --- a/hw/9pfs/9p.c
>>> +++ b/hw/9pfs/9p.c
>>> @@ -1389,20 +1389,6 @@ static int stat_to_v9stat_dotl(V9fsPDU *pdu, const
>>> struct stat *stbuf,>
>>>        return stat_to_qid(pdu, stbuf, &v9lstat->qid);
>>>    
>>>    }
>>>
>>> -static void print_sg(struct iovec *sg, int cnt)
>>> -{
>>> -    int i;
>>> -
>>> -    printf("sg[%d]: {", cnt);
>>> -    for (i = 0; i < cnt; i++) {
>>> -        if (i) {
>>> -            printf(", ");
>>> -        }
>>> -        printf("(%p, %zd)", sg[i].iov_base, sg[i].iov_len);
>>> -    }
>>> -    printf("}\n");
>>> -}
>>> -
>>>
>>>    /* Will call this only for path name based fid */
>>>    static void v9fs_fix_path(V9fsPath *dst, V9fsPath *src, int len)
>>>    {
>>>
>>> @@ -2468,9 +2454,6 @@ static void coroutine_fn v9fs_read(void *opaque)
>>>
>>>            do {
>>>            
>>>                qemu_iovec_reset(&qiov);
>>>                qemu_iovec_concat(&qiov, &qiov_full, count, qiov_full.size -
>>>                count);
>>>
>>> -            if (0) {
>>> -                print_sg(qiov.iov, qiov.niov);
>>
>> Alternatively, consider converting to trace event so we can keep
>> dumping the entries, but select that at runtime (see for conversion
>> example commit 4847c5701a3 "hw/rtc/mc146818rtc: Convert CMOS_DPRINTF
>> into trace events").
> 
> Probably overkill. We have a bunch of trace events where it makes, especially
> for investigating issues on 9p protocol level. But this debug code is usually
> just enabled if you are working on a virtio transport issue or new virtio
> feature and then you are usually working on this source code already.
> 
> But again: no strong opinion about this overall issue whatsoever.

Fine then!