Re: [libvirt] [PATCH 4/4] util: print errno in virObjectLockableNew

2014-07-16 Thread Jincheng Miao
- Original Message -
> - Original Message -
> > Oh, sorry; /me facepalms...
> > 
> > You're right then, but if you want to change it everywhere in the code
> > (which I don't require), it would be better to create a
> > virMutexInitInternal that takes 2 parameters; the second being a bool
> > that controls error reporting and virMutexInit and virMutexInitQuiet
> > would be two macros.  Similarly to virVasprintf for example.
> 
> Good idea :)

I will remove this patch from this patchset on V2, and send a separate one
for virMutexInit* implement.

> 
> > 
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 4/4] util: print errno in virObjectLockableNew

2014-07-15 Thread Jincheng Miao
- Original Message -
> Oh, sorry; /me facepalms...
> 
> You're right then, but if you want to change it everywhere in the code
> (which I don't require), it would be better to create a
> virMutexInitInternal that takes 2 parameters; the second being a bool
> that controls error reporting and virMutexInit and virMutexInitQuiet
> would be two macros.  Similarly to virVasprintf for example.

Good idea :)

> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 4/4] util: print errno in virObjectLockableNew

2014-07-15 Thread Martin Kletzander

On Tue, Jul 15, 2014 at 10:47:04PM -0400, Jincheng Miao wrote:

- Original Message -

I'm not sure errno is set when using our virMutexInit().  Most of the
code uses virReportError instead, I suggest using that.  This should


Actually, the implement of virMutexInit() already set errno when failure:

int virMutexInit(virMutexPtr m)
{
   int ret;
   pthread_mutexattr_t attr;
   pthread_mutexattr_init(&attr);
   pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_NORMAL);
   ret = pthread_mutex_init(&m->lock, &attr);
   pthread_mutexattr_destroy(&attr);
   if (ret != 0) {
   errno = ret;
   return -1;
   }
   return 0;
}



Oh, sorry; /me facepalms...

You're right then, but if you want to change it everywhere in the code
(which I don't require), it would be better to create a
virMutexInitInternal that takes 2 parameters; the second being a bool
that controls error reporting and virMutexInit and virMutexInitQuiet
would be two macros.  Similarly to virVasprintf for example.




be changed everywhere in the code.  Rough idea of the places could be


I think it worthy of adding after all virMutexInit, I will include it in
my V2 patchset.


gotten by the following command:

git grep -nA5 virMutexInit | grep SystemError

but as I said, only rough idea :)

Martin



signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 4/4] util: print errno in virObjectLockableNew

2014-07-15 Thread Jincheng Miao
- Original Message -
> I'm not sure errno is set when using our virMutexInit().  Most of the
> code uses virReportError instead, I suggest using that.  This should

Actually, the implement of virMutexInit() already set errno when failure:

int virMutexInit(virMutexPtr m)
{
int ret;
pthread_mutexattr_t attr;
pthread_mutexattr_init(&attr);
pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_NORMAL);
ret = pthread_mutex_init(&m->lock, &attr);
pthread_mutexattr_destroy(&attr);
if (ret != 0) {
errno = ret;
return -1;
}
return 0;
}


> be changed everywhere in the code.  Rough idea of the places could be

I think it worthy of adding after all virMutexInit, I will include it in
my V2 patchset.

> gotten by the following command:
> 
> git grep -nA5 virMutexInit | grep SystemError
> 
> but as I said, only rough idea :)
> 
> Martin
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 4/4] util: print errno in virObjectLockableNew

2014-07-15 Thread Martin Kletzander

On Tue, Jul 15, 2014 at 04:32:03PM +0800, Jincheng Miao wrote:

In virObjectLockableNew, when virMutexInit fails,
virReportSystemError should use errno to get the right
error number, instead of VIR_ERR_INTERNAL_ERROR.

Signed-off-by: Jincheng Miao 
---
src/util/virobject.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/util/virobject.c b/src/util/virobject.c
index 6cb84b4..b5d2c02 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -220,7 +220,7 @@ void *virObjectLockableNew(virClassPtr klass)
return NULL;

if (virMutexInit(&obj->lock) < 0) {
-virReportSystemError(VIR_ERR_INTERNAL_ERROR, "%s",
+virReportSystemError(errno, "%s",
 _("Unable to initialize mutex"));


I'm not sure errno is set when using our virMutexInit().  Most of the
code uses virReportError instead, I suggest using that.  This should
be changed everywhere in the code.  Rough idea of the places could be
gotten by the following command:

git grep -nA5 virMutexInit | grep SystemError

but as I said, only rough idea :)

Martin


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 4/4] util: print errno in virObjectLockableNew

2014-07-15 Thread Jincheng Miao
In virObjectLockableNew, when virMutexInit fails,
virReportSystemError should use errno to get the right
error number, instead of VIR_ERR_INTERNAL_ERROR.

Signed-off-by: Jincheng Miao 
---
 src/util/virobject.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/util/virobject.c b/src/util/virobject.c
index 6cb84b4..b5d2c02 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -220,7 +220,7 @@ void *virObjectLockableNew(virClassPtr klass)
 return NULL;
 
 if (virMutexInit(&obj->lock) < 0) {
-virReportSystemError(VIR_ERR_INTERNAL_ERROR, "%s",
+virReportSystemError(errno, "%s",
  _("Unable to initialize mutex"));
 virObjectUnref(obj);
 return NULL;
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list