LXD refuses to start if kernel does not support AIO about raft HOT 7 CLOSED

The AIO requirement is documented in README.md in this repo -- I can add a note in the dqlite, go-dqlite, and lxd repos as well.

The specific error here is a null pointer dereference because something in the call stack of dqlite_node_create doesn't set the errmsg field we try to use a nonexistent struct dqlite_node. I'll fix that, and then the feedback when running without AIO support should be better.

from raft.

Error message should now be something like:

Error: create node: failed to create node: internal dqlite error

Not the greatest, but better than a panic. Maybe it would be a good idea to modify dqlite_node_create to allow returning an error string (using some other channel than the errmsg field of struct dqlite_node)? Then we wouldn't have to interpret the rather non-specific return code in go-dqlite.

from raft.

That seems reasonable to me. I assume the current behavior (clearing the struct dqlite_node) is defense in depth against callers who neglect to check for errors -- maybe we could address that concern in some other way.

I don't think there's a particular reason for the current behavior. As long as we document that dqlite_node_destroy(), must always be called, even in case of errors, we should be good and don't need to do anything special.

The idiomatic way would be something like:

int rc;
dqlite* node;

rc = dqlite_node_create(..., &node);
if (rc != 0) {
    if (rc == DQLITE_NOMEM) {
        printf("out of memory\n");
   } else {
        printf("%s\n", dqlite_node_errmsg(node));
        dqlite_node_destroy(node)
   }
   exit(-1); /* or whatever */
}

from raft.

Now that the AIO requirement is documented everywhere and the go-dqlite error message is fixed (see canonical/go-dqlite#199) I think this can be closed?

from raft.

Error message should now be something like:

Error: create node: failed to create node: internal dqlite error

Not the greatest, but better than a panic. Maybe it would be a good idea to modify dqlite_node_create to allow returning an error string (using some other channel than the errmsg field of struct dqlite_node)? Then we wouldn't have to interpret the rather non-specific return code in go-dqlite.

What do you think of changing the behavior of dqlite_node_create() so that even if it returns an error, it does not free the dqlite_node **n pointer, so call sites can invoke dqlite_node_errmsg() to extract the error message, and then call dqlite_node_destroy() to free the memory.

The only exception to the above behavior would perhaps be if dqlite_node_create fails with DQLITE_NOMEM, but it's a minor detail practically speaking, probably something we don't need to worry too much in real world.

from raft.

That seems reasonable to me. I assume the current behavior (clearing the struct dqlite_node) is defense in depth against callers who neglect to check for errors -- maybe we could address that concern in some other way.

from raft.

from raft.