From: Christian Schoenebeck <qemu_oss@crudebyte.com>
To: Greg Kurz <groug@kaod.org>,
qemu-devel@nongnu.org, Markus Armbruster <armbru@redhat.com>
Subject: Re: [PATCH] 9pfs: Fix some return statements in the synth backend
Date: Mon, 28 Nov 2022 15:21:52 +0100 [thread overview]
Message-ID: <2336646.SJcN5NvsQQ@silver> (raw)
In-Reply-To: <87a64bv0tz.fsf@pond.sub.org>
On Monday, November 28, 2022 11:18:48 AM CET Markus Armbruster wrote:
> 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.
Like what comment would you expect there?
The synth driver is a simplified hack fs driver with hard coded directories &
files, only used for 9p protocol conformance test cases.
Best regards,
Christian Schoenebeck
prev parent reply other threads:[~2022-11-28 14:23 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-24 15:58 [PATCH] 9pfs: Fix some return statements in the synth backend Greg Kurz
2022-11-26 13:47 ` Christian Schoenebeck
2022-11-28 7:35 ` Markus Armbruster
2022-11-28 9:37 ` Greg Kurz
2022-11-28 10:18 ` Markus Armbruster
2022-11-28 14:21 ` Christian Schoenebeck [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2336646.SJcN5NvsQQ@silver \
--to=qemu_oss@crudebyte.com \
--cc=armbru@redhat.com \
--cc=groug@kaod.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.