[libvirt] [PATCH] Unify *ReportError logic

[Daniel P. Berrange]

On Thu, Oct 02, 2008 at 03:41:21PM -0400, Cole Robinson wrote:
> Currently, most src/* files have their own ReportError
> function. Some support printf style arguments, others
> only allow reporting a single string message. The code
> for all of them does virtually the same thing, possibly
> passing a different constant off to another function.
> 
> The attached patch adds a function to virterror.c which
> encapsulates the common ReportError logic. I used this
> to replace qemudReportError with a macro, which also 
> allows passing off filename and line number info if
> we wanted to do something with it later.
> 
> I did just the one function conversion to see what
> people think: if I'm missing something, or ideas for
> anything else to add. Seems to work as expected in
> my testing.


Basically a good idea - we've discussed doing exactly this in the past.
You can do further though, and kill off the 'dom' and 'net' parameters
here too. Those are deprecated and should always be left NULL these
days. Likewise passing the filename/linenr is not much use - if a
particular error message wants to include file/line number then they
can be includes as args to the printf fmt string.

> +
> +/**
> + * __virReportErrorHelper
> + *
> + * @conn: the connection to the hypervisor if available
> + * @dom: the domain if available
> + * @net: the network if available
> + * @domcode: the virErrorDomain indicating where it's coming from
> + * @errcode: the virErrorNumber code for the error
> + * @filename: Source file error is dispatched from
> + * @linenr: Line number error is dispatched from
> + * @msg:  the message to display/transmit
> + * @...:  extra parameters for the message display
> + *
> + * Helper function to do most of the grunt work for individual driver ReportError
> + */
> +void __virReportErrorHelper(virConnectPtr conn, virDomainPtr dom, virNetworkPtr net,
> +                            int domcode, int errcode,
> +                            const char *filename ATTRIBUTE_UNUSED,
> +                            long long linenr ATTRIBUTE_UNUSED,
> +                            const char *fmt, ...)

So I think this would be sufficient:

void __virReportErrorHelper(virConnectPtr conn,
                            int domcode,
                            int errcode,
                            const char *fmt, ...)


> +{
> +    va_list args;
> +    char errorMessage[1024];
> +    const char *virerr;
> +
> +    if (fmt) {
> +        va_start(args, fmt);
> +        vsnprintf(errorMessage, sizeof(errorMessage)-1, fmt, args);
> +        va_end(args);
> +    } else {
> +        errorMessage[0] = '\0';
> +    }
> +
> +    virerr = __virErrorMsg(errcode, (errorMessage[0] ? errorMessage : NULL));
> +    __virRaiseError(conn, dom, net, domcode, errcode, VIR_ERR_ERROR,
> +                    virerr, errorMessage, NULL, -1, -1, virerr, errorMessage);
> +
> +}


Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|