[libvirt] [PATCH 1/2] conf: Provide error on undefined iothreadsched entry

John Ferlan jferlan at redhat.com
Mon Aug 15 16:03:53 UTC 2016

On 08/15/2016 11:07 AM, Peter Krempa wrote:
> On Mon, Aug 15, 2016 at 10:55:33 -0400, John Ferlan wrote:
>> When commit id '6dfb4507' refactored where the iothreadsched data was
>> stored, the error message for when the virDomainIOThreadIDFind failed
>> to find an iothreadid ("iothreadsched attribute 'iothreads' uses
>> undefined iothread ids") was lost. This led to the possibility that
>> someone would try to use it, but receive the generic message "An error
>> occurred, but the cause is unknown".
>> This patch adds the error message back so that someone will know that
>> they have an invalid configuration.
>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>> ---
>>  src/conf/domain_conf.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
> Same as for 2/2. Code paths that care should report the error in place
> since we would report duplicate errors in cases where it can be ignored
> or is overwritten.

Unlike 2/2 the error here is being checked where appropriate unless you
are advocating a generic error message in virDomainDefParseXML after
calling virDomainIOThreadSchedParse and returning a < 0 value, but no
error message set, then create error message (which doesn't seem right)

    for (i = 0; i < n; i++) {
        if (virDomainIOThreadSchedParse(nodes[i], def) < 0) {
            if (!virGetLastError()) {
                virReportError(..., "some error occurred"...);
            goto error;

For virDomainDefGetIOThreadSched the only callers that care are Parse
and Format. NB: All callers of virDomainIOThreadIDFind care to check if
the ID was valid or not and that's where I believe the check should be.

As for patch 2/2, fair enough I can move the error message to
virDomainDefGetVcpuSched... Of course that if for some unknown reason in
the future virDomainDefGetVcpu adds a new reason for NULL return the
assumption could be lost or the error message overwritten.  Does that
then address your concern?


More information about the libvir-list mailing list