Re: [libvirt] [PATCH] storage: netfs: Handle backend errors
On 04/14/2014 08:13 PM, John Ferlan wrote:
On 04/14/2014 12:47 PM, Ján Tomko wrote:
>
> The whole hunk would IMHO look simpler as:
>
> bool fail = false;
>
> if (FindNFSPoolSources() < 0)
> fail = true;
>
> #ifdef GLUSTER_CLI
> if (FindGlusterPoolSources() < 0)
> fail = true;
> else
> fail = false;
> #endif
>
> if (fail)
> goto cleanup;
ahh, but if NFS succeeds and Gluster exists and fails, then we fail even
though we could return sources from NFS?
Oops, I oversimplified it.
What I meant is: gluster should only touch the 'fail' value if fail is true,
so we might just as well save return values for both calls and
if (nfs < 0 && gluster < 0)
goto cleanup;
(Assuming -1 for gluster if we don't have it)
If NFS fails and Gluster
succeeds we succeed, but how do we let someone know that NFS failed? Or
should we? I would think so. That's what I was attempting to do with all
the message stuff - make sure it's logged somewhere.
You logged the error that was already logged (unless virGetLastError returns
NULL, which is a bug in the function that returned < 0 without setting an
error). Even with the 'Failed to get Gluster/NFS pool sources' prefix, I
don't
think this is much more friendly.
I don't know whether there is hope of some sane error reporting here -
even if we convert both NFSPoolSources and GlusterPoolSources to report errors
if the command failed and they both fail, the NFS error will get overwritten.
On the other hand, an unresolvable hostname should make them both fail.
Jan
Attachments:
- signature.asc (application/pgp-signature — 836 bytes)