7:09 a.m.
NOTE that minimal command to migrate vz domain is like next:
virsh -c vz:///system migrate 200 vz+ssh://shiny0/system --p2p
--live --compressed --persistent --undefinesource
==Difference from v1:
1. Patch is quite different. First patchset implements migration thru managed
migration scheme. This one goes thru p2p scheme. I belive this is a better
approach. Vz migration is done via vz sdk and first patchset uses 5 phased
migration only to get a token from destination on prepare phase which is kind a
misuse. This patch just adds vz specific function to driver interface
to archive the same goal.
2. Offline migration is supported as there is no more dependency on current
flow of managed migration scheme.
==Difference from v2:
1. Implement thru direct migration instead of p2p. p2p is just managed
5-staged migration when managing is done on daemon side. Vz migration stages
are all hidden in vz sdk and thus it would be more consistent to use direct
scheme.
2. Use existing driver function for prepare migration phase to pass session
uuid from destination to source instead of new one. As vz migration is direct
one we will not use prepare phase function in a straight forward manner
anyway.
==Difference from v3:
Return back to p2p implementation. Difference from p2p implementation
in v2 is that prepare phase driver function is used to pass session_uuid
from destination to source instead of new ad-hoc driver function.
src/vz/vz_driver.c | 342 ++++++++++++++++++++++++++++++++++++++++++++++++++++
src/vz/vz_sdk.c | 86 +++++++++++--
src/vz/vz_sdk.h | 6 +
src/vz/vz_utils.h | 4 +-
4 files changed, 423 insertions(+), 15 deletions(-)
7:09 a.m.
New subject: [libvirt] [PATCH v4 1/6] vz: save session uuid on login
This session uuid acts as authN token for different multihost vz operations one
of which is migration. Unfortunately we can't get it from server at any time
thus we need to save it at login.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/vz/vz_sdk.c | 39 +++++++++++++++++++++++++++++----------
src/vz/vz_utils.h | 2 +-
2 files changed, 30 insertions(+), 11 deletions(-)
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 744b58a..f7253de 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -37,6 +37,9 @@
#define VIR_FROM_THIS VIR_FROM_PARALLELS
#define JOB_INFINIT_WAIT_TIMEOUT UINT_MAX
+static int
+prlsdkUUIDParse(const char *uuidstr, unsigned char *uuid);
+
VIR_LOG_INIT("parallels.sdk");
/*
@@ -228,24 +231,40 @@ prlsdkDeinit(void)
int
prlsdkConnect(vzConnPtr privconn)
{
- PRL_RESULT ret;
+ int ret = -1;
+ PRL_RESULT pret;
PRL_HANDLE job = PRL_INVALID_HANDLE;
+ PRL_HANDLE result = PRL_INVALID_HANDLE;
+ PRL_HANDLE response = PRL_INVALID_HANDLE;
+ char session_uuid[VIR_UUID_STRING_BUFLEN + 2];
+ PRL_UINT32 buflen = ARRAY_CARDINALITY(session_uuid);
- ret = PrlSrv_Create(&privconn->server);
- if (PRL_FAILED(ret)) {
- logPrlError(ret);
- return -1;
- }
+ pret = PrlSrv_Create(&privconn->server);
+ prlsdkCheckRetGoto(pret, cleanup);
job = PrlSrv_LoginLocalEx(privconn->server, NULL, 0,
PSL_HIGH_SECURITY, PACF_NON_INTERACTIVE_MODE);
+ if (PRL_FAILED(getJobResult(job, &result)))
+ goto cleanup;
- if (waitJob(job)) {
+ pret = PrlResult_GetParam(result, &response);
+ prlsdkCheckRetGoto(pret, cleanup);
+
+ pret = PrlLoginResponse_GetSessionUuid(response, session_uuid, &buflen);
+ prlsdkCheckRetGoto(pret, cleanup);
+
+ if (prlsdkUUIDParse(session_uuid, privconn->session_uuid) < 0)
+ goto cleanup;
+
+ ret = 0;
+
+ cleanup:
+ if (ret < 0)
PrlHandle_Free(privconn->server);
- return -1;
- }
+ PrlHandle_Free(result);
+ PrlHandle_Free(response);
- return 0;
+ return ret;
}
void
diff --git a/src/vz/vz_utils.h b/src/vz/vz_utils.h
index db09647..fe54b25 100644
--- a/src/vz/vz_utils.h
+++ b/src/vz/vz_utils.h
@@ -60,7 +60,7 @@ struct _vzConn {
/* Immutable pointer, self-locking APIs */
virDomainObjListPtr domains;
-
+ unsigned char session_uuid[VIR_UUID_BUFLEN];
PRL_HANDLE server;
virStoragePoolObjList pools;
virNetworkObjListPtr networks;
--
1.7.1
11:25 a.m.
New subject: [libvirt] [PATCH v4 1/6] vz: save session uuid on login
On Wed, Sep 02, 2015 at 03:09:22PM +0300, Nikolay Shirokovskiy wrote:
This session uuid acts as authN token for different multihost vz
operations one
of which is migration. Unfortunately we can't get it from server at any time
thus we need to save it at login.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/vz/vz_sdk.c | 39 +++++++++++++++++++++++++++++----------
src/vz/vz_utils.h | 2 +-
2 files changed, 30 insertions(+), 11 deletions(-)
ACK
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
7:09 a.m.
New subject: [libvirt] [PATCH v4 2/6] vz: add migration backbone code
From: nshirokovskiy(a)virtuozzo.com <nshirokovskiy(a)virtuozzo.com>
This patch makes basic vz migration possible. For example by virsh:
virsh -c vz:///system migrate $NAME vz+ssh://$DST/system --p2p
Vz migration is implemented as p2p migration. The reason
is that vz sdk do all the job. The question may arise then
why don't implement it as a direct migration. The reason
is that we want to leverage rich libvirt authentication abilities
we lack in vz sdk. We can do it because vz sdk can use tokens to
factor out authentication from migration command.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/vz/vz_driver.c | 192 ++++++++++++++++++++++++++++++++++++++++++++++++++++
src/vz/vz_sdk.c | 33 +++++++++
src/vz/vz_sdk.h | 2 +
3 files changed, 227 insertions(+), 0 deletions(-)
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 8fa7957..f3fd599 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -1343,6 +1343,195 @@ vzDomainMemoryStats(virDomainPtr domain,
return ret;
}
+static char*
+vzFormatCookie(const unsigned char *session_uuid)
+{
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+ virBufferAddLit(&buf, "<vz-migration1>\n");
+ virUUIDFormat(session_uuid, uuidstr);
+ virBufferAsprintf(&buf,
"<session_uuid>%s</session_uuid>\n", uuidstr);
+ virBufferAddLit(&buf, "</vz-migration1>\n");
+
+ if (virBufferCheckError(&buf) < 0)
+ return NULL;
+
+ return virBufferContentAndReset(&buf);
+}
+
+static int
+vzParseCookie(const char *xml, unsigned char *session_uuid)
+{
+ xmlDocPtr doc = NULL;
+ xmlXPathContextPtr ctx = NULL;
+ char *tmp = NULL;
+ int ret = -1;
+
+ if (!(doc = virXMLParseStringCtxt(xml, _("(_migration_cookie)"),
&ctx)))
+ goto cleanup;
+
+ if (!(tmp = virXPathString("string(./session_uuid[1])", ctx))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("missing session_uuid element in migration
data"));
+ goto cleanup;
+ }
+ if (virUUIDParse(tmp, session_uuid) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("malformed session_uuid element in
migration data"));
+ goto cleanup;
+ }
+ ret = 0;
+
+ cleanup:
+ xmlXPathFreeContext(ctx);
+ xmlFreeDoc(doc);
+ VIR_FREE(tmp);
+
+ return ret;
+}
+
+static int
+vzDomainMigratePrepare3Params(virConnectPtr conn,
+ virTypedParameterPtr params ATTRIBUTE_UNUSED,
+ int nparams ATTRIBUTE_UNUSED,
+ const char *cookiein ATTRIBUTE_UNUSED,
+ int cookieinlen ATTRIBUTE_UNUSED,
+ char **cookieout,
+ int *cookieoutlen,
+ char **uri_out ATTRIBUTE_UNUSED,
+ unsigned int flags)
+{
+ vzConnPtr privconn = conn->privateData;
+ int ret = -1;
+
+ virCheckFlags(0, -1);
+
+ if (!(*cookieout = vzFormatCookie(privconn->session_uuid)))
+ goto cleanup;
+ *cookieoutlen = strlen(*cookieout) + 1;
+ ret = 0;
+
+ cleanup:
+ if (ret != 0) {
+ VIR_FREE(*cookieout);
+ *cookieoutlen = 0;
+ }
+
+ return ret;
+}
+
+static int
+vzConnectSupportsFeature(virConnectPtr conn ATTRIBUTE_UNUSED, int feature)
+{
+ switch (feature) {
+ case VIR_DRV_FEATURE_MIGRATION_PARAMS:
+ case VIR_DRV_FEATURE_MIGRATION_P2P:
+ return 1;
+ default:
+ return 0;
+ }
+}
+
+static virURIPtr
+vzMakeVzUri(const char *connuri_str)
+{
+ virURIPtr connuri = NULL;
+ virURIPtr vzuri = NULL;
+ int ret = -1;
+
+ if (!(connuri = virURIParse(connuri_str)))
+ goto cleanup;
+
+ if (VIR_ALLOC(vzuri) < 0)
+ goto cleanup;
+ memset(vzuri, 0, sizeof(*vzuri));
+
+ if (VIR_STRDUP(vzuri->server, connuri->server) == -1)
+ goto cleanup;
+
+ ret = 0;
+
+ cleanup:
+
+ virURIFree(connuri);
+ if (ret < 0) {
+ virURIFree(vzuri);
+ vzuri = NULL;
+ }
+
+ return vzuri;
+}
+
+#define VZ_MIGRATION_FLAGS (VIR_MIGRATE_PEER2PEER)
+
+#define VZ_MIGRATION_PARAMETERS (NULL)
+
+static int
+vzDomainMigratePerform3Params(virDomainPtr domain,
+ const char *dconnuri,
+ virTypedParameterPtr params,
+ int nparams,
+ const char *cookiein ATTRIBUTE_UNUSED,
+ int cookieinlen ATTRIBUTE_UNUSED,
+ char **cookieout ATTRIBUTE_UNUSED,
+ int *cookieoutlen ATTRIBUTE_UNUSED,
+ unsigned int flags)
+{
+ int ret = -1;
+ virDomainObjPtr dom = NULL;
+ virConnectPtr dconn = NULL;
+ virURIPtr vzuri = NULL;
+ unsigned char session_uuid[VIR_UUID_BUFLEN];
+ vzConnPtr privconn = domain->conn->privateData;
+ char *cookie = NULL;
+ int cookielen = 0;
+
+ virCheckFlags(VZ_MIGRATION_FLAGS, -1);
+
+ if (virTypedParamsValidate(params, nparams, VZ_MIGRATION_PARAMETERS) < 0)
+ goto cleanup;
+
+ if (!(vzuri = vzMakeVzUri(dconnuri)))
+ goto cleanup;
+
+ if (!(dom = vzDomObjFromDomain(domain)))
+ goto cleanup;
+
+ dconn = virConnectOpen(dconnuri);
+ if (dconn == NULL) {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("Failed to connect to remote libvirt URI %s: %s"),
+ dconnuri, virGetLastErrorMessage());
+ goto cleanup;
+ }
+
+ /* NULL and zero elements are unused */
+ if (virDomainMigratePrepare3Params(dconn, NULL, 0, NULL, 0,
+ &cookie, &cookielen, NULL, 0) < 0)
+ goto cleanup;
+
+ if (vzParseCookie(cookie, session_uuid) < 0)
+ goto cleanup;
+
+ if (prlsdkMigrate(dom, vzuri, session_uuid) < 0)
+ goto cleanup;
+
+ virDomainObjListRemove(privconn->domains, dom);
+ dom = NULL;
+
+ ret = 0;
+
+ cleanup:
+ if (dom)
+ virObjectUnlock(dom);
+ virObjectUnref(dconn);
+ virURIFree(vzuri);
+ VIR_FREE(cookie);
+
+ return ret;
+}
+
static virHypervisorDriver vzDriver = {
.name = "vz",
.connectOpen = vzConnectOpen, /* 0.10.0 */
@@ -1396,6 +1585,9 @@ static virHypervisorDriver vzDriver = {
.domainBlockStatsFlags = vzDomainBlockStatsFlags, /* 1.2.17 */
.domainInterfaceStats = vzDomainInterfaceStats, /* 1.2.17 */
.domainMemoryStats = vzDomainMemoryStats, /* 1.2.17 */
+ .connectSupportsFeature = vzConnectSupportsFeature, /* 1.2.20 */
+ .domainMigratePrepare3Params = vzDomainMigratePrepare3Params, /* 1.2.20 */
+ .domainMigratePerform3Params = vzDomainMigratePerform3Params, /* 1.2.20 */
};
static virConnectDriver vzConnectDriver = {
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index f7253de..783438d 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -4054,3 +4054,36 @@ prlsdkGetMemoryStats(virDomainObjPtr dom,
return ret;
}
+
+/* high security is default choice for 2 reasons:
+ 1. as this is the highest set security we can't get
+ reject from server with high security settings
+ 2. this is on par with security level of driver
+ connection to dispatcher */
+
+#define PRLSDK_MIGRATION_FLAGS (PSL_HIGH_SECURITY)
+
+int prlsdkMigrate(virDomainObjPtr dom, virURIPtr uri,
+ const unsigned char *session_uuid)
+{
+ int ret = -1;
+ vzDomObjPtr privdom = dom->privateData;
+ PRL_HANDLE job = PRL_INVALID_HANDLE;
+ char uuidstr[VIR_UUID_STRING_BUFLEN + 2];
+
+ prlsdkUUIDFormat(session_uuid, uuidstr);
+ job = PrlVm_MigrateEx(privdom->sdkdom, uri->server, uri->port, uuidstr,
+ "", /* use default dir for migrated instance bundle
*/
+ PRLSDK_MIGRATION_FLAGS,
+ 0, /* reserved flags */
+ PRL_TRUE /* don't ask for confirmations */
+ );
+
+ if (PRL_FAILED(waitJob(job)))
+ goto cleanup;
+
+ ret = 0;
+
+ cleanup:
+ return ret;
+}
diff --git a/src/vz/vz_sdk.h b/src/vz/vz_sdk.h
index ebe4591..d3f0caf 100644
--- a/src/vz/vz_sdk.h
+++ b/src/vz/vz_sdk.h
@@ -76,3 +76,5 @@ int
prlsdkGetVcpuStats(virDomainObjPtr dom, int idx, unsigned long long *time);
int
prlsdkGetMemoryStats(virDomainObjPtr dom, virDomainMemoryStatPtr stats, unsigned int
nr_stats);
+int
+prlsdkMigrate(virDomainObjPtr dom, virURIPtr uri, const char unsigned *session_uuid);
--
1.7.1
11:45 a.m.
New subject: [libvirt] [PATCH v4 2/6] vz: add migration backbone code
On Wed, Sep 02, 2015 at 03:09:23PM +0300, Nikolay Shirokovskiy wrote:
From: nshirokovskiy(a)virtuozzo.com
<nshirokovskiy(a)virtuozzo.com>
This patch makes basic vz migration possible. For example by virsh:
virsh -c vz:///system migrate $NAME vz+ssh://$DST/system --p2p
Vz migration is implemented as p2p migration. The reason
is that vz sdk do all the job. The question may arise then
why don't implement it as a direct migration. The reason
is that we want to leverage rich libvirt authentication abilities
we lack in vz sdk. We can do it because vz sdk can use tokens to
factor out authentication from migration command.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/vz/vz_driver.c | 192 ++++++++++++++++++++++++++++++++++++++++++++++++++++
src/vz/vz_sdk.c | 33 +++++++++
src/vz/vz_sdk.h | 2 +
3 files changed, 227 insertions(+), 0 deletions(-)
+static int
+vzDomainMigratePrepare3Params(virConnectPtr conn,
+ virTypedParameterPtr params ATTRIBUTE_UNUSED,
+ int nparams ATTRIBUTE_UNUSED,
+ const char *cookiein ATTRIBUTE_UNUSED,
+ int cookieinlen ATTRIBUTE_UNUSED,
+ char **cookieout,
+ int *cookieoutlen,
+ char **uri_out ATTRIBUTE_UNUSED,
+ unsigned int flags)
+{
+ vzConnPtr privconn = conn->privateData;
+ int ret = -1;
+
+ virCheckFlags(0, -1);
+
+ if (!(*cookieout = vzFormatCookie(privconn->session_uuid)))
+ goto cleanup;
+ *cookieoutlen = strlen(*cookieout) + 1;
As mentioned before, you should fill in uri_out here with the
hypervisor URI to use for migration, by filling in the URI of
the current host (ie dest host). eg
char *thishost = virGetHostname();
virAsprintf(uri_out, "tcp://%s", thishost);
VIR_FREE(thishost);
+ ret = 0;
+
+ cleanup:
+ if (ret != 0) {
+ VIR_FREE(*cookieout);
+ *cookieoutlen = 0;
+ }
+
+ return ret;
+}
+
+static int
+vzDomainMigratePerform3Params(virDomainPtr domain,
+ const char *dconnuri,
+ virTypedParameterPtr params,
+ int nparams,
+ const char *cookiein ATTRIBUTE_UNUSED,
+ int cookieinlen ATTRIBUTE_UNUSED,
+ char **cookieout ATTRIBUTE_UNUSED,
+ int *cookieoutlen ATTRIBUTE_UNUSED,
+ unsigned int flags)
+{
+ int ret = -1;
+ virDomainObjPtr dom = NULL;
+ virConnectPtr dconn = NULL;
+ virURIPtr vzuri = NULL;
+ unsigned char session_uuid[VIR_UUID_BUFLEN];
+ vzConnPtr privconn = domain->conn->privateData;
+ char *cookie = NULL;
+ int cookielen = 0;
+
+ virCheckFlags(VZ_MIGRATION_FLAGS, -1);
+
+ if (virTypedParamsValidate(params, nparams, VZ_MIGRATION_PARAMETERS) < 0)
+ goto cleanup;
+
+ if (!(vzuri = vzMakeVzUri(dconnuri)))
+ goto cleanup;
This is not right - you can't use the libvirt URI to form the
hypervisor migration URI, since the libvirt URI may not in
fact refer to the hypervisor host.
eg people may be accessing libvirt(d) via a SSH tunnel in
which case the dconnuri would include 'localhost' and not
the actual target host. This is why you must fill in the
uri_out parameter in the Prepare() method and use that,
if the "migrateuri" parameter is not provided in the
virTypedParams array.
+
+ if (!(dom = vzDomObjFromDomain(domain)))
+ goto cleanup;
+
+ dconn = virConnectOpen(dconnuri);
+ if (dconn == NULL) {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("Failed to connect to remote libvirt URI %s: %s"),
+ dconnuri, virGetLastErrorMessage());
+ goto cleanup;
+ }
+
+ /* NULL and zero elements are unused */
+ if (virDomainMigratePrepare3Params(dconn, NULL, 0, NULL, 0,
+ &cookie, &cookielen, NULL, 0) < 0)
+ goto cleanup;
Since this is implementing v3, I'd prefer to see you provide the
full set of 5 callbacks, even though they will currently be no-ops.
This provides better future proofing for the migration impl in case
those become neccessary later.
You can also then trivially implement the non-p2p plain migration
in this method, by checking whether or not the PEER2PEER flag
is set or not. If it is not set, you can just skip the connect
open & prepare calls on the basis that libvirt will have done
them for you.
+
+ if (vzParseCookie(cookie, session_uuid) < 0)
+ goto cleanup;
+
+ if (prlsdkMigrate(dom, vzuri, session_uuid) < 0)
+ goto cleanup;
+
+ virDomainObjListRemove(privconn->domains, dom);
+ dom = NULL;
+
+ ret = 0;
+
+ cleanup:
+ if (dom)
+ virObjectUnlock(dom);
+ virObjectUnref(dconn);
+ virURIFree(vzuri);
+ VIR_FREE(cookie);
+
+ return ret;
+}
+
static virHypervisorDriver vzDriver = {
.name = "vz",
.connectOpen = vzConnectOpen, /* 0.10.0 */
@@ -1396,6 +1585,9 @@ static virHypervisorDriver vzDriver = {
.domainBlockStatsFlags = vzDomainBlockStatsFlags, /* 1.2.17 */
.domainInterfaceStats = vzDomainInterfaceStats, /* 1.2.17 */
.domainMemoryStats = vzDomainMemoryStats, /* 1.2.17 */
+ .connectSupportsFeature = vzConnectSupportsFeature, /* 1.2.20 */
+ .domainMigratePrepare3Params = vzDomainMigratePrepare3Params, /* 1.2.20 */
+ .domainMigratePerform3Params = vzDomainMigratePerform3Params, /* 1.2.20 */
Somewhat annoyingly you also need to implement the callbacks for
.domainMigratePrepare3 and .domainMigratePerform3, as we don't
automatically convert non-params usage to the params based
method AFAICT.
Your impl of .domainMigratePerform3 could pack the values into a
virTypedParams array and then call .domainMigratePerform3Params,
or do the reverse.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
2:56 a.m.
New subject: [libvirt] [PATCH v4 2/6] vz: add migration backbone code
On 03.09.2015 19:45, Daniel P. Berrange wrote:
On Wed, Sep 02, 2015 at 03:09:23PM +0300, Nikolay Shirokovskiy
wrote:
> From: nshirokovskiy(a)virtuozzo.com <nshirokovskiy(a)virtuozzo.com>
>
> This patch makes basic vz migration possible. For example by virsh:
>
> virsh -c vz:///system migrate $NAME vz+ssh://$DST/system --p2p
>
> Vz migration is implemented as p2p migration. The reason
> is that vz sdk do all the job. The question may arise then
> why don't implement it as a direct migration. The reason
> is that we want to leverage rich libvirt authentication abilities
> we lack in vz sdk. We can do it because vz sdk can use tokens to
> factor out authentication from migration command.
>
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
> ---
> src/vz/vz_driver.c | 192 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> src/vz/vz_sdk.c | 33 +++++++++
> src/vz/vz_sdk.h | 2 +
> 3 files changed, 227 insertions(+), 0 deletions(-)
>
> +static int
> +vzDomainMigratePrepare3Params(virConnectPtr conn,
> + virTypedParameterPtr params ATTRIBUTE_UNUSED,
> + int nparams ATTRIBUTE_UNUSED,
> + const char *cookiein ATTRIBUTE_UNUSED,
> + int cookieinlen ATTRIBUTE_UNUSED,
> + char **cookieout,
> + int *cookieoutlen,
> + char **uri_out ATTRIBUTE_UNUSED,
> + unsigned int flags)
> +{
> + vzConnPtr privconn = conn->privateData;
> + int ret = -1;
> +
> + virCheckFlags(0, -1);
> +
> + if (!(*cookieout = vzFormatCookie(privconn->session_uuid)))
> + goto cleanup;
> + *cookieoutlen = strlen(*cookieout) + 1;
As mentioned before, you should fill in uri_out here with the
hypervisor URI to use for migration, by filling in the URI of
the current host (ie dest host). eg
char *thishost = virGetHostname();
virAsprintf(uri_out, "tcp://%s", thishost);
VIR_FREE(thishost);
> + ret = 0;
> +
> + cleanup:
> + if (ret != 0) {
> + VIR_FREE(*cookieout);
> + *cookieoutlen = 0;
> + }
> +
> + return ret;
> +}
> +
> +static int
> +vzDomainMigratePerform3Params(virDomainPtr domain,
> + const char *dconnuri,
> + virTypedParameterPtr params,
> + int nparams,
> + const char *cookiein ATTRIBUTE_UNUSED,
> + int cookieinlen ATTRIBUTE_UNUSED,
> + char **cookieout ATTRIBUTE_UNUSED,
> + int *cookieoutlen ATTRIBUTE_UNUSED,
> + unsigned int flags)
> +{
> + int ret = -1;
> + virDomainObjPtr dom = NULL;
> + virConnectPtr dconn = NULL;
> + virURIPtr vzuri = NULL;
> + unsigned char session_uuid[VIR_UUID_BUFLEN];
> + vzConnPtr privconn = domain->conn->privateData;
> + char *cookie = NULL;
> + int cookielen = 0;
> +
> + virCheckFlags(VZ_MIGRATION_FLAGS, -1);
> +
> + if (virTypedParamsValidate(params, nparams, VZ_MIGRATION_PARAMETERS) < 0)
> + goto cleanup;
> +
> + if (!(vzuri = vzMakeVzUri(dconnuri)))
> + goto cleanup;
This is not right - you can't use the libvirt URI to form the
hypervisor migration URI, since the libvirt URI may not in
fact refer to the hypervisor host.
eg people may be accessing libvirt(d) via a SSH tunnel in
which case the dconnuri would include 'localhost' and not
the actual target host. This is why you must fill in the
uri_out parameter in the Prepare() method and use that,
if the "migrateuri" parameter is not provided in the
virTypedParams array.
> +
> + if (!(dom = vzDomObjFromDomain(domain)))
> + goto cleanup;
> +
> + dconn = virConnectOpen(dconnuri);
> + if (dconn == NULL) {
> + virReportError(VIR_ERR_OPERATION_FAILED,
> + _("Failed to connect to remote libvirt URI %s:
%s"),
> + dconnuri, virGetLastErrorMessage());
> + goto cleanup;
> + }
> +
> + /* NULL and zero elements are unused */
> + if (virDomainMigratePrepare3Params(dconn, NULL, 0, NULL, 0,
> + &cookie, &cookielen, NULL, 0) <
0)
> + goto cleanup;
Since this is implementing v3, I'd prefer to see you provide the
full set of 5 callbacks, even though they will currently be no-ops.
This provides better future proofing for the migration impl in case
those become neccessary later.
You can also then trivially implement the non-p2p plain migration
in this method, by checking whether or not the PEER2PEER flag
is set or not. If it is not set, you can just skip the connect
open & prepare calls on the basis that libvirt will have done
them for you.
Ok.
> +
> + if (vzParseCookie(cookie, session_uuid) < 0)
> + goto cleanup;
> +
> + if (prlsdkMigrate(dom, vzuri, session_uuid) < 0)
> + goto cleanup;
> +
> + virDomainObjListRemove(privconn->domains, dom);
> + dom = NULL;
> +
> + ret = 0;
> +
> + cleanup:
> + if (dom)
> + virObjectUnlock(dom);
> + virObjectUnref(dconn);
> + virURIFree(vzuri);
> + VIR_FREE(cookie);
> +
> + return ret;
> +}
> +
> static virHypervisorDriver vzDriver = {
> .name = "vz",
> .connectOpen = vzConnectOpen, /* 0.10.0 */
> @@ -1396,6 +1585,9 @@ static virHypervisorDriver vzDriver = {
> .domainBlockStatsFlags = vzDomainBlockStatsFlags, /* 1.2.17 */
> .domainInterfaceStats = vzDomainInterfaceStats, /* 1.2.17 */
> .domainMemoryStats = vzDomainMemoryStats, /* 1.2.17 */
> + .connectSupportsFeature = vzConnectSupportsFeature, /* 1.2.20 */
> + .domainMigratePrepare3Params = vzDomainMigratePrepare3Params, /* 1.2.20 */
> + .domainMigratePerform3Params = vzDomainMigratePerform3Params, /* 1.2.20 */
Somewhat annoyingly you also need to implement the callbacks for
.domainMigratePrepare3 and .domainMigratePerform3, as we don't
automatically convert non-params usage to the params based
method AFAICT.
Your impl of .domainMigratePerform3 could pack the values into a
virTypedParams array and then call .domainMigratePerform3Params,
or do the reverse.
Yes, without plain(non-params) callbacks we get working only
toURI3
API function and I create a patch not included in this patchset
to make toURI{1,2} work too. I take this approach of converting
parameters and use one common worker function but patch a different
place - API implementaion itself. So I'll include this patch
in next version of the set.
As in this case I need to patch 2 different sets of API implementation
*migrate{N} and *migrateURI{N} I'd rather put direct managed support
to a different patchset. Is it ok?
Regards,
Daniel
3:40 a.m.
New subject: [libvirt] [PATCH v4 2/6] vz: add migration backbone code
On Fri, Sep 04, 2015 at 10:56:52AM +0300, Nikolay Shirokovskiy wrote:
>> @@ -1396,6 +1585,9 @@ static virHypervisorDriver vzDriver =
{
>> .domainBlockStatsFlags = vzDomainBlockStatsFlags, /* 1.2.17 */
>> .domainInterfaceStats = vzDomainInterfaceStats, /* 1.2.17 */
>> .domainMemoryStats = vzDomainMemoryStats, /* 1.2.17 */
>> + .connectSupportsFeature = vzConnectSupportsFeature, /* 1.2.20 */
>> + .domainMigratePrepare3Params = vzDomainMigratePrepare3Params, /* 1.2.20 */
>> + .domainMigratePerform3Params = vzDomainMigratePerform3Params, /* 1.2.20 */
>
> Somewhat annoyingly you also need to implement the callbacks for
> .domainMigratePrepare3 and .domainMigratePerform3, as we don't
> automatically convert non-params usage to the params based
> method AFAICT.
>
> Your impl of .domainMigratePerform3 could pack the values into a
> virTypedParams array and then call .domainMigratePerform3Params,
> or do the reverse.
Yes, without plain(non-params) callbacks we get working only toURI3
API function and I create a patch not included in this patchset
to make toURI{1,2} work too. I take this approach of converting
parameters and use one common worker function but patch a different
place - API implementaion itself. So I'll include this patch
in next version of the set.
As in this case I need to patch 2 different sets of API implementation
*migrate{N} and *migrateURI{N} I'd rather put direct managed support
to a different patchset. Is it ok?
Yeah, that'd be fine as long as we still compile at each step it isn't
a problem if the impl is not final.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
4 a.m.
New subject: [libvirt] [PATCH v4 2/6] vz: add migration backbone code
On 04.09.2015 11:40, Daniel P. Berrange wrote:
On Fri, Sep 04, 2015 at 10:56:52AM +0300, Nikolay Shirokovskiy
wrote:
>>> @@ -1396,6 +1585,9 @@ static virHypervisorDriver vzDriver = {
>>> .domainBlockStatsFlags = vzDomainBlockStatsFlags, /* 1.2.17 */
>>> .domainInterfaceStats = vzDomainInterfaceStats, /* 1.2.17 */
>>> .domainMemoryStats = vzDomainMemoryStats, /* 1.2.17 */
>>> + .connectSupportsFeature = vzConnectSupportsFeature, /* 1.2.20 */
>>> + .domainMigratePrepare3Params = vzDomainMigratePrepare3Params, /* 1.2.20
*/
>>> + .domainMigratePerform3Params = vzDomainMigratePerform3Params, /* 1.2.20
*/
>>
>> Somewhat annoyingly you also need to implement the callbacks for
>> .domainMigratePrepare3 and .domainMigratePerform3, as we don't
>> automatically convert non-params usage to the params based
>> method AFAICT.
>>
>> Your impl of .domainMigratePerform3 could pack the values into a
>> virTypedParams array and then call .domainMigratePerform3Params,
>> or do the reverse.
> Yes, without plain(non-params) callbacks we get working only toURI3
> API function and I create a patch not included in this patchset
> to make toURI{1,2} work too. I take this approach of converting
> parameters and use one common worker function but patch a different
> place - API implementaion itself. So I'll include this patch
> in next version of the set.
>
> As in this case I need to patch 2 different sets of API implementation
> *migrate{N} and *migrateURI{N} I'd rather put direct managed support
> to a different patchset. Is it ok?
Yeah, that'd be fine as long as we still compile at each step it isn't
a problem if the impl is not final.
Ok. Then I'll send patch supporting
toURI{1, 2} in a different patchset as it will
be a new thing that could lead to meaningless ping-ponging of parts
already areed upon.
Regards,
Daniel
7:09 a.m.
New subject: [libvirt] [PATCH v4 3/6] vz: support domain rename on migrate
From: nshirokovskiy(a)virtuozzo.com <nshirokovskiy(a)virtuozzo.com>
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/vz/vz_driver.c | 12 ++++++++++--
src/vz/vz_sdk.c | 16 +++++++++-------
src/vz/vz_sdk.h | 5 ++++-
3 files changed, 23 insertions(+), 10 deletions(-)
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index f3fd599..2760e63 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -1465,7 +1465,9 @@ vzMakeVzUri(const char *connuri_str)
#define VZ_MIGRATION_FLAGS (VIR_MIGRATE_PEER2PEER)
-#define VZ_MIGRATION_PARAMETERS (NULL)
+#define VZ_MIGRATION_PARAMETERS \
+ VIR_MIGRATE_PARAM_DEST_NAME, VIR_TYPED_PARAM_STRING, \
+ NULL
static int
vzDomainMigratePerform3Params(virDomainPtr domain,
@@ -1486,6 +1488,7 @@ vzDomainMigratePerform3Params(virDomainPtr domain,
vzConnPtr privconn = domain->conn->privateData;
char *cookie = NULL;
int cookielen = 0;
+ const char *dname = NULL;
virCheckFlags(VZ_MIGRATION_FLAGS, -1);
@@ -1495,6 +1498,11 @@ vzDomainMigratePerform3Params(virDomainPtr domain,
if (!(vzuri = vzMakeVzUri(dconnuri)))
goto cleanup;
+ if (virTypedParamsGetString(params, nparams,
+ VIR_MIGRATE_PARAM_DEST_NAME,
+ &dname) < 0)
+ goto cleanup;
+
if (!(dom = vzDomObjFromDomain(domain)))
goto cleanup;
@@ -1514,7 +1522,7 @@ vzDomainMigratePerform3Params(virDomainPtr domain,
if (vzParseCookie(cookie, session_uuid) < 0)
goto cleanup;
- if (prlsdkMigrate(dom, vzuri, session_uuid) < 0)
+ if (prlsdkMigrate(dom, vzuri, session_uuid, dname) < 0)
goto cleanup;
virDomainObjListRemove(privconn->domains, dom);
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 783438d..89a2429 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -4064,7 +4064,8 @@ prlsdkGetMemoryStats(virDomainObjPtr dom,
#define PRLSDK_MIGRATION_FLAGS (PSL_HIGH_SECURITY)
int prlsdkMigrate(virDomainObjPtr dom, virURIPtr uri,
- const unsigned char *session_uuid)
+ const unsigned char *session_uuid,
+ const char *dname)
{
int ret = -1;
vzDomObjPtr privdom = dom->privateData;
@@ -4072,12 +4073,13 @@ int prlsdkMigrate(virDomainObjPtr dom, virURIPtr uri,
char uuidstr[VIR_UUID_STRING_BUFLEN + 2];
prlsdkUUIDFormat(session_uuid, uuidstr);
- job = PrlVm_MigrateEx(privdom->sdkdom, uri->server, uri->port, uuidstr,
- "", /* use default dir for migrated instance bundle
*/
- PRLSDK_MIGRATION_FLAGS,
- 0, /* reserved flags */
- PRL_TRUE /* don't ask for confirmations */
- );
+ job = PrlVm_MigrateWithRenameEx(privdom->sdkdom, uri->server, uri->port,
uuidstr,
+ dname == NULL ? "" : dname,
+ "", /* use default dir for migrated
instance bundle */
+ PRLSDK_MIGRATION_FLAGS,
+ 0, /* reserved flags */
+ PRL_TRUE /* don't ask for confirmations */
+ );
if (PRL_FAILED(waitJob(job)))
goto cleanup;
diff --git a/src/vz/vz_sdk.h b/src/vz/vz_sdk.h
index d3f0caf..0aa70b3 100644
--- a/src/vz/vz_sdk.h
+++ b/src/vz/vz_sdk.h
@@ -77,4 +77,7 @@ prlsdkGetVcpuStats(virDomainObjPtr dom, int idx, unsigned long long
*time);
int
prlsdkGetMemoryStats(virDomainObjPtr dom, virDomainMemoryStatPtr stats, unsigned int
nr_stats);
int
-prlsdkMigrate(virDomainObjPtr dom, virURIPtr uri, const char unsigned *session_uuid);
+prlsdkMigrate(virDomainObjPtr dom,
+ virURIPtr uri,
+ const char unsigned *session_uuid,
+ const char *dname);
--
1.7.1
7:09 a.m.
New subject: [libvirt] [PATCH v4 4/6] vz: support migration uri
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/vz/vz_driver.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++---
1 files changed, 54 insertions(+), 4 deletions(-)
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 2760e63..e8b198a 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -1463,10 +1463,53 @@ vzMakeVzUri(const char *connuri_str)
return vzuri;
}
+virURIPtr
+vzParseVzURI(const char *uri_str)
+{
+ virURIPtr uri = NULL;
+ int ret = -1;
+
+ if (!(uri = virURIParse(uri_str)))
+ goto cleanup;
+
+ if (uri->scheme == NULL || uri->server == NULL) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("scheme and host are mandatory vz migration URI:
%s"),
+ uri_str);
+ goto cleanup;
+ }
+
+ if (uri->user != NULL || uri->path != NULL ||
+ uri->query != NULL || uri->fragment != NULL) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("only scheme, host and port are supported in "
+ "vz migration URI: %s"), uri_str);
+ goto cleanup;
+ }
+
+ if (STRNEQ(uri->scheme, "tcp")) {
+ virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
+ _("unsupported scheme %s in migration URI %s"),
+ uri->scheme, uri_str);
+ goto cleanup;
+ }
+
+ ret = 0;
+
+ cleanup:
+ if (ret < 0) {
+ virURIFree(uri);
+ uri = NULL;
+ }
+
+ return uri;
+}
+
#define VZ_MIGRATION_FLAGS (VIR_MIGRATE_PEER2PEER)
#define VZ_MIGRATION_PARAMETERS \
VIR_MIGRATE_PARAM_DEST_NAME, VIR_TYPED_PARAM_STRING, \
+ VIR_MIGRATE_PARAM_URI, VIR_TYPED_PARAM_STRING, \
NULL
static int
@@ -1489,18 +1532,25 @@ vzDomainMigratePerform3Params(virDomainPtr domain,
char *cookie = NULL;
int cookielen = 0;
const char *dname = NULL;
+ const char *miguri = NULL;
virCheckFlags(VZ_MIGRATION_FLAGS, -1);
if (virTypedParamsValidate(params, nparams, VZ_MIGRATION_PARAMETERS) < 0)
goto cleanup;
- if (!(vzuri = vzMakeVzUri(dconnuri)))
- goto cleanup;
-
if (virTypedParamsGetString(params, nparams,
VIR_MIGRATE_PARAM_DEST_NAME,
- &dname) < 0)
+ &dname) < 0 ||
+ virTypedParamsGetString(params, nparams,
+ VIR_MIGRATE_PARAM_URI, &miguri) < 0)
+ goto cleanup;
+
+ if (miguri == NULL)
+ vzuri = vzMakeVzUri(dconnuri);
+ else
+ vzuri = vzParseVzURI(miguri);
+ if (vzuri == NULL)
goto cleanup;
if (!(dom = vzDomObjFromDomain(domain)))
--
1.7.1
11:48 a.m.
New subject: [libvirt] [PATCH v4 4/6] vz: support migration uri
On Wed, Sep 02, 2015 at 03:09:25PM +0300, Nikolay Shirokovskiy wrote:
Signed-off-by: Nikolay Shirokovskiy
<nshirokovskiy(a)virtuozzo.com>
---
src/vz/vz_driver.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++---
1 files changed, 54 insertions(+), 4 deletions(-)
This should really be part of the 2nd patch, since you need to
deal with the migration URI right from start.
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 2760e63..e8b198a 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -1463,10 +1463,53 @@ vzMakeVzUri(const char *connuri_str)
return vzuri;
}
+virURIPtr
+vzParseVzURI(const char *uri_str)
+{
+ virURIPtr uri = NULL;
+ int ret = -1;
+
+ if (!(uri = virURIParse(uri_str)))
+ goto cleanup;
+
+ if (uri->scheme == NULL || uri->server == NULL) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("scheme and host are mandatory vz migration URI:
%s"),
+ uri_str);
+ goto cleanup;
+ }
+
+ if (uri->user != NULL || uri->path != NULL ||
+ uri->query != NULL || uri->fragment != NULL) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("only scheme, host and port are supported in "
+ "vz migration URI: %s"), uri_str);
+ goto cleanup;
+ }
+
+ if (STRNEQ(uri->scheme, "tcp")) {
+ virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
+ _("unsupported scheme %s in migration URI %s"),
+ uri->scheme, uri_str);
+ goto cleanup;
+ }
BTW, we should also fill in the 'migrateTrans' field in the virCapsHost
struct to list the 'tcp' scheme. Nothing much actually uses this in
reality, but it is technical best practice for us to list supported
URIs there
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
7:09 a.m.
New subject: [libvirt] [PATCH v4 5/6] vz: support misc migration options
From: nshirokovskiy(a)virtuozzo.com <nshirokovskiy(a)virtuozzo.com>
Migration API has a lot of options. This patch intention is to provide
support for those options that can be trivially supported and give
estimation for other options support in this commit message.
I. Supported.
1. VIR_MIGRATE_COMPRESSED. Means 'use compression when migration domain
memory'. It is supported but quite uncommon way: vz migration demands that this
option should be set. This is due to vz is hardcoded to moving VMs memory using
compression. So anyone who wants to migrate vz domain should set this option
thus declaring it knows it uses compression.
Why bother? May be just support this option and ignore if it is not set or
don't support at all as we can't change behaviour in this aspect. Well I
believe that this option is, first, inherent to hypervisor implementation as
we have a task of moving domain memory to different place and, second, we have
a tradeoff here between cpu and network resources and some managment should
choose the stratery via this option. If we choose ignoring or unsupporting
implementation than this option has a too vague meaning. Let's go into more
detail.
First if we ignore situation where option is not set than we put user into
fallacy that vz hypervisor don't use compression and thus have lower cpu
usage. Second approach is to not support the option. The main reason not to
follow this way is that 'not supported and not set' is indistinguishable from
'supported and not set' and thus again fool the user.
2. VIR_MIGRATE_LIVE. Means 'reduce domain downtime by suspending it as lately
as possible' which technically means 'migrate as much domain memory as possible
before suspending'. Supported in same manner as VIR_MIGRATE_COMPRESSED as both
vz VMs and CTs are always migrated via live scheme.
One may be fooled by vz sdk flags of migration api: PVMT_HOT_MIGRATION(aka
live) and PVMT_WARM_MIGRATION(aka normal). Current implementation ignore these
flags and always use live migration.
3. VIR_MIGRATE_PERSIST_DEST, VIR_MIGRATE_UNDEFINE_SOURCE. This two comes
together. Vz domain are always persistent so we have to demand that options
VIR_MIGRATE_PERSIST_DEST and VIR_MIGRATE_UNDEFINE_SOURCE are both set (and
this is done just by unsupporting it).
4. VIR_MIGRATE_PAUSED. Means 'don't resume domain on destination'. This is
trivially supported as we have a corresponding option in vz migration.
5. VIR_MIGRATE_OFFLINE. Means 'migrate only XML definition of a domain'. It is
a forcing option that is it is ignored if domain is running and must be set
to migrate stopped domain. Vz implemenation follows this unformal definition
with one exception: non-shared disks will be migrated too. This desicion is on
par with VIR_MIGRATE_NON_SHARED_DISK condideration(see last part of this
notes).
All that said the minimal command to migrate vz domain looks like next:
migrate $DOMAIN $DESTINATION --compressed --live --persistent --undefinesource
Not good. Say if you want to just migrate a domain without further
details you will get error messages until you add these options to
command line. I think there is a lack of notion 'default' behaviour
in all these aspects. If we have it we could just issue:
migrate $DOMAIN $DESTINATION
For vz this would give default compression for example, for qemu - default
no-compression. Then we could have flags --compressed and -no-compressed
and for vz the latter would give unsupported error.
II. Unsupported.
1. VIR_MIGRATE_UNSAFE. Vz disks are always have 'cache=none' set (this
is not reflected in current version of vz driver and will be fixed
soon). So we need not to support this option.
2. VIR_MIGRATE_CHANGE_PROTECTION. Unsupported as we have no appopriate
support from vz sdk. Although we have locks they are advisory and
cant help us.
3. VIR_MIGRATE_TUNNELLED. Means 'use libvirtd to libvirtd connection
to pass hypervisor migration traffic'. Unsupported as not among
vz hypervisor usecases.
4. Direct migration. Which is exposed via *toURI* interface with
VIR_MIGRATE_PEER2PEER flag unset. Means 'migrate without using
libvirtd on the other side'. To support it we should add authN
means to vz driver as mentioned in 'backbone patch' which looks
ugly.
5. VIR_MIGRATE_ABORT_ON_ERROR, VIR_MIGRATE_AUTO_CONVERGE,
VIR_MIGRATE_RDMA_PIN_ALL, VIR_MIGRATE_NON_SHARED_INC,
VIR_MIGRATE_PARAM_DEST_XML, VIR_MIGRATE_PARAM_BANDWIDTH,
VIR_MIGRATE_PARAM_GRAPHICS_URI, VIR_MIGRATE_PARAM_LISTEN_ADDRESS,
VIR_MIGRATE_PARAM_MIGRATE_DISKS.
Without further discussion. They are just not usecases of vz hypevisor.
III. Undecided and thus unsupported.
6. VIR_MIGRATE_NON_SHARED_DISK. The meaning of this option is not clear to me.
Look, if qemu domain has a non-shared disk than it will refuse to migrate. But
after you specify this option it will refuse too. You need to create image file
for the disk on the destination side. Only after that you can migrate.
Unexpectedly existence of this file is enough to migrate without option too. In
this case you will get a domain on the destination with disk image unrelated to
source one and this is in case of live migration! Looks like a bug. Ok, imagine
this is fixed so that migration of non-shared disk is only possible with
actual coping disk to destination. What we get from this option? We get
that you have to specify this option if you want to migrate a domain with
non-shared disk like some forcing option. May be it is a good approach
but it is incompatible with vz. Vz don't demand any user awareness of
migration of non-shared disks. And this case incompatibility can not
be easily resolved as for 'compressed' option as this option depends on
classifying of shared/non-shared for disks which is done inside vz.
vz: implement misc migration options
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
Conflicts:
src/vz/vz_driver.c
---
src/vz/vz_driver.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++--
src/vz/vz_sdk.c | 8 +++-
src/vz/vz_sdk.h | 3 +-
3 files changed, 103 insertions(+), 6 deletions(-)
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index e8b198a..a9baaa7 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -1505,13 +1505,96 @@ vzParseVzURI(const char *uri_str)
return uri;
}
-#define VZ_MIGRATION_FLAGS (VIR_MIGRATE_PEER2PEER)
+#define VZ_MIGRATION_FLAGS \
+ (VIR_MIGRATE_PEER2PEER | \
+ VIR_MIGRATE_OFFLINE | \
+ VIR_MIGRATE_LIVE | \
+ VIR_MIGRATE_COMPRESSED | \
+ VIR_MIGRATE_PERSIST_DEST | \
+ VIR_MIGRATE_UNDEFINE_SOURCE | \
+ VIR_MIGRATE_PAUSED)
#define VZ_MIGRATION_PARAMETERS \
VIR_MIGRATE_PARAM_DEST_NAME, VIR_TYPED_PARAM_STRING, \
VIR_MIGRATE_PARAM_URI, VIR_TYPED_PARAM_STRING, \
NULL
+/* TODO this code should be in common place as these
+ rules follows from options (informal) definitions.
+ Qemu makes some of these checks on begin phase but not all. */
+int vzCheckOfflineFlags(int flags)
+{
+ if (!(flags & VIR_MIGRATE_OFFLINE)) {
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ "%s", _("domain is not running"));
+ return -1;
+ }
+
+ if (flags & VIR_MIGRATE_LIVE) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("live offline migration does not "
+ "make sense"));
+ return -1;
+ }
+
+ if (flags & VIR_MIGRATE_COMPRESSED) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("compressed offline migration does not "
+ "make sense"));
+ return -1;
+ }
+
+ if (flags & VIR_MIGRATE_PAUSED) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("paused offline migration does not "
+ "make sense"));
+ return -1;
+ }
+
+ return 0;
+}
+
+int vzCheckCommonFlags(int flags)
+{
+ virCheckFlags(VZ_MIGRATION_FLAGS, -1);
+
+ if (!(flags & VIR_MIGRATE_PERSIST_DEST)) {
+ virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("flags VIR_MIGRATE_PERSIST_DEST must be set"
+ " for vz migration"));
+ return -1;
+ }
+
+ if (!(flags & VIR_MIGRATE_UNDEFINE_SOURCE)) {
+ virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("flags VIR_MIGRATE_UNDEFINE_SOURCE must be set"
+ " for vz migration"));
+ return -1;
+ }
+
+ return 0;
+}
+
+int vzCheckOnlineFlags(int flags)
+{
+
+ if (!(flags & VIR_MIGRATE_LIVE)) {
+ virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("flags VIR_MIGRATE_LIVE must be set"
+ " for online vz migration"));
+ return -1;
+ }
+
+ if (!(flags & VIR_MIGRATE_COMPRESSED)) {
+ virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("flags VIR_MIGRATE_COMPRESSED must be set"
+ " for online vz migration"));
+ return -1;
+ }
+
+ return 0;
+}
+
static int
vzDomainMigratePerform3Params(virDomainPtr domain,
const char *dconnuri,
@@ -1534,7 +1617,8 @@ vzDomainMigratePerform3Params(virDomainPtr domain,
const char *dname = NULL;
const char *miguri = NULL;
- virCheckFlags(VZ_MIGRATION_FLAGS, -1);
+ if (vzCheckCommonFlags(flags))
+ goto cleanup;
if (virTypedParamsValidate(params, nparams, VZ_MIGRATION_PARAMETERS) < 0)
goto cleanup;
@@ -1556,6 +1640,14 @@ vzDomainMigratePerform3Params(virDomainPtr domain,
if (!(dom = vzDomObjFromDomain(domain)))
goto cleanup;
+ if (virDomainObjIsActive(dom))
+ ret = vzCheckOnlineFlags(flags);
+ else
+ ret = vzCheckOfflineFlags(flags);
+
+ if (ret < 0)
+ goto cleanup;
+
dconn = virConnectOpen(dconnuri);
if (dconn == NULL) {
virReportError(VIR_ERR_OPERATION_FAILED,
@@ -1572,7 +1664,7 @@ vzDomainMigratePerform3Params(virDomainPtr domain,
if (vzParseCookie(cookie, session_uuid) < 0)
goto cleanup;
- if (prlsdkMigrate(dom, vzuri, session_uuid, dname) < 0)
+ if (prlsdkMigrate(dom, vzuri, session_uuid, dname, flags) < 0)
goto cleanup;
virDomainObjListRemove(privconn->domains, dom);
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 89a2429..9a2b5df 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -4065,18 +4065,22 @@ prlsdkGetMemoryStats(virDomainObjPtr dom,
int prlsdkMigrate(virDomainObjPtr dom, virURIPtr uri,
const unsigned char *session_uuid,
- const char *dname)
+ const char *dname, unsigned int flags)
{
int ret = -1;
vzDomObjPtr privdom = dom->privateData;
PRL_HANDLE job = PRL_INVALID_HANDLE;
char uuidstr[VIR_UUID_STRING_BUFLEN + 2];
+ PRL_UINT32 vzflags = PRLSDK_MIGRATION_FLAGS;
+
+ if (flags & VIR_MIGRATE_PAUSED)
+ vzflags |= PVMT_DONT_RESUME_VM;
prlsdkUUIDFormat(session_uuid, uuidstr);
job = PrlVm_MigrateWithRenameEx(privdom->sdkdom, uri->server, uri->port,
uuidstr,
dname == NULL ? "" : dname,
"", /* use default dir for migrated
instance bundle */
- PRLSDK_MIGRATION_FLAGS,
+ vzflags,
0, /* reserved flags */
PRL_TRUE /* don't ask for confirmations */
);
diff --git a/src/vz/vz_sdk.h b/src/vz/vz_sdk.h
index 0aa70b3..5b26b70 100644
--- a/src/vz/vz_sdk.h
+++ b/src/vz/vz_sdk.h
@@ -80,4 +80,5 @@ int
prlsdkMigrate(virDomainObjPtr dom,
virURIPtr uri,
const char unsigned *session_uuid,
- const char *dname);
+ const char *dname,
+ unsigned int flags);
--
1.7.1
12:04 p.m.
New subject: [libvirt] [PATCH v4 5/6] vz: support misc migration options
On Wed, Sep 02, 2015 at 03:09:26PM +0300, Nikolay Shirokovskiy wrote:
From: nshirokovskiy(a)virtuozzo.com
<nshirokovskiy(a)virtuozzo.com>
Migration API has a lot of options. This patch intention is to provide
support for those options that can be trivially supported and give
estimation for other options support in this commit message.
I. Supported.
1. VIR_MIGRATE_COMPRESSED. Means 'use compression when migration domain
memory'. It is supported but quite uncommon way: vz migration demands that this
option should be set. This is due to vz is hardcoded to moving VMs memory using
compression. So anyone who wants to migrate vz domain should set this option
thus declaring it knows it uses compression.
Why bother? May be just support this option and ignore if it is not set or
don't support at all as we can't change behaviour in this aspect. Well I
believe that this option is, first, inherent to hypervisor implementation as
we have a task of moving domain memory to different place and, second, we have
a tradeoff here between cpu and network resources and some managment should
choose the stratery via this option. If we choose ignoring or unsupporting
implementation than this option has a too vague meaning. Let's go into more
detail.
First if we ignore situation where option is not set than we put user into
fallacy that vz hypervisor don't use compression and thus have lower cpu
usage. Second approach is to not support the option. The main reason not to
follow this way is that 'not supported and not set' is indistinguishable from
'supported and not set' and thus again fool the user.
I think it is reasonable to say that the absence of 'VIR_MIGRATE_COMPRESSED'
does not mean compression should be turned off.
IOW, if VZ does compression by default, that's fine. You don't need to
raise an error if VIR_MIGRATE_COMPRESSED is not passed.
2. VIR_MIGRATE_LIVE. Means 'reduce domain downtime by suspending
it as lately
as possible' which technically means 'migrate as much domain memory as possible
before suspending'. Supported in same manner as VIR_MIGRATE_COMPRESSED as both
vz VMs and CTs are always migrated via live scheme.
One may be fooled by vz sdk flags of migration api: PVMT_HOT_MIGRATION(aka
live) and PVMT_WARM_MIGRATION(aka normal). Current implementation ignore these
flags and always use live migration.
Actually that's not quite right interpretation. VIR_MIGRATE_LIVE
means that the guest CPUs should remain running throughout migration.
ie, if VIR_MIGRATE_LIVE is *not* provided by the caller, then the
guest CPUs /must/ be paused while migration takes place. ie the
migration is non-live / coma migration. If VZ can't do this
automatically as part of its migration API, you can fake it by
just pausing the guest before hand & unpausing after.
Or if you don't wish to support coma-migration, you could raise
an error if VIR_MIGRATE_LIVE is not specified by caller.
3. VIR_MIGRATE_PERSIST_DEST, VIR_MIGRATE_UNDEFINE_SOURCE. This two
comes
together. Vz domain are always persistent so we have to demand that options
VIR_MIGRATE_PERSIST_DEST and VIR_MIGRATE_UNDEFINE_SOURCE are both set (and
this is done just by unsupporting it).
This is a tricky one. It would be nice if we could have just "done the
right thing" by default with the migrate API, since the default behaviour
is kind of stupid honestly, but we have to live with backwards compatible
API semantics now :(
To avoid forcing people to always specify these flags, could you "undo"
by VZ does by default ?
ie, if VIR_MIGRATE_PERSIST_DEST is *not* passed, then once migration
has completed, you could use the domainMigrateFinish step to delete
the config that VZ just created.
Similarly if VIR_MIGRATE_UNDEFINE_SOURCE is *not* passed, then once
migration completed, you could use the domainMigrateConfirm step
to re-create the config on the src that VZ just deleted.
4. VIR_MIGRATE_PAUSED. Means 'don't resume domain on
destination'. This is
trivially supported as we have a corresponding option in vz migration.
5. VIR_MIGRATE_OFFLINE. Means 'migrate only XML definition of a domain'. It is
a forcing option that is it is ignored if domain is running and must be set
to migrate stopped domain. Vz implemenation follows this unformal definition
with one exception: non-shared disks will be migrated too. This desicion is on
par with VIR_MIGRATE_NON_SHARED_DISK condideration(see last part of this
notes).
Ok
All that said the minimal command to migrate vz domain looks like
next:
migrate $DOMAIN $DESTINATION --compressed --live --persistent --undefinesource
Pretty much all apps should be passing those regardless. eg OpenStack
should definitely pass all those flags by default. virsh is kind of
a dumb tool so it forces users to decide exactly what they want.
Not good. Say if you want to just migrate a domain without further
details you will get error messages until you add these options to
command line. I think there is a lack of notion 'default' behaviour
in all these aspects. If we have it we could just issue:
migrate $DOMAIN $DESTINATION
I think we can get to that, if you're able to take the approachs I
suggest above to workaround/override VZ defaults.
For vz this would give default compression for example, for qemu -
default
no-compression. Then we could have flags --compressed and -no-compressed
and for vz the latter would give unsupported error.
II. Unsupported.
1. VIR_MIGRATE_UNSAFE. Vz disks are always have 'cache=none' set (this
is not reflected in current version of vz driver and will be fixed
soon). So we need not to support this option.
Ok, that's fine.
2. VIR_MIGRATE_CHANGE_PROTECTION. Unsupported as we have no
appopriate
support from vz sdk. Although we have locks they are advisory and
cant help us.
Also fine.
3. VIR_MIGRATE_TUNNELLED. Means 'use libvirtd to libvirtd
connection
to pass hypervisor migration traffic'. Unsupported as not among
vz hypervisor usecases.
That's fine, particularly if VZ provides encryption for its
migration data stream anyway. The tunnnelled mode is just a
hack to deal with fact that QEMU migration stream is clear
text on the wire :-(
4. Direct migration. Which is exposed via *toURI* interface with
VIR_MIGRATE_PEER2PEER flag unset. Means 'migrate without using
libvirtd on the other side'. To support it we should add authN
means to vz driver as mentioned in 'backbone patch' which looks
ugly.
I don't think you need to worry about supporting that.
You can support the traditional migrate those ie the non-ToURI
migrate method without P2P set.
5. VIR_MIGRATE_ABORT_ON_ERROR, VIR_MIGRATE_AUTO_CONVERGE,
VIR_MIGRATE_RDMA_PIN_ALL, VIR_MIGRATE_NON_SHARED_INC,
VIR_MIGRATE_PARAM_DEST_XML, VIR_MIGRATE_PARAM_BANDWIDTH,
VIR_MIGRATE_PARAM_GRAPHICS_URI, VIR_MIGRATE_PARAM_LISTEN_ADDRESS,
VIR_MIGRATE_PARAM_MIGRATE_DISKS.
Without further discussion. They are just not usecases of vz hypevisor.
Also fine.
III. Undecided and thus unsupported.
6. VIR_MIGRATE_NON_SHARED_DISK. The meaning of this option is not clear to me.
Look, if qemu domain has a non-shared disk than it will refuse to migrate. But
after you specify this option it will refuse too. You need to create image file
for the disk on the destination side. Only after that you can migrate.
Yep, kind of silly QEMU semantics here.
Unexpectedly existence of this file is enough to migrate without
option too. In
this case you will get a domain on the destination with disk image unrelated to
source one and this is in case of live migration! Looks like a bug. Ok, imagine
If the file exists on the dst, it is assumed the file is exposed via
NFS or some other sensible shared filesystem. Libvirt probably ought
to actually validate that the FS is truely shared and raise an error
if not.
this is fixed so that migration of non-shared disk is only possible
with
actual coping disk to destination. What we get from this option? We get
that you have to specify this option if you want to migrate a domain with
non-shared disk like some forcing option. May be it is a good approach
but it is incompatible with vz. Vz don't demand any user awareness of
migration of non-shared disks. And this case incompatibility can not
be easily resolved as for 'compressed' option as this option depends on
classifying of shared/non-shared for disks which is done inside vz.
vz: implement misc migration options
So are you saying that if the disk is not on shared filesystem,
then VZ will just "do the right thing" and copy the disk data
across to the host ? If so, then I agree we can deal with this
the same way as the compressed flag. ie just ignore it and let
vz do the right thing automatically and not require the user to
explicitly pass the SHARED flag.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
2:42 a.m.
New subject: [libvirt] [PATCH v4 5/6] vz: support misc migration options
On 03.09.2015 20:04, Daniel P. Berrange wrote:
On Wed, Sep 02, 2015 at 03:09:26PM +0300, Nikolay Shirokovskiy
wrote:
> From: nshirokovskiy(a)virtuozzo.com <nshirokovskiy(a)virtuozzo.com>
>
> Migration API has a lot of options. This patch intention is to provide
> support for those options that can be trivially supported and give
> estimation for other options support in this commit message.
>
> I. Supported.
>
> 1. VIR_MIGRATE_COMPRESSED. Means 'use compression when migration domain
> memory'. It is supported but quite uncommon way: vz migration demands that this
> option should be set. This is due to vz is hardcoded to moving VMs memory using
> compression. So anyone who wants to migrate vz domain should set this option
> thus declaring it knows it uses compression.
>
> Why bother? May be just support this option and ignore if it is not set or
> don't support at all as we can't change behaviour in this aspect. Well I
> believe that this option is, first, inherent to hypervisor implementation as
> we have a task of moving domain memory to different place and, second, we have
> a tradeoff here between cpu and network resources and some managment should
> choose the stratery via this option. If we choose ignoring or unsupporting
> implementation than this option has a too vague meaning. Let's go into more
> detail.
>
> First if we ignore situation where option is not set than we put user into
> fallacy that vz hypervisor don't use compression and thus have lower cpu
> usage. Second approach is to not support the option. The main reason not to
> follow this way is that 'not supported and not set' is indistinguishable
from
> 'supported and not set' and thus again fool the user.
I think it is reasonable to say that the absence of 'VIR_MIGRATE_COMPRESSED'
does not mean compression should be turned off.
For me saing so is possible only if
impact of compressing/not compressing
is rather insignificant. Otherwise it would be strange that turning on/off
this option does not change anything. For example for live migration
option we can't afford go this way.
So for clarity i would rather raise an error. One day may be vz
will turn to supporting this option too or default will change.
IOW, if VZ does compression by default, that's fine. You don't need to
raise an error if VIR_MIGRATE_COMPRESSED is not passed.
> 2. VIR_MIGRATE_LIVE. Means 'reduce domain downtime by suspending it as lately
> as possible' which technically means 'migrate as much domain memory as
possible
> before suspending'. Supported in same manner as VIR_MIGRATE_COMPRESSED as both
> vz VMs and CTs are always migrated via live scheme.
>
> One may be fooled by vz sdk flags of migration api: PVMT_HOT_MIGRATION(aka
> live) and PVMT_WARM_MIGRATION(aka normal). Current implementation ignore these
> flags and always use live migration.
Actually that's not quite right interpretation. VIR_MIGRATE_LIVE
means that the guest CPUs should remain running throughout migration.
ie, if VIR_MIGRATE_LIVE is *not* provided by the caller, then the
guest CPUs /must/ be paused while migration takes place. ie the
We are talking
about the same thing.
migration is non-live / coma migration. If VZ can't do this
automatically as part of its migration API, you can fake it by
just pausing the guest before hand & unpausing after.
Good idea, i really could
go this way and support the option.
Or if you don't wish to support coma-migration, you could raise
an error if VIR_MIGRATE_LIVE is not specified by caller.
> 3. VIR_MIGRATE_PERSIST_DEST, VIR_MIGRATE_UNDEFINE_SOURCE. This two comes
> together. Vz domain are always persistent so we have to demand that options
> VIR_MIGRATE_PERSIST_DEST and VIR_MIGRATE_UNDEFINE_SOURCE are both set (and
> this is done just by unsupporting it).
This is a tricky one. It would be nice if we could have just "done the
right thing" by default with the migrate API, since the default behaviour
is kind of stupid honestly, but we have to live with backwards compatible
API semantics now :(
To avoid forcing people to always specify these flags, could you "undo"
by VZ does by default ?
ie, if VIR_MIGRATE_PERSIST_DEST is *not* passed, then once migration
has completed, you could use the domainMigrateFinish step to delete
the config that VZ just created.
This is *not* possible. Vz domains are always
persistent. Technically
vz has it own domain configuration file and we convert it to libvirt
xml when asked.
Similarly if VIR_MIGRATE_UNDEFINE_SOURCE is *not* passed, then once
migration completed, you could use the domainMigrateConfirm step
to re-create the config on the src that VZ just deleted.
This is possible.
Eventually this means that we just need to
define domain with the same configuration as migrated one
right after the migration.
So as one option is possible and the other is not and they are somewhat
paired may be not bother? Or should we take the ignore path as
for compressed so to finish *no-demanding options* strategy?
> 4. VIR_MIGRATE_PAUSED. Means 'don't resume domain on destination'. This
is
> trivially supported as we have a corresponding option in vz migration.
>
> 5. VIR_MIGRATE_OFFLINE. Means 'migrate only XML definition of a domain'. It
is
> a forcing option that is it is ignored if domain is running and must be set
> to migrate stopped domain. Vz implemenation follows this unformal definition
> with one exception: non-shared disks will be migrated too. This desicion is on
> par with VIR_MIGRATE_NON_SHARED_DISK condideration(see last part of this
> notes).
Ok
> All that said the minimal command to migrate vz domain looks like next:
>
> migrate $DOMAIN $DESTINATION --compressed --live --persistent --undefinesource
Pretty much all apps should be passing those regardless. eg OpenStack
should definitely pass all those flags by default. virsh is kind of
a dumb tool so it forces users to decide exactly what they want.
>
> Not good. Say if you want to just migrate a domain without further
> details you will get error messages until you add these options to
> command line. I think there is a lack of notion 'default' behaviour
> in all these aspects. If we have it we could just issue:
>
> migrate $DOMAIN $DESTINATION
I think we can get to that, if you're able to take the approachs I
suggest above to workaround/override VZ defaults.
> For vz this would give default compression for example, for qemu - default
> no-compression. Then we could have flags --compressed and -no-compressed
> and for vz the latter would give unsupported error.
>
> II. Unsupported.
>
> 1. VIR_MIGRATE_UNSAFE. Vz disks are always have 'cache=none' set (this
> is not reflected in current version of vz driver and will be fixed
> soon). So we need not to support this option.
Ok, that's fine.
> 2. VIR_MIGRATE_CHANGE_PROTECTION. Unsupported as we have no appopriate
> support from vz sdk. Although we have locks they are advisory and
> cant help us.
Also fine.
> 3. VIR_MIGRATE_TUNNELLED. Means 'use libvirtd to libvirtd connection
> to pass hypervisor migration traffic'. Unsupported as not among
> vz hypervisor usecases.
That's fine, particularly if VZ provides encryption for its
migration data stream anyway. The tunnnelled mode is just a
hack to deal with fact that QEMU migration stream is clear
text on the wire :-(
> 4. Direct migration. Which is exposed via *toURI* interface with
> VIR_MIGRATE_PEER2PEER flag unset. Means 'migrate without using
> libvirtd on the other side'. To support it we should add authN
> means to vz driver as mentioned in 'backbone patch' which looks
> ugly.
I don't think you need to worry about supporting that.
You can support the traditional migrate those ie the non-ToURI
migrate method without P2P set.
>
> 5. VIR_MIGRATE_ABORT_ON_ERROR, VIR_MIGRATE_AUTO_CONVERGE,
> VIR_MIGRATE_RDMA_PIN_ALL, VIR_MIGRATE_NON_SHARED_INC,
> VIR_MIGRATE_PARAM_DEST_XML, VIR_MIGRATE_PARAM_BANDWIDTH,
> VIR_MIGRATE_PARAM_GRAPHICS_URI, VIR_MIGRATE_PARAM_LISTEN_ADDRESS,
> VIR_MIGRATE_PARAM_MIGRATE_DISKS.
> Without further discussion. They are just not usecases of vz hypevisor.
Also fine.
>
> III. Undecided and thus unsupported.
>
> 6. VIR_MIGRATE_NON_SHARED_DISK. The meaning of this option is not clear to me.
> Look, if qemu domain has a non-shared disk than it will refuse to migrate. But
> after you specify this option it will refuse too. You need to create image file
> for the disk on the destination side. Only after that you can migrate.
Yep, kind of silly QEMU semantics here.
> Unexpectedly existence of this file is enough to migrate without option too. In
> this case you will get a domain on the destination with disk image unrelated to
> source one and this is in case of live migration! Looks like a bug. Ok, imagine
If the file exists on the dst, it is assumed the file is exposed via
NFS or some other sensible shared filesystem. Libvirt probably ought
to actually validate that the FS is truely shared and raise an error
if not.
> this is fixed so that migration of non-shared disk is only possible with
> actual coping disk to destination. What we get from this option? We get
> that you have to specify this option if you want to migrate a domain with
> non-shared disk like some forcing option. May be it is a good approach
> but it is incompatible with vz. Vz don't demand any user awareness of
> migration of non-shared disks. And this case incompatibility can not
> be easily resolved as for 'compressed' option as this option depends on
> classifying of shared/non-shared for disks which is done inside vz.
> vz: implement misc migration options
So are you saying that if the disk is not on shared filesystem,
then VZ will just "do the right thing" and copy the disk data
across to the host ? If so, then I agree we can deal with this
the same way as the compressed flag. ie just ignore it and let
vz do the right thing automatically and not require the user to
explicitly pass the SHARED flag.
Agree if we will finally go the ignore path for 'compressed' option
(and 'persist dest' too).
Regards,
Daniel
3:44 a.m.
New subject: [libvirt] [PATCH v4 5/6] vz: support misc migration options
On Fri, Sep 04, 2015 at 10:42:00AM +0300, Nikolay Shirokovskiy wrote:
On 03.09.2015 20:04, Daniel P. Berrange wrote:
> On Wed, Sep 02, 2015 at 03:09:26PM +0300, Nikolay Shirokovskiy wrote:
>> From: nshirokovskiy(a)virtuozzo.com <nshirokovskiy(a)virtuozzo.com>
>>
>> Migration API has a lot of options. This patch intention is to provide
>> support for those options that can be trivially supported and give
>> estimation for other options support in this commit message.
>>
>> I. Supported.
>>
>> 1. VIR_MIGRATE_COMPRESSED. Means 'use compression when migration domain
>> memory'. It is supported but quite uncommon way: vz migration demands that
this
>> option should be set. This is due to vz is hardcoded to moving VMs memory using
>> compression. So anyone who wants to migrate vz domain should set this option
>> thus declaring it knows it uses compression.
>>
>> Why bother? May be just support this option and ignore if it is not set or
>> don't support at all as we can't change behaviour in this aspect. Well
I
>> believe that this option is, first, inherent to hypervisor implementation as
>> we have a task of moving domain memory to different place and, second, we have
>> a tradeoff here between cpu and network resources and some managment should
>> choose the stratery via this option. If we choose ignoring or unsupporting
>> implementation than this option has a too vague meaning. Let's go into more
>> detail.
>>
>> First if we ignore situation where option is not set than we put user into
>> fallacy that vz hypervisor don't use compression and thus have lower cpu
>> usage. Second approach is to not support the option. The main reason not to
>> follow this way is that 'not supported and not set' is indistinguishable
from
>> 'supported and not set' and thus again fool the user.
>
> I think it is reasonable to say that the absence of
'VIR_MIGRATE_COMPRESSED'
> does not mean compression should be turned off.
For me saing so is possible only if impact of compressing/not compressing
is rather insignificant. Otherwise it would be strange that turning on/off
this option does not change anything. For example for live migration
option we can't afford go this way.
So for clarity i would rather raise an error. One day may be vz
will turn to supporting this option too or default will change.
Sure, if you prefer to raise an error that's fine with me.. It just
means users will need to always pass --compressed to virsh, but that
probably isn't a big deal.
>> 3. VIR_MIGRATE_PERSIST_DEST, VIR_MIGRATE_UNDEFINE_SOURCE.
This two comes
>> together. Vz domain are always persistent so we have to demand that options
>> VIR_MIGRATE_PERSIST_DEST and VIR_MIGRATE_UNDEFINE_SOURCE are both set (and
>> this is done just by unsupporting it).
>
> This is a tricky one. It would be nice if we could have just "done the
> right thing" by default with the migrate API, since the default behaviour
> is kind of stupid honestly, but we have to live with backwards compatible
> API semantics now :(
>
> To avoid forcing people to always specify these flags, could you "undo"
> by VZ does by default ?
>
> ie, if VIR_MIGRATE_PERSIST_DEST is *not* passed, then once migration
> has completed, you could use the domainMigrateFinish step to delete
> the config that VZ just created.
This is *not* possible. Vz domains are always persistent. Technically
vz has it own domain configuration file and we convert it to libvirt
xml when asked.
Oh right, I understand now.
> Similarly if VIR_MIGRATE_UNDEFINE_SOURCE is *not* passed, then
once
> migration completed, you could use the domainMigrateConfirm step
> to re-create the config on the src that VZ just deleted.
This is possible. Eventually this means that we just need to
define domain with the same configuration as migrated one
right after the migration.
So as one option is possible and the other is not and they are somewhat
paired may be not bother? Or should we take the ignore path as
for compressed so to finish *no-demanding options* strategy?
I don't mind which way you choose really. I think allowing the user
to not pass UNDEFINE_SOURCE is fairly low priority, since most really
/want/ to use UNDEFINE_SOURCE. It only really causes pain for people
using the low level virsh tool. For serious apps like OpenStack the
default VZ behaviour is what they want all the time.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
3:57 a.m.
New subject: [libvirt] [PATCH v4 5/6] vz: support misc migration options
On 04.09.2015 11:44, Daniel P. Berrange wrote:
On Fri, Sep 04, 2015 at 10:42:00AM +0300, Nikolay Shirokovskiy
wrote:
>
>
> On 03.09.2015 20:04, Daniel P. Berrange wrote:
>> On Wed, Sep 02, 2015 at 03:09:26PM +0300, Nikolay Shirokovskiy wrote:
>>> From: nshirokovskiy(a)virtuozzo.com <nshirokovskiy(a)virtuozzo.com>
>>>
>>> Migration API has a lot of options. This patch intention is to provide
>>> support for those options that can be trivially supported and give
>>> estimation for other options support in this commit message.
>>>
>>> I. Supported.
>>>
>>> 1. VIR_MIGRATE_COMPRESSED. Means 'use compression when migration domain
>>> memory'. It is supported but quite uncommon way: vz migration demands
that this
>>> option should be set. This is due to vz is hardcoded to moving VMs memory
using
>>> compression. So anyone who wants to migrate vz domain should set this option
>>> thus declaring it knows it uses compression.
>>>
>>> Why bother? May be just support this option and ignore if it is not set or
>>> don't support at all as we can't change behaviour in this aspect.
Well I
>>> believe that this option is, first, inherent to hypervisor implementation as
>>> we have a task of moving domain memory to different place and, second, we
have
>>> a tradeoff here between cpu and network resources and some managment should
>>> choose the stratery via this option. If we choose ignoring or unsupporting
>>> implementation than this option has a too vague meaning. Let's go into
more
>>> detail.
>>>
>>> First if we ignore situation where option is not set than we put user into
>>> fallacy that vz hypervisor don't use compression and thus have lower cpu
>>> usage. Second approach is to not support the option. The main reason not to
>>> follow this way is that 'not supported and not set' is
indistinguishable from
>>> 'supported and not set' and thus again fool the user.
>>
>> I think it is reasonable to say that the absence of
'VIR_MIGRATE_COMPRESSED'
>> does not mean compression should be turned off.
> For me saing so is possible only if impact of compressing/not compressing
> is rather insignificant. Otherwise it would be strange that turning on/off
> this option does not change anything. For example for live migration
> option we can't afford go this way.
>
> So for clarity i would rather raise an error. One day may be vz
> will turn to supporting this option too or default will change.
Sure, if you prefer to raise an error that's fine with me.. It just
means users will need to always pass --compressed to virsh, but that
probably isn't a big deal.
>>> 3. VIR_MIGRATE_PERSIST_DEST, VIR_MIGRATE_UNDEFINE_SOURCE. This two comes
>>> together. Vz domain are always persistent so we have to demand that options
>>> VIR_MIGRATE_PERSIST_DEST and VIR_MIGRATE_UNDEFINE_SOURCE are both set (and
>>> this is done just by unsupporting it).
>>
>> This is a tricky one. It would be nice if we could have just "done the
>> right thing" by default with the migrate API, since the default behaviour
>> is kind of stupid honestly, but we have to live with backwards compatible
>> API semantics now :(
>>
>> To avoid forcing people to always specify these flags, could you
"undo"
>> by VZ does by default ?
>>
>> ie, if VIR_MIGRATE_PERSIST_DEST is *not* passed, then once migration
>> has completed, you could use the domainMigrateFinish step to delete
>> the config that VZ just created.
> This is *not* possible. Vz domains are always persistent. Technically
> vz has it own domain configuration file and we convert it to libvirt
> xml when asked.
Oh right, I understand now.
>> Similarly if VIR_MIGRATE_UNDEFINE_SOURCE is *not* passed, then once
>> migration completed, you could use the domainMigrateConfirm step
>> to re-create the config on the src that VZ just deleted.
> This is possible. Eventually this means that we just need to
> define domain with the same configuration as migrated one
> right after the migration.
>
> So as one option is possible and the other is not and they are somewhat
> paired may be not bother? Or should we take the ignore path as
> for compressed so to finish *no-demanding options* strategy?
I don't mind which way you choose really. I think allowing the user
to not pass UNDEFINE_SOURCE is fairly low priority, since most really
/want/ to use UNDEFINE_SOURCE. It only really causes pain for people
using the low level virsh tool. For serious apps like OpenStack the
default VZ behaviour is what they want all the time.
After all that said I'll keep this patch unchanged then. The only
thing that can be done is the support of non-live migration, but
as it is an elaborated I'd prefer to delay it to the moment when there
will be a need to do it.
Regards,
Daniel
7:09 a.m.
New subject: [libvirt] [PATCH v4 6/6] vz: cleanup: define vz format of uuids
vz puts uuids into curly braces. Simply introduce new contstant to reflect this
and get rid of magic +2 in code.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/vz/vz_sdk.c | 12 ++++++------
src/vz/vz_utils.h | 2 ++
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 9a2b5df..bafc6e4 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -236,7 +236,7 @@ prlsdkConnect(vzConnPtr privconn)
PRL_HANDLE job = PRL_INVALID_HANDLE;
PRL_HANDLE result = PRL_INVALID_HANDLE;
PRL_HANDLE response = PRL_INVALID_HANDLE;
- char session_uuid[VIR_UUID_STRING_BUFLEN + 2];
+ char session_uuid[VZ_UUID_STRING_BUFLEN];
PRL_UINT32 buflen = ARRAY_CARDINALITY(session_uuid);
pret = PrlSrv_Create(&privconn->server);
@@ -316,7 +316,7 @@ prlsdkUUIDFormat(const unsigned char *uuid, char *uuidstr)
static PRL_HANDLE
prlsdkSdkDomainLookupByUUID(vzConnPtr privconn, const unsigned char *uuid)
{
- char uuidstr[VIR_UUID_STRING_BUFLEN + 2];
+ char uuidstr[VZ_UUID_STRING_BUFLEN];
PRL_HANDLE sdkdom = PRL_INVALID_HANDLE;
prlsdkUUIDFormat(uuid, uuidstr);
@@ -365,7 +365,7 @@ prlsdkGetDomainIds(PRL_HANDLE sdkdom,
char **name,
unsigned char *uuid)
{
- char uuidstr[VIR_UUID_STRING_BUFLEN + 2];
+ char uuidstr[VZ_UUID_STRING_BUFLEN];
PRL_UINT32 len;
PRL_RESULT pret;
@@ -1722,7 +1722,7 @@ prlsdkEventsHandler(PRL_HANDLE prlEvent, PRL_VOID_PTR opaque)
vzConnPtr privconn = opaque;
PRL_RESULT pret = PRL_ERR_FAILURE;
PRL_HANDLE_TYPE handleType;
- char uuidstr[VIR_UUID_STRING_BUFLEN + 2];
+ char uuidstr[VZ_UUID_STRING_BUFLEN];
unsigned char uuid[VIR_UUID_BUFLEN];
PRL_UINT32 bufsize = ARRAY_CARDINALITY(uuidstr);
PRL_EVENT_TYPE prlEventType;
@@ -3480,7 +3480,7 @@ prlsdkDoApplyConfig(virConnectPtr conn,
{
PRL_RESULT pret;
size_t i;
- char uuidstr[VIR_UUID_STRING_BUFLEN + 2];
+ char uuidstr[VZ_UUID_STRING_BUFLEN];
bool needBoot = true;
char *mask = NULL;
@@ -4070,7 +4070,7 @@ int prlsdkMigrate(virDomainObjPtr dom, virURIPtr uri,
int ret = -1;
vzDomObjPtr privdom = dom->privateData;
PRL_HANDLE job = PRL_INVALID_HANDLE;
- char uuidstr[VIR_UUID_STRING_BUFLEN + 2];
+ char uuidstr[VZ_UUID_STRING_BUFLEN];
PRL_UINT32 vzflags = PRLSDK_MIGRATION_FLAGS;
if (flags & VIR_MIGRATE_PAUSED)
diff --git a/src/vz/vz_utils.h b/src/vz/vz_utils.h
index fe54b25..2a59426 100644
--- a/src/vz/vz_utils.h
+++ b/src/vz/vz_utils.h
@@ -55,6 +55,8 @@
# define PARALLELS_REQUIRED_BRIDGED_NETWORK "Bridged"
# define PARALLELS_BRIDGED_NETWORK_TYPE "bridged"
+# define VZ_UUID_STRING_BUFLEN (VIR_UUID_STRING_BUFLEN + 2)
+
struct _vzConn {
virMutex lock;
--
1.7.1
3401
days inactive
3403
days old
16 comments
3 participants
participants (3)
-
Daniel P. Berrange -
Nikolay Shirokovskiy -
Nikolay Shirokovskiy