qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] 9pfs: Fix some return statements in the synth backend


From: Markus Armbruster
Subject: Re: [PATCH] 9pfs: Fix some return statements in the synth backend
Date: Mon, 28 Nov 2022 11:18:48 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux)

Greg Kurz <groug@kaod.org> writes:

> On Mon, 28 Nov 2022 08:35:22 +0100
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> Greg Kurz <groug@kaod.org> writes:
>> 
>> > The qemu_v9fs_synth_mkdir() and qemu_v9fs_synth_add_file() functions
>> > currently return a positive errno value on failure. This causes
>> > checkpatch.pl to spit several errors like the one below:
>> >
>> > ERROR: return of an errno should typically be -ve (return -EAGAIN)
>> > #79: FILE: hw/9pfs/9p-synth.c:79:
>> > +        return EAGAIN;
>> >
>> > Simply change the sign. This has no consequence since callers
>> > assert() the returned value to be equal to 0.
>> 
>> Out of curiosity: why is assert() appropriate?
>> 
>
> Most of the code base comes from the original synth backend which
> was designed to expose QEMU internals to the guest using 9p. The
> hope of the virtio-9p authors was that each QEMU subsystem would
> create its own tree using these two functions (note that they
> are declared extern). Of course these never happened and the synth
> backend remained nearly dead code for years, until finally it got
> re-used to implement 9p qtest. In this context, failure to create a
> synthetic directory or file means the related test has a bug (e.g.
> messing with the paths used by some other test). This code likely
> needs improvements but we never got to it.

I was about to suggest putting this in a file comment, but then I saw

    /*
     * Not so fast! You might want to read the 9p developer docs first:
     * https://wiki.qemu.org/Documentation/9p
     */

and behind the link, there's a paragraph "3. synth fs driver".

Perhaps a brief note on the use of assert() in synth_init() would still
make sense.  Up to you.

Thanks!

[...]




reply via email to

[Prev in Thread] Current Thread [Next in Thread]