On 05/03/2018 03:39 AM, Nikolay Shirokovskiy wrote: > > > On 01.05.2018 01:03, John Ferlan wrote: >> >> >> On 04/18/2018 10:44 AM, Nikolay Shirokovskiy wrote: >>> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@xxxxxxxxxxxxx> >>> --- >>> src/libvirt_private.syms | 1 + >>> src/util/virerror.c | 12 +++++++++--- >>> src/util/virerror.h | 1 + >>> 3 files changed, 11 insertions(+), 3 deletions(-) >>> >> >> NACK, you should be using virErrorCopyNew >> >> John > > I introduced monError in qemuDomainObjPrivatePtr in next patch and it is not a pointer so > I need this function. I did not make monError pointer for the same reason it is not pointer > in monitor object - I use monError both as flag and as error message. > I saw what you did - the fact is virCopyError is coded as private to the module. Just making it global because you have a use for it is I believe incorrect. Ironically in the next patch you have: + virErrorPtr err = qemuMonitorLastError(mon); + + virCopyError(err, &priv->monError); The first call isn't guaranteed to set @err and all you're essentially doing is wanting to copy from mon->lastError to priv->monError or perhaps similar to virCopyLastError. Maybe some new API in virerror.c should handle that. John >> >>> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms >>> index b31f599..6bbbf77 100644 >>> --- a/src/libvirt_private.syms >>> +++ b/src/libvirt_private.syms >>> @@ -1702,6 +1702,7 @@ ebtablesRemoveForwardAllowIn; >>> >>> >>> # util/virerror.h >>> +virCopyError; >>> virDispatchError; >>> virErrorCopyNew; >>> virErrorInitialize; >>> diff --git a/src/util/virerror.c b/src/util/virerror.c >>> index c000b00..2ff8a3e 100644 >>> --- a/src/util/virerror.c >>> +++ b/src/util/virerror.c >>> @@ -188,10 +188,16 @@ virErrorGenericFailure(virErrorPtr err) >>> } >>> >>> >>> -/* >>> - * Internal helper to perform a deep copy of an error >>> +/** >>> + * virCopyError: >>> + * @from: error to copy from >>> + * @to: error to copy to >>> + * >>> + * Copy error fields from @from to @to. >>> + * >>> + * Returns 0 on success, -1 on failure. >>> */ >>> -static int >>> +int >>> virCopyError(virErrorPtr from, >>> virErrorPtr to) >>> { >>> diff --git a/src/util/virerror.h b/src/util/virerror.h >>> index 760bfa4..90ac619 100644 >>> --- a/src/util/virerror.h >>> +++ b/src/util/virerror.h >>> @@ -192,6 +192,7 @@ void virReportOOMErrorFull(int domcode, >>> >>> int virSetError(virErrorPtr newerr); >>> virErrorPtr virErrorCopyNew(virErrorPtr err); >>> +int virCopyError(virErrorPtr from, virErrorPtr to); >>> void virDispatchError(virConnectPtr conn); >>> const char *virStrerror(int theerrno, char *errBuf, size_t errBufLen); >>> >>> -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list