On 10/23/2014 02:17 PM, Eric Blake wrote:
On 10/22/2014 11:15 AM, Daniel P. Berrange wrote:
> Introduce a src/libvirt-domain.c file to hold all the
> methods related to the virDomain type.
> ---
> docs/apibuild.py | 2 +
> po/POTFILES.in | 1 +
> src/Makefile.am | 2 +
> src/libvirt-domain.c | 11112 ++++++++++++++++++++++++++++++++++++++++++
> src/libvirt.c | 12388 +++--------------------------------------------
> src/libvirt_internal.h | 6 +
> 6 files changed, 11774 insertions(+), 11737 deletions(-)
> create mode 100644 src/libvirt-domain.c
My trick for reviewing the earlier patches via a pre-process comparison
of lines added vs. lines removed failed on this one; you must have
reordered some functions or something similar that made the diff messy :(
> +int
> +virTypedParameterValidateSet(virConnectPtr conn,
> + virTypedParameterPtr params,
> + int nparams)
Ah. You left this function in place, but changed it from 'static int' to
'int', which in turn caused diff to think you re-arranged its location...
> int
> -virConnectListDomains(virConnectPtr conn, int *ids, int maxids)
> +virNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info)
followed by all sorts of junk like this.
Aha. You should run 'git config diff.algorithm patience'. git's
default non-patience algorithm is LOUSY at code motion patches. Once I
applied your patch, then regenerated it using the patience algorithm, it
became MUCH more readable, to the point that my code motion review trick
became a LOT easier to read.
ACK (but you may STILL want to do the refactoring of
virTypedParameterValidateSet in its own patch). Here's how I reviewed:
$ diff -u <(git diff --patience HEAD^ | sed -n 's/^\-//p') \
<(git diff --patience HEAD^ | sed -n 's/^\+//p')
--- /dev/fd/63 2014-10-23 14:27:58.067126468 -0600
+++ /dev/fd/62 2014-10-23 14:27:58.067126468 -0600
@@ -1,9 +1,49 @@
--- c/docs/apibuild.py
--- c/po/POTFILES.in
--- c/src/Makefile.am
--- /dev/null
--- c/src/libvirt.c
+++ w/docs/apibuild.py
+ "libvirt-domain.c": "Domain interfaces for the libvirt library",
+ "virTypedParameterValidateSet": "internal function in
virtypedparam.c",
+++ w/po/POTFILES.in
+src/libvirt-domain.c
+++ w/src/Makefile.am
+ libvirt-domain.c \
+ libvirt-domain.c \
+++ w/src/libvirt-domain.c
+/*
+ * libvirt-domain.c: entry points for virDomainPtr APIs
+ *
+ * Copyright (C) 2006-2014 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * <
http://www.gnu.org/licenses/>.
+ */
+
+#include <config.h>
+#include <sys/stat.h>
+
#include "intprops.h"
+
+#include "datatypes.h"
+#include "viralloc.h"
+#include "virfile.h"
+#include "virlog.h"
+#include "virtypedparam.h"
+
+VIR_LOG_INIT("libvirt.domain");
+
+#define VIR_FROM_THIS VIR_FROM_NONE
+
+
+/**
* virConnectListDomains:
* @conn: pointer to the hypervisor connection
* @ids: array to collect the list of IDs of active domains
@@ -2033,48 +2073,6 @@
}
-/* Helper function called to validate incoming client array on any
- * interface that sets typed parameters in the hypervisor. */
-static int
-virTypedParameterValidateSet(virConnectPtr conn,
- virTypedParameterPtr params,
- int nparams)
-{
- bool string_okay;
- size_t i;
-
- string_okay = VIR_DRV_SUPPORTS_FEATURE(conn->driver,
- conn,
- VIR_DRV_FEATURE_TYPED_PARAM_STRING);
- for (i = 0; i < nparams; i++) {
- if (strnlen(params[i].field, VIR_TYPED_PARAM_FIELD_LENGTH) ==
- VIR_TYPED_PARAM_FIELD_LENGTH) {
- virReportInvalidArg(params,
- _("string parameter name '%.*s' too
long"),
- VIR_TYPED_PARAM_FIELD_LENGTH,
- params[i].field);
- return -1;
- }
- if (params[i].type == VIR_TYPED_PARAM_STRING) {
- if (string_okay) {
- if (!params[i].value.s) {
- virReportInvalidArg(params,
- _("NULL string parameter
'%s'"),
- params[i].field);
- return -1;
- }
- } else {
- virReportInvalidArg(params,
- _("string parameter '%s'
unsupported"),
- params[i].field);
- return -1;
- }
- }
- }
- return 0;
-}
-
-
/**
* virDomainSetMemoryParameters:
* @domain: pointer to domain object
@@ -6435,12 +6433,6 @@
}
-/************************************************************************
- * *
- * Handling of defined but not running domains *
- * *
- ************************************************************************/
-
/**
* virDomainDefineXML:
* @conn: pointer to the hypervisor connection
@@ -7938,6 +7930,8 @@
virDispatchError(domain->conn);
return -1;
}
+
+
/**
* virDomainSetMetadata:
* @domain: a domain object
@@ -8395,11 +8389,6 @@
/**
-/*
- * Domain Event Notification
- */
-
-/**
* virConnectDomainEventRegister:
* @conn: pointer to the connection
* @cb: callback to the function handling domain events
@@ -8592,6 +8581,7 @@
}
+/**
* virDomainGetJobInfo:
* @domain: a domain object
* @info: pointer to a virDomainJobInfo structure allocated by the user
@@ -10797,6 +10787,7 @@
}
+
/**
* virConnectGetDomainCapabilities:
* @conn: pointer to the hypervisor connection
@@ -11128,7 +11119,53 @@
VIR_FREE(stats);
}
+++ w/src/libvirt.c
+/* Helper function called to validate incoming client array on any
+ * interface that sets typed parameters in the hypervisor. */
+int
+virTypedParameterValidateSet(virConnectPtr conn,
+ virTypedParameterPtr params,
+ int nparams)
+{
+ bool string_okay;
+ size_t i;
+ string_okay = VIR_DRV_SUPPORTS_FEATURE(conn->driver,
+ conn,
+ VIR_DRV_FEATURE_TYPED_PARAM_STRING);
+ for (i = 0; i < nparams; i++) {
+ if (strnlen(params[i].field, VIR_TYPED_PARAM_FIELD_LENGTH) ==
+ VIR_TYPED_PARAM_FIELD_LENGTH) {
+ virReportInvalidArg(params,
+ _("string parameter name '%.*s' too
long"),
+ VIR_TYPED_PARAM_FIELD_LENGTH,
+ params[i].field);
+ return -1;
+ }
+ if (params[i].type == VIR_TYPED_PARAM_STRING) {
+ if (string_okay) {
+ if (!params[i].value.s) {
+ virReportInvalidArg(params,
+ _("NULL string parameter
'%s'"),
+ params[i].field);
+ return -1;
+ }
+ } else {
+ virReportInvalidArg(params,
+ _("string parameter '%s'
unsupported"),
+ params[i].field);
+ return -1;
+ }
+ }
+ }
+ return 0;
+}
+
+
+++ w/src/libvirt_internal.h
+
+int
+virTypedParameterValidateSet(virConnectPtr conn,
+ virTypedParameterPtr params,
+ int nparams);
-/**
--- c/src/libvirt_internal.h
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library
http://libvirt.org