qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Claudio Fontana <cfontana@suse.de>
Cc: "Markus Armbruster" <armbru@redhat.com>,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Richard Henderson" <richard.henderson@linaro.org>,
	"Kevin Wolf" <kwolf@redhat.com>,
	qemu-devel@nongnu.org, dinechin@redhat.com,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>
Subject: Re: [PATCH v4 2/3] module: add Error arguments to module_load_one and module_load_qom_one
Date: Thu, 22 Sep 2022 10:31:21 +0100	[thread overview]
Message-ID: <YywraWlVnJoy6ypN@redhat.com> (raw)
In-Reply-To: <5e6d958d-3b69-1472-b1b8-3a63186f0c5b@suse.de>

On Thu, Sep 22, 2022 at 11:20:07AM +0200, Claudio Fontana wrote:
> On 9/22/22 10:28, Daniel P. Berrangé wrote:
> > 
> >> Another interface that does: return -1 for error, 0 for module not found
> >> (no error), and 1 for loaded.
> > 
> > IMHO this pattern is generally easier to understand when looking at
> > the callers, as the fatal error scenario is always clear.
> > 
> > That said I would suggest neither approach as the public facing
> > API. Rather stop trying to overload 3 states onto an error reporting
> > pattern that inherantly wants to be 2 states. Instead just have
> > distinct methods
> > 
> >   bool module_load_one(const char *prefix, const char *name, Error *errp)
> >   bool module_try_load_one(const char *prefix, const char *name, Error *errp)
> 
> 
> Here we are murking again the normal behavior and the error path.
> 
> What is the meaning of try? It's not as though we would error out inside the function module_load_one,
> it's the _caller_ that needs to decide how to treat a return value of found/not found, and the exception (Error).

I suggested "try" as in the  g_malloc vs g_try_malloc API naming pattern,
where the latter ignores the OOM error condition.

So in this case 'try' means try to load the module, but don't fail if
the module is missing on disk.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



  parent reply	other threads:[~2022-09-22 10:30 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-08 18:30 [PATCH v4 0/3] improve error handling for module load Claudio Fontana
2022-09-08 18:30 ` [PATCH v4 1/3] module: removed unused function argument "mayfail" Claudio Fontana
2022-09-08 18:30 ` [PATCH v4 2/3] module: add Error arguments to module_load_one and module_load_qom_one Claudio Fontana
2022-09-15  8:43   ` Claudio Fontana
2022-09-16  8:13   ` Richard Henderson
2022-09-16  8:16     ` Claudio Fontana
2022-09-16  9:27   ` Markus Armbruster
2022-09-16 10:48     ` Claudio Fontana
2022-09-16 14:26       ` Markus Armbruster
2022-09-16 15:06         ` Claudio Fontana
2022-09-19  8:17           ` Markus Armbruster
2022-09-19  8:45             ` Claudio Fontana
2022-09-21 12:47               ` Markus Armbruster
2022-09-19 10:18     ` Philippe Mathieu-Daudé via
2022-09-21 12:16       ` Markus Armbruster
2022-09-21 16:03         ` Claudio Fontana
2022-09-22  6:07           ` Markus Armbruster
2022-09-22  8:28             ` Daniel P. Berrangé
2022-09-22  9:20               ` Claudio Fontana
2022-09-22  9:21                 ` Claudio Fontana
2022-09-22  9:27                   ` Claudio Fontana
2022-09-22  9:31                 ` Daniel P. Berrangé [this message]
2022-09-22  9:34                   ` Claudio Fontana
2022-09-22 10:37                     ` Daniel P. Berrangé
2022-09-22 12:30                       ` Claudio Fontana
2022-09-22 12:33                         ` Daniel P. Berrangé
2022-09-22 12:35                           ` Claudio Fontana
2022-09-22  9:38               ` Markus Armbruster
2022-09-22  9:43                 ` Claudio Fontana
2022-09-22 12:42                   ` Markus Armbruster
2022-09-22 12:45                     ` Claudio Fontana
2022-09-22 13:20                       ` Markus Armbruster
2022-09-22 13:33                         ` Claudio Fontana
2022-09-22 14:36                           ` Markus Armbruster
2022-09-22 15:22                             ` Claudio Fontana
2022-09-23  5:31                               ` Markus Armbruster
2022-09-23  9:40                                 ` Claudio Fontana
2022-09-22 13:34                         ` Philippe Mathieu-Daudé via
2022-09-22 13:42                           ` Claudio Fontana
2022-09-22 13:44                           ` Daniel P. Berrangé
2022-09-22 14:01                             ` Claudio Fontana
2022-09-22 14:54                     ` Kevin Wolf
2022-09-22 15:08                       ` Claudio Fontana
2022-09-22 15:27                       ` Markus Armbruster
2022-09-22 15:51                         ` Claudio Fontana
2022-09-22 17:05                         ` Kevin Wolf
2022-09-23  9:42                           ` Claudio Fontana
2022-09-23  9:44                           ` Claudio Fontana
2022-09-25 10:35           ` Richard Henderson
2022-09-08 18:30 ` [PATCH v4 3/3] accel: abort if we fail to load the accelerator plugin Claudio Fontana

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=YywraWlVnJoy6ypN@redhat.com \
    --to=berrange@redhat.com \
    --cc=armbru@redhat.com \
    --cc=cfontana@suse.de \
    --cc=dinechin@redhat.com \
    --cc=f4bug@amsat.org \
    --cc=kraxel@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).