[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/3] use virReportOOMError, not VIR_ERR_NO_MEMORY



"Daniel P. Berrange" <berrange redhat com> wrote:
> On Tue, Jan 27, 2009 at 04:59:20PM +0100, Jim Meyering wrote:
>> Here's the big one:
>> >From 099536470ae2cbe9503ed471d391e403fb2587a4 Mon Sep 17 00:00:00 2001
>> From: Jim Meyering <meyering redhat com>
>> Date: Tue, 27 Jan 2009 12:20:06 +0100
>> Subject: [PATCH 1/3] error-reporting calls using VIR_ERR_NO_MEMORY: use virReportOOMError instead
>>
>> * src/uml_conf.c (VIR_FROM_THIS): Define to VIR_FROM_UML.
>> * src/xs_internal.c (VIR_FROM_THIS): Define to VIR_FROM_XEN.
>> * src/xml.c (VIR_FROM_THIS): Define to VIR_FROM_XML.
>> * src/stats_linux.c (VIR_FROM_THIS): Define to VIR_FROM_STATS_LINUX.
>> * src/datatypes.c (VIR_FROM_THIS): Define to VIR_FROM_NONE.
>> * src/lxc_conf.c (VIR_FROM_THIS): Define to VIR_FROM_LXC.
>> * src/libvirt.c (VIR_FROM_THIS): Define to VIR_FROM_NONE.
>> * src/node_device_conf.c (VIR_FROM_THIS): Define to VIR_FROM_NODEDEV.
>> * src/openvz_conf.c (VIR_FROM_THIS): Define to VIR_FROM_OPENVZ.
>> * src/openvz_driver.c (VIR_FROM_THIS): Define to VIR_FROM_OPENVZ.
>> * src/conf.c (VIR_FROM_THIS): Define to VIR_FROM_CONF.
>> Note: this loses config_filename:config_lineno diagnostics,
>> but that's ok.
>> * src/node_device.c (VIR_FROM_THIS): Define to VIR_FROM_NODEDEV.
>> * src/sexpr.c (VIR_FROM_THIS): Define to VIR_FROM_SEXPR.
>
> ACK.
>
> After a quick run through the patch I don't see any problems hiding
> there - well at least none which weren't already present.
...
> We should add a Makefile.maint rule to prohibit use of VIR_ERR_NO_MEMORY
> in any file except virterror.c

There were a few more to remove, so here are two patches.
First, an incremental removing the remaining uses of VIR_ERR_NO_MEMORY.
Then a patch adding a syntax-check rule to prevent reintroduction.

Note that the Makefile.maint patch isn't usable as-is;
it requires a preceding patch to sync from coreutils's maint.mk.
And once I did that, a few more syntax-check failures were triggered,
and so I disabled the corresponding rules (temporarily) in another patch.
Both coming up.

FYI, here are the exceptions in .c/.h files:

include/libvirt/virterror.h-typedef enum {
include/libvirt/virterror.h-    VIR_ERR_OK = 0,
include/libvirt/virterror.h-    VIR_ERR_INTERNAL_ERROR, /* internal error */
include/libvirt/virterror.h:    VIR_ERR_NO_MEMORY,  /* memory allocation failure */
include/libvirt/virterror.h-    VIR_ERR_NO_SUPPORT, /* no support for this function */
include/libvirt/virterror.h-    VIR_ERR_UNKNOWN_HOST,/* could not resolve hostname */
include/libvirt/virterror.h-    VIR_ERR_NO_CONNECT, /* can't connect to hypervisor */
--
qemud/remote.c-remoteDispatchOOMError (remote_error *rerr)
qemud/remote.c-{
qemud/remote.c-    remoteDispatchStringError(rerr,
qemud/remote.c:                              VIR_ERR_NO_MEMORY,
qemud/remote.c-                              NULL);
qemud/remote.c-}
qemud/remote.c-
--
src/virterror.c-            else
src/virterror.c-              errmsg = _("internal error");
src/virterror.c-            break;
src/virterror.c:        case VIR_ERR_NO_MEMORY:
src/virterror.c-            errmsg = _("out of memory");
src/virterror.c-            break;
src/virterror.c-        case VIR_ERR_NO_SUPPORT:
--
src/virterror.c-{
src/virterror.c-    const char *virerr;
src/virterror.c-
src/virterror.c:    virerr = virErrorMsg(VIR_ERR_NO_MEMORY, NULL);
src/virterror.c:    virRaiseError(conn, NULL, NULL, domcode, VIR_ERR_NO_MEMORY, VIR_ERR_ERROR,
src/virterror.c-                  virerr, NULL, NULL, -1, -1, virerr, NULL);
src/virterror.c-}


>From afa312df336037fcfc166939f3aa37513da4e41d Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering redhat com>
Date: Tue, 27 Jan 2009 21:37:13 +0100
Subject: [PATCH 4/8] FIXME: to-be-squashed: VIR_ERR_NO_MEMORY -> virReportOOMError

---
 src/remote_internal.c         |    8 ++------
 src/sexpr.c                   |    3 +--
 src/storage_backend_fs.c      |    7 +++----
 src/storage_backend_logical.c |    4 ++--
 4 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/src/remote_internal.c b/src/remote_internal.c
index 5c2e705..0fb5d87 100644
--- a/src/remote_internal.c
+++ b/src/remote_internal.c
@@ -4957,10 +4957,7 @@ static char *addrToString(struct sockaddr_storage *sa, socklen_t salen)
     }

     if (VIR_ALLOC_N(addr, strlen(host) + 1 + strlen(port) + 1) < 0) {
-        virRaiseError (NULL, NULL, NULL, VIR_FROM_REMOTE,
-                         VIR_ERR_NO_MEMORY, VIR_ERR_ERROR,
-                         NULL, NULL, NULL, 0, 0,
-                         "address");
+        virReportOOMError (NULL);
         return NULL;
     }

@@ -6315,8 +6312,7 @@ call (virConnectPtr conn, struct private_data *priv,
                            ret_filter, ret);

     if (!thiscall) {
-        error (flags & REMOTE_CALL_IN_OPEN ? NULL : conn,
-               VIR_ERR_NO_MEMORY, NULL);
+        virReportOOMError (flags & REMOTE_CALL_IN_OPEN ? NULL : conn);
         return -1;
     }

diff --git a/src/sexpr.c b/src/sexpr.c
index 7450c79..bc82d1f 100644
--- a/src/sexpr.c
+++ b/src/sexpr.c
@@ -361,8 +361,7 @@ _string2sexpr(const char *buffer, size_t * end)

             ret->u.value = strndup(start, ptr - start);
             if (ret->u.value == NULL) {
-                virSexprError(VIR_ERR_NO_MEMORY,
-                              "%s", _("failed to copy a string"));
+                virReportOOMError(NULL);
                 goto error;
             }
         }
diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c
index 6fd9f8f..345dc40 100644
--- a/src/storage_backend_fs.c
+++ b/src/storage_backend_fs.c
@@ -196,7 +196,7 @@ qcowXGetBackingStore(virConnectPtr conn,
     if (size + 1 == 0)
         return BACKING_STORE_INVALID;
     if (VIR_ALLOC_N(*res, size + 1) < 0) {
-        virStorageReportError(conn, VIR_ERR_NO_MEMORY, _("backing store path"));
+        virReportOOMError(conn);
         return BACKING_STORE_ERROR;
     }
     memcpy(*res, buf + offset, size);
@@ -237,7 +237,7 @@ vmdk4GetBackingStore(virConnectPtr conn,
     *end = '\0';
     *res = strdup(start);
     if (*res == NULL) {
-        virStorageReportError(conn, VIR_ERR_NO_MEMORY, _("backing store path"));
+        virReportOOMError(conn);
         return BACKING_STORE_ERROR;
     }
     return BACKING_STORE_OK;
@@ -395,8 +395,7 @@ static int virStorageBackendProbeTarget(virConnectPtr conn,
                     = absolutePathFromBaseFile(target->path, base);
                 VIR_FREE(base);
                 if (*backingStore == NULL) {
-                    virStorageReportError(conn, VIR_ERR_NO_MEMORY,
-                                          _("backing store path"));
+                    virReportOOMError(conn);
                     return -1;
                 }
             }
diff --git a/src/storage_backend_logical.c b/src/storage_backend_logical.c
index 857262d..702a191 100644
--- a/src/storage_backend_logical.c
+++ b/src/storage_backend_logical.c
@@ -127,7 +127,7 @@ virStorageBackendLogicalMakeVol(virConnectPtr conn,

     if (vol->key == NULL &&
         (vol->key = strdup(groups[2])) == NULL) {
-        virStorageReportError(conn, VIR_ERR_NO_MEMORY, "%s", _("volume"));
+        virReportOOMError(conn);
         return -1;
     }

@@ -144,7 +144,7 @@ virStorageBackendLogicalMakeVol(virConnectPtr conn,

     if ((vol->source.extents[vol->source.nextent].path =
          strdup(groups[3])) == NULL) {
-        virStorageReportError(conn, VIR_ERR_NO_MEMORY, "%s", _("extents"));
+        virReportOOMError(conn);
         return -1;
     }

--
1.6.1.1.374.g0d9d7


>From 3d47ae92e1148c85ba0e4a22848b250681c13a22 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering redhat com>
Date: Wed, 28 Jan 2009 16:49:58 +0100
Subject: [PATCH 8/8] prohibit new uses of VIR_ERR_NO_MEMORY

* Makefile.maint (sc_prohibit_VIR_ERR_NO_MEMORY): New rule.
* .x-sc_prohibit_VIR_ERR_NO_MEMORY: New file: exceptions.
---
 .x-sc_prohibit_VIR_ERR_NO_MEMORY |    8 ++++++++
 Makefile.maint                   |    6 ++++++
 2 files changed, 14 insertions(+), 0 deletions(-)
 create mode 100644 .x-sc_prohibit_VIR_ERR_NO_MEMORY

diff --git a/.x-sc_prohibit_VIR_ERR_NO_MEMORY b/.x-sc_prohibit_VIR_ERR_NO_MEMORY
new file mode 100644
index 0000000..efb2012
--- /dev/null
+++ b/.x-sc_prohibit_VIR_ERR_NO_MEMORY
@@ -0,0 +1,8 @@
+ChangeLog
+docs/devhelp/libvirt-virterror.html
+docs/html/libvirt-virterror.html
+docs/libvirt-api.xml
+docs/libvirt-refs.xml
+include/libvirt/virterror.h
+qemud/remote.c
+src/virterror.c
diff --git a/Makefile.maint b/Makefile.maint
index c50d100..97f2526 100644
--- a/Makefile.maint
+++ b/Makefile.maint
@@ -112,6 +112,11 @@ sc_prohibit_asprintf:
 	msg='use virAsprintf, not a'sprintf				\
 	  $(_prohibit_regexp)

+sc_prohibit_VIR_ERR_NO_MEMORY:
+	@re='\<V''IR_ERR_NO_MEMORY\>'					\
+	msg='use virReportOOMError, not V'IR_ERR_NO_MEMORY		\
+	  $(_prohibit_regexp)
+
 sc_prohibit_nonreentrant:
 	@fail=0 ; \
 	for i in $(NON_REENTRANT) ; \
--
1.6.1.1.374.g0d9d7


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]