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

[Osama Abdelkader <osama.abdelkader@gmail.com>]

On Wed, Nov 12, 2025 at 02:42:58PM +0100, Philippe Mathieu-Daudé wrote:
> 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!

Thanks for your review.