Re: [libvirt] PATCH: Pass 'remote_error' object to RPC handlers

In the libvirtd daemon, remote.c file the current RPC handlers have a return value contract saying 0 - success -1 - failure in libvirt call, no error returned -2 - failure in dispatch process, error already serialized & sent While this isn't a problem for the code as it stands today, for the thread support I want to be able to avoid the dispatch handlers having to touch the 'struct qemud_client' object in normal usage. Allowing the RPC handlers to directly serialize & send the error makes this impossible because it requires access to the client object. So this patch changes the way the RPC handlers deal with errors. The RPC handler API changes from typedef int (*dispatch_fn) (struct qemud_server *server, struct qemud_client *client, dispatch_args *args, dispatch_ret *ret); To typedef int (*dispatch_fn) (struct qemud_server *server, struct qemud_client *client, remote_error *err, dispatch_args *args, dispatch_ret *ret); Note, the addition of a 'remote_error *err' argument. Whenever an error occurs during the dispatch process, the RPC handler must populate this 'remote_error *err' object with the error details. This rule applies whether its a libvirt error, or a dispatch process error, so there is no longer any need to have separate -1 or -2 return values, a simple -1 or 0 return value suffices.

@@ -1381,7 +1380,14 @@ static void qemudDispatchClientRead(stru if (client->bufferOffset < client->bufferLength) return; /* Not read enough */ - remoteDispatchClientRequest (server, client); + if ((len = remoteDispatchClientRequest (server, client)) == 0) + qemudDispatchClientFailure(server, client);

+ + /* Set up the output buffer. */ + client->mode = QEMUD_MODE_TX_PACKET; + client->bufferLength = len; + client->bufferOffset = 0; + if (qemudRegisterClientEvent(server, client, 1) < 0) qemudDispatchClientFailure(server, client);

+static void +remoteDispatchFormatError (remote_error *rerr, + const char *fmt, ...) +{ + va_list args; + char msgbuf[1024]; + char *msg = msgbuf; + + va_start (args, fmt); + vsnprintf (msgbuf, sizeof msgbuf, fmt, args); + va_end (args); + + remoteDispatchStringError (rerr, VIR_ERR_RPC, msg); +}

- remoteDispatchError (client, &req, - _("program mismatch (actual %x, expected %x)"), - req.prog, REMOTE_PROGRAM); - xdr_destroy (&xdr); - return; + remoteDispatchFormatError (&rerr, + _("program mismatch (actual %x, expected %x)"), + req.prog, REMOTE_PROGRAM);

@@ -1395,6 +1368,7 @@ remoteDispatchDomainMigratePrepare (stru args->flags, dname, args->resource); if (r == -1) {

VIR_FREE(uri_out); + remoteDispatchConnError(rerr, client->conn); return -1; } @@ -1439,7 +1413,10 @@ remoteDispatchDomainMigratePerform (stru args->uri, args->flags, dname, args->resource); virDomainFree (dom); - if (r == -1) return -1; + if (r == -1) {

@@ -482,8 +482,12 @@ void virDomainRemoveInactive(virDomainOb memmove(doms->objs + i, doms->objs + i + 1, sizeof(*(doms->objs)) * (doms->count - (i + 1))); - if (VIR_REALLOC_N(doms->objs, doms->count - 1) < 0) { - ; /* Failure to reduce memory allocation isn't fatal */ + if (doms->count > 1) { + if (VIR_REALLOC_N(doms->objs, doms->count - 1) < 0) { + ; /* Failure to reduce memory allocation isn't fatal */ + } + } else { + VIR_FREE(doms->objs); } doms->count--;