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

Re: [libvirt] [PATCH v2 2/3] libxl: streamline top-level migrate functions



On 02/14/2017 04:44 AM, Joao Martins wrote:
On 02/14/2017 03:13 AM, Jim Fehlig wrote:
On 02/07/2017 05:35 PM, Joao Martins wrote:
This allows us to reuse a single function for both tunnelled and
non-tunnelled variants.

Signed-off-by: Joao Martins <joao m martins oracle com>
---
New in v2
---
 src/libxl/libxl_driver.c | 36 +++++++++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 9 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 3a69720..7bc8adf 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -5930,21 +5930,22 @@ libxlDomainMigrateBegin3Params(virDomainPtr domain,
 }

 static int
-libxlDomainMigratePrepare3Params(virConnectPtr dconn,
-                                 virTypedParameterPtr params,
-                                 int nparams,
-                                 const char *cookiein,
-                                 int cookieinlen,
-                                 char **cookieout ATTRIBUTE_UNUSED,
-                                 int *cookieoutlen ATTRIBUTE_UNUSED,
-                                 char **uri_out,
-                                 unsigned int flags)
+libxlDomainMigratePrepareCommon(virConnectPtr dconn,
+                                virTypedParameterPtr params,
+                                int nparams,
+                                const char *cookiein,
+                                int cookieinlen,
+                                char **cookieout ATTRIBUTE_UNUSED,
+                                int *cookieoutlen ATTRIBUTE_UNUSED,
+                                unsigned int flags,
+                                void *data)
 {
     libxlDriverPrivatePtr driver = dconn->privateData;
     virDomainDefPtr def = NULL;
     const char *dom_xml = NULL;
     const char *dname = NULL;
     const char *uri_in = NULL;
+    char **uri_out = data;

 #ifdef LIBXL_HAVE_NO_SUSPEND_RESUME
     virReportUnsupportedError();
@@ -5985,6 +5986,23 @@ libxlDomainMigratePrepare3Params(virConnectPtr dconn,
 }

 static int
+libxlDomainMigratePrepare3Params(virConnectPtr dconn,
+                                 virTypedParameterPtr params,
+                                 int nparams,
+                                 const char *cookiein,
+                                 int cookieinlen,
+                                 char **cookieout ATTRIBUTE_UNUSED,
+                                 int *cookieoutlen ATTRIBUTE_UNUSED,
+                                 char **uri_out,
+                                 unsigned int flags)
+{
+    return libxlDomainMigratePrepareCommon(dconn, params, nparams,
+                                           cookiein, cookieinlen,
+                                           cookieout, cookieoutlen,
+                                           flags, uri_out);
+}

It appears the ACL check must be done in libxlDomainMigratePrepare3Params to
satisfy 'make check'

./libxl/libxl_driver.c:5978 Mismatch check
'virDomainMigratePrepare3ParamsEnsureACL' for function
'libxlDomainMigratePrepareCommon'

Sorry, I naively ran solely the syntax checks when rebasing this series.

The ACL checks require dconn and def, and consequently the call to fetch def:

def = libxlDomainMigrationPrepareDef(driver, dom_xml, dname);

Requires dom_xml and dname being fetched from params.

:-(


Given all these dependencies having this common function for both prepare
functions doesn't seem to be doing much, as the top-level functions would end up
being very similar after these dependencies. Which makes me wonder if we should
dropped this (i.e. remove this PrepareCommon function) instead and go like
initial v1 (same comment would be applicable for the PrepareTunnel3Params in
patch 3). What do you think?

Agree. Drop this and go with v1 approach of separate functions.

Regards,
Jim


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