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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list